Re: [PATCH v2 10/19] iommufd/viommu: Add vdev_id helpers for IOMMU drivers

On Thu, Sep 05, 2024 at 10:53:31AM -0700, Nicolin Chen wrote:

On Thu, Sep 05, 2024 at 01:14:15PM -0300, Jason Gunthorpe wrote:

quoted

On Tue, Aug 27, 2024 at 09:59:47AM -0700, Nicolin Chen wrote:

quoted

Driver can call the iommufd_viommu_find_device() to find a device pointer
using its per-viommu virtual ID. The returned device must be protected by
the pair of iommufd_viommu_lock/unlock_vdev_id() function.

Put these three functions into a new viommu_api file, to build it with the
IOMMUFD_DRIVER config.

Signed-off-by: Nicolin Chen <redacted>
---
 drivers/iommu/iommufd/Makefile     |  2 +-
 drivers/iommu/iommufd/viommu_api.c | 39 ++++++++++++++++++++++++++++++
 include/linux/iommufd.h            | 16 ++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iommu/iommufd/viommu_api.c

I still think this is better to just share the struct content with the
driver, eventually we want to do this anyhow as the driver will
want to use container_of() techniques to reach its private data.

In my mind, exposing everything to the driver is something that
we have to (for driver-managed structures) v.s. we want to...
Even in that case, a driver actually only need to know the size
of the core structure, without touching what's inside(?).

I am a bit worried that drivers would abuse the content in the
core-level structure.. Providing a set of API would encourage
them to keep the core structure intact, hopefully..

This is always a tension in the kernel. If the core apis can be nice
and tidy then it is a reasonable direction

But here I think we've cross some threshold where the APIs are
complex, want to be inlined and really we just want to expose data not
APIs to drivers.

quoted

No need for this lock, xa_load is rcu safe against concurrent writer

I see iommufd's device.c and main.c grab xa_lock before xa_load?

That is not to protect the xa_load, it is to protect the lifetime of
pointer it returns

Jason