Set user memory region #78
Conversation
Add a macro, _IOWR_LIST, as a hack to assist with kvm structs that
contain zero-length arrays. We cannot use zero-length arrays in this
project without many consequences in testing, clang-tidy, etc. However,
in these cases, we can get similiar behavior by using a pointer to the
array element type. For example,
struct kvm_msr_list {
__u32 nmsrs; /* number of msrs in entries */
__u32 indices[0];
};
translates to
struct kvm_msr_list {
uint32_t nmsrs; /* number of msrs in entries */
uint32_t *indices;
};
However, this changes the ioctl number calculated with _IOC, and causes
the shim to error out with "invalid ioctl cmd." This is because the
sizeof the two structs are not the same, because a zero-length array has
size zero, while a uin32_t * has size 8 on a 64-bit system.
Re-define KVM_GET_MSR_INDEX_LIST using this macro so that the ioctl
number matches the real KVM implementation.
Signed-off-by: Nick Rosbrook <[email protected]>
The KVM API states that when the number of MSRs is greater than the value set in nmsrs by the caller, the kernel will set the correct value in nmsrs and return -E2BIG. This behavior was excluded from the first implementation of KVM_GET_MSR_INDEX_LIST. Define SHIM_2BIG so that other shim functions can implement this behavior, which is a common pattern in ioctls. Also, fix the final copy_to_user in dispatch_system_kvm_get_msr_index_list. Although we use a uint32_t * in our struct definition, the real KVM API uses a zero-length array. This means that the memory region for indices is guaranteed to be contiguous with nmsrs, whereas our implementation requires two separate copies: one to the struct base address, and one to the address pointed to by indices. This change means that our integration test is now broken. However, the QEMU integration test works as expected. Signed-off-by: Nick Rosbrook <[email protected]>
Signed-off-by: Brendan Kerrigan <[email protected]>
Signed-off-by: Brendan Kerrigan <[email protected]>
Codecov Report
@@ Coverage Diff @@
## demo #78 +/- ##
===========================================
- Coverage 100.00% 99.85% -0.15%
===========================================
Files 210 210
Lines 6397 6417 +20
Branches 123 128 +5
===========================================
+ Hits 6397 6408 +11
- Misses 0 6 +6
- Partials 0 3 +3
|
| @@ -258,8 +258,7 @@ namespace microv | |||
| /// because guest software will not attempt to undo a | |||
| /// failed map operation. | |||
| /// | |||
| auto const ret{ | |||
| m_slpt.map(tls, mut_page_pool, gpa, spa, MAP_PAGE_RWE, false, mut_sys)}; | |||
| auto const ret{m_slpt.map(tls, mut_page_pool, gpa, spa, flags, false, mut_sys)}; | |||
There was a problem hiding this comment.
This change will cause all of the integration tests that use mv_vm_op_mmio_map to fail because they are not currently properly setting the flags argument. Because of this, the flags will be set to no-read no-write and no-execute causing an NPF VMExit on AMD or an EPT Violation on Intel as soon as the VM runs.
For Qemu, these NPF / EPT Violation VMExits would need to be properly handled in the dispatch_vmexit_mmio functions. A world switch needs to occur to switch from the context of the guest VM to the root VM and cause a mv_exit_reason_t_mmio event to occur in the shim as a return value to mv_vs_op_run. Finally, this event needs to propagate to Qemu with a KVM_EXIT_MMIO.
Since we are currently not allowing KVM_CAP_READONLY_MEM Qemu should not be requesting the KVM_MEM_READONLY flag. Therefore, we could set them to RWE from the shim, or here in the hypervisor.
| NODISCARD static inline uint64_t | ||
| kvm_to_mv_page_flags(uint32_t const flags) NOEXCEPT | ||
| { | ||
| uint64_t mut_flags = ((uint64_t)0); |
There was a problem hiding this comment.
The default value for these flags should be RWE. Only when KVM_MEM_READONLY is given, the write permission should be removed.
No description provided.