- 27 Apr, 2021 1 commit
-
-
Arnd Bergmann authored
The Kconfig dependency is incomplete since DRM_I915_GVT is a 'bool' symbol that depends on the 'tristate' VFIO_MDEV. This allows a configuration with VFIO_MDEV=m, DRM_I915_GVT=y and DRM_I915=y that causes a link failure: x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function `available_instances_show': gvt.c:(.text+0x67a): undefined reference to `mtype_get_parent_dev' x86_64-linux-ld: gvt.c:(.text+0x6a5): undefined reference to `mtype_get_type_group_id' x86_64-linux-ld: drivers/gpu/drm/i915/gvt/gvt.o: in function `description_show': gvt.c:(.text+0x76e): undefined reference to `mtype_get_parent_dev' x86_64-linux-ld: gvt.c:(.text+0x799): undefined reference to `mtype_get_type_group_id' Clarify the dependency by specifically disallowing the broken configuration. If VFIO_MDEV is built-in, it will work, but if VFIO_MDEV=m, the i915 driver cannot be built-in here. Fixes: 07e543f4 ("vfio/gvt: Make DRM_I915_GVT depend on VFIO_MDEV") Fixes: 9169cff1 ("vfio/mdev: Correct the function signatures for the mdev_type_attributes") Signed-off-by:
Arnd Bergmann <arnd@arndb.de> Acked-by:
Zhenyu Wang <zhenyuw@linux.intel.com> Message-Id: <20210422133547.1861063-1-arnd@kernel.org> Reviewed-by:
Jason Gunthorpe <jgg@nvidia.com> Signed-off-by:
Alex Williamson <alex.williamson@redhat.com>
-
- 14 Apr, 2021 1 commit
-
-
Keqian Zhu authored
pinned_page_dirty_scope is optimized out by commit 01032156 ("vfio/iommu_type1: Mantain a counter for non_pinned_groups"), but appears again due to some issues during merging branches. We can safely remove it here. Signed-off-by:
Keqian Zhu <zhukeqian1@huawei.com> Message-Id: <20210412024415.30676-1-zhukeqian1@huawei.com> Signed-off-by:
-
- 12 Apr, 2021 4 commits
-
-
Jason Gunthorpe authored
The driver core standard is to pass in the properly typed object, the properly typed attribute and the buffer data. It stems from the root kobject method: ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,..) Each subclass of kobject should provide their own function with the same signature but more specific types, eg struct device uses: ssize_t (*show)(struct device *dev, struct device_attribute *attr,..) In this case the existing signature is: ssize_t (*show)(struct kobject *kobj, struct device *dev,..) Where kobj is a 'struct mdev_type *' and dev is 'mdev_type->parent->dev'. Change the mdev_type related sysfs attribute functions to: ssize_t (*show)(struct mdev_type *mtype, struct mdev_type_attribute *attr,..) In order to restore type safety and match the driver core standard There are no current users of 'attr', but if it is ever needed it would be hard to add in retroactively, so do it now. Reviewed-by:
Kevin Tian <kevin.tian@intel.com> Reviewed-by:
Cornelia Huck <cohuck@redhat.com> Reviewed-by:
Christoph Hellwig <hch@lst.de> Signed-off-by:
-
Jason Gunthorpe authored
The kobj here is a type-erased version of mdev_type, which is already stored in the struct mdev_device being passed in. It was only ever used to compute the type_group_id, which is now extracted directly from the mdev. Reviewed-by:
-
Jason Gunthorpe authored
intel_gvt_init_vgpu_type_groups() makes gvt->types 1:1 with the supported_type_groups array, so the type_group_id is also the index into gvt->types. Use it directly and remove the string matching. Reviewed-by:
-
Jason Gunthorpe authored
At some point there may have been some reason for this weird split in this driver, but today only the VFIO side is actually implemented. However, it got messed up at some point and mdev code was put in gvt.c and is pretending to be "generic" by masquerading as some generic attribute list: static MDEV_TYPE_ATTR_RO(description); But MDEV_TYPE attributes are only usable with mdev_device, nothing else. Ideally all of this would be moved to kvmgt.c, but it is entangled with the rest of the "generic" code in an odd way. Thus put in a kconfig dependency so we don't get randconfig failures when the next patch creates a link time dependency related to the use of MDEV_TYPE. Reviewed-by:
-
- 07 Apr, 2021 14 commits
-
-
Jason Gunthorpe authored
The mbochs_types array is parallel to the supported_type_groups array, so the type_group_id indexes both. Instead of doing string searching just directly index with type_group_id in all places. Reviewed-by:
-
Jason Gunthorpe authored
The mdpy_types array is parallel to the supported_type_groups array, so the type_group_id indexes both. Instead of doing string searching just directly index with type_group_id in all places. Reviewed-by:
-
Jason Gunthorpe authored
The type_group_id directly gives the single or dual port index, no need for string searching. Reviewed-by:
-
Jason Gunthorpe authored
This returns the index in the supported_type_groups array that is associated with the mdev_type attached to the struct mdev_device or its containing struct kobject. Each mdev_device can be spawned from exactly one mdev_type, which in turn originates from exactly one supported_type_group. Drivers are using weird string calculations to try and get back to this index, providing a direct access to the index removes a bunch of wonky driver code. mdev_type->group can be deleted as the group is obtained using the type_group_id. Reviewed-by:
-
Jason Gunthorpe authored
mdev_device->type->parent is the same thing. The struct mdev_device was relying on the kref on the mdev_parent to also indirectly hold a kref on the mdev_type pointer. Now that the type holds a kref on the parent we can directly kref the mdev_type and remove this implicit relationship. Reviewed-by:
-
Jason Gunthorpe authored
This can fail, and seems to be a popular target for syzkaller error injection. Check the error return and unwind with put_device(). Fixes: 7b96953b ("vfio: Mediated device Core driver") Reviewed-by:
Max Gurtovoy <mgurtovoy@nvidia.com> Reviewed-by:
-
Jason Gunthorpe authored
Once the memory for the struct mdev_device is allocated it should immediately be device_initialize()'d and filled in so that put_device() can always be used to undo the allocation. Place the mdev_get/put_parent() so that they are clearly protecting the mdev->parent pointer. Move the final put to the release function so that the lifetime rules are trivial to understand. Update the goto labels to follow the normal convention. Remove mdev_device_free() as the release function via device_put() is now usable in all cases. Reviewed-by:
-
Jason Gunthorpe authored
struct mdev_type holds a pointer to the kref'd object struct mdev_parent, but doesn't hold the kref. The lifetime of the parent becomes implicit because parent_remove_sysfs_files() is supposed to remove all the access before the parent can be freed, but this is very hard to reason about. Make it obviously correct by adding the missing get. Reviewed-by:
-
Jason Gunthorpe authored
The next patch will use these in mdev_sysfs.c While here remove the now dead code checks for NULL, a mdev_type can never have a NULL parent. Reviewed-by:
-
Jason Gunthorpe authored
The kobj pointer in mdev_device is actually pointing at a struct mdev_type. Use the proper type so things are understandable. There are a number of places that are confused and passing both the mdev and the mtype as function arguments, fix these to derive the mtype directly from the mdev to remove the redundancy. Reviewed-by:
-
Jason Gunthorpe authored
This is only done once, we don't need to generate code to initialize a structure stored in the ELF .data segment. Fill in the three required .driver members directly instead of copying data into them during mdev_register_driver(). Further the to_mdev_driver() function doesn't belong in a public header, just inline it into the two places that need it. Finally, we can now clearly see that 'drv' derived from dev->driver cannot be NULL, firstly because the driver core forbids it, and secondly because NULL won't pass through the container_of(). Remove the dead code. Reviewed-by:
-
Jason Gunthorpe authored
The mdev API should accept and pass a 'struct mdev_device *' in all places, not pass a 'struct device *' and cast it internally with to_mdev_device(). Particularly in its struct mdev_driver functions, the whole point of a bus's struct device_driver wrapper is to provide type safety compared to the default struct device_driver. Further, the driver core standard is for bus drivers to expose their device structure in their public headers that can be used with container_of() inlines and '&foo->dev' to go between the class levels, and '&foo->dev' to be used with dev_err/etc driver core helper functions. Move 'struct mdev_device' to mdev.h Once done this allows moving some one instruction exported functions to static inlines, which in turns allows removing one of the two grotesque symbol_get()'s related to mdev in the core code. Reviewed-by:
-
Jason Gunthorpe authored
There is a small race where the parent is NULL even though the kobj has already been made visible in sysfs. For instance the attribute_group is made visible in sysfs_create_files() and the mdev_type_attr_show() does: ret = attr->show(kobj, type->parent->dev, buf); Which will crash on NULL parent. Move the parent setup to before the type pointer leaves the stack frame. Fixes: 7b96953b ("vfio: Mediated device Core driver") Reviewed-by: -
Jason Gunthorpe authored
These should always be prefixed with static, otherwise compilation will fail on non-modular builds with ld: samples/vfio-mdev/mbochs.o:(.data+0x2e0): multiple definition of `mdev_type_attr_name'; samples/vfio-mdev/mdpy.o:(.data+0x240): first defined here Fixes: a5e6e650 ("sample: vfio bochs vbe display (host device for bochs-drm)") Fixes: d61fc96f ("sample: vfio mdev display - host device") Signed-off-by:
-
- 06 Apr, 2021 20 commits
-
-
Alex Williamson authored
Merge branches 'v5.13/vfio/embed-vfio_device', 'v5.13/vfio/misc' and 'v5.13/vfio/nvlink' into v5.13/vfio/next Spelling fixes merged with file deletion. Conflicts: drivers/vfio/pci/vfio_pci_nvlink2.c Signed-off-by: -
Jason Gunthorpe authored
There are no longer any users, so it can go away. Everything is using container_of now. Reviewed-by:
-
Jason Gunthorpe authored
This tidies a few confused places that think they can have a refcount on the vfio_device but the device_data could be NULL, that isn't possible by design. Most of the change falls out when struct vfio_devices is updated to just store the struct vfio_pci_device itself. This wasn't possible before because there was no easy way to get from the 'struct vfio_pci_device' to the 'struct vfio_device' to put back the refcount. Reviewed-by:
-
Jason Gunthorpe authored
This is the standard kernel pattern, the ops associated with a struct get the struct pointer in for typesafety. The expected design is to use container_of to cleanly go from the subsystem level type to the driver level type without having any type erasure in a void *. Reviewed-by:
Dan Williams <dan.j.williams@intel.com> Reviewed-by:
-
Jason Gunthorpe authored
The macro wrongly uses 'dev' as both the macro argument and the member name, which means it fails compilation if any caller uses a word other than 'dev' as the single argument. Fix this defect by making it into proper static inline, which is more clear and typesafe anyhow. Fixes: 99e3123e ("vfio-mdev: Make mdev_device private and abstract interfaces") Reviewed-by:
-
Jason Gunthorpe authored
mdev gets little benefit because it doesn't actually do anything, however it is the last user, so move the vfio_init/register/unregister_group_dev() code here for now. Reviewed-by:
Liu Yi L <yi.l.liu@intel.com> Reviewed-by:
-
Jason Gunthorpe authored
pci already allocates a struct vfio_pci_device with exactly the same lifetime as vfio_device, switch to the new API and embed vfio_device in vfio_pci_device. Reviewed-by:
Eric Auger <eric.auger@redhat.com> Signed-off-by:
-
Jason Gunthorpe authored
vfio_add_group_dev() must be called only after all of the private data in vdev is fully setup and ready, otherwise there could be races with user space instantiating a device file descriptor and starting to call ops. For instance vfio_pci_reflck_attach() sets vdev->reflck and vfio_pci_open(), called by fops open, unconditionally derefs it, which will crash if things get out of order. Fixes: cc20d799 ("vfio/pci: Introduce VF token") Fixes: e309df5b ("vfio/pci: Parallelize device open and release") Fixes: 6eb70187 ("vfio-pci: Move idle devices to D3hot power state") Fixes: ecaa1f6a ("vfio-pci: Add VGA arbiter client") Reviewed-by:
-
Jason Gunthorpe authored
vfio_pci_probe() is quite complicated, with optional VF and VGA sub components. Move these into clear init/uninit functions and have a linear flow in probe/remove. This fixes a few little buglets: - vfio_pci_remove() is in the wrong order, vga_client_register() removes a notifier and is after kfree(vdev), but the notifier refers to vdev, so it can use after free in a race. - vga_client_register() can fail but was ignored Organize things so destruction order is the reverse of creation order. Fixes: ecaa1f6a ("vfio-pci: Add VGA arbiter client") Reviewed-by:
-
Jason Gunthorpe authored
fsl-mc already allocates a struct vfio_fsl_mc_device with exactly the same lifetime as vfio_device, switch to the new API and embed vfio_device in vfio_fsl_mc_device. While here remove the devm usage for the vdev, this code is clean and doesn't need devm. Reviewed-by:
-
Jason Gunthorpe authored
vfio_add_group_dev() must be called only after all of the private data in vdev is fully setup and ready, otherwise there could be races with user space instantiating a device file descriptor and starting to call ops. For instance vfio_fsl_mc_reflck_attach() sets vdev->reflck and vfio_fsl_mc_open(), called by fops open, unconditionally derefs it, which will crash if things get out of order. This driver started life with the right sequence, but two commits added stuff after vfio_add_group_dev(). Fixes: 2e0d2956 ("vfio/fsl-mc: Add irq infrastructure for fsl-mc devices") Fixes: f2ba7e8c ("vfio/fsl-mc: Added lock support in preparation for interrupt handling") Co-developed-by:
Diana Craciun OSS <diana.craciun@oss.nxp.com> Signed-off-by:
-
Jason Gunthorpe authored
platform already allocates a struct vfio_platform_device with exactly the same lifetime as vfio_device, switch to the new API and embed vfio_device in vfio_platform_device. Reviewed-by:
-
Jason Gunthorpe authored
This makes the struct vfio_device part of the public interface so it can be used with container_of and so forth, as is typical for a Linux subystem. This is the first step to bring some type-safety to the vfio interface by allowing the replacement of 'void *' and 'struct device *' inputs with a simple and clear 'struct vfio_device *' For now the self-allocating vfio_add_group_dev() interface is kept so each user can be updated as a separate patch. The expected usage pattern is driver core probe() function: my_device = kzalloc(sizeof(*mydevice)); vfio_init_group_dev(&my_device->vdev, dev, ops, mydevice); /* other driver specific prep */ vfio_register_group_dev(&my_device->vdev); dev_set_drvdata(dev, my_device); driver core remove() function: my_device = dev_get_drvdata(dev); vfio_unregister_group_dev(&my_device->vdev); /* other driver specific tear down */ kfree(my_device); Allowing the driver to be able to use the drvdata and vfio_device to go to/from its own data. The pattern also makes it clear that vfio_register_group_dev() must be last in the sequence, as once it is called the core code can immediately start calling ops. The init/register gap is provided to allow for the driver to do setup before ops can be called and thus avoid races. Reviewed-by: -
Jason Gunthorpe authored
The vfio_device is using a 'sleep until all refs go to zero' pattern for its lifetime, but it is indirectly coded by repeatedly scanning the group list waiting for the device to be removed on its own. Switch this around to be a direct representation, use a refcount to count the number of places that are blocking destruction and sleep directly on a completion until that counter goes to zero. kfree the device after other accesses have been excluded in vfio_del_group_dev(). This is a fairly common Linux idiom. Due to this we can now remove kref_put_mutex(), which is very rarely used in the kernel. Here it is being used to prevent a zero ref device from being seen in the group list. Instead allow the zero ref device to continue to exist in the device_list and use refcount_inc_not_zero() to exclude it once refs go to zero. This patch is organized so the next patch will be able to alter the API to allow drivers to provide the kfree. Reviewed-by:
-
Jason Gunthorpe authored
The vfio_device->group value has a get obtained during vfio_add_group_dev() which gets moved from the stack to vfio_device->group in vfio_group_create_device(). The reference remains until we reach the end of vfio_del_group_dev() when it is put back. Thus anything that already has a kref on the vfio_device is guaranteed a valid group pointer. Remove all the extra reference traffic. It is tricky to see, but the get at the start of vfio_del_group_dev() is actually pairing with the put hidden inside vfio_device_put() a few lines below. A later patch merges vfio_group_create_device() into vfio_add_group_dev() which makes the ownership and error flow on the create side easier to follow. Reviewed-by:
-
Christoph Hellwig authored
This driver never had any open userspace (which for VFIO would include VM kernel drivers) that use it, and thus should never have been added by our normal userspace ABI rules. Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> Message-Id: <20210326061311.1497642-2-hch@lst.de> Signed-off-by:
-
Shenming Lu authored
The check i > npage at the end of vfio_iommu_type1_unpin_pages is unused unless npage < 0, but if npage < 0, this function will return npage, which should return -EINVAL instead. So let's just check the parameter npage at the start of the function. By the way, replace unpin_exit with break. Signed-off-by:
Shenming Lu <lushenming@huawei.com> Message-Id: <20210406135009.1707-1-lushenming@huawei.com> Signed-off-by:
-
Zhen Lei authored
There is a spelling mistake in a comment, fix it. Signed-off-by:
Zhen Lei <thunder.leizhen@huawei.com> Acked-by:
-
Zhen Lei authored
There are several spelling mistakes, as follows: thru ==> through presense ==> presence Signed-off-by:
-
Zhen Lei authored
There is a spelling mistake in a comment, fix it. Signed-off-by:
-
