On Wed, May 18, 2011 at 09:27:56PM +0100, Andrew Cooper wrote:
>
>
> On 18/05/11 19:49, Konrad Rzeszutek Wilk wrote:
> >On Wed, May 18, 2011 at 07:08:14PM +0100, Andrew Cooper wrote:
> >>Xen does not store the boot local apic state which leads to problems
> >>when shutting down for a kexec jump. This patch records the boot
> >>state so we can return to the boot state when kexec'ing.
> >>
> >>This is per CPU because all 3 bioses on the boxes I have tested dont
> >>enabled all local apics on boot. As a result, we have to return to
> >>the bios state so the ACPI tables match up with the hardware state
> >>for the booting kernel.
> >Which ACPI table requires this?
> Cant remember offhand but linux (2.6.32) was doing finger pointing
> at the multi-processor tables
MP tables? Those don't get used when ACPI is used..
> >>Signed-off-by: Andrew Cooper<andrew.cooper3@xxxxxxxxxx>
> >>
> >>diff -r f531ed84b066 -r 62a8ce6595ad xen/arch/x86/apic.c
> >>--- a/xen/arch/x86/apic.c Tue May 17 17:32:19 2011 +0100
> >>+++ b/xen/arch/x86/apic.c Wed May 18 19:00:13 2011 +0100
> >>@@ -74,6 +74,8 @@ u8 __read_mostly apic_verbosity;
> >> static bool_t __initdata opt_x2apic = 1;
> >> boolean_param("x2apic", opt_x2apic);
> >>
> >>+DEFINE_PER_CPU_READ_MOSTLY(enum apic_mode, apic_boot_mode) =
> >>APIC_MODE_INVALID;
> >>+
> >> bool_t __read_mostly x2apic_enabled = 0;
> >> bool_t __read_mostly directed_eoi_enabled = 0;
> >>
> >>@@ -1437,6 +1439,41 @@ int __init APIC_init_uniprocessor (void)
> >> return 0;
> >> }
> >>
> >>+/* Needs to be called once per CPU during startup. It records the state
> >>the BIOS
> >>+ * leaves the local APIC so we can tare back down upon shutdown/crash
> >tare?
> I fail at spelling - I meant "tear down"
> >>+ */
> >>+void __init record_boot_APIC_mode(void)
> >>+{
> >>+ enum apic_mode this_apic_mode;
> >>+ u64 msr_contents;
> >>+
> >>+ this_apic_mode = APIC_MODE_INVALID;
> >>+
> >>+ /* Sanity check - we should only ever run once */
> >>+ BUG_ON( APIC_MODE_INVALID != this_cpu(apic_boot_mode) );
> >>+
> >>+ rdmsrl(MSR_IA32_APICBASE, msr_contents);
> >>+
> >>+ /* Reading EXTD bit from the MSR is only valid if CPUID says so, else
> >>reserved */
> >>+ if ( cpu_has(¤t_cpu_data, X86_FEATURE_X2APIC)
> >>+&& (msr_contents& MSR_IA32_APICBASE_EXTD) )
> >>+ this_apic_mode = APIC_MODE_X2APIC;
> >>+ else
> >>+ {
> >>+ /* EN bit should always be valid as long as we can read the MSR
> >>+ * Can anyone confirm this?
> >Email Vivek Goyal. He is the kexec/kdump maintainer.
> He doesn't appear in the maintainers file, and I don't have an email
Hmm..This is what I see in the MAINTAINERS file:
KDUMP
M: Vivek Goyal <vgoyal@xxxxxxxxxx>
M: Haren Myneni <hbabu@xxxxxxxxxx>
L: kexec@xxxxxxxxxxxxxxxxxxx
W: http://lse.sourceforge.net/kdump/
S: Maintained
F: Documentation/kdump/
> address.
http://lmgtfy.com/?q=Vivek+Goyal+kexec
> >>+ */
> >>+ if ( msr_contents& MSR_IA32_APICBASE_ENABLE )
> >>+ this_apic_mode = APIC_MODE_XAPIC;
> >>+ else
> >>+ this_apic_mode = APIC_MODE_DISABLED;
> >>+ }
> >>+
> >>+ this_cpu(apic_boot_mode) = this_apic_mode;
> >>+ apic_printk(APIC_DEBUG, "APIC boot state is %d on core #%d\n",
> >>+ this_apic_mode, smp_processor_id());
> >This begs of a function to convert those enums to strings..
> True - this was left over from my debugging - I shall fix that tomorrow.
Ok.
> >>+}
> >>+
> >> void check_for_unexpected_msi(unsigned int vector)
> >> {
> >> unsigned long v = apic_read(APIC_ISR + ((vector& ~0x1f)>> 1));
> >>diff -r f531ed84b066 -r 62a8ce6595ad xen/arch/x86/genapic/probe.c
> >>--- a/xen/arch/x86/genapic/probe.c Tue May 17 17:32:19 2011 +0100
> >>+++ b/xen/arch/x86/genapic/probe.c Wed May 18 19:00:13 2011 +0100
> >>@@ -60,6 +60,8 @@ void __init generic_apic_probe(void)
> >> {
> >> int i, changed;
> >>
> >>+ record_boot_APIC_mode();
> >>+
> >The spacing looks odd.
> Does it? Looks fine for me - I have been following the 1 tab to 4
> spaces convention in half the codebase.
Could be my mailer..
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|