RE: [PATCH v8 01/11] iommu: Add max_pasids field in struct iommu_device
> From: Jason Gunthorpe > Sent: Friday, June 10, 2022 7:53 AM > > On Thu, Jun 09, 2022 at 05:25:42PM +, Raj, Ashok wrote: > > > > 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? > > I would like to see the concept of a 'global PASID' and this is the > only place we'd union all the per-device limits together. SVM can > optionally use a global PASID and ENQCMD requires it, but I don't want > to see the core code assuming everything is ENQCMD. > Agree. and I think this is what this v8 is leaning toward. The core code simply populates the pasid entry of the target device w/o assuming the pasid is 'local' or 'global'. Then sva helpers actually decides how the pasid is allocated. Currently only global pasids are supported which is how sva works before. We don't plan to change it in this series. In parallel Jacob is working on per-device local pasids which will then be used by his DMA API pasid work and also iommufd. Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 01/11] iommu: Add max_pasids field in struct iommu_device
On 2022/6/10 01:25, Raj, Ashok wrote:
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?
This is to make pasid_supported() usable in dmar.c. It's only needed by
the change in this patch. I should make this clear in the commit
message. :-)
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 01/11] iommu: Add max_pasids field in struct iommu_device
On Thu, Jun 09, 2022 at 05:25:42PM +, Raj, Ashok wrote: > > 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? I would like to see the concept of a 'global PASID' and this is the only place we'd union all the per-device limits together. SVM can optionally use a global PASID and ENQCMD requires it, but I don't want to see the core code assuming everything is ENQCMD. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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
> Reviewed-by: Jean-Philippe Brucker
> ---
> 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..6c33061a 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
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v8 01/11] iommu: Add max_pasids field in struct iommu_device
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
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
Reviewed-by: Jean-Philippe Brucker
---
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)
#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..6c33061a 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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
