Re: [Intel-gfx] [PATCH v9 10/25] vfio: Make vfio_device_open() single open for device cdev path

Hi Yi, On 4/1/23 17:18, Yi Liu wrote: > VFIO group has historically allowed multi-open of the device FD. This > was made secure because the "open" was executed via an ioctl to the > group FD which is itself only single open. > > However, no known use of multiple device FDs today. It is kind of a >

Re: [Intel-gfx] [PATCH v9 06/25] kvm/vfio: Accept vfio device file from userspace

Hi Yi, On 4/7/23 05:42, Liu, Yi L wrote: >> From: Alex Williamson >> Sent: Friday, April 7, 2023 2:58 AM You don't say anything about potential restriction, ie. what if the user calls KVM_DEV_VFIO_FILE with device fds while it has been using legacy >> container/group API? >>>

Re: [Intel-gfx] [PATCH v9 10/25] vfio: Make vfio_device_open() single open for device cdev path

gt; > Reviewed-by: Kevin Tian > Reviewed-by: Jason Gunthorpe > Tested-by: Terrence Xu > Tested-by: Nicolin Chen > Tested-by: Yanting Jiang > Signed-off-by: Yi Liu Reviewed-by: Eric Auger Thanks Eric > --- > drivers/vfio/group.c | 2 ++ > drivers/vfio/vfio

Re: [Intel-gfx] [PATCH v9 09/25] vfio: Add cdev_device_open_cnt to vfio_group

nting Jiang > Signed-off-by: Yi Liu Reviewed-by: Eric Auger Thanks Eric > --- > drivers/vfio/group.c | 33 + > drivers/vfio/vfio.h | 3 +++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c

Re: [Intel-gfx] [PATCH v9 08/25] vfio: Block device access via device fd until device is opened

gt; device FD is bound, it remains bound until the FD is closed. > > Suggested-by: Jason Gunthorpe > Reviewed-by: Kevin Tian > Reviewed-by: Jason Gunthorpe > Tested-by: Terrence Xu > Tested-by: Nicolin Chen > Tested-by: Matthew Rosato > Tested-by: Yant

Re: [Intel-gfx] [PATCH v9 07/25] vfio: Pass struct vfio_device_file * to vfio_device_open/close()

dev_set->lock); > > vfio_assert_device_open(device); > if (device->open_count == 1) > - vfio_device_last_close(device, iommufd); > + vfio_device_last_close(df); > device->open_count--; > } > > @@ -535,7 +540,7 @@ static int vfio_device_fops_release(struct inode *inode, > struct file *filep) > struct vfio_device_file *df = filep->private_data; > struct vfio_device *device = df->device; > > - vfio_device_group_close(device); > + vfio_device_group_close(df); > > vfio_device_put_registration(device); > Maybe it reduces the number of parameters but the diffstat shows it does not really simplify the code overall. I am not really sure it was worth and the df->iommufd pre and post-setting is not really nice looking to me. But well it others are OK ... Reviewed-by: Eric Auger Thanks Eric

Re: [Intel-gfx] [PATCH v9 06/25] kvm/vfio: Accept vfio device file from userspace

Hi Yi, On 4/1/23 17:18, Yi Liu wrote: > This defines KVM_DEV_VFIO_FILE* and make alias with KVM_DEV_VFIO_GROUP*. > Old userspace uses KVM_DEV_VFIO_GROUP* works as well. > > Reviewed-by: Jason Gunthorpe > Reviewed-by: Kevin Tian > Tested-by: Terrence Xu > Tested-by: Nicolin Chen > Tested-by: Ma

Re: [Intel-gfx] [PATCH v9 04/25] vfio: Accept vfio device file in the KVM facing kAPI

thew Rosato > Tested-by: Yanting Jiang > Signed-off-by: Yi Liu Reviewed-by: Eric Auger Thanks Eric > --- > drivers/vfio/vfio.h | 2 ++ > drivers/vfio/vfio_main.c | 18 ++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/vfio/vfio.h

Re: [Intel-gfx] [PATCH v3 12/12] vfio/pci: Report dev_id in VFIO_DEVICE_GET_PCI_HOT_RESET_INFO

On 4/5/23 18:25, Alex Williamson wrote: > On Wed, 5 Apr 2023 14:04:51 + > "Liu, Yi L" wrote: > >> Hi Eric, >> >>> From: Eric Auger >>> Sent: Wednesday, April 5, 2023 8:20 PM >>> >>> Hi Yi, >>> On 4/1/23

Re: [Intel-gfx] [PATCH v3 02/12] vfio/pci: Only check ownership of opened devices in hot reset

