Steal two bits from the 32-bit "type" in struct iommu_domain to store a
new tag for private data owned by either dma-iommu or iommufd.
Set the domain->private_data_owner in dma-iommu and iommufd. These will
be used to replace the sw_msi function pointer.
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
include/linux/iommu.h | 7 ++++++-
drivers/iommu/dma-iommu.c | 2 ++
drivers/iommu/iommufd/hw_pagetable.c | 3 +++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e93d2e918599..4f2774c08262 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -209,8 +209,13 @@ struct iommu_domain_geometry {
#define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM)
#define IOMMU_DOMAIN_NESTED (__IOMMU_DOMAIN_NESTED)
+#define IOMMU_DOMAIN_DATA_OWNER_NONE (0U)
+#define IOMMU_DOMAIN_DATA_OWNER_DMA (1U)
+#define IOMMU_DOMAIN_DATA_OWNER_IOMMUFD (2U)
+
struct iommu_domain {
- unsigned type;
+ u32 type : 30;
+ u32 private_data_owner : 2;
const struct iommu_domain_ops *ops;
const struct iommu_dirty_ops *dirty_ops;
const struct iommu_ops *owner; /* Whose domain_alloc we came from */
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 94263ed2c564..78915d74e8fa 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -403,6 +403,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
mutex_init(&domain->iova_cookie->mutex);
iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
+ domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_DMA;
return 0;
}
@@ -435,6 +436,7 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
cookie->msi_iova = base;
domain->iova_cookie = cookie;
iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
+ domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_DMA;
return 0;
}
EXPORT_SYMBOL(iommu_get_msi_cookie);
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 7de6e914232e..5640444de475 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -157,6 +157,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
}
}
iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
+ hwpt->domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_IOMMUFD;
/*
* Set the coherency mode before we do iopt_table_add_domain() as some
@@ -253,6 +254,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
}
hwpt->domain->owner = ops;
iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
+ hwpt->domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_IOMMUFD;
if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
rc = -EINVAL;
@@ -310,6 +312,7 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
}
hwpt->domain->owner = viommu->iommu_dev->ops;
iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
+ hwpt->domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_IOMMUFD;
if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
rc = -EINVAL;
--
2.43.0On 28/02/2025 1:31 am, Nicolin Chen wrote:
> Steal two bits from the 32-bit "type" in struct iommu_domain to store a
> new tag for private data owned by either dma-iommu or iommufd.
>
> Set the domain->private_data_owner in dma-iommu and iommufd. These will
> be used to replace the sw_msi function pointer.
>
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
> include/linux/iommu.h | 7 ++++++-
> drivers/iommu/dma-iommu.c | 2 ++
> drivers/iommu/iommufd/hw_pagetable.c | 3 +++
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e93d2e918599..4f2774c08262 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -209,8 +209,13 @@ struct iommu_domain_geometry {
> #define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM)
> #define IOMMU_DOMAIN_NESTED (__IOMMU_DOMAIN_NESTED)
>
> +#define IOMMU_DOMAIN_DATA_OWNER_NONE (0U)
> +#define IOMMU_DOMAIN_DATA_OWNER_DMA (1U)
> +#define IOMMU_DOMAIN_DATA_OWNER_IOMMUFD (2U)
> +
> struct iommu_domain {
> - unsigned type;
> + u32 type : 30;
> + u32 private_data_owner : 2;
> const struct iommu_domain_ops *ops;
> const struct iommu_dirty_ops *dirty_ops;
> const struct iommu_ops *owner; /* Whose domain_alloc we came from */
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 94263ed2c564..78915d74e8fa 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -403,6 +403,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>
> mutex_init(&domain->iova_cookie->mutex);
> iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
> + domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_DMA;
> return 0;
> }
>
> @@ -435,6 +436,7 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
> cookie->msi_iova = base;
> domain->iova_cookie = cookie;
> iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi);
> + domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_DMA;
This doesn't help all that much when it can still be called on any old
unmanaged domain and silently overwrite whatever the previous user
thought they owned...
And really the "owner" of MSI cookies is VFIO, it just happens that all
the code for them still lives in iommu-dma because it was written to
piggyback off the same irqchip hooks. TBH I've long been keen on
separating them further from "real" IOVA cookies, and generalising said
hooks really removes any remaining reason to keep the implementations
tied together at all, should they have cause to diverge further (e.g.
with makign the MSI cookie window programmable). What I absolutely want
to avoid is a notion of having two types of MSI-mapping cookies, one of
which is two types of MSI-mapping cookies.
Hopefully that is all clear in the patch I proposed.
Thanks,
Robin.
> return 0;
> }
> EXPORT_SYMBOL(iommu_get_msi_cookie);
> diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
> index 7de6e914232e..5640444de475 100644
> --- a/drivers/iommu/iommufd/hw_pagetable.c
> +++ b/drivers/iommu/iommufd/hw_pagetable.c
> @@ -157,6 +157,7 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
> }
> }
> iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
> + hwpt->domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_IOMMUFD;
>
> /*
> * Set the coherency mode before we do iopt_table_add_domain() as some
> @@ -253,6 +254,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx,
> }
> hwpt->domain->owner = ops;
> iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
> + hwpt->domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_IOMMUFD;
>
> if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
> rc = -EINVAL;
> @@ -310,6 +312,7 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,
> }
> hwpt->domain->owner = viommu->iommu_dev->ops;
> iommu_domain_set_sw_msi(hwpt->domain, iommufd_sw_msi);
> + hwpt->domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_IOMMUFD;
>
> if (WARN_ON_ONCE(hwpt->domain->type != IOMMU_DOMAIN_NESTED)) {
> rc = -EINVAL;
On Fri, Feb 28, 2025 at 04:29:17PM +0000, Robin Murphy wrote: > On 28/02/2025 1:31 am, Nicolin Chen wrote: > > @@ -435,6 +436,7 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base) > > cookie->msi_iova = base; > > domain->iova_cookie = cookie; > > iommu_domain_set_sw_msi(domain, iommu_dma_sw_msi); > > + domain->private_data_owner = IOMMU_DOMAIN_DATA_OWNER_DMA; > > This doesn't help all that much when it can still be called on any old > unmanaged domain and silently overwrite whatever the previous user thought > they owned... We could add another validation on top of this function to reject private_data_owner != IOMMU_DOMAIN_DATA_OWNER_NONE? > And really the "owner" of MSI cookies is VFIO, it just happens that all the > code for them still lives in iommu-dma because it was written to piggyback > off the same irqchip hooks. TBH I've long been keen on separating them > further from "real" IOVA cookies, and generalising said hooks really removes > any remaining reason to keep the implementations tied together at all, > should they have cause to diverge further (e.g. with makign the MSI cookie > window programmable). What I absolutely want to avoid is a notion of having > two types of MSI-mapping cookies, one of which is two types of MSI-mapping > cookies. > > Hopefully that is all clear in the patch I proposed. Yea, decoupling the MSI cookie from the iova_cookie makes sense to me. I had the same feeling last week when I was preparing the cleanup patch that added a iommu_put_msi_cookie(), but hesitated after foreseeing a big rework that I was not sure you would like or not. Glad that you did it.. With your patch now we even merged the two unions, which is nicer from my point of view. Thanks Nicolin
On 2/28/25 09:31, Nicolin Chen wrote:
> Steal two bits from the 32-bit "type" in struct iommu_domain to store a
> new tag for private data owned by either dma-iommu or iommufd.
>
> Set the domain->private_data_owner in dma-iommu and iommufd. These will
> be used to replace the sw_msi function pointer.
>
> Suggested-by: Jason Gunthorpe<jgg@nvidia.com>
> Signed-off-by: Nicolin Chen<nicolinc@nvidia.com>
> ---
> include/linux/iommu.h | 7 ++++++-
> drivers/iommu/dma-iommu.c | 2 ++
> drivers/iommu/iommufd/hw_pagetable.c | 3 +++
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e93d2e918599..4f2774c08262 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -209,8 +209,13 @@ struct iommu_domain_geometry {
> #define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM)
> #define IOMMU_DOMAIN_NESTED (__IOMMU_DOMAIN_NESTED)
>
> +#define IOMMU_DOMAIN_DATA_OWNER_NONE (0U)
> +#define IOMMU_DOMAIN_DATA_OWNER_DMA (1U)
> +#define IOMMU_DOMAIN_DATA_OWNER_IOMMUFD (2U)
> +
> struct iommu_domain {
> - unsigned type;
> + u32 type : 30;
> + u32 private_data_owner : 2;
Is there any special consideration for reserving only 2 bits for the
private data owner? Is it possible to allocate more bits so that it
could be more extensible for the future?
For example, current iommu core allows a kernel device driver to
allocate a paging domain and replace the default domain for kernel DMA.
This suggests the private data owner bits may be needed beyond iommu-dma
and iommufd.
Thanks,
baolu
On Fri, Feb 28, 2025 at 11:13:23AM +0800, Baolu Lu wrote:
> On 2/28/25 09:31, Nicolin Chen wrote:
> > Steal two bits from the 32-bit "type" in struct iommu_domain to store a
> > new tag for private data owned by either dma-iommu or iommufd.
> >
> > Set the domain->private_data_owner in dma-iommu and iommufd. These will
> > be used to replace the sw_msi function pointer.
> >
> > Suggested-by: Jason Gunthorpe<jgg@nvidia.com>
> > Signed-off-by: Nicolin Chen<nicolinc@nvidia.com>
> > ---
> > include/linux/iommu.h | 7 ++++++-
> > drivers/iommu/dma-iommu.c | 2 ++
> > drivers/iommu/iommufd/hw_pagetable.c | 3 +++
> > 3 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index e93d2e918599..4f2774c08262 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -209,8 +209,13 @@ struct iommu_domain_geometry {
> > #define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM)
> > #define IOMMU_DOMAIN_NESTED (__IOMMU_DOMAIN_NESTED)
> > +#define IOMMU_DOMAIN_DATA_OWNER_NONE (0U)
> > +#define IOMMU_DOMAIN_DATA_OWNER_DMA (1U)
> > +#define IOMMU_DOMAIN_DATA_OWNER_IOMMUFD (2U)
> > +
> > struct iommu_domain {
> > - unsigned type;
> > + u32 type : 30;
> > + u32 private_data_owner : 2;
>
> Is there any special consideration for reserving only 2 bits for the
> private data owner? Is it possible to allocate more bits so that it
> could be more extensible for the future?
It could. This "2" is copied from Jason's suggestion:
https://lore.kernel.org/linux-iommu/20250227194749.GJ39591@nvidia.com/
> For example, current iommu core allows a kernel device driver to
> allocate a paging domain and replace the default domain for kernel DMA.
> This suggests the private data owner bits may be needed beyond iommu-dma
> and iommufd.
What's the potential "private data" in this case?
Thanks
Nicolin
On 2/28/25 11:23, Nicolin Chen wrote:
> On Fri, Feb 28, 2025 at 11:13:23AM +0800, Baolu Lu wrote:
>> On 2/28/25 09:31, Nicolin Chen wrote:
>>> Steal two bits from the 32-bit "type" in struct iommu_domain to store a
>>> new tag for private data owned by either dma-iommu or iommufd.
>>>
>>> Set the domain->private_data_owner in dma-iommu and iommufd. These will
>>> be used to replace the sw_msi function pointer.
>>>
>>> Suggested-by: Jason Gunthorpe<jgg@nvidia.com>
>>> Signed-off-by: Nicolin Chen<nicolinc@nvidia.com>
>>> ---
>>> include/linux/iommu.h | 7 ++++++-
>>> drivers/iommu/dma-iommu.c | 2 ++
>>> drivers/iommu/iommufd/hw_pagetable.c | 3 +++
>>> 3 files changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index e93d2e918599..4f2774c08262 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -209,8 +209,13 @@ struct iommu_domain_geometry {
>>> #define IOMMU_DOMAIN_PLATFORM (__IOMMU_DOMAIN_PLATFORM)
>>> #define IOMMU_DOMAIN_NESTED (__IOMMU_DOMAIN_NESTED)
>>> +#define IOMMU_DOMAIN_DATA_OWNER_NONE (0U)
>>> +#define IOMMU_DOMAIN_DATA_OWNER_DMA (1U)
>>> +#define IOMMU_DOMAIN_DATA_OWNER_IOMMUFD (2U)
>>> +
>>> struct iommu_domain {
>>> - unsigned type;
>>> + u32 type : 30;
>>> + u32 private_data_owner : 2;
>> Is there any special consideration for reserving only 2 bits for the
>> private data owner? Is it possible to allocate more bits so that it
>> could be more extensible for the future?
> It could. This "2" is copied from Jason's suggestion:
> https://lore.kernel.org/linux-iommu/20250227194749.GJ39591@nvidia.com/
>
>> For example, current iommu core allows a kernel device driver to
>> allocate a paging domain and replace the default domain for kernel DMA.
>> This suggests the private data owner bits may be needed beyond iommu-dma
>> and iommufd.
> What's the potential "private data" in this case?
I have no idea, just feeling that we could leave room for tomorrow.
© 2016 - 2026 Red Hat, Inc.