mbox series

[v4,00/12] Initial support for SMMUv3 nested translation

Message ID 0-v4-9e99b76f3518+3a8-smmuv3_nesting_jgg@nvidia.com (mailing list archive)
Headers show
Series Initial support for SMMUv3 nested translation | expand

Message

Jason Gunthorpe Oct. 31, 2024, 12:20 a.m. UTC
[This is now based on Nicolin's iommufd patches for vIOMMU and will need
to go through the iommufd tree, please ack]

This brings support for the IOMMFD ioctls:

 - IOMMU_GET_HW_INFO
 - IOMMU_HWPT_ALLOC_NEST_PARENT
 - IOMMU_VIOMMU_ALLOC
 - IOMMU_DOMAIN_NESTED
 - IOMMU_HWPT_INVALIDATE
 - ops->enforce_cache_coherency()

This is quite straightforward as the nested STE can just be built in the
special NESTED domain op and fed through the generic update machinery.

The design allows the user provided STE fragment to control several
aspects of the translation, including putting the STE into a "virtual
bypass" or a aborting state. This duplicates functionality available by
other means, but it allows trivially preserving the VMID in the STE as we
eventually move towards the vIOMMU owning the VMID.

Nesting support requires the system to either support S2FWB or the
stronger CANWBS ACPI flag. This is to ensure the VM cannot bypass the
cache and view incoherent data, currently VFIO lacks any cache flushing
that would make this safe.

Yan has a series to add some of the needed infrastructure for VFIO cache
flushing here:

 https://lore.kernel.org/linux-iommu/20240507061802.20184-1-yan.y.zhao@intel.com/

Which may someday allow relaxing this further.

The VIOMMU object provides the framework to allow the invalidation path to
translate the vSID to a pSID and then issue the correct physical
invalidation. This is all done in the kernel as pSID has to
limited. Future patches will extend VIOMMU to handle specific HW features
like vMPAM and NVIDIA's vCMDQ.

Remove VFIO_TYPE1_NESTING_IOMMU since it was never used and superseded by
this.

This is the first series in what will be several to complete nesting
support. At least:
 - IOMMU_RESV_SW_MSI related fixups
    https://lore.kernel.org/linux-iommu/cover.1722644866.git.nicolinc@nvidia.com/
 - vCMDQ hypervisor support for direct invalidation queue assignment
    https://lore.kernel.org/linux-iommu/cover.1712978212.git.nicolinc@nvidia.com/
 - KVM pinned VMID using vIOMMU for vBTM
    https://lore.kernel.org/linux-iommu/20240208151837.35068-1-shameerali.kolothum.thodi@huawei.com/
 - Cross instance S2 sharing
 - Virtual Machine Structure using vIOMMU (for vMPAM?)
 - Fault forwarding support through IOMMUFD's fault fd for vSVA

The vIOMMU series is essential to allow the invalidations to be processed
for the CD as well.

It is enough to allow qemu work to progress.

This is on github: https://github.com/jgunthorpe/linux/commits/smmuv3_nesting

v4:
 - Rebase on Nicolin's patches
 - Add user_pasid_table=1 to support fault reporting on NESTED domains
 - Reorder STRTAB constants
 - Fix whitespace
 - Roll in the patches Nicolin had and merge together into a logical order
   Includes vIOMMU, ATS and invalidation patches
v3: https://patch.msgid.link/r/0-v3-e2e16cd7467f+2a6a1-smmuv3_nesting_jgg@nvidia.com
 - Rebase on v6.12-rc2
 - Revise commit messages
 - Consolidate CANWB checks into arm_smmu_master_canwbs()
 - Add CONFIG_ARM_SMMU_V3_IOMMUFD to compile out iommufd only features
   like nesting
 - Shift code into arm-smmu-v3-iommufd.c
 - Add missed IS_ERR check
 - Add S2FWB to arm_smmu_get_ste_used()
 - Fixup quirks checks
 - Drop ARM_SMMU_FEAT_COHERENCY checks for S2FWB
 - Limit S2FWB to S2 Nesting Parent domains "just in case"
