PageTable: Handling of pages without the present flag set about x86_64 HOT 2 CLOSED

Ok, turns out I was wrong here on a few counts when I originally wrote this. I only started looking into this more thoroughly in the last few days.

Mapper::update_flags works fine on pages that don't have the present flag set as long as the page entry is not 0. This means that either the frame has to be set or any flag. For some reason I was under the impression that update_flags checked for the present flag and not just that the entry is not 0.
I haven't spent much time looking at the cleanup function, but I think it also checks for not 0, so I don't think that needs to be updated. I just threw that in there originally as I thought that it would be related.

That said, there are still issues with pages without the present flag, mainly unmapping them.

Some pseudo example code:

let fake_frame = PhysFram::from_start_address(0);
let page = allocate_free_page();

// This works fine. There is no check that the present flag is set
unsafe { page_table.map_to(page, fake_frame, PageTableFlags::BIT_9, frame_allocator) }
            .map(|flusher| flusher.flush())
            .unwrap();

// This also works, the only check is that the page_entry for the page is not 0
unsafe { page_table.update_flags(page, PageTableFlags::BIT_10) }
            .map(|flusher| flusher.flush())
            .unwrap();

// This fails, with page not mapped error.
page_table.unmap(page).unwrap();

One option would be to just use update_flags to zero out the flags. Assuming that the fake frame uses the frame at address 0 this would be the same as unmapping the page. However I don't think we can assume that this is always the case, so there should be some option of actually clearing a page, even if the present flag is not set.

There are 2 solutions to this I can think of:

1. update unmap to allow unmapping of pages without the present flag.
2. add a new function unmap_ignore_present

I prefer option 2 as option 1 would require the return type to be updated so that we can communicate whether or not the page was present. Also as far as I understand it there is no reason to flush the tlb when the page wasn't present so no need to return a flusher.

Right now my suggestion for an api would be

enum UnmapResult {
     Present (frame, flusher),
     NotPresent(page_table_entry),
     Error(UnmapError)
}
fn unmap_ignore_present(&mut self, page: Page) -> UnmapResult;

From what I have seen so far, there is no consistent concept for when the page-table implementation checks for the present flag or when it is ok that the entry is just unused. If you want me to I don't mind spending some additional time, looking into all other cases where this is done. Hopefully the way it is currently checked is good enough. With that in mind, what are any policies that you have in regards to breaking changes in terms of functionality? Is this something that could even be changed?

from x86_64.

Closed by #484

from x86_64.