OSDev.org

Posted: **Thu Jan 26, 2023 11:25 am**

I was able to confirm that the reset_gdt() function is overwriting the boot_data table, specifically in the call to memset(). Here is the results when watching boot->mem_table in gdb.

Code: Select all

(gdb) c
Continuing.

Hardware watchpoint 3: -location boot_data->mem_table

Old value = {{base = 0, length = 654336, type = 1, ext = 1}, {base = 654336, length = 1024, type = 2, ext = 1}, {base = 983040, length = 65536, type = 2, ext = 1}, {base = 1048576, length = 133038080, type = 1, ext = 1}, {base = 134086656, length = 131072, type = 2, ext = 1}, {base = 4294705152, length = 262144, type = 2, ext = 1}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}}
New value = {{base = 0, length = 589824, type = 1, ext = 1}, {base = 654336, length = 1024, type = 2, ext = 1}, {base = 983040, length = 65536, type = 2, ext = 1}, {base = 1048576, length = 133038080, type = 1, ext = 1}, {base = 134086656, length = 131072, type = 2, ext = 1}, {base = 4294705152, length = 262144, type = 2, ext = 1}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}}
memset (ptr=0x0, value=0 '\000', num=28145) at src/mem.c:46
46	    while (num--)
(gdb) c
Continuing.

Hardware watchpoint 3: -location boot_data->mem_table

Old value = {{base = 0, length = 589824, type = 1, ext = 1}, {base = 654336, length = 1024, type = 2, ext = 1}, {base = 983040, length = 65536, type = 2, ext = 1}, {base = 1048576, length = 133038080, type = 1, ext = 1}, {base = 134086656, length = 131072, type = 2, ext = 1}, {base = 4294705152, length = 262144, type = 2, ext = 1}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}}
New value = {{base = 0, length = 0, type = 1, ext = 1}, {base = 654336, length = 1024, type = 2, ext = 1}, {base = 983040, length = 65536, type = 2, ext = 1}, {base = 1048576, length = 133038080, type = 1, ext = 1}, {base = 134086656, length = 131072, type = 2, ext = 1}, {base = 4294705152, length = 262144, type = 2, ext = 1}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}}
memset (ptr=0x0, value=0 '\000', num=28144) at src/mem.c:46
46	    while (num--)
(gdb) c
Continuing.

Hardware watchpoint 3: -location boot_data->mem_table

Old value = {{base = 0, length = 0, type = 1, ext = 1}, {base = 654336, length = 1024, type = 2, ext = 1}, {base = 983040, length = 65536, type = 2, ext = 1}, {base = 1048576, length = 133038080, type = 1, ext = 1}, {base = 134086656, length = 131072, type = 2, ext = 1}, {base = 4294705152, length = 262144, type = 2, ext = 1}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}}
New value = {{base = 0, length = 0, type = 0, ext = 1}, {base = 654336, length = 1024, type = 2, ext = 1}, {base = 983040, length = 65536, type = 2, ext = 1}, {base = 1048576, length = 133038080, type = 1, ext = 1}, {base = 134086656, length = 131072, type = 2, ext = 1}, {base = 4294705152, length = 262144, type = 2, ext = 1}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}, {base = 0, length = 0, type = 0, ext = 0}}
memset (ptr=0x0, value=0 '\000', num=28138) at src/mem.c:46

I now think that it is due to a problem in my boot loader's default page definitions, but I am not certain where. Here is what comes up when I use info mem in the QEMU monitor.

Code: Select all

(qemu) info mem
0000000000000000-0000000000100000 0000000000100000 -r-
00000000c0000000-00000000c1800000 0000000001800000 -r-

This appears to show that the page table identity maps the first 1MiB, and then maps the first 18 MiB in the higher half region, which is in fact what I intended.

The current version of the bootloader paging code is

Code: Select all

%ifndef _PAGING__INC__
%define _PAGING__INC__


