Re: [PATCH] x86/desc: build boot GDT descriptors with a macro

[Furkan Çalışkan <frn1furkan10@xxxxxxxxx>]

On 10/26/26 15:27, Andrew Cooper wrote:
> On 20/10/2026 2:57 pm, Furkan Caliskan wrote:
>> The raw 64-bit constants in boot_gdt[] and boot_compat_gdt[] hide what
>> each descriptor actually is.  Replace them with a GDTE() macro, modelled
>> on Linux's GDT_ENTRY_INIT(), plus named GDTE_FLAGS_* constants and a
>> GDTE_FLAT() helper for flat segments.
>>
>> No functional change.
>>
>> Signed-off-by: Furkan Caliskan <frn1furkan10@xxxxxxxxx>
>> ---
>>  xen/arch/x86/desc.c | 61 +++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 48 insertions(+), 13 deletions(-)
>>
>> diff --git a/xen/arch/x86/desc.c b/xen/arch/x86/desc.c
>> index 9f63928154..647affb6a3 100644
>> --- a/xen/arch/x86/desc.c
>> +++ b/xen/arch/x86/desc.c
>> @@ -27,32 +27,67 @@
>>  
>>  #define SEL2GDT(sel) (((sel) >> 3) - FIRST_RESERVED_GDT_ENTRY)
>>  
>> +/*
>> + * Expands to a brace-enclosed initializer for seg_desc_t; only use it
>> + * inside an initializer list.
>> + *
>> + * flags: bit layout matches its position in the descriptor's high dword
>> + *   bits 0-3   : type
>> + *   bit  4     : s     (0 = system, 1 = code/data)
>> + *   bits 5-6   : dpl
>> + *   bit  7     : p     (present)
>> + *   bit  12    : avl
>> + *   bit  13    : l     (64-bit code segment)
>> + *   bit  14    : d/b
>> + *   bit  15    : g     (granularity: 0 = byte, 1 = 4KiB)
>> + */
>> +#define GDTE(flags, base, limit)                                        \
>> +    { (((uint64_t)(limit) & 0x0ffffULL)            |                    \
>> +       (((uint64_t)(base)  & 0x0ffffffULL)  << 16) |                    \
>> +       (((uint64_t)(flags) & 0x0ffULL)      << 40) |                    \
>> +       (((uint64_t)(limit) & 0xf0000ULL)    << 32) |                    \
>> +       (((uint64_t)(flags) & 0xf000ULL)     << 40) |                    \
>> +       (((uint64_t)(base)  & 0xff000000ULL) << 32)) }
>> +
>> +/* Common flag combinations (same values Linux uses for equivalent 
>> segments) */
>> +#define GDTE_FLAGS_RING0_CS64   0xa09b   /* P DPL0 S code  RX A, L, G */
>> +#define GDTE_FLAGS_RING0_DS32   0xc093   /* P DPL0 S data  RW A,    G */
>> +#define GDTE_FLAGS_RING1_CS32   0xc0bb   /* P DPL1 S code  RX A,    G */
>> +#define GDTE_FLAGS_RING1_DS32   0xc0b3   /* P DPL1 S data  RW A,    G */
>> +#define GDTE_FLAGS_RING3_CS32   0xc0fb   /* P DPL3 S code  RX A,    G */
>> +#define GDTE_FLAGS_RING3_DS32   0xc0f3   /* P DPL3 S data  RW A,    G */
>> +#define GDTE_FLAGS_RING3_CS64   0xa0fb   /* P DPL3 S code  RX A, L, G */
>> +#define GDTE_FLAGS_PERCPU       0x0091   /* P DPL0 S data  RO A       */
>> +
>> +/* Xen maps every flat segment across all of memory. */
>> +#define GDTE_FLAT(flags) GDTE(flags, 0, 0xfffff)
>> +
>>  __section(".data.page_aligned") __aligned(PAGE_SIZE)
>>  seg_desc_t boot_gdt[PAGE_SIZE / sizeof(seg_desc_t)] =
>>  {
>>      /* 0xe008 - Ring 0 code, 64bit mode */
>> -    [SEL2GDT(__HYPERVISOR_CS)] =      { 0x00af9b000000ffff },
>> +    [SEL2GDT(__HYPERVISOR_CS)] =      GDTE_FLAT(GDTE_FLAGS_RING0_CS64),
>>  
>>      /* 0xe010 - Ring 0 data */
>> -    [SEL2GDT(__HYPERVISOR_DS32)] =    { 0x00cf93000000ffff },
>> +    [SEL2GDT(__HYPERVISOR_DS32)] =    GDTE_FLAT(GDTE_FLAGS_RING0_DS32),
>>  
>>      /* 0xe018 - reserved */
>>  
>>      /* 0xe023 - Ring 3 code, compatibility */
>> -    [SEL2GDT(FLAT_RING3_CS32)] =      { 0x00cffb000000ffff },
>> +    [SEL2GDT(FLAT_RING3_CS32)] =      GDTE_FLAT(GDTE_FLAGS_RING3_CS32),
>>  
>>      /* 0xe02b - Ring 3 data */
>> -    [SEL2GDT(FLAT_RING3_DS32)] =      { 0x00cff3000000ffff },
>> +    [SEL2GDT(FLAT_RING3_DS32)] =      GDTE_FLAT(GDTE_FLAGS_RING3_DS32),
>>  
>>      /* 0xe033 - Ring 3 code, 64-bit mode */
>> -    [SEL2GDT(FLAT_RING3_CS64)] =      { 0x00affb000000ffff },
>> +    [SEL2GDT(FLAT_RING3_CS64)] =      GDTE_FLAT(GDTE_FLAGS_RING3_CS64),
>>  
>>      /* 0xe038 - reserved */
>>      /* 0xe040 - TSS */
>>      /* 0xe050 - LDT */
>>  
>>      /* 0xe060 - per-CPU entry (limit == cpu) */
>> -    [SEL2GDT(PER_CPU_SELECTOR)] =     { 0x0000910000000000 },
>> +    [SEL2GDT(PER_CPU_SELECTOR)] =     GDTE(GDTE_FLAGS_PERCPU, 0, 0),
>>  };
>>  
>>  #ifdef CONFIG_PV32
>> @@ -60,29 +95,29 @@ __section(".data.page_aligned") __aligned(PAGE_SIZE)
>>  seg_desc_t boot_compat_gdt[PAGE_SIZE / sizeof(seg_desc_t)] =
>>  {
>>      /* 0xe008 - Ring 0 code, 64bit mode */
>> -    [SEL2GDT(__HYPERVISOR_CS)] =      { 0x00af9b000000ffff },
>> +    [SEL2GDT(__HYPERVISOR_CS)] =      GDTE_FLAT(GDTE_FLAGS_RING0_CS64),
>>  
>>      /* 0xe010 - Ring 0 data */
>> -    [SEL2GDT(__HYPERVISOR_DS32)] =    { 0x00cf93000000ffff },
>> +    [SEL2GDT(__HYPERVISOR_DS32)] =    GDTE_FLAT(GDTE_FLAGS_RING0_DS32),
>>  
>>      /* 0xe019 - Ring 1 code, compatibility */
>> -    [SEL2GDT(FLAT_COMPAT_RING1_CS)] = { 0x00cfbb000000ffff },
>> +    [SEL2GDT(FLAT_COMPAT_RING1_CS)] = GDTE_FLAT(GDTE_FLAGS_RING1_CS32),
>>  
>>      /* 0xe021 - Ring 1 data */
>> -    [SEL2GDT(FLAT_COMPAT_RING1_DS)] = { 0x00cfb3000000ffff },
>> +    [SEL2GDT(FLAT_COMPAT_RING1_DS)] = GDTE_FLAT(GDTE_FLAGS_RING1_DS32),
>>  
>>      /* 0xe02b - Ring 3 code, compatibility */
>> -    [SEL2GDT(FLAT_COMPAT_RING3_CS)] = { 0x00cffb000000ffff },
>> +    [SEL2GDT(FLAT_COMPAT_RING3_CS)] = GDTE_FLAT(GDTE_FLAGS_RING3_CS32),
>>  
>>      /* 0xe033 - Ring 3 data */
>> -    [SEL2GDT(FLAT_COMPAT_RING3_DS)] = { 0x00cff3000000ffff },
>> +    [SEL2GDT(FLAT_COMPAT_RING3_DS)] = GDTE_FLAT(GDTE_FLAGS_RING3_DS32),
>>  
>>      /* 0xe038 - reserved */
>>      /* 0xe040 - TSS */
>>      /* 0xe050 - LDT */
>>  
>>      /* 0xe060 - per-CPU entry (limit == cpu) */
>> -    [SEL2GDT(PER_CPU_SELECTOR)] =     { 0x0000910000000000 },
>> +    [SEL2GDT(PER_CPU_SELECTOR)] =     GDTE(GDTE_FLAGS_PERCPU, 0, 0),
>>  };
>>  #endif
>>  
> 
> I don't see this as an improvement.  All you're doing is putting a layer
> of indirection, and moving the attributes away the comment explaining
> what they are, and the attributes are still raw.
> 
> If you really want to macro this up, then at least use INIT_GDTE_SYM()
> from XTF or MiniOS.  That way you get things in a legible form at the end.
> 
> ~Andrew

Fair point. I'll fix it in v2.

Thanks,
Furkan