linux-kernel - Re: [PATCH v8 01/11] iommu: Add max_pasids field in struct iommu

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: <20220609172542.GB33363@araj-dh-work>
Date:   Thu, 9 Jun 2022 17:25:42 +0000
From:   "Raj, Ashok" <ashok.raj@...el.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>
Cc:     Joerg Roedel <joro@...tes.org>, Jason Gunthorpe <jgg@...dia.com>,
        Christoph Hellwig <hch@...radead.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Jean-Philippe Brucker <jean-philippe@...aro.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Vinod Koul <vkoul@...nel.org>,
        Eric Auger <eric.auger@...hat.com>,
        Liu Yi L <yi.l.liu@...el.com>,
        Jacob jun Pan <jacob.jun.pan@...el.com>,
        iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Ashok Raj <ashok.raj@...el.com>
Subject: Re: [PATCH v8 01/11] iommu: Add max_pasids field in struct
 iommu_device


On Tue, Jun 07, 2022 at 09:49:32AM +0800, Lu Baolu wrote:
> Use this field to keep the number of supported PASIDs that an IOMMU
> hardware is able to support. This is a generic attribute of an IOMMU
> and lifting it into the per-IOMMU device structure makes it possible

There is also a per-device attribute that tells what width is supported on
each device. When a device enables SVM, for simplicity we were proposing to
disable SVM on devices that don't support the full width, since it adds
additional complexity on the allocation interface. Is that factored into
this?

> to allocate a PASID for device without calls into the IOMMU drivers.
> Any iommu driver which supports PASID related features should set this
> field before enabling them on the devices.
> 
> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
> Reviewed-by: Jean-Philippe Brucker <jean-philippe@...aro.org>
> ---
>  include/linux/intel-iommu.h                 | 3 ++-
>  include/linux/iommu.h                       | 2 ++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
>  drivers/iommu/intel/dmar.c                  | 7 +++++++
>  4 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index 4f29139bbfc3..e065cbe3c857 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -479,7 +479,6 @@ enum {
>  #define VTD_FLAG_IRQ_REMAP_PRE_ENABLED	(1 << 1)
>  #define VTD_FLAG_SVM_CAPABLE		(1 << 2)
>  
> -extern int intel_iommu_sm;
>  extern spinlock_t device_domain_lock;
>  
>  #define sm_supported(iommu)	(intel_iommu_sm && ecap_smts((iommu)->ecap))
> @@ -786,6 +785,7 @@ struct context_entry *iommu_context_addr(struct intel_iommu *iommu, u8 bus,
>  extern const struct iommu_ops intel_iommu_ops;
>  
>  #ifdef CONFIG_INTEL_IOMMU
> +extern int intel_iommu_sm;
>  extern int iommu_calculate_agaw(struct intel_iommu *iommu);
>  extern int iommu_calculate_max_sagaw(struct intel_iommu *iommu);
>  extern int dmar_disabled;
> @@ -802,6 +802,7 @@ static inline int iommu_calculate_max_sagaw(struct intel_iommu *iommu)
>  }
>  #define dmar_disabled	(1)
>  #define intel_iommu_enabled (0)
> +#define intel_iommu_sm (0)

Is the above part of this patch? Or should be moved up somewhere?
>  #endif
>  
>  static inline const char *decode_prq_descriptor(char *str, size_t size,
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 5e1afe169549..03fbb1b71536 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -318,12 +318,14 @@ struct iommu_domain_ops {
>   * @list: Used by the iommu-core to keep a list of registered iommus
>   * @ops: iommu-ops for talking to this iommu
>   * @dev: struct device for sysfs handling
> + * @max_pasids: number of supported PASIDs
>   */
>  struct iommu_device {
>  	struct list_head list;
>  	const struct iommu_ops *ops;
>  	struct fwnode_handle *fwnode;
>  	struct device *dev;
> +	u32 max_pasids;
>  };
>  
>  /**
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 88817a3376ef..ae8ec8df47c1 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -3546,6 +3546,7 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
>  	/* SID/SSID sizes */
>  	smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg);
>  	smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, reg);
> +	smmu->iommu.max_pasids = 1UL << smmu->ssid_bits;
>  
>  	/*
>  	 * If the SMMU supports fewer bits than would fill a single L2 stream
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 592c1e1a5d4b..6c338888061a 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1123,6 +1123,13 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
>  
>  	raw_spin_lock_init(&iommu->register_lock);
>  
> +	/*
> +	 * A value of N in PSS field of eCap register indicates hardware
> +	 * supports PASID field of N+1 bits.
> +	 */
> +	if (pasid_supported(iommu))
> +		iommu->iommu.max_pasids = 2UL << ecap_pss(iommu->ecap);
> +
>  	/*
>  	 * This is only for hotplug; at boot time intel_iommu_enabled won't
>  	 * be set yet. When intel_iommu_init() runs, it registers the units
> -- 
> 2.25.1
>