Error in mem.c/task.h

[cycl0ne]

In Task.h:

typedef struct page {
    unsigned int present:1;
    unsigned int rw:1;
    unsigned int user:1;
    unsigned int accessed:1;
    unsigned int dirty:1;
    unsigned int unused:7;
    unsigned int frame:20;
} __attribute__((packed)) page_t;

accessed and dirty are in the wrong location. at this point you have writeback/cachedisable. So the structure should look like this:

typedef struct page {
    unsigned int present:1;
    unsigned int rw:1;
    unsigned int user:1;
    unsigned int writethrough:1;
    unsigned int cachedisable:1;
    unsigned int accessed:1;
    unsigned int dirty:1;
    unsigned int unused:1;
    unsigned int global:1;
    unsigned int available:3;
    unsigned int frame:20;
} __attribute__((packed)) page_t;

In file mem.c under function alloc_frame we find the following error:

} else {
    spin_lock(&frame_alloc_lock);
    uint32_t index = first_frame();
    assert(index != (uint32_t)-1 && "Out of frames.");
    set_frame(index * 0x1000);
    page->frame   = index; 

the last entry should be : page->frame = index * 0x1000 since we need the adress and not an index;

This error could be hard to find, because normaly you call alloc_frame(get_page(...,1,..)); so get_page already allocated it for you, but in some code (shm.c) you are relying directly on the allocation.

One thing i dont understand:

void paging_install(uint32_t memsize) 
{
    nframes = memsize  / 4;
    frames  = (uint32_t *)kmalloc(INDEX_FROM_BIT(nframes * 8));
....
}

here you divide memsize / 4.. and then you call your malloc with (nframes8)/32 ... Why the 8 ?

Another "bug" ?

void dma_frame(
        page_t *page,
        int is_kernel,
        int is_writeable,
        uintptr_t address
        ) {
    /* Page this address directly */
    page->present = 1;
    page->rw      = (is_writeable) ? 1 : 0;
    page->user    = (is_kernel)    ? 0 : 1;
    page->frame   = address / 0x1000;

shouldnt the last line be: page->frame = address & FFFFF000;

Cheers C.

[klange]

You have several different things in this report, at least one of which is definitely not an issue.

• The incorrect page_t:
  • I believe you are correct here (and I'm honestly not sure how I ended up with the correct padding bits but still having accessed/dirty in the wrong spots), but I don't attempt to use the accessed/dirty bits so I don't really care at the moment. That should be filed as a separate bug.
• Two instances of page->frame using an index.
  • No, this is correct as written due to how the bitfield works. There are several places where alloc_frame is called on a new page, and dma_frame works the same way. Since this is a 20-bit field in the overall bitfield, the appropriate shifts will be applied during the assignment, similar to how we can set the various other bits to 1/0 appropriately.
• nframes...
  • I'll have to get back to you on this one, as I don't remember how this works (it's been a few years, and it was always weird). There could well be problems in the math here...

[cycl0ne]

Hey,

sorry.. you are right! damn.. i saw the bitfield and ignored it later :dizzy_face:

for the dirty flag.. this is not so important maybe for you, but the other two which are located there: Cachedisable should be your DMA Pages flagged Writethrough should be your framebuffer flagged