linux-kernel - Re: [PATCH v3 05/11] iommu: Pass in a viommu pointer to domain_alloc

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20241017165151.GH3559746@nvidia.com>
Date: Thu, 17 Oct 2024 13:51:51 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: kevin.tian@...el.com, will@...nel.org, joro@...tes.org,
	suravee.suthikulpanit@....com, robin.murphy@....com,
	dwmw2@...radead.org, baolu.lu@...ux.intel.com, shuah@...nel.org,
	linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	linux-kselftest@...r.kernel.org, eric.auger@...hat.com,
	jean-philippe@...aro.org, mdf@...nel.org, mshavit@...gle.com,
	shameerali.kolothum.thodi@...wei.com, smostafa@...gle.com,
	yi.l.liu@...el.com, aik@....com, patches@...ts.linux.dev
Subject: Re: [PATCH v3 05/11] iommu: Pass in a viommu pointer to
 domain_alloc_user op

On Wed, Oct 09, 2024 at 09:38:05AM -0700, Nicolin Chen wrote:
> With a viommu object wrapping a potentially shareable S2 domain, a nested
> domain should be allocated by associating to a viommu instead.
> 
> For drivers without a viommu support, keep the parent domain input, which
> should be just viommu->hwpt->common.domain otherwise.
> 
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
>  include/linux/iommu.h                       | 1 +
>  drivers/iommu/amd/iommu.c                   | 1 +
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
>  drivers/iommu/intel/iommu.c                 | 1 +
>  drivers/iommu/iommufd/hw_pagetable.c        | 5 +++--
>  drivers/iommu/iommufd/selftest.c            | 1 +
>  6 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 3a50f57b0861..9105478bdbcd 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -573,6 +573,7 @@ struct iommu_ops {
>  	struct iommu_domain *(*domain_alloc)(unsigned iommu_domain_type);
>  	struct iommu_domain *(*domain_alloc_user)(
>  		struct device *dev, u32 flags, struct iommu_domain *parent,
> +		struct iommufd_viommu *viommu,
>  		const struct iommu_user_data *user_data);

This re-enforces my feeling we should have made a
domain_alloc_nested()..

>  	struct iommu_domain *(*domain_alloc_paging)(struct device *dev);
>  	struct iommu_domain *(*domain_alloc_sva)(struct device *dev,
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 8364cd6fa47d..3100ddcaf62e 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2394,6 +2394,7 @@ static struct iommu_domain *amd_iommu_domain_alloc(unsigned int type)
>  static struct iommu_domain *
>  amd_iommu_domain_alloc_user(struct device *dev, u32 flags,
>  			    struct iommu_domain *parent,
> +			    struct iommufd_viommu *viommu,
>  			    const struct iommu_user_data *user_data)
>  
>  {

Do these all need to check for NULL viommu now too? Something is
needed at least, only valid viommus should be accepted here. Like you
can't pass an ARM viommu to AMD or something silly.

Should drivers accept the default viommu?

Jason