From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Guthro Subject: Re: [PATCH] xen: Support 64-bit PV guest receiving NMIs Date: Fri, 19 Jul 2013 12:19:36 -0400 Message-ID: <51E96718.7060109@citrix.com> References: <1374249091-11285-1-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1374249091-11285-1-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xensource.com, Lisa Nguyen , Ian.Campbell@citrix.com, Zhenzhong Duan , david.vrabel@citrix.com, boris.ostrovsky@oracle.com List-Id: xen-devel@lists.xenproject.org On 07/19/2013 11:51 AM, Konrad Rzeszutek Wilk wrote: > Zhenzhong Duan sent a patch that adds some of this functionality > but this code adds the remaining pieces. The kernel has the > logic to handle Xen-type-exceptions using the paravirt interface > in the assembler code (see PARAVIRT_ADJUST_EXCEPTION_FRAME - > pv_irq_ops.adjust_exception_frame and and INTERRUPT_RETURN - > pv_cpu_ops.iret). > > That means the nmi handler (and other exception handlers) use > the hypervisor iret. > > The other changes that would be neccessary for this would > be to translate the NMI_VECTOR to one of the entries on the > ipi_vector and make xen_send_IPI_mask_allbutself use different > events. > > Fortunately for us commit 1db01b4903639fcfaec213701a494fe3fb2c490b > (xen: Clean up apic ipi interface) implemented this and we piggyback > on the cleanup such that the apic IPI interface will pass the right > vector value for NMI. > > With this patch we can trigger NMIs within a PV guest (only tested > x86_64). > > SysRq : Show backtrace of all active CPUs > sending NMI to all CPUs: > NMI backtrace for cpu 2 > CPU 2 > RIP: e030:[] [] xen_hypercall_vcpu_op+0xa/0x20 > . snip.. > Call Trace: > [] ? xen_send_IPI_one+0x40/0x60 > [] __xen_send_IPI_mask+0x2b/0x50 > [] xen_send_IPI_all+0x79/0xa0 > [] arch_trigger_all_cpu_backtrace+0x59/0xa0 > [] sysrq_handle_showallcpus+0x9/0x10 > [] __handle_sysrq+0x129/0x190 > [] ? __handle_sysrq+0x190/0x190 > [] write_sysrq_trigger+0x64/0x70 > [] proc_reg_write+0x8b/0xe0 > [] vfs_write+0xb4/0x130 > [] sys_write+0x5a/0xa0 > [] system_call_fastpath+0x16/0x1b > NMI backtrace for cpu 0 > CPU 0 > . snip.. > Call Trace: > [] ? xen_safe_halt+0x10/0x20 > [] default_idle+0x3c/0x130 > [] cpu_idle+0x99/0xe0 > [] rest_init+0x8a/0xa0 > [] start_kernel+0x3da/0x3e7 > [] ? repair_env_string+0x5b/0x5b > [] x86_64_start_reservations+0x2a/0x2c > [] xen_start_kernel+0x56e/0x570 > NMI backtrace for cpu 1 > CPU 1 > RIP: e030:[] [] xen_hypercall_sched_op+0xa/0x20 > .snip.. > Call Trace: > [] ? xen_safe_halt+0x10/0x20 > [] default_idle+0x3c/0x130 > [] cpu_idle+0x99/0xe0 > [] ? xen_irq_enable_direct_reloc+0x4/0x4 > [] cpu_bringup_and_idle+0xe/0x10 > NMI backtrace for cpu 3 > CPU 3 > .snip.. > Call Trace: > [] ? xen_safe_halt+0x10/0x20 > [] default_idle+0x3c/0x130 > [] cpu_idle+0x99/0xe0 > [] ? xen_irq_enable_direct_reloc+0x4/0x4 > [] cpu_bringup_and_idle+0xe/0x10 > > Incidentally that means kgdb will also now work within > a PV guest without using the 'nokgdbroundup' workaround. > > Note that the 32-bit version is different and this patch > does not enable that. > > CC: Lisa Nguyen > CC: Ben Guthro > CC: Zhenzhong Duan > Signed-off-by: Konrad Rzeszutek Wilk Looks good to me. Reviewed-by: Ben Guthro > --- > arch/x86/include/asm/xen/events.h | 1 + > arch/x86/xen/enlighten.c | 13 ++++++++----- > arch/x86/xen/setup.c | 13 +++++++++++-- > arch/x86/xen/smp.c | 5 +++++ > drivers/xen/events.c | 11 +++++++++++ > include/xen/interface/vcpu.h | 2 ++ > 6 files changed, 38 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h > index ca842f2..608a79d 100644 > --- a/arch/x86/include/asm/xen/events.h > +++ b/arch/x86/include/asm/xen/events.h > @@ -7,6 +7,7 @@ enum ipi_vector { > XEN_CALL_FUNCTION_SINGLE_VECTOR, > XEN_SPIN_UNLOCK_VECTOR, > XEN_IRQ_WORK_VECTOR, > + XEN_NMI_VECTOR, > > XEN_NR_IPIS, > }; > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index 2fa02bc..231382a 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -427,8 +427,7 @@ static void __init xen_init_cpuid_mask(void) > > if (!xen_initial_domain()) > cpuid_leaf1_edx_mask &= > - ~((1 << X86_FEATURE_APIC) | /* disable local APIC */ > - (1 << X86_FEATURE_ACPI)); /* disable ACPI */ > + ~((1 << X86_FEATURE_ACPI)); /* disable ACPI */ > > cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_X2APIC % 32)); > > @@ -735,8 +734,7 @@ static int cvt_gate_to_trap(int vector, const gate_desc *val, > addr = (unsigned long)xen_int3; > else if (addr == (unsigned long)stack_segment) > addr = (unsigned long)xen_stack_segment; > - else if (addr == (unsigned long)double_fault || > - addr == (unsigned long)nmi) { > + else if (addr == (unsigned long)double_fault) { > /* Don't need to handle these */ > return 0; > #ifdef CONFIG_X86_MCE > @@ -747,7 +745,12 @@ static int cvt_gate_to_trap(int vector, const gate_desc *val, > */ > ; > #endif > - } else { > + } else if (addr == (unsigned long)nmi) > + /* > + * Use the native version as well. > + */ > + ; > + else { > /* Some other trap using IST? */ > if (WARN_ON(val->ist != 0)) > return 0; > diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c > index 94eac5c..f78877c 100644 > --- a/arch/x86/xen/setup.c > +++ b/arch/x86/xen/setup.c > @@ -33,6 +33,9 @@ > /* These are code, but not functions. Defined in entry.S */ > extern const char xen_hypervisor_callback[]; > extern const char xen_failsafe_callback[]; > +#ifdef CONFIG_X86_64 > +extern const char nmi[]; > +#endif > extern void xen_sysenter_target(void); > extern void xen_syscall_target(void); > extern void xen_syscall32_target(void); > @@ -525,7 +528,13 @@ void __cpuinit xen_enable_syscall(void) > } > #endif /* CONFIG_X86_64 */ > } > - > +void __cpuinit xen_enable_nmi(void) > +{ > +#ifdef CONFIG_X86_64 > + if (register_callback(CALLBACKTYPE_nmi, nmi)) > + BUG(); > +#endif > +} > void __init xen_arch_setup(void) > { > xen_panic_handler_init(); > @@ -543,7 +552,7 @@ void __init xen_arch_setup(void) > > xen_enable_sysenter(); > xen_enable_syscall(); > - > + xen_enable_nmi(); > #ifdef CONFIG_ACPI > if (!(xen_start_info->flags & SIF_INITDOMAIN)) { > printk(KERN_INFO "ACPI in unprivileged domain disabled\n"); > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c > index c1367b2..d792cce 100644 > --- a/arch/x86/xen/smp.c > +++ b/arch/x86/xen/smp.c > @@ -572,6 +572,10 @@ static inline int xen_map_vector(int vector) > case IRQ_WORK_VECTOR: > xen_vector = XEN_IRQ_WORK_VECTOR; > break; > + case NMI_VECTOR: > + case APIC_DM_NMI: > + xen_vector = XEN_NMI_VECTOR; > + break; > default: > xen_vector = -1; > printk(KERN_ERR "xen: vector 0x%x is not implemented\n", > @@ -659,6 +663,7 @@ static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id) > return IRQ_HANDLED; > } > > + > static const struct smp_ops xen_smp_ops __initconst = { > .smp_prepare_boot_cpu = xen_smp_prepare_boot_cpu, > .smp_prepare_cpus = xen_smp_prepare_cpus, > diff --git a/drivers/xen/events.c b/drivers/xen/events.c > index a58ac43..419cc44 100644 > --- a/drivers/xen/events.c > +++ b/drivers/xen/events.c > @@ -56,6 +56,7 @@ > #include > #include > #include > +#include > #include > > /* > @@ -1213,6 +1214,16 @@ EXPORT_SYMBOL_GPL(evtchn_put); > void xen_send_IPI_one(unsigned int cpu, enum ipi_vector vector) > { > int irq = per_cpu(ipi_to_irq, cpu)[vector]; > + > + /* > + * In which the IRQ will be -1. > + */ > + if (unlikely(vector == XEN_NMI_VECTOR)) { > + int rc = HYPERVISOR_vcpu_op(VCPUOP_send_nmi, cpu, NULL); > + if (rc < 0) > + printk(KERN_WARNING "Sending nmi to CPU%d failed (rc:%d)\n", cpu, rc); > + return; > + } > BUG_ON(irq < 0); > notify_remote_via_irq(irq); > } > diff --git a/include/xen/interface/vcpu.h b/include/xen/interface/vcpu.h > index 87e6f8a..b05288c 100644 > --- a/include/xen/interface/vcpu.h > +++ b/include/xen/interface/vcpu.h > @@ -170,4 +170,6 @@ struct vcpu_register_vcpu_info { > }; > DEFINE_GUEST_HANDLE_STRUCT(vcpu_register_vcpu_info); > > +/* Send an NMI to the specified VCPU. @extra_arg == NULL. */ > +#define VCPUOP_send_nmi 11 > #endif /* __XEN_PUBLIC_VCPU_H__ */ >