+Jim who is poking at this area too.
On Wed, Oct 20, 2021, Paolo Bonzini wrote:
> Use the same names and definitions (apart from the high base field)
> for GDT descriptors. gdt_entry_t is for 8-byte entries and
> gdt_desc_entry_t is for when 64-bit tests should use a 16-byte
> entry.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> diff --git a/lib/x86/desc.h b/lib/x86/desc.h
> index a6ffb38..3aa1eca 100644
> --- a/lib/x86/desc.h
> +++ b/lib/x86/desc.h
> @@ -164,15 +164,28 @@ typedef struct {
> } idt_entry_t;
>
> typedef struct {
> - u16 limit_low;
> - u16 base_low;
> - u8 base_middle;
> - u8 access;
> - u8 granularity;
> - u8 base_high;
> -} gdt_entry_t;
> -
> -struct segment_desc64 {
> + uint16_t limit1;
> + uint16_t base1;
> + uint8_t base2;
> + union {
> + uint16_t type_limit_flags; /* Type and limit flags */
> + struct {
> + uint16_t type:4;
> + uint16_t s:1;
> + uint16_t dpl:2;
> + uint16_t p:1;
> + uint16_t limit:4;
> + uint16_t avl:1;
> + uint16_t l:1;
> + uint16_t db:1;
> + uint16_t g:1;
> + } __attribute__((__packed__));
> + } __attribute__((__packed__));
> + uint8_t base3;
> +} __attribute__((__packed__)) gdt_entry_t;
> +
> +#ifdef __x86_64__
> +typedef struct {
> uint16_t limit1;
Changelog says "unify struct", yet gdt_entry_t and gdt_desc_entry_t have fully
redundant information. Apparently anonymous structs aren't a thing (or I'm just
doing it wrong), but even so, fully unifying this is not hugely problematic for
the sole consumer.
> uint16_t base1;
> uint8_t base2;
> @@ -193,7 +206,10 @@ struct segment_desc64 {
> uint8_t base3;
> uint32_t base4;
> uint32_t zero;
> -} __attribute__((__packed__));
> +} __attribute__((__packed__)) gdt_desc_entry_t;
This is misleading, only system descriptors have the extra 8 bytes.
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 3aa1eca..3b213c2 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -186,29 +186,13 @@ typedef struct {
#ifdef __x86_64__
typedef struct {
- uint16_t limit1;
- uint16_t base1;
- uint8_t base2;
- union {
- uint16_t type_limit_flags; /* Type and limit flags */
- struct {
- uint16_t type:4;
- uint16_t s:1;
- uint16_t dpl:2;
- uint16_t p:1;
- uint16_t limit:4;
- uint16_t avl:1;
- uint16_t l:1;
- uint16_t db:1;
- uint16_t g:1;
- } __attribute__((__packed__));
- } __attribute__((__packed__));
+ gdt_entry_t common;
uint8_t base3;
uint32_t base4;
uint32_t zero;
-} __attribute__((__packed__)) gdt_desc_entry_t;
+} __attribute__((__packed__)) gdt_system_desc_entry_t;
#else
-typedef gdt_entry_t gdt_desc_entry_t;
+typedef gdt_entry_t gdt_system_desc_entry_t;
#endif
#define DESC_BUSY ((uint64_t) 1 << 41)
diff --git a/x86/vmware_backdoors.c b/x86/vmware_backdoors.c
index 24870d7..4d82617 100644
--- a/x86/vmware_backdoors.c
+++ b/x86/vmware_backdoors.c
@@ -134,7 +134,7 @@ static void set_tss_ioperm(void)
{
struct descriptor_table_ptr gdt;
gdt_entry_t *gdt_table;
- gdt_desc_entry_t *tss_entry;
+ gdt_system_desc_entry_t *tss_entry;
u16 tr = 0;
tss64_t *tss;
unsigned char *ioperm_bitmap;
@@ -143,11 +143,11 @@ static void set_tss_ioperm(void)
sgdt(&gdt);
tr = str();
gdt_table = (gdt_entry_t *) gdt.base;
- tss_entry = (gdt_desc_entry_t *) &gdt_table[tr / sizeof(gdt_entry_t)];
- tss_base = ((uint64_t) tss_entry->base1 |
- ((uint64_t) tss_entry->base2 << 16) |
- ((uint64_t) tss_entry->base3 << 24) |
- ((uint64_t) tss_entry->base4 << 32));
+ tss_entry = (gdt_system_desc_entry_t *) &gdt_table[tr / sizeof(gdt_entry_t)];
+ tss_base = ((uint64_t) tss_entry->common.base1 |
+ ((uint64_t) tss_entry->common.base2 << 16) |
+ ((uint64_t) tss_entry->base3 << 24) |
+ ((uint64_t) tss_entry->base4 << 32));
tss = (tss64_t *)tss_base;
tss->iomap_base = sizeof(*tss);
ioperm_bitmap = ((unsigned char *)tss+tss->iomap_base);
