On 09/08/2019 11:40, Jan Beulich wrote:
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>
> Introduce SEL2GDT(). Correct GDT indices in public header.

"Correct" here is ambiguous because it implies there is a breakage.

You appear to have reversed FLAT_RING3_CS64 and DS32 (retaining the
original comments) and changed the comments for FLAT_RING3_SS{32,64}.

Except that now they are out of their logical order (CS 32 then 64, DS
32 then 64, SS 32 then 64).

What is the reasoning for the new order?  It isn't sorted by index.

>
> --- /dev/null
> +++ b/xen/arch/x86/desc.c
> @@ -0,0 +1,109 @@
> +
> +#define SEL2GDT(sel) (((sel) >> 3) - FIRST_RESERVED_GDT_ENTRY)
> +
> +__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_CS64)] = { 0x00af9b000000ffff },
> +
> +    /* 0xe010 - Ring 0 data */
> +    [SEL2GDT(__HYPERVISOR_DS32)] = { 0x00cf93000000ffff },
> +
> +    /* 0xe018 - reserved */
> +
> +    /* 0xe023 - Ring 3 code, compatibility */
> +    [SEL2GDT(FLAT_RING3_CS32)] = { 0x00cffb000000ffff },
> +
> +    /* 0xe02b - Ring 3 data */
> +    [SEL2GDT(FLAT_RING3_DS32)] = { 0x00cff3000000ffff },
> +
> +    /* 0xe033 - Ring 3 code, 64-bit mode */
> +    [SEL2GDT(FLAT_RING3_CS64)] = { 0x00affb000000ffff },
> +
> +    /* 0xe038 - Ring 0 code, compatibility */
> +    [SEL2GDT(__HYPERVISOR_CS32)] = { 0x00cf9b000000ffff },
> +
> +    /* 0xe040 - TSS */
> +    /* 0xe050 - LDT */
> +
> +    /* 0xe060 - per-CPU entry (limit == cpu) */
> +    [SEL2GDT(PER_CPU_GDT_SELECTOR)] = { 0x0000910000000000 },

It would be better if the = { } were vertically aligned.  It makes
reading them easier.

Also, now that SEL2GDT() is present, we need a BUILD_BUG_ON() to check
that the size doesn't vary from one page.  At the moment, changing a
selector to use 0xfxxx will cause this to grow beyond 1 page without any
compiler diagnostic.  I'd go with

static void __init __maybe_unused
build_assertions(void)                                                          
                                                                                
            

{
    BUILD_BUG_ON(sizeof(boot_gdt) != PAGE_SIZE);
    BUILD_BUG_ON(sizeof(boot_compat_gdt) != PAGE_SIZE);
}

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to