| Message ID | 20220701214455.14992-1-nicolinc@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | cover-letter: Simplify vfio_iommu_type1 attach/detach routine | expand |
On Fri, 1 Jul 2022 14:44:50 -0700 Nicolin Chen <nicolinc@nvidia.com> wrote: > This is a preparatory series for IOMMUFD v2 patches. It enforces error > code -EMEDIUMTYPE in iommu_attach_device() and iommu_attach_group() when > an IOMMU domain and a device/group are incompatible. It also drops the > useless domain->ops check since it won't fail in current environment. > > These allow VFIO iommu code to simplify its group attachment routine, by > avoiding the extra IOMMU domain allocations and attach/detach sequences > of the old code. > > Worths mentioning the exact match for enforce_cache_coherency is removed > with this series, since there's very less value in doing that as KVM will > not be able to take advantage of it -- this just wastes domain memory. > Instead, we rely on Intel IOMMU driver taking care of that internally. > > This is on github: > https://github.com/nicolinc/iommufd/commits/vfio_iommu_attach How do you foresee this going in, I'm imagining Joerg would merge the first patch via the IOMMU tree and provide a topic branch that I'd merge into the vfio tree along with the remaining patches. Sound right? Thanks, Alex > Changelog > v5: > * Rebased on top of Robin's "Simplify bus_type determination". > * Fixed a wrong change returning -EMEDIUMTYPE in arm-smmu driver. > * Added Baolu's "Reviewed-by". > v4: > * Dropped -EMEDIUMTYPE change in mtk_v1 driver per Robin's input > * Added Baolu's and Kevin's Reviewed-by lines > v3: https://lore.kernel.org/kvm/20220623200029.26007-1-nicolinc@nvidia.com/ > * Dropped all dev_err since -EMEDIUMTYPE clearly indicates what error. > * Updated commit message of enforce_cache_coherency removing patch. > * Updated commit message of domain->ops removing patch. > * Replaced "goto out_unlock" with simply mutex_unlock() and return. > * Added a line of comments for -EMEDIUMTYPE return check. > * Moved iommu_get_msi_cookie() into alloc_attach_domain() as a cookie > should be logically tied to the lifetime of a domain itself. > * Added Kevin's "Reviewed-by". > v2: https://lore.kernel.org/kvm/20220616000304.23890-1-nicolinc@nvidia.com/ > * Added -EMEDIUMTYPE to more IOMMU drivers that fit the category. > * Changed dev_err to dev_dbg for -EMEDIUMTYPE to avoid kernel log spam. > * Dropped iommu_ops patch, and removed domain->ops in VFIO directly, > since there's no mixed-driver use case that would fail the sanity. > * Updated commit log of the patch removing enforce_cache_coherency. > * Fixed a misplace of "num_non_pinned_groups--" in detach_group patch. > * Moved "num_non_pinned_groups++" in PATCH-5 to the common path between > domain-reusing and new-domain pathways, like the code previously did. > * Fixed a typo in EMEDIUMTYPE patch. > v1: https://lore.kernel.org/kvm/20220606061927.26049-1-nicolinc@nvidia.com/ > > Jason Gunthorpe (1): > vfio/iommu_type1: Prefer to reuse domains vs match enforced cache > coherency > > Nicolin Chen (4): > iommu: Return -EMEDIUMTYPE for incompatible domain and device/group > vfio/iommu_type1: Remove the domain->ops comparison > vfio/iommu_type1: Clean up update_dirty_scope in detach_group() > vfio/iommu_type1: Simplify group attachment > > drivers/iommu/amd/iommu.c | 2 +- > drivers/iommu/apple-dart.c | 4 +- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 9 +- > drivers/iommu/intel/iommu.c | 10 +- > drivers/iommu/iommu.c | 28 ++ > drivers/iommu/ipmmu-vmsa.c | 4 +- > drivers/iommu/omap-iommu.c | 3 +- > drivers/iommu/s390-iommu.c | 2 +- > drivers/iommu/sprd-iommu.c | 6 +- > drivers/iommu/tegra-gart.c | 2 +- > drivers/iommu/virtio-iommu.c | 3 +- > drivers/vfio/vfio_iommu_type1.c | 352 ++++++++++---------- > 14 files changed, 229 insertions(+), 216 deletions(-) >
On Wed, Jul 06, 2022 at 11:42:17AM -0600, Alex Williamson wrote: > On Fri, 1 Jul 2022 14:44:50 -0700 > Nicolin Chen <nicolinc@nvidia.com> wrote: > > > This is a preparatory series for IOMMUFD v2 patches. It enforces error > > code -EMEDIUMTYPE in iommu_attach_device() and iommu_attach_group() when > > an IOMMU domain and a device/group are incompatible. It also drops the > > useless domain->ops check since it won't fail in current environment. > > > > These allow VFIO iommu code to simplify its group attachment routine, by > > avoiding the extra IOMMU domain allocations and attach/detach sequences > > of the old code. > > > > Worths mentioning the exact match for enforce_cache_coherency is removed > > with this series, since there's very less value in doing that as KVM will > > not be able to take advantage of it -- this just wastes domain memory. > > Instead, we rely on Intel IOMMU driver taking care of that internally. > > > > This is on github: > > https://github.com/nicolinc/iommufd/commits/vfio_iommu_attach > > How do you foresee this going in, I'm imagining Joerg would merge the > first patch via the IOMMU tree and provide a topic branch that I'd > merge into the vfio tree along with the remaining patches. Sound > right? Thanks, We don't have any build dependency between the IOMMU change and VFIO changes, yet, without the IOMMU one, any iommu_attach_group() failure now would be a hard failure without a chance falling back to a new_domain, which is slightly different from the current flow. For a potential existing use case that relies on reusing existing domain, I think it'd be safer to have Joerg acking the first change so you merge them all? Thank!
On Wed, 6 Jul 2022 10:53:52 -0700 Nicolin Chen <nicolinc@nvidia.com> wrote: > On Wed, Jul 06, 2022 at 11:42:17AM -0600, Alex Williamson wrote: > > > On Fri, 1 Jul 2022 14:44:50 -0700 > > Nicolin Chen <nicolinc@nvidia.com> wrote: > > > > > This is a preparatory series for IOMMUFD v2 patches. It enforces error > > > code -EMEDIUMTYPE in iommu_attach_device() and iommu_attach_group() when > > > an IOMMU domain and a device/group are incompatible. It also drops the > > > useless domain->ops check since it won't fail in current environment. > > > > > > These allow VFIO iommu code to simplify its group attachment routine, by > > > avoiding the extra IOMMU domain allocations and attach/detach sequences > > > of the old code. > > > > > > Worths mentioning the exact match for enforce_cache_coherency is removed > > > with this series, since there's very less value in doing that as KVM will > > > not be able to take advantage of it -- this just wastes domain memory. > > > Instead, we rely on Intel IOMMU driver taking care of that internally. > > > > > > This is on github: > > > https://github.com/nicolinc/iommufd/commits/vfio_iommu_attach > > > > How do you foresee this going in, I'm imagining Joerg would merge the > > first patch via the IOMMU tree and provide a topic branch that I'd > > merge into the vfio tree along with the remaining patches. Sound > > right? Thanks, > > We don't have any build dependency between the IOMMU change and > VFIO changes, yet, without the IOMMU one, any iommu_attach_group() > failure now would be a hard failure without a chance falling back > to a new_domain, which is slightly different from the current flow. > > For a potential existing use case that relies on reusing existing > domain, I think it'd be safer to have Joerg acking the first change > so you merge them all? Thank! Works for me, I'll look for buy-in + ack from Joerg. Thanks, Alex
On Wed, Jul 06, 2022 at 12:03:25PM -0600, Alex Williamson wrote: > On Wed, 6 Jul 2022 10:53:52 -0700 > Nicolin Chen <nicolinc@nvidia.com> wrote: > > > On Wed, Jul 06, 2022 at 11:42:17AM -0600, Alex Williamson wrote: > > > > > On Fri, 1 Jul 2022 14:44:50 -0700 > > > Nicolin Chen <nicolinc@nvidia.com> wrote: > > > > > > > This is a preparatory series for IOMMUFD v2 patches. It enforces error > > > > code -EMEDIUMTYPE in iommu_attach_device() and iommu_attach_group() when > > > > an IOMMU domain and a device/group are incompatible. It also drops the > > > > useless domain->ops check since it won't fail in current environment. > > > > > > > > These allow VFIO iommu code to simplify its group attachment routine, by > > > > avoiding the extra IOMMU domain allocations and attach/detach sequences > > > > of the old code. > > > > > > > > Worths mentioning the exact match for enforce_cache_coherency is removed > > > > with this series, since there's very less value in doing that as KVM will > > > > not be able to take advantage of it -- this just wastes domain memory. > > > > Instead, we rely on Intel IOMMU driver taking care of that internally. > > > > > > > > This is on github: > > > > https://github.com/nicolinc/iommufd/commits/vfio_iommu_attach > > > > > > How do you foresee this going in, I'm imagining Joerg would merge the > > > first patch via the IOMMU tree and provide a topic branch that I'd > > > merge into the vfio tree along with the remaining patches. Sound > > > right? Thanks, > > > > We don't have any build dependency between the IOMMU change and > > VFIO changes, yet, without the IOMMU one, any iommu_attach_group() > > failure now would be a hard failure without a chance falling back > > to a new_domain, which is slightly different from the current flow. > > > > For a potential existing use case that relies on reusing existing > > domain, I think it'd be safer to have Joerg acking the first change > > so you merge them all? Thank! > > Works for me, I'll look for buy-in + ack from Joerg. Thanks, > > Alex Joerg, would it be possible for you to ack at the IOMMU patch? Thanks! Nic
On Wed, Jul 13, 2022 at 04:57:36PM -0700, Nicolin Chen wrote: > > > > > This is on github: > > > > > https://github.com/nicolinc/iommufd/commits/vfio_iommu_attach > > > > > > > > How do you foresee this going in, I'm imagining Joerg would merge the > > > > first patch via the IOMMU tree and provide a topic branch that I'd > > > > merge into the vfio tree along with the remaining patches. Sound > > > > right? Thanks, > > > > > > We don't have any build dependency between the IOMMU change and > > > VFIO changes, yet, without the IOMMU one, any iommu_attach_group() > > > failure now would be a hard failure without a chance falling back > > > to a new_domain, which is slightly different from the current flow. > > > > > > For a potential existing use case that relies on reusing existing > > > domain, I think it'd be safer to have Joerg acking the first change > > > so you merge them all? Thank! > > > > Works for me, I'll look for buy-in + ack from Joerg. Thanks, > > > > Alex > > Joerg, would it be possible for you to ack at the IOMMU patch? Joerg, sorry for pinning again. Would it be possible for you to give an ack at the IOMMU patch so that this series might catch the last train of this cycle? Thanks!
This is a preparatory series for IOMMUFD v2 patches. It enforces error code -EMEDIUMTYPE in iommu_attach_device() and iommu_attach_group() when an IOMMU domain and a device/group are incompatible. It also drops the useless domain->ops check since it won't fail in current environment. These allow VFIO iommu code to simplify its group attachment routine, by avoiding the extra IOMMU domain allocations and attach/detach sequences of the old code. Worths mentioning the exact match for enforce_cache_coherency is removed with this series, since there's very less value in doing that as KVM will not be able to take advantage of it -- this just wastes domain memory. Instead, we rely on Intel IOMMU driver taking care of that internally. This is on github: https://github.com/nicolinc/iommufd/commits/vfio_iommu_attach Changelog v5: * Rebased on top of Robin's "Simplify bus_type determination". * Fixed a wrong change returning -EMEDIUMTYPE in arm-smmu driver. * Added Baolu's "Reviewed-by". v4: * Dropped -EMEDIUMTYPE change in mtk_v1 driver per Robin's input * Added Baolu's and Kevin's Reviewed-by lines v3: https://lore.kernel.org/kvm/20220623200029.26007-1-nicolinc@nvidia.com/ * Dropped all dev_err since -EMEDIUMTYPE clearly indicates what error. * Updated commit message of enforce_cache_coherency removing patch. * Updated commit message of domain->ops removing patch. * Replaced "goto out_unlock" with simply mutex_unlock() and return. * Added a line of comments for -EMEDIUMTYPE return check. * Moved iommu_get_msi_cookie() into alloc_attach_domain() as a cookie should be logically tied to the lifetime of a domain itself. * Added Kevin's "Reviewed-by". v2: https://lore.kernel.org/kvm/20220616000304.23890-1-nicolinc@nvidia.com/ * Added -EMEDIUMTYPE to more IOMMU drivers that fit the category. * Changed dev_err to dev_dbg for -EMEDIUMTYPE to avoid kernel log spam. * Dropped iommu_ops patch, and removed domain->ops in VFIO directly, since there's no mixed-driver use case that would fail the sanity. * Updated commit log of the patch removing enforce_cache_coherency. * Fixed a misplace of "num_non_pinned_groups--" in detach_group patch. * Moved "num_non_pinned_groups++" in PATCH-5 to the common path between domain-reusing and new-domain pathways, like the code previously did. * Fixed a typo in EMEDIUMTYPE patch. v1: https://lore.kernel.org/kvm/20220606061927.26049-1-nicolinc@nvidia.com/ Jason Gunthorpe (1): vfio/iommu_type1: Prefer to reuse domains vs match enforced cache coherency Nicolin Chen (4): iommu: Return -EMEDIUMTYPE for incompatible domain and device/group vfio/iommu_type1: Remove the domain->ops comparison vfio/iommu_type1: Clean up update_dirty_scope in detach_group() vfio/iommu_type1: Simplify group attachment drivers/iommu/amd/iommu.c | 2 +- drivers/iommu/apple-dart.c | 4 +- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +- drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 9 +- drivers/iommu/intel/iommu.c | 10 +- drivers/iommu/iommu.c | 28 ++ drivers/iommu/ipmmu-vmsa.c | 4 +- drivers/iommu/omap-iommu.c | 3 +- drivers/iommu/s390-iommu.c | 2 +- drivers/iommu/sprd-iommu.c | 6 +- drivers/iommu/tegra-gart.c | 2 +- drivers/iommu/virtio-iommu.c | 3 +- drivers/vfio/vfio_iommu_type1.c | 352 ++++++++++---------- 14 files changed, 229 insertions(+), 216 deletions(-)