%define PDE_Present           0b00000000000000000000000000000001
%define PDE_Read_Write        0b00000000000000000000000000000010
%define PDE_User              0b00000000000000000000000000000100
%define PDE_Write_Thru        0b00000000000000000000000000001000
%define PDE_Cache_Disable     0b00000000000000000000000000010000
%define PDE_Acccessed         0b00000000000000000000000000100000
%define PDE_Dirty             0b00000000000000000000000001000000
%define PDE_Page_Size         0b00000000000000000000000010000000
%define PDE_Global            0b00000000000000000000000100000000
%define PDE_Availability_Mask 0b00000000000000000000111000000000
%define PDE_Page_Attr_Table   0b00000000000000000001000000000000
%define PDE_Page_Index_Mask   0b11111111111111111110000000000000



%define PTE_Present           0b00000000000000000000000000000001
%define PTE_Read_Write        0b00000000000000000000000000000010
%define PTE_User              0b00000000000000000000000000000100
%define PTE_Write_Through     0b00000000000000000000000000001000
%define PTE_Cache_Disable     0b00000000000000000000000000010000
%define PTE_Acccessed         0b00000000000000000000000000100000
%define PTE_Dirty             0b00000000000000000000000001000000
%define PTE_Page_Attr_Table   0b00000000000000000000000010000000
%define PTE_Global            0b00000000000000000000000100000000
%define PTE_Availability_Mask 0b00000000000000000000111000000000
%define PTE_Page_Index_Mask   0b11111111111111111111000000000000



%macro populate_pte 4
        ; set up the page table
        mov ebx, dword %1             ; location to save the table entry
        mov ecx, dword %2             ; number of entries to fill
        mov eax, dword %3             ; the physical address to map

        memset32 0, 0x1000, ebx       ; clear the table entries
    %%pt_fill:
        mov edx, eax
        and edx, PTE_Page_Index_Mask  ; clear the flags before setting them
        or edx, %4                    ; flags
        mov [ebx], dword edx
        add eax, 0x1000
        add ebx, 4
        loop %%pt_fill
%endmacro


%macro populate_pde 3
        mov ebx, page_directory
        mov eax, %1                   ; directory entry address
        add ebx, %2 * 4               ; directory entry index
        or eax,  %3                   ; flags
        mov [ebx], eax
%endmacro

%endif

Code: Select all

%ifndef _PAGING_CODE__INC__
%define _PAGING_CODE__INC__

%include "paging.inc"

%line 0, "paging.asm"
bits 32

page_directory           equ 0x0040000
page_table_0x0000        equ page_directory    + 0x1000
page_table_0x0300        equ page_table_0x0000 + 0x1000
page_table_0x0301        equ page_table_0x0300 + 0x1000
page_table_0x0302        equ page_table_0x0301 + 0x1000
page_table_0x0303        equ page_table_0x0302 + 0x1000
page_table_0x0304        equ page_table_0x0303 + 0x1000
page_table_0x0305        equ page_table_0x0304 + 0x1000

init_page_directory:
        mov ebx, dword page_directory
        memset32 0, 0x1000, ebx              ; clear the page dir table

        populate_pte page_table_0x0000, 0x0100, 0x00000000, PTE_Present
        populate_pte page_table_0x0300, 0x1000, 0x00100000, PTE_Present
        populate_pte page_table_0x0301, 0x1000, 0x00400000, PTE_Present
        populate_pte page_table_0x0302, 0x1000, 0x00800000, PTE_Present
        populate_pte page_table_0x0303, 0x1000, 0x00C00000, PTE_Present
        populate_pte page_table_0x0304, 0x1000, 0x01000000, PTE_Present
        populate_pte page_table_0x0305, 0x1000, 0x01400000, PTE_Present


    .setup_directory:
        populate_pde page_table_0x0000, 0x0000, PDE_Present
        populate_pde page_table_0x0300, 0x0300, PDE_Present
        populate_pde page_table_0x0301, 0x0301, PDE_Present
        populate_pde page_table_0x0302, 0x0302, PDE_Present
        populate_pde page_table_0x0303, 0x0303, PDE_Present
        populate_pde page_table_0x0304, 0x0304, PDE_Present
        populate_pde page_table_0x0305, 0x0305, PDE_Present

        ; set the page directory
        mov eax, page_directory
        mov cr3, eax
        ret


%endif

The variables in question are defined in the linker script, as shown earlier.

My suspicion is that there is an issue in the physical->virtual address mapping, but I don't see where - as things stand, I am (or at least I think I am) defining the page tables particularly sparsely - all of the page entries are fully populated, and the physical addresses are spread out over the full 4 MiB range. I am aware that this is far from necessary, but I was doing it deliberately to limit the risk of overlapping. Apparently, this caution was not enough.

