[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 2/2] x86/desc: Build boot_{, compat_}gdt[] in C
On 09.08.2019 15:07, Andrew Cooper wrote:
On 09/08/2019 13:43, Jan Beulich wrote:
On 09.08.2019 14:19, Andrew Cooper wrote:
On 09/08/2019 11:40, Jan Beulich wrote:
--- /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);
}
Will do, albeit for the build assertions this isn't really the
right place imo, because this isn't the place where we depend
on them being just single pages. I'll put it there nevertheless,
but I'll add a comment for why they're there.
IMO this is the right place, because it is right beside where the array
is specifically defined to be [PAGE_SIZE / sizeof()].
I was about to ask why we then need build_assertions() at all,
until I also saw ...
What it is doing is working around what is arguably a compiler bug by
allowing foo[x] = { [x + 1] = ... } to work.
... this. Which made me go check, and both gcc 4.3 and gcc 9.1
correctly complain "array index in initializer exceeds array
bounds".
Anyway, with these assertions and the tweaked constant clenaup,
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Thanks, but as per above I'm now irritated: With the explicit
specification of the array size, build_assertions() should either
be dropped again, or its comment be extended to cover why it's
needed _despite_ the specified array size (i.e. in which case
your example above would not cause a build failure).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