v2: https://patch.msgid.link/r/0-v2-621370057090+91fec-smmuv3_nesting_jgg@nvidia.com
 - Revise commit messages
 - Guard S2FWB support with ARM_SMMU_FEAT_COHERENCY, since it doesn't make
   sense to use S2FWB to enforce coherency on inherently non-coherent hardware.
 - Add missing IO_PGTABLE_QUIRK_ARM_S2FWB validation
 - Include formal ACPIA commit for IORT built using
   generate/linux/gen-patch.sh
 - Use FEAT_NESTING to block creating a NESTING_PARENT
 - Use an abort STE instead of non-valid if the user requests a non-valid
   vSTE
 - Consistently use 'nest_parent' for naming variables
 - Use the right domain for arm_smmu_remove_master_domain() when it
   removes the master
 - Join bitfields together
 - Drop arm_smmu_cache_invalidate_user patch, invalidation will
   exclusively go via viommu
v1: https://patch.msgid.link/r/0-v1-54e734311a7f+14f72-smmuv3_nesting_jgg@nvidia.com

Jason Gunthorpe (7):
  vfio: Remove VFIO_TYPE1_NESTING_IOMMU
  iommu/arm-smmu-v3: Report IOMMU_CAP_ENFORCE_CACHE_COHERENCY for CANWBS
  iommu/arm-smmu-v3: Implement IOMMU_HWPT_ALLOC_NEST_PARENT
  iommu/arm-smmu-v3: Expose the arm_smmu_attach interface
  iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED
  iommu/arm-smmu-v3: Use S2FWB for NESTED domains
  iommu/arm-smmu-v3: Allow ATS for IOMMU_DOMAIN_NESTED

Nicolin Chen (5):
  ACPICA: IORT: Update for revision E.f
  ACPI/IORT: Support CANWBS memory access flag
  iommu/arm-smmu-v3: Support IOMMU_GET_HW_INFO via struct
    arm_smmu_hw_info
  iommu/arm-smmu-v3: Support IOMMU_VIOMMU_ALLOC
  iommu/arm-smmu-v3: Support IOMMU_HWPT_INVALIDATE using a VIOMMU object

 drivers/acpi/arm64/iort.c                     |  13 +
 drivers/iommu/Kconfig                         |   9 +
 drivers/iommu/arm/arm-smmu-v3/Makefile        |   1 +
 .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c     | 393 ++++++++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 139 +++++--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  92 +++-
 drivers/iommu/arm/arm-smmu/arm-smmu.c         |  16 -
 drivers/iommu/io-pgtable-arm.c                |  27 +-
 drivers/iommu/iommu.c                         |  10 -
 drivers/iommu/iommufd/vfio_compat.c           |   7 +-
 drivers/vfio/vfio_iommu_type1.c               |  12 +-
 include/acpi/actbl2.h                         |   3 +-
 include/linux/io-pgtable.h                    |   2 +
 include/linux/iommu.h                         |   5 +-
 include/uapi/linux/iommufd.h                  |  83 ++++
 include/uapi/linux/vfio.h                     |   2 +-
 16 files changed, 712 insertions(+), 102 deletions(-)
 create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c


base-commit: 9ffbeb478d44c57b9b2e263750b1056e5faebc9b

Comments

Nicolin Chen Oct. 31, 2024, 12:53 a.m. UTC | #1
On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> The vIOMMU series is essential to allow the invalidations to be processed
> for the CD as well.
> 
> It is enough to allow qemu work to progress.
> 
> This is on github: https://github.com/jgunthorpe/linux/commits/smmuv3_nesting

Tested-by: Nicolin Chen <nicolinc@nvidia.com>

With a branch adding RMR changes on top of this smmuv3_nesting:
https://github.com/nicolinc/iommufd/commits/smmuv3_nesting-with-rmr
and with an *updated* paring QEMU branch:
https://github.com/nicolinc/qemu/commits/wip/for_smmuv3_nesting-v4

For folks who are interested in testing, please use this QEMU
branch, as there are uAPI changes against previous verions.

Thanks
Nicolin
Will Deacon Nov. 1, 2024, 12:18 p.m. UTC | #2
On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> [This is now based on Nicolin's iommufd patches for vIOMMU and will need
> to go through the iommufd tree, please ack]

Can't we separate out the SMMUv3 driver changes? They shouldn't depend on
Nicolin's work afaict.

Will
Jason Gunthorpe Nov. 1, 2024, 1:25 p.m. UTC | #3
On Fri, Nov 01, 2024 at 12:18:50PM +0000, Will Deacon wrote:
> On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> > [This is now based on Nicolin's iommufd patches for vIOMMU and will need
> > to go through the iommufd tree, please ack]
> 
> Can't we separate out the SMMUv3 driver changes? They shouldn't depend on
> Nicolin's work afaict.

We can put everything before "iommu/arm-smmu-v3: Support
IOMMU_VIOMMU_ALLOC" directly on a rc and share a branch with your tree.

That patch and following all depend on Nicolin's work, as they rely on
the VIOMMU due to how different ARM is from Intel.

How about you take these patches:

 [PATCH v4 01/12] vfio: Remove VFIO_TYPE1_NESTING_IOMMU
 [PATCH v4 02/12] ACPICA: IORT: Update for revision E.f
 [PATCH v4 03/12] ACPI/IORT: Support CANWBS memory access flag
 [PATCH v4 04/12] iommu/arm-smmu-v3: Report IOMMU_CAP_ENFORCE_CACHE_COHERENCY for CANWBS
 [PATCH v4 05/12] iommu/arm-smmu-v3: Support IOMMU_GET_HW_INFO via struct arm_smmu_hw_info
 [PATCH v4 06/12] iommu/arm-smmu-v3: Implement IOMMU_HWPT_ALLOC_NEST_PARENT
 [PATCH v4 07/12] iommu/arm-smmu-v3: Expose the arm_smmu_attach interface

Onto a branch.

I'll take these patches after merging your branch and Nicolin's:

 [PATCH v4 08/12] iommu/arm-smmu-v3: Support IOMMU_VIOMMU_ALLOC
 [PATCH v4 09/12] iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED
 [PATCH v4 10/12] iommu/arm-smmu-v3: Use S2FWB for NESTED domains
 [PATCH v4 11/12] iommu/arm-smmu-v3: Allow ATS for IOMMU_DOMAIN_NESTED
 [PATCH v4 12/12] iommu/arm-smmu-v3: Support IOMMU_HWPT_INVALIDATE using a VIOMMU object

?

I can also probably push most of S2FWB and ATS into the first batch.

Please let me know, I would like this to be done this cycle, Nicolin's
vIOMMU series are all reviewed now.

Jason
Will Deacon Nov. 5, 2024, 4:48 p.m. UTC | #4
Hi Jason,

On Fri, Nov 01, 2024 at 10:25:03AM -0300, Jason Gunthorpe wrote:
> On Fri, Nov 01, 2024 at 12:18:50PM +0000, Will Deacon wrote:
> > On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> > > [This is now based on Nicolin's iommufd patches for vIOMMU and will need
> > > to go through the iommufd tree, please ack]
> > 
> > Can't we separate out the SMMUv3 driver changes? They shouldn't depend on
> > Nicolin's work afaict.
> 
> We can put everything before "iommu/arm-smmu-v3: Support
> IOMMU_VIOMMU_ALLOC" directly on a rc and share a branch with your tree.
> 
> That patch and following all depend on Nicolin's work, as they rely on
> the VIOMMU due to how different ARM is from Intel.
> 
> How about you take these patches:
> 
>  [PATCH v4 01/12] vfio: Remove VFIO_TYPE1_NESTING_IOMMU
>  [PATCH v4 02/12] ACPICA: IORT: Update for revision E.f
>  [PATCH v4 03/12] ACPI/IORT: Support CANWBS memory access flag
>  [PATCH v4 04/12] iommu/arm-smmu-v3: Report IOMMU_CAP_ENFORCE_CACHE_COHERENCY for CANWBS
>  [PATCH v4 05/12] iommu/arm-smmu-v3: Support IOMMU_GET_HW_INFO via struct arm_smmu_hw_info
>  [PATCH v4 06/12] iommu/arm-smmu-v3: Implement IOMMU_HWPT_ALLOC_NEST_PARENT
>  [PATCH v4 07/12] iommu/arm-smmu-v3: Expose the arm_smmu_attach interface
> 
> Onto a branch.

I've pushed these onto a new branch in the IOMMU tree:

	iommufd/arm-smmuv3-nested

However, please can you give it a day or two to get some exposure in
-next before you merge that into iommufd? I'll ping back here later in
the week.

Cheers,

Will
Jason Gunthorpe Nov. 5, 2024, 5:03 p.m. UTC | #5
On Tue, Nov 05, 2024 at 04:48:29PM +0000, Will Deacon wrote:
> Hi Jason,
> 
> On Fri, Nov 01, 2024 at 10:25:03AM -0300, Jason Gunthorpe wrote:
> > On Fri, Nov 01, 2024 at 12:18:50PM +0000, Will Deacon wrote:
> > > On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> > > > [This is now based on Nicolin's iommufd patches for vIOMMU and will need
> > > > to go through the iommufd tree, please ack]
> > > 
> > > Can't we separate out the SMMUv3 driver changes? They shouldn't depend on
> > > Nicolin's work afaict.
> > 
> > We can put everything before "iommu/arm-smmu-v3: Support
> > IOMMU_VIOMMU_ALLOC" directly on a rc and share a branch with your tree.
> > 
> > That patch and following all depend on Nicolin's work, as they rely on
> > the VIOMMU due to how different ARM is from Intel.
> > 
> > How about you take these patches:
> > 
> >  [PATCH v4 01/12] vfio: Remove VFIO_TYPE1_NESTING_IOMMU
> >  [PATCH v4 02/12] ACPICA: IORT: Update for revision E.f
> >  [PATCH v4 03/12] ACPI/IORT: Support CANWBS memory access flag
> >  [PATCH v4 04/12] iommu/arm-smmu-v3: Report IOMMU_CAP_ENFORCE_CACHE_COHERENCY for CANWBS
> >  [PATCH v4 05/12] iommu/arm-smmu-v3: Support IOMMU_GET_HW_INFO via struct arm_smmu_hw_info
> >  [PATCH v4 06/12] iommu/arm-smmu-v3: Implement IOMMU_HWPT_ALLOC_NEST_PARENT
> >  [PATCH v4 07/12] iommu/arm-smmu-v3: Expose the arm_smmu_attach interface
> > 
> > Onto a branch.
> 
> I've pushed these onto a new branch in the IOMMU tree:
> 
> 	iommufd/arm-smmuv3-nested
> 
> However, please can you give it a day or two to get some exposure in
> -next before you merge that into iommufd? I'll ping back here later in
> the week.

Thanks, I will hope to put the other parts together on Friday then so
it can also get its time in -next, as we are running out of days
before the merge window

0-day is still complaining about some kconfig in Nicolin's patches so
we need to settle that anyhow.

Jason
Will Deacon Nov. 8, 2024, 2:56 p.m. UTC | #6
On Tue, Nov 05, 2024 at 04:48:29PM +0000, Will Deacon wrote:
> On Fri, Nov 01, 2024 at 10:25:03AM -0300, Jason Gunthorpe wrote:
> > On Fri, Nov 01, 2024 at 12:18:50PM +0000, Will Deacon wrote:
> > > On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> > > > [This is now based on Nicolin's iommufd patches for vIOMMU and will need
> > > > to go through the iommufd tree, please ack]
> > > 
> > > Can't we separate out the SMMUv3 driver changes? They shouldn't depend on
> > > Nicolin's work afaict.
> > 
> > We can put everything before "iommu/arm-smmu-v3: Support
> > IOMMU_VIOMMU_ALLOC" directly on a rc and share a branch with your tree.
> > 
> > That patch and following all depend on Nicolin's work, as they rely on
> > the VIOMMU due to how different ARM is from Intel.
> > 
> > How about you take these patches:
> > 
> >  [PATCH v4 01/12] vfio: Remove VFIO_TYPE1_NESTING_IOMMU
> >  [PATCH v4 02/12] ACPICA: IORT: Update for revision E.f
> >  [PATCH v4 03/12] ACPI/IORT: Support CANWBS memory access flag
> >  [PATCH v4 04/12] iommu/arm-smmu-v3: Report IOMMU_CAP_ENFORCE_CACHE_COHERENCY for CANWBS
> >  [PATCH v4 05/12] iommu/arm-smmu-v3: Support IOMMU_GET_HW_INFO via struct arm_smmu_hw_info
> >  [PATCH v4 06/12] iommu/arm-smmu-v3: Implement IOMMU_HWPT_ALLOC_NEST_PARENT
> >  [PATCH v4 07/12] iommu/arm-smmu-v3: Expose the arm_smmu_attach interface
> > 
> > Onto a branch.
> 
> I've pushed these onto a new branch in the IOMMU tree:
> 
> 	iommufd/arm-smmuv3-nested
> 
> However, please can you give it a day or two to get some exposure in
> -next before you merge that into iommufd? I'll ping back here later in
> the week.

Not seen anything go bang in -next, so I think we can consider this
branch stable now.

Will
Jason Gunthorpe Nov. 12, 2024, 6:29 p.m. UTC | #7
On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> Jason Gunthorpe (7):
>   iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED
>   iommu/arm-smmu-v3: Use S2FWB for NESTED domains
>   iommu/arm-smmu-v3: Allow ATS for IOMMU_DOMAIN_NESTED
> 
> Nicolin Chen (5):
>   iommu/arm-smmu-v3: Support IOMMU_VIOMMU_ALLOC
>   iommu/arm-smmu-v3: Support IOMMU_HWPT_INVALIDATE using a VIOMMU object

Applied to iommufd for-next along with all the dependencies and the
additional hunk Zhangfei pointed out.

Thanks,
Jason
Zhangfei Gao Nov. 13, 2024, 1:01 a.m. UTC | #8
On Wed, 13 Nov 2024 at 02:29, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> > Jason Gunthorpe (7):
> >   iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED
> >   iommu/arm-smmu-v3: Use S2FWB for NESTED domains
> >   iommu/arm-smmu-v3: Allow ATS for IOMMU_DOMAIN_NESTED
> >
> > Nicolin Chen (5):
> >   iommu/arm-smmu-v3: Support IOMMU_VIOMMU_ALLOC
> >   iommu/arm-smmu-v3: Support IOMMU_HWPT_INVALIDATE using a VIOMMU object
>
> Applied to iommufd for-next along with all the dependencies and the
> additional hunk Zhangfei pointed out.

Thanks Jason, I have verified on aarch64 based on your
jason/smmuv3_nesting branch

https://github.com/Linaro/linux-kernel-uadk/tree/6.12-wip
https://github.com/Linaro/qemu/tree/6.12-wip

Still need this hack
https://github.com/Linaro/linux-kernel-uadk/commit/eaa194d954112cad4da7852e29343e546baf8683

One is adding iommu_dev_enable/disable_feature IOMMU_DEV_FEAT_SVA,
which you have patchset before.
The other is to temporarily ignore S2FWB or CANWBS.

Thanks
Jason Gunthorpe Nov. 13, 2024, 1:23 a.m. UTC | #9
On Wed, Nov 13, 2024 at 09:01:41AM +0800, Zhangfei Gao wrote:
> On Wed, 13 Nov 2024 at 02:29, Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Wed, Oct 30, 2024 at 09:20:44PM -0300, Jason Gunthorpe wrote:
> > > Jason Gunthorpe (7):
> > >   iommu/arm-smmu-v3: Support IOMMU_DOMAIN_NESTED
> > >   iommu/arm-smmu-v3: Use S2FWB for NESTED domains
> > >   iommu/arm-smmu-v3: Allow ATS for IOMMU_DOMAIN_NESTED
> > >
> > > Nicolin Chen (5):
> > >   iommu/arm-smmu-v3: Support IOMMU_VIOMMU_ALLOC
> > >   iommu/arm-smmu-v3: Support IOMMU_HWPT_INVALIDATE using a VIOMMU object
> >
> > Applied to iommufd for-next along with all the dependencies and the
> > additional hunk Zhangfei pointed out.
> 
> Thanks Jason, I have verified on aarch64 based on your
> jason/smmuv3_nesting branch

Great, thanks

> https://github.com/Linaro/linux-kernel-uadk/tree/6.12-wip
> https://github.com/Linaro/qemu/tree/6.12-wip
> 
> Still need this hack
> https://github.com/Linaro/linux-kernel-uadk/commit/eaa194d954112cad4da7852e29343e546baf8683
>
> One is adding iommu_dev_enable/disable_feature IOMMU_DEV_FEAT_SVA,
> which you have patchset before.

Yes, I have a more complete version of that here someplace. Need some
help on vt-d but hope to get that done next cycle.

> The other is to temporarily ignore S2FWB or CANWBS.

Yes, ideally you should do that in the device FW, or perhaps via the
Linux ACPI patching?

Jason
Baolu Lu Nov. 13, 2024, 2:55 a.m. UTC | #10
On 11/13/24 09:23, Jason Gunthorpe wrote:
>> https://github.com/Linaro/linux-kernel-uadk/tree/6.12-wip
>> https://github.com/Linaro/qemu/tree/6.12-wip
>>
>> Still need this hack
>> https://github.com/Linaro/linux-kernel-uadk/commit/ 
>> eaa194d954112cad4da7852e29343e546baf8683
>>
>> One is adding iommu_dev_enable/disable_feature IOMMU_DEV_FEAT_SVA,
>> which you have patchset before.
> Yes, I have a more complete version of that here someplace. Need some
> help on vt-d but hope to get that done next cycle.

Can you please elaborate this a bit more? Are you talking about below
change

+	ret = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_SVA);
+	if (ret)
+		return ret;

in iommufd_fault_iopf_enable()?

I have no idea about why SVA is affected when enabling iopf.

--
baolu
Zhangfei Gao Nov. 13, 2024, 6:28 a.m. UTC | #11
Hi, Baolu

On Wed, 13 Nov 2024 at 10:56, Baolu Lu <baolu.lu@linux.intel.com> wrote:
>
> On 11/13/24 09:23, Jason Gunthorpe wrote:
> >> https://github.com/Linaro/linux-kernel-uadk/tree/6.12-wip
> >> https://github.com/Linaro/qemu/tree/6.12-wip
> >>
> >> Still need this hack
> >> https://github.com/Linaro/linux-kernel-uadk/commit/
> >> eaa194d954112cad4da7852e29343e546baf8683
> >>
> >> One is adding iommu_dev_enable/disable_feature IOMMU_DEV_FEAT_SVA,
> >> which you have patchset before.
> > Yes, I have a more complete version of that here someplace. Need some
> > help on vt-d but hope to get that done next cycle.
>
> Can you please elaborate this a bit more? Are you talking about below
> change
>
> +       ret = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_SVA);
> +       if (ret)
> +               return ret;
>
> in iommufd_fault_iopf_enable()?
>
> I have no idea about why SVA is affected when enabling iopf.

In drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c,
iommu_dev_enable_feature(IOMMU_DEV_FEAT_SVA) will real call
iopf_queue_add_device,
while iommu_dev_enable_feature(IOPF)  only set flag.

arm_smmu_dev_enable_feature
case IOMMU_DEV_FEAT_SVA:
arm_smmu_master_enable_sva(master)
iopf_queue_add_device(master->smmu->evtq.iopf, dev);

Thanks
Jason Gunthorpe Nov. 13, 2024, 4:43 p.m. UTC | #12
On Wed, Nov 13, 2024 at 10:55:41AM +0800, Baolu Lu wrote:
> On 11/13/24 09:23, Jason Gunthorpe wrote:
> > > https://github.com/Linaro/linux-kernel-uadk/tree/6.12-wip
> > > https://github.com/Linaro/qemu/tree/6.12-wip
> > > 
> > > Still need this hack
> > > https://github.com/Linaro/linux-kernel-uadk/commit/
> > > eaa194d954112cad4da7852e29343e546baf8683
> > > 
> > > One is adding iommu_dev_enable/disable_feature IOMMU_DEV_FEAT_SVA,
> > > which you have patchset before.
> > Yes, I have a more complete version of that here someplace. Need some
> > help on vt-d but hope to get that done next cycle.
> 
> Can you please elaborate this a bit more? Are you talking about below
> change

I need your help to remove IOMMU_DEV_FEAT_IOPF from the intel
driver. I have a patch series that eliminates it from all the other
drivers, and I wrote a patch to remove FEAT_SVA from intel..

> +	ret = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_SVA);
> +	if (ret)
> +		return ret;
> 
> in iommufd_fault_iopf_enable()?
> 
> I have no idea about why SVA is affected when enabling iopf.

It is ARM not implementing the API correctly. Only SVA turns on the
page fault reporting mechanism.

In the new world the page fault reporting should be managed during
domain attachment. If the domain is fault capable then faults should
be delivered to that domain. That is the correct time to setup the
iopf mechanism as well.

So I fixed that and now ARM and AMD both have no-op implementations of
IOMMU_DEV_FEAT_IOPF and IOMMU_DEV_FEAT_SVA. Thus I'd like to remove it
entirely.

Jason
Baolu Lu Nov. 14, 2024, 12:51 a.m. UTC | #13
On 11/14/24 00:43, Jason Gunthorpe wrote:
> On Wed, Nov 13, 2024 at 10:55:41AM +0800, Baolu Lu wrote:
>> On 11/13/24 09:23, Jason Gunthorpe wrote:
>>>> https://github.com/Linaro/linux-kernel-uadk/tree/6.12-wip
>>>> https://github.com/Linaro/qemu/tree/6.12-wip
>>>>
>>>> Still need this hack
>>>> https://github.com/Linaro/linux-kernel-uadk/commit/
>>>> eaa194d954112cad4da7852e29343e546baf8683
>>>>
>>>> One is adding iommu_dev_enable/disable_feature IOMMU_DEV_FEAT_SVA,
>>>> which you have patchset before.
>>> Yes, I have a more complete version of that here someplace. Need some
>>> help on vt-d but hope to get that done next cycle.
>>
>> Can you please elaborate this a bit more? Are you talking about below
>> change
> 
> I need your help to remove IOMMU_DEV_FEAT_IOPF from the intel
> driver. I have a patch series that eliminates it from all the other
> drivers, and I wrote a patch to remove FEAT_SVA from intel..

Yes, sure. Let's make this happen in the next cycle.

FEAT_IOPF could be removed. IOPF manipulation can be handled in the
domain attachment path. A per-device refcount can be implemented. This
count increments with each iopf-capable domain attachment and decrements
with each detachment. PCI PRI is enabled for the first iopf-capable
domain and disabled when the last one is removed. Probably we can also
solve the PF/VF sharing PRI issue.

With iopf moved to the domain attach path and hardware capability checks
to the SVA domain allocation path, FEAT_SVA becomes essentially a no-op.

> 
>> +	ret = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_SVA);
>> +	if (ret)
>> +		return ret;
>>
>> in iommufd_fault_iopf_enable()?
>>
>> I have no idea about why SVA is affected when enabling iopf.
> 
> It is ARM not implementing the API correctly. Only SVA turns on the
> page fault reporting mechanism.
> 
> In the new world the page fault reporting should be managed during
> domain attachment. If the domain is fault capable then faults should
> be delivered to that domain. That is the correct time to setup the
> iopf mechanism as well.
> 
> So I fixed that and now ARM and AMD both have no-op implementations of
> IOMMU_DEV_FEAT_IOPF and IOMMU_DEV_FEAT_SVA. Thus I'd like to remove it
> entirely.

Thank you for the explanation.

--
baolu
Jason Gunthorpe Nov. 15, 2024, 5:55 p.m. UTC | #14
On Thu, Nov 14, 2024 at 08:51:47AM +0800, Baolu Lu wrote:
> On 11/14/24 00:43, Jason Gunthorpe wrote:
> > On Wed, Nov 13, 2024 at 10:55:41AM +0800, Baolu Lu wrote:
> > > On 11/13/24 09:23, Jason Gunthorpe wrote:
> > > > > https://github.com/Linaro/linux-kernel-uadk/tree/6.12-wip
> > > > > https://github.com/Linaro/qemu/tree/6.12-wip
> > > > > 
> > > > > Still need this hack
> > > > > https://github.com/Linaro/linux-kernel-uadk/commit/
> > > > > eaa194d954112cad4da7852e29343e546baf8683
> > > > > 
> > > > > One is adding iommu_dev_enable/disable_feature IOMMU_DEV_FEAT_SVA,
> > > > > which you have patchset before.
> > > > Yes, I have a more complete version of that here someplace. Need some
> > > > help on vt-d but hope to get that done next cycle.
> > > 
> > > Can you please elaborate this a bit more? Are you talking about below
> > > change
> > 
> > I need your help to remove IOMMU_DEV_FEAT_IOPF from the intel
> > driver. I have a patch series that eliminates it from all the other
> > drivers, and I wrote a patch to remove FEAT_SVA from intel..
> 
> Yes, sure. Let's make this happen in the next cycle.
>
> FEAT_IOPF could be removed. IOPF manipulation can be handled in the
> domain attachment path. A per-device refcount can be implemented. This
> count increments with each iopf-capable domain attachment and decrements
> with each detachment. PCI PRI is enabled for the first iopf-capable
> domain and disabled when the last one is removed. Probably we can also
> solve the PF/VF sharing PRI issue.

Here is what I have so far, if you send me a patch for vt-d to move
FEAT_IOPF into attach as you describe above (see what I did to arm for
example), then I can send it next cycle

https://github.com/jgunthorpe/linux/commits/iommu_no_feat/

Thanks,
Jason