On Wed, Jan 03, 2024 at 04:44:57PM +0800, Yan Zhao wrote:
> This driver is for testing KVM memory slots for device MMIO BARs that are
> mapped to pages serving as device resources.
>
> This driver implements a mock device whose device resources are pages
> array that can be mmaped into user space. It provides ioctl interface to
> users to configure whether the pages are allocated as a compound huge page
> or not.
I just think that it can be used in other scenarios, not only KVM.
>
> KVM selftest code can then map the mock device resource to KVM memslots
> to check if any error encountered. After VM shutdown, mock device
> resource's page reference counters are checked to ensure KVM does not hold
> extra reference count during memslot add/removal.
>
> Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> ---
> lib/Kconfig.debug | 14 ++
> lib/Makefile | 1 +
> lib/test_kvm_mock_device.c | 281 ++++++++++++++++++++++++++++++++
> lib/test_kvm_mock_device_uapi.h | 16 ++
> 4 files changed, 312 insertions(+)
> create mode 100644 lib/test_kvm_mock_device.c
> create mode 100644 lib/test_kvm_mock_device_uapi.h
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index cc7d53d9dc01..c0fd4b53db89 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2922,6 +2922,20 @@ config TEST_HMM
>
> If unsure, say N.
>
> +config TEST_KVM_MOCK_DEVICE
> + tristate "Test page-backended BAR to KVM mock device"
> + help
> + This is a mock KVM assigned device whose MMIO BAR is backended by
> + struct page.
> + Say M here if you want to build the "test_kvm_mock_device" module.
> + Doing so will allow you to run KVM selftest
> + tools/testing/selftest/kvm/set_memory_region_io, which tests
> + functionality of adding page-backended MMIO memslots in KVM and
> + ensures that reference count of the backend pages are correctly
> + handled.
> +
> + If unsure, say N.
> +
> config TEST_FREE_PAGES
> tristate "Test freeing pages"
> help
> diff --git a/lib/Makefile b/lib/Makefile
> index 6b09731d8e61..894a185bbabd 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
> obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
> obj-$(CONFIG_TEST_PRINTF) += test_printf.o
> obj-$(CONFIG_TEST_SCANF) += test_scanf.o
> +obj-$(CONFIG_TEST_KVM_MOCK_DEVICE) += test_kvm_mock_device.o
>
> obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
> ifeq ($(CONFIG_CC_IS_CLANG)$(CONFIG_KASAN),yy)
> diff --git a/lib/test_kvm_mock_device.c b/lib/test_kvm_mock_device.c
> new file mode 100644
> index 000000000000..4e7527c230cd
> --- /dev/null
> +++ b/lib/test_kvm_mock_device.c
> @@ -0,0 +1,281 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This is a module to test KVM DEVICE MMIO PASSTHROUGH.
> + */
> +#include <linux/init.h>
> +#include <linux/fs.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/mm.h>
> +
> +#include "test_kvm_mock_device_uapi.h"
> +
> +/* kvm mock device */
> +struct kvm_mock_dev {
> + dev_t devt;
> + struct device device;
> + struct cdev cdev;
> +};
> +static struct kvm_mock_dev kvm_mock_dev;
> +
> +struct kvm_mock_device {
> + bool compound;
> + struct page *resource;
> + u64 bar_size;
> + int order;
Do you have plan to allow user to change the bar_size via IOCTL ?
If no "order" and "bar_size" can be removed.
> + int *ref_array;
> + struct mutex lock;
> + bool prepared;
> +};
> +
> +static bool opened;
> +
> +#define BAR_SIZE 0x200000UL
> +#define DEFAULT_COMPOUND true
"kmdev->compound = true;" is more easy to understand,
but "kmdev->compound = DEFAULT_COMPOUND;" not.
> +
> +static vm_fault_t kvm_mock_device_mmap_fault(struct vm_fault *vmf)
> +{
> + struct vm_area_struct *vma = vmf->vma;
> + struct kvm_mock_device *kmdev = vma->vm_private_data;
> + struct page *p = kmdev->resource;
> + vm_fault_t ret = VM_FAULT_NOPAGE;
> + unsigned long addr;
> + int i;
> +
> + for (addr = vma->vm_start, i = vma->vm_pgoff; addr < vma->vm_end;
> + addr += PAGE_SIZE, i++) {
Just question:
Will it be enough if only map the accessed page for the testing purpose ?
> +
> + ret = vmf_insert_pfn(vma, addr, page_to_pfn(p + i));
> + if (ret == VM_FAULT_NOPAGE)
> + continue;
> +
> + zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start);
> + return ret;
> +
> + }
> + return ret;
> +}
> +
> +static const struct vm_operations_struct kvm_mock_device_mmap_ops = {
> + .fault = kvm_mock_device_mmap_fault,
> +};
> +
> +static int kvm_mock_device_fops_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct kvm_mock_device *kmdev = file->private_data;
> + u64 offset, req_len;
> + int ret = 0;
> +
> + mutex_lock(&kmdev->lock);
> + if (!kmdev->prepared) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + offset = vma->vm_pgoff << PAGE_SHIFT;
> + req_len = vma->vm_end - vma->vm_start;
> + if (offset + req_len > BAR_SIZE) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
> + vma->vm_ops = &kvm_mock_device_mmap_ops;
> + vma->vm_private_data = kmdev;
> +out:
> + mutex_unlock(&kmdev->lock);
> + return ret;
> +}
> +
> +static int kvm_mock_device_prepare_resource(struct kvm_mock_device *kmdev)
> +{
> + gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO;
> + unsigned int order = kmdev->order;
> + unsigned long count = 1 << order;
> + unsigned long i;
> + struct page *p;
> + int ret;
> +
> + mutex_lock(&kmdev->lock);
> + if (kmdev->prepared) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + if (kmdev->compound)
> + gfp_flags |= __GFP_COMP;
> +
> + p = alloc_pages_node(0, gfp_flags, order);
Please alloc_pages() to honor the memory policy of current task.
Hardcode to node 0 just works, but not hard to do this better.
> + if (!p) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + kmdev->ref_array = kmalloc_array(count, sizeof(kmdev->ref_array),
> + GFP_KERNEL_ACCOUNT);
> + if (!kmdev->ref_array) {
> + __free_pages(p, order);
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + for (i = 0; i < count; i++)
> + kmdev->ref_array[i] = page_ref_count(p + i);
> +
> + kmdev->resource = p;
> + kmdev->prepared = true;
> +out:
> + mutex_unlock(&kmdev->lock);
> + return ret;
> +}
> +
> +static int kvm_mock_device_check_resource_ref(struct kvm_mock_device *kmdev)
> +{
> + u32 i, count = 1 << kmdev->order;
> + struct page *p = kmdev->resource;
> + int inequal = 0;
> +
> + mutex_lock(&kmdev->lock);
> + if (!kmdev->prepared) {
> + mutex_unlock(&kmdev->lock);
> + return -ENODEV;
> + }
> +
> + for (i = 0; i < count; i++) {
> + if (kmdev->ref_array[i] == page_ref_count(p + i))
> + continue;
> +
> + pr_err("kvm test device check resource page %d old ref=%d new ref=%d\n",
> + i, kmdev->ref_array[i], page_ref_count(p + i));
How about just return a bitmap to userspace for each page ineuqal ?
Or if inequal number itself is enough then just remove this output, in worst case
it prints 512 times for 2MB bar case, which looks just useless.
> + inequal++;
> + }
> + mutex_unlock(&kmdev->lock);
> +
> + return inequal;
> +}
> +
> +static int kvm_mock_device_fops_open(struct inode *inode, struct file *filp)
> +{
> + struct kvm_mock_device *kmdev;
> +
> + if (opened)
> + return -EBUSY;
It can't work in case of 2 who open the device file at *real* same time, at least
you need atomic helpers for that purpose.
BTW I saw "kvm_mock_devie" instance is per file level, so maybe not hard
to remove this limitation ?
> +
> + kmdev = kzalloc(sizeof(*kmdev), GFP_KERNEL_ACCOUNT);
> + if (!kmdev)
> + return -ENOMEM;
> +
> + kmdev->compound = DEFAULT_COMPOUND;
> + kmdev->bar_size = BAR_SIZE;
> + kmdev->order = get_order(kmdev->bar_size);
> + mutex_init(&kmdev->lock);
> + filp->private_data = kmdev;
> +
> + opened = true;
> + return 0;
> +}
> +
> +static int kvm_mock_device_fops_release(struct inode *inode, struct file *filp)
> +{
> + struct kvm_mock_device *kmdev = filp->private_data;
> +
> + if (kmdev->prepared)
> + __free_pages(kmdev->resource, kmdev->order);
> + mutex_destroy(&kmdev->lock);
> + kfree(kmdev->ref_array);
> + kfree(kmdev);
> + opened = false;
> + return 0;
> +}
> +
> +static long kvm_mock_device_fops_unlocked_ioctl(struct file *filp,
> + unsigned int command,
> + unsigned long arg)
> +{
> + struct kvm_mock_device *kmdev = filp->private_data;
> + int r;
> +
> + switch (command) {
> + case KVM_MOCK_DEVICE_GET_BAR_SIZE: {
> + u64 bar_size;
> +
> + bar_size = kmdev->bar_size;
> + r = put_user(bar_size, (u64 __user *)arg);
> + break;
> + }
> + case KVM_MOCK_DEVICE_PREPARE_RESOURCE: {
> + u32 compound;
> +
> + r = get_user(compound, (u32 __user *)arg);
> + if (r)
> + return r;
> +
> + kmdev->compound = compound;
> + r = kvm_mock_device_prepare_resource(kmdev);
> + break;
> +
> + }
> + case KVM_MOCK_DEVICE_CHECK_BACKEND_REF: {
> + int inequal;
> +
> + inequal = kvm_mock_device_check_resource_ref(kmdev);
> +
> + if (inequal < 0)
> + return inequal;
> +
> + r = put_user(inequal, (u32 __user *)arg);
> + break;
> + }
> + default:
> + r = -EOPNOTSUPP;
> + }
> +
> + return r;
> +}
> +
> +
> +static const struct file_operations kvm_mock_device_fops = {
> + .open = kvm_mock_device_fops_open,
> + .release = kvm_mock_device_fops_release,
> + .mmap = kvm_mock_device_fops_mmap,
> + .unlocked_ioctl = kvm_mock_device_fops_unlocked_ioctl,
> + .llseek = default_llseek,
> + .owner = THIS_MODULE,
> +};
> +
> +
> +static int __init kvm_mock_device_test_init(void)
> +{
> + int ret;
> +
> + ret = alloc_chrdev_region(&kvm_mock_dev.devt, 0, 1, "KVM-MOCK-DEVICE");
How about misc_register() ? Like how KVM create /dev/kvm.
I think that will be more simpler.
> + if (ret)
> + goto out;
> +
> + cdev_init(&kvm_mock_dev.cdev, &kvm_mock_device_fops);
> + kvm_mock_dev.cdev.owner = THIS_MODULE;
> + device_initialize(&kvm_mock_dev.device);
> + kvm_mock_dev.device.devt = MKDEV(MAJOR(kvm_mock_dev.devt), 0);
> + ret = dev_set_name(&kvm_mock_dev.device, "kvm_mock_device");
> + if (ret)
> + goto out;
> +
> + ret = cdev_device_add(&kvm_mock_dev.cdev, &kvm_mock_dev.device);
> + if (ret)
> + goto out;
> +
> +out:
> + return ret;
> +}
> +
> +static void __exit kvm_mock_device_test_exit(void)
> +{
> + cdev_device_del(&kvm_mock_dev.cdev, &kvm_mock_dev.device);
> + unregister_chrdev_region(kvm_mock_dev.devt, 1);
> +}
> +
> +module_init(kvm_mock_device_test_init);
> +module_exit(kvm_mock_device_test_exit);
> +MODULE_LICENSE("GPL");
> diff --git a/lib/test_kvm_mock_device_uapi.h b/lib/test_kvm_mock_device_uapi.h
> new file mode 100644
> index 000000000000..227d0bf1d430
> --- /dev/null
> +++ b/lib/test_kvm_mock_device_uapi.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * This is a module to help test KVM guest access of KVM mock device's BAR,
> + * whose backend is mapped to pages.
> + */
> +#ifndef _LIB_TEST_KVM_MOCK_DEVICE_UAPI_H
> +#define _LIB_TEST_KVM_MOCK_DEVICE_UAPI_H
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +
> +#define KVM_MOCK_DEVICE_GET_BAR_SIZE _IOR('M', 0x00, u64)
> +#define KVM_MOCK_DEVICE_PREPARE_RESOURCE _IOWR('M', 0x01, u32)
> +#define KVM_MOCK_DEVICE_CHECK_BACKEND_REF _IOWR('M', 0x02, u32)
> +
> +#endif /* _LIB_TEST_KVM_MOCK_DEVICE_UAPI_H */
> --
> 2.17.1
>
>
