Re: [PATCH 06/17] virt: acrn: Introduce VM management interfaces

From: Greg Kroah-Hartman
Date: Fri Aug 28 2020 - 06:27:09 EST


On Tue, Aug 25, 2020 at 10:45:06AM +0800, shuo.a.liu@xxxxxxxxx wrote:
> From: Shuo Liu <shuo.a.liu@xxxxxxxxx>
>
> The VM management interfaces expose several VM operations to ACRN
> userspace via ioctls. For example, creating VM, starting VM, destroying
> VM and so on.
>
> The ACRN Hypervisor needs to exchange data with the ACRN userspace
> during the VM operations. HSM provides VM operation ioctls to the ACRN
> userspace and communicates with the ACRN Hypervisor for VM operations
> via hypercalls.
>
> HSM maintains a list of User VM. Each User VM will be bound to an
> existing file descriptor of /dev/acrn_hsm. The User VM will be
> destroyed when the file descriptor is closed.
>
> Signed-off-by: Shuo Liu <shuo.a.liu@xxxxxxxxx>
> Reviewed-by: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> Reviewed-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> Cc: Yu Wang <yu1.wang@xxxxxxxxx>
> Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> ---
> drivers/virt/acrn/Makefile | 2 +-
> drivers/virt/acrn/acrn_drv.h | 16 ++++++++-
> drivers/virt/acrn/hsm.c | 58 +++++++++++++++++++++++++++++-
> drivers/virt/acrn/hypercall.h | 62 ++++++++++++++++++++++++++++++++
> drivers/virt/acrn/vm.c | 66 +++++++++++++++++++++++++++++++++++
> include/uapi/linux/acrn.h | 36 +++++++++++++++++++
> 6 files changed, 237 insertions(+), 3 deletions(-)
> create mode 100644 drivers/virt/acrn/vm.c
>
> diff --git a/drivers/virt/acrn/Makefile b/drivers/virt/acrn/Makefile
> index 6920ed798aaf..cf8b4ed5e74e 100644
> --- a/drivers/virt/acrn/Makefile
> +++ b/drivers/virt/acrn/Makefile
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_ACRN_HSM) := acrn.o
> -acrn-y := hsm.o
> +acrn-y := hsm.o vm.o
> diff --git a/drivers/virt/acrn/acrn_drv.h b/drivers/virt/acrn/acrn_drv.h
> index 36f43d8d43d0..35fcb5cbbff3 100644
> --- a/drivers/virt/acrn/acrn_drv.h
> +++ b/drivers/virt/acrn/acrn_drv.h
> @@ -10,12 +10,26 @@
>
> #define ACRN_INVALID_VMID (0xffffU)
>
> +#define ACRN_VM_FLAG_DESTROYED 0U
> +extern struct list_head acrn_vm_list;
> +extern rwlock_t acrn_vm_list_lock;
> /**
> * struct acrn_vm - Properties of ACRN User VM.
> + * @list: Entry within global list of all VMs
> * @vmid: User VM ID
> + * @vcpu_num: Number of virtual CPUs in the VM
> + * @flags: Flags (ACRN_VM_FLAG_*) of the VM. This is VM flag management
> + * in HSM which is different from the &acrn_vm_creation.vm_flag.
> */
> struct acrn_vm {
> - u16 vmid;
> + struct list_head list;
> + u16 vmid;
> + int vcpu_num;
> + unsigned long flags;
> };
>
> +struct acrn_vm *acrn_vm_create(struct acrn_vm *vm,
> + struct acrn_vm_creation *vm_param);
> +int acrn_vm_destroy(struct acrn_vm *vm);
> +
> #endif /* __ACRN_HSM_DRV_H */
> diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
> index a08169f35c96..ed8921a6c68b 100644
> --- a/drivers/virt/acrn/hsm.c
> +++ b/drivers/virt/acrn/hsm.c
> @@ -45,19 +45,75 @@ static int acrn_dev_open(struct inode *inode, struct file *filp)
> static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> unsigned long ioctl_param)
> {
> + struct acrn_vm *vm = filp->private_data;
> + struct acrn_vm_creation *vm_param;
> + int ret = 0;
> +
> if (cmd == ACRN_IOCTL_GET_API_VERSION) {
> if (copy_to_user((void __user *)ioctl_param,
> &api_version, sizeof(api_version)))
> return -EFAULT;
> + return 0;
> }
>
> - return 0;
> + if (vm->vmid == ACRN_INVALID_VMID && cmd != ACRN_IOCTL_CREATE_VM) {
> + pr_err("ioctl 0x%x: Invalid VM state!\n", cmd);

For this whole driver, you have a real 'struct device' to use, please
use it for all of these error messages everywhere. dev_err() gives you
much more information than pr_err() does.

Same everywhere in this patch series.

thanks,

greg k-h