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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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