Hi Jason, On 4/5/23 13:41, Jason Gunthorpe wrote: > On Tue, Apr 04, 2023 at 05:59:01PM +0200, Eric Auger wrote: > >>> but the hot reset shall fail as the group is not owned by the user. >> sure it shall but I fail to understand if the reset fails or the device >> plug i

Re: [Intel-gfx] [PATCH v9 25/25] docs: vfio: Add vfio device cdev description

Hi Yi, On 4/1/23 17:18, Yi Liu wrote: > This gives notes for userspace applications on device cdev usage. > > Reviewed-by: Kevin Tian > Signed-off-by: Yi Liu > --- > Documentation/driver-api/vfio.rst | 132 ++ > 1 file changed, 132 insertions(+) > > diff --git a/Docu

Re: [Intel-gfx] [PATCH v9 03/25] vfio: Remove vfio_file_is_group()

Hi Yi, On 4/1/23 17:18, Yi Liu wrote: > since no user of vfio_file_is_group() now. > > Reviewed-by: Kevin Tian > Reviewed-by: Jason Gunthorpe > Tested-by: Terrence Xu > Tested-by: Nicolin Chen > Tested-by: Yanting Jiang > Signed-off-by: Yi Liu Review

Re: [Intel-gfx] [PATCH v3 12/12] vfio/pci: Report dev_id in VFIO_DEVICE_GET_PCI_HOT_RESET_INFO

Hi Yi, On 4/1/23 16:44, Yi Liu wrote: > for the users that accept device fds passed from management stacks to be > able to figure out the host reset affected devices among the devices > opened by the user. This is needed as such users do not have BDF (bus, > devfn) knowledge about the devices it

Re: [Intel-gfx] [PATCH v3 10/12] vfio: Mark cdev usage in vfio_device

On 4/1/23 16:44, Yi Liu wrote: > There are users that need to check if vfio_device is opened as cdev. > e.g. vfio-pci. This adds a flag in vfio_device, it will be set in the > cdev path when device is opened. This is not used at this moment, but > a preparation for vfio device cdev support. bet

Re: [Intel-gfx] [PATCH v3 11/12] iommufd: Define IOMMUFD_INVALID_ID in uapi

Hi Yi On 4/1/23 16:44, Yi Liu wrote: > as there are IOMMUFD users that want to know check if an ID generated s/want to know check/ need to check which type of ID? > by IOMMUFD is valid or not. e.g. vfio-pci optionaly returns invalid optionally > dev_id to user in the VFIO_DEVICE_GET_PCI_HOT_RESET_

Re: [Intel-gfx] [PATCH v3 09/12] vfio/pci: Accept device fd in VFIO_DEVICE_PCI_HOT_RESET ioctl

rpe > Tested-by: Yanting Jiang > Signed-off-by: Yi Liu Reviewed-by: Eric Auger Eric > --- > drivers/vfio/pci/vfio_pci_core.c | 9 + > include/uapi/linux/vfio.h| 3 ++- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/driver

Re: [Intel-gfx] [PATCH v3 08/12] vfio/pci: Renaming for accepting device fd in hot reset path

On 4/1/23 16:44, Yi Liu wrote: > No functional change is intended. > > Reviewed-by: Kevin Tian > Reviewed-by: Jason Gunthorpe > Tested-by: Yanting Jiang > Signed-off-by: Yi Liu Reviewed-by: Eric Auger Eric > --- > drivers/vfio/

Re: [Intel-gfx] [PATCH v3 06/12] vfio: Refine vfio file kAPIs for vfio PCI hot reset

Hi Yi, On 4/1/23 16:44, Yi Liu wrote: > This prepares vfio core to accept vfio device file from the vfio PCI > hot reset path. vfio_file_is_group() is still kept for KVM usage. > > Reviewed-by: Kevin Tian > Reviewed-by: Jason Gunthorpe > Tested-by: Yanting Jiang > Signed-off-by: Yi Liu > --- >

Re: [Intel-gfx] [PATCH v3 07/12] vfio: Accpet device file from vfio PCI hot reset path

return false; > + if (group) > + return vfio_group_has_dev(group, device); > + > + vdev = vfio_device_from_file(file); > + if (vdev) > + return vdev == device; > > - return vfio_group_has_dev(group, device); > + return false; > } > EXPORT_SYMBOL_GPL(vfio_file_has_dev); > With Alex' suggestion Reviewed-by: Eric Auger Eric

Re: [Intel-gfx] [PATCH v3 05/12] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET

On 4/4/23 22:18, Alex Williamson wrote: > On Sat, 1 Apr 2023 07:44:22 -0700 > Yi Liu wrote: > >> as an alternative method for ownership check when iommufd is used. In >> this case all opened devices in the affected dev_set are verified to >> be bound to a same valid iommufd value to allow rese

Re: [Intel-gfx] [PATCH v3 05/12] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET

Hi Yi, On 4/1/23 16:44, Yi Liu wrote: > as an alternative method for ownership check when iommufd is used. In I don't understand the 1st sentence. > this case all opened devices in the affected dev_set are verified to > be bound to a same valid iommufd value to allow reset. It's simpler > and fast

Re: [Intel-gfx] [PATCH v3 02/12] vfio/pci: Only check ownership of opened devices in hot reset

On 4/4/23 17:29, Liu, Yi L wrote: >> From: Eric Auger >> Sent: Tuesday, April 4, 2023 11:19 PM >> >> Hi Yi, >> >> On 4/4/23 16:37, Liu, Yi L wrote: >>> Hi Eric, >>> >>>> From: Eric Auger >>>> Sent: Tuesday, A

Re: [Intel-gfx] [PATCH v3 04/12] vfio-iommufd: Add helper to retrieve iommufd_ctx and devid for vfio_device

o_device *vdev, u32 *pt_id); > #else > +static inline struct iommufd_ctx * > +vfio_iommufd_physical_ictx(struct vfio_device *vdev) > +{ > + return NULL; > +} > + > +static inline void > +vfio_iommufd_physical_devid(struct vfio_device *vdev, u32 *id) > +{ > +} > + > #define vfio_iommufd_physical_bind \ > ((int (*)(struct vfio_device *vdev, struct iommufd_ctx *ictx, \ > u32 *out_device_id)) NULL) besides Reviewed-by: Eric Auger Eric

Re: [Intel-gfx] [PATCH v3 02/12] vfio/pci: Only check ownership of opened devices in hot reset

Hi Yi, On 4/4/23 16:37, Liu, Yi L wrote: > Hi Eric, > >> From: Eric Auger >> Sent: Tuesday, April 4, 2023 10:00 PM >> >> Hi YI, >> >> On 4/1/23 16:44, Yi Liu wrote: >>> If the affected device is not opened by any user, it's safe to reset

Re: [Intel-gfx] [PATCH v3 01/12] vfio/pci: Update comment around group_fd get in vfio_pci_ioctl_pci_hot_reset()

