On 26/02/2025 12:39 pm, Jan Beulich wrote: > 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?
Eclair doesn't complain, although I'm not entirely sure why. >> +#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.) No. INTO can still be used in compatibility mode segment. Furthermore, for any exception we know about, we want a manual one to avoid the error-code realignment logic where possible. > >> --- /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? Oh, yes. > 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. I'm afraid it's necessary (and too in entry stubs). It's the GEN16() macro, which expands 16x GEN() on the same line. I could drop the GEN16() macro and do everything longhand, but I suspect you'd like that even less. ~Andrew
