On Fri, Jan 05, 2024 at 02:25:26PM +0800, Yuan Yao wrote:
On Wed, Jan 03, 2024 at 04:45:35PM +0800, Yan Zhao wrote:
Added a selftest set_memory_region_io to test memslots for MMIO BARs.
Emm.. "set_memory_region_io" doesn't represent the real testing purpose, but not sure if things like "memory_region_page_refcount_test" become better...
Hmm, memory_region_io_page_test? Not just ref count is tested. Without patch 1, mapping of pages will even fail.
The MMIO BARs' backends are compound/non-compound huge pages serving as device resources allocated by a mock device driver.
This selftest will assert and report "errno=14 - Bad address" in vcpu_run() if any failure is met to add such MMIO BAR memslots. After MMIO memslots removal, page reference counts of the device resources are also checked.
As this selftest will interacts with a mock device "/dev/kvm_mock_device", it depends on test driver test_kvm_mock_device.ko in the kernel. CONFIG_TEST_KVM_MOCK_DEVICE=m must be enabled in the kernel.
Currently, this selftest is only compiled for __x86_64__.
Signed-off-by: Yan Zhao yan.y.zhao@intel.com
...
+static void *vcpu_worker(void *data) +{
- struct kvm_vcpu *vcpu = data;
- struct kvm_run *run = vcpu->run;
- struct ucall uc;
- uint64_t cmd;
- /*
* Loop until the guest is done. Re-enter the guest on all MMIO exits,* which will occur if the guest attempts to access a memslot after it* has been deleted or while it is being moved .*/- while (1) {
vcpu_run(vcpu);if (run->exit_reason == KVM_EXIT_IO) {cmd = get_ucall(vcpu, &uc);if (cmd != UCALL_SYNC)break;sem_post(&vcpu_ready);continue;}if (run->exit_reason != KVM_EXIT_MMIO)break;Can the KVM_EXIT_MMIO happen on x86 ? IIUC the accessed GVAs in guest code have 1:1 mapping to MEM_REGION_GPA_BASE, which is covered by the memslot, and the memory slot is there until the guest code path done.
It can, if the GPAs accessed by guest code are not even mapped as a memslot. This check is to ensure GPAs are read from the testing memslot for mock IO.
TEST_ASSERT(!run->mmio.is_write, "Unexpected exit mmio write");TEST_ASSERT(run->mmio.len == 8,"Unexpected exit mmio size = %u", run->mmio.len);TEST_ASSERT(run->mmio.phys_addr < MEM_REGION_GPA_BASE ||run->mmio.phys_addr >= MEM_REGION_GPA_BASE + bar_size,"Unexpected exit mmio address = 0x%llx",run->mmio.phys_addr);Ditto, I just think you don't need this part in this testing.
- }
- if (run->exit_reason == KVM_EXIT_IO && cmd == UCALL_ABORT)
REPORT_GUEST_ASSERT(uc);- return NULL;
+}
+static void wait_for_vcpu(void) +{
- struct timespec ts;
- TEST_ASSERT(!clock_gettime(CLOCK_REALTIME, &ts),
"clock_gettime() failed: %d\n", errno);- ts.tv_sec += 2;
- TEST_ASSERT(!sem_timedwait(&vcpu_ready, &ts),
"sem_timedwait() failed: %d\n", errno);- /* Wait for the vCPU thread to reenter the guest. */
- usleep(100000);
In this testing it's not needed. Because you only check guest state after guest code path done, so pthread_join() is enough there.
Right. Just keep to the convention :)
+}
+static void test_kvm_mock_device_bar(bool compound) +{
- struct kvm_vm *vm;
- void *mem;
- struct kvm_vcpu *vcpu;
- pthread_t vcpu_thread;
- int fd, ret;
- u32 param_compound = compound;
- u32 inequal = 0;
- fd = open("/dev/kvm_mock_device", O_RDWR);
- if (fd < 0) {
pr_info("Please ensure \"CONFIG_TEST_KVM_MOCK_DEVICE=m\" is enabled in the kernel");pr_info(", and execute\n\"modprobe test_kvm_mock_device\n");- }
- TEST_ASSERT(fd >= 0, "Failed to open kvm mock device.");
Minor: May better to move this part into main(), highlight it's a must have dependency at beginning.
I don't think so. main() can do other tests that are not relying on the mock device. Actually I'm not even sure if this test needs to be put in set_memory_region_test.c.
- ret = ioctl(fd, KVM_MOCK_DEVICE_GET_BAR_SIZE, &bar_size);
- TEST_ASSERT(ret == 0, "Failed to get bar size of kvm mock device");
- ret = ioctl(fd, KVM_MOCK_DEVICE_PREPARE_RESOURCE, ¶m_compound);
- TEST_ASSERT(ret == 0, "Failed to prepare resource of kvm mock device");
- mem = mmap(NULL, (size_t)bar_size, PROT_READ | PROT_WRITE, MAP_SHARED,
fd, 0);- TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() kvm mock device bar");
- *(u64 *)mem = BASE_VAL;
- *(u64 *)(mem + RANDOM_OFFSET) = RANDOM_VAL;
- vm = vm_create_with_one_vcpu(&vcpu, guest_code_read_bar);
- vm_set_user_memory_region(vm, MEM_REGION_SLOT_ID, 0, MEM_REGION_GPA_BASE,
bar_size, mem);- virt_map(vm, MEM_REGION_GPA_BASE, MEM_REGION_GPA_BASE,
(RANDOM_OFFSET / getpagesize()) + 1);- pthread_create(&vcpu_thread, NULL, vcpu_worker, vcpu);
- /* Ensure the guest thread is spun up. */
- wait_for_vcpu();
- pthread_join(vcpu_thread, NULL);
- vm_set_user_memory_region(vm, MEM_REGION_SLOT_ID, 0, 0, 0, 0);
- kvm_vm_free(vm);
- ret = ioctl(fd, KVM_MOCK_DEVICE_CHECK_BACKEND_REF, &inequal);
- TEST_ASSERT(ret == 0 && inequal == 0, "Incorrect resource ref of KVM device");
- munmap(mem, bar_size);
- close(fd);
+}
+static void test_non_compound_backend(void) +{
- pr_info("Testing non-compound huge page backend for mem slot\n");
- test_kvm_mock_device_bar(false);
+}
+static void test_compound_backend(void) +{
- pr_info("Testing compound huge page backend for mem slot\n");
- test_kvm_mock_device_bar(true);
+}
+int main(int argc, char *argv[]) +{ +#ifdef __x86_64__
- test_compound_backend();
- if (non_compound_supported)
Nobody set this, but the mock device looks already supported it, so how about just run the 2 testings directly here ?
Without the series "allow mapping non-refcounted pages" [1], the test test_non_compound_backend() will simply fail.
I added this test case is to show it non-compound pages as backend can also be tested with this selftest. And actually, I did tested that series with this selftets.
So, can remove the "if non_compound_supported" after [1] is merged.
[1] https://lore.kernel.org/all/20230911021637.1941096-1-stevensd@google.com/,
Thanks
test_non_compound_backend();+#endif
- return 0;
+}
2.17.1