Posted: **Thu Jan 26, 2023 11:48 am**

Schol-R-LEA wrote:I was able to confirm that the reset_gdt() function is overwriting the boot_data table, specifically in the call to memset().

What are the values of the parameters to this memset() call?

Schol-R-LEA wrote:I now think that it is due to a problem in my boot loader's default page definitions, but I am not certain where.

I don't think so. Watchpoints are usually triggered only by writes to the correct virtual address, not an alias. The fact that GDB catches the write suggests that the problem is unrelated to paging.

Schol-R-LEA wrote:Here is what comes up when I use info mem in the QEMU monitor.

How about "info tlb"? You need to check both; they're not equivalent despite their apparent similarity.

Posted: **Thu Jan 26, 2023 12:13 pm**

Octocontrabass wrote:
Schol-R-LEA wrote:I was able to confirm that the reset_gdt() function is overwriting the boot_data table, specifically in the call to memset().
What are the values of the parameters to this memset() call?

Code: Select all

#define MAX_GDT_ENTRIES 0x1000
#define GDT_SIZE ((sizeof(union GDT_Entry) * MAX_GDT_ENTRIES) - 1)

// ...

extern union GDT_Entry *gdt;

Code: Select all

void reset_gdt()
{
    struct GDT_R gdt_r = { GDT_SIZE, gdt };

    union GDT_Entry *entry = gdt;

    // first, clear the whole table
    memset(entry, 0, GDT_SIZE);

The two variables in question are defined in the linker script as:

Code: Select all

    /* hardware tables */
    . = 0xC0100000;

    .tables :
	{
		tables_base = .;
	}

    .boot_data BLOCK(4K) : ALIGN (4K)
    {
        boot_data = .;
		. = . + 8K;
    }

	.gdt BLOCK(4K) : ALIGN (4K)
	{
		gdt = .;
		. = . + 64K;
	}

	.tss BLOCK(4K) : ALIGN(4K)
	{
		default_tss = .;
		. = . + 4K;
	}

The relevant part of the TLB listing seems to be:

Code: Select all

00000000c1000000: 0000000001000000 ---------
00000000c1001000: 0000000001001000 ---------
00000000c1002000: 0000000001002000 ---------
00000000c1003000: 0000000001003000 ---DA----

Posted: **Thu Jan 26, 2023 12:27 pm**

Schol-R-LEA wrote:The two variables in question are defined

Definitions are nice, but I want to know the actual runtime values.

Schol-R-LEA wrote:The relevant part of the TLB listing seems to be:

That's your stack pages, but those addresses look a bit... off. Can I see the rest?

Posted: **Thu Jan 26, 2023 12:46 pm**

Octocontrabass wrote:Definitions are nice, but I want to know the actual runtime values.

This screenshot may help on that, as it shows the addresses in question. This should confirm that they are at different virtual addresses.

Octocontrabass wrote: That's your stack pages, but those addresses look a bit... off. Can I see the rest?

OK, though it is extremely large. I'll elide the instances where it hasn't been touched. Also, this is the TLB as of when the existing code has finished; if you need it during the period when it is actually accessing the GDT, let me know.

Code: Select all

0000000000000000: 0000000000000000 ---DA----
0000000000001000: 0000000000001000 ---DA----
0000000000002000: 0000000000002000 ---DA----
0000000000003000: 0000000000003000 ---DA----
0000000000004000: 0000000000004000 ---DA----
0000000000005000: 0000000000005000 ---DA----
0000000000006000: 0000000000006000 ---DA----
0000000000007000: 0000000000007000 ---DA----
0000000000008000: 0000000000008000 ---------
0000000000009000: 0000000000009000 ---------
000000000000a000: 000000000000a000 ----A----
000000000000b000: 000000000000b000 ---DA----

...

00000000000b8000: 00000000000b8000 ---DA----

...

00000000c0000000: 0000000000100000 ----A----
00000000c0001000: 0000000000101000 ----A----
00000000c0002000: 0000000000102000 ----A----
00000000c0003000: 0000000000103000 ---DA----
00000000c0004000: 0000000000104000 ---DA----
00000000c0005000: 0000000000105000 ---------
00000000c0006000: 0000000000106000 ---------
00000000c0007000: 0000000000107000 ---------
00000000c0008000: 0000000000108000 ---------
00000000c0009000: 0000000000109000 ---------
00000000c000a000: 000000000010a000 ---------
00000000c000b000: 000000000010b000 ---------
00000000c000c000: 000000000010c000 ---------
00000000c000d000: 000000000010d000 ---------
00000000c000e000: 000000000010e000 ----A----
00000000c000f000: 000000000010f000 ----A----

...

00000000c00ff000: 00000000001ff000 ---DA----
00000000c0100000: 0000000000200000 ----A----
00000000c0101000: 0000000000201000 ---------
00000000c0102000: 0000000000202000 ----A----
00000000c0103000: 0000000000203000 ---------

...

00000000c0112000: 0000000000212000 ---------
00000000c0113000: 0000000000213000 ----A----
00000000c0114000: 0000000000214000 ---------

...

00000000c1002000: 0000000001002000 ---------
00000000c1003000: 0000000001003000 ---DA----
00000000c1004000: 0000000001004000 ---------

Posted: **Thu Jan 26, 2023 1:07 pm**

Schol-R-LEA wrote:This screenshot may help on that, as it shows the addresses in question.

It shows the addresses you expect to see, which is good, but it doesn't show the parameters you're passing to memset(). Set a breakpoint on your memset() call or add another printf().

Schol-R-LEA wrote:

Code: Select all

00000000c0114000: 0000000000214000 ---------

...

00000000c1002000: 0000000001002000 ---------

There's a discontinuity somewhere between these lines. Is it supposed to be like that?

Posted: **Thu Jan 26, 2023 1:17 pm**

Totally unrelated to the bug, just a suggestion for your project. There is a package `mtools` that you can install (it is available as an installable package on most distros and OSes these days) that can handle DOS files and FAT file systems. Rather than using mount to copy a file to a disk image you can use `mcopy` (there are other useful command like `mdir`, `mdel` etc). In your Makefile you have:

Code: Select all

       mkdir -p temp
       sudo mount $(OBJPATH)/$(DISKTARGET) temp
       sudo cp $(OBJPATH)/$(STAGE_TWO).bin temp/STAGETWO.SYS
       sudo cp $(OBJPATH)/$(KERNEL).elf temp/KERNEL.SYS
       sudo umount temp
       rmdir temp

Which could be replaced by:

Code: Select all

       mcopy -i $(OBJPATH)/$(DISKTARGET) $(OBJPATH)/$(STAGE_TWO).bin ::/STAGETWO.SYS
       mcopy -i $(OBJPATH)/$(DISKTARGET) $(OBJPATH)/$(KERNEL).elf ::/KERNEL.SYS

The big advantage to `mtools` is that you don't need elevated permissions/root access to manipulate FAT file systems. Which means in userland you don't have to use `sudo` and require a password to be entered.

Posted: **Thu Jan 26, 2023 1:31 pm**

OK, so I added a printf of the value of entry, and found it was 0 for some reason.

It turns out the problem was with how I was defining the external reference to the GDT. Instead of:

Code: Select all

extern union GDT_Entry *gdt;

it needs to be

Code: Select all

extern union GDT_Entry gdt[];

This is absurdly embarrassing. I'll need to make the same changes to the IDT and page table externs, too.

Posted: **Thu Jan 26, 2023 2:01 pm**

Schol-R-LEA wrote:It turns out the problem was with how I was defining the external reference to the GDT. Instead of:
Code: Select all
extern union GDT_Entry *gdt;
it needs to be
Code: Select all
extern union GDT_Entry gdt[];

This of course is the case for symbols defined by the linker (from the linker script in this case). When you did `gdt = .` in the linker script it didn't create a variable with an address stored in it. It creates a symbol where the value of that symbol is equal to an address. That would be why you can't reference it as just a pointer variable in C/C++. In C/C++ the address of a symbol (declared extern) is the symbols value.

In assembly the symbol can be treated as a constant, not a memory address that has a pointer in it.

Don't be embarrassed this catches a lot of people off guard. You aren't the first person to run into this confusion and you won't be the last.

OSDev.org

GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect

Re: GDT reset code seems to have no effect