On Wed, Oct 20, 2021 at 12:27 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> Use the same names and definitions (apart from the high base field)
> for GDT descriptors in both 32-bit and 64-bit code. The next patch
> will also reuse gdt_entry_t in the 16-byte struct definition, for now
> some duplication remains.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> lib/x86/desc.c | 16 ++++++----------
> lib/x86/desc.h | 28 +++++++++++++++++++++-------
> x86/taskswitch.c | 2 +-
> 3 files changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index e7378c1..3d38565 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -285,17 +285,13 @@ void set_gdt_entry(int sel, u32 base, u32 limit, u8 access, u8 gran)
> int num = sel >> 3;
>
> /* Setup the descriptor base address */
> - gdt32[num].base_low = (base & 0xFFFF);
> - gdt32[num].base_middle = (base >> 16) & 0xFF;
> - gdt32[num].base_high = (base >> 24) & 0xFF;
> + gdt32[num].base1 = (base & 0xFFFF);
> + gdt32[num].base2 = (base >> 16) & 0xFF;
> + gdt32[num].base3 = (base >> 24) & 0xFF;
>
> - /* Setup the descriptor limits */
> - gdt32[num].limit_low = (limit & 0xFFFF);
> - gdt32[num].granularity = ((limit >> 16) & 0x0F);
> -
> - /* Finally, set up the granularity and access flags */
> - gdt32[num].granularity |= (gran & 0xF0);
> - gdt32[num].access = access;
> + /* Setup the descriptor limits, granularity and flags */
> + gdt32[num].limit1 = (limit & 0xFFFF);
> + gdt32[num].type_limit_flags = ((limit & 0xF0000) >> 8) | ((gran & 0xF0) << 8) | access;
> }
>
> void set_gdt_task_gate(u16 sel, u16 tss_sel)
> diff --git a/lib/x86/desc.h b/lib/x86/desc.h
> index a6ffb38..0af37e3 100644
> --- a/lib/x86/desc.h
> +++ b/lib/x86/desc.h
> @@ -164,14 +164,27 @@ 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;
> + 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__
> struct segment_desc64 {
> uint16_t limit1;
> uint16_t base1;
I had been thinking of suggesting to use `gdt_entry_t` inline, within
the `segment_desc64` struct to avoid all of this redundancy. But I see
that's done in the next patch (which I haven't reviewed yet).
> @@ -194,6 +207,7 @@ struct segment_desc64 {
> uint32_t base4;
> uint32_t zero;
> } __attribute__((__packed__));
> +#endif
>
> #define DESC_BUSY ((uint64_t) 1 << 41)
>
> diff --git a/x86/taskswitch.c b/x86/taskswitch.c
> index 889831e..b6b3451 100644
> --- a/x86/taskswitch.c
> +++ b/x86/taskswitch.c
> @@ -21,7 +21,7 @@ fault_handler(unsigned long error_code)
>
> tss.eip += 2;
>
> - gdt32[TSS_MAIN / 8].access &= ~2;
> + gdt32[TSS_MAIN / 8].type &= ~2;
>
> set_gdt_task_gate(TSS_RETURN, tss_intr.prev);
> }
> --
> 2.27.0
>
>
Reviewed-by: Marc Orr <marcorr@xxxxxxxxxx>
