linux-kernel - RE: [PATCH v2 2/3] iommu/dma: Support MSIs through nested domains

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BN9PR11MB5276E59FBD67B1119B3E2A858CBF2@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Tue, 6 Aug 2024 08:25:33 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Nicolin Chen <nicolinc@...dia.com>, "jgg@...dia.com" <jgg@...dia.com>
CC: "robin.murphy@....com" <robin.murphy@....com>, "joro@...tes.org"
	<joro@...tes.org>, "will@...nel.org" <will@...nel.org>, "shuah@...nel.org"
	<shuah@...nel.org>, "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: RE: [PATCH v2 2/3] iommu/dma: Support MSIs through nested domains

> From: Nicolin Chen <nicolinc@...dia.com>
> Sent: Saturday, August 3, 2024 8:32 AM
> 
> From: Robin Murphy <robin.murphy@....com>
> 
> Currently, iommu-dma is the only place outside of IOMMUFD and drivers
> which might need to be aware of the stage 2 domain encapsulated within
> a nested domain. This would be in the legacy-VFIO-style case where we're

why is it a legacy-VFIO-style? We only support nested in IOMMUFD.

> using host-managed MSIs with an identity mapping at stage 1, where it is
> the underlying stage 2 domain which owns an MSI cookie and holds the
> corresponding dynamic mappings. Hook up the new op to resolve what we
> need from a nested domain.
> 
> Signed-off-by: Robin Murphy <robin.murphy@....com>
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
>  drivers/iommu/dma-iommu.c | 18 ++++++++++++++++--
>  include/linux/iommu.h     |  4 ++++
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 7b1dfa0665df6..05e04934a5f81 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1799,6 +1799,20 @@ static struct iommu_dma_msi_page
> *iommu_dma_get_msi_page(struct device *dev,
>  	return NULL;
>  }
> 
> +/*
> + * Nested domains may not have an MSI cookie or accept mappings, but
> they may
> + * be related to a domain which does, so we let them tell us what they need.
> + */
> +static struct iommu_domain
> *iommu_dma_get_msi_mapping_domain(struct device *dev)
> +{
> +	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> +
> +	if (domain && domain->type == IOMMU_DOMAIN_NESTED &&
> +	    domain->ops->get_msi_mapping_domain)

I'm not sure the core should restrict it to the NESTED type. Given
there is a new domain ops any type restriction can be handled
inside the callback. Anyway the driver should register the op
for a domain only when there is a need. 

> +		domain = domain->ops->get_msi_mapping_domain(domain);
> +	return domain;
> +}
> +
>  /**
>   * iommu_dma_prepare_msi() - Map the MSI page in the IOMMU domain
>   * @desc: MSI descriptor, will store the MSI page
> @@ -1809,7 +1823,7 @@ static struct iommu_dma_msi_page
> *iommu_dma_get_msi_page(struct device *dev,
>  int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr)
>  {
>  	struct device *dev = msi_desc_to_dev(desc);
> -	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> +	struct iommu_domain *domain =
> iommu_dma_get_msi_mapping_domain(dev);
>  	struct iommu_dma_msi_page *msi_page;
>  	static DEFINE_MUTEX(msi_prepare_lock); /* see below */
> 
> @@ -1842,7 +1856,7 @@ int iommu_dma_prepare_msi(struct msi_desc
> *desc, phys_addr_t msi_addr)
>  void iommu_dma_compose_msi_msg(struct msi_desc *desc, struct msi_msg
> *msg)
>  {
>  	struct device *dev = msi_desc_to_dev(desc);
> -	const struct iommu_domain *domain =
> iommu_get_domain_for_dev(dev);
> +	const struct iommu_domain *domain =
> iommu_dma_get_msi_mapping_domain(dev);
>  	const struct iommu_dma_msi_page *msi_page;
> 
>  	msi_page = msi_desc_get_iommu_cookie(desc);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 4d47f2c333118..69ed76f9c3ec4 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -638,6 +638,8 @@ struct iommu_ops {
>   * @enable_nesting: Enable nesting
>   * @set_pgtable_quirks: Set io page table quirks (IO_PGTABLE_QUIRK_*)
>   * @free: Release the domain after use.
> + * @get_msi_mapping_domain: Return the related iommu_domain that
> should hold the
> + *                          MSI cookie and accept mapping(s).
>   */
>  struct iommu_domain_ops {
>  	int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
> @@ -668,6 +670,8 @@ struct iommu_domain_ops {
>  				  unsigned long quirks);
> 
>  	void (*free)(struct iommu_domain *domain);
> +	struct iommu_domain *
> +		(*get_msi_mapping_domain)(struct iommu_domain
> *domain);
>  };
> 
>  /**
> --
> 2.43.0
>