[Resolved] Long return after LGDT is causing #GP

Ethin · **Posted:** Sat Jul 29, 2023 1:29 pm

Hi guys,

So I'm posting this here because I've been hacking on it for the last few days (as well as asking a few people) and I'm struggling to understand what I'm doing wrong, since unless I'm misreading the GDT tutorial and Global Discriptor Table articles, it doesn't look like I'm doing anything wrong. However, I've been given the magic numbers below for the GDT entries and I can't seem to reproduce them (and the values in the GDT tutorial article don't appear to match those in the magic numbers).

Here's my problem: when I load the GDT, it works fine. No exceptions occur when LGDT is executed. However, when I do the long return (LRETQ), I get a #GP. I'm doing this in Zig (I've done this in Rust before, started playing with Zig and thought Hey, why not see how far I can get?), so To completely ensure that nothing gets mangled across the Zig/assembly FFI boundaries, I've used an assembly stub for the actual GDT loading:

Code:

.section .data
gdtr:
    .word 0
    .quad 0
.section .text
.global load_gdt
.type load_gdt, @function
.align 8
load_gdt:
    pushq %rbp
    movq %rsp, %rbp
    subq $32, %rsp
    movw %di, gdtr+0
    movq %rsi, gdtr+2
    lgdtq (gdtr)
    pushq $0x08
    leaq reload_segment_regs(%rip), %rax
    pushq %rax
    lretq
reload_segment_regs:
    movw $0x10, %ax
    movw %ax, %ds
    movw %ax, %es
    movw %ax, %fs
    movw %ax, %gs
    movw %ax, %ss
    movq %rbp, %rsp
    popq %rbp
    retq

I've also used bitfields so that I actually understand the flags/access byte bits I'm trying to set (I could use binary bits but this is much easier). My GDT is therefore set up as follows:

Code:

const GdtEntry = packed struct {
    limit_lo: u16 = 0,
    base_lo: u16 = 0,
    base_mid: u8 = 0,
    access: AccessByte = .{ .raw = 0x00 },
    limit_hi: u4 = 0,
    flags: Flags = .{ .raw = 0x0 },
    base_hi: u8 = 0,

    pub fn asU64(self: *GdtEntry) u64 {
        return mem.readIntNative(u64, &mem.toBytes(self.*));
    }
};
comptime {
    if (@sizeOf(GdtEntry) != 8 or @bitSizeOf(GdtEntry) != 64) @compileError("GdtEntry must be 8 bytes!");
    if (@bitOffsetOf(GdtEntry, "limit_lo") != 0) @compileError("Limit lo must be at bit offset 0!");
    if (@bitOffsetOf(GdtEntry, "base_lo") == 15) @compileError("base_lo must be at bit offset 16!");
    if (@bitOffsetOf(GdtEntry, "base_mid") != 32) @compileError("base_mid must be at bit offset 32!");
    if (@bitOffsetOf(GdtEntry, "access") != 40) @compileError("access byte must be at bit offset 40!");
    if (@bitOffsetOf(GdtEntry, "limit_hi") != 48) @compileError("limit_hi must be at bit offset 48!");
    if (@bitOffsetOf(GdtEntry, "flags") != 52) @compileError("flags must be a bit offset 52!");
    if (@bitOffsetOf(GdtEntry, "base_hi") != 56) @compileError("base_hi must be at bit offset 56!");
}

const AccessByte = packed union {
    user_segment: packed struct {
        accessed: bool,
        read_write: bool,
        direction_conforming: bool,
        executable: bool,
        system: bool,
        dpl: u2,
        present: bool,
    },
    system_segment: packed struct {
        segment_type: u4,
        system: bool,
        dpl: u2,
        present: bool,
    },
    raw: u8,
};

const Flags = packed union {
    fields: packed struct {
        reserved: bool,
        long_mode: bool,
        pm_segment: bool,
        granularity: bool,
    },
    raw: u4,
};

var gdt = [_]GdtEntry{.{}} ** 8;
...
const TssDescriptor = packed struct {
    reserved1: u32 = 0,
    rsp0: u64,
    rsp1: u64,
    rsp2: u64,
    reserved2: u64 = 0,
    ist1: u64,
    ist2: u64,
    ist3: u64,
    ist4: u64,
    ist5: u64,
    ist6: u64,
    ist7: u64,
    reserved3: u32 = 0,
    reserved4: u32 = 0,
    reserved5: u8 = 0,
    iopb: u16,
};
comptime {
    if (@sizeOf(TssDescriptor) != 104) @compileError("TSS descriptor must be 104 bytes in size");
}

var tss: TssDescriptor = undefined;

// In init function
            // 64-bit kernel code
            gdt[1] = .{
                .limit_lo = 0xFFFF,
                .limit_hi = 0xF,
                .access = .{
                    .user_segment = .{
                        .accessed = false,
                        .read_write = true,
                        .direction_conforming = false,
                        .executable = true,
                        .system = true,
                        .dpl = 0,
                        .present = true,
                    },
                },
                .flags = .{
                    .fields = .{
                        .reserved = false,
                        .long_mode = true,
                        .pm_segment = false,
                        .granularity = true,
                    },
                },
            };
            // 64-bit kernel data
            gdt[2] = .{
                .limit_lo = 0xFFFF,
                .limit_hi = 0xF,
                .access = .{
                    .user_segment = .{
                        .accessed = false,
                        .read_write = true,
                        .direction_conforming = false,
                        .executable = false,
                        .system = true,
                        .dpl = 0,
                        .present = true,
                    },
                },
                .flags = .{
                    .fields = .{
                        .reserved = false,
                        .long_mode = false,
                        .pm_segment = true,
                        .granularity = true,
                    },
                },
            };
            // 64-bit user code
            gdt[3] = .{
                .limit_lo = 0xFFFF,
                .limit_hi = 0xF,
                .access = .{
                    .user_segment = .{
                        .accessed = false,
                        .read_write = true,
                        .direction_conforming = false,
                        .executable = true,
                        .system = true,
                        .dpl = 3,
                        .present = true,
                    },
                },
                .flags = .{
                    .fields = .{
                        .reserved = false,
                        .long_mode = true,
                        .pm_segment = false,
                        .granularity = true,
                    },
                },
            };
            // 64-bit user data
            gdt[4] = .{
                .limit_lo = 0xFFFF,
                .limit_hi = 0xF,
                .access = .{
                    .user_segment = .{
                        .accessed = false,
                        .read_write = true,
                        .direction_conforming = false,
                        .executable = false,
                        .system = true,
                        .dpl = 3,
                        .present = true,
                    },
                },
                .flags = .{
                    .fields = .{
                        .reserved = false,
                        .long_mode = false,
                        .pm_segment = true,
                        .granularity = true,
                    },
                },
            };
// ... Code for setting up the TSS...
            const tss_base = @intFromPtr(&tss);
            const tss_limit = @sizeOf(TssDescriptor) - 1;
            gdt[5] = .{
                .base_lo = @truncate(tss_base >> 0),
                .base_mid = @truncate(tss_base >> 16),
                .base_hi = @truncate(tss_base >> 24),
                .limit_lo = @truncate(tss_limit >> 0),
                .limit_hi = @truncate(tss_limit >> 16),
                .access = .{
                    .system_segment = .{
                        .segment_type = 0x9,
                        .system = false,
                        .dpl = 0,
                        .present = true,
                    },
                },
            };
            gdt[6] = .{
                .base_lo = @truncate(tss_base >> 32),
                .base_mid = @truncate(tss_base >> 48),
                .base_hi = @truncate(tss_base >> 56),
                .limit_lo = 0x0,
                .limit_hi = 0x0,
            };
            log.debug("GDT entries: {X}, {X}, {X}, {X}, {X}, {X}, {X}, {X}", .{ gdt[0].asU64(), gdt[1].asU64(), gdt[2].asU64(), gdt[3].asU64(), gdt[4].asU64(), gdt[5].asU64(), gdt[6].asU64(), gdt[7].asU64() });
            load_gdt(@sizeOf(@TypeOf(gdt)) - 1, @intFromPtr(&gdt));

So, I get the following output from my kernel for the GDT entries:

Quote:

[info] [default]Initializing GDT and IDT
[debug] [default]GDT entries: 0, A000000000FF9B, C000000000FF93, A000000000FFFB, C000000000FFF3, 80000900AE200087, FF0000FFFFFF0000, 0

(I don't know if this is because of how the standard library functions work or whether I'm doing something wrong.) The magic numbers I was given, for the GDT entries, are 0x0000000000000000 (null), 0x00af9b000000ffff (64-bit code), 0x00af93000000ffff (64-bit data), 0x00affb000000ffff (64-bit user-mode code), and 0x00aff3000000ffff (64-bit user-mode data). I don't believe their wrong; I'm just unable to get them, and I feel like I've tried pretty much every strategy I can think of (I tried just dumping them into an array of u64s but that made computing the descriptors for the TSS annoying, so I settled for the struct approach.) I can confirm that the values are indeed being passed correctly to my assembly stub; probing the memory of the GDT pointer base does correctly yield the actual GDT, so that's functioning fine. Is there something super obvious that I'm overlooking? (I'm doing this all in Qemu 8.0.0, if that matters.)

Barry · **Joined:** Mon Aug 27, 2018 12:50 pm **Posts:** 54

Ethin wrote:

Here's my problem: when I load the GDT, it works fine. No exceptions occur when LGDT is executed. However, when I do the long return (LRETQ), I get a #GP.

When you execute LRETQ you're flushing the CS register, which is when the issues in your GDT get realised.

On x86_64, the GDT flags nibble has a flag (bit 1, you've named it long_mode) that determines whether the segment is a 64-bit code segment. You need to set this for your code segment, which is currently 0xAF instead of 0xCF. In full it should look like 0x00CF9A000000FFFF for e.g. kernel code.

You're also setting the "accessed" bit (bit 0 of access byte) on the segments, which you don't need to do.

#GP ought to set an error code which can tell you which segment the error is occurring in. Take a look at that to help pinpoint which entries are causing the fault.

Octocontrabass · **Joined:** Mon Mar 25, 2013 7:01 pm **Posts:** 5146

Ethin wrote:

So, I get the following output from my kernel for the GDT entries:

It looks like the values you're trying to write into your struct entries are mostly being written to offset 0 instead of the correct offset. I don't know enough about Zig to say whether that's a bug in the code you've posted, a bug in the Zig compiler, or a bug somewhere else.

Ethin · **Posted:** Sat Jul 29, 2023 4:20 pm

Octocontrabass wrote:

Ethin wrote:

So, I get the following output from my kernel for the GDT entries:

It looks like the values you're trying to write into your struct entries are mostly being written to offset 0 instead of the correct offset. I don't know enough about Zig to say whether that's a bug in the code you've posted, a bug in the Zig compiler, or a bug somewhere else.

I've talked with some people in the Zig Embedded Group (ZEG) and they told me some fixes (e.g. using explicit packing information), but now I'm even more confused because now the values I get from bit-casting the structs to u64s are 0, FF9B, FF93, FFFB, and FFF3. Which is close, but not quite. So a GDT entry is now:

Code:

const GdtEntry = packed struct(u64) {
    limit_lo: u16 = 0,
    base_lo: u24 = 0,
    access: AccessByte = .{ .raw = 0x00 },
    limit_hi: u4 = 0,
    flags: Flags = .{ .raw = 0x0 },
    base_hi: u8 = 0,

    pub fn asU64(self: GdtEntry) u64 {
        return @bitCast(self);
    }
};
comptime {
    if (@sizeOf(GdtEntry) != 8 or @bitSizeOf(GdtEntry) != 64) @compileError("GdtEntry must be 8 bytes!");
    if (@bitOffsetOf(GdtEntry, "limit_lo") != 0) @compileError("Limit lo must be at bit offset 0!");
    if (@bitOffsetOf(GdtEntry, "base_lo") == 15) @compileError("base_lo must be at bit offset 16!");
    if (@bitOffsetOf(GdtEntry, "access") != 40) @compileError("access byte must be at bit offset 40!");
    if (@bitOffsetOf(GdtEntry, "limit_hi") != 48) @compileError("limit_hi must be at bit offset 48!");
    if (@bitOffsetOf(GdtEntry, "flags") != 52) @compileError("flags must be a bit offset 52!");
    if (@bitOffsetOf(GdtEntry, "base_hi") != 56) @compileError("base_hi must be at bit offset 56!");
}

const AccessByte = packed union {
    raw: u8,
    user_segment: packed struct(u8) {
        accessed: bool,
        read_write: bool,
        direction_conforming: bool,
        executable: bool,
        system: bool,
        dpl: u2,
        present: bool,
    },
    system_segment: packed struct(u8) {
        segment_type: u4,
        system: bool,
        dpl: u2,
        present: bool,
    },
};

const Flags = packed union {
    raw: u4,
    fields: packed struct(u4) {
        reserved: bool,
        long_mode: bool,
        pm_segment: bool,
        granularity: bool,
    },
};

I'm wondering if I screwed up in the flags or access byte assignment somewhere. (As an aside, what is the "authoritative" info for what flags/access should be? The GDT tutorial has differing data than the magic numbers I was given, unless I made a typo when I was decomposing them.)

Octocontrabass · **Joined:** Mon Mar 25, 2013 7:01 pm **Posts:** 5146

Ethin wrote:

I'm wondering if I screwed up in the flags or access byte assignment somewhere.

I don't see anything that looks like it would assign everything to limit_lo, but I don't speak Zig.

Ethin wrote:

(As an aside, what is the "authoritative" info for what flags/access should be? The GDT tutorial has differing data than the magic numbers I was given, unless I made a typo when I was decomposing them.)

I think some of the magic numbers you were given might be wrong. The flags should be 0xA for code segments, 0xC for data segments, and 0 for your TSS. The access byte should be 0x9B for your ring 0 code segment, 0x93 for your ring 0 data segment, 0xFB for your ring 3 code segment, 0xF3 for your ring 3 data segment, and 0x89 for your TSS.

Ethin · **Posted:** Sun Jul 30, 2023 12:47 pm

Octocontrabass wrote:

Ethin wrote:

I'm wondering if I screwed up in the flags or access byte assignment somewhere.

I don't see anything that looks like it would assign everything to limit_lo, but I don't speak Zig.

Ethin wrote:

(As an aside, what is the "authoritative" info for what flags/access should be? The GDT tutorial has differing data than the magic numbers I was given, unless I made a typo when I was decomposing them.)

I think some of the magic numbers you were given might be wrong. The flags should be 0xA for code segments, 0xC for data segments, and 0 for your TSS. The access byte should be 0x9B for your ring 0 code segment, 0x93 for your ring 0 data segment, 0xFB for your ring 3 code segment, 0xF3 for your ring 3 data segment, and 0x89 for your TSS.

Thank you for the new numbers (it might be worth updating the wiki to ensure it's information is accurate and consistent on this matter.) I've done some poking around and have managed to reproduce the problem in a hosted app (I just set up the GDT entries but didn't try loading them) and have filed an issue on Zig's repo since it's definitely a miscompile (all the bits in limit_lo are being set but all other assignments are being ignored). I would try to do this in assembly, but I'd like to avoid that. (It would be good practice, but still...)

Ethin · **Posted:** Sun Jul 30, 2023 1:25 pm

Update: I resolved it thanks to a comment on the issue; apparently, defining the struct inside the function works, but not globally. It's a bit odd, and I don't understand it myself (I haven't looked too deeply into the Zig compiler), but everything works now:

Quote:

limine: Loading kernel `boot:///EFI/BOOT/kernel`...
[info] [default]Initializing GDT and IDT
[debug] [default]GDT entries: 0000000000000000, AF9B000000FFFF00, CF93000000FFFF00, AFFB000000FFFF00, CFF3000000FFFF00, 80008900AA980067, FF0000FFFFFF0000, 00000000000000
00
[info] [default]Done

Thank you guys for the help -- hopefully I won't have as much difficulty with the IDT (which I have to do next)!

Octocontrabass · **Joined:** Mon Mar 25, 2013 7:01 pm **Posts:** 5146

Ethin wrote:

everything works now:

Those values still don't look right. Everything is shifted by one byte?

Ethin · **Posted:** Sun Jul 30, 2023 6:46 pm

Octocontrabass wrote:

Ethin wrote:

everything works now:

Those values still don't look right. Everything is shifted by one byte?

I'm not sure how to resolve this.... I'm just setting the (packed) struct's fields and then just casting it to a u64. Like so:

Code:

            // 64-bit kernel code
            var entry = GdtEntry {
                .limit_lo = 0xFFFF,
                .limit_hi = 0xF,
                .access = 0x9B,
                .flags = 0xA,
            };
            gdt[1] = @bitCast(entry);

davmac314 · **Joined:** Mon Jul 05, 2021 6:57 pm **Posts:** 118

Quote:

Update: I resolved it thanks to a comment on the issue; apparently, defining the struct inside the function works, but not globally. It's a bit odd, and I don't understand it myself

Can I suggest that this isn't really a resolution at all. If you've made some seemingly unrelated change and that appears to make something "work", but for reasons that you don't understand, then it's very possible that there is still something wrong and it is only "working" by accident, and it may stop working after making another seemingly unrelated change.

It's more important to understand the problem than it is to find a half-baked "solution". That may seem like a harder path, but it will lead you to a correct solution and not to more pain further down the line.

Ethin · **Posted:** Sun Jul 30, 2023 10:35 pm

davmac314 wrote:

Quote:

Update: I resolved it thanks to a comment on the issue; apparently, defining the struct inside the function works, but not globally. It's a bit odd, and I don't understand it myself

Can I suggest that this isn't really a resolution at all. If you've made some seemingly unrelated change and that appears to make something "work", but for reasons that you don't understand, then it's very possible that there is still something wrong and it is only "working" by accident, and it may stop working after making another seemingly unrelated change.

It's more important to understand the problem than it is to find a half-baked "solution". That may seem like a harder path, but it will lead you to a correct solution and not to more pain further down the line.

Your certainly right, and as I noted I've already reported the problem to the language developers. According to some disassembly dumps, the problem occurs when the struct is defined globally because the code that's generated is wrong, and does things like writing outside the bounds of the struct. (I'm surprised this doesn't cause code corruption.) Specifically, it rights one byte past the end of the struct, along with a couple other things. Defining the struct locally fixes the problem. It's being investigated since it's definitely a miscompile. But understanding the problem and "doing it correctly" on my end of things is, in this instance, not really something I can do, since it's at the compiler level, not anything I'm necessarily doing wrong. All I can do is use workarounds until the problem is resolved. The numbers being shifted by a byte may be something I'm doing wrong, though, or it could be another miscompile. I'm honestly unsure at the moment.

davmac314 · **Joined:** Mon Jul 05, 2021 6:57 pm **Posts:** 118

Quote:

the problem occurs when the struct is defined globally because the code that's generated is wrong

Ok, if you've verified that it's a wrong-code bug in the compiler, that's a real resolution! (have you opened a bug on the Zig compiler? some of us might be interested in seeing progress on that)

But the weird issue with the display(?) of the GDT values in the "working" version are also troubling.

I agree, there's probably not much you can do in the meantime, short of try to fix the compiler bug yourself (likely to be a difficult task). You could consider switching to another language if it looks like this is not going to be fixed quickly. I would definitely try to figure out the issue with the seemingly-working version which is displaying odd values as well.

Ethin · **Posted:** Mon Jul 31, 2023 5:02 pm

davmac314 wrote:

Quote:

the problem occurs when the struct is defined globally because the code that's generated is wrong

Ok, if you've verified that it's a wrong-code bug in the compiler, that's a real resolution! (have you opened a bug on the Zig compiler? some of us might be interested in seeing progress on that)

But the weird issue with the display(?) of the GDT values in the "working" version are also troubling.

I agree, there's probably not much you can do in the meantime, short of try to fix the compiler bug yourself (likely to be a difficult task). You could consider switching to another language if it looks like this is not going to be fixed quickly. I would definitely try to figure out the issue with the seemingly-working version which is displaying odd values as well.

I did report this issue initially; someone else managed to find the relevant disasm that was wrong (I'm not the most skilled person at figuring out things like that, especially in really large assembly files, as occurred in this instance). That issue can be found here.

Yes, the new results are concerning. I'm unsure in what direction the values are shifted by, and so I'm unsure how to "correct" them.

OSDev.org

[Resolved] Long return after LGDT is causing #GP

Who is online