Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751967AbdHGU5U (ORCPT ); Mon, 7 Aug 2017 16:57:20 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:51353 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751761AbdHGU5T (ORCPT ); Mon, 7 Aug 2017 16:57:19 -0400 Subject: Re: [PATCH v3] xen: get rid of paravirt op adjust_exception_frame To: Juergen Gross , linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, x86@kernel.org References: <20170803154548.13817-1-jgross@suse.com> Cc: hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com, luto@amacapital.net From: Boris Ostrovsky Message-ID: Date: Mon, 7 Aug 2017 16:56:52 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170803154548.13817-1-jgross@suse.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4399 Lines: 129 > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > index 811e4ddb3f37..a3dcd83187ce 100644 > --- a/arch/x86/xen/enlighten_pv.c > +++ b/arch/x86/xen/enlighten_pv.c > @@ -579,6 +579,71 @@ static void xen_write_ldt_entry(struct desc_struct *dt, int entrynum, > preempt_enable(); > } > > +#ifdef CONFIG_X86_64 > +static struct { > + void (*orig)(void); > + void (*xen)(void); > + bool ist_okay; > + bool handle; > +} trap_array[] = { > + { debug, xen_xendebug, true, true }, > + { int3, xen_xenint3, true, true }, > + { double_fault, xen_double_fault, true, false }, Is it really worth adding 'handle' member to the structure because of a single special case? We don't expect to ever have another such vector. (TBH, I think current implementation of cvt_gate_to_trap() is clearer, even if it is not as general as what is in this patch. I know that Andy disagrees). -boris > +#ifdef CONFIG_X86_MCE > + { machine_check, xen_machine_check, true, true }, > +#endif > + { nmi, xen_nmi, true, true }, > + { overflow, xen_overflow, false, true }, > +#ifdef CONFIG_IA32_EMULATION > + { entry_INT80_compat, xen_entry_INT80_compat, false, true }, > +#endif > + { page_fault, xen_page_fault, false, true }, > + { divide_error, xen_divide_error, false, true }, > + { bounds, xen_bounds, false, true }, > + { invalid_op, xen_invalid_op, false, true }, > + { device_not_available, xen_device_not_available, false, true }, > + { coprocessor_segment_overrun, xen_coprocessor_segment_overrun, > + false, true }, > + { invalid_TSS, xen_invalid_TSS, false, true }, > + { segment_not_present, xen_segment_not_present, false, true }, > + { stack_segment, xen_stack_segment, false, true }, > + { general_protection, xen_general_protection, false, true }, > + { spurious_interrupt_bug, xen_spurious_interrupt_bug, false, true }, > + { coprocessor_error, xen_coprocessor_error, false, true }, > + { alignment_check, xen_alignment_check, false, true }, > + { simd_coprocessor_error, xen_simd_coprocessor_error, false, true }, > +#ifdef CONFIG_TRACING > + { trace_page_fault, xen_trace_page_fault, false, true }, > +#endif > +}; > + > +static bool get_trap_addr(unsigned long *addr, unsigned int ist) > +{ > + unsigned int nr; > + bool handle = true, ist_okay = false; > + > + /* > + * Replace trap handler addresses by Xen specific ones. > + * Check for known traps using IST and whitelist them. > + * The debugger ones are the only ones we care about. > + * Xen will handle faults like double_fault, * so we should never see > + * them. Warn if there's an unexpected IST-using fault handler. > + */ > + for (nr = 0; nr < ARRAY_SIZE(trap_array); nr++) > + if (*addr == (unsigned long)trap_array[nr].orig) { > + *addr = (unsigned long)trap_array[nr].xen; > + ist_okay = trap_array[nr].ist_okay; > + handle = trap_array[nr].handle; > + break; > + } > + > + if (WARN_ON(ist != 0 && !ist_okay)) > + handle = false; > + > + return handle; > +} > +#endif > + > static int cvt_gate_to_trap(int vector, const gate_desc *val, > struct trap_info *info) > { > @@ -591,40 +656,8 @@ static int cvt_gate_to_trap(int vector, const gate_desc *val, > > addr = gate_offset(*val); > #ifdef CONFIG_X86_64 > - /* > - * Look for known traps using IST, and substitute them > - * appropriately. The debugger ones are the only ones we care > - * about. Xen will handle faults like double_fault, > - * so we should never see them. Warn if > - * there's an unexpected IST-using fault handler. > - */ > - if (addr == (unsigned long)debug) > - addr = (unsigned long)xen_debug; > - else if (addr == (unsigned long)int3) > - 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) { > - /* Don't need to handle these */ > + if (!get_trap_addr(&addr, val->ist)) > return 0; > -#ifdef CONFIG_X86_MCE > - } else if (addr == (unsigned long)machine_check) { > - /* > - * when xen hypervisor inject vMCE to guest, > - * use native mce handler to handle it > - */ > - ; > -#endif > - } 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; > - } > #endif /* CONFIG_X86_64 */ > info->address = addr; >