[PATCH v2 0/3] Introduce some interfaces for ACRN hypervisor

[PATCH v2 0/3] Introduce some interfaces for ACRN hypervisor

[Fei Li]

Add some new interfaces for ACRN hypervisor HSM driver:
  - MMIO device passthrough
  - virtual device creating/destroying
  - platform information fetching from the hypervisor

ChangeLog:
v2:
 - remove "stable@vger.kernel.org" in the cc list since this patch set
   doesn't fix any anything, it's not for Linux -stable releases. 

Shuo Liu (3):
  virt: acrn: Introduce interfaces for MMIO device passthrough
  virt: acrn: Introduce interfaces for virtual device
    creating/destroying
  virt: acrn: Introduce interface to fetch platform info from the
    hypervisor

 drivers/virt/acrn/hsm.c       | 102 ++++++++++++++++++++++++++++++
 drivers/virt/acrn/hypercall.h |  64 +++++++++++++++++++
 include/uapi/linux/acrn.h     | 114 ++++++++++++++++++++++++++++++++++
 3 files changed, 280 insertions(+)


base-commit: 7c60610d476766e128cc4284bb6349732cbd6606
-- 
2.25.1

[PATCH v2 1/3] virt: acrn: Introduce interfaces for MMIO device passthrough

[Fei Li]

From: Shuo Liu <shuo.a.liu@intel.com>

MMIO device passthrough enables an OS in a virtual machine to directly
access a MMIO device in the host. It promises almost the native
performance, which is required in performance-critical scenarios of
ACRN.

HSM provides the following ioctls:
  - Assign - ACRN_IOCTL_ASSIGN_MMIODEV
    Pass data struct acrn_mmiodev from userspace to the hypervisor, and
    inform the hypervisor to assign a MMIO device to a User VM.

  - De-assign - ACRN_IOCTL_DEASSIGN_PCIDEV
    Pass data struct acrn_mmiodev from userspace to the hypervisor, and
    inform the hypervisor to de-assign a MMIO device from a User VM.

Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
Signed-off-by: Fei Li <fei1.li@intel.com>
---
 drivers/virt/acrn/hsm.c       | 25 +++++++++++++++++++++++++
 drivers/virt/acrn/hypercall.h | 26 ++++++++++++++++++++++++++
 include/uapi/linux/acrn.h     | 28 ++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)

diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
index 130e12b8652a..f567ca59d7c2 100644
--- a/drivers/virt/acrn/hsm.c
+++ b/drivers/virt/acrn/hsm.c
@@ -114,6 +114,7 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
 	struct acrn_ptdev_irq *irq_info;
 	struct acrn_ioeventfd ioeventfd;
 	struct acrn_vm_memmap memmap;
+	struct acrn_mmiodev *mmiodev;
 	struct acrn_msi_entry *msi;
 	struct acrn_pcidev *pcidev;
 	struct acrn_irqfd irqfd;
@@ -217,6 +218,30 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
 
 		ret = acrn_vm_memseg_unmap(vm, &memmap);
 		break;
+	case ACRN_IOCTL_ASSIGN_MMIODEV:
+		mmiodev = memdup_user((void __user *)ioctl_param,
+				      sizeof(struct acrn_mmiodev));
+		if (IS_ERR(mmiodev))
+			return PTR_ERR(mmiodev);
+
+		ret = hcall_assign_mmiodev(vm->vmid, virt_to_phys(mmiodev));
+		if (ret < 0)
+			dev_dbg(acrn_dev.this_device,
+				"Failed to assign MMIO device!\n");
+		kfree(mmiodev);
+		break;
+	case ACRN_IOCTL_DEASSIGN_MMIODEV:
+		mmiodev = memdup_user((void __user *)ioctl_param,
+				      sizeof(struct acrn_mmiodev));
+		if (IS_ERR(mmiodev))
+			return PTR_ERR(mmiodev);
+
+		ret = hcall_deassign_mmiodev(vm->vmid, virt_to_phys(mmiodev));
+		if (ret < 0)
+			dev_dbg(acrn_dev.this_device,
+				"Failed to deassign MMIO device!\n");
+		kfree(mmiodev);
+		break;
 	case ACRN_IOCTL_ASSIGN_PCIDEV:
 		pcidev = memdup_user((void __user *)ioctl_param,
 				     sizeof(struct acrn_pcidev));
diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
index 0cfad05bd1a9..f0c78e52cebb 100644
--- a/drivers/virt/acrn/hypercall.h
+++ b/drivers/virt/acrn/hypercall.h
@@ -41,6 +41,8 @@
 #define HC_RESET_PTDEV_INTR		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x04)
 #define HC_ASSIGN_PCIDEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x05)
 #define HC_DEASSIGN_PCIDEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x06)
+#define HC_ASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x07)
+#define HC_DEASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x08)
 
 #define HC_ID_PM_BASE			0x80UL
 #define HC_PM_GET_CPU_STATE		_HC_ID(HC_ID, HC_ID_PM_BASE + 0x00)
@@ -194,6 +196,30 @@ static inline long hcall_set_memory_regions(u64 regions_pa)
 	return acrn_hypercall1(HC_VM_SET_MEMORY_REGIONS, regions_pa);
 }
 
+/**
+ * hcall_assign_mmiodev() - Assign a MMIO device to a User VM
+ * @vmid:	User VM ID
+ * @addr:	Service VM GPA of the &struct acrn_mmiodev
+ *
+ * Return: 0 on success, <0 on failure
+ */
+static inline long hcall_assign_mmiodev(u64 vmid, u64 addr)
+{
+	return acrn_hypercall2(HC_ASSIGN_MMIODEV, vmid, addr);
+}
+
+/**
+ * hcall_deassign_mmiodev() - De-assign a PCI device from a User VM
+ * @vmid:	User VM ID
+ * @addr:	Service VM GPA of the &struct acrn_mmiodev
+ *
+ * Return: 0 on success, <0 on failure
+ */
+static inline long hcall_deassign_mmiodev(u64 vmid, u64 addr)
+{
+	return acrn_hypercall2(HC_DEASSIGN_MMIODEV, vmid, addr);
+}
+
 /**
  * hcall_assign_pcidev() - Assign a PCI device to a User VM
  * @vmid:	User VM ID
diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
index 353b2a2e4536..470036d6b1ac 100644
--- a/include/uapi/linux/acrn.h
+++ b/include/uapi/linux/acrn.h
@@ -396,6 +396,7 @@ struct acrn_ptdev_irq {
 /* Type of PCI device assignment */
 #define ACRN_PTDEV_QUIRK_ASSIGN	(1U << 0)
 
+#define ACRN_MMIODEV_RES_NUM	3
 #define ACRN_PCI_NUM_BARS	6
 /**
  * struct acrn_pcidev - Info for assigning or de-assigning a PCI device
@@ -417,6 +418,29 @@ struct acrn_pcidev {
 	__u32	bar[ACRN_PCI_NUM_BARS];
 };
 
+/**
+ * struct acrn_mmiodev - Info for assigning or de-assigning a MMIO device
+ * @name:			Name of the MMIO device.
+ * @res[].user_vm_pa:		Physical address of User VM of the MMIO region
+ *				for the MMIO device.
+ * @res[].service_vm_pa:	Physical address of Service VM of the MMIO
+ *				region for the MMIO device.
+ * @res[].size:			Size of the MMIO region for the MMIO device.
+ * @res[].mem_type:		Memory type of the MMIO region for the MMIO
+ *				device.
+ *
+ * This structure will be passed to hypervisor directly.
+ */
+struct acrn_mmiodev {
+	__u8	name[8];
+	struct {
+		__u64	user_vm_pa;
+		__u64	service_vm_pa;
+		__u64	size;
+		__u64	mem_type;
+	} res[ACRN_MMIODEV_RES_NUM];
+};
+
 /**
  * struct acrn_msi_entry - Info for injecting a MSI interrupt to a VM
  * @msi_addr:	MSI addr[19:12] with dest vCPU ID
@@ -568,6 +592,10 @@ struct acrn_irqfd {
 	_IOW(ACRN_IOCTL_TYPE, 0x55, struct acrn_pcidev)
 #define ACRN_IOCTL_DEASSIGN_PCIDEV	\
 	_IOW(ACRN_IOCTL_TYPE, 0x56, struct acrn_pcidev)
+#define ACRN_IOCTL_ASSIGN_MMIODEV	\
+	_IOW(ACRN_IOCTL_TYPE, 0x57, struct acrn_mmiodev)
+#define ACRN_IOCTL_DEASSIGN_MMIODEV	\
+	_IOW(ACRN_IOCTL_TYPE, 0x58, struct acrn_mmiodev)
 
 #define ACRN_IOCTL_PM_GET_CPU_STATE	\
 	_IOWR(ACRN_IOCTL_TYPE, 0x60, __u64)
-- 
2.25.1

[PATCH v2 2/3] virt: acrn: Introduce interfaces for virtual device creating/destroying

[Fei Li]

From: Shuo Liu <shuo.a.liu@intel.com>

The ACRN hypervisor can emulate a virtual device within hypervisor for a
Guest VM. The emulated virtual device can work without the ACRN
userspace after creation. The hypervisor do the emulation of that device.

To support the virtual device creating/destroying, HSM provides the
following ioctls:
  - ACRN_IOCTL_CREATE_VDEV
    Pass data struct acrn_vdev from userspace to the hypervisor, and inform
    the hypervisor to create a virtual device for a User VM.
  - ACRN_IOCTL_DESTROY_VDEV
    Pass data struct acrn_vdev from userspace to the hypervisor, and inform
    the hypervisor to destroy a virtual device of a User VM.

Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
Signed-off-by: Fei Li <fei1.li@intel.com>
---
 drivers/virt/acrn/hsm.c       | 24 ++++++++++++++++++++
 drivers/virt/acrn/hypercall.h | 26 ++++++++++++++++++++++
 include/uapi/linux/acrn.h     | 42 +++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)

diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
index f567ca59d7c2..5419794fccf1 100644
--- a/drivers/virt/acrn/hsm.c
+++ b/drivers/virt/acrn/hsm.c
@@ -118,6 +118,7 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
 	struct acrn_msi_entry *msi;
 	struct acrn_pcidev *pcidev;
 	struct acrn_irqfd irqfd;
+	struct acrn_vdev *vdev;
 	struct page *page;
 	u64 cstate_cmd;
 	int i, ret = 0;
@@ -266,6 +267,29 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
 				"Failed to deassign pci device!\n");
 		kfree(pcidev);
 		break;
+	case ACRN_IOCTL_CREATE_VDEV:
+		vdev = memdup_user((void __user *)ioctl_param,
+				   sizeof(struct acrn_vdev));
+		if (IS_ERR(vdev))
+			return PTR_ERR(vdev);
+
+		ret = hcall_create_vdev(vm->vmid, virt_to_phys(vdev));
+		if (ret < 0)
+			dev_dbg(acrn_dev.this_device,
+				"Failed to create virtual device!\n");
+		kfree(vdev);
+		break;
+	case ACRN_IOCTL_DESTROY_VDEV:
+		vdev = memdup_user((void __user *)ioctl_param,
+				   sizeof(struct acrn_vdev));
+		if (IS_ERR(vdev))
+			return PTR_ERR(vdev);
+		ret = hcall_destroy_vdev(vm->vmid, virt_to_phys(vdev));
+		if (ret < 0)
+			dev_dbg(acrn_dev.this_device,
+				"Failed to destroy virtual device!\n");
+		kfree(vdev);
+		break;
 	case ACRN_IOCTL_SET_PTDEV_INTR:
 		irq_info = memdup_user((void __user *)ioctl_param,
 				       sizeof(struct acrn_ptdev_irq));
diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
index f0c78e52cebb..71d300821a18 100644
--- a/drivers/virt/acrn/hypercall.h
+++ b/drivers/virt/acrn/hypercall.h
@@ -43,6 +43,8 @@
 #define HC_DEASSIGN_PCIDEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x06)
 #define HC_ASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x07)
 #define HC_DEASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x08)
+#define HC_CREATE_VDEV			_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x09)
+#define HC_DESTROY_VDEV			_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x0A)
 
 #define HC_ID_PM_BASE			0x80UL
 #define HC_PM_GET_CPU_STATE		_HC_ID(HC_ID, HC_ID_PM_BASE + 0x00)
@@ -196,6 +198,30 @@ static inline long hcall_set_memory_regions(u64 regions_pa)
 	return acrn_hypercall1(HC_VM_SET_MEMORY_REGIONS, regions_pa);
 }
 
+/**
+ * hcall_create_vdev() - Create a virtual device for a User VM
+ * @vmid:	User VM ID
+ * @addr:	Service VM GPA of the &struct acrn_vdev
+ *
+ * Return: 0 on success, <0 on failure
+ */
+static inline long hcall_create_vdev(u64 vmid, u64 addr)
+{
+	return acrn_hypercall2(HC_CREATE_VDEV, vmid, addr);
+}
+
+/**
+ * hcall_destroy_vdev() - Destroy a virtual device of a User VM
+ * @vmid:	User VM ID
+ * @addr:	Service VM GPA of the &struct acrn_vdev
+ *
+ * Return: 0 on success, <0 on failure
+ */
+static inline long hcall_destroy_vdev(u64 vmid, u64 addr)
+{
+	return acrn_hypercall2(HC_DESTROY_VDEV, vmid, addr);
+}
+
 /**
  * hcall_assign_mmiodev() - Assign a MMIO device to a User VM
  * @vmid:	User VM ID
diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
index 470036d6b1ac..1408d1063339 100644
--- a/include/uapi/linux/acrn.h
+++ b/include/uapi/linux/acrn.h
@@ -441,6 +441,44 @@ struct acrn_mmiodev {
 	} res[ACRN_MMIODEV_RES_NUM];
 };
 
+/**
+ * struct acrn_vdev - Info for creating or destroying a virtual device
+ * @id:				Union of identifier of the virtual device
+ * @id.value:			Raw data of the identifier
+ * @id.fields.vendor:		Vendor id of the virtual PCI device
+ * @id.fields.device:		Device id of the virtual PCI device
+ * @id.fields.legacy_id:	ID of the virtual device if not a PCI device
+ * @slot:			Virtual Bus/Device/Function of the virtual
+ *				device
+ * @io_base:			IO resource base address of the virtual device
+ * @io_size:			IO resource size of the virtual device
+ * @args:			Arguments for the virtual device creation
+ *
+ * The created virtual device can be a PCI device or a legacy device (e.g.
+ * a virtual UART controller) and it is emulated by the hypervisor. This
+ * structure will be passed to hypervisor directly.
+ */
+struct acrn_vdev {
+	/*
+	 * the identifier of the device, the low 32 bits represent the vendor
+	 * id and device id of PCI device and the high 32 bits represent the
+	 * device number of the legacy device
+	 */
+	union {
+		__u64 value;
+		struct {
+			__u16 vendor;
+			__u16 device;
+			__u32 legacy_id;
+		} fields;
+	} id;
+
+	__u64	slot;
+	__u32	io_addr[ACRN_PCI_NUM_BARS];
+	__u32	io_size[ACRN_PCI_NUM_BARS];
+	__u8	args[128];
+};
+
 /**
  * struct acrn_msi_entry - Info for injecting a MSI interrupt to a VM
  * @msi_addr:	MSI addr[19:12] with dest vCPU ID
@@ -596,6 +634,10 @@ struct acrn_irqfd {
 	_IOW(ACRN_IOCTL_TYPE, 0x57, struct acrn_mmiodev)
 #define ACRN_IOCTL_DEASSIGN_MMIODEV	\
 	_IOW(ACRN_IOCTL_TYPE, 0x58, struct acrn_mmiodev)
+#define ACRN_IOCTL_CREATE_VDEV	\
+	_IOW(ACRN_IOCTL_TYPE, 0x59, struct acrn_vdev)
+#define ACRN_IOCTL_DESTROY_VDEV	\
+	_IOW(ACRN_IOCTL_TYPE, 0x5A, struct acrn_vdev)
 
 #define ACRN_IOCTL_PM_GET_CPU_STATE	\
 	_IOWR(ACRN_IOCTL_TYPE, 0x60, __u64)
-- 
2.25.1

[PATCH v2 3/3] virt: acrn: Introduce interface to fetch platform info from the hypervisor

[Fei Li]

From: Shuo Liu <shuo.a.liu@intel.com>

The ACRN hypervisor configures the guest VMs information statically and
builds guest VM configurations within the hypervisor. There are also
some hardware information are stored in the hypervisor in boot stage.
The ACRN userspace needs platform information to do the orchestration.

The HSM provides the following interface for the ACRN userspace to fetch
platform info:
 - ACRN_IOCTL_GET_PLATFORM_INFO
   Exchange the basic information by a struct acrn_platform_info. If the
   ACRN userspace provides a userspace buffer (whose vma filled in
   vm_configs_addr), the HSM creates a bounce buffer (kmalloced for
   continuous memory region) to fetch VM configurations data from the
   hypervisor.

Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
Signed-off-by: Fei Li <fei1.li@intel.com>
---
 drivers/virt/acrn/hsm.c       | 53 +++++++++++++++++++++++++++++++++++
 drivers/virt/acrn/hypercall.h | 12 ++++++++
 include/uapi/linux/acrn.h     | 44 +++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+)

diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
index 5419794fccf1..eb824a1a86a0 100644
--- a/drivers/virt/acrn/hsm.c
+++ b/drivers/virt/acrn/hsm.c
@@ -108,6 +108,7 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
 			   unsigned long ioctl_param)
 {
 	struct acrn_vm *vm = filp->private_data;
+	struct acrn_platform_info *plat_info;
 	struct acrn_vm_creation *vm_param;
 	struct acrn_vcpu_regs *cpu_regs;
 	struct acrn_ioreq_notify notify;
@@ -115,9 +116,12 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
 	struct acrn_ioeventfd ioeventfd;
 	struct acrn_vm_memmap memmap;
 	struct acrn_mmiodev *mmiodev;
+	void __user *vm_configs_user;
 	struct acrn_msi_entry *msi;
 	struct acrn_pcidev *pcidev;
 	struct acrn_irqfd irqfd;
+	void *vm_configs = NULL;
+	size_t vm_configs_size;
 	struct acrn_vdev *vdev;
 	struct page *page;
 	u64 cstate_cmd;
@@ -130,6 +134,55 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
 	}
 
 	switch (cmd) {
+	case ACRN_IOCTL_GET_PLATFORM_INFO:
+		plat_info = memdup_user((void __user *)ioctl_param,
+					sizeof(struct acrn_platform_info));
+		if (IS_ERR(plat_info))
+			return PTR_ERR(plat_info);
+
+		for (i = 0; i < ARRAY_SIZE(plat_info->sw.reserved); i++)
+			if (plat_info->sw.reserved[i])
+				return -EINVAL;
+
+		for (i = 0; i < ARRAY_SIZE(plat_info->hw.reserved); i++)
+			if (plat_info->hw.reserved[i])
+				return -EINVAL;
+
+		vm_configs_size = plat_info->sw.vm_config_size *
+						plat_info->sw.max_vms;
+		if (plat_info->sw.vm_configs_addr && vm_configs_size) {
+			vm_configs_user = plat_info->sw.vm_configs_addr;
+			vm_configs = kzalloc(vm_configs_size, GFP_KERNEL);
+			if (IS_ERR(vm_configs)) {
+				kfree(plat_info);
+				return PTR_ERR(vm_configs);
+			}
+			plat_info->sw.vm_configs_addr =
+					(void __user *)virt_to_phys(vm_configs);
+		}
+
+		ret = hcall_get_platform_info(virt_to_phys(plat_info));
+		if (ret < 0) {
+			kfree(vm_configs);
+			kfree(plat_info);
+			dev_dbg(acrn_dev.this_device,
+				"Failed to get info of VM %u!\n", vm->vmid);
+			break;
+		}
+
+		if (vm_configs) {
+			if (copy_to_user(vm_configs_user, vm_configs,
+					 vm_configs_size))
+				ret = -EFAULT;
+			plat_info->sw.vm_configs_addr = vm_configs_user;
+		}
+		if (!ret && copy_to_user((void __user *)ioctl_param, plat_info,
+					 sizeof(*plat_info)))
+			ret = -EFAULT;
+
+		kfree(vm_configs);
+		kfree(plat_info);
+		break;
 	case ACRN_IOCTL_CREATE_VM:
 		vm_param = memdup_user((void __user *)ioctl_param,
 				       sizeof(struct acrn_vm_creation));
diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
index 71d300821a18..440e204d731a 100644
--- a/drivers/virt/acrn/hypercall.h
+++ b/drivers/virt/acrn/hypercall.h
@@ -15,6 +15,7 @@
 
 #define HC_ID_GEN_BASE			0x0UL
 #define HC_SOS_REMOVE_CPU		_HC_ID(HC_ID, HC_ID_GEN_BASE + 0x01)
+#define HC_GET_PLATFORM_INFO		_HC_ID(HC_ID, HC_ID_GEN_BASE + 0x03)
 
 #define HC_ID_VM_BASE			0x10UL
 #define HC_CREATE_VM			_HC_ID(HC_ID, HC_ID_VM_BASE + 0x00)
@@ -60,6 +61,17 @@ static inline long hcall_sos_remove_cpu(u64 cpu)
 	return acrn_hypercall1(HC_SOS_REMOVE_CPU, cpu);
 }
 
+/**
+ * hcall_get_platform_info() - Get platform information from the hypervisor
+ * @platform_info: Service VM GPA of the &struct acrn_platform_info
+ *
+ * Return: 0 on success, <0 on failure
+ */
+static inline long hcall_get_platform_info(u64 platform_info)
+{
+	return acrn_hypercall1(HC_GET_PLATFORM_INFO, platform_info);
+}
+
 /**
  * hcall_create_vm() - Create a User VM
  * @vminfo:	Service VM GPA of info of User VM creation
diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
index 1408d1063339..2675d17bc803 100644
--- a/include/uapi/linux/acrn.h
+++ b/include/uapi/linux/acrn.h
@@ -580,12 +580,56 @@ struct acrn_irqfd {
 	struct acrn_msi_entry	msi;
 };
 
+#define ACRN_PLATFORM_LAPIC_IDS_MAX	64
+/**
+ * struct acrn_platform_info - Information of a platform from hypervisor
+ * @hw.cpu_num:			Physical CPU number of the platform
+ * @hw.version:			Version of this structure
+ * @hw.l2_cat_shift:		Order of the number of threads sharing L2 cache
+ * @hw.l3_cat_shift:		Order of the number of threads sharing L3 cache
+ * @hw.lapic_ids:		IDs of LAPICs of all threads
+ * @hw.reserved:		Reserved for alignment and should be 0
+ * @sw.max_vcpus_per_vm:	Maximum number of vCPU of a VM
+ * @sw.max_vms:			Maximum number of VM
+ * @sw.vm_config_size:		Size of configuration of a VM
+ * @sw.vm_configss_addr:	Memory address which user space provided to
+ *				store the VM configurations
+ * @sw.max_kata_containers:	Maximum number of VM for Kata containers
+ * @sw.reserved:		Reserved for alignment and should be 0
+ *
+ * If vm_configs_addr is provided, the driver uses a bounce buffer (kmalloced
+ * for continuous memory region) to fetch VM configurations data from the
+ * hypervisor.
+ */
+struct acrn_platform_info {
+	struct {
+		__u16	cpu_num;
+		__u16	version;
+		__u32	l2_cat_shift;
+		__u32	l3_cat_shift;
+		__u8	lapic_ids[ACRN_PLATFORM_LAPIC_IDS_MAX];
+		__u8	reserved[52];
+	} hw;
+
+	struct {
+		__u16	max_vcpus_per_vm;
+		__u16	max_vms;
+		__u32	vm_config_size;
+		void	__user *vm_configs_addr;
+		__u64	max_kata_containers;
+		__u8	reserved[104];
+	} sw;
+};
+
 /* The ioctl type, documented in ioctl-number.rst */
 #define ACRN_IOCTL_TYPE			0xA2
 
 /*
  * Common IOCTL IDs definition for ACRN userspace
  */
+#define ACRN_IOCTL_GET_PLATFORM_INFO	\
+	_IOR(ACRN_IOCTL_TYPE, 0x03, struct acrn_platform_info)
+
 #define ACRN_IOCTL_CREATE_VM		\
 	_IOWR(ACRN_IOCTL_TYPE, 0x10, struct acrn_vm_creation)
 #define ACRN_IOCTL_DESTROY_VM		\
-- 
2.25.1

Re: [PATCH v2 1/3] virt: acrn: Introduce interfaces for MMIO device passthrough

[Greg KH]

On Wed, Aug 25, 2021 at 05:01:40PM +0800, Fei Li wrote:
> From: Shuo Liu <shuo.a.liu@intel.com>
> 
> MMIO device passthrough enables an OS in a virtual machine to directly
> access a MMIO device in the host. It promises almost the native
> performance, which is required in performance-critical scenarios of
> ACRN.
> 
> HSM provides the following ioctls:
>   - Assign - ACRN_IOCTL_ASSIGN_MMIODEV
>     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
>     inform the hypervisor to assign a MMIO device to a User VM.
> 
>   - De-assign - ACRN_IOCTL_DEASSIGN_PCIDEV
>     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
>     inform the hypervisor to de-assign a MMIO device from a User VM.
> 
> Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> Signed-off-by: Fei Li <fei1.li@intel.com>

Where is the userspace code that uses this new api?

thanks,

greg k-h

Re: [PATCH v2 1/3] virt: acrn: Introduce interfaces for MMIO device passthrough

[Li Fei1]

On Wed, Aug 25, 2021 at 10:43:29PM +0200, Greg KH wrote:
> On Wed, Aug 25, 2021 at 05:01:40PM +0800, Fei Li wrote:
> > From: Shuo Liu <shuo.a.liu@intel.com>
> > 
> > MMIO device passthrough enables an OS in a virtual machine to directly
> > access a MMIO device in the host. It promises almost the native
> > performance, which is required in performance-critical scenarios of
> > ACRN.
> > 
> > HSM provides the following ioctls:
> >   - Assign - ACRN_IOCTL_ASSIGN_MMIODEV
> >     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
> >     inform the hypervisor to assign a MMIO device to a User VM.
> > 
> >   - De-assign - ACRN_IOCTL_DEASSIGN_PCIDEV
> >     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
> >     inform the hypervisor to de-assign a MMIO device from a User VM.
> > 
> > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> > Signed-off-by: Fei Li <fei1.li@intel.com>
> 
> Where is the userspace code that uses this new api?

Hi Greg

An ACRN userspace application (like QEMU) would uses these new apis in
https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/core/vmmapi.c#L562
and
https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/core/vmmapi.c#L568

thanks.

> 
> thanks,
> 
> greg k-h

Re: [PATCH v2 1/3] virt: acrn: Introduce interfaces for MMIO device passthrough

[Greg KH]

On Thu, Aug 26, 2021 at 09:38:38AM +0800, Li Fei1 wrote:
> On Wed, Aug 25, 2021 at 10:43:29PM +0200, Greg KH wrote:
> > On Wed, Aug 25, 2021 at 05:01:40PM +0800, Fei Li wrote:
> > > From: Shuo Liu <shuo.a.liu@intel.com>
> > > 
> > > MMIO device passthrough enables an OS in a virtual machine to directly
> > > access a MMIO device in the host. It promises almost the native
> > > performance, which is required in performance-critical scenarios of
> > > ACRN.
> > > 
> > > HSM provides the following ioctls:
> > >   - Assign - ACRN_IOCTL_ASSIGN_MMIODEV
> > >     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
> > >     inform the hypervisor to assign a MMIO device to a User VM.
> > > 
> > >   - De-assign - ACRN_IOCTL_DEASSIGN_PCIDEV
> > >     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
> > >     inform the hypervisor to de-assign a MMIO device from a User VM.
> > > 
> > > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> > > Signed-off-by: Fei Li <fei1.li@intel.com>
> > 
> > Where is the userspace code that uses this new api?
> 
> Hi Greg
> 
> An ACRN userspace application (like QEMU) would uses these new apis in
> https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/core/vmmapi.c#L562
> and
> https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/core/vmmapi.c#L568

Please provide links in all of these patches when you resend them, to
the userspace code as well, so that we know how it all works together.

thanks,

greg k-h

Re: [PATCH v2 2/3] virt: acrn: Introduce interfaces for virtual device creating/destroying

[Greg KH]

On Wed, Aug 25, 2021 at 05:01:41PM +0800, Fei Li wrote:
> From: Shuo Liu <shuo.a.liu@intel.com>
> 
> The ACRN hypervisor can emulate a virtual device within hypervisor for a
> Guest VM. The emulated virtual device can work without the ACRN
> userspace after creation. The hypervisor do the emulation of that device.
> 
> To support the virtual device creating/destroying, HSM provides the
> following ioctls:
>   - ACRN_IOCTL_CREATE_VDEV
>     Pass data struct acrn_vdev from userspace to the hypervisor, and inform
>     the hypervisor to create a virtual device for a User VM.
>   - ACRN_IOCTL_DESTROY_VDEV
>     Pass data struct acrn_vdev from userspace to the hypervisor, and inform
>     the hypervisor to destroy a virtual device of a User VM.
> 
> Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> Signed-off-by: Fei Li <fei1.li@intel.com>
> ---
>  drivers/virt/acrn/hsm.c       | 24 ++++++++++++++++++++
>  drivers/virt/acrn/hypercall.h | 26 ++++++++++++++++++++++
>  include/uapi/linux/acrn.h     | 42 +++++++++++++++++++++++++++++++++++
>  3 files changed, 92 insertions(+)
> 
> diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
> index f567ca59d7c2..5419794fccf1 100644
> --- a/drivers/virt/acrn/hsm.c
> +++ b/drivers/virt/acrn/hsm.c
> @@ -118,6 +118,7 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
>  	struct acrn_msi_entry *msi;
>  	struct acrn_pcidev *pcidev;
>  	struct acrn_irqfd irqfd;
> +	struct acrn_vdev *vdev;
>  	struct page *page;
>  	u64 cstate_cmd;
>  	int i, ret = 0;
> @@ -266,6 +267,29 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
>  				"Failed to deassign pci device!\n");
>  		kfree(pcidev);
>  		break;
> +	case ACRN_IOCTL_CREATE_VDEV:
> +		vdev = memdup_user((void __user *)ioctl_param,
> +				   sizeof(struct acrn_vdev));
> +		if (IS_ERR(vdev))
> +			return PTR_ERR(vdev);
> +
> +		ret = hcall_create_vdev(vm->vmid, virt_to_phys(vdev));

No validation of the structure fields?


> +		if (ret < 0)
> +			dev_dbg(acrn_dev.this_device,
> +				"Failed to create virtual device!\n");
> +		kfree(vdev);
> +		break;
> +	case ACRN_IOCTL_DESTROY_VDEV:
> +		vdev = memdup_user((void __user *)ioctl_param,
> +				   sizeof(struct acrn_vdev));
> +		if (IS_ERR(vdev))
> +			return PTR_ERR(vdev);
> +		ret = hcall_destroy_vdev(vm->vmid, virt_to_phys(vdev));

Again, no validation?

> +		if (ret < 0)
> +			dev_dbg(acrn_dev.this_device,
> +				"Failed to destroy virtual device!\n");
> +		kfree(vdev);
> +		break;
>  	case ACRN_IOCTL_SET_PTDEV_INTR:
>  		irq_info = memdup_user((void __user *)ioctl_param,
>  				       sizeof(struct acrn_ptdev_irq));
> diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
> index f0c78e52cebb..71d300821a18 100644
> --- a/drivers/virt/acrn/hypercall.h
> +++ b/drivers/virt/acrn/hypercall.h
> @@ -43,6 +43,8 @@
>  #define HC_DEASSIGN_PCIDEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x06)
>  #define HC_ASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x07)
>  #define HC_DEASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x08)
> +#define HC_CREATE_VDEV			_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x09)
> +#define HC_DESTROY_VDEV			_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x0A)
>  
>  #define HC_ID_PM_BASE			0x80UL
>  #define HC_PM_GET_CPU_STATE		_HC_ID(HC_ID, HC_ID_PM_BASE + 0x00)
> @@ -196,6 +198,30 @@ static inline long hcall_set_memory_regions(u64 regions_pa)
>  	return acrn_hypercall1(HC_VM_SET_MEMORY_REGIONS, regions_pa);
>  }
>  
> +/**
> + * hcall_create_vdev() - Create a virtual device for a User VM
> + * @vmid:	User VM ID
> + * @addr:	Service VM GPA of the &struct acrn_vdev
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_create_vdev(u64 vmid, u64 addr)
> +{
> +	return acrn_hypercall2(HC_CREATE_VDEV, vmid, addr);
> +}
> +
> +/**
> + * hcall_destroy_vdev() - Destroy a virtual device of a User VM
> + * @vmid:	User VM ID
> + * @addr:	Service VM GPA of the &struct acrn_vdev
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_destroy_vdev(u64 vmid, u64 addr)
> +{
> +	return acrn_hypercall2(HC_DESTROY_VDEV, vmid, addr);
> +}
> +
>  /**
>   * hcall_assign_mmiodev() - Assign a MMIO device to a User VM
>   * @vmid:	User VM ID
> diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
> index 470036d6b1ac..1408d1063339 100644
> --- a/include/uapi/linux/acrn.h
> +++ b/include/uapi/linux/acrn.h
> @@ -441,6 +441,44 @@ struct acrn_mmiodev {
>  	} res[ACRN_MMIODEV_RES_NUM];
>  };
>  
> +/**
> + * struct acrn_vdev - Info for creating or destroying a virtual device
> + * @id:				Union of identifier of the virtual device
> + * @id.value:			Raw data of the identifier
> + * @id.fields.vendor:		Vendor id of the virtual PCI device
> + * @id.fields.device:		Device id of the virtual PCI device
> + * @id.fields.legacy_id:	ID of the virtual device if not a PCI device
> + * @slot:			Virtual Bus/Device/Function of the virtual
> + *				device
> + * @io_base:			IO resource base address of the virtual device
> + * @io_size:			IO resource size of the virtual device
> + * @args:			Arguments for the virtual device creation
> + *
> + * The created virtual device can be a PCI device or a legacy device (e.g.
> + * a virtual UART controller) and it is emulated by the hypervisor. This
> + * structure will be passed to hypervisor directly.
> + */
> +struct acrn_vdev {
> +	/*
> +	 * the identifier of the device, the low 32 bits represent the vendor
> +	 * id and device id of PCI device and the high 32 bits represent the
> +	 * device number of the legacy device
> +	 */
> +	union {
> +		__u64 value;
> +		struct {
> +			__u16 vendor;
> +			__u16 device;

Endian of these values?

> +			__u32 legacy_id;

What is "legacy"?  What types of devices?


> +		} fields;
> +	} id;
> +
> +	__u64	slot;
> +	__u32	io_addr[ACRN_PCI_NUM_BARS];
> +	__u32	io_size[ACRN_PCI_NUM_BARS];
> +	__u8	args[128];

What are these args for exactly?

> +};
> +
>  /**
>   * struct acrn_msi_entry - Info for injecting a MSI interrupt to a VM
>   * @msi_addr:	MSI addr[19:12] with dest vCPU ID
> @@ -596,6 +634,10 @@ struct acrn_irqfd {
>  	_IOW(ACRN_IOCTL_TYPE, 0x57, struct acrn_mmiodev)
>  #define ACRN_IOCTL_DEASSIGN_MMIODEV	\
>  	_IOW(ACRN_IOCTL_TYPE, 0x58, struct acrn_mmiodev)
> +#define ACRN_IOCTL_CREATE_VDEV	\
> +	_IOW(ACRN_IOCTL_TYPE, 0x59, struct acrn_vdev)
> +#define ACRN_IOCTL_DESTROY_VDEV	\
> +	_IOW(ACRN_IOCTL_TYPE, 0x5A, struct acrn_vdev)

Why do you need the full structure to destroy the device?

thanks,

greg k-h

Re: [PATCH v2 3/3] virt: acrn: Introduce interface to fetch platform info from the hypervisor

[Greg KH]

On Wed, Aug 25, 2021 at 05:01:42PM +0800, Fei Li wrote:
> From: Shuo Liu <shuo.a.liu@intel.com>
> 
> The ACRN hypervisor configures the guest VMs information statically and
> builds guest VM configurations within the hypervisor. There are also
> some hardware information are stored in the hypervisor in boot stage.
> The ACRN userspace needs platform information to do the orchestration.
> 
> The HSM provides the following interface for the ACRN userspace to fetch
> platform info:
>  - ACRN_IOCTL_GET_PLATFORM_INFO
>    Exchange the basic information by a struct acrn_platform_info. If the
>    ACRN userspace provides a userspace buffer (whose vma filled in
>    vm_configs_addr), the HSM creates a bounce buffer (kmalloced for
>    continuous memory region) to fetch VM configurations data from the
>    hypervisor.
> 
> Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> Signed-off-by: Fei Li <fei1.li@intel.com>
> ---
>  drivers/virt/acrn/hsm.c       | 53 +++++++++++++++++++++++++++++++++++
>  drivers/virt/acrn/hypercall.h | 12 ++++++++
>  include/uapi/linux/acrn.h     | 44 +++++++++++++++++++++++++++++
>  3 files changed, 109 insertions(+)
> 
> diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
> index 5419794fccf1..eb824a1a86a0 100644
> --- a/drivers/virt/acrn/hsm.c
> +++ b/drivers/virt/acrn/hsm.c
> @@ -108,6 +108,7 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
>  			   unsigned long ioctl_param)
>  {
>  	struct acrn_vm *vm = filp->private_data;
> +	struct acrn_platform_info *plat_info;
>  	struct acrn_vm_creation *vm_param;
>  	struct acrn_vcpu_regs *cpu_regs;
>  	struct acrn_ioreq_notify notify;
> @@ -115,9 +116,12 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
>  	struct acrn_ioeventfd ioeventfd;
>  	struct acrn_vm_memmap memmap;
>  	struct acrn_mmiodev *mmiodev;
> +	void __user *vm_configs_user;
>  	struct acrn_msi_entry *msi;
>  	struct acrn_pcidev *pcidev;
>  	struct acrn_irqfd irqfd;
> +	void *vm_configs = NULL;
> +	size_t vm_configs_size;
>  	struct acrn_vdev *vdev;
>  	struct page *page;
>  	u64 cstate_cmd;
> @@ -130,6 +134,55 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
>  	}
>  
>  	switch (cmd) {
> +	case ACRN_IOCTL_GET_PLATFORM_INFO:
> +		plat_info = memdup_user((void __user *)ioctl_param,
> +					sizeof(struct acrn_platform_info));
> +		if (IS_ERR(plat_info))
> +			return PTR_ERR(plat_info);
> +
> +		for (i = 0; i < ARRAY_SIZE(plat_info->sw.reserved); i++)
> +			if (plat_info->sw.reserved[i])
> +				return -EINVAL;
> +
> +		for (i = 0; i < ARRAY_SIZE(plat_info->hw.reserved); i++)
> +			if (plat_info->hw.reserved[i])
> +				return -EINVAL;
> +
> +		vm_configs_size = plat_info->sw.vm_config_size *
> +						plat_info->sw.max_vms;
> +		if (plat_info->sw.vm_configs_addr && vm_configs_size) {
> +			vm_configs_user = plat_info->sw.vm_configs_addr;
> +			vm_configs = kzalloc(vm_configs_size, GFP_KERNEL);
> +			if (IS_ERR(vm_configs)) {
> +				kfree(plat_info);
> +				return PTR_ERR(vm_configs);
> +			}
> +			plat_info->sw.vm_configs_addr =
> +					(void __user *)virt_to_phys(vm_configs);
> +		}
> +
> +		ret = hcall_get_platform_info(virt_to_phys(plat_info));
> +		if (ret < 0) {
> +			kfree(vm_configs);
> +			kfree(plat_info);
> +			dev_dbg(acrn_dev.this_device,
> +				"Failed to get info of VM %u!\n", vm->vmid);
> +			break;
> +		}
> +
> +		if (vm_configs) {
> +			if (copy_to_user(vm_configs_user, vm_configs,
> +					 vm_configs_size))
> +				ret = -EFAULT;
> +			plat_info->sw.vm_configs_addr = vm_configs_user;
> +		}
> +		if (!ret && copy_to_user((void __user *)ioctl_param, plat_info,
> +					 sizeof(*plat_info)))
> +			ret = -EFAULT;
> +
> +		kfree(vm_configs);
> +		kfree(plat_info);
> +		break;
>  	case ACRN_IOCTL_CREATE_VM:
>  		vm_param = memdup_user((void __user *)ioctl_param,
>  				       sizeof(struct acrn_vm_creation));
> diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
> index 71d300821a18..440e204d731a 100644
> --- a/drivers/virt/acrn/hypercall.h
> +++ b/drivers/virt/acrn/hypercall.h
> @@ -15,6 +15,7 @@
>  
>  #define HC_ID_GEN_BASE			0x0UL
>  #define HC_SOS_REMOVE_CPU		_HC_ID(HC_ID, HC_ID_GEN_BASE + 0x01)
> +#define HC_GET_PLATFORM_INFO		_HC_ID(HC_ID, HC_ID_GEN_BASE + 0x03)
>  
>  #define HC_ID_VM_BASE			0x10UL
>  #define HC_CREATE_VM			_HC_ID(HC_ID, HC_ID_VM_BASE + 0x00)
> @@ -60,6 +61,17 @@ static inline long hcall_sos_remove_cpu(u64 cpu)
>  	return acrn_hypercall1(HC_SOS_REMOVE_CPU, cpu);
>  }
>  
> +/**
> + * hcall_get_platform_info() - Get platform information from the hypervisor
> + * @platform_info: Service VM GPA of the &struct acrn_platform_info
> + *
> + * Return: 0 on success, <0 on failure
> + */
> +static inline long hcall_get_platform_info(u64 platform_info)
> +{
> +	return acrn_hypercall1(HC_GET_PLATFORM_INFO, platform_info);
> +}
> +
>  /**
>   * hcall_create_vm() - Create a User VM
>   * @vminfo:	Service VM GPA of info of User VM creation
> diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
> index 1408d1063339..2675d17bc803 100644
> --- a/include/uapi/linux/acrn.h
> +++ b/include/uapi/linux/acrn.h
> @@ -580,12 +580,56 @@ struct acrn_irqfd {
>  	struct acrn_msi_entry	msi;
>  };
>  
> +#define ACRN_PLATFORM_LAPIC_IDS_MAX	64
> +/**
> + * struct acrn_platform_info - Information of a platform from hypervisor
> + * @hw.cpu_num:			Physical CPU number of the platform
> + * @hw.version:			Version of this structure
> + * @hw.l2_cat_shift:		Order of the number of threads sharing L2 cache
> + * @hw.l3_cat_shift:		Order of the number of threads sharing L3 cache
> + * @hw.lapic_ids:		IDs of LAPICs of all threads
> + * @hw.reserved:		Reserved for alignment and should be 0

"must be"

> + * @sw.max_vcpus_per_vm:	Maximum number of vCPU of a VM
> + * @sw.max_vms:			Maximum number of VM
> + * @sw.vm_config_size:		Size of configuration of a VM
> + * @sw.vm_configss_addr:	Memory address which user space provided to
> + *				store the VM configurations
> + * @sw.max_kata_containers:	Maximum number of VM for Kata containers
> + * @sw.reserved:		Reserved for alignment and should be 0

"must be"

> + *
> + * If vm_configs_addr is provided, the driver uses a bounce buffer (kmalloced
> + * for continuous memory region) to fetch VM configurations data from the
> + * hypervisor.
> + */
> +struct acrn_platform_info {
> +	struct {
> +		__u16	cpu_num;
> +		__u16	version;
> +		__u32	l2_cat_shift;
> +		__u32	l3_cat_shift;
> +		__u8	lapic_ids[ACRN_PLATFORM_LAPIC_IDS_MAX];
> +		__u8	reserved[52];

These are huge buffer padding, why so large?

> +	} hw;
> +
> +	struct {
> +		__u16	max_vcpus_per_vm;
> +		__u16	max_vms;
> +		__u32	vm_config_size;
> +		void	__user *vm_configs_addr;

pointers do not work in ioctl structures, please fix.

thanks,

greg k-h

Re: [PATCH v2 1/3] virt: acrn: Introduce interfaces for MMIO device passthrough

[Li Fei1]

On Fri, Aug 27, 2021 at 10:45:32AM +0200, Greg KH wrote:
> On Thu, Aug 26, 2021 at 09:38:38AM +0800, Li Fei1 wrote:
> > On Wed, Aug 25, 2021 at 10:43:29PM +0200, Greg KH wrote:
> > > On Wed, Aug 25, 2021 at 05:01:40PM +0800, Fei Li wrote:
> > > > From: Shuo Liu <shuo.a.liu@intel.com>
> > > > 
> > > > MMIO device passthrough enables an OS in a virtual machine to directly
> > > > access a MMIO device in the host. It promises almost the native
> > > > performance, which is required in performance-critical scenarios of
> > > > ACRN.
> > > > 
> > > > HSM provides the following ioctls:
> > > >   - Assign - ACRN_IOCTL_ASSIGN_MMIODEV
> > > >     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
> > > >     inform the hypervisor to assign a MMIO device to a User VM.
> > > > 
> > > >   - De-assign - ACRN_IOCTL_DEASSIGN_PCIDEV
> > > >     Pass data struct acrn_mmiodev from userspace to the hypervisor, and
> > > >     inform the hypervisor to de-assign a MMIO device from a User VM.
> > > > 
> > > > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> > > > Signed-off-by: Fei Li <fei1.li@intel.com>
> > > 
> > > Where is the userspace code that uses this new api?
> > 
> > Hi Greg
> > 
> > An ACRN userspace application (like QEMU) would uses these new apis in
> > https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/core/vmmapi.c#L562
> > and
> > https://github.com/projectacrn/acrn-hypervisor/blob/master/devicemodel/core/vmmapi.c#L568

Hi Greg

Sorry to reply late.
> 
> Please provide links in all of these patches when you resend them, to
> the userspace code as well, so that we know how it all works together.
> 

Sure.


> thanks,
> 
> greg k-h

Re: [PATCH v2 2/3] virt: acrn: Introduce interfaces for virtual device creating/destroying

[Li Fei1]

On Fri, Aug 27, 2021 at 10:47:41AM +0200, Greg KH wrote:
> On Wed, Aug 25, 2021 at 05:01:41PM +0800, Fei Li wrote:
> > From: Shuo Liu <shuo.a.liu@intel.com>
> > 
> > The ACRN hypervisor can emulate a virtual device within hypervisor for a
> > Guest VM. The emulated virtual device can work without the ACRN
> > userspace after creation. The hypervisor do the emulation of that device.
> > 
> > To support the virtual device creating/destroying, HSM provides the
> > following ioctls:
> >   - ACRN_IOCTL_CREATE_VDEV
> >     Pass data struct acrn_vdev from userspace to the hypervisor, and inform
> >     the hypervisor to create a virtual device for a User VM.
> >   - ACRN_IOCTL_DESTROY_VDEV
> >     Pass data struct acrn_vdev from userspace to the hypervisor, and inform
> >     the hypervisor to destroy a virtual device of a User VM.
> > 
> > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> > Signed-off-by: Fei Li <fei1.li@intel.com>
> > ---
> >  drivers/virt/acrn/hsm.c       | 24 ++++++++++++++++++++
> >  drivers/virt/acrn/hypercall.h | 26 ++++++++++++++++++++++
> >  include/uapi/linux/acrn.h     | 42 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 92 insertions(+)
> > 
> > diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
> > index f567ca59d7c2..5419794fccf1 100644
> > --- a/drivers/virt/acrn/hsm.c
> > +++ b/drivers/virt/acrn/hsm.c
> > @@ -118,6 +118,7 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> >  	struct acrn_msi_entry *msi;
> >  	struct acrn_pcidev *pcidev;
> >  	struct acrn_irqfd irqfd;
> > +	struct acrn_vdev *vdev;
> >  	struct page *page;
> >  	u64 cstate_cmd;
> >  	int i, ret = 0;
> > @@ -266,6 +267,29 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> >  				"Failed to deassign pci device!\n");
> >  		kfree(pcidev);
> >  		break;
> > +	case ACRN_IOCTL_CREATE_VDEV:
> > +		vdev = memdup_user((void __user *)ioctl_param,
> > +				   sizeof(struct acrn_vdev));
> > +		if (IS_ERR(vdev))
> > +			return PTR_ERR(vdev);
> > +
> > +		ret = hcall_create_vdev(vm->vmid, virt_to_phys(vdev));
Hi Greg

> 
> No validation of the structure fields?

Service VM knows nothing about how to create these vdevs, it only needs to
pass through this structure to ACRN Hypervisor to let ACRN Hypervisor check it.
> 
> 
> > +		if (ret < 0)
> > +			dev_dbg(acrn_dev.this_device,
> > +				"Failed to create virtual device!\n");
> > +		kfree(vdev);
> > +		break;
> > +	case ACRN_IOCTL_DESTROY_VDEV:
> > +		vdev = memdup_user((void __user *)ioctl_param,
> > +				   sizeof(struct acrn_vdev));
> > +		if (IS_ERR(vdev))
> > +			return PTR_ERR(vdev);
> > +		ret = hcall_destroy_vdev(vm->vmid, virt_to_phys(vdev));
> 
> Again, no validation?

Service VM knows nothing about how to destroy these vdevs, it only needs to
pass through this structure to ACRN Hypervisor to let ACRN Hypervisor check it.

> 
> > +		if (ret < 0)
> > +			dev_dbg(acrn_dev.this_device,
> > +				"Failed to destroy virtual device!\n");
> > +		kfree(vdev);
> > +		break;
> >  	case ACRN_IOCTL_SET_PTDEV_INTR:
> >  		irq_info = memdup_user((void __user *)ioctl_param,
> >  				       sizeof(struct acrn_ptdev_irq));
> > diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
> > index f0c78e52cebb..71d300821a18 100644
> > --- a/drivers/virt/acrn/hypercall.h
> > +++ b/drivers/virt/acrn/hypercall.h
> > @@ -43,6 +43,8 @@
> >  #define HC_DEASSIGN_PCIDEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x06)
> >  #define HC_ASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x07)
> >  #define HC_DEASSIGN_MMIODEV		_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x08)
> > +#define HC_CREATE_VDEV			_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x09)
> > +#define HC_DESTROY_VDEV			_HC_ID(HC_ID, HC_ID_PCI_BASE + 0x0A)
> >  
> >  #define HC_ID_PM_BASE			0x80UL
> >  #define HC_PM_GET_CPU_STATE		_HC_ID(HC_ID, HC_ID_PM_BASE + 0x00)
> > @@ -196,6 +198,30 @@ static inline long hcall_set_memory_regions(u64 regions_pa)
> >  	return acrn_hypercall1(HC_VM_SET_MEMORY_REGIONS, regions_pa);
> >  }
> >  
> > +/**
> > + * hcall_create_vdev() - Create a virtual device for a User VM
> > + * @vmid:	User VM ID
> > + * @addr:	Service VM GPA of the &struct acrn_vdev
> > + *
> > + * Return: 0 on success, <0 on failure
> > + */
> > +static inline long hcall_create_vdev(u64 vmid, u64 addr)
> > +{
> > +	return acrn_hypercall2(HC_CREATE_VDEV, vmid, addr);
> > +}
> > +
> > +/**
> > + * hcall_destroy_vdev() - Destroy a virtual device of a User VM
> > + * @vmid:	User VM ID
> > + * @addr:	Service VM GPA of the &struct acrn_vdev
> > + *
> > + * Return: 0 on success, <0 on failure
> > + */
> > +static inline long hcall_destroy_vdev(u64 vmid, u64 addr)
> > +{
> > +	return acrn_hypercall2(HC_DESTROY_VDEV, vmid, addr);
> > +}
> > +
> >  /**
> >   * hcall_assign_mmiodev() - Assign a MMIO device to a User VM
> >   * @vmid:	User VM ID
> > diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
> > index 470036d6b1ac..1408d1063339 100644
> > --- a/include/uapi/linux/acrn.h
> > +++ b/include/uapi/linux/acrn.h
> > @@ -441,6 +441,44 @@ struct acrn_mmiodev {
> >  	} res[ACRN_MMIODEV_RES_NUM];
> >  };
> >  
> > +/**
> > + * struct acrn_vdev - Info for creating or destroying a virtual device
> > + * @id:				Union of identifier of the virtual device
> > + * @id.value:			Raw data of the identifier
> > + * @id.fields.vendor:		Vendor id of the virtual PCI device
> > + * @id.fields.device:		Device id of the virtual PCI device
> > + * @id.fields.legacy_id:	ID of the virtual device if not a PCI device
> > + * @slot:			Virtual Bus/Device/Function of the virtual
> > + *				device
> > + * @io_base:			IO resource base address of the virtual device
> > + * @io_size:			IO resource size of the virtual device
> > + * @args:			Arguments for the virtual device creation
> > + *
> > + * The created virtual device can be a PCI device or a legacy device (e.g.
> > + * a virtual UART controller) and it is emulated by the hypervisor. This
> > + * structure will be passed to hypervisor directly.
> > + */
> > +struct acrn_vdev {
> > +	/*
> > +	 * the identifier of the device, the low 32 bits represent the vendor
> > +	 * id and device id of PCI device and the high 32 bits represent the
> > +	 * device number of the legacy device
> > +	 */
> > +	union {
> > +		__u64 value;
> > +		struct {
> > +			__u16 vendor;
> > +			__u16 device;
> 
> Endian of these values?

little-endian or big-endian ?


> 
> > +			__u32 legacy_id;
> 
> What is "legacy"?  What types of devices?

A PCI device which under a PCI bridge.
> 
> 
> > +		} fields;
> > +	} id;
> > +
> > +	__u64	slot;
> > +	__u32	io_addr[ACRN_PCI_NUM_BARS];
> > +	__u32	io_size[ACRN_PCI_NUM_BARS];
> > +	__u8	args[128];
> 
> What are these args for exactly?
For different kinds of vdevs, it represents differently.
For current usages, it may be:
a) a vdev's name of a virtual PCI device which used to communicate between VMs
b) an index of virtual Uart
c) a structure to represent a virtual Root Port.


> 
> > +};
> > +
> >  /**
> >   * struct acrn_msi_entry - Info for injecting a MSI interrupt to a VM
> >   * @msi_addr:	MSI addr[19:12] with dest vCPU ID
> > @@ -596,6 +634,10 @@ struct acrn_irqfd {
> >  	_IOW(ACRN_IOCTL_TYPE, 0x57, struct acrn_mmiodev)
> >  #define ACRN_IOCTL_DEASSIGN_MMIODEV	\
> >  	_IOW(ACRN_IOCTL_TYPE, 0x58, struct acrn_mmiodev)
> > +#define ACRN_IOCTL_CREATE_VDEV	\
> > +	_IOW(ACRN_IOCTL_TYPE, 0x59, struct acrn_vdev)
> > +#define ACRN_IOCTL_DESTROY_VDEV	\
> > +	_IOW(ACRN_IOCTL_TYPE, 0x5A, struct acrn_vdev)
> 
> Why do you need the full structure to destroy the device?
We need the id field + other fields (optional, the slot or the args) to identify a vdev.

thanks.
> 
> thanks,
> 
> greg k-h

Re: [PATCH v2 3/3] virt: acrn: Introduce interface to fetch platform info from the hypervisor

[Li Fei1]

On Fri, Aug 27, 2021 at 10:49:47AM +0200, Greg KH wrote:
> On Wed, Aug 25, 2021 at 05:01:42PM +0800, Fei Li wrote:
> > From: Shuo Liu <shuo.a.liu@intel.com>
> > 
> > The ACRN hypervisor configures the guest VMs information statically and
> > builds guest VM configurations within the hypervisor. There are also
> > some hardware information are stored in the hypervisor in boot stage.
> > The ACRN userspace needs platform information to do the orchestration.
> > 
> > The HSM provides the following interface for the ACRN userspace to fetch
> > platform info:
> >  - ACRN_IOCTL_GET_PLATFORM_INFO
> >    Exchange the basic information by a struct acrn_platform_info. If the
> >    ACRN userspace provides a userspace buffer (whose vma filled in
> >    vm_configs_addr), the HSM creates a bounce buffer (kmalloced for
> >    continuous memory region) to fetch VM configurations data from the
> >    hypervisor.
> > 
> > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
> > Signed-off-by: Fei Li <fei1.li@intel.com>
> > ---
> >  drivers/virt/acrn/hsm.c       | 53 +++++++++++++++++++++++++++++++++++
> >  drivers/virt/acrn/hypercall.h | 12 ++++++++
> >  include/uapi/linux/acrn.h     | 44 +++++++++++++++++++++++++++++
> >  3 files changed, 109 insertions(+)
> > 
> > diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c
> > index 5419794fccf1..eb824a1a86a0 100644
> > --- a/drivers/virt/acrn/hsm.c
> > +++ b/drivers/virt/acrn/hsm.c
> > @@ -108,6 +108,7 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> >  			   unsigned long ioctl_param)
> >  {
> >  	struct acrn_vm *vm = filp->private_data;
> > +	struct acrn_platform_info *plat_info;
> >  	struct acrn_vm_creation *vm_param;
> >  	struct acrn_vcpu_regs *cpu_regs;
> >  	struct acrn_ioreq_notify notify;
> > @@ -115,9 +116,12 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> >  	struct acrn_ioeventfd ioeventfd;
> >  	struct acrn_vm_memmap memmap;
> >  	struct acrn_mmiodev *mmiodev;
> > +	void __user *vm_configs_user;
> >  	struct acrn_msi_entry *msi;
> >  	struct acrn_pcidev *pcidev;
> >  	struct acrn_irqfd irqfd;
> > +	void *vm_configs = NULL;
> > +	size_t vm_configs_size;
> >  	struct acrn_vdev *vdev;
> >  	struct page *page;
> >  	u64 cstate_cmd;
> > @@ -130,6 +134,55 @@ static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> >  	}
> >  
> >  	switch (cmd) {
> > +	case ACRN_IOCTL_GET_PLATFORM_INFO:
> > +		plat_info = memdup_user((void __user *)ioctl_param,
> > +					sizeof(struct acrn_platform_info));
> > +		if (IS_ERR(plat_info))
> > +			return PTR_ERR(plat_info);
> > +
> > +		for (i = 0; i < ARRAY_SIZE(plat_info->sw.reserved); i++)
> > +			if (plat_info->sw.reserved[i])
> > +				return -EINVAL;
> > +
> > +		for (i = 0; i < ARRAY_SIZE(plat_info->hw.reserved); i++)
> > +			if (plat_info->hw.reserved[i])
> > +				return -EINVAL;
> > +
> > +		vm_configs_size = plat_info->sw.vm_config_size *
> > +						plat_info->sw.max_vms;
> > +		if (plat_info->sw.vm_configs_addr && vm_configs_size) {
> > +			vm_configs_user = plat_info->sw.vm_configs_addr;
> > +			vm_configs = kzalloc(vm_configs_size, GFP_KERNEL);
> > +			if (IS_ERR(vm_configs)) {
> > +				kfree(plat_info);
> > +				return PTR_ERR(vm_configs);
> > +			}
> > +			plat_info->sw.vm_configs_addr =
> > +					(void __user *)virt_to_phys(vm_configs);
> > +		}
> > +
> > +		ret = hcall_get_platform_info(virt_to_phys(plat_info));
> > +		if (ret < 0) {
> > +			kfree(vm_configs);
> > +			kfree(plat_info);
> > +			dev_dbg(acrn_dev.this_device,
> > +				"Failed to get info of VM %u!\n", vm->vmid);
> > +			break;
> > +		}
> > +
> > +		if (vm_configs) {
> > +			if (copy_to_user(vm_configs_user, vm_configs,
> > +					 vm_configs_size))
> > +				ret = -EFAULT;
> > +			plat_info->sw.vm_configs_addr = vm_configs_user;
> > +		}
> > +		if (!ret && copy_to_user((void __user *)ioctl_param, plat_info,
> > +					 sizeof(*plat_info)))
> > +			ret = -EFAULT;
> > +
> > +		kfree(vm_configs);
> > +		kfree(plat_info);
> > +		break;
> >  	case ACRN_IOCTL_CREATE_VM:
> >  		vm_param = memdup_user((void __user *)ioctl_param,
> >  				       sizeof(struct acrn_vm_creation));
> > diff --git a/drivers/virt/acrn/hypercall.h b/drivers/virt/acrn/hypercall.h
> > index 71d300821a18..440e204d731a 100644
> > --- a/drivers/virt/acrn/hypercall.h
> > +++ b/drivers/virt/acrn/hypercall.h
> > @@ -15,6 +15,7 @@
> >  
> >  #define HC_ID_GEN_BASE			0x0UL
> >  #define HC_SOS_REMOVE_CPU		_HC_ID(HC_ID, HC_ID_GEN_BASE + 0x01)
> > +#define HC_GET_PLATFORM_INFO		_HC_ID(HC_ID, HC_ID_GEN_BASE + 0x03)
> >  
> >  #define HC_ID_VM_BASE			0x10UL
> >  #define HC_CREATE_VM			_HC_ID(HC_ID, HC_ID_VM_BASE + 0x00)
> > @@ -60,6 +61,17 @@ static inline long hcall_sos_remove_cpu(u64 cpu)
> >  	return acrn_hypercall1(HC_SOS_REMOVE_CPU, cpu);
> >  }
> >  
> > +/**
> > + * hcall_get_platform_info() - Get platform information from the hypervisor
> > + * @platform_info: Service VM GPA of the &struct acrn_platform_info
> > + *
> > + * Return: 0 on success, <0 on failure
> > + */
> > +static inline long hcall_get_platform_info(u64 platform_info)
> > +{
> > +	return acrn_hypercall1(HC_GET_PLATFORM_INFO, platform_info);
> > +}
> > +
> >  /**
> >   * hcall_create_vm() - Create a User VM
> >   * @vminfo:	Service VM GPA of info of User VM creation
> > diff --git a/include/uapi/linux/acrn.h b/include/uapi/linux/acrn.h
> > index 1408d1063339..2675d17bc803 100644
> > --- a/include/uapi/linux/acrn.h
> > +++ b/include/uapi/linux/acrn.h
> > @@ -580,12 +580,56 @@ struct acrn_irqfd {
> >  	struct acrn_msi_entry	msi;
> >  };
> >  
Hi Greg

> > +#define ACRN_PLATFORM_LAPIC_IDS_MAX	64
> > +/**
> > + * struct acrn_platform_info - Information of a platform from hypervisor
> > + * @hw.cpu_num:			Physical CPU number of the platform
> > + * @hw.version:			Version of this structure
> > + * @hw.l2_cat_shift:		Order of the number of threads sharing L2 cache
> > + * @hw.l3_cat_shift:		Order of the number of threads sharing L3 cache
> > + * @hw.lapic_ids:		IDs of LAPICs of all threads
> > + * @hw.reserved:		Reserved for alignment and should be 0
> 
> "must be"
Would change it.
> 
> > + * @sw.max_vcpus_per_vm:	Maximum number of vCPU of a VM
> > + * @sw.max_vms:			Maximum number of VM
> > + * @sw.vm_config_size:		Size of configuration of a VM
> > + * @sw.vm_configss_addr:	Memory address which user space provided to
> > + *				store the VM configurations
> > + * @sw.max_kata_containers:	Maximum number of VM for Kata containers
> > + * @sw.reserved:		Reserved for alignment and should be 0
> 
> "must be"
Would change it.
> 
> > + *
> > + * If vm_configs_addr is provided, the driver uses a bounce buffer (kmalloced
> > + * for continuous memory region) to fetch VM configurations data from the
> > + * hypervisor.
> > + */
> > +struct acrn_platform_info {
> > +	struct {
> > +		__u16	cpu_num;
> > +		__u16	version;
> > +		__u32	l2_cat_shift;
> > +		__u32	l3_cat_shift;
> > +		__u8	lapic_ids[ACRN_PLATFORM_LAPIC_IDS_MAX];
> > +		__u8	reserved[52];
> 
> These are huge buffer padding, why so large?

We hope the hw structure should by 128 Bytes aligned. Maybe enlarge length of
_lapic_ids field from 64 to 116 and remove reserved field ?

> 
> > +	} hw;
> > +
> > +	struct {
> > +		__u16	max_vcpus_per_vm;
> > +		__u16	max_vms;
> > +		__u32	vm_config_size;
> > +		void	__user *vm_configs_addr;
> 
> pointers do not work in ioctl structures, please fix.

Could I change it to "__u64 vm_configs_addr" then convert its type to
"void    __user *" just like  ioctl_param or define a large data here
__u8 vm_configs[0x8000] ?

thanks.

> 
> thanks,
> 
> greg k-h