[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/3] x86/entry: Rework the exception entrypoints
On 20.02.2023 12:59, Andrew Cooper wrote: > This fixes two issues preventing livepatching. First, that #PF and NMI fall > through into other functions, I thought this was deliberate, aiming at avoiding the unconditional branch for the most commonly taken path each. I'm not really opposed to the change, but I think it wants saying a little more as to how little (or big) of an effect this has (or at least is expected to have). > and second to add ELF metadata. > > Use a macro to generate the entrypoints programatically, rather than > opencoding them all. Switch to using the architectural short names. > > Remove the DECLARE_TRAP_HANDLER{,_CONST}() infrastructure. Only NMI/#MC are > referenced externally (and NMI will cease to be soon, as part of adding FRED > support). Move the entrypoint declarations into the respective traps.c where > they're used, rather than keeping them visible across ~all of Xen. What about Misra? Won't they be unhappy about global functions with no declaration in any header? > Drop the long-stale comment at the top of init_idt_traps(). It's mostly > discussing a 32bit Xen. The use of interrupt gates isn't 32-bit only, and justifying why trap gates aren't used looks to me like quite reasonable a purpose of that comment. > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -142,6 +142,50 @@ process_trap: > > .section .text.entry, "ax", @progbits > > +.macro IDT_ENTRY vec label handler As said in reply to another recent patch: Could we agree on separating macro parameters by commas? I also wonder whether they shouldn't all have ":req" tagged onto them, as none of them is optional. > +ENTRY(\label) > + ENDBR64 > + .if ((1 << \vec) & X86_EXC_HAVE_EC) == 0 Nit: Hard tab slipped in here. > + push $0 > + .endif > + movl $\vec, 4(%rsp) > + jmp \handler > + > + .type \label, @function > + .size \label, . - \label > +.endm > + > +.macro ENTRY vec label > + IDT_ENTRY \vec \label handle_exception > +.endm > +.macro ENTRY_IST vec label > + IDT_ENTRY \vec \label handle_ist_exception > +.endm > + > + > +ENTRY X86_EXC_DE entry_DE /* 00 Divide Error */ > +ENTRY_IST X86_EXC_DB entry_DB /* 01 Debug Exception */ > +ENTRY_IST X86_EXC_NMI entry_NMI /* 02 Non-Maskable Interrupt */ > +ENTRY X86_EXC_BP entry_BP /* 03 Breakpoint (int3) */ > +ENTRY X86_EXC_OF entry_OF /* 04 Overflow (into) */ > +ENTRY X86_EXC_BR entry_BR /* 05 Bound Range */ > +ENTRY X86_EXC_UD entry_UD /* 06 Undefined Opcode */ > +ENTRY X86_EXC_NM entry_NM /* 07 No Maths (Device Not Present) */ > +/* _IST X86_EXC_DF entry_DF 08 Double Fault - Handled specially */ > +/* X86_EXC_CSO entry_CSO 09 Coprocessor Segment Override - Autogen > */ > +ENTRY X86_EXC_TS entry_TS /* 10 Invalid TSS */ > +ENTRY X86_EXC_NP entry_NP /* 11 Segment Not Present */ > +ENTRY X86_EXC_SS entry_SS /* 12 Stack Segment Fault */ > +ENTRY X86_EXC_GP entry_GP /* 13 General Protection Fault */ > +ENTRY X86_EXC_PF entry_PF /* 14 Page Fault */ > +/* X86_EXC_SPV entry_SPV 15 PIC Spurious Interrupt Vector - > Autogen */ > +ENTRY X86_EXC_MF entry_MF /* 16 Maths Fault (x87 FPU) */ > +ENTRY X86_EXC_AC entry_AC /* 17 Alignment Check */ > +ENTRY_IST X86_EXC_MC entry_MC /* 18 Machine Check */ > +ENTRY X86_EXC_XM entry_XM /* 19 SIMD Maths Fault */ > +/* X86_EXC_VE entry_VE 20 Virtualisation Exception - Not > implemented */ > +ENTRY X86_EXC_CP entry_CP /* 21 Control-flow Protection */ I expect you won't like the request, but still: There's a lot of redundancy here. The first two arguments could all be folded, having the macro attach the X86_EXC_ and entry_ prefixes. Or wait - only quite, as long as X86_EXC_* are C macros rather than assembler equates. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |