Message ID | 0-v4-9e99b76f3518+3a8-smmuv3_nesting_jgg@nvidia.com (mailing list archive) |
---|---|
Headers | show |
Series | Initial support for SMMUv3 nested translation | expand |
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
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
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
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
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
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
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
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
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
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
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
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
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
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
On Fri, Nov 15, 2024 at 01:55:22PM -0400, Jason Gunthorpe wrote: > > > 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/ Hey Baolu, a reminder on this, lets try for it next cycle? Thanks, Jason
On 2025/1/23 3:26, Jason Gunthorpe wrote: > On Fri, Nov 15, 2024 at 01:55:22PM -0400, Jason Gunthorpe wrote: >>>> 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/ > Hey Baolu, a reminder on this, lets try for it next cycle? Oh, I forgot this. Thanks for the reminding. Sure, let's try to make it in the next cycle. --- baolu
On 2025/2/5 11:45, Baolu Lu wrote: > On 2025/1/23 3:26, Jason Gunthorpe wrote: >> On Fri, Nov 15, 2024 at 01:55:22PM -0400, Jason Gunthorpe wrote: >>>>> 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/ >> Hey Baolu, a reminder on this, lets try for it next cycle? > > Oh, I forgot this. Thanks for the reminding. Sure, let's try to make it > in the next cycle. Hi Jason, I've worked through the entire series. The patches are available here: https://github.com/LuBaolu/intel-iommu/commits/iommu-no-feat-v6.14-rc2 Please check if this is the right direction. Thanks, baolu
On Thu, Feb 13, 2025 at 07:57:51PM +0800, Baolu Lu wrote: > On 2025/2/5 11:45, Baolu Lu wrote: > > On 2025/1/23 3:26, Jason Gunthorpe wrote: > > > On Fri, Nov 15, 2024 at 01:55:22PM -0400, Jason Gunthorpe wrote: > > > > > > 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/ > > > Hey Baolu, a reminder on this, lets try for it next cycle? > > > > Oh, I forgot this. Thanks for the reminding. Sure, let's try to make it > > in the next cycle. > > Hi Jason, > > I've worked through the entire series. The patches are available here: > > https://github.com/LuBaolu/intel-iommu/commits/iommu-no-feat-v6.14-rc2 > > Please check if this is the right direction. Looks great, and you did all the cleanup stuff too! The vt-d flow is a little more complicated than the ARM logic because the driver flow is structed differently. Do we really want ATS turned on if the only thing attached is an IDENTITY domain? That will unnecessarily slow down ATS capable HW.. It is functionally OK though. Also, are there enough ATC flushes around any domain type change? I didn't check.. I feel like we should leave "iommu: Move PRI enablement for VF to iommu driver" out for now, every driver needs this check? AMD supports SVA and PRI so it needs it too. Do you want to squash those fixup patches and post it? Thanks, Jason
On 2/14/25 02:43, Jason Gunthorpe wrote: > On Thu, Feb 13, 2025 at 07:57:51PM +0800, Baolu Lu wrote: >> On 2025/2/5 11:45, Baolu Lu wrote: >>> On 2025/1/23 3:26, Jason Gunthorpe wrote: >>>> On Fri, Nov 15, 2024 at 01:55:22PM -0400, Jason Gunthorpe wrote: >>>>>>> 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/ >>>> Hey Baolu, a reminder on this, lets try for it next cycle? >>> >>> Oh, I forgot this. Thanks for the reminding. Sure, let's try to make it >>> in the next cycle. >> >> Hi Jason, >> >> I've worked through the entire series. The patches are available here: >> >> https://github.com/LuBaolu/intel-iommu/commits/iommu-no-feat-v6.14-rc2 >> >> Please check if this is the right direction. > > Looks great, and you did all the cleanup stuff too! > > The vt-d flow is a little more complicated than the ARM logic because > the driver flow is structed differently. > > Do we really want ATS turned on if the only thing attached is an > IDENTITY domain? That will unnecessarily slow down ATS capable HW.. It > is functionally OK though. It depends. The Intel driver uses a simple approach. When the IOMMU is working in legacy mode, PASID and PRI are not supported. In this case, ATS will not be enabled if the identity domain is attached to the device. When the IOMMU is working in scalable mode, PASID and PRI are supported. ATS will always be enabled, even if the identity domain is attached to the device, because the PASID might use PRI, which depends on ATS functionality. This might not be the best choice, but it is the simplest and functional. If any device does not work with the identity domain and ATS enabled, then we can add a quirk to the driver, as we already have such a mechanism. > > Also, are there enough ATC flushes around any domain type change? I > didn't check.. The VT-d specification defines guidance for software to manage caches, both the IOTLB in the IOMMU and the ATC on the device (Table 28, Guidance to Software for Invalidations). The driver was written according to that guidance. > I feel like we should leave "iommu: Move PRI enablement for VF to > iommu driver" out for now, every driver needs this check? AMD > supports SVA and PRI so it needs it too. Yes, agreed. > > Do you want to squash those fixup patches and post it? Yes, sure. Let me post it for further review. > Thanks, > Jason Thanks, baolu
On Fri, Feb 14, 2025 at 01:39:52PM +0800, Baolu Lu wrote: > When the IOMMU is working in scalable mode, PASID and PRI are supported. > ATS will always be enabled, even if the identity domain is attached to > the device, because the PASID might use PRI, which depends on ATS > functionality. This might not be the best choice, but it is the > simplest and functional. The arm driver keeps track of things and enables ATS when PASIDs are present > If any device does not work with the identity domain and ATS enabled, > then we can add a quirk to the driver, as we already have such a > mechanism. The device should not care, as long as the HW works.. ARM has a weird quirk where one of the two ways to configure an identity domain does not work with ATS. If you have something like that as well then you have to switch ATS off if the IOMMU is in a state where it will not respond to it. Otherwise, the HW I'm aware of uses ATS pretty selectively and it may not make any real difference.. > > I feel like we should leave "iommu: Move PRI enablement for VF to > > iommu driver" out for now, every driver needs this check? AMD > > supports SVA and PRI so it needs it too. > > Yes, agreed. Although, I'm wondering now, that check should be on the SVA paths as well as the iommufd path.. > > Do you want to squash those fixup patches and post it? > > Yes, sure. Let me post it for further review. Thanks Jason
On 2/14/25 20:41, Jason Gunthorpe wrote: > On Fri, Feb 14, 2025 at 01:39:52PM +0800, Baolu Lu wrote: > >> When the IOMMU is working in scalable mode, PASID and PRI are supported. >> ATS will always be enabled, even if the identity domain is attached to >> the device, because the PASID might use PRI, which depends on ATS >> functionality. This might not be the best choice, but it is the >> simplest and functional. > The arm driver keeps track of things and enables ATS when PASIDs are > present I am not aware of any VT-d hardware implementation that supports scalable mode but not PASID. If there were one, it would be worthwhile to add an optimization to avoid enabling ATS during probe if PASID is not supported. > >> If any device does not work with the identity domain and ATS enabled, >> then we can add a quirk to the driver, as we already have such a >> mechanism. > The device should not care, as long as the HW works.. ARM has a weird > quirk where one of the two ways to configure an identity domain does > not work with ATS. If you have something like that as well then you > have to switch ATS off if the IOMMU is in a state where it will not > respond to it. > > Otherwise, the HW I'm aware of uses ATS pretty selectively and it may > not make any real difference.. > >>> I feel like we should leave "iommu: Move PRI enablement for VF to >>> iommu driver" out for now, every driver needs this check? AMD >>> supports SVA and PRI so it needs it too. >> Yes, agreed. > Although, I'm wondering now, that check should be on the SVA paths as > well as the iommufd path.. That appears to be a fix. Thanks, baolu