On 24.02.2025 17:05, Andrew Cooper wrote: > --- /dev/null > +++ b/xen/arch/x86/include/asm/gen-idt.h > @@ -0,0 +1,121 @@ > +/* > + * Generator for IDT entries. > + * > + * Caller to provide GEN(vector, symbol, dpl, autogen) macro > + * > + * Symbols are 'entry_0xYY' if there is no better name available. Regular > + * handlers set autogen=1, while manual (autogen=0) require the symbol to be > + * implemented somewhere else. > + */
Doesn't this need something for Eclair to spot the deliberate absence of a header guard? > +#define DPL0 0 > +#define DPL1 1 > +#define DPL3 3 > + > +#define manual 0 > +#define autogen 1 > + > +#define GEN16(i) \ > + GEN(0x ## i ## 0, entry_0x ## i ## 0, DPL0, autogen) \ > + GEN(0x ## i ## 1, entry_0x ## i ## 1, DPL0, autogen) \ > + GEN(0x ## i ## 2, entry_0x ## i ## 2, DPL0, autogen) \ > + GEN(0x ## i ## 3, entry_0x ## i ## 3, DPL0, autogen) \ > + GEN(0x ## i ## 4, entry_0x ## i ## 4, DPL0, autogen) \ > + GEN(0x ## i ## 5, entry_0x ## i ## 5, DPL0, autogen) \ > + GEN(0x ## i ## 6, entry_0x ## i ## 6, DPL0, autogen) \ > + GEN(0x ## i ## 7, entry_0x ## i ## 7, DPL0, autogen) \ > + GEN(0x ## i ## 8, entry_0x ## i ## 8, DPL0, autogen) \ > + GEN(0x ## i ## 9, entry_0x ## i ## 9, DPL0, autogen) \ > + GEN(0x ## i ## a, entry_0x ## i ## a, DPL0, autogen) \ > + GEN(0x ## i ## b, entry_0x ## i ## b, DPL0, autogen) \ > + GEN(0x ## i ## c, entry_0x ## i ## c, DPL0, autogen) \ > + GEN(0x ## i ## d, entry_0x ## i ## d, DPL0, autogen) \ > + GEN(0x ## i ## e, entry_0x ## i ## e, DPL0, autogen) \ > + GEN(0x ## i ## f, entry_0x ## i ## f, DPL0, autogen) > + > + > +GEN(0x00, entry_DE, DPL0, manual) > +GEN(0x01, entry_DB, DPL0, manual) > +GEN(0x02, entry_NMI, DPL0, manual) > +GEN(0x03, entry_BP, DPL3, manual) > +GEN(0x04, entry_OF, DPL3, manual) Would this better be #ifdef CONFIG_PV32 GEN(0x04, entry_OF, DPL3, manual) #else GEN(0x04, entry_0x04, DPL0, autogen) #endif ? (Not necessarily in this patch, but in principle.) > --- /dev/null > +++ b/xen/arch/x86/include/asm/gen-idt.lds.h > @@ -0,0 +1,27 @@ > +/* > + * Linker file fragment to help format the IDT correctly > + * > + * The IDT, having grown compatibly since the 16 bit days, has the entrypoint > + * address field split into 3. x86 ELF lacks the @lo/@hi/etc relocation > forms > + * commonly found in other architectures for accessing a part of a resolved > + * symbol address. > + * > + * However, the linker can perform the necessary calculations and provide > them > + * under new symbol names. We use this to generate the low and next 16 bits > + * of the address for each handler. > + * > + * The upper 32 bits are always a constant as Xen's .text/data/rodata sits in > + * a single aligned 1G range, so do not need calculating in this manner. > + */ > +#ifndef X86_IDT_GEN_LDS_H > +#define X86_IDT_GEN_LDS_H > + > +#define GEN(vec, sym, dpl, auto) \ > + PROVIDE_HIDDEN(IDT_ ## sym ## _ADDR1 = ABSOLUTE(((sym) & 0xffff))); \ > + PROVIDE_HIDDEN(IDT_ ## sym ## _ADDR2 = ABSOLUTE(((sym >> 16) & 0xffff))); Not sure if Eclair gets to see this at all, but maybe better parenthesize sym also in the latter instance? As to the final semicolon - ideally this would be on the use sites of GEN(), for things to look more C-ish. Yet I won't insist, as gen-idt.h ends up looking sufficiently uniform for this to not be a major concern. > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -1229,6 +1229,82 @@ FUNC(trap_nop, 0) > iretq > END(trap_nop) > > +/* > + * Automatically generated entrypoints, and IDT > + */ > + > + .pushsection .data.page_aligned, "aw", @progbits > +DATA(bsp_idt, PAGE_SIZE) > + .popsection > + > +/* > + * Write an IDT Entry. The linker provides us new _ADDR1/2 symbols > calculated > + * from \sym. > + */ > +.macro write_idte sym, dpl > + .pushsection .data.page_aligned, "aw", @progbits > + .word IDT_\sym\()_ADDR1 > + .word __HYPERVISOR_CS > + .word 0x8e00 | (\dpl << 13) /* Present, DPL, Interrupt Gate */ > + .word IDT_\sym\()_ADDR2 Just to mention it: I've come across a number of issues with the not-really- standard relocation types needed to express the linker-generated-symbol references here. For the purpose here I think we're okay. > + .long __XEN_VIRT_START >> 32 > + .long 0 > + .popsection > +.endm > + > +/* > + * Write an automatically generated stub. Vectors in the exception range > keep > + * the stack properly aligned by judging whether the CPU pushed an error code > + * or not. > + * > + * Alignment is forced to 16 because that's the size of the interrupt stub > + * with CET active. Yet only because we still don't put INT3 after the JMPs to guard against SLS. (I keep carrying an ugly patch doing so.) > + */ > +.macro gen_entry vec, sym > + > +FUNC(\sym, 16) > + ENDBR64 > + > + .if \vec < 0x20 /* Exception. */ > + > + test $8, %spl /* 64bit exception frames are 16 byte > aligned, but the word */ > + jz 1f /* size is 8 bytes. Check whether the > processor gave us an */ > + pushq $0 /* error code, and insert an empty one if > not. */ > +1: movb $\vec, EFRAME_entry_vector(%rsp) > + jmp handle_exception > + > + .else /* Interrupt. */ > + > + pushq $0 > + movb $\vec, EFRAME_entry_vector(%rsp) > + jmp common_interrupt > + > + .endif > +END(\sym) > +.endm > + > +/* > + * Generator. Write an entrypoint if necessary, and record an IDT entry. > + */ > +.macro gen vec, sym, dpl, auto > + > + .if \auto > + gen_entry \vec, \sym > + .endif > + > + write_idte \sym, \dpl > +.endm > +#define GEN(v, s, d, a) gen vec=v, sym=s, dpl=d auto=a; > +#include <asm/gen-idt.h> > +#undef GEN > + > + .pushsection .data.page_aligned, "aw", @progbits > +END(bsp_idt) > + .if . - bsp_idt != PAGE_SIZE > + .error "Bad bsp_idt size, should be PAGE_SIZE" > + .endif > + .popsection Again something certainly not for this patch, and probably also not for this series: In principle, with the BSP IDT fully generated at build time, it ought to be possible to move to .rodata.page_aligned. Jan
