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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.10.2026 15:57, 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.

Can this be "upgraded" to "No change in generated code" or some such? Else:
Are there changes in generated code?

> --- 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)) }

As you only ever use this for GDTs, the macro name here is okay. However, ...

> +/* 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)

... nothing in any of these constants is GDT-specific, and the FLAGS infix
isn't very useful there either (albeit I probably also wouldn't outright
insist on its dropping).

As to the GDTE_FLAGS_* constants' comments: The P and S bits (which are only
ever set) are mentioned, while the D/B bit (which varies) isn't. For the
constants themselves, there's clearly redundancy with asm/desc.h's _SEGMENT_*
constants. Possibly those could be used here, perhaps eliminating the need
for those comments.

Jan