Set user memory region

shim/include/handle_vm_kvm_set_user_memory_region.h

@@ -35,11 +35,6 @@ extern "C"
 {
 #endif
 
-/** @brief TBD */
-#define KVM_MEM_LOG_DIRTY_PAGES (((uint64_t)1) << ((uint64_t)0))
-/** @brief allows a slot to be read-only */
-#define KVM_MEM_READONLY (((uint64_t)1) << ((uint64_t)1))
-
     /**
      * <!-- description -->
      *   @brief Handles the execution of kvm_set_user_memory_region.

shim/include/kvm_userspace_memory_region.h

@@ -36,6 +36,9 @@ extern "C"
 
 #pragma pack(push, 1)
 
+#define KVM_MEM_LOG_DIRTY_PAGES (1UL << 0)
+#define KVM_MEM_READONLY (1UL << 1)
+
     /**
      * @struct kvm_userspace_memory_region
      *

shim/integration/kvm_get_msr_index_list.cpp

@@ -63,6 +63,18 @@ main() noexcept -> bsl::exit_code
             mut_system_ctl.write(shim::KVM_GET_MSR_INDEX_LIST, &mut_msr_list).is_neg());
     }
 
+    // nmsrs is too small
+    {
+        mut_msr_list.nmsrs = bsl::safe_u32::magic_0().get();
+        mut_msr_list.indices = mut_msr_indices.front_if();
+
+        integration::verify(
+            mut_system_ctl.write(shim::KVM_GET_MSR_INDEX_LIST, &mut_msr_list).is_neg());
+
+        auto mut_nmsrs{bsl::to_u32(mut_msr_list.nmsrs)};
+        integration::verify(mut_nmsrs > bsl::safe_u32::magic_0());
+    }
+
     {
         mut_msr_list.nmsrs = init_nmsrs.get();
         mut_msr_list.indices = mut_msr_indices.front_if();
@@ -100,6 +112,9 @@ main() noexcept -> bsl::exit_code
 
     // Try a bunch of times
     {
+        mut_msr_list.nmsrs = init_nmsrs.get();
+        mut_msr_list.indices = mut_msr_indices.front_if();
+
         constexpr auto num_loops{0x1000_umx};
         for (bsl::safe_idx mut_i{}; mut_i < num_loops; ++mut_i) {
             integration::verify(

shim/integration/kvm_set_user_memory_region.cpp

@@ -201,6 +201,84 @@ main() noexcept -> bsl::exit_code
         mut_vm.close();
     }
 
+    // non-canonical address below the canonical boundary
+    {
+        auto const vmfd{mut_system_ctl.send(shim::KVM_CREATE_VM)};
+        integration::ioctl_t mut_vm{bsl::to_i32(vmfd)};
+        constexpr auto non_canonical_address{0xFFFF7FFFFFFFFFFF_u64};
+        shim::kvm_userspace_memory_region mut_region{};
+        mut_region.slot = {};
+        mut_region.flags = {};
+        mut_region.guest_phys_addr = {};
+        mut_region.memory_size = vm_image.size().get();
+        mut_region.userspace_addr = reinterpret_cast<void *>(non_canonical_address.get());
+
+        auto const ret{mut_vm.write(shim::KVM_SET_USER_MEMORY_REGION, &mut_region)};
+        integration::verify(ret.is_neg());
+
+        mut_vm.close();
+    }
+
+    // non-canonical address above the canonical boundary
+    {
+        auto const vmfd{mut_system_ctl.send(shim::KVM_CREATE_VM)};
+        integration::ioctl_t mut_vm{bsl::to_i32(vmfd)};
+        constexpr auto non_canonical_address{0x0008000000000000_u64};
+        shim::kvm_userspace_memory_region mut_region{};
+        mut_region.slot = {};
+        mut_region.flags = {};
+        mut_region.guest_phys_addr = {};
+        mut_region.memory_size = vm_image.size().get();
+        mut_region.userspace_addr = reinterpret_cast<void *>(non_canonical_address.get());
+
+        auto const ret{mut_vm.write(shim::KVM_SET_USER_MEMORY_REGION, &mut_region)};
+        integration::verify(ret.is_neg());
+
+        mut_vm.close();
+    }
+
+    // canonical address on the higher canonical boundary
+    {
+        auto const vmfd{mut_system_ctl.send(shim::KVM_CREATE_VM)};
+        integration::ioctl_t mut_vm{bsl::to_i32(vmfd)};
+        constexpr auto non_canonical_address{0xFFFF800000000000_u64};
+        shim::kvm_userspace_memory_region mut_region{};
+        mut_region.slot = {};
+        mut_region.flags = {};
+        mut_region.guest_phys_addr = {};
+        mut_region.memory_size = vm_image.size().get();
+        mut_region.userspace_addr = reinterpret_cast<void *>(non_canonical_address.get());
+
+        auto const ret{mut_vm.write(shim::KVM_SET_USER_MEMORY_REGION, &mut_region)};
+        // This fails on platform_virt_to_phys, but none of the error paths distinguish
+        // between error type. This test should verify ret.is_zero(). Might have to add
+        // flags for the shim to short circuit.
+        integration::verify(ret.is_neg());
+
+        mut_vm.close();
+    }
+
+    // canonical address on the lower canonical boundary
+    {
+        auto const vmfd{mut_system_ctl.send(shim::KVM_CREATE_VM)};
+        integration::ioctl_t mut_vm{bsl::to_i32(vmfd)};
+        constexpr auto non_canonical_address{0x0007FFFFFFFFFFFF_u64};
+        shim::kvm_userspace_memory_region mut_region{};
+        mut_region.slot = {};
+        mut_region.flags = {};
+        mut_region.guest_phys_addr = {};
+        mut_region.memory_size = vm_image.size().get();
+        mut_region.userspace_addr = reinterpret_cast<void *>(non_canonical_address.get());
+
+        auto const ret{mut_vm.write(shim::KVM_SET_USER_MEMORY_REGION, &mut_region)};
+        // This fails on platform_virt_to_phys, but none of the error paths distinguish
+        // between error type. This test should verify ret.is_zero(). Might have to add
+        // flags for the shim to short circuit.
+        integration::verify(ret.is_neg());
+
+        mut_vm.close();
+    }
+
     // The shim has a limited number of slots
     {
         auto const vmfd{mut_system_ctl.send(shim::KVM_CREATE_VM)};

shim/linux/include/platform_interface/shim_platform_interface.h

@@ -78,12 +78,23 @@
 /** @brief defines the /dev name of the shim */
 #define SHIM_DEVICE_NAME "/dev/microv_shim"
 
+/**
+ * @brief Hack for defining ioctl commands that require structs
+ * with zero-length arrays. This is usually for ioctls that return
+ * a list.
+ *
+ * It is just like _IOWR, except it subtracts the size of a pointer
+ * from the size of the struct passed.
+ */
+#define _IOWR_LIST(type, nr, size)                                                                 \
+    _IOC(_IOC_READ | _IOC_WRITE, (type), (nr), sizeof(size) - sizeof(void *))
+
 /** @brief defines KVM's KVM_GET_API_VERSION IOCTL */
 #define KVM_GET_API_VERSION _IO(SHIMIO, 0x00)
 /** @brief defines KVM's KVM_CREATE_VM IOCTL */
 #define KVM_CREATE_VM _IO(SHIMIO, 0x01)
 /** @brief defines KVM's KVM_GET_MSR_INDEX_LIST IOCTL */
-#define KVM_GET_MSR_INDEX_LIST _IOWR(SHIMIO, 0x02, struct kvm_msr_list)
+#define KVM_GET_MSR_INDEX_LIST _IOWR_LIST(SHIMIO, 0x02, struct kvm_msr_list)
 /** @brief defines KVM's KVM_GET_MSR_FEATURE_INDEX_LIST IOCTL */
 #define KVM_GET_MSR_FEATURE_INDEX_LIST _IOWR(SHIMIO, 0x0a, struct kvm_msr_list)
 /** @brief defines KVM's KVM_CHECK_EXTENSION IOCTL */

shim/linux/include/platform_interface/shim_platform_interface.hpp

@@ -79,6 +79,17 @@
 // #include <kvm_xen_hvm_config.hpp>
 // #include <kvm_xsave.hpp>
 
+/**
+ * @brief Hack for defining ioctl commands that require structs
+ * with zero-length arrays. This is usually for ioctls that return
+ * a list.
+ *
+ * It is just like _IOWR, except it subtracts the size of a pointer
+ * from the size of the struct passed.
+ */
+#define _IOWR_LIST(type, nr, size)                                                                 \
+    _IOC(_IOC_READ | _IOC_WRITE, (type), (nr), sizeof(size) - sizeof(void *))
+
 namespace shim
 {
     /// @brief magic number for KVM IOCTLs
@@ -95,7 +106,7 @@ namespace shim
     constexpr bsl::safe_umx KVM_CREATE_VM{static_cast<bsl::uintmx>(_IO(SHIMIO.get(), 0x01))};
     /// @brief defines KVM's KVM_GET_MSR_INDEX_LIST IOCTL
     constexpr bsl::safe_umx KVM_GET_MSR_INDEX_LIST{
-        static_cast<bsl::uintmx>(_IOWR(SHIMIO.get(), 0x02, struct kvm_msr_list))};
+        static_cast<bsl::uintmx>(_IOWR_LIST(SHIMIO.get(), 0x02, struct kvm_msr_list))};
     // /// @brief defines KVM's KVM_GET_MSR_FEATURE_INDEX_LIST IOCTL
     // constexpr bsl::safe_umx KVM_GET_MSR_FEATURE_INDEX_LIST{static_cast<bsl::uintmx>(_IOWR(SHIMIO.get(), 0x0a, struct kvm_msr_list))};
     /// @brief defines KVM's KVM_CHECK_EXTENSION IOCTL

shim/linux/include/types.h

@@ -50,4 +50,9 @@
  */
 #define SHIM_INTERRUPTED ((int64_t)-EINTR)
 
+/**
+ * @brief Returned by a shim function when a provided data structure
+ *   is not large enough to hold the return data.
+ */
+#define SHIM_2BIG ((int64_t)-E2BIG)
 #endif

shim/linux/src/entry.c

@@ -233,7 +233,6 @@ dispatch_system_kvm_get_msr_index_list(
     struct kvm_msr_list __user *const user_args)
 {
     struct kvm_msr_list mut_args;
-    uint32_t __user *pmut_mut_user_indices;
     int64_t mut_ret;
     uint64_t mut_alloc_size;
 
@@ -242,35 +241,44 @@ dispatch_system_kvm_get_msr_index_list(
         return -EINVAL;
     }
 
-    mut_alloc_size = mut_args.nmsrs * sizeof(*mut_args.indices);
+    if (mut_args.nmsrs > 0) {
+        mut_alloc_size = mut_args.nmsrs * sizeof(*mut_args.indices);
+    }
+    else {
+        mut_alloc_size = sizeof(*mut_args.indices);
+    }
+
     if (mut_alloc_size > HYPERVISOR_PAGE_SIZE) {
         bferror("requested nmsrs too big");
         return -ENOMEM;
     }
 
-    pmut_mut_user_indices = mut_args.indices;
     mut_args.indices = vzalloc(mut_alloc_size);
-
     if (NULL == mut_args.indices) {
         bferror("vzalloc failed");
         return -ENOMEM;
     }
 
-    mut_ret = -EINVAL;
-    if (handle_system_kvm_get_msr_index_list(&mut_args)) {
-        bferror("handle_system_kvm_get_msr_index_list failed");
+    mut_ret = handle_system_kvm_get_msr_index_list(&mut_args);
+    if (SHIM_2BIG == mut_ret) {
+        if (platform_copy_to_user(
+                user_args, &mut_args, sizeof(mut_args.nmsrs))) {
+            bferror("platform_copy_to_user nmsrs failed");
+            mut_ret = -EINVAL;
+        }
+
         goto out;
     }
-
-    if (platform_copy_to_user(user_args, &mut_args, sizeof(mut_args.nmsrs))) {
-        bferror("platform_copy_to_user nmsrs failed");
+    else if (mut_ret) {
+        bferror("handle_system_kvm_get_msr_index_list failed");
         goto out;
     }
 
     if (platform_copy_to_user(
-            pmut_mut_user_indices,
-            mut_args.indices,
-            mut_args.nmsrs * sizeof(*mut_args.indices))) {
+            user_args,
+            &mut_args,
+            sizeof(mut_args.nmsrs) +
+                mut_args.nmsrs * sizeof(*mut_args.indices))) {
         bferror("platform_copy_to_user indices failed");
         goto out;
     }

shim/src/handle_system_kvm_get_msr_index_list.c

@@ -41,11 +41,14 @@
  *
  * <!-- inputs/outputs -->
  *   @param pmut_ioctl_args the arguments provided by userspace
- *   @return SHIM_SUCCESS on success, SHIM_FAILURE on failure.
+ *   @return SHIM_SUCCESS on success, SHIM_FAILURE on failure, and SHIM_2BIG when
+ *      the number of MSRs is greater than what was set in nmsrs. When SHIM_2BIG is
+ *      returned, the correct number of MSRs is set in the nmsrs field.
  */
 NODISCARD int64_t
 handle_system_kvm_get_msr_index_list(struct kvm_msr_list *const pmut_ioctl_args) NOEXCEPT
 {
+    int64_t mut_ret;
     int64_t mut_i;
     uint32_t mut_nmsrs = ((uint32_t)0);
 
@@ -78,18 +81,29 @@ handle_system_kvm_get_msr_index_list(struct kvm_msr_list *const pmut_ioctl_args)
             return SHIM_FAILURE;
         }
 
+        /**
+         * If the provided buffer is not large enough to fit all the MSRs, we still
+         * need to calculate the total number, and set the nmsrs field correctly.
+         */
         if (pmut_rdl->num_entries + ((uint64_t)mut_nmsrs) > ((uint64_t)pmut_ioctl_args->nmsrs)) {
-            bferror("number of MSRs is larger than kvm_msr_list indices");
-            return SHIM_FAILURE;
+            mut_nmsrs += pmut_rdl->num_entries;
         }
-
-        for (mut_i = ((int64_t)0); mut_i < ((int64_t)pmut_rdl->num_entries); ++mut_i) {
-            pmut_ioctl_args->indices[mut_nmsrs] = ((uint32_t)pmut_rdl->entries[mut_i].reg);
-            ++mut_nmsrs;
+        else {
+            for (mut_i = ((int64_t)0); mut_i < ((int64_t)pmut_rdl->num_entries); ++mut_i) {
+                pmut_ioctl_args->indices[mut_nmsrs] = ((uint32_t)pmut_rdl->entries[mut_i].reg);
+                ++mut_nmsrs;
+            }
         }
     } while (((uint64_t)0) != pmut_rdl->reg1);
 
+    if (mut_nmsrs > pmut_ioctl_args->nmsrs) {
+        mut_ret = SHIM_2BIG;
+    }
+    else {
+        mut_ret = SHIM_SUCCESS;
+    }
+
     pmut_ioctl_args->nmsrs = mut_nmsrs;
 
-    return SHIM_SUCCESS;
+    return mut_ret;
 }