On Tue, Sep 22, 2020 at 07:43:00PM +0800, [email protected] wrote:
> From: Shuo Liu <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Zhi Wang <[email protected]>
> Reviewed-by: Reinette Chatre <[email protected]>
> Cc: Zhi Wang <[email protected]>
> Cc: Zhenyu Wang <[email protected]>
> Cc: Yu Wang <[email protected]>
> Cc: Reinette Chatre <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> ---
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 1 +
> drivers/virt/acrn/Makefile | 2 +-
> drivers/virt/acrn/acrn_drv.h | 23 +++++-
> drivers/virt/acrn/hsm.c | 73 ++++++++++++++++-
> drivers/virt/acrn/hypercall.h | 78 +++++++++++++++++++
> drivers/virt/acrn/vm.c | 71 +++++++++++++++++
> include/uapi/linux/acrn.h | 56 +++++++++++++
> 8 files changed, 301 insertions(+), 4 deletions(-)
> create mode 100644 drivers/virt/acrn/hypercall.h
> create mode 100644 drivers/virt/acrn/vm.c
> create mode 100644 include/uapi/linux/acrn.h
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 2a198838fca9..ac60efedb104 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -319,6 +319,7 @@ Code Seq# Include File Comments
> 0xA0 all linux/sdp/sdp.h Industrial Device Project
> <mailto:[email protected]>
> 0xA1 0 linux/vtpm_proxy.h TPM Emulator Proxy Driver
> +0xA2 all uapi/linux/acrn.h ACRN hypervisor
> 0xA3 80-8F Port ACL in development:
> <mailto:[email protected]>
> 0xA3 90-9F linux/dtlk.h
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3030d0e93d02..d4c1ef303c2d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -443,6 +443,7 @@ S: Supported
> W: https://projectacrn.org
> F: Documentation/virt/acrn/
> F: drivers/virt/acrn/
> +F: include/uapi/linux/acrn.h
>
> AD1889 ALSA SOUND DRIVER
> L: [email protected]
> 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 29eedd696327..72d92b60d944 100644
> --- a/drivers/virt/acrn/acrn_drv.h
> +++ b/drivers/virt/acrn/acrn_drv.h
> @@ -3,16 +3,37 @@
> #ifndef __ACRN_HSM_DRV_H
> #define __ACRN_HSM_DRV_H
>
> +#include <linux/acrn.h>
> +#include <linux/dev_printk.h>
> +#include <linux/miscdevice.h>
> #include <linux/types.h>
>
> +#include "hypercall.h"
> +
> +extern struct miscdevice acrn_dev;
> +
> #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 28a3052ffa55..f3e6467b8723 100644
> --- a/drivers/virt/acrn/hsm.c
> +++ b/drivers/virt/acrn/hsm.c
> @@ -9,7 +9,6 @@
> * Yakui Zhao <[email protected]>
> */
>
> -#include <linux/miscdevice.h>
> #include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> @@ -38,10 +37,79 @@ static int acrn_dev_open(struct inode *inode, struct file *filp)
> return 0;
> }
>
> +/*
> + * HSM relies on hypercall layer of the ACRN hypervisor to do the
> + * sanity check against the input parameters.
> + */
> +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 (vm->vmid == ACRN_INVALID_VMID && cmd != ACRN_IOCTL_CREATE_VM) {
> + dev_dbg(acrn_dev.this_device,
> + "ioctl 0x%x: Invalid VM state!\n", cmd);
> + return -EINVAL;
> + }
> +
> + switch (cmd) {
> + case ACRN_IOCTL_CREATE_VM:
> + vm_param = memdup_user((void __user *)ioctl_param,
> + sizeof(struct acrn_vm_creation));
> + if (IS_ERR(vm_param))
> + return PTR_ERR(vm_param);
> +
> + vm = acrn_vm_create(vm, vm_param);
> + if (!vm) {
> + ret = -EINVAL;
> + kfree(vm_param);
> + break;
> + }
> +
> + if (copy_to_user((void __user *)ioctl_param, vm_param,
> + sizeof(struct acrn_vm_creation))) {
> + acrn_vm_destroy(vm);
> + ret = -EFAULT;
> + }
> +
> + kfree(vm_param);
> + break;
> + case ACRN_IOCTL_START_VM:
> + ret = hcall_start_vm(vm->vmid);
> + if (ret < 0)
> + dev_err(acrn_dev.this_device,
> + "Failed to start VM %u!\n", vm->vmid);
> + break;
> + case ACRN_IOCTL_PAUSE_VM:
> + ret = hcall_pause_vm(vm->vmid);
> + if (ret < 0)
> + dev_err(acrn_dev.this_device,
> + "Failed to pause VM %u!\n", vm->vmid);
> + break;
> + case ACRN_IOCTL_RESET_VM:
> + ret = hcall_reset_vm(vm->vmid);
> + if (ret < 0)
> + dev_err(acrn_dev.this_device,
> + "Failed to restart VM %u!\n", vm->vmid);
> + break;
> + case ACRN_IOCTL_DESTROY_VM:
> + ret = acrn_vm_destroy(vm);
> + break;
> + default:
> + dev_warn(acrn_dev.this_device, "Unknown IOCTL 0x%x!\n", cmd);
> + ret = -ENOTTY;
> + }
> +
> + return ret;
> +}
> +
> static int acrn_dev_release(struct inode *inode, struct file *filp)
> {
> struct acrn_vm *vm = filp->private_data;
>
> + acrn_vm_destroy(vm);
> kfree(vm);
> return 0;
> }
> @@ -50,9 +118,10 @@ static const struct file_operations acrn_fops = {
> .owner = THIS_MODULE,
> .open = acrn_dev_open,
> .release = acrn_dev_release,
> + .unlocked_ioctl = acrn_dev_ioctl,
> };
>
> -static struct miscdevice acrn_dev = {
> +struct miscdevice acrn_dev = {
> .minor = MISC_DYNAMIC_MINOR,
> .name = "acrn_hsm",
> .fops = &acrn_fops,
> diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
> new file mode 100644
> index 000000000000..426b66cadb1f
> --- /dev/null
> +++ b/drivers/virt/acrn/hypercall.h
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * ACRN HSM: hypercalls of ACRN Hypervisor
> + */
> +#ifndef __ACRN_HSM_HYPERCALL_H
> +#define __ACRN_HSM_HYPERCALL_H
> +#include <asm/acrn.h>
> +
> +/*
> + * Hypercall IDs of the ACRN Hypervisor
> + */
> +#define _HC_ID(x, y) (((x) << 24) | (y))
> +
> +#define HC_ID 0x80UL
> +
> +#define HC_ID_VM_BASE 0x10UL
> +#define HC_CREATE_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x00)
> +#define HC_DESTROY_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x01)
> +#define HC_START_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x02)
> +#define HC_PAUSE_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x03)
> +#define HC_RESET_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x05)
> +
> +/**
> + * hcall_create_vm() - Create a User VM
> + * @vminfo: Service VM GPA of info of User VM creation
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_create_vm(u64 vminfo)
> +{
> + return acrn_hypercall1(HC_CREATE_VM, vminfo);
> +}
> +
> +/**
> + * hcall_start_vm() - Start a User VM
> + * @vmid: User VM ID
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_start_vm(u64 vmid)
> +{
> + return acrn_hypercall1(HC_START_VM, vmid);
> +}
> +
> +/**
> + * hcall_pause_vm() - Pause a User VM
> + * @vmid: User VM ID
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_pause_vm(u64 vmid)
> +{
> + return acrn_hypercall1(HC_PAUSE_VM, vmid);
> +}
> +
> +/**
> + * hcall_destroy_vm() - Destroy a User VM
> + * @vmid: User VM ID
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_destroy_vm(u64 vmid)
> +{
> + return acrn_hypercall1(HC_DESTROY_VM, vmid);
> +}
> +
> +/**
> + * hcall_reset_vm() - Reset a User VM
> + * @vmid: User VM ID
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_reset_vm(u64 vmid)
> +{
> + return acrn_hypercall1(HC_RESET_VM, vmid);
> +}
> +
> +#endif /* __ACRN_HSM_HYPERCALL_H */
> diff --git a/drivers/virt/acrn/vm.c b/drivers/virt/acrn/vm.c
> new file mode 100644
> index 000000000000..920ca48f4847
> --- /dev/null
> +++ b/drivers/virt/acrn/vm.c
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ACRN_HSM: Virtual Machine management
> + *
> + * Copyright (C) 2020 Intel Corporation. All rights reserved.
> + *
> + * Authors:
> + * Jason Chen CJ <[email protected]>
> + * Yakui Zhao <[email protected]>
> + */
> +#include <linux/io.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +
> +#include "acrn_drv.h"
> +
> +/* List of VMs */
> +LIST_HEAD(acrn_vm_list);
> +/*
> + * acrn_vm_list is read in a tasklet which dispatch I/O requests and is wrote
> + * in VM creation ioctl. Use the rwlock mechanism to protect it.
> + */
> +DEFINE_RWLOCK(acrn_vm_list_lock);
> +
> +struct acrn_vm *acrn_vm_create(struct acrn_vm *vm,
> + struct acrn_vm_creation *vm_param)
> +{
> + int ret;
> +
> + ret = hcall_create_vm(virt_to_phys(vm_param));
> + if (ret < 0 || vm_param->vmid == ACRN_INVALID_VMID) {
> + dev_err(acrn_dev.this_device,
> + "Failed to create VM! Error: %d\n", ret);
> + return NULL;
> + }
> +
> + vm->vmid = vm_param->vmid;
> + vm->vcpu_num = vm_param->vcpu_num;
> +
> + write_lock_bh(&acrn_vm_list_lock);
> + list_add(&vm->list, &acrn_vm_list);
> + write_unlock_bh(&acrn_vm_list_lock);
Why are the _bh() variants being used here?
You are only accessing this list from userspace context in this patch.
Heck, you aren't even reading from the list, only writing to it...
greg k-h
Hi Greg,
On Sun 27.Sep'20 at 12:47:02 +0200, Greg Kroah-Hartman wrote:
>On Tue, Sep 22, 2020 at 07:43:00PM +0800, [email protected] wrote:
>> From: Shuo Liu <[email protected]>
>>
>> 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 <[email protected]>
>> Reviewed-by: Zhi Wang <[email protected]>
>> Reviewed-by: Reinette Chatre <[email protected]>
>> Cc: Zhi Wang <[email protected]>
>> Cc: Zhenyu Wang <[email protected]>
>> Cc: Yu Wang <[email protected]>
>> Cc: Reinette Chatre <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> ---
>> .../userspace-api/ioctl/ioctl-number.rst | 1 +
>> MAINTAINERS | 1 +
>> drivers/virt/acrn/Makefile | 2 +-
>> drivers/virt/acrn/acrn_drv.h | 23 +++++-
>> drivers/virt/acrn/hsm.c | 73 ++++++++++++++++-
>> drivers/virt/acrn/hypercall.h | 78 +++++++++++++++++++
>> drivers/virt/acrn/vm.c | 71 +++++++++++++++++
>> include/uapi/linux/acrn.h | 56 +++++++++++++
>> 8 files changed, 301 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/virt/acrn/hypercall.h
>> create mode 100644 drivers/virt/acrn/vm.c
>> create mode 100644 include/uapi/linux/acrn.h
>>
>> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> index 2a198838fca9..ac60efedb104 100644
>> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
>> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> @@ -319,6 +319,7 @@ Code Seq# Include File Comments
>> 0xA0 all linux/sdp/sdp.h Industrial Device Project
>> <mailto:[email protected]>
>> 0xA1 0 linux/vtpm_proxy.h TPM Emulator Proxy Driver
>> +0xA2 all uapi/linux/acrn.h ACRN hypervisor
>> 0xA3 80-8F Port ACL in development:
>> <mailto:[email protected]>
>> 0xA3 90-9F linux/dtlk.h
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 3030d0e93d02..d4c1ef303c2d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -443,6 +443,7 @@ S: Supported
>> W: https://projectacrn.org
>> F: Documentation/virt/acrn/
>> F: drivers/virt/acrn/
>> +F: include/uapi/linux/acrn.h
>>
>> AD1889 ALSA SOUND DRIVER
>> L: [email protected]
>> 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 29eedd696327..72d92b60d944 100644
>> --- a/drivers/virt/acrn/acrn_drv.h
>> +++ b/drivers/virt/acrn/acrn_drv.h
>> @@ -3,16 +3,37 @@
>> #ifndef __ACRN_HSM_DRV_H
>> #define __ACRN_HSM_DRV_H
>>
>> +#include <linux/acrn.h>
>> +#include <linux/dev_printk.h>
>> +#include <linux/miscdevice.h>
>> #include <linux/types.h>
>>
>> +#include "hypercall.h"
>> +
>> +extern struct miscdevice acrn_dev;
>> +
>> #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 28a3052ffa55..f3e6467b8723 100644
>> --- a/drivers/virt/acrn/hsm.c
>> +++ b/drivers/virt/acrn/hsm.c
>> @@ -9,7 +9,6 @@
>> * Yakui Zhao <[email protected]>
>> */
>>
>> -#include <linux/miscdevice.h>
>> #include <linux/mm.h>
>> #include <linux/module.h>
>> #include <linux/slab.h>
>> @@ -38,10 +37,79 @@ static int acrn_dev_open(struct inode *inode, struct file *filp)
>> return 0;
>> }
>>
>> +/*
>> + * HSM relies on hypercall layer of the ACRN hypervisor to do the
>> + * sanity check against the input parameters.
>> + */
>> +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 (vm->vmid == ACRN_INVALID_VMID && cmd != ACRN_IOCTL_CREATE_VM) {
>> + dev_dbg(acrn_dev.this_device,
>> + "ioctl 0x%x: Invalid VM state!\n", cmd);
>> + return -EINVAL;
>> + }
>> +
>> + switch (cmd) {
>> + case ACRN_IOCTL_CREATE_VM:
>> + vm_param = memdup_user((void __user *)ioctl_param,
>> + sizeof(struct acrn_vm_creation));
>> + if (IS_ERR(vm_param))
>> + return PTR_ERR(vm_param);
>> +
>> + vm = acrn_vm_create(vm, vm_param);
>> + if (!vm) {
>> + ret = -EINVAL;
>> + kfree(vm_param);
>> + break;
>> + }
>> +
>> + if (copy_to_user((void __user *)ioctl_param, vm_param,
>> + sizeof(struct acrn_vm_creation))) {
>> + acrn_vm_destroy(vm);
>> + ret = -EFAULT;
>> + }
>> +
>> + kfree(vm_param);
>> + break;
>> + case ACRN_IOCTL_START_VM:
>> + ret = hcall_start_vm(vm->vmid);
>> + if (ret < 0)
>> + dev_err(acrn_dev.this_device,
>> + "Failed to start VM %u!\n", vm->vmid);
>> + break;
>> + case ACRN_IOCTL_PAUSE_VM:
>> + ret = hcall_pause_vm(vm->vmid);
>> + if (ret < 0)
>> + dev_err(acrn_dev.this_device,
>> + "Failed to pause VM %u!\n", vm->vmid);
>> + break;
>> + case ACRN_IOCTL_RESET_VM:
>> + ret = hcall_reset_vm(vm->vmid);
>> + if (ret < 0)
>> + dev_err(acrn_dev.this_device,
>> + "Failed to restart VM %u!\n", vm->vmid);
>> + break;
>> + case ACRN_IOCTL_DESTROY_VM:
>> + ret = acrn_vm_destroy(vm);
>> + break;
>> + default:
>> + dev_warn(acrn_dev.this_device, "Unknown IOCTL 0x%x!\n", cmd);
>> + ret = -ENOTTY;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int acrn_dev_release(struct inode *inode, struct file *filp)
>> {
>> struct acrn_vm *vm = filp->private_data;
>>
>> + acrn_vm_destroy(vm);
>> kfree(vm);
>> return 0;
>> }
>> @@ -50,9 +118,10 @@ static const struct file_operations acrn_fops = {
>> .owner = THIS_MODULE,
>> .open = acrn_dev_open,
>> .release = acrn_dev_release,
>> + .unlocked_ioctl = acrn_dev_ioctl,
>> };
>>
>> -static struct miscdevice acrn_dev = {
>> +struct miscdevice acrn_dev = {
>> .minor = MISC_DYNAMIC_MINOR,
>> .name = "acrn_hsm",
>> .fops = &acrn_fops,
>> diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
>> new file mode 100644
>> index 000000000000..426b66cadb1f
>> --- /dev/null
>> +++ b/drivers/virt/acrn/hypercall.h
>> @@ -0,0 +1,78 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * ACRN HSM: hypercalls of ACRN Hypervisor
>> + */
>> +#ifndef __ACRN_HSM_HYPERCALL_H
>> +#define __ACRN_HSM_HYPERCALL_H
>> +#include <asm/acrn.h>
>> +
>> +/*
>> + * Hypercall IDs of the ACRN Hypervisor
>> + */
>> +#define _HC_ID(x, y) (((x) << 24) | (y))
>> +
>> +#define HC_ID 0x80UL
>> +
>> +#define HC_ID_VM_BASE 0x10UL
>> +#define HC_CREATE_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x00)
>> +#define HC_DESTROY_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x01)
>> +#define HC_START_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x02)
>> +#define HC_PAUSE_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x03)
>> +#define HC_RESET_VM _HC_ID(HC_ID, HC_ID_VM_BASE + 0x05)
>> +
>> +/**
>> + * hcall_create_vm() - Create a User VM
>> + * @vminfo: Service VM GPA of info of User VM creation
>> + *
>> + * Return: 0 on success, <0 on failure
>> + */
>> +static inline long hcall_create_vm(u64 vminfo)
>> +{
>> + return acrn_hypercall1(HC_CREATE_VM, vminfo);
>> +}
>> +
>> +/**
>> + * hcall_start_vm() - Start a User VM
>> + * @vmid: User VM ID
>> + *
>> + * Return: 0 on success, <0 on failure
>> + */
>> +static inline long hcall_start_vm(u64 vmid)
>> +{
>> + return acrn_hypercall1(HC_START_VM, vmid);
>> +}
>> +
>> +/**
>> + * hcall_pause_vm() - Pause a User VM
>> + * @vmid: User VM ID
>> + *
>> + * Return: 0 on success, <0 on failure
>> + */
>> +static inline long hcall_pause_vm(u64 vmid)
>> +{
>> + return acrn_hypercall1(HC_PAUSE_VM, vmid);
>> +}
>> +
>> +/**
>> + * hcall_destroy_vm() - Destroy a User VM
>> + * @vmid: User VM ID
>> + *
>> + * Return: 0 on success, <0 on failure
>> + */
>> +static inline long hcall_destroy_vm(u64 vmid)
>> +{
>> + return acrn_hypercall1(HC_DESTROY_VM, vmid);
>> +}
>> +
>> +/**
>> + * hcall_reset_vm() - Reset a User VM
>> + * @vmid: User VM ID
>> + *
>> + * Return: 0 on success, <0 on failure
>> + */
>> +static inline long hcall_reset_vm(u64 vmid)
>> +{
>> + return acrn_hypercall1(HC_RESET_VM, vmid);
>> +}
>> +
>> +#endif /* __ACRN_HSM_HYPERCALL_H */
>> diff --git a/drivers/virt/acrn/vm.c b/drivers/virt/acrn/vm.c
>> new file mode 100644
>> index 000000000000..920ca48f4847
>> --- /dev/null
>> +++ b/drivers/virt/acrn/vm.c
>> @@ -0,0 +1,71 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * ACRN_HSM: Virtual Machine management
>> + *
>> + * Copyright (C) 2020 Intel Corporation. All rights reserved.
>> + *
>> + * Authors:
>> + * Jason Chen CJ <[email protected]>
>> + * Yakui Zhao <[email protected]>
>> + */
>> +#include <linux/io.h>
>> +#include <linux/mm.h>
>> +#include <linux/slab.h>
>> +
>> +#include "acrn_drv.h"
>> +
>> +/* List of VMs */
>> +LIST_HEAD(acrn_vm_list);
>> +/*
>> + * acrn_vm_list is read in a tasklet which dispatch I/O requests and is wrote
>> + * in VM creation ioctl. Use the rwlock mechanism to protect it.
>> + */
>> +DEFINE_RWLOCK(acrn_vm_list_lock);
>> +
>> +struct acrn_vm *acrn_vm_create(struct acrn_vm *vm,
>> + struct acrn_vm_creation *vm_param)
>> +{
>> + int ret;
>> +
>> + ret = hcall_create_vm(virt_to_phys(vm_param));
>> + if (ret < 0 || vm_param->vmid == ACRN_INVALID_VMID) {
>> + dev_err(acrn_dev.this_device,
>> + "Failed to create VM! Error: %d\n", ret);
>> + return NULL;
>> + }
>> +
>> + vm->vmid = vm_param->vmid;
>> + vm->vcpu_num = vm_param->vcpu_num;
>> +
>> + write_lock_bh(&acrn_vm_list_lock);
>> + list_add(&vm->list, &acrn_vm_list);
>> + write_unlock_bh(&acrn_vm_list_lock);
>
>Why are the _bh() variants being used here?
>
>You are only accessing this list from userspace context in this patch.
>
>Heck, you aren't even reading from the list, only writing to it...
acrn_vm_list is read in a tasklet which dispatch I/O requests and is wrote
in VM creation ioctl. Use the rwlock mechanism to protect it.
The reading operation is introduced in the following patches of this
series. So i keep the lock type at the moment of introduction.
Thanks
shuo