mbox series

[v4,00/14] iommufd: Add vIOMMU infrastructure (Part-2: vDEVICE)

Message ID cover.1729555967.git.nicolinc@nvidia.com (mailing list archive)
Headers show
Series iommufd: Add vIOMMU infrastructure (Part-2: vDEVICE) | expand

Message

Nicolin Chen Oct. 22, 2024, 12:20 a.m. UTC
Following the previous vIOMMU series, this adds another vDEVICE structure,
representing the association from an iommufd_device to an iommufd_viommu.
This gives the whole architecture a new "v" layer:
  _______________________________________________________________________
 |                      iommufd (with vIOMMU/vDEVICE)                    |
 |                        _____________      _____________               |
 |                       |             |    |             |              |
 |      |----------------|    vIOMMU   |<---|   vDEVICE   |<------|      |
 |      |                |             |    |_____________|       |      |
 |      |     ______     |             |     _____________     ___|____  |
 |      |    |      |    |             |    |             |   |        | |
 |      |    | IOAS |<---|(HWPT_PAGING)|<---| HWPT_NESTED |<--| DEVICE | |
 |      |    |______|    |_____________|    |_____________|   |________| |
 |______|________|______________|__________________|_______________|_____|
        |        |              |                  |               |
  ______v_____   |        ______v_____       ______v_____       ___v__
 |   struct   |  |  PFN  |  (paging)  |     |  (nested)  |     |struct|
 |iommu_device|  |------>|iommu_domain|<----|iommu_domain|<----|device|
 |____________|   storage|____________|     |____________|     |______|

This vDEVICE object is used to collect and store all vIOMMU-related device
information/attributes in a VM. As an initial series for vDEVICE, add only
the virt_id to the vDEVICE, which is a vIOMMU specific device ID in a VM:
e.g. vSID of ARM SMMUv3, vDeviceID of AMD IOMMU, and vID of Intel VT-d to
a Context Table. This virt_id helps IOMMU drivers to link the vID to a pID
of the device against the physical IOMMU instance. This is essential for a
vIOMMU-based invalidation, where the request contains a device's vID for a
device cache flush, e.g. ATC invalidation.

Therefore, with this vDEVICE object, support a vIOMMU-based invalidation,
by reusing IOMMUFD_CMD_HWPT_INVALIDATE for a vIOMMU object to flush cache
with a given driver data.

As for the implementation of the series, add driver support in ARM SMMUv3
for a real world use case.

This series is on Github:
https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4

For testing, try this "with-rmr" branch:
https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4-with-rmr
Paring QEMU branch for testing:
https://github.com/nicolinc/qemu/commits/wip/for_iommufd_viommu_p2-v4

Changelog
v4
 * Added missing brackets in switch-case
 * Fixed the unreleased idev refcount issue
 * Reworked the iommufd_vdevice_alloc allocator
 * Dropped support for IOMMU_VIOMMU_TYPE_DEFAULT
 * Added missing TEST_LENGTH and fail_nth coverages
 * Added a verification to the driver-allocated vDEVICE object
 * Added an iommufd_vdevice_abort for a missing mutex protection
 * Added a u64 structure arm_vsmmu_invalidation_cmd for user command
   conversion
v3
 https://lore.kernel.org/all/cover.1728491532.git.nicolinc@nvidia.com/
 * Added Jason's Reviewed-by
 * Split this invalidation part out of the part-1 series
 * Repurposed VDEV_ID ioctl to a wider vDEVICE structure and ioctl
 * Reduced viommu_api functions by allowing drivers to access viommu
   and vdevice structure directly
 * Dropped vdevs_rwsem by using xa_lock instead
 * Dropped arm_smmu_cache_invalidate_user
v2
 https://lore.kernel.org/all/cover.1724776335.git.nicolinc@nvidia.com/
 * Limited vdev_id to one per idev
 * Added a rw_sem to protect the vdev_id list
 * Reworked driver-level APIs with proper lockings
 * Added a new viommu_api file for IOMMUFD_DRIVER config
 * Dropped useless iommu_dev point from the viommu structure
 * Added missing index numnbers to new types in the uAPI header
 * Dropped IOMMU_VIOMMU_INVALIDATE uAPI; Instead, reuse the HWPT one
 * Reworked mock_viommu_cache_invalidate() using the new iommu helper
 * Reordered details of set/unset_vdev_id handlers for proper lockings
v1
 https://lore.kernel.org/all/cover.1723061377.git.nicolinc@nvidia.com/

Thanks!
Nicolin

Jason Gunthorpe (2):
  iommu: Add iommu_copy_struct_from_full_user_array helper
  iommu/arm-smmu-v3: Allow ATS for IOMMU_DOMAIN_NESTED

Nicolin Chen (12):
  iommufd/viommu: Introduce IOMMUFD_OBJ_VDEVICE and its related struct
  iommufd/viommu: Add IOMMU_VDEVICE_ALLOC ioctl
  iommufd/selftest: Add IOMMU_VDEVICE_ALLOC test coverage
  iommu/viommu: Add cache_invalidate to iommufd_viommu_ops
  iommufd/hw_pagetable: Enforce cache invalidation op on vIOMMU-based
    hwpt_nested
  iommufd: Allow hwpt_id to carry viommu_id for IOMMU_HWPT_INVALIDATE
  iommufd/viommu: Add vdev_to_dev helper
  iommufd/selftest: Add mock_viommu_cache_invalidate
  iommufd/selftest: Add IOMMU_TEST_OP_DEV_CHECK_CACHE test command
  iommufd/selftest: Add vIOMMU coverage for IOMMU_HWPT_INVALIDATE ioctl
  Documentation: userspace-api: iommufd: Update vDEVICE
  iommu/arm-smmu-v3: Add arm_vsmmu_cache_invalidate

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   9 +-
 drivers/iommu/iommufd/iommufd_private.h       |  12 ++
 drivers/iommu/iommufd/iommufd_test.h          |  30 +++
 include/linux/iommu.h                         |  49 ++++-
 include/linux/iommufd.h                       |  50 +++++
 include/uapi/linux/iommufd.h                  |  61 +++++-
 tools/testing/selftests/iommu/iommufd_utils.h |  83 +++++++
 .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     | 162 +++++++++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  32 ++-
 drivers/iommu/iommufd/device.c                |  11 +
 drivers/iommu/iommufd/driver.c                |   7 +
 drivers/iommu/iommufd/hw_pagetable.c          |  36 +++-
 drivers/iommu/iommufd/main.c                  |   7 +
 drivers/iommu/iommufd/selftest.c              | 115 +++++++++-
 drivers/iommu/iommufd/viommu.c                | 108 ++++++++++
 tools/testing/selftests/iommu/iommufd.c       | 204 +++++++++++++++++-
 .../selftests/iommu/iommufd_fail_nth.c        |   4 +
 Documentation/userspace-api/iommufd.rst       |  41 +++-
 18 files changed, 983 insertions(+), 38 deletions(-)

Comments

