|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] kexec: disable iommu and x2apic before jumping into the kdump kernel
>>> On 13.05.11 at 14:47, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> This is a massive hack but is better than the code currently in place.
> Some functions have been duped into a crash_* version to separate them
> from the regular shutdown path, where disabling these subsystems is
> irrelevant.
Hmm, I'd rather not see this go in (in this form). A few comment below,
but even with them addressed I'm not going to be in favor of it.
Also did you compare all of this with the way Linux deals with the same
situation?
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> diff -r 0c446850d85e -r efa733ebd00d xen/arch/x86/apic.c
> --- a/xen/arch/x86/apic.c Wed May 11 12:58:04 2011 +0100
> +++ b/xen/arch/x86/apic.c Fri May 13 13:45:05 2011 +0100
> @@ -344,6 +344,42 @@
> }
> }
>
> +/* Customised version of disable_local_APIC which is only called on the
> kexec crash path.
> + * It is a brutal hack and only here untill the APIC code is overhauled. We
> NEED to swtich
> + * out of x2apic mode whatever the case as the kdump kernel cant cope due to
> the MMIO <-> MSR
> + * swap invovled with x2apic mode.
Unless the system booted in x2apic mode. Otherwise you'll confuse
the secondary kernel even more.
> + */
> +void crash_disable_local_APIC(void)
> +{
> + uint64_t msr_content;
> + clear_local_APIC();
> +
> + apic_write_around(APIC_SPIV,
> + apic_read(APIC_SPIV) & ~APIC_SPIV_APIC_ENABLED);
> +
> + rdmsrl(MSR_IA32_APICBASE, msr_content);
> +
> + /* This is the only way to exit x2apic mode. Trying to disable x2apic
> mode while staying
> + * xapic mode will result in a protection fault which leads to a general
> protection fault
> + * as this is on the fault handler codepath.
> + */
> + msr_content &= ~(MSR_IA32_APICBASE_ENABLE|MSR_IA32_APICBASE_EXTD);
> + wrmsrl(MSR_IA32_APICBASE, msr_content);
> +
> + /* In most cases, we should leave the lapic in xapic mode as this is the
> default state on
> + * CPU reset. However, some bioses specifically disable xapic mode for
> hilarity sake so
> + * we should follow suit so the kdump kernel does not get confused about
> mismatching ACPI
> + * tables.
> + */
> + if ( ! enabled_via_apicbase) {
> + /* If we did not manually enable xapic mode, I.E. the bios left it
> enabled,
> + * lets follow suit and leave it enabled for the kdump kernel.
> + */
> + wrmsrl(MSR_IA32_APICBASE, msr_content | MSR_IA32_APICBASE_ENABLE);
> + }
> +}
> +
> +
> /*
> * This is to verify that we're looking at a real local APIC.
> * Check these against your board if the CPUs aren't getting
> diff -r 0c446850d85e -r efa733ebd00d xen/arch/x86/crash.c
> --- a/xen/arch/x86/crash.c Wed May 11 12:58:04 2011 +0100
> +++ b/xen/arch/x86/crash.c Fri May 13 13:45:05 2011 +0100
> @@ -27,6 +27,8 @@
> #include <asm/hvm/support.h>
> #include <asm/apic.h>
> #include <asm/io_apic.h>
> +#include <xen/iommu.h>
> +#include <asm/hvm/iommu.h>
>
> static atomic_t waiting_for_crash_ipi;
> static unsigned int crashing_cpu;
> @@ -43,7 +45,12 @@
>
> kexec_crash_save_cpu();
>
> - __stop_this_cpu();
> + crash_disable_local_APIC();
> + hvm_cpu_down();
> + /* Clear FPU, zapping any pending exceptions. Needed for warm reset with
> + * some BIOSes.*/
> + clts();
> + asm volatile ( "fninit" );
>
> atomic_dec(&waiting_for_crash_ipi);
>
> @@ -77,7 +84,15 @@
> msecs--;
> }
>
> - __stop_this_cpu();
> + crash_disable_local_APIC();
> + hvm_cpu_down();
> + /* Clear FPU, zapping any pending exceptions. Needed for warm reset with
> + * some BIOSes.*/
> + clts();
> + asm volatile ( "fninit" );
> +
> + x2apic_enabled = 0;
Why? And if indeed needed here, why not above? (Your patch wasn't
generated with -p, so it's a little difficult to understand what functions
you modify here.)
> +
> disable_IO_APIC();
>
> local_irq_enable();
> @@ -86,9 +101,24 @@
> void machine_crash_shutdown(void)
> {
> crash_xen_info_t *info;
> + const struct iommu_ops * ops;
>
> nmi_shootdown_cpus();
>
> + local_irq_disable();
local_irq_save()
> +
> + /* Yes i know this is hacky but it is the easiest solution. I should
> add an iommu_ops
> + * function called crash() or so which just disables the iommu 'fun'
> without saving state
> + */
> + ops = iommu_get_ops();
if ( ops )
> + ops->suspend();
> +
> + /* Yes i know this is from driver/passthrough/vtd/ but it appears to be
> architecture
> + * independant, and also bears little/no relation to x2apic. Needs
> cleaning up
> + */
> + iommu_disable_x2apic_IR();
> + local_irq_enable();
local_irq_restore() - you absolutely want to avoid turning on
interrupts if they weren't on entry to the function.
> +
> info = kexec_crash_save_info();
> info->xen_phys_start = xen_phys_start;
> info->dom0_pfn_to_mfn_frame_list_list =
> diff -r 0c446850d85e -r efa733ebd00d xen/arch/x86/hpet.c
> --- a/xen/arch/x86/hpet.c Wed May 11 12:58:04 2011 +0100
> +++ b/xen/arch/x86/hpet.c Fri May 13 13:45:05 2011 +0100
> @@ -670,6 +670,34 @@
> smp_send_event_check_mask(&cpu_online_map);
> }
>
> +void crash_hpet_disable_legacy_broadcast(void)
> +{
> + u32 cfg;
> +
> + if ( !hpet_events || !(hpet_events->flags & HPET_EVT_LEGACY) )
> + return;
> +
> + /* By this point in the crash handler, we are the only CPU still going,
> + * so the lock is irrelevant
> + */
> + /* spin_lock_irqsave(&hpet_events->lock, flags); */
> +
> + hpet_events->flags |= HPET_EVT_DISABLE;
> +
> + /* disable HPET T0 */
> + cfg = hpet_read32(HPET_Tn_CFG(0));
> + cfg &= ~HPET_TN_ENABLE;
> + hpet_write32(cfg, HPET_Tn_CFG(0));
> +
> + /* Stop HPET legacy interrupts */
> + cfg = hpet_read32(HPET_CFG);
> + cfg &= ~HPET_CFG_LEGACY;
> + hpet_write32(cfg, HPET_CFG);
> +
> + /* spin_unlock_irqrestore(&hpet_events->lock, flags); */
> +}
This looks to be the same except for the skipping of the locks.
Calls for adding a parameter to the original function.
Jan
> +
> +
> void hpet_broadcast_enter(void)
> {
> unsigned int cpu = smp_processor_id();
> diff -r 0c446850d85e -r efa733ebd00d xen/arch/x86/machine_kexec.c
> --- a/xen/arch/x86/machine_kexec.c Wed May 11 12:58:04 2011 +0100
> +++ b/xen/arch/x86/machine_kexec.c Fri May 13 13:45:05 2011 +0100
> @@ -97,7 +97,7 @@
> };
>
> if ( hpet_broadcast_is_available() )
> - hpet_disable_legacy_broadcast();
> + crash_hpet_disable_legacy_broadcast();
>
> /*
> * compat_machine_kexec() returns to idle pagetables, which requires us
> diff -r 0c446850d85e -r efa733ebd00d xen/include/asm-x86/apic.h
> --- a/xen/include/asm-x86/apic.h Wed May 11 12:58:04 2011 +0100
> +++ b/xen/include/asm-x86/apic.h Fri May 13 13:45:05 2011 +0100
> @@ -182,6 +182,7 @@
> extern void connect_bsp_APIC (void);
> extern void disconnect_bsp_APIC (int virt_wire_setup);
> extern void disable_local_APIC (void);
> +extern void crash_disable_local_APIC (void);
> extern int verify_local_APIC (void);
> extern void cache_APIC_registers (void);
> extern void sync_Arb_IDs (void);
> diff -r 0c446850d85e -r efa733ebd00d xen/include/asm-x86/hpet.h
> --- a/xen/include/asm-x86/hpet.h Wed May 11 12:58:04 2011 +0100
> +++ b/xen/include/asm-x86/hpet.h Fri May 13 13:45:05 2011 +0100
> @@ -73,5 +73,6 @@
> void hpet_broadcast_exit(void);
> int hpet_broadcast_is_available(void);
> void hpet_disable_legacy_broadcast(void);
> +void crash_hpet_disable_legacy_broadcast(void);
>
> #endif /* __X86_HPET_H__ */
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |