From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27625C432BE for ; Fri, 27 Aug 2021 08:47:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0DFCC60187 for ; Fri, 27 Aug 2021 08:47:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244597AbhH0Isj (ORCPT ); Fri, 27 Aug 2021 04:48:39 -0400 Received: from mail.kernel.org ([198.145.29.99]:47226 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232048AbhH0Isi (ORCPT ); Fri, 27 Aug 2021 04:48:38 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 6EF3960F92; Fri, 27 Aug 2021 08:47:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1630054069; bh=BxO8igqb4zLfhBhTE+eWClgMJlJyMFnkm8uBiJjMsbg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=M0vsFvQ6HYqhFnE9MBeSycaP6rTN8rd4x0vIGOjFGjpTCrVKwqW/o93GKSs5aD0gN bu2DY5zsdEV0kO+MiBC3fKEebhWWZASkarGt5QlLdA3qZQfDmRnTeD4LEFeU3NUqZ6 68O3OaEwfgLNxJJWCAFI5aM44CBu6YcBu5jIb5fg= Date: Fri, 27 Aug 2021 10:47:41 +0200 From: Greg KH To: Fei Li Cc: linux-kernel@vger.kernel.org, yu1.wang@intel.com, shuox.liu@gmail.com Subject: Re: [PATCH v2 2/3] virt: acrn: Introduce interfaces for virtual device creating/destroying Message-ID: References: <20210825090142.4418-1-fei1.li@intel.com> <20210825090142.4418-3-fei1.li@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210825090142.4418-3-fei1.li@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 25, 2021 at 05:01:41PM +0800, Fei Li wrote: > From: Shuo Liu > > 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 > Signed-off-by: Fei Li > --- > 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