t. > + * Get the group file for each fd to ensure the group held across to ensure the group is held Besides Reviewed-by: Eric Auger Eric > + * the reset >*/ > for (file_idx = 0; file_idx < hdr.count; file_idx++) { > struct file *file = fget(group_fds[file_idx]);

Re: [Intel-gfx] [PATCH v3 03/12] vfio/pci: Move the existing hot reset logic to be a helper

we do a slot or bus reset or neither? */ > + if (!pci_probe_reset_slot(vdev->pdev->slot)) > + slot = true; > + else if (pci_probe_reset_bus(vdev->pdev->bus)) > + return -ENODEV; > + > + return vfio_pci_ioctl_pci_hot_reset_groups(vdev, &hdr, slot, arg); > +} > + > static int vfio_pci_ioctl_ioeventfd(struct vfio_pci_core_device *vdev, > struct vfio_device_ioeventfd __user *arg) > { Besides Reviewed-by: Eric Auger Thanks Eric

Re: [Intel-gfx] [PATCH v3 02/12] vfio/pci: Only check ownership of opened devices in hot reset

Hi YI, On 4/1/23 16:44, Yi Liu wrote: > If the affected device is not opened by any user, it's safe to reset it > given it's not in use. > > Reviewed-by: Kevin Tian > Reviewed-by: Jason Gunthorpe > Tested-by: Yanting Jiang > Signed-off-by: Yi Liu > --- > drivers/vfio/pci/vfio_pci_core.c | 14

Re: [Intel-gfx] [PATCH v2 15/15] vfio: Add struct device to vfio_device

On 9/8/22 11:17, Yi Liu wrote: > On 2022/9/8 17:06, Eric Auger wrote: >> Hi Kevin, >> >> On 9/1/22 16:37, Kevin Tian wrote: >>> From: Yi Liu >>> >>> and replace kref. With it a 'vfio-dev/vfioX' node is created under the >>>

Re: [Intel-gfx] [PATCH v2 01/15] vfio: Add helpers for unifying vfio_device life cycle

Hi Kevin, On 9/8/22 08:19, Tian, Kevin wrote: >> From: Eric Auger >> Sent: Thursday, September 8, 2022 3:28 AM >>> +/* >>> + * Alloc and initialize vfio_device so it can be registered to vfio >>> + * core. >>> + * >>> + * Drivers

Re: [Intel-gfx] [PATCH v2 15/15] vfio: Add struct device to vfio_device

Hi Kevin, On 9/1/22 16:37, Kevin Tian wrote: > From: Yi Liu > > and replace kref. With it a 'vfio-dev/vfioX' node is created under the > sysfs path of the parent, indicating the device is bound to a vfio > driver, e.g.: > > /sys/devices/pci\:6f/\:6f\:01.0/vfio-dev/vfio0 > > It is also a p

Re: [Intel-gfx] [PATCH v2 14/15] vfio: Rename vfio_device_put() and vfio_device_try_get()

ce. > > Now rename them: > > - vfio_device_put() -> vfio_device_put_registration() > - vfio_device_try_get() -> vfio_device_try_get_registration() > > Signed-off-by: Kevin Tian > Reviewed-by: Jason Gunthorpe Reviewed-by: Eric Auger Eric > --- > drivers/vfi

Re: [Intel-gfx] [PATCH v2 12/15] vfio/amba: Use the new device life cycle helpers

_device *vdev) > -{ > - vfio_unregister_group_dev(&vdev->vdev); > - > - pm_runtime_disable(vdev->device); > - vfio_platform_put_reset(vdev); > - vfio_uninit_group_dev(&vdev->vdev); > -} > -EXPORT_SYMBOL_GPL(vfio_platform_remove_common); > - > void __vfio_platform_register_reset(struct vfio_platform_reset_node *node) > { > mutex_lock(&driver_lock); > diff --git a/drivers/vfio/platform/vfio_platform_private.h > b/drivers/vfio/platform/vfio_platform_private.h > index a769d649fb97..8d8fab516849 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -78,9 +78,6 @@ struct vfio_platform_reset_node { > vfio_platform_reset_fn_t of_reset; > }; > > -int vfio_platform_probe_common(struct vfio_platform_device *vdev, > -struct device *dev); > -void vfio_platform_remove_common(struct vfio_platform_device *vdev); > int vfio_platform_init_common(struct vfio_platform_device *vdev); > void vfio_platform_release_common(struct vfio_platform_device *vdev); > Reviewed-by: Eric Auger Eric

Re: [Intel-gfx] [PATCH v2 01/15] vfio: Add helpers for unifying vfio_device life cycle

struct dev_struct, member)), > \ > + dev, ops), > \ > + struct dev_struct, member) > + > +int vfio_init_device(struct vfio_device *device, struct device *dev, > + const struct vfio_device_ops *ops); > +void vfio_free_device(struct vfio_device *device); > +void vfio_device_release(struct kref *kref); > +static inline void vfio_put_device(struct vfio_device *device) > +{ > + kref_put(&device->kref, vfio_device_release); > +} > + > void vfio_init_group_dev(struct vfio_device *device, struct device *dev, >const struct vfio_device_ops *ops); > void vfio_uninit_group_dev(struct vfio_device *device); Besides Reviewed-by: Eric Auger Eric

Re: [Intel-gfx] [PATCH v2 11/15] vfio/platform: Use the new device life cycle helpers

rm_device *vdev); > +int vfio_platform_init_common(struct vfio_platform_device *vdev); > +void vfio_platform_release_common(struct vfio_platform_device *vdev); > + > +int vfio_platform_open_device(struct vfio_device *core_vdev); > +void vfio_platform_close_device(struct vfio_device *core_vdev); > +long vfio_platform_ioctl(struct vfio_device *core_vdev, > + unsigned int cmd, unsigned long arg); > +ssize_t vfio_platform_read(struct vfio_device *core_vdev, > +char __user *buf, size_t count, > +loff_t *ppos); > +ssize_t vfio_platform_write(struct vfio_device *core_vdev, > + const char __user *buf, > + size_t count, loff_t *ppos); > +int vfio_platform_mmap(struct vfio_device *core_vdev, > +struct vm_area_struct *vma); > > int vfio_platform_irq_init(struct vfio_platform_device *vdev); > void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev); Looks good to me. I also ran basic non regression testing Reviewed-by: Eric Auger Tested-by: Eric Auger Eric

Re: [Intel-gfx] [PATCH v3 07/14] vfio/platform: Use open_device() instead of open coding a refcnt scheme

> Signed-off-by: Jason Gunthorpe > Signed-off-by: Yishai Hadas > Reviewed-by: Cornelia Huck > Reviewed-by: Christoph Hellwig Reviewed-by: Eric Auger Thanks Eric > --- > drivers/vfio/platform/vfio_platform_common.c | 79 --- > drivers/vfio/platform/vfio_