Alexey Kardashevskiy Oct. 25, 2024, 4:54 a.m. UTC | #1
On 22/10/24 11:20, Nicolin Chen wrote:
> Following the previous vIOMMU series, this adds another vDEVICE structure,
> representing the association from an iommufd_device to an iommufd_viommu.
> This gives the whole architecture a new "v" layer:
>    _______________________________________________________________________
>   |                      iommufd (with vIOMMU/vDEVICE)                    |
>   |                        _____________      _____________               |
>   |                       |             |    |             |              |
>   |      |----------------|    vIOMMU   |<---|   vDEVICE   |<------|      |
>   |      |                |             |    |_____________|       |      |
>   |      |     ______     |             |     _____________     ___|____  |
>   |      |    |      |    |             |    |             |   |        | |
>   |      |    | IOAS |<---|(HWPT_PAGING)|<---| HWPT_NESTED |<--| DEVICE | |
>   |      |    |______|    |_____________|    |_____________|   |________| |
>   |______|________|______________|__________________|_______________|_____|
>          |        |              |                  |               |
>    ______v_____   |        ______v_____       ______v_____       ___v__
>   |   struct   |  |  PFN  |  (paging)  |     |  (nested)  |     |struct|
>   |iommu_device|  |------>|iommu_domain|<----|iommu_domain|<----|device|
>   |____________|   storage|____________|     |____________|     |______|
> 
> This vDEVICE object is used to collect and store all vIOMMU-related device
> information/attributes in a VM. As an initial series for vDEVICE, add only
> the virt_id to the vDEVICE, which is a vIOMMU specific device ID in a VM:
> e.g. vSID of ARM SMMUv3, vDeviceID of AMD IOMMU, and vID of Intel VT-d to
> a Context Table. This virt_id helps IOMMU drivers to link the vID to a pID
> of the device against the physical IOMMU instance. This is essential for a
> vIOMMU-based invalidation, where the request contains a device's vID for a
> device cache flush, e.g. ATC invalidation.
> 
> Therefore, with this vDEVICE object, support a vIOMMU-based invalidation,
> by reusing IOMMUFD_CMD_HWPT_INVALIDATE for a vIOMMU object to flush cache
> with a given driver data.
> 
> As for the implementation of the series, add driver support in ARM SMMUv3
> for a real world use case.
> 
> This series is on Github:
> https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4
> 
> For testing, try this "with-rmr" branch:
> https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4-with-rmr

Is there any real example of a .vdevice_alloc hook, besides the 
selftests? It is not in iommufd_viommu_p2-v4-with-rmr, hence the 
question. I am trying to sketch something with this new machinery and 
less guessing would be nice. Thanks,


> Paring QEMU branch for testing:
> https://github.com/nicolinc/qemu/commits/wip/for_iommufd_viommu_p2-v4
> 
> Changelog
> v4
>   * Added missing brackets in switch-case
>   * Fixed the unreleased idev refcount issue
>   * Reworked the iommufd_vdevice_alloc allocator
>   * Dropped support for IOMMU_VIOMMU_TYPE_DEFAULT
>   * Added missing TEST_LENGTH and fail_nth coverages
>   * Added a verification to the driver-allocated vDEVICE object
>   * Added an iommufd_vdevice_abort for a missing mutex protection
>   * Added a u64 structure arm_vsmmu_invalidation_cmd for user command
>     conversion
> v3
>   https://lore.kernel.org/all/cover.1728491532.git.nicolinc@nvidia.com/
>   * Added Jason's Reviewed-by
>   * Split this invalidation part out of the part-1 series
>   * Repurposed VDEV_ID ioctl to a wider vDEVICE structure and ioctl
>   * Reduced viommu_api functions by allowing drivers to access viommu
>     and vdevice structure directly
>   * Dropped vdevs_rwsem by using xa_lock instead
>   * Dropped arm_smmu_cache_invalidate_user
> v2
>   https://lore.kernel.org/all/cover.1724776335.git.nicolinc@nvidia.com/
>   * Limited vdev_id to one per idev
>   * Added a rw_sem to protect the vdev_id list
>   * Reworked driver-level APIs with proper lockings
>   * Added a new viommu_api file for IOMMUFD_DRIVER config
>   * Dropped useless iommu_dev point from the viommu structure
>   * Added missing index numnbers to new types in the uAPI header
>   * Dropped IOMMU_VIOMMU_INVALIDATE uAPI; Instead, reuse the HWPT one
>   * Reworked mock_viommu_cache_invalidate() using the new iommu helper
>   * Reordered details of set/unset_vdev_id handlers for proper lockings
> v1
>   https://lore.kernel.org/all/cover.1723061377.git.nicolinc@nvidia.com/
> 
> Thanks!
> Nicolin
> 
> Jason Gunthorpe (2):
>    iommu: Add iommu_copy_struct_from_full_user_array helper
>    iommu/arm-smmu-v3: Allow ATS for IOMMU_DOMAIN_NESTED
> 
> Nicolin Chen (12):
>    iommufd/viommu: Introduce IOMMUFD_OBJ_VDEVICE and its related struct
>    iommufd/viommu: Add IOMMU_VDEVICE_ALLOC ioctl
>    iommufd/selftest: Add IOMMU_VDEVICE_ALLOC test coverage
>    iommu/viommu: Add cache_invalidate to iommufd_viommu_ops
>    iommufd/hw_pagetable: Enforce cache invalidation op on vIOMMU-based
>      hwpt_nested
>    iommufd: Allow hwpt_id to carry viommu_id for IOMMU_HWPT_INVALIDATE
>    iommufd/viommu: Add vdev_to_dev helper
>    iommufd/selftest: Add mock_viommu_cache_invalidate
>    iommufd/selftest: Add IOMMU_TEST_OP_DEV_CHECK_CACHE test command
>    iommufd/selftest: Add vIOMMU coverage for IOMMU_HWPT_INVALIDATE ioctl
>    Documentation: userspace-api: iommufd: Update vDEVICE
>    iommu/arm-smmu-v3: Add arm_vsmmu_cache_invalidate
> 
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   9 +-
>   drivers/iommu/iommufd/iommufd_private.h       |  12 ++
>   drivers/iommu/iommufd/iommufd_test.h          |  30 +++
>   include/linux/iommu.h                         |  49 ++++-
>   include/linux/iommufd.h                       |  50 +++++
>   include/uapi/linux/iommufd.h                  |  61 +++++-
>   tools/testing/selftests/iommu/iommufd_utils.h |  83 +++++++
>   .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     | 162 +++++++++++++-
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  32 ++-
>   drivers/iommu/iommufd/device.c                |  11 +
>   drivers/iommu/iommufd/driver.c                |   7 +
>   drivers/iommu/iommufd/hw_pagetable.c          |  36 +++-
>   drivers/iommu/iommufd/main.c                  |   7 +
>   drivers/iommu/iommufd/selftest.c              | 115 +++++++++-
>   drivers/iommu/iommufd/viommu.c                | 108 ++++++++++
>   tools/testing/selftests/iommu/iommufd.c       | 204 +++++++++++++++++-
>   .../selftests/iommu/iommufd_fail_nth.c        |   4 +
>   Documentation/userspace-api/iommufd.rst       |  41 +++-
>   18 files changed, 983 insertions(+), 38 deletions(-)
>
Nicolin Chen Oct. 25, 2024, 4:59 a.m. UTC | #2
On Fri, Oct 25, 2024 at 03:54:44PM +1100, Alexey Kardashevskiy wrote:
> On 22/10/24 11:20, Nicolin Chen wrote:
> > Following the previous vIOMMU series, this adds another vDEVICE structure,
> > representing the association from an iommufd_device to an iommufd_viommu.
> > This gives the whole architecture a new "v" layer:
> >    _______________________________________________________________________
> >   |                      iommufd (with vIOMMU/vDEVICE)                    |
> >   |                        _____________      _____________               |
> >   |                       |             |    |             |              |
> >   |      |----------------|    vIOMMU   |<---|   vDEVICE   |<------|      |
> >   |      |                |             |    |_____________|       |      |
> >   |      |     ______     |             |     _____________     ___|____  |
> >   |      |    |      |    |             |    |             |   |        | |
> >   |      |    | IOAS |<---|(HWPT_PAGING)|<---| HWPT_NESTED |<--| DEVICE | |
> >   |      |    |______|    |_____________|    |_____________|   |________| |
> >   |______|________|______________|__________________|_______________|_____|
> >          |        |              |                  |               |
> >    ______v_____   |        ______v_____       ______v_____       ___v__
> >   |   struct   |  |  PFN  |  (paging)  |     |  (nested)  |     |struct|
> >   |iommu_device|  |------>|iommu_domain|<----|iommu_domain|<----|device|
> >   |____________|   storage|____________|     |____________|     |______|
> > 
> > This vDEVICE object is used to collect and store all vIOMMU-related device
> > information/attributes in a VM. As an initial series for vDEVICE, add only
> > the virt_id to the vDEVICE, which is a vIOMMU specific device ID in a VM:
> > e.g. vSID of ARM SMMUv3, vDeviceID of AMD IOMMU, and vID of Intel VT-d to
> > a Context Table. This virt_id helps IOMMU drivers to link the vID to a pID
> > of the device against the physical IOMMU instance. This is essential for a
> > vIOMMU-based invalidation, where the request contains a device's vID for a
> > device cache flush, e.g. ATC invalidation.
> > 
> > Therefore, with this vDEVICE object, support a vIOMMU-based invalidation,
> > by reusing IOMMUFD_CMD_HWPT_INVALIDATE for a vIOMMU object to flush cache
> > with a given driver data.
> > 
> > As for the implementation of the series, add driver support in ARM SMMUv3
> > for a real world use case.
> > 
> > This series is on Github:
> > https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4
> > 
> > For testing, try this "with-rmr" branch:
> > https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4-with-rmr
> 
> Is there any real example of a .vdevice_alloc hook, besides the
> selftests? It is not in iommufd_viommu_p2-v4-with-rmr, hence the
> question. I am trying to sketch something with this new machinery and
> less guessing would be nice. Thanks,

No, I am actually dropping that one, and moving the vdevice struct
to the private header, as there seems to be no use case:
https://lore.kernel.org/linux-iommu/ZxsSYbK3gqyC84U7@Asurada-Nvidia/
https://lore.kernel.org/linux-iommu/ZxsTAANTTuQzQ9HR@Asurada-Nvidia/

Do you need vdevice_alloc in the driver for your sketch?

Thanks
Nicolin
Alexey Kardashevskiy Oct. 25, 2024, 5:32 a.m. UTC | #3
On 25/10/24 15:59, Nicolin Chen wrote:
> On Fri, Oct 25, 2024 at 03:54:44PM +1100, Alexey Kardashevskiy wrote:
>> On 22/10/24 11:20, Nicolin Chen wrote:
>>> Following the previous vIOMMU series, this adds another vDEVICE structure,
>>> representing the association from an iommufd_device to an iommufd_viommu.
>>> This gives the whole architecture a new "v" layer:
>>>     _______________________________________________________________________
>>>    |                      iommufd (with vIOMMU/vDEVICE)                    |
>>>    |                        _____________      _____________               |
>>>    |                       |             |    |             |              |
>>>    |      |----------------|    vIOMMU   |<---|   vDEVICE   |<------|      |
>>>    |      |                |             |    |_____________|       |      |
>>>    |      |     ______     |             |     _____________     ___|____  |
>>>    |      |    |      |    |             |    |             |   |        | |
>>>    |      |    | IOAS |<---|(HWPT_PAGING)|<---| HWPT_NESTED |<--| DEVICE | |
>>>    |      |    |______|    |_____________|    |_____________|   |________| |
>>>    |______|________|______________|__________________|_______________|_____|
>>>           |        |              |                  |               |
>>>     ______v_____   |        ______v_____       ______v_____       ___v__
>>>    |   struct   |  |  PFN  |  (paging)  |     |  (nested)  |     |struct|
>>>    |iommu_device|  |------>|iommu_domain|<----|iommu_domain|<----|device|
>>>    |____________|   storage|____________|     |____________|     |______|
>>>
>>> This vDEVICE object is used to collect and store all vIOMMU-related device
>>> information/attributes in a VM. As an initial series for vDEVICE, add only
>>> the virt_id to the vDEVICE, which is a vIOMMU specific device ID in a VM:
>>> e.g. vSID of ARM SMMUv3, vDeviceID of AMD IOMMU, and vID of Intel VT-d to
>>> a Context Table. This virt_id helps IOMMU drivers to link the vID to a pID
>>> of the device against the physical IOMMU instance. This is essential for a
>>> vIOMMU-based invalidation, where the request contains a device's vID for a
>>> device cache flush, e.g. ATC invalidation.
>>>
>>> Therefore, with this vDEVICE object, support a vIOMMU-based invalidation,
>>> by reusing IOMMUFD_CMD_HWPT_INVALIDATE for a vIOMMU object to flush cache
>>> with a given driver data.
>>>
>>> As for the implementation of the series, add driver support in ARM SMMUv3
>>> for a real world use case.
>>>
>>> This series is on Github:
>>> https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4
>>>
>>> For testing, try this "with-rmr" branch:
>>> https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4-with-rmr
>>
>> Is there any real example of a .vdevice_alloc hook, besides the
>> selftests? It is not in iommufd_viommu_p2-v4-with-rmr, hence the
>> question. I am trying to sketch something with this new machinery and
>> less guessing would be nice. Thanks,
> 
> No, I am actually dropping that one, and moving the vdevice struct
> to the private header, as there seems to be no use case:

Why keep it then?

> https://lore.kernel.org/linux-iommu/ZxsSYbK3gqyC84U7@Asurada-Nvidia/
> https://lore.kernel.org/linux-iommu/ZxsTAANTTuQzQ9HR@Asurada-Nvidia/
> 
> Do you need vdevice_alloc in the driver for your sketch?

At the moment one of the things I am looking for is a place to add 
tsm_bind(host_bdfn, guest_bdfn, kvm_vmid). I assumed that this vdevice 
represents the guest's IOMMU attributes for a passed through device and 
naturally stores the guest_bdfn as an id (for AMD). I am still trying to 
wrap my head around these new things. Thanks,
Nicolin Chen Oct. 25, 2024, 5:41 a.m. UTC | #4
On Fri, Oct 25, 2024 at 04:32:10PM +1100, Alexey Kardashevskiy wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 25/10/24 15:59, Nicolin Chen wrote:
> > On Fri, Oct 25, 2024 at 03:54:44PM +1100, Alexey Kardashevskiy wrote:
> > > On 22/10/24 11:20, Nicolin Chen wrote:
> > > > Following the previous vIOMMU series, this adds another vDEVICE structure,
> > > > representing the association from an iommufd_device to an iommufd_viommu.
> > > > This gives the whole architecture a new "v" layer:
> > > >     _______________________________________________________________________
> > > >    |                      iommufd (with vIOMMU/vDEVICE)                    |
> > > >    |                        _____________      _____________               |
> > > >    |                       |             |    |             |              |
> > > >    |      |----------------|    vIOMMU   |<---|   vDEVICE   |<------|      |
> > > >    |      |                |             |    |_____________|       |      |
> > > >    |      |     ______     |             |     _____________     ___|____  |
> > > >    |      |    |      |    |             |    |             |   |        | |
> > > >    |      |    | IOAS |<---|(HWPT_PAGING)|<---| HWPT_NESTED |<--| DEVICE | |
> > > >    |      |    |______|    |_____________|    |_____________|   |________| |
> > > >    |______|________|______________|__________________|_______________|_____|
> > > >           |        |              |                  |               |
> > > >     ______v_____   |        ______v_____       ______v_____       ___v__
> > > >    |   struct   |  |  PFN  |  (paging)  |     |  (nested)  |     |struct|
> > > >    |iommu_device|  |------>|iommu_domain|<----|iommu_domain|<----|device|
> > > >    |____________|   storage|____________|     |____________|     |______|
> > > > 
> > > > This vDEVICE object is used to collect and store all vIOMMU-related device
> > > > information/attributes in a VM. As an initial series for vDEVICE, add only
> > > > the virt_id to the vDEVICE, which is a vIOMMU specific device ID in a VM:
> > > > e.g. vSID of ARM SMMUv3, vDeviceID of AMD IOMMU, and vID of Intel VT-d to
> > > > a Context Table. This virt_id helps IOMMU drivers to link the vID to a pID
> > > > of the device against the physical IOMMU instance. This is essential for a
> > > > vIOMMU-based invalidation, where the request contains a device's vID for a
> > > > device cache flush, e.g. ATC invalidation.
> > > > 
> > > > Therefore, with this vDEVICE object, support a vIOMMU-based invalidation,
> > > > by reusing IOMMUFD_CMD_HWPT_INVALIDATE for a vIOMMU object to flush cache
> > > > with a given driver data.
> > > > 
> > > > As for the implementation of the series, add driver support in ARM SMMUv3
> > > > for a real world use case.
> > > > 
> > > > This series is on Github:
> > > > https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4
> > > > 
> > > > For testing, try this "with-rmr" branch:
> > > > https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4-with-rmr
> > > 
> > > Is there any real example of a .vdevice_alloc hook, besides the
> > > selftests? It is not in iommufd_viommu_p2-v4-with-rmr, hence the
> > > question. I am trying to sketch something with this new machinery and
> > > less guessing would be nice. Thanks,
> > 
> > No, I am actually dropping that one, and moving the vdevice struct
> > to the private header, as there seems to be no use case:
> 
> Why keep it then?

We need that structure to store per-vIOMMU virtual ID. Hiding it
in the core only means we need to provide another vIOMMU APIs for
drivers to look up the ID, v.s. exposing it for drivers to access
directly.

Thanks
Nicolin
Alexey Kardashevskiy Oct. 25, 2024, 5:58 a.m. UTC | #5
On 25/10/24 16:41, Nicolin Chen wrote:
> On Fri, Oct 25, 2024 at 04:32:10PM +1100, Alexey Kardashevskiy wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 25/10/24 15:59, Nicolin Chen wrote:
>>> On Fri, Oct 25, 2024 at 03:54:44PM +1100, Alexey Kardashevskiy wrote:
>>>> On 22/10/24 11:20, Nicolin Chen wrote:
>>>>> Following the previous vIOMMU series, this adds another vDEVICE structure,
>>>>> representing the association from an iommufd_device to an iommufd_viommu.
>>>>> This gives the whole architecture a new "v" layer:
>>>>>      _______________________________________________________________________
>>>>>     |                      iommufd (with vIOMMU/vDEVICE)                    |
>>>>>     |                        _____________      _____________               |
>>>>>     |                       |             |    |             |              |
>>>>>     |      |----------------|    vIOMMU   |<---|   vDEVICE   |<------|      |
>>>>>     |      |                |             |    |_____________|       |      |
>>>>>     |      |     ______     |             |     _____________     ___|____  |
>>>>>     |      |    |      |    |             |    |             |   |        | |
>>>>>     |      |    | IOAS |<---|(HWPT_PAGING)|<---| HWPT_NESTED |<--| DEVICE | |
>>>>>     |      |    |______|    |_____________|    |_____________|   |________| |
>>>>>     |______|________|______________|__________________|_______________|_____|
>>>>>            |        |              |                  |               |
>>>>>      ______v_____   |        ______v_____       ______v_____       ___v__
>>>>>     |   struct   |  |  PFN  |  (paging)  |     |  (nested)  |     |struct|
>>>>>     |iommu_device|  |------>|iommu_domain|<----|iommu_domain|<----|device|
>>>>>     |____________|   storage|____________|     |____________|     |______|
>>>>>
>>>>> This vDEVICE object is used to collect and store all vIOMMU-related device
>>>>> information/attributes in a VM. As an initial series for vDEVICE, add only
>>>>> the virt_id to the vDEVICE, which is a vIOMMU specific device ID in a VM:
>>>>> e.g. vSID of ARM SMMUv3, vDeviceID of AMD IOMMU, and vID of Intel VT-d to
>>>>> a Context Table. This virt_id helps IOMMU drivers to link the vID to a pID
>>>>> of the device against the physical IOMMU instance. This is essential for a
>>>>> vIOMMU-based invalidation, where the request contains a device's vID for a
>>>>> device cache flush, e.g. ATC invalidation.
>>>>>
>>>>> Therefore, with this vDEVICE object, support a vIOMMU-based invalidation,
>>>>> by reusing IOMMUFD_CMD_HWPT_INVALIDATE for a vIOMMU object to flush cache
>>>>> with a given driver data.
>>>>>
>>>>> As for the implementation of the series, add driver support in ARM SMMUv3
>>>>> for a real world use case.
>>>>>
>>>>> This series is on Github:
>>>>> https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4
>>>>>
>>>>> For testing, try this "with-rmr" branch:
>>>>> https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p2-v4-with-rmr
>>>>
>>>> Is there any real example of a .vdevice_alloc hook, besides the
>>>> selftests? It is not in iommufd_viommu_p2-v4-with-rmr, hence the
>>>> question. I am trying to sketch something with this new machinery and
>>>> less guessing would be nice. Thanks,
>>>
>>> No, I am actually dropping that one, and moving the vdevice struct
>>> to the private header, as there seems to be no use case:
>>
>> Why keep it then?
> 
> We need that structure to store per-vIOMMU virtual ID. Hiding it
> in the core only means we need to provide another vIOMMU APIs for
> drivers to look up the ID, v.s. exposing it for drivers to access
> directly.

Sorry I lost you here. If we need it, then there should be an example of 
.vdevice_alloc() somewhere but you say they is not one. How do you test 
this, with just selftests? :) Thanks,


> 
> Thanks
> Nicolin
Nicolin Chen Oct. 25, 2024, 6:14 a.m. UTC | #6
On Fri, Oct 25, 2024 at 04:58:33PM +1100, Alexey Kardashevskiy wrote:
> > > > > Is there any real example of a .vdevice_alloc hook, besides the
> > > > > selftests? It is not in iommufd_viommu_p2-v4-with-rmr, hence the
> > > > > question. I am trying to sketch something with this new machinery and
> > > > > less guessing would be nice. Thanks,
> > > > 
> > > > No, I am actually dropping that one, and moving the vdevice struct
> > > > to the private header, as there seems to be no use case:
> > > 
> > > Why keep it then?
> > 
> > We need that structure to store per-vIOMMU virtual ID. Hiding it
> > in the core only means we need to provide another vIOMMU APIs for
> > drivers to look up the ID, v.s. exposing it for drivers to access
> > directly.
> 
> Sorry I lost you here. If we need it, then there should be an example of
> .vdevice_alloc() somewhere but you say they is not one. How do you test
> this, with just selftests? :) Thanks,

A vDEVICE object will be core-allocated and core-managed, while the
vdevice_alloc is for driver-allocated purpose for which there is no
use case (at least with this series). You can check the vdev ioctl
in this version that has two pathways to allocate a vDEVICE object.

A vdev_id is used to index viommu's xarray for a driver to convert
the id to a dev pointer via a vIOMMU API. Dropping .vdevice_alloc
just means the driver only lost its direct access.

Nicolin
Jason Gunthorpe Oct. 25, 2024, 1:29 p.m. UTC | #7
On Thu, Oct 24, 2024 at 11:14:21PM -0700, Nicolin Chen wrote:
> On Fri, Oct 25, 2024 at 04:58:33PM +1100, Alexey Kardashevskiy wrote:
> > > > > > Is there any real example of a .vdevice_alloc hook, besides the
> > > > > > selftests? It is not in iommufd_viommu_p2-v4-with-rmr, hence the
> > > > > > question. I am trying to sketch something with this new machinery and
> > > > > > less guessing would be nice. Thanks,
> > > > > 
> > > > > No, I am actually dropping that one, and moving the vdevice struct
> > > > > to the private header, as there seems to be no use case:
> > > > 
> > > > Why keep it then?
> > > 
> > > We need that structure to store per-vIOMMU virtual ID. Hiding it
> > > in the core only means we need to provide another vIOMMU APIs for
> > > drivers to look up the ID, v.s. exposing it for drivers to access
> > > directly.
> > 
> > Sorry I lost you here. If we need it, then there should be an example of
> > .vdevice_alloc() somewhere but you say they is not one. How do you test
> > this, with just selftests? :) Thanks,
> 
> A vDEVICE object will be core-allocated and core-managed, while the
> vdevice_alloc is for driver-allocated purpose for which there is no
> use case (at least with this series). You can check the vdev ioctl
> in this version that has two pathways to allocate a vDEVICE object.
> 
> A vdev_id is used to index viommu's xarray for a driver to convert
> the id to a dev pointer via a vIOMMU API. Dropping .vdevice_alloc
> just means the driver only lost its direct access.

I think the point here is this has to go in stages at the present
moment the iommu drivers don't need to hook the vdevice object, so
Nicolin should take it out of this series.

I would expect CC to need to be in this path, so we should bring it
back in the CC series.

For CC I'm broadly expecting that creating the CC type vIOMMU will
call a CC implementation, and then creating a vdevice against the
vIOMMU will also call the CC implementation. The two callbacks would
ask the secure world to create the relevant VM visible objects.

Jason