Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing VMO/VMAR #170

Open
benpigchu opened this issue Aug 30, 2020 · 9 comments
Open

Fixing VMO/VMAR #170

benpigchu opened this issue Aug 30, 2020 · 9 comments
Labels
bug Something isn't working

Comments

@benpigchu
Copy link
Contributor

There are still many failed vmar/vmo related core-test. Also, the attempt to run rustc on Linux mode of zCore failed due to mmap related issue. I believe these are related. We should revisited our VMO/VMAR implementation now.

@benpigchu benpigchu added the bug Something isn't working label Aug 30, 2020
@wangrunji0408
Copy link
Member

And recover a simple version of VMO (#76), even if it will fail many core-tests.

@benpigchu
Copy link
Contributor Author

benpigchu commented Sep 6, 2020

Let's do some simple fixes first.

  • Allocate a child vmar with size larger the parent should be an INVALID_ARGS instead of a NO_MEMORY (see Vmar.AllocateUnsatisfiableTest)
  • We should not check the permission of a mapping in zx_vmar_map, since we can add the permission with zx_vmar_protect later (see Vmar.VmarMapRangeOffsetTest)
  • the actual in zx_process_*_memory should be a pointer to usize
  • You cannot use MAP_RANGE with SPECIFIC_OVERWRITE in zx_vmar_map (see Vmar.InvalidArgsTest)
  • Length in zx_vmar_map cannot be zero (see Vmar.InvalidArgsTest)
  • Invalid bits in zx_vm_option_t should be rejected

@benpigchu
Copy link
Contributor Author

We somehow handles permission of a VmMapping incorrectly. We should storage a limitation of permission in the VmMapping, and allowing increasing premission with zx_vmar_protect .

@benpigchu
Copy link
Contributor Author

zx_vmar_protect is actually doing nothing. I attempted to fix it but since it do not support partially protect a VmMapping it will break at startup.
Maybe we should store flags of each pages in the VmMapping?

@benpigchu
Copy link
Contributor Author

Currently there is a workaround in VmAddressRegion::new_root to separate address space of different processes. We do not need this in qemu since we have the real page table, so we should remove this workaround on qemu.

@benpigchu
Copy link
Contributor Author

Another easy fix: vmar.*_memory should limit length with map size
Also, we should add support to multiple mappings.
Should we use a unified lock in vmar instead to avoid race condition when read/write memory?

@benpigchu
Copy link
Contributor Author

zx_vmar_protect somehow works now when I modified how the flags of each page storaged.
The next thing could be implement the ZX_VM_MAP_RANGE flag

@benpigchu
Copy link
Contributor Author

benpigchu commented Oct 12, 2020

It seems the content of the vmo is not correct if we do not always map the vmo in the page table(!)

See core-test VmoClone2TestCase.ManyCloneMappingOffset

The first issue is: we should not map the page as writable when we page fault when reading the page, otherwise we will breaks the copy-on-write semantic.
The second issue is: after a vmo.write call, the correct page to map should be changed. However, if we need to update page table when commiting the page, we get a deadlock when mapping the pages. But fortunately, If we are updating the page table, we do not need to update it with vmo, so it should be OK to just use try_lock instead of lock.

@benpigchu
Copy link
Contributor Author

benpigchu commented Oct 13, 2020

It should be possible to commit the page in vmo when we are copying things to user space! (See core-test VmoTestCase.MapRead etc.)
We need #146 to solve this problem
Another thing: VmoTestCase.ZeroPage encounter page fault forever since the 0x8 bit is set in the error_code
Another thing: vmo contiguous may breaks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants