{"action":"created","issue":{"url":"https://api.github.com/repos/cahirwpz/mimiker/issues/166","repository_url":"https://api.github.com/repos/cahirwpz/mimiker","labels_url":"https://api.github.com/repos/cahirwpz/mimiker/issues/166/labels{/name}","comments_url":"https://api.github.com/repos/cahirwpz/mimiker/issues/166/comments","events_url":"https://api.github.com/repos/cahirwpz/mimiker/issues/166/events","html_url":"https://github.com/cahirwpz/mimiker/pull/166","id":194054871,"number":166,"title":"Fixed indices in tlb_exception_handler","user":{"login":"rafalcieslak","id":6637987,"avatar_url":"https://avatars.githubusercontent.com/u/6637987?v=3","gravatar_id":"","url":"https://api.github.com/users/rafalcieslak","html_url":"https://github.com/rafalcieslak","followers_url":"https://api.github.com/users/rafalcieslak/followers","following_url":"https://api.github.com/users/rafalcieslak/following{/other_user}","gists_url":"https://api.github.com/users/rafalcieslak/gists{/gist_id}","starred_url":"https://api.github.com/users/rafalcieslak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rafalcieslak/subscriptions","organizations_url":"https://api.github.com/users/rafalcieslak/orgs","repos_url":"https://api.github.com/users/rafalcieslak/repos","events_url":"https://api.github.com/users/rafalcieslak/events{/privacy}","received_events_url":"https://api.github.com/users/rafalcieslak/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-12-07T13:52:43Z","updated_at":"2016-12-10T14:46:31Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cahirwpz/mimiker/pulls/166","html_url":"https://github.com/cahirwpz/mimiker/pull/166","diff_url":"https://github.com/cahirwpz/mimiker/pull/166.diff","patch_url":"https://github.com/cahirwpz/mimiker/pull/166.patch"},"body":"The issue I found in `tlb_exception_handler` is that the PTE validity check:\r\n\r\n```c\r\nif (is_valid(pmap->pde[index]) || is_valid(pmap->pde[index + 1])) {\r\n```\r\nis not accurate. It may happen (and that's probably what I observed in #161) that a mapping created ends at an even page, and some other mapping, created after the first one is accessed, starts at a corresponding odd page.\r\n\r\nIn that case the PDE will have a valid entry for `index`, but not for `index+1`. If we happen to access data that is backed by `index+1`, then the test quoted above will succeed, because `pde[index]` is valid. So we end up updating TLB with an invalid entry for `index+1`, while we should actually call `vm_page_fault`, to have `pde` updated.\r\n\r\nMy proposed solution is to test not whether **any** of pages in pair has valid entry in `pde`, but to test whether **the one accessed** has. In order to do that, I use variable `index` to store the index of the page accessed, while `base_index` stores the index rounded down to an even number.\r\n\r\n*Also, by the way: is tlb_exception_handler the right place to check `td->td_onfault`? What if `copyin/copyout` tries to access a valid address that is accessible by the user program, but just happens to be not present in TLB? Shouldn't we check for `td->td_onfault` iff `vm_page_fault` fails?*"},"comment":{"url":"https://api.github.com/repos/cahirwpz/mimiker/issues/comments/266213614","html_url":"https://github.com/cahirwpz/mimiker/pull/166#issuecomment-266213614","issue_url":"https://api.github.com/repos/cahirwpz/mimiker/issues/166","id":266213614,"user":{"login":"cahirwpz","id":265236,"avatar_url":"https://avatars.githubusercontent.com/u/265236?v=3","gravatar_id":"","url":"https://api.github.com/users/cahirwpz","html_url":"https://github.com/cahirwpz","followers_url":"https://api.github.com/users/cahirwpz/followers","following_url":"https://api.github.com/users/cahirwpz/following{/other_user}","gists_url":"https://api.github.com/users/cahirwpz/gists{/gist_id}","starred_url":"https://api.github.com/users/cahirwpz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cahirwpz/subscriptions","organizations_url":"https://api.github.com/users/cahirwpz/orgs","repos_url":"https://api.github.com/users/cahirwpz/repos","events_url":"https://api.github.com/users/cahirwpz/events{/privacy}","received_events_url":"https://api.github.com/users/cahirwpz/received_events","type":"User","site_admin":false},"created_at":"2016-12-10T14:46:31Z","updated_at":"2016-12-10T14:46:31Z","body":"There's something wrong with `exec` and I cannot successfully run `exec_misbehave.elf` test:\r\n\r\n```\r\n[pmap.c:186] Add mapping for page 00408000 (PTE at c0001020)\r\n[exec.c:132] Processing a PT_LOAD segment: VirtAddr = 0x407020, Offset = 0x00008020, FileSiz = 0x00000000, MemSiz = 0x0000001c, Flags = 6\r\n[exec.c:134] Exec failed: Segment p_vaddr is not page alligned\r\n```\r\n\r\nPlease note that proper spelling is `aligned` not `alligned`!"}}
{"actor":{"display_login":"cahirwpz"}}