SMMUv3 Nested Stage Setup (IOMMU part)
Subsystems (4)
v11 cover SMMUv3 Nested Stage Setup (IOMMU part)
This version fixes an issue observed by Shameer on an SMMU 3.2, when moving from dual stage config to stage 1 only config. The 2 high 64b of the STE now get reset. Otherwise, leaving the S2TTB set may cause a C_BAD_STE error.
This series can be found at:
https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
(including the VFIO part)
The QEMU fellow series still can be found at:
https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6
Users have expressed interest in that work and tested v9/v10:
- https://patchwork.kernel.org/cover/11039995/#23012381
- https://patchwork.kernel.org/cover/11039995/#23197235
Background:
This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately.
The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindings
Then those capabilities gets implemented in the SMMUv3 driver.
The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE).
Best Regards
Eric
History:
v10 -> v11:
- S2TTB reset when S2 is off
- fix compil issue when CONFIG_IOMMU_DMA is not set
v9 -> v10:
- rebase on top of 5.6.0-rc3
v8 -> v9:
- rebase on 5.3
- split iommu/vfio parts
v6 -> v8:
- Implement VFIO-PCI device specific interrupt framework
v7 -> v8:
- rebase on top of v5.2-rc1 and especially
8be39a1a04c1 iommu/arm-smmu-v3: Add a master->domain pointer
- dynamic alloc of s1_cfg/s2_cfg
- __arm_smmu_tlb_inv_asid/s1_range_nosync
- check there is no HW MSI regions
- asid invalidation using pasid extended struct (change in the uapi)
- add s1_live/s2_live checks
- move check about support of nested stages in domain finalise
- fixes in error reporting according to the discussion with Robin
- reordered the patches to have first iommu/smmuv3 patches and then VFIO patches
v6 -> v7:
- removed device handle from bind/unbind_guest_msi
- added "iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement"
- added few uapi comments as suggested by Jean, Jacop and Alex
v5 -> v6:
- Fix compilation issue when CONFIG_IOMMU_API is unset
v4 -> v5:
- fix bug reported by Vincent: fault handler unregistration now happens in vfio_pci_release
- IOMMU_FAULT_PERM_* moved outside of struct definition + small uapi changes suggested by Kean-Philippe (except fetch_addr)
- iommu: introduce device fault report API: removed the PRI part.
- see individual logs for more details
- reset the ste abort flag on detach
v3 -> v4:
- took into account Alex, jean-Philippe and Robin's comments on v3
- rework of the smmuv3 driver integration
- add tear down ops for msi binding and PASID table binding
- fix S1 fault propagation
- put fault reporting patches at the beginning of the series following Jean-Philippe's request
- update of the cache invalidate and fault API uapis
- VFIO fault reporting rework with 2 separate regions and one mmappable segment for the fault queue
- moved to PATCH
v2 -> v3:
- When registering the S1 MSI binding we now store the device handle. This addresses Robin's comment about discimination of devices beonging to different S1 groups and using different physical MSI doorbells.
- Change the fault reporting API: use VFIO_PCI_DMA_FAULT_IRQ_INDEX to set the eventfd and expose the faults through an mmappable fault region
v1 -> v2:
- Added the fault reporting capability
- asid properly passed on invalidation (fix assignment of multiple devices)
- see individual change logs for more info
Eric Auger (11):
iommu: Introduce bind/unbind_guest_msi
iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg iommu/smmuv3: Get prepared for nested stage support iommu/smmuv3: Implement attach/detach_pasid_table iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs iommu/smmuv3: Implement cache_invalidate
dma-iommu: Implement NESTED_MSI cookie
iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI
regions
iommu/smmuv3: Implement bind/unbind_guest_msi iommu/smmuv3: Report non recoverable faults
Jacob Pan (1):
iommu: Introduce attach/detach_pasid_table API
Jean-Philippe Brucker (1):
iommu/arm-smmu-v3: Maintain a SID->device structure
drivers/iommu/arm-smmu-v3.c | 744 ++++++++++++++++++++++++++++++++---- drivers/iommu/dma-iommu.c | 142 ++++++- drivers/iommu/iommu.c | 56 +++ include/linux/dma-iommu.h | 16 + include/linux/iommu.h | 38 ++ include/uapi/linux/iommu.h | 51 +++ 6 files changed, 975 insertions(+), 72 deletions(-)
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 1/13 iommu: Introduce attach/detach_pasid_table API
From: Jacob Pan <[email protected]>
In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/Intel terminology) while the host owns the stage/level 2.
In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.
This patch adds a new API to the iommu subsystem that allows to set/unset the pasid table information.
A generic iommu_pasid_table_config struct is introduced in a new iommu.h uapi header. This is going to be used by the VFIO user API.
--- drivers/iommu/iommu.c | 19 ++++++++++++++ include/linux/iommu.h | 18 ++++++++++++++ include/uapi/linux/iommu.h | 51 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+)
drivers/iommu/iommu.c +19 -0
@@ -1723,6 +1723,25 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, }EXPORT_SYMBOL_GPL (iommu_sva_unbind_gpasid); +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +if (unlikely (!domain->ops->attach_pasid_table)) +return -ENODEV ; + +return domain->ops->attach_pasid_table (domain, cfg); +} +EXPORT_SYMBOL_GPL (iommu_attach_pasid_table); + +void iommu_detach_pasid_table (struct iommu_domain *domain) +{ +if (unlikely (!domain->ops->detach_pasid_table)) +return ; + + domain->ops->detach_pasid_table (domain); +} +EXPORT_SYMBOL_GPL (iommu_detach_pasid_table); +static void __iommu_detach_device (struct iommu_domain *domain,struct device *dev) {
include/linux/iommu.h +18 -0
@@ -248,6 +248,8 @@ struct iommu_iotlb_gather {* @cache_invalidate: invalidate translation caches * @sva_bind_gpasid: bind guest pasid and mm * @sva_unbind_gpasid: unbind guest pasid and mm +* @attach_pasid_table: attach a pasid table +* @detach_pasid_table: detach the pasid table * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops */
@@ -307,6 +309,9 @@ struct iommu_ops { void *drvdata);void (*sva_unbind)(struct iommu_sva *handle);int (*sva_get_pasid)(struct iommu_sva *handle); +int (*attach_pasid_table)(struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg); +void (*detach_pasid_table)(struct iommu_domain *domain); int (*page_response)(struct device *dev, struct iommu_fault_event *evt,
@@ -446,6 +451,9 @@ extern int iommu_sva_bind_gpasid(struct iommu_domain *domain, struct device *dev, struct iommu_gpasid_bind_data *data);extern int iommu_sva_unbind_gpasid (struct iommu_domain *domain,struct device *dev,ioasid_t pasid); +extern int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg); +extern void iommu_detach_pasid_table (struct iommu_domain *domain);extern struct iommu_domain *iommu_get_domain_for_dev (struct device *dev);extern struct iommu_domain *iommu_get_dma_domain (struct device *dev); extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1048,6 +1056,16 @@ iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)return -ENODEV ; } +static inline +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +return -ENODEV ; +} + +static inline +void iommu_detach_pasid_table (struct iommu_domain *domain) {} +static inline struct iommu_sva *iommu_sva_bind_device (struct device *dev,struct mm_struct *mm,void *drvdata) {
include/uapi/linux/iommu.h +51 -0
55 lines not on this page · raw message
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v11 01/13] iommu: Introduce attach/detach_pasid_table API
Hi Eric,
There are some discussions about how to size the uAPI data.
https://lkml.org/lkml/2020/4/14/939
I think the problem with the current scheme is that when uAPI data gets extended, if VFIO continue to use:
minsz = offsetofend(struct vfio_iommu_type1_set_pasid_table, config); if (copy_from_user(&spt, (void __user *)arg, minsz))
It may copy more data from user than what was setup by the user.
So, as suggested by Alex, we could add argsz to the IOMMU uAPI struct. So if argsz > minsz, then fail the attach_table since kernel might be old, doesn't know about the extra data.
If argsz <= minsz, kernel can support the attach_table but must process the data based on flags or config.
Does it make sense to you?
Quoted 171 lines · On Tue, 14 Apr 2020 17:05:55 +0200 Eric Auger <[email protected]> wrote:
From: Jacob Pan <[email protected]>
In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/Intel terminology) while the host owns the stage/level 2.In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.
This patch adds a new API to the iommu subsystem that allows to set/unset the pasid table information.
A generic
iommu_pasid_table_configstruct is introduced in a newiommu.huapi header. This is going to be used by the VFIO user API.Signed-off-by Jean-Philippe Brucker <[email protected]>Signed-off-by Liu, Yi L <[email protected]>Signed-off-by Ashok Raj <[email protected]>Signed-off-by Jacob Pan <[email protected]>Signed-off-by Eric Auger <[email protected]>Reviewed-by Jean-Philippe Brucker <[email protected]>--- drivers/iommu/iommu.c | 19 ++++++++++++++ include/linux/iommu.h | 18 ++++++++++++++ include/uapi/linux/iommu.h | 51++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+)
drivers/iommu/iommu.c +19
-0@@ -1723,6 +1723,25 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, }EXPORT_SYMBOL_GPL (iommu_sva_unbind_gpasid ); +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +if (unlikely (!domain->ops->attach_pasid_table)) +return -ENODEV ; + +return domain->ops->attach_pasid_table (domain, cfg); +} +EXPORT_SYMBOL_GPL (iommu_attach_pasid_table); + +void iommu_detach_pasid_table (struct iommu_domain *domain) +{ +if (unlikely (!domain->ops->detach_pasid_table)) +return ; + + domain->ops->detach_pasid_table (domain); +} +EXPORT_SYMBOL_GPL (iommu_detach_pasid_table); +static void __iommu_detach_device (struct iommu_domain *domain,struct device *dev) {include/linux/iommu.h +5
-0@@ -248,6 +248,8 @@ struct iommu_iotlb_gather {* @cache_invalidate: invalidate translation caches * @sva_bind_gpasid: bind guest pasid and mm * @sva_unbind_gpasid: unbind guest pasid and mm +* @attach_pasid_table: attach a pasid table +* @detach_pasid_table: detach the pasid table * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops */ @@ -307,6 +309,9 @@ struct iommu_ops { void *drvdata);void (*sva_unbind)(struct iommu_sva *handle);int (*sva_get_pasid)(struct iommu_sva *handle); +int (*attach_pasid_table)(struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg); +void (*detach_pasid_table)(struct iommu_domain *domain); int (*page_response)(struct device *dev,include/linux/iommu.h +0
-0struct iommu_fault_event *evt,@@ -446,6 +451,9 @@ extern int iommu_sva_bind_gpasid(struct
iommu_domain*domain, struct device *dev, structiommu_gpasid_bind_data*data); extern intiommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid); +extern int iommu_attach_pasid_table(structiommu_domain*domain,include/linux/iommu.h +1
-0+struct iommu_pasid_table_config *cfg); +extern void
iommu_detach_pasid_table(struct iommu_domain *domain); extern structiommu_domain*iommu_get_domain_for_dev(struct device *dev); extern structiommu_domain*iommu_get_dma_domain(struct device *dev); extern int iommu_map(structiommu_domain*domain, unsigned long iova, @@ -1048,6 +1056,16 @@iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)return-ENODEV;
}include/linux/iommu.h +10
-0+static inline +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +return -ENODEV ; +} + +static inline +void iommu_detach_pasid_table (struct iommu_domain *domain) {} +static inline struct iommu_sva * iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void*drvdata) {
include/uapi/linux/iommu.h +2
-0@@ -321,4 +321,55 @@ struct iommu_gpasid_bind_data { }; }; +/** +* struct iommu_pasid_smmuv3 - ARM SMMUv3 Stream Table Entry stage 1 related
+ * information
+ * @version: API version of this structure
+ * @s1fmt: STE s1fmt (format of the CD table: single CD, linear table
+ * or 2-level table)
+ * @s1dss: STE s1dss (specifies the behavior when @pasid_bits != 0
+ * and no PASID is passed along with the incoming
transaction)
+ * @padding: reserved for future use (should be zero)
+ *
+ * The PASID table is referred to as the Context Descriptor (CD) table on ARM
+ * SMMUv3. Please refer to the ARM SMMU 3.x spec (ARM IHI 0070A) for full
+ * details.
+ */include/uapi/linux/iommu.h +10
-0+struct iommu_pasid_smmuv3 { +#define PASID_TABLE_SMMUV3_CFG_VERSION_1 1 +__u32 version; +__u8 s1fmt; +__u8 s1dss; +__u8 padding[2 ]; +}; + +/** +* struct iommu_pasid_table_config - PASID table data used to bind guest PASID
+ * table to the host IOMMU
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ * @base_ptr: guest physical address of the PASID table
+ * @pasid_bits: number of PASID bits used in the PASID table
+ * @config: indicates whether the guest translation stage must
+ * be translated, bypassed or aborted.
+ * @padding: reserved for future use (should be zero)
+ * @smmuv3: table information when @format is %IOMMU_PASID_FORMAT_SMMUV3
+ */include/uapi/linux/iommu.h +8
-0+struct iommu_pasid_table_config { +#define PASID_TABLE_CFG_VERSION_1 1 +__u32 version; +#define IOMMU_PASID_FORMAT_SMMUV3 1 +__u32 format; +__u64 base_ptr; +__u8 pasid_bits; +#define IOMMU_PASID_CONFIG_TRANSLATE 1
include/uapi/linux/iommu.h struct iommu_gpasid_bind_data { · line 375
Quoted 10 lines
include/uapi/linux/iommu.h +9
-0+#define IOMMU_PASID_CONFIG_BYPASS 2 +#define IOMMU_PASID_CONFIG_ABORT 3 +__u8 config; +__u8 padding[6 ]; +union { +struct iommu_pasid_smmuv3 smmuv3; + }; +}; +#endif /* _UAPI_IOMMU_H */
[Jacob Pan]
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v11 01/13] iommu: Introduce attach/detach_pasid_table API
Hi Jacob,
Quoted 11 lines · Jacob Pan
Hi Eric,
There are some discussions about how to size the uAPI data.
https://lkml.org/lkml/2020/4/14/939I think the problem with the current scheme is that when uAPI data gets extended, if VFIO continue to use:
minsz =
offsetofend(struct vfio_iommu_type1_set_pasid_table, config); if (copy_from_user(&spt, (void __user *)arg, minsz))
Quoted 7 lines
It may copy more data from user than what was setup by the user.
So, as suggested by Alex, we could add argsz to the IOMMU uAPI struct. So if argsz > minsz, then fail the
attach_tablesince kernel might be old, doesn't know about the extra data.
If argsz <= minsz, kernel can support theattach_tablebut must process the data based on flags or config.
So I guess we would need both an argsz _u32 + a new flag _u32 right?
I am ok with that idea. Besides how will you manage for existing IOMMU UAPIs? At some point you envisionned to have a getter at iommu api level to retrieve the size of a structure for a given version, right?
Thanks
Eric
Quoted 182 lines
Does it make sense to you?
Quoted 175 lines · On Tue, 14 Apr 2020 17:05:55 +0200 Eric Auger <[email protected]> wrote:
From: Jacob Pan <[email protected]>
In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/Intel terminology) while the host owns the stage/level 2.In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.
This patch adds a new API to the iommu subsystem that allows to set/unset the pasid table information.
A generic
iommu_pasid_table_configstruct is introduced in a newiommu.huapi header. This is going to be used by the VFIO user API.Signed-off-by Jean-Philippe Brucker <[email protected]>Signed-off-by Liu, Yi L <[email protected]>Signed-off-by Ashok Raj <[email protected]>Signed-off-by Jacob Pan <[email protected]>Signed-off-by Eric Auger <[email protected]>Reviewed-by Jean-Philippe Brucker <[email protected]>--- drivers/iommu/iommu.c | 19 ++++++++++++++ include/linux/iommu.h | 18 ++++++++++++++ include/uapi/linux/iommu.h | 51++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+)
drivers/iommu/iommu.c +19
-0@@ -1723,6 +1723,25 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, }EXPORT_SYMBOL_GPL (iommu_sva_unbind_gpasid ); +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +if (unlikely (!domain->ops->attach_pasid_table)) +return -ENODEV ; + +return domain->ops->attach_pasid_table (domain, cfg); +} +EXPORT_SYMBOL_GPL (iommu_attach_pasid_table); + +void iommu_detach_pasid_table (struct iommu_domain *domain) +{ +if (unlikely (!domain->ops->detach_pasid_table)) +return ; + + domain->ops->detach_pasid_table (domain); +} +EXPORT_SYMBOL_GPL (iommu_detach_pasid_table); +static void __iommu_detach_device (struct iommu_domain *domain,struct device *dev) {include/linux/iommu.h +5
-0@@ -248,6 +248,8 @@ struct iommu_iotlb_gather {* @cache_invalidate: invalidate translation caches * @sva_bind_gpasid: bind guest pasid and mm * @sva_unbind_gpasid: unbind guest pasid and mm +* @attach_pasid_table: attach a pasid table +* @detach_pasid_table: detach the pasid table * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops */ @@ -307,6 +309,9 @@ struct iommu_ops { void *drvdata);void (*sva_unbind)(struct iommu_sva *handle);int (*sva_get_pasid)(struct iommu_sva *handle); +int (*attach_pasid_table)(struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg); +void (*detach_pasid_table)(struct iommu_domain *domain); int (*page_response)(struct device *dev,include/linux/iommu.h +0
-0struct iommu_fault_event *evt,@@ -446,6 +451,9 @@ extern int iommu_sva_bind_gpasid(struct
iommu_domain*domain, struct device *dev, structiommu_gpasid_bind_data*data); extern intiommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid); +extern int iommu_attach_pasid_table(structiommu_domain*domain,include/linux/iommu.h +1
-0+struct iommu_pasid_table_config *cfg); +extern void
iommu_detach_pasid_table(struct iommu_domain *domain); extern structiommu_domain*iommu_get_domain_for_dev(struct device *dev); extern structiommu_domain*iommu_get_dma_domain(struct device *dev); extern int iommu_map(structiommu_domain*domain, unsigned long iova, @@ -1048,6 +1056,16 @@iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)return-ENODEV;
}include/linux/iommu.h +10
-0+static inline +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +return -ENODEV ; +} + +static inline +void iommu_detach_pasid_table (struct iommu_domain *domain) {} +static inline struct iommu_sva * iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void*drvdata) {
include/uapi/linux/iommu.h +2
-0@@ -321,4 +321,55 @@ struct iommu_gpasid_bind_data { }; }; +/** +* struct iommu_pasid_smmuv3 - ARM SMMUv3 Stream Table Entry stage 1 related
+ * information
+ * @version: API version of this structure
+ * @s1fmt: STE s1fmt (format of the CD table: single CD, linear table
+ * or 2-level table)
+ * @s1dss: STE s1dss (specifies the behavior when @pasid_bits != 0
+ * and no PASID is passed along with the incoming
transaction)
+ * @padding: reserved for future use (should be zero)
+ *
+ * The PASID table is referred to as the Context Descriptor (CD) table on ARM
+ * SMMUv3. Please refer to the ARM SMMU 3.x spec (ARM IHI 0070A) for full
+ * details.
+ */include/uapi/linux/iommu.h +10
-0+struct iommu_pasid_smmuv3 { +#define PASID_TABLE_SMMUV3_CFG_VERSION_1 1 +__u32 version; +__u8 s1fmt; +__u8 s1dss; +__u8 padding[2 ]; +}; + +/** +* struct iommu_pasid_table_config - PASID table data used to bind guest PASID
+ * table to the host IOMMU
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ * @base_ptr: guest physical address of the PASID table
+ * @pasid_bits: number of PASID bits used in the PASID table
+ * @config: indicates whether the guest translation stage must
+ * be translated, bypassed or aborted.
+ * @padding: reserved for future use (should be zero)
+ * @smmuv3: table information when @format is %IOMMU_PASID_FORMAT_SMMUV3
+ */include/uapi/linux/iommu.h +12
-0+struct iommu_pasid_table_config { +#define PASID_TABLE_CFG_VERSION_1 1 +__u32 version; +#define IOMMU_PASID_FORMAT_SMMUV3 1 +__u32 format; +__u64 base_ptr; +__u8 pasid_bits; +#define IOMMU_PASID_CONFIG_TRANSLATE 1 +#define IOMMU_PASID_CONFIG_BYPASS 2 +#define IOMMU_PASID_CONFIG_ABORT 3 +__u8 config; +__u8 padding[6 ];
Quoted 9 lines
include/uapi/linux/iommu.h struct iommu_gpasid_bind_data { · line 375
Quoted 6 lines
include/uapi/linux/iommu.h +5
-0+union { +struct iommu_pasid_smmuv3 smmuv3; + }; +}; +#endif /* _UAPI_IOMMU_H */ [Jacob Pan]
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 01/13] iommu: Introduce attach/detach_pasid_table API
Quoted 15 lines · On Wed, 15 Apr 2020 16:52:10 +0200 Auger Eric <[email protected]> wrote:
Hi Jacob,
Quoted 13 lines · Jacob Pan
Hi Eric,
There are some discussions about how to size the uAPI data.
https://lkml.org/lkml/2020/4/14/939I think the problem with the current scheme is that when uAPI data gets extended, if VFIO continue to use:
minsz =
offsetofend(struct vfio_iommu_type1_set_pasid_table, config); if (copy_from_user(&spt, (void __user *)arg, minsz))It may copy more data from user than what was setup by the user.
Quoted 8 lines
Quoted 5 lines
So, as suggested by Alex, we could add argsz to the IOMMU uAPI struct. So if argsz > minsz, then fail the
attach_tablesince kernel might be old, doesn't know about the extra data. If argsz <= minsz, kernel can support theattach_tablebut must process the data based on flags or config.So I guess we would need both an argsz
_u32+ a new flag_u32right?
Yes.
Quoted 2 lines
I am ok with that idea. Besides how will you manage for existing IOMMU UAPIs?
I plan to add argsz and flags (if not already have one)
Quoted 3 lines
At some point you envisionned to have a getter at iommu api level to retrieve the size of a structure for a given version, right?
This idea is shot down. There is no version-size lookup. So the current plan is for user to fill out argsz in each IOMMU uAPI struct. VFIO does the copy_from_user() based on argsz (sanitized against the size of current kernel struct).
IOMMU vendor driver process the data based on flags which indicates new capability/extensions.
Quoted 196 lines
Thanks
Eric
Quoted 192 lines
Does it make sense to you?
Quoted 172 lines · On Tue, 14 Apr 2020 17:05:55 +0200 Eric Auger <[email protected]> wrote:
From: Jacob Pan <[email protected]>
In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/Intel terminology) while the host owns the stage/level 2.In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.
This patch adds a new API to the iommu subsystem that allows to set/unset the pasid table information.
A generic
iommu_pasid_table_configstruct is introduced in a newiommu.huapi header. This is going to be used by the VFIO user API.Signed-off-by: Jean-Philippe Brucker <[email protected]> Signed-off-by: Liu, Yi L <[email protected]> Signed-off-by: Ashok Raj <[email protected]> Signed-off-by: Jacob Pan <[email protected]> Signed-off-by: Eric Auger <[email protected]> Reviewed-by: Jean-Philippe Brucker <[email protected]> ---
drivers/iommu/iommu.c | 19 ++++++++++++++ include/linux/iommu.h | 18 ++++++++++++++ include/uapi/linux/iommu.h | 51++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+)
drivers/iommu/iommu.c +19
-0@@ -1723,6 +1723,25 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, }EXPORT_SYMBOL_GPL (iommu_sva_unbind_gpasid ); +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +if (unlikely (!domain->ops->attach_pasid_table)) +return -ENODEV ; + +return domain->ops->attach_pasid_table (domain, cfg); +} +EXPORT_SYMBOL_GPL (iommu_attach_pasid_table); + +void iommu_detach_pasid_table (struct iommu_domain *domain) +{ +if (unlikely (!domain->ops->detach_pasid_table)) +return ; + + domain->ops->detach_pasid_table (domain); +} +EXPORT_SYMBOL_GPL (iommu_detach_pasid_table); +static void __iommu_detach_device (struct iommu_domain *domain,struct device *dev) {include/linux/iommu.h +5
-0@@ -248,6 +248,8 @@ struct iommu_iotlb_gather {* @cache_invalidate: invalidate translation caches * @sva_bind_gpasid: bind guest pasid and mm * @sva_unbind_gpasid: unbind guest pasid and mm +* @attach_pasid_table: attach a pasid table +* @detach_pasid_table: detach the pasid table * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops */ @@ -307,6 +309,9 @@ struct iommu_ops { void *drvdata);void (*sva_unbind)(struct iommu_sva *handle);int (*sva_get_pasid)(struct iommu_sva *handle); +int (*attach_pasid_table)(struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg); +void (*detach_pasid_table)(struct iommu_domain *domain); int (*page_response)(struct device *dev,include/linux/iommu.h +0
-0struct iommu_fault_event *evt,@@ -446,6 +451,9 @@ extern int iommu_sva_bind_gpasid(struct
iommu_domain*domain, struct device *dev, structiommu_gpasid_bind_data*data); extern intiommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid); +extern int iommu_attach_pasid_table(structiommu_domain*domain,include/linux/iommu.h +1
-0+ struct
iommu_pasid_table_config*cfg); +extern voidiommu_detach_pasid_table(struct iommu_domain *domain); extern structiommu_domain*iommu_get_domain_for_dev(struct device *dev); extern structiommu_domain*iommu_get_dma_domain(struct device *dev); extern int iommu_map(structiommu_domain*domain, unsigned long iova, @@ -1048,6 +1056,16 @@iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)return-ENODEV; }include/linux/iommu.h +10
-0+static inline +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +return -ENODEV ; +} + +static inline +void iommu_detach_pasid_table (struct iommu_domain *domain) {} +static inline struct iommu_sva * iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,void *drvdata) {
(quoted diff) +0
-0
b/include/uapi/linux/iommu.hindex4ad3496e5c43..8d00be10dc6d100644 ---a/include/uapi/linux/iommu.h
+++b/include/uapi/linux/iommu.hinclude/linux/iommu.h +2
-0@@ -321,4 +321,55 @@ struct iommu_gpasid_bind_data { }; }; +/** +* struct iommu_pasid_smmuv3 - ARM SMMUv3 Stream Table Entry stage 1 related
+ * information
+ * @version: API version of this structure
+ * @s1fmt: STE s1fmt (format of the CD table: single CD, linear table
+ * or 2-level table)
+ * @s1dss: STE s1dss (specifies the behavior when @pasid_bits != 0
+ * and no PASID is passed along with the incoming
transaction)
+ * @padding: reserved for future use (should be zero)
+ *
+ * The PASID table is referred to as the Context Descriptor (CD) table on ARM
+ * SMMUv3. Please refer to the ARM SMMU 3.x spec (ARM IHI 0070A) for full
+ * details.
+ */include/linux/iommu.h +10
-0+struct iommu_pasid_smmuv3 { +#define PASID_TABLE_SMMUV3_CFG_VERSION_1 1 +__u32 version; +__u8 s1fmt; +__u8 s1dss; +__u8 padding[2 ]; +}; + +/** +* struct iommu_pasid_table_config - PASID table data used to bind guest PASID
+ * table to the host IOMMU
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ * @base_ptr: guest physical address of the PASID table
+ * @pasid_bits: number of PASID bits used in the PASID table
+ * @config: indicates whether the guest translation stage must
+ * be translated, bypassed or aborted.
+ * @padding: reserved for future use (should be zero)
+ * @smmuv3: table information when @format is %IOMMU_PASID_FORMAT_SMMUV3
+ */include/linux/iommu.h +8
-0+struct iommu_pasid_table_config { +#define PASID_TABLE_CFG_VERSION_1 1 +__u32 version; +#define IOMMU_PASID_FORMAT_SMMUV3 1 +__u32 format; +__u64 base_ptr; +__u8 pasid_bits; +#define IOMMU_PASID_CONFIG_TRANSLATE 1include/linux/iommu.h
Quoted 10 lines
include/linux/iommu.h +9
-0+#define IOMMU_PASID_CONFIG_BYPASS 2 +#define IOMMU_PASID_CONFIG_ABORT 3 +__u8 config; +__u8 padding[6 ]; +union { +struct iommu_pasid_smmuv3 smmuv3; + }; +}; +#endif /* _UAPI_IOMMU_H */ [Jacob Pan]
[Jacob Pan]
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 01/13] iommu: Introduce attach/detach_pasid_table API
Hi Jacob,
Quoted 34 lines · Jacob Pan
Quoted 15 lines · On Wed, 15 Apr 2020 16:52:10 +0200 Auger Eric <[email protected]> wrote:
Hi Jacob,
Quoted 13 lines · Jacob Pan
Hi Eric,
There are some discussions about how to size the uAPI data.
https://lkml.org/lkml/2020/4/14/939I think the problem with the current scheme is that when uAPI data gets extended, if VFIO continue to use:
minsz =
offsetofend(struct vfio_iommu_type1_set_pasid_table, config); if (copy_from_user(&spt, (void __user *)arg, minsz))It may copy more data from user than what was setup by the user.
Quoted 8 lines
Quoted 5 lines
So, as suggested by Alex, we could add argsz to the IOMMU uAPI struct. So if argsz > minsz, then fail the
attach_tablesince kernel might be old, doesn't know about the extra data. If argsz <= minsz, kernel can support theattach_tablebut must process the data based on flags or config.So I guess we would need both an argsz
_u32+ a new flag_u32right?Yes.
Quoted 2 lines
I am ok with that idea. Besides how will you manage for existing IOMMU UAPIs?
I plan to add argsz and flags (if not already have one)
Quoted 3 lines
At some point you envisionned to have a getter at iommu api level to retrieve the size of a structure for a given version, right?
Quoted 7 lines
This idea is shot down. There is no version-size lookup. So the current plan is for user to fill out argsz in each IOMMU uAPI struct. VFIO does the
copy_from_user()based on argsz (sanitized against the size of current kernel struct).IOMMU vendor driver process the data based on flags which indicates new capability/extensions.
OK. Sounds sensible
Thanks
Eric
Quoted 198 lines
Quoted 197 lines
Thanks
Eric
Quoted 192 lines
Does it make sense to you?
Quoted 172 lines · On Tue, 14 Apr 2020 17:05:55 +0200 Eric Auger <[email protected]> wrote:
From: Jacob Pan <[email protected]>
In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/Intel terminology) while the host owns the stage/level 2.In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.
This patch adds a new API to the iommu subsystem that allows to set/unset the pasid table information.
A generic
iommu_pasid_table_configstruct is introduced in a newiommu.huapi header. This is going to be used by the VFIO user API.Signed-off-by: Jean-Philippe Brucker <[email protected]> Signed-off-by: Liu, Yi L <[email protected]> Signed-off-by: Ashok Raj <[email protected]> Signed-off-by: Jacob Pan <[email protected]> Signed-off-by: Eric Auger <[email protected]> Reviewed-by: Jean-Philippe Brucker <[email protected]> ---
drivers/iommu/iommu.c | 19 ++++++++++++++ include/linux/iommu.h | 18 ++++++++++++++ include/uapi/linux/iommu.h | 51++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+)
drivers/iommu/iommu.c +19
-0@@ -1723,6 +1723,25 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, }EXPORT_SYMBOL_GPL (iommu_sva_unbind_gpasid ); +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +if (unlikely (!domain->ops->attach_pasid_table)) +return -ENODEV ; + +return domain->ops->attach_pasid_table (domain, cfg); +} +EXPORT_SYMBOL_GPL (iommu_attach_pasid_table); + +void iommu_detach_pasid_table (struct iommu_domain *domain) +{ +if (unlikely (!domain->ops->detach_pasid_table)) +return ; + + domain->ops->detach_pasid_table (domain); +} +EXPORT_SYMBOL_GPL (iommu_detach_pasid_table); +static void __iommu_detach_device (struct iommu_domain *domain,struct device *dev) {include/linux/iommu.h +5
-0@@ -248,6 +248,8 @@ struct iommu_iotlb_gather {* @cache_invalidate: invalidate translation caches * @sva_bind_gpasid: bind guest pasid and mm * @sva_unbind_gpasid: unbind guest pasid and mm +* @attach_pasid_table: attach a pasid table +* @detach_pasid_table: detach the pasid table * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops */ @@ -307,6 +309,9 @@ struct iommu_ops { void *drvdata);void (*sva_unbind)(struct iommu_sva *handle);int (*sva_get_pasid)(struct iommu_sva *handle); +int (*attach_pasid_table)(struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg); +void (*detach_pasid_table)(struct iommu_domain *domain); int (*page_response)(struct device *dev,include/linux/iommu.h +0
-0struct iommu_fault_event *evt,@@ -446,6 +451,9 @@ extern int iommu_sva_bind_gpasid(struct
iommu_domain*domain, struct device *dev, structiommu_gpasid_bind_data*data); extern intiommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid); +extern int iommu_attach_pasid_table(structiommu_domain*domain,include/linux/iommu.h +1
-0+ struct
iommu_pasid_table_config*cfg); +extern voidiommu_detach_pasid_table(struct iommu_domain *domain); extern structiommu_domain*iommu_get_domain_for_dev(struct device *dev); extern structiommu_domain*iommu_get_dma_domain(struct device *dev); extern int iommu_map(structiommu_domain*domain, unsigned long iova, @@ -1048,6 +1056,16 @@iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)return-ENODEV; }include/linux/iommu.h +10
-0+static inline +int iommu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +return -ENODEV ; +} + +static inline +void iommu_detach_pasid_table (struct iommu_domain *domain) {} +static inline struct iommu_sva * iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,void *drvdata) {
(quoted diff) +0
-0
b/include/uapi/linux/iommu.hindex4ad3496e5c43..8d00be10dc6d100644 ---a/include/uapi/linux/iommu.h
+++b/include/uapi/linux/iommu.hinclude/linux/iommu.h +2
-0@@ -321,4 +321,55 @@ struct iommu_gpasid_bind_data { }; }; +/** +* struct iommu_pasid_smmuv3 - ARM SMMUv3 Stream Table Entry stage 1 related
+ * information
+ * @version: API version of this structure
+ * @s1fmt: STE s1fmt (format of the CD table: single CD, linear table
+ * or 2-level table)
+ * @s1dss: STE s1dss (specifies the behavior when @pasid_bits != 0
+ * and no PASID is passed along with the incoming
transaction)
+ * @padding: reserved for future use (should be zero)
+ *
+ * The PASID table is referred to as the Context Descriptor (CD) table on ARM
+ * SMMUv3. Please refer to the ARM SMMU 3.x spec (ARM IHI 0070A) for full
+ * details.
+ */include/linux/iommu.h +10
-0+struct iommu_pasid_smmuv3 { +#define PASID_TABLE_SMMUV3_CFG_VERSION_1 1 +__u32 version; +__u8 s1fmt; +__u8 s1dss; +__u8 padding[2 ]; +}; + +/** +* struct iommu_pasid_table_config - PASID table data used to bind guest PASID
+ * table to the host IOMMU
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ * @base_ptr: guest physical address of the PASID table
+ * @pasid_bits: number of PASID bits used in the PASID table
+ * @config: indicates whether the guest translation stage must
+ * be translated, bypassed or aborted.
+ * @padding: reserved for future use (should be zero)
+ * @smmuv3: table information when @format is %IOMMU_PASID_FORMAT_SMMUV3
+ */include/linux/iommu.h +8
-0+struct iommu_pasid_table_config { +#define PASID_TABLE_CFG_VERSION_1 1 +__u32 version; +#define IOMMU_PASID_FORMAT_SMMUV3 1 +__u32 format; +__u64 base_ptr; +__u8 pasid_bits; +#define IOMMU_PASID_CONFIG_TRANSLATE 1include/linux/iommu.h
Quoted 10 lines
include/linux/iommu.h +9
-0+#define IOMMU_PASID_CONFIG_BYPASS 2 +#define IOMMU_PASID_CONFIG_ABORT 3 +__u8 config; +__u8 padding[6 ]; +union { +struct iommu_pasid_smmuv3 smmuv3; + }; +}; +#endif /* _UAPI_IOMMU_H */ [Jacob Pan]
Quoted 2 lines
[Jacob Pan]
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 2/13 iommu: Introduce bind/unbind_guest_msi
In reply to Eric AugerOn ARM, MSI are translated by the SMMU. An IOVA is allocated for each MSI doorbell. If both the host and the guest are exposed with SMMUs, we end up with 2 different IOVAs allocated by each. guest allocates an IOVA (gIOVA) to map onto the guest MSI doorbell (gDB). The Host allocates another IOVA (hIOVA) to map onto the physical doorbell (hDB).
So we end up with 2 untied mappings:
S1 S2
gIOVA -> gDB
hIOVA -> hDB
Currently the PCI device is programmed by the host with hIOVA as MSI doorbell. So this does not work.
This patch introduces an API to pass gIOVA/gDB to the host so that gIOVA can be reused by the host instead of re-allocating a new IOVA. So the goal is to create the following nested mapping:
S1 S2
gIOVA -> gDB -> hDB
and program the PCI device with gIOVA MSI doorbell.
In case we have several devices attached to this nested domain (devices belonging to the same group), they cannot be isolated on guest side either. So they should also end up in the same domain on guest side. We will enforce that all the devices attached to the host iommu domain use the same physical doorbell and similarly a single virtual doorbell mapping gets registered (1 single virtual doorbell is used on guest as well).
---
v7 -> v8:
- dummy iommu_unbind_guest_msi turned into a void function
v6 -> v7:
- remove the device handle parameter.
- Add comments saying there can only be a single MSI binding registered per iommu_domain
v5 -> v6:
-fix compile issue when IOMMU_API is not set
v3 -> v4:
- add unbind
v2 -> v3:
- add a struct device handle
--- drivers/iommu/iommu.c | 37 +++++++++++++++++++++++++++++++++++++ include/linux/iommu.h | 20 ++++++++++++++++++++ 2 files changed, 57 insertions(+)
drivers/iommu/iommu.c +37 -0
@@ -1756,6 +1756,43 @@ static void __iommu_detach_device(struct iommu_domain *domain,trace_detach_device_from_domain (dev); } +/** +* iommu_bind_guest_msi - Passes the stage1 GIOVA/GPA mapping of a +* virtual doorbell +* +* @domain: iommu domain the stage 1 mapping will be attached to +* @iova: iova allocated by the guest +* @gpa: guest physical address of the virtual doorbell +* @size: granule size used for the mapping +* +* The associated IOVA can be reused by the host to create a nested +* stage2 binding mapping translating into the physical doorbell used +* by the devices attached to the domain. +* +* All devices within the domain must share the same physical doorbell. +* A single MSI GIOVA/GPA mapping can be attached to an iommu_domain. +*/ + +int iommu_bind_guest_msi (struct iommu_domain *domain, +dma_addr_t giova,phys_addr_t gpa,size_t size) +{ +if (unlikely (!domain->ops->bind_guest_msi)) +return -ENODEV ; + +return domain->ops->bind_guest_msi (domain, giova, gpa, size); +} +EXPORT_SYMBOL_GPL (iommu_bind_guest_msi); + +void iommu_unbind_guest_msi (struct iommu_domain *domain, +dma_addr_t iova) +{ +if (unlikely (!domain->ops->unbind_guest_msi)) +return ; + + domain->ops->unbind_guest_msi (domain, iova); +} +EXPORT_SYMBOL_GPL (iommu_unbind_guest_msi); +void iommu_detach_device (struct iommu_domain *domain,struct device *dev) {struct iommu_group *group;
include/linux/iommu.h +20 -0
@@ -250,6 +250,8 @@ struct iommu_iotlb_gather {* @sva_unbind_gpasid: unbind guest pasid and mm * @attach_pasid_table: attach a pasid table * @detach_pasid_table: detach the pasid table +* @bind_guest_msi: provides a stage1 giova/gpa MSI doorbell mapping +* @unbind_guest_msi: withdraw a stage1 giova/gpa MSI doorbell mapping * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops */
@@ -323,6 +325,10 @@ struct iommu_ops {int (*sva_unbind_gpasid)(struct device *dev,int pasid); +int (*bind_guest_msi)(struct iommu_domain *domain, +dma_addr_t giova,phys_addr_t gpa,size_t size); +void (*unbind_guest_msi)(struct iommu_domain *domain,dma_addr_t giova); +unsigned long pgsize_bitmap;struct module *owner; };
@@ -454,6 +460,10 @@ extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,extern int iommu_attach_pasid_table (struct iommu_domain *domain,struct iommu_pasid_table_config *cfg);extern void iommu_detach_pasid_table (struct iommu_domain *domain); +extern int iommu_bind_guest_msi (struct iommu_domain *domain, +dma_addr_t giova,phys_addr_t gpa,size_t size); +extern void iommu_unbind_guest_msi (struct iommu_domain *domain, +dma_addr_t giova);extern struct iommu_domain *iommu_get_domain_for_dev (struct device *dev);extern struct iommu_domain *iommu_get_dma_domain (struct device *dev); extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1110,6 +1120,16 @@ static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev) {return NULL ; } + +static inline +int iommu_bind_guest_msi (struct iommu_domain *domain, +dma_addr_t giova,phys_addr_t gpa,size_t size) +{ +return -ENODEV ; +} +static inline +void iommu_unbind_guest_msi (struct iommu_domain *domain,dma_addr_t giova) {} +#endif /* CONFIG_IOMMU_API */ #ifdef CONFIG_IOMMU_DEBUGFS
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 3/13 iommu/arm-smmu-v3: Maintain a SID->device structure
In reply to Eric AugerFrom: Jean-Philippe Brucker <[email protected]>
When handling faults from the event or PRI queue, we need to find the struct device associated to a SID. Add a rb_tree to keep track of SIDs.
--- drivers/iommu/arm-smmu-v3.c | 112 +++++++++++++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-)
drivers/iommu/arm-smmu-v3.c +111 -1
@@ -677,6 +677,16 @@ struct arm_smmu_device {/* IOMMU core code handle */ struct iommu_device iommu; + +struct rb_root streams; +struct mutex streams_mutex; + +}; + +struct arm_smmu_stream { +u32 id; +struct arm_smmu_master *master; +struct rb_node node; };/* SMMU private data for each master */
@@ -687,6 +697,7 @@ struct arm_smmu_master {struct list_head domain_head;u32 *sids;unsigned int num_sids; +struct arm_smmu_stream *streams;bool ats_enabled;unsigned int ssid_bits; };
@@ -1967,6 +1978,32 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)return 0 ; } +__maybe_unused +static struct arm_smmu_master * +arm_smmu_find_master (struct arm_smmu_device *smmu,u32 sid) +{ +struct rb_node *node; +struct arm_smmu_stream *stream; +struct arm_smmu_master *master =NULL ; + +mutex_lock (&smmu->streams_mutex); + node = smmu->streams.rb_node; +while (node) { + stream = rb_entry(node, struct arm_smmu_stream, node); +if (stream->id < sid) { + node = node->rb_right; + }else if (stream->id > sid) { + node = node->rb_left; + }else { + master = stream->master; +break ; + } + } +mutex_unlock (&smmu->streams_mutex); + +return master; +} +/* IRQ and event handlers */ static irqreturn_t arm_smmu_evtq_thread (int irq,void *dev) {
@@ -2912,6 +2949,69 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)return sid < limit; } +static int arm_smmu_insert_master (struct arm_smmu_device *smmu, +struct arm_smmu_master *master) +{ +int i; +int ret =0 ; +struct arm_smmu_stream *new_stream, *cur_stream; +struct rb_node **new_node, *parent_node =NULL ; + + master->streams =kcalloc (master->num_sids, +sizeof (struct arm_smmu_stream ),GFP_KERNEL ); +if (!master->streams) +return -ENOMEM ; + +mutex_lock (&smmu->streams_mutex); +for (i =0 ; i < master->num_sids && !ret; i++) { + new_stream = &master->streams[i]; + new_stream->id = master->sids[i]; + new_stream->master = master; + + new_node = &(smmu->streams.rb_node); +while (*new_node) { + cur_stream = rb_entry(*new_node, struct arm_smmu_stream, + node); + parent_node = *new_node; +if (cur_stream->id > new_stream->id) { + new_node = &((*new_node)->rb_left); + }else if (cur_stream->id < new_stream->id) { + new_node = &((*new_node)->rb_right); + }else { +dev_warn (master->dev, +"stream %u already in tree\n" , + cur_stream->id); + ret = -EINVAL ; +break ; + } + } + +if (!ret) { +rb_link_node (&new_stream->node, parent_node, new_node); +rb_insert_color (&new_stream->node, &smmu->streams); + } + } +mutex_unlock (&smmu->streams_mutex); + +return ret; +} + +static void arm_smmu_remove_master (struct arm_smmu_device *smmu, +struct arm_smmu_master *master) +{ +int i; + +if (!master->streams) +return ; + +mutex_lock (&smmu->streams_mutex); +for (i =0 ; i < master->num_sids; i++) +rb_erase (&master->streams[i].node, &smmu->streams); +mutex_unlock (&smmu->streams_mutex); + +kfree (master->streams); +} +static struct iommu_ops arm_smmu_ops;static int arm_smmu_add_device (struct device *dev)
@@ -2979,15 +3079,21 @@ static int arm_smmu_add_device(struct device *dev)if (ret) goto err_disable_pasid; + ret =arm_smmu_insert_master (smmu, master); +if (ret) + goto err_unlink; + group =iommu_group_get_for_dev (dev);if (IS_ERR (group)) { ret =PTR_ERR (group); - goto err_unlink; + goto err_remove_master; }iommu_group_put (group);return 0 ; +err_remove_master: +arm_smmu_remove_master (smmu, master); err_unlink:iommu_device_unlink (&smmu->iommu, dev); err_disable_pasid:
@@ -3011,6 +3117,7 @@ static void arm_smmu_remove_device(struct device *dev) smmu = master->smmu;arm_smmu_detach_dev (master);iommu_group_remove_device (dev); +arm_smmu_remove_master (smmu, master);iommu_device_unlink (&smmu->iommu, dev);arm_smmu_disable_pasid (master);kfree (master);
@@ -3365,6 +3472,9 @@ static int arm_smmu_init_structures(struct arm_smmu_device *smmu) {int ret; +mutex_init (&smmu->streams_mutex); + smmu->streams =RB_ROOT ; + ret =arm_smmu_init_queues (smmu);if (ret)return ret;
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 4/13 iommu/smmuv3: Dynamically allocate s1_cfg and s2_cfg
In reply to Eric AugerIn preparation for the introduction of nested stages let's turn s1_cfg and s2_cfg fields into pointers which are dynamically allocated depending on the smmu_domain stage.
In nested mode, both stages will coexist and s1_cfg will be allocated when the guest configuration gets passed.
--- drivers/iommu/arm-smmu-v3.c | 94 ++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 42 deletions(-)
drivers/iommu/arm-smmu-v3.c +52 -42
@@ -719,10 +719,8 @@ struct arm_smmu_domain {atomic_t nr_ats_masters;enum arm_smmu_domain_stage stage; -union { -struct arm_smmu_s1_cfg s1_cfg; -struct arm_smmu_s2_cfg s2_cfg; - }; +struct arm_smmu_s1_cfg *s1_cfg; +struct arm_smmu_s2_cfg *s2_cfg;struct iommu_domain domain;
@@ -1598,9 +1596,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,unsigned int idx;struct arm_smmu_l1_ctx_desc *l1_desc;struct arm_smmu_device *smmu = smmu_domain->smmu; -struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg; +struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg->cdcfg; -if (smmu_domain->s1_cfg.s1fmt ==STRTAB_STE_0_S1FMT_LINEAR ) +if (smmu_domain->s1_cfg->s1fmt ==STRTAB_STE_0_S1FMT_LINEAR )return cdcfg->cdtab + ssid *CTXDESC_CD_DWORDS ; idx = ssid >>CTXDESC_SPLIT ;
@@ -1635,7 +1633,7 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,__le64 *cdptr;struct arm_smmu_device *smmu = smmu_domain->smmu; -if (WARN_ON (ssid >= (1 << smmu_domain->s1_cfg.s1cdmax))) +if (WARN_ON (ssid >= (1 << smmu_domain->s1_cfg->s1cdmax)))return -E2BIG ; cdptr =arm_smmu_get_cd_ptr (smmu_domain, ssid);
@@ -1700,7 +1698,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)size_t l1size;size_t max_contexts;struct arm_smmu_device *smmu = smmu_domain->smmu; -struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg; +struct arm_smmu_s1_cfg *cfg = smmu_domain->s1_cfg;struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg; max_contexts =1 << cfg->s1cdmax;
@@ -1748,7 +1746,7 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)int i;size_t size, l1size;struct arm_smmu_device *smmu = smmu_domain->smmu; -struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg; +struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg->cdcfg;if (cdcfg->l1_desc) { size =CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS <<3 );
@@ -1839,17 +1837,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, }if (smmu_domain) { -switch (smmu_domain->stage) { -case ARM_SMMU_DOMAIN_S1 : - s1_cfg = &smmu_domain->s1_cfg; -break ; -case ARM_SMMU_DOMAIN_S2 : -case ARM_SMMU_DOMAIN_NESTED : - s2_cfg = &smmu_domain->s2_cfg; -break ; -default : -break ; - } + s1_cfg = smmu_domain->s1_cfg; + s2_cfg = smmu_domain->s2_cfg; }if (val &STRTAB_STE_0_V ) {
@@ -2286,11 +2275,11 @@ static void arm_smmu_tlb_inv_context(void *cookie)if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 ) { cmd.opcode =CMDQ_OP_TLBI_NH_ASID ; - cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid; + cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid; cmd.tlbi.vmid =0 ; }else { cmd.opcode =CMDQ_OP_TLBI_S12_VMALL ; - cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid; + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid; }/*
@@ -2324,10 +2313,10 @@ static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 ) { cmd.opcode =CMDQ_OP_TLBI_NH_VA ; - cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid; + cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid; }else { cmd.opcode =CMDQ_OP_TLBI_S2_IPA ; - cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid; + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid; }if (smmu->features &ARM_SMMU_FEAT_RANGE_INV ) {
@@ -2477,22 +2466,24 @@ static void arm_smmu_domain_free(struct iommu_domain *domain) {struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain);struct arm_smmu_device *smmu = smmu_domain->smmu; +struct arm_smmu_s1_cfg *s1_cfg = smmu_domain->s1_cfg; +struct arm_smmu_s2_cfg *s2_cfg = smmu_domain->s2_cfg;iommu_put_dma_cookie (domain);free_io_pgtable_ops (smmu_domain->pgtbl_ops);/* Free the CD and ASID, if we allocated them */ -if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 ) { -struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg; - -if (cfg->cdcfg.cdtab) { +if (s1_cfg) { +if (s1_cfg->cdcfg.cdtab) {arm_smmu_free_cd_tables (smmu_domain); -arm_smmu_bitmap_free (smmu->asid_map, cfg->cd.asid); +arm_smmu_bitmap_free (smmu->asid_map, s1_cfg->cd.asid); } - }else { -struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg; -if (cfg->vmid) -arm_smmu_bitmap_free (smmu->vmid_map, cfg->vmid); +kfree (s1_cfg); + } +if (s2_cfg) { +if (s2_cfg->vmid) +arm_smmu_bitmap_free (smmu->vmid_map, s2_cfg->vmid); +kfree (s2_cfg); }kfree (smmu_domain);
@@ -2505,15 +2496,21 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,int ret;int asid;struct arm_smmu_device *smmu = smmu_domain->smmu; -struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg; +struct arm_smmu_s1_cfg *cfg =kzalloc (sizeof (*cfg),GFP_KERNEL ); typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr = &pgtbl_cfg->arm_lpae_s1_cfg.tcr; +if (!cfg) +return -ENOMEM ; + asid =arm_smmu_bitmap_alloc (smmu->asid_map, smmu->asid_bits); -if (asid <0 ) -return asid; +if (asid <0 ) { + ret = asid; + goto out_free_cfg; + } cfg->s1cdmax = master->ssid_bits; + smmu_domain->s1_cfg = cfg; ret =arm_smmu_alloc_cd_tables (smmu_domain);if (ret) goto out_free_asid;
@@ -2544,6 +2541,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,arm_smmu_free_cd_tables (smmu_domain); out_free_asid:arm_smmu_bitmap_free (smmu->asid_map, asid); +out_free_cfg: +kfree (cfg); + smmu_domain->s1_cfg =NULL ;return ret; }
@@ -2551,14 +2551,19 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,struct arm_smmu_master *master, struct io_pgtable_cfg *pgtbl_cfg) { - int vmid; + int vmid, ret;struct arm_smmu_device *smmu = smmu_domain->smmu; -struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg; +struct arm_smmu_s2_cfg *cfg =kzalloc (sizeof (*cfg),GFP_KERNEL ); typeof(&pgtbl_cfg->arm_lpae_s2_cfg.vtcr) vtcr; +if (!cfg) +return -ENOMEM ; + vmid =arm_smmu_bitmap_alloc (smmu->vmid_map, smmu->vmid_bits); -if (vmid <0 ) -return vmid; +if (vmid <0 ) { + ret = vmid; + goto out_free_cfg; + } vtcr = &pgtbl_cfg->arm_lpae_s2_cfg.vtcr; cfg->vmid = (u16 )vmid;
@@ -2570,7 +2575,12 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,FIELD_PREP (STRTAB_STE_2_VTCR_S2SH0 , vtcr->sh) |FIELD_PREP (STRTAB_STE_2_VTCR_S2TG , vtcr->tg) |FIELD_PREP (STRTAB_STE_2_VTCR_S2PS , vtcr->ps); + smmu_domain->s2_cfg = cfg;return 0 ; + +out_free_cfg: +kfree (cfg); +return ret; } static int arm_smmu_domain_finalise(struct iommu_domain *domain,
@@ -2848,10 +2858,10 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) ret = -ENXIO ; goto out_unlock; }else if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 && - master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) { + master->ssid_bits != smmu_domain->s1_cfg->s1cdmax) {dev_err (dev,"cannot attach to incompatible domain (%u SSID bits != %u)\n" , - smmu_domain->s1_cfg.s1cdmax, master->ssid_bits); + smmu_domain->s1_cfg->s1cdmax, master->ssid_bits); ret = -EINVAL ; goto out_unlock; }
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 5/13 iommu/smmuv3: Get prepared for nested stage support
In reply to Eric AugerWhen nested stage translation is setup, both s1_cfg and s2_cfg are allocated.
We introduce a new smmu domain abort field that will be set upon guest stage1 configuration passing.
arm_smmu_write_strtab_ent() is modified to write both stage fields in the STE and deal with the abort field.
In nested mode, only stage 2 is "finalized" as the host does not own/configure the stage 1 context descriptor; guest does.
---
v10 -> v11:
- Fix an issue reported by Shameer when switching from with vSMMU to without vSMMU. Despite the spec does not seem to mention it seems to be needed to reset the 2 high 64b when switching from S1+S2 cfg to S1 only. Especially dst[3] needs to be reset (S2TTB). On some implementations, if the S2TTB is not reset, this causes a C_BAD_STE error
v7 -> v8:
- rebase on 8be39a1a04c1 iommu/arm-smmu-v3: Add a master->domain pointer
- restore live checks for not nested cases and add s1_live and s2_live to be more previse. Remove bypass local variable. In STE live case, move the ste to abort state and send a CFGI_STE before updating the rest of the fields.
- check s2ttb in case of live s2
v4 -> v5:
- reset ste.abort on detach
v3 -> v4:
- s1_cfg.nested_abort and nested_bypass removed.
- s/ste.nested/ste.abort
- arm_smmu_write_strtab_ent modifications with introduction of local abort, bypass and translate local variables
- comment updated
--- drivers/iommu/arm-smmu-v3.c | 68 +++++++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 10 deletions(-)
drivers/iommu/arm-smmu-v3.c +58 -10
@@ -223,6 +223,7 @@#define STRTAB_STE_0_CFG_BYPASS 4#define STRTAB_STE_0_CFG_S1_TRANS 5#define STRTAB_STE_0_CFG_S2_TRANS 6 +#define STRTAB_STE_0_CFG_NESTED 7#define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)#define STRTAB_STE_0_S1FMT_LINEAR 0
@@ -721,6 +722,7 @@ struct arm_smmu_domain {enum arm_smmu_domain_stage stage;struct arm_smmu_s1_cfg *s1_cfg;struct arm_smmu_s2_cfg *s2_cfg; +bool abort;struct iommu_domain domain;
@@ -1807,8 +1809,10 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,* three cases at the moment: * * 1. Invalid (all zero) -> bypass/fault (init) -* 2. Bypass/fault -> translation/bypass (attach) -* 3. Translation/bypass -> bypass/fault (detach) +* 2. Bypass/fault -> single stage translation/bypass (attach) +* 3. Single or nested stage Translation/bypass -> bypass/fault (detach) +* 4. S2 -> S1 + S2 (attach_pasid_table) +* 5. S1 + S2 -> S2 (detach_pasid_table) * * Given that we can't update the STE atomically and the SMMU * doesn't read the thing in a defined order, that leaves us
@@ -1819,7 +1823,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,* 3. Update Config, sync */ u64 val =le64_to_cpu (dst[0 ]); -bool ste_live = false; +bool abort, translate, s1_live = false, s2_live = false, ste_live; +bool nested = false;struct arm_smmu_device *smmu =NULL ;struct arm_smmu_s1_cfg *s1_cfg =NULL ;struct arm_smmu_s2_cfg *s2_cfg =NULL ;
@@ -1839,6 +1844,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,if (smmu_domain) { s1_cfg = smmu_domain->s1_cfg; s2_cfg = smmu_domain->s2_cfg; + nested = (smmu_domain->stage ==ARM_SMMU_DOMAIN_NESTED ); }if (val &STRTAB_STE_0_V ) {
@@ -1846,23 +1852,37 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,case STRTAB_STE_0_CFG_BYPASS :break ;case STRTAB_STE_0_CFG_S1_TRANS : + s1_live = true; +break ;case STRTAB_STE_0_CFG_S2_TRANS : - ste_live = true; + s2_live = true; +break ; +case STRTAB_STE_0_CFG_NESTED : + s1_live = true; + s2_live = true;break ;case STRTAB_STE_0_CFG_ABORT : -BUG_ON (!disable_bypass);break ;default :BUG ();/* STE corruption */ } } + ste_live = s1_live || s2_live; +/* Nuke the existing STE_0 value, as we're going to rewrite it */ val =STRTAB_STE_0_V ;/* Bypass/fault */ -if (!smmu_domain || !(s1_cfg || s2_cfg)) { -if (!smmu_domain && disable_bypass) + +if (!smmu_domain) + abort = disable_bypass; +else + abort = smmu_domain->abort; + translate = s1_cfg || s2_cfg; + +if (abort || !translate) { +if (abort) val |=FIELD_PREP (STRTAB_STE_0_CFG ,STRTAB_STE_0_CFG_ABORT );else val |=FIELD_PREP (STRTAB_STE_0_CFG ,STRTAB_STE_0_CFG_BYPASS );
@@ -1880,8 +1900,18 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,return ; } +/* S1 or S2 translation */ + +BUG_ON (ste_live && !nested); + +if (ste_live) { +/* First invalidate the live STE */ + dst[0 ] =cpu_to_le64 (STRTAB_STE_0_CFG_ABORT ); +arm_smmu_sync_ste_for_sid (smmu, sid); + } +if (s1_cfg) { -BUG_ON (ste_live); +BUG_ON (s1_live); dst[1] = cpu_to_le64( FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) | FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
@@ -1900,7 +1930,14 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, }if (s2_cfg) { -BUG_ON (ste_live); +u64 vttbr = s2_cfg->vttbr &STRTAB_STE_3_S2TTB_MASK ; + +if (s2_live) { +u64 s2ttb =le64_to_cpu (dst[3 ] &STRTAB_STE_3_S2TTB_MASK ); + +BUG_ON (s2ttb != vttbr); + } + dst[2] = cpu_to_le64( FIELD_PREP(STRTAB_STE_2_S2VMID, s2_cfg->vmid) | FIELD_PREP(STRTAB_STE_2_VTCR, s2_cfg->vtcr) |
@@ -1910,9 +1947,12 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,STRTAB_STE_2_S2PTW |STRTAB_STE_2_S2AA64 | STRTAB_STE_2_S2R); - dst[3 ] =cpu_to_le64 (s2_cfg->vttbr &STRTAB_STE_3_S2TTB_MASK ); + dst[3 ] =cpu_to_le64 (vttbr); val |=FIELD_PREP (STRTAB_STE_0_CFG ,STRTAB_STE_0_CFG_S2_TRANS ); + }else { + dst[2 ] =0 ; + dst[3 ] =0 ; }if (master->ats_enabled)
@@ -2602,6 +2642,14 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,return 0 ; } +if (smmu_domain->stage ==ARM_SMMU_DOMAIN_NESTED && + (!(smmu->features &ARM_SMMU_FEAT_TRANS_S1 ) || + !(smmu->features &ARM_SMMU_FEAT_TRANS_S2 ))) { +dev_info (smmu_domain->smmu->dev, +"does not implement two stages\n" ); +return -EINVAL ; + } +/* Restrict the stage to what we can actually support */ if (!(smmu->features &ARM_SMMU_FEAT_TRANS_S1 )) smmu_domain->stage =ARM_SMMU_DOMAIN_S2 ;
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 6/13 iommu/smmuv3: Implement attach/detach_pasid_table
In reply to Eric AugerOn attach_pasid_table() we program STE S1 related info set by the guest into the actual physical STEs. At minimum we need to program the context descriptor GPA and compute whether the stage1 is translated/bypassed or aborted.
---
v7 -> v8:
- remove smmu->features check, now done on domain finalize
v6 -> v7:
- check versions and comment the fact we don't need to take into account s1dss and s1fmt
v3 -> v4:
- adapt to changes in iommu_pasid_table_config
- different programming convention at s1_cfg/s2_cfg/ste.abort
v2 -> v3:
- callback now is named set_pasid_table and struct fields are laid out differently.
v1 -> v2:
- invalidate the STE before changing them
- hold init_mutex
- handle new fields
--- drivers/iommu/arm-smmu-v3.c | 98 +++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+)
drivers/iommu/arm-smmu-v3.c +98 -0
@@ -3293,6 +3293,102 @@ static void arm_smmu_get_resv_regions(struct device *dev,iommu_dma_get_resv_regions (dev, head); } +static int arm_smmu_attach_pasid_table (struct iommu_domain *domain, +struct iommu_pasid_table_config *cfg) +{ +struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain); +struct arm_smmu_master *master; +struct arm_smmu_device *smmu; +unsigned long flags; +int ret = -EINVAL ; + +if (cfg->format !=IOMMU_PASID_FORMAT_SMMUV3 ) +return -EINVAL ; + +if (cfg->version !=PASID_TABLE_CFG_VERSION_1 || + cfg->smmuv3.version !=PASID_TABLE_SMMUV3_CFG_VERSION_1 ) +return -EINVAL ; + +mutex_lock (&smmu_domain->init_mutex); + + smmu = smmu_domain->smmu; + +if (!smmu) + goto out; + +if (smmu_domain->stage !=ARM_SMMU_DOMAIN_NESTED ) + goto out; + +switch (cfg->config) { +case IOMMU_PASID_CONFIG_ABORT : +kfree (smmu_domain->s1_cfg); + smmu_domain->s1_cfg =NULL ; + smmu_domain->abort = true; +break ; +case IOMMU_PASID_CONFIG_BYPASS : +kfree (smmu_domain->s1_cfg); + smmu_domain->s1_cfg =NULL ; + smmu_domain->abort = false; +break ; +case IOMMU_PASID_CONFIG_TRANSLATE : +/* we do not support S1 <-> S1 transitions */ +if (smmu_domain->s1_cfg) + goto out; + +/* +* we currently support a single CD so s1fmt and s1dss +* fields are also ignored +*/ +if (cfg->pasid_bits) + goto out; + + smmu_domain->s1_cfg =kzalloc (sizeof (*smmu_domain->s1_cfg), +GFP_KERNEL ); +if (!smmu_domain->s1_cfg) { + ret = -ENOMEM ; + goto out; + } + + smmu_domain->s1_cfg->cdcfg.cdtab_dma = cfg->base_ptr; + smmu_domain->abort = false; +break ; +default : + goto out; + } +spin_lock_irqsave (&smmu_domain->devices_lock, flags); +list_for_each_entry (master, &smmu_domain->devices, domain_head) +arm_smmu_install_ste_for_dev (master); +spin_unlock_irqrestore (&smmu_domain->devices_lock, flags); + ret =0 ; +out: +mutex_unlock (&smmu_domain->init_mutex); +return ret; +} + +static void arm_smmu_detach_pasid_table (struct iommu_domain *domain) +{ +struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain); +struct arm_smmu_master *master; +unsigned long flags; + +mutex_lock (&smmu_domain->init_mutex); + +if (smmu_domain->stage !=ARM_SMMU_DOMAIN_NESTED ) + goto unlock; + +kfree (smmu_domain->s1_cfg); + smmu_domain->s1_cfg =NULL ; + smmu_domain->abort = true; + +spin_lock_irqsave (&smmu_domain->devices_lock, flags); +list_for_each_entry (master, &smmu_domain->devices, domain_head) +arm_smmu_install_ste_for_dev (master); +spin_unlock_irqrestore (&smmu_domain->devices_lock, flags); + +unlock: +mutex_unlock (&smmu_domain->init_mutex); +} +static struct iommu_ops arm_smmu_ops = { .capable = arm_smmu_capable, .domain_alloc = arm_smmu_domain_alloc,
@@ -3311,6 +3407,8 @@ static struct iommu_ops arm_smmu_ops = { .of_xlate = arm_smmu_of_xlate, .get_resv_regions = arm_smmu_get_resv_regions, .put_resv_regions = generic_iommu_put_resv_regions, + .attach_pasid_table = arm_smmu_attach_pasid_table, + .detach_pasid_table = arm_smmu_detach_pasid_table, .pgsize_bitmap = -1UL, /* Restricted during device attach */ };
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 7/13 iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs
In reply to Eric AugerWith nested stage support, soon we will need to invalidate S1 contexts and ranges tagged with an unmanaged asid, this latter being managed by the guest. So let's introduce 2 helpers that allow to invalidate with externally managed ASIDs
--- drivers/iommu/arm-smmu-v3.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-)
drivers/iommu/arm-smmu-v3.c +30 -6
@@ -2307,13 +2307,18 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, }/* IO_PGTABLE API */ -static void arm_smmu_tlb_inv_context (void *cookie) + +static void __arm_smmu_tlb_inv_context (struct arm_smmu_domain *smmu_domain, +int ext_asid) { -struct arm_smmu_domain *smmu_domain = cookie;struct arm_smmu_device *smmu = smmu_domain->smmu;struct arm_smmu_cmdq_ent cmd; -if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 ) { +if (ext_asid >=0 ) {/* guest stage 1 invalidation */ + cmd.opcode =CMDQ_OP_TLBI_NH_ASID ; + cmd.tlbi.asid = ext_asid; + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid; + }else if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 ) { cmd.opcode =CMDQ_OP_TLBI_NH_ASID ; cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid; cmd.tlbi.vmid =0 ;
@@ -2334,9 +2339,17 @@ static void arm_smmu_tlb_inv_context(void *cookie)arm_smmu_atc_inv_domain (smmu_domain,0 ,0 ,0 ); } -static void arm_smmu_tlb_inv_range (unsigned long iova,size_t size, +static void arm_smmu_tlb_inv_context (void *cookie) +{ +struct arm_smmu_domain *smmu_domain = cookie; + +__arm_smmu_tlb_inv_context (smmu_domain,-1 ); +} + +static void __arm_smmu_tlb_inv_range (unsigned long iova,size_t size,size_t granule,bool leaf, -struct arm_smmu_domain *smmu_domain) +struct arm_smmu_domain *smmu_domain, +int ext_asid) {struct arm_smmu_device *smmu = smmu_domain->smmu;unsigned long start = iova, end = iova + size, num_pages =0 , tg =0 ;
@@ -2351,7 +2364,11 @@ static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,if (!size)return ; -if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 ) { +if (ext_asid >=0 ) {/* guest stage 1 invalidation */ + cmd.opcode =CMDQ_OP_TLBI_NH_VA ; + cmd.tlbi.asid = ext_asid; + cmd.tlbi.vmid = smmu_domain->s2_cfg->vmid; + }else if (smmu_domain->stage ==ARM_SMMU_DOMAIN_S1 ) { cmd.opcode =CMDQ_OP_TLBI_NH_VA ; cmd.tlbi.asid = smmu_domain->s1_cfg->cd.asid; }else {
@@ -2411,6 +2428,13 @@ static void arm_smmu_tlb_inv_range(unsigned long iova, size_t size,arm_smmu_atc_inv_domain (smmu_domain,0 , start, size); } +static void arm_smmu_tlb_inv_range (unsigned long iova,size_t size, +size_t granule,bool leaf, +struct arm_smmu_domain *smmu_domain) +{ +__arm_smmu_tlb_inv_range (iova, size, granule, leaf, smmu_domain,-1 ); +} +static void arm_smmu_tlb_inv_page_nosync (struct iommu_iotlb_gather *gather,unsigned long iova,size_t granule,void *cookie)
Signature · 6 lines
2.20.1 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
PATCH v11 8/13 iommu/smmuv3: Implement cache_invalidate
In reply to Eric AugerImplement domain-selective and page-selective IOTLB invalidations.
---
v7 -> v8:
- ASID based invalidation using iommu_inv_pasid_info
- check ARCHID/PASID flags in addr based invalidation
- use __arm_smmu_tlb_inv_context and __arm_smmu_tlb_inv_range_nosync
v6 -> v7
- check the uapi version
v3 -> v4:
- adapt to changes in the uapi
- add support for leaf parameter
- do not use arm_smmu_tlb_inv_range_nosync or arm_smmu_tlb_inv_context anymore
v2 -> v3:
- replace __arm_smmu_tlb_sync by arm_smmu_cmdq_issue_sync
v1 -> v2:
- properly pass the asid
--- drivers/iommu/arm-smmu-v3.c | 53 +++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
drivers/iommu/arm-smmu-v3.c +53 -0
@@ -3413,6 +3413,58 @@ static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)mutex_unlock (&smmu_domain->init_mutex); } +static int +arm_smmu_cache_invalidate (struct iommu_domain *domain,struct device *dev, +struct iommu_cache_invalidate_info *inv_info) +{ +struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain); +struct arm_smmu_device *smmu = smmu_domain->smmu; + +if (smmu_domain->stage !=ARM_SMMU_DOMAIN_NESTED ) +return -EINVAL ; + +if (!smmu) +return -EINVAL ; + +if (inv_info->version !=IOMMU_CACHE_INVALIDATE_INFO_VERSION_1 ) +return -EINVAL ; + +if (inv_info->cache &IOMMU_CACHE_INV_TYPE_IOTLB ) { +if (inv_info->granularity ==IOMMU_INV_GRANU_PASID ) { +struct iommu_inv_pasid_info *info = + &inv_info->pasid_info; + +if (!(info->flags &IOMMU_INV_PASID_FLAGS_ARCHID ) || + (info->flags &IOMMU_INV_PASID_FLAGS_PASID )) +return -EINVAL ; + +__arm_smmu_tlb_inv_context (smmu_domain, info->archid); + + }else if (inv_info->granularity ==IOMMU_INV_GRANU_ADDR ) { +struct iommu_inv_addr_info *info = &inv_info->addr_info; +size_t size = info->nb_granules * info->granule_size; +bool leaf = info->flags &IOMMU_INV_ADDR_FLAGS_LEAF ; + +if (!(info->flags &IOMMU_INV_ADDR_FLAGS_ARCHID ) || + (info->flags &IOMMU_INV_ADDR_FLAGS_PASID )) +return -EINVAL ; + +__arm_smmu_tlb_inv_range (info->addr, size, + info->granule_size, leaf, + smmu_domain, info->archid); + +arm_smmu_cmdq_issue_sync (smmu); + }else { +return -EINVAL ; + } + } +if (inv_info->cache &IOMMU_CACHE_INV_TYPE_PASID || + inv_info->cache &IOMMU_CACHE_INV_TYPE_DEV_IOTLB ) { +return -ENOENT ; + } +return 0 ; +} +static struct iommu_ops arm_smmu_ops = { .capable = arm_smmu_capable, .domain_alloc = arm_smmu_domain_alloc,
@@ -3433,6 +3485,7 @@ static struct iommu_ops arm_smmu_ops = { .put_resv_regions = generic_iommu_put_resv_regions, .attach_pasid_table = arm_smmu_attach_pasid_table, .detach_pasid_table = arm_smmu_detach_pasid_table, + .cache_invalidate = arm_smmu_cache_invalidate, .pgsize_bitmap = -1UL, /* Restricted during device attach */ };
Signature · 6 lines
2.20.1 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
PATCH v11 9/13 dma-iommu: Implement NESTED_MSI cookie
In reply to Eric AugerUp to now, when the type was UNMANAGED, we used to allocate IOVA pages within a reserved IOVA MSI range.
If both the host and the guest are exposed with SMMUs, each would allocate an IOVA. The guest allocates an IOVA (gIOVA) to map onto the guest MSI doorbell (gDB). The Host allocates another IOVA (hIOVA) to map onto the physical doorbell (hDB).
So we end up with 2 unrelated mappings, at S1 and S2:
S1 S2
gIOVA -> gDB
hIOVA -> hDB
The PCI device would be programmed with hIOVA.
No stage 1 mapping would existing, causing the MSIs to fault.
iommu_dma_bind_guest_msi() allows to pass gIOVA/gDB to the host so that gIOVA can be used by the host instead of re-allocating a new hIOVA.
S1 S2
gIOVA -> gDB -> hDB
this time, the PCI device can be programmed with the gIOVA MSI doorbell which is correctly mapped through both stages.
Nested mode is not compatible with HW MSI regions as in that case gDB and hDB should have a 1-1 mapping. This check will be done when attaching each device to the IOMMU domain.
---
v10 -> v11:
- fix compilation if !CONFIG_IOMMU_DMA
v7 -> v8:
- correct iommu_dma_(un)bind_guest_msi when !CONFIG_IOMMU_DMA
- Mentioned nested mode is not compatible with HW MSI regions in commit message
- protect with msi_lock on unbind
v6 -> v7:
- removed device handle
v3 -> v4:
- change function names; add unregister
- protect with msi_lock
v2 -> v3:
- also store the device handle on S1 mapping registration. This garantees we associate the associated S2 mapping binds to the correct physical MSI controller.
v1 -> v2:
- unmap stage2 on put()
--- drivers/iommu/dma-iommu.c | 142 +++++++++++++++++++++++++++++++++++++- include/linux/dma-iommu.h | 16 +++++ 2 files changed, 155 insertions(+), 3 deletions(-)
drivers/iommu/dma-iommu.c +139 -3
@@ -20,6 +20,7 @@#include <linux/irq.h> #include <linux/mm.h> #include <linux/mutex.h> +#include <linux/mutex.h> #include <linux/pci.h> #include <linux/scatterlist.h> #include <linux/vmalloc.h>
@@ -28,12 +29,15 @@struct iommu_dma_msi_page {struct list_head list;dma_addr_t iova; +dma_addr_t gpa;phys_addr_t phys; +size_t s1_granule; };enum iommu_dma_cookie_type {IOMMU_DMA_IOVA_COOKIE ,IOMMU_DMA_MSI_COOKIE , +IOMMU_DMA_NESTED_MSI_COOKIE , };struct iommu_dma_cookie {
@@ -45,6 +49,7 @@ struct iommu_dma_cookie {dma_addr_t msi_iova; };struct list_head msi_page_list; +spinlock_t msi_lock;/* Domain for flush queue callback; NULL if flush queue not in use */ struct iommu_domain *fq_domain;
@@ -63,6 +68,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type) cookie =kzalloc (sizeof (*cookie),GFP_KERNEL );if (cookie) { +spin_lock_init (&cookie->msi_lock);INIT_LIST_HEAD (&cookie->msi_page_list); cookie->type = type; }
@@ -96,14 +102,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);* * Users who manage their own IOVA allocation and do not want DMA API support, * but would still like to take advantage of automatic MSI remapping, can use -* this to initialise their own domain appropriately. Users should reserve a +* this to initialise their own domain appropriately. Users may reserve a * contiguous IOVA region, starting at @base, large enough to accommodate the * number of PAGE_SIZE mappings necessary to cover every MSI doorbell address -* used by the devices attached to @domain. +* used by the devices attached to @domain. The other way round is to provide +* usable iova pages through the iommu_dma_bind_doorbell API (nested stages +* use case) */ int iommu_get_msi_cookie (struct iommu_domain *domain,dma_addr_t base) {struct iommu_dma_cookie *cookie; +int nesting, ret;if (domain->type !=IOMMU_DOMAIN_UNMANAGED )return -EINVAL ;
@@ -111,7 +120,12 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)if (domain->iova_cookie)return -EEXIST ; - cookie =cookie_alloc (IOMMU_DMA_MSI_COOKIE ); + ret =iommu_domain_get_attr (domain,DOMAIN_ATTR_NESTING , &nesting); +if (!ret && nesting) + cookie =cookie_alloc (IOMMU_DMA_NESTED_MSI_COOKIE ); +else + cookie =cookie_alloc (IOMMU_DMA_MSI_COOKIE ); +if (!cookie)return -ENOMEM ;
@@ -132,6 +146,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain) {struct iommu_dma_cookie *cookie = domain->iova_cookie;struct iommu_dma_msi_page *msi, *tmp; +bool s2_unmap = false;if (!cookie)return ;
@@ -139,7 +154,15 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)if (cookie->type ==IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)put_iova_domain (&cookie->iovad); +if (cookie->type ==IOMMU_DMA_NESTED_MSI_COOKIE ) + s2_unmap = true; +list_for_each_entry_safe (msi, tmp, &cookie->msi_page_list, list) { +if (s2_unmap && msi->phys) { +size_t size =cookie_msi_granule (cookie); + +WARN_ON (iommu_unmap (domain, msi->gpa, size) != size); + }list_del (&msi->list);kfree (msi); }
@@ -148,6 +171,92 @@ void iommu_put_dma_cookie(struct iommu_domain *domain) }EXPORT_SYMBOL (iommu_put_dma_cookie); +/** +* iommu_dma_bind_guest_msi - Allows to pass the stage 1 +* binding of a virtual MSI doorbell used by @dev. +* +* @domain: domain handle +* @iova: guest iova +* @gpa: gpa of the virtual doorbell +* @size: size of the granule used for the stage1 mapping +* +* In nested stage use case, the user can provide IOVA/IPA bindings +* corresponding to a guest MSI stage 1 mapping. When the host needs +* to map its own MSI doorbells, it can use @gpa as stage 2 input +* and map it onto the physical MSI doorbell. +*/ +int iommu_dma_bind_guest_msi (struct iommu_domain *domain, +dma_addr_t iova,phys_addr_t gpa,size_t size) +{ +struct iommu_dma_cookie *cookie = domain->iova_cookie; +struct iommu_dma_msi_page *msi; +int ret =0 ; + +if (!cookie) +return -EINVAL ; + +if (cookie->type !=IOMMU_DMA_NESTED_MSI_COOKIE ) +return -EINVAL ; + + iova = iova & ~(dma_addr_t)(size -1 ); + gpa = gpa & ~(phys_addr_t)(size -1 ); + +spin_lock (&cookie->msi_lock); + +list_for_each_entry (msi, &cookie->msi_page_list, list) { +if (msi->iova == iova) + goto unlock;/* this page is already registered */ + } + + msi =kzalloc (sizeof (*msi),GFP_ATOMIC ); +if (!msi) { + ret = -ENOMEM ; + goto unlock; + } + + msi->iova = iova; + msi->gpa = gpa; + msi->s1_granule = size; +list_add (&msi->list, &cookie->msi_page_list); +unlock: +spin_unlock (&cookie->msi_lock); +return ret; +} +EXPORT_SYMBOL (iommu_dma_bind_guest_msi); + +void iommu_dma_unbind_guest_msi (struct iommu_domain *domain,dma_addr_t giova) +{ +struct iommu_dma_cookie *cookie = domain->iova_cookie; +struct iommu_dma_msi_page *msi; + +if (!cookie) +return ; + +if (cookie->type !=IOMMU_DMA_NESTED_MSI_COOKIE ) +return ; + +spin_lock (&cookie->msi_lock); + +list_for_each_entry (msi, &cookie->msi_page_list, list) { +dma_addr_t aligned_giova = + giova & ~(dma_addr_t)(msi->s1_granule -1 ); + +if (msi->iova == aligned_giova) { +if (msi->phys) { +/* unmap the stage 2 */ +size_t size =cookie_msi_granule (cookie); + +WARN_ON (iommu_unmap (domain, msi->gpa, size) != size); + } +list_del (&msi->list); +kfree (msi); +break ; + } + } +spin_unlock (&cookie->msi_lock); +} +EXPORT_SYMBOL (iommu_dma_unbind_guest_msi); +/** * iommu_dma_get_resv_regions - Reserved region driver helper * @dev: Device from iommu_get_resv_regions()
@@ -1175,6 +1284,33 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,if (msi_page->phys == msi_addr)return msi_page; +/* +* In nested stage mode, we do not allocate an MSI page in +* a range provided by the user. Instead, IOVA/IPA bindings are +* individually provided. We reuse thise IOVAs to build the +* GIOVA -> GPA -> MSI HPA nested stage mapping. +*/ +if (cookie->type ==IOMMU_DMA_NESTED_MSI_COOKIE ) { +list_for_each_entry (msi_page, &cookie->msi_page_list, list) +if (!msi_page->phys) { +int ret; + +/* do the stage 2 mapping */ + ret =iommu_map (domain, + msi_page->gpa, msi_addr, size, +IOMMU_MMIO |IOMMU_WRITE ); +if (ret) { +pr_warn ("MSI S2 mapping failed (%d)\n" , + ret); +return NULL ; + } + msi_page->phys = msi_addr; +return msi_page; + } +pr_warn ("%s no MSI binding found\n" , __func__); +return NULL ; + } + msi_page =kzalloc (sizeof (*msi_page),GFP_KERNEL );if (!msi_page)return NULL ;
include/linux/dma-iommu.h +16 -0
@@ -12,6 +12,7 @@#include <linux/dma-mapping.h> #include <linux/iommu.h> #include <linux/msi.h> +#include <uapi/linux/iommu.h> /* Domain management interface for IOMMU drivers */ int iommu_get_dma_cookie (struct iommu_domain *domain);
@@ -36,6 +37,9 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc, struct msi_msg *msg);void iommu_dma_get_resv_regions (struct device *dev,struct list_head *list); +int iommu_dma_bind_guest_msi (struct iommu_domain *domain, +dma_addr_t iova,phys_addr_t gpa,size_t size); +void iommu_dma_unbind_guest_msi (struct iommu_domain *domain,dma_addr_t giova);#else /* CONFIG_IOMMU_DMA */
@@ -74,6 +78,18 @@ static inline void iommu_dma_compose_msi_msg(struct msi_desc *desc, { } +static inline int +iommu_dma_bind_guest_msi (struct iommu_domain *domain, +dma_addr_t iova,phys_addr_t gpa,size_t size) +{ +return -ENODEV ; +} + +static inline void +iommu_dma_unbind_guest_msi (struct iommu_domain *domain,dma_addr_t giova) +{ +} +static inline void iommu_dma_get_resv_regions (struct device *dev,struct list_head *list) { }
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v11 10/13 iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement
In reply to Eric AugerIn nested mode we enforce the rule that all devices belonging to the same iommu_domain share the same msi_domain.
Indeed if there were several physical MSI doorbells being used within a single iommu_domain, it becomes really difficult to resolve the nested stage mapping translating into the correct physical doorbell. So let's forbid this situation.
--- drivers/iommu/arm-smmu-v3.c | 41 +++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
drivers/iommu/arm-smmu-v3.c +41 -0
@@ -2896,6 +2896,37 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)arm_smmu_install_ste_for_dev (master); } +static bool arm_smmu_share_msi_domain (struct iommu_domain *domain, +struct device *dev) +{ +struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain); +struct irq_domain *irqd =dev_get_msi_domain (dev); +struct arm_smmu_master *master; +unsigned long flags; +bool share = false; + +if (!irqd) +return true; + +spin_lock_irqsave (&smmu_domain->devices_lock, flags); +list_for_each_entry (master, &smmu_domain->devices, domain_head) { +struct irq_domain *d =dev_get_msi_domain (master->dev); + +if (!d) +continue ; +if (irqd != d) { +dev_info (dev,"Nested mode forbids to attach devices " +"using different physical MSI doorbells " +"to the same iommu_domain" ); + goto unlock; + } + } + share = true; +unlock: +spin_unlock_irqrestore (&smmu_domain->devices_lock, flags); +return share; +} +static int arm_smmu_attach_dev (struct iommu_domain *domain,struct device *dev) {int ret =0 ;
@@ -2937,6 +2968,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) ret = -EINVAL ; goto out_unlock; } +/* +* In nested mode we must check all devices belonging to the +* domain share the same physical MSI doorbell. Otherwise nested +* stage MSI binding is not supported. +*/ +if (smmu_domain->stage ==ARM_SMMU_DOMAIN_NESTED && + !arm_smmu_share_msi_domain (domain, dev)) { + ret = -EINVAL ; + goto out_unlock; + } master->domain = smmu_domain;
Signature · 6 lines
2.20.1 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
PATCH v11 11/13 iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI regions
In reply to Eric AugerNested mode currently is not compatible with HW MSI reserved regions. Indeed MSI transactions targeting this MSI doorbells bypass the SMMU.
Let's check nested mode is not attempted in such configuration.
--- drivers/iommu/arm-smmu-v3.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)
drivers/iommu/arm-smmu-v3.c +21 -2
@@ -2927,6 +2927,23 @@ static bool arm_smmu_share_msi_domain(struct iommu_domain *domain,return share; } +static bool arm_smmu_has_hw_msi_resv_region (struct device *dev) +{ +struct iommu_resv_region *region; +bool has_msi_resv_region = false; +LIST_HEAD (resv_regions); + +iommu_get_resv_regions (dev, &resv_regions); +list_for_each_entry (region, &resv_regions, list) { +if (region->type ==IOMMU_RESV_MSI ) { + has_msi_resv_region = true; +break ; + } + } +iommu_put_resv_regions (dev, &resv_regions); +return has_msi_resv_region; +} +static int arm_smmu_attach_dev (struct iommu_domain *domain,struct device *dev) {int ret =0 ;
@@ -2971,10 +2988,12 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)/* * In nested mode we must check all devices belonging to the * domain share the same physical MSI doorbell. Otherwise nested -* stage MSI binding is not supported. +* stage MSI binding is not supported. Also nested mode is not +* compatible with MSI HW reserved regions. */ if (smmu_domain->stage ==ARM_SMMU_DOMAIN_NESTED && - !arm_smmu_share_msi_domain (domain, dev)) { + (!arm_smmu_share_msi_domain (domain, dev) || +arm_smmu_has_hw_msi_resv_region (dev))) { ret = -EINVAL ; goto out_unlock; }
Signature · 6 lines
2.20.1 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
PATCH v11 12/13 iommu/smmuv3: Implement bind/unbind_guest_msi
In reply to Eric AugerThe bind/unbind_guest_msi() callbacks check the domain is NESTED and redirect to the dma-iommu implementation.
---
v6 -> v7:
- remove device handle argument
--- drivers/iommu/arm-smmu-v3.c | 43 +++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
drivers/iommu/arm-smmu-v3.c +43 -0
@@ -3377,6 +3377,47 @@ static void arm_smmu_get_resv_regions(struct device *dev,iommu_dma_get_resv_regions (dev, head); } +static int +arm_smmu_bind_guest_msi (struct iommu_domain *domain, +dma_addr_t giova,phys_addr_t gpa,size_t size) +{ +struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain); +struct arm_smmu_device *smmu; +int ret = -EINVAL ; + +mutex_lock (&smmu_domain->init_mutex); + smmu = smmu_domain->smmu; +if (!smmu) + goto out; + +if (smmu_domain->stage !=ARM_SMMU_DOMAIN_NESTED ) + goto out; + + ret =iommu_dma_bind_guest_msi (domain, giova, gpa, size); +out: +mutex_unlock (&smmu_domain->init_mutex); +return ret; +} + +static void +arm_smmu_unbind_guest_msi (struct iommu_domain *domain,dma_addr_t giova) +{ +struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain); +struct arm_smmu_device *smmu; + +mutex_lock (&smmu_domain->init_mutex); + smmu = smmu_domain->smmu; +if (!smmu) + goto unlock; + +if (smmu_domain->stage !=ARM_SMMU_DOMAIN_NESTED ) + goto unlock; + +iommu_dma_unbind_guest_msi (domain, giova); +unlock: +mutex_unlock (&smmu_domain->init_mutex); +} +static int arm_smmu_attach_pasid_table (struct iommu_domain *domain,struct iommu_pasid_table_config *cfg) {
@@ -3546,6 +3587,8 @@ static struct iommu_ops arm_smmu_ops = { .attach_pasid_table = arm_smmu_attach_pasid_table, .detach_pasid_table = arm_smmu_detach_pasid_table, .cache_invalidate = arm_smmu_cache_invalidate, + .bind_guest_msi = arm_smmu_bind_guest_msi, + .unbind_guest_msi = arm_smmu_unbind_guest_msi, .pgsize_bitmap = -1UL, /* Restricted during device attach */ };
Signature · 6 lines
2.20.1 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
PATCH v11 13/13 iommu/smmuv3: Report non recoverable faults
In reply to Eric AugerWhen a stage 1 related fault event is read from the event queue, let's propagate it to potential external fault listeners, ie. users who registered a fault handler.
---
v8 -> v9:
- adapt to the removal of IOMMU_FAULT_UNRECOV_PERM_VALID:
only look at IOMMU_FAULT_UNRECOV_ADDR_VALID which comes with perm
- do not advertise IOMMU_FAULT_UNRECOV_PASID_VALID faults for translation faults
- trace errors if !master
- test nested before calling iommu_report_device_fault
- call the fault handler unconditionnally in non nested mode
v4 -> v5:
- s/IOMMU_FAULT_PERM_INST/IOMMU_FAULT_PERM_EXEC
--- drivers/iommu/arm-smmu-v3.c | 182 +++++++++++++++++++++++++++++++++--- 1 file changed, 171 insertions(+), 11 deletions(-)
drivers/iommu/arm-smmu-v3.c +171 -11
@@ -171,6 +171,26 @@#define ARM_SMMU_PRIQ_IRQ_CFG1 0xd8#define ARM_SMMU_PRIQ_IRQ_CFG2 0xdc +/* Events */ +#define ARM_SMMU_EVT_F_UUT 0x01 +#define ARM_SMMU_EVT_C_BAD_STREAMID 0x02 +#define ARM_SMMU_EVT_F_STE_FETCH 0x03 +#define ARM_SMMU_EVT_C_BAD_STE 0x04 +#define ARM_SMMU_EVT_F_BAD_ATS_TREQ 0x05 +#define ARM_SMMU_EVT_F_STREAM_DISABLED 0x06 +#define ARM_SMMU_EVT_F_TRANSL_FORBIDDEN 0x07 +#define ARM_SMMU_EVT_C_BAD_SUBSTREAMID 0x08 +#define ARM_SMMU_EVT_F_CD_FETCH 0x09 +#define ARM_SMMU_EVT_C_BAD_CD 0x0a +#define ARM_SMMU_EVT_F_WALK_EABT 0x0b +#define ARM_SMMU_EVT_F_TRANSLATION 0x10 +#define ARM_SMMU_EVT_F_ADDR_SIZE 0x11 +#define ARM_SMMU_EVT_F_ACCESS 0x12 +#define ARM_SMMU_EVT_F_PERMISSION 0x13 +#define ARM_SMMU_EVT_F_TLB_CONFLICT 0x20 +#define ARM_SMMU_EVT_F_CFG_CONFLICT 0x21 +#define ARM_SMMU_EVT_E_PAGE_REQUEST 0x24 +/* Common MSI config fields */ #define MSI_CFG0_ADDR_MASK GENMASK_ULL(51, 2)#define MSI_CFG2_SH GENMASK(5, 4)
@@ -387,6 +407,15 @@#define EVTQ_MAX_SZ_SHIFT (Q_MAX_SZ_SHIFT - EVTQ_ENT_SZ_SHIFT)#define EVTQ_0_ID GENMASK_ULL(7, 0) +#define EVTQ_0_SSV GENMASK_ULL(11, 11) +#define EVTQ_0_SUBSTREAMID GENMASK_ULL(31, 12) +#define EVTQ_0_STREAMID GENMASK_ULL(63, 32) +#define EVTQ_1_PNU GENMASK_ULL(33, 33) +#define EVTQ_1_IND GENMASK_ULL(34, 34) +#define EVTQ_1_RNW GENMASK_ULL(35, 35) +#define EVTQ_1_S2 GENMASK_ULL(39, 39) +#define EVTQ_1_CLASS GENMASK_ULL(40, 41) +#define EVTQ_3_FETCH_ADDR GENMASK_ULL(51, 3)/* PRI queue */ #define PRIQ_ENT_SZ_SHIFT 4
@@ -730,6 +759,57 @@ struct arm_smmu_domain {spinlock_t devices_lock; }; +/* fault propagation */ +struct arm_smmu_fault_propagation_data { +enum iommu_fault_reason reason; +bool s1_check; +u32 fields;/* IOMMU_FAULT_UNRECOV_*_VALID bits */ +}; + +/* +* Describes how SMMU faults translate into generic IOMMU faults +* and if they need to be reported externally +*/ +static const struct arm_smmu_fault_propagation_data fault_propagation[] = { +[ARM_SMMU_EVT_F_UUT ] = { }, +[ARM_SMMU_EVT_C_BAD_STREAMID ] = { }, +[ARM_SMMU_EVT_F_STE_FETCH ] = { }, +[ARM_SMMU_EVT_C_BAD_STE ] = { }, +[ARM_SMMU_EVT_F_BAD_ATS_TREQ ] = { }, +[ARM_SMMU_EVT_F_STREAM_DISABLED ] = { }, +[ARM_SMMU_EVT_F_TRANSL_FORBIDDEN ] = { }, +[ARM_SMMU_EVT_C_BAD_SUBSTREAMID ] = {IOMMU_FAULT_REASON_PASID_INVALID , + false, +IOMMU_FAULT_UNRECOV_PASID_VALID + }, +[ARM_SMMU_EVT_F_CD_FETCH ] = {IOMMU_FAULT_REASON_PASID_FETCH , + false, +IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID + }, +[ARM_SMMU_EVT_C_BAD_CD ] = {IOMMU_FAULT_REASON_BAD_PASID_ENTRY , + false, + }, +[ARM_SMMU_EVT_F_WALK_EABT ] = {IOMMU_FAULT_REASON_WALK_EABT , true, +IOMMU_FAULT_UNRECOV_ADDR_VALID | +IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID + }, +[ARM_SMMU_EVT_F_TRANSLATION ] = {IOMMU_FAULT_REASON_PTE_FETCH , true, +IOMMU_FAULT_UNRECOV_ADDR_VALID + }, +[ARM_SMMU_EVT_F_ADDR_SIZE ] = {IOMMU_FAULT_REASON_OOR_ADDRESS , true, +IOMMU_FAULT_UNRECOV_ADDR_VALID + }, +[ARM_SMMU_EVT_F_ACCESS ] = {IOMMU_FAULT_REASON_ACCESS , true, +IOMMU_FAULT_UNRECOV_ADDR_VALID + }, +[ARM_SMMU_EVT_F_PERMISSION ] = {IOMMU_FAULT_REASON_PERMISSION , true, +IOMMU_FAULT_UNRECOV_ADDR_VALID + }, +[ARM_SMMU_EVT_F_TLB_CONFLICT ] = { }, +[ARM_SMMU_EVT_F_CFG_CONFLICT ] = { }, +[ARM_SMMU_EVT_E_PAGE_REQUEST ] = { }, +}; +struct arm_smmu_option_prop {u32 opt;const char *prop;
@@ -2007,7 +2087,6 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)return 0 ; } -__maybe_unused static struct arm_smmu_master *arm_smmu_find_master (struct arm_smmu_device *smmu,u32 sid) {
@@ -2033,25 +2112,106 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)return master; } +/* Populates the record fields according to the input SMMU event */ +static bool arm_smmu_transcode_fault (u64 *evt,u8 type, +struct iommu_fault_unrecoverable *record) +{ +const struct arm_smmu_fault_propagation_data *data; +u32 fields; + +if (type >=ARRAY_SIZE (fault_propagation)) +return false; + + data = &fault_propagation[type]; +if (!data->reason) +return false; + + fields = data->fields; + +if (data->s1_check &FIELD_GET (EVTQ_1_S2 , evt[1 ])) +return false;/* S2 related fault, don't propagate */ + +if (fields &IOMMU_FAULT_UNRECOV_PASID_VALID ) + record->pasid =FIELD_GET (EVTQ_0_SUBSTREAMID , evt[0 ]); +else { +/* all other transcoded errors have SSV */ +if (FIELD_GET (EVTQ_0_SSV , evt[0 ])) { + record->pasid =FIELD_GET (EVTQ_0_SUBSTREAMID , evt[0 ]); + fields |=IOMMU_FAULT_UNRECOV_PASID_VALID ; + } + } + +if (fields &IOMMU_FAULT_UNRECOV_ADDR_VALID ) { +if (FIELD_GET (EVTQ_1_RNW , evt[1 ])) + record->perm =IOMMU_FAULT_PERM_READ ; +else + record->perm =IOMMU_FAULT_PERM_WRITE ; +if (FIELD_GET (EVTQ_1_PNU , evt[1 ])) + record->perm |=IOMMU_FAULT_PERM_PRIV ; +if (FIELD_GET (EVTQ_1_IND , evt[1 ])) + record->perm |=IOMMU_FAULT_PERM_EXEC ; + record->addr = evt[2 ]; + } + +if (fields &IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID ) + record->fetch_addr =FIELD_GET (EVTQ_3_FETCH_ADDR , evt[3 ]); + + record->flags = fields; + record->reason = data->reason; +return true; +} + +static void arm_smmu_report_event (struct arm_smmu_device *smmu,u64 *evt) +{ +u32 sid =FIELD_GET (EVTQ_0_STREAMID , evt[0 ]); +u8 type =FIELD_GET (EVTQ_0_ID , evt[0 ]); +struct arm_smmu_master *master; +struct iommu_fault_event event = {}; +bool nested; +int i; + + master =arm_smmu_find_master (smmu, sid); +if (!master || !master->domain) + goto out; + + event.fault.type =IOMMU_FAULT_DMA_UNRECOV ; + + nested = (master->domain->stage ==ARM_SMMU_DOMAIN_NESTED ); + +if (nested) { +if (arm_smmu_transcode_fault (evt, type, &event.fault.event)) { +/* +* Only S1 related faults should be reported to the +* guest and must not flood the host log. +* Also a fault handler should have been registered +* to guarantee the full nested functionality +*/ +WARN_ON_ONCE (iommu_report_device_fault (master->dev, + &event)); +return ; + } + }else { +iommu_report_device_fault (master->dev, &event); + } +out: +dev_info (smmu->dev,"event 0x%02x received:\n" , type); +for (i =0 ; i <EVTQ_ENT_DWORDS ; ++i) { +dev_info (smmu->dev,"\t0x%016llx\n" , + (unsigned long long )evt[i]); + } +} +/* IRQ and event handlers */ static irqreturn_t arm_smmu_evtq_thread (int irq,void *dev) { -int i;struct arm_smmu_device *smmu = dev;struct arm_smmu_queue *q = &smmu->evtq.q;struct arm_smmu_ll_queue *llq = &q->llq;u64 evt[EVTQ_ENT_DWORDS ]; do { - while (!queue_remove_raw(q, evt)) { - u8 id = FIELD_GET(EVTQ_0_ID, evt[0]); - - dev_info(smmu->dev, "event 0x%02x received:\n", id); - for (i = 0; i < ARRAY_SIZE(evt); ++i) - dev_info(smmu->dev, "\t0x%016llx\n", - (unsigned long long)evt[i]); - - } + while (!queue_remove_raw(q, evt)) + arm_smmu_report_event(smmu, evt); /* * Not much we can do on overflow, so scream and pretend we're
Signature · 6 lines
2.20.1 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Quoted 24 lines · Eric Auger
This version fixes an issue observed by Shameer on an SMMU 3.2, when moving from dual stage config to stage 1 only config. The 2 high 64b of the STE now get reset. Otherwise, leaving the S2TTB set may cause a
C_BAD_STEerror.This series can be found at:
https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
(including the VFIO part)
The QEMU fellow series still can be found at:
https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6Users have expressed interest in that work and tested v9/v10:
- https://patchwork.kernel.org/cover/11039995/#23012381
- https://patchwork.kernel.org/cover/11039995/#23197235Background:
This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately.
The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindings
Quoted 9 lines
Then those capabilities gets implemented in the SMMUv3 driver.
The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE).
Thanks Eric
Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
1. no-sva works, where guest app directly use physical address via ioctl.
2. vSVA still not work, same as v10,
3. the v10 issue reported by Shameer has been solved, first start qemu with iommu=smmuv3, then start qemu without iommu=smmuv3
4. no-sva also works without iommu=smmuv3
Test details in https://docs.qq.com/doc/DRU5oR1NtUERseFNL
Thanks
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi Zhangfei,
Quoted 37 lines · Zhangfei Gao
Quoted 24 lines · Eric Auger
This version fixes an issue observed by Shameer on an SMMU 3.2, when moving from dual stage config to stage 1 only config. The 2 high 64b of the STE now get reset. Otherwise, leaving the S2TTB set may cause a
C_BAD_STEerror.This series can be found at:
https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
(including the VFIO part)
The QEMU fellow series still can be found at:
https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6Users have expressed interest in that work and tested v9/v10:
- https://patchwork.kernel.org/cover/11039995/#23012381
- https://patchwork.kernel.org/cover/11039995/#23197235Background:
This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately.
The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindingsQuoted 9 lines
Then those capabilities gets implemented in the SMMUv3 driver.
The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE).
Quoted 4 lines
Thanks Eric
Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
1. no-sva works, where guest app directly use physical address via ioctl.
Thank you for the testing. Glad it works for you.
Quoted 1 line
2. vSVA still not work, same as v10,
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Thanks
Eric
Quoted 8 lines
3. the v10 issue reported by Shameer has been solved, first start qemu with iommu=smmuv3, then start qemu without iommu=smmuv3
4. no-sva also works without iommu=smmuv3Test details in https://docs.qq.com/doc/DRU5oR1NtUERseFNL
Thanks
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Hi Eric,
Quoted 55 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 16 April 2020 08:45
To: Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; Shameerali Kolothum Thodi <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Zhangfei,
Quoted 39 lines · Zhangfei Gao
Quoted 24 lines · Eric Auger
This version fixes an issue observed by Shameer on an SMMU 3.2, when moving from dual stage config to stage 1 only config. The 2 high 64b of the STE now get reset. Otherwise, leaving the S2TTB set may cause a
C_BAD_STEerror.This series can be found at:
https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
(including the VFIO part)
The QEMU fellow series still can be found at:
https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6Users have expressed interest in that work and tested v9/v10:
- https://patchwork.kernel.org/cover/11039995/#23012381
- https://patchwork.kernel.org/cover/11039995/#23197235Background:
This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately.
The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindingsQuoted 9 lines
Then those capabilities gets implemented in the SMMUv3 driver.
The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE).
Thanks Eric
Quoted 7 lines
Quoted 2 lines
Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
1. no-sva works, where guest app directly use physical address via ioctl.Thank you for the testing. Glad it works for you.
Quoted 1 line
2. vSVA still not work, same as v10,
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Thanks for that. I have made an attempt to add the vSVA based on your v10 + JPBs sva patches. The host kernel and Qemu changes can be found here[1][2].
This basically adds multiple pasid support on top of your changes. I have done some basic sanity testing and we have some initial success with the zip vf dev on our D06 platform. Please note that the STALL event is not yet supported though, but works fine if we mlock() guest usr mem.
I also noted that Intel patches for vSVA has couple of changes in the vfio interfaces and hope there will be a convergence soon. Please let me know your plans of a respin of this series and see whether incorporating the changes for multiple pasid make sense or not for now.
Thanks,
Shameer
[1]https://github.com/hisilicon/qemu/tree/v4.2.0-2stage-rfcv6-vsva-prototype-v1 [2]https://github.com/hisilicon/kernel-dev/tree/vsva-prototype-host-v1
Quoted 11 lines
Thanks
Eric
Quoted 8 lines
3. the v10 issue reported by Shameer has been solved, first start qemu with iommu=smmuv3, then start qemu without iommu=smmuv3
4. no-sva also works without iommu=smmuv3Test details in https://docs.qq.com/doc/DRU5oR1NtUERseFNL
Thanks
Hi Eric,
Quoted 78 lines
-----Original Message-----
From: Shameerali Kolothum Thodi
Sent: 30 April 2020 10:38
To: 'Auger Eric' <[email protected]>; Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Eric,
Quoted 55 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 16 April 2020 08:45
To: Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; Shameerali Kolothum Thodi <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Zhangfei,
Quoted 39 lines · Zhangfei Gao
Quoted 24 lines · Eric Auger
This version fixes an issue observed by Shameer on an SMMU 3.2, when moving from dual stage config to stage 1 only config. The 2 high 64b of the STE now get reset. Otherwise, leaving the S2TTB set may cause a
C_BAD_STEerror.This series can be found at:
https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
(including the VFIO part)
The QEMU fellow series still can be found at:
https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6Users have expressed interest in that work and tested v9/v10:
- https://patchwork.kernel.org/cover/11039995/#23012381
- https://patchwork.kernel.org/cover/11039995/#23197235Background:
This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately.
The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindingsQuoted 9 lines
Then those capabilities gets implemented in the SMMUv3 driver.
The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE).
Thanks Eric
Quoted 7 lines
Quoted 2 lines
Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
1. no-sva works, where guest app directly use physical address via ioctl.Thank you for the testing. Glad it works for you.
Quoted 1 line
2. vSVA still not work, same as v10,
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Quoted 8 lines
Thanks for that. I have made an attempt to add the vSVA based on your v10 + JPBs sva patches. The host kernel and Qemu changes can be found here[1][2].
This basically adds multiple pasid support on top of your changes. I have done some basic sanity testing and we have some initial success with the zip vf dev on our D06 platform. Please note that the STALL event is not yet supported though, but works fine if we
mlock()guest usr mem.
I have added STALL support for our vSVA prototype and it seems to be working(on our hardware). I have updated the kernel and qemu branches with the same[1][2]. I should warn you though that these are prototype code and I am pretty much re-using the VFIO_IOMMU_SET_PASID_TABLE interface for almost everything. But thought of sharing, in case if it is useful somehow!.
Thanks,
Shameer
[1]https://github.com/hisilicon/kernel-dev/commits/vsva-prototype-host-v1
[2]https://github.com/hisilicon/qemu/tree/v4.2.0-2stage-rfcv6-vsva-prototype-v1
Hi Shameer,
Quoted 89 lines · Shameerali Kolothum Thodi
Hi Eric,
Quoted 78 lines
-----Original Message-----
From: Shameerali Kolothum Thodi
Sent: 30 April 2020 10:38
To: 'Auger Eric' <[email protected]>; Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Eric,
Quoted 55 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 16 April 2020 08:45
To: Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; Shameerali Kolothum Thodi <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Zhangfei,
Quoted 39 lines · Zhangfei Gao
Quoted 24 lines · Eric Auger
This version fixes an issue observed by Shameer on an SMMU 3.2, when moving from dual stage config to stage 1 only config. The 2 high 64b of the STE now get reset. Otherwise, leaving the S2TTB set may cause a
C_BAD_STEerror.This series can be found at:
https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
(including the VFIO part)
The QEMU fellow series still can be found at:
https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6Users have expressed interest in that work and tested v9/v10:
- https://patchwork.kernel.org/cover/11039995/#23012381
- https://patchwork.kernel.org/cover/11039995/#23197235Background:
This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately.
The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindingsQuoted 9 lines
Then those capabilities gets implemented in the SMMUv3 driver.
The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE).
Thanks Eric
Quoted 7 lines
Quoted 2 lines
Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
1. no-sva works, where guest app directly use physical address via ioctl.Thank you for the testing. Glad it works for you.
Quoted 1 line
2. vSVA still not work, same as v10,
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Quoted 8 lines
Thanks for that. I have made an attempt to add the vSVA based on your v10 + JPBs sva patches. The host kernel and Qemu changes can be found here[1][2].
This basically adds multiple pasid support on top of your changes. I have done some basic sanity testing and we have some initial success with the zip vf dev on our D06 platform. Please note that the STALL event is not yet supported though, but works fine if we
mlock()guest usr mem.
Quoted 5 lines
I have added STALL support for our vSVA prototype and it seems to be working(on our hardware). I have updated the kernel and qemu branches with the same[1][2]. I should warn you though that these are prototype code and I am pretty much re-using the
VFIO_IOMMU_SET_PASID_TABLEinterface for almost everything. But thought of sharing, in case if it is useful somehow!.
Thank you very much for your work. I intend to look at your additions by beginning of next week.
Best Regards
Eric
Hi Shameer,
Quoted 89 lines · Shameerali Kolothum Thodi
Hi Eric,
Quoted 78 lines
-----Original Message-----
From: Shameerali Kolothum Thodi
Sent: 30 April 2020 10:38
To: 'Auger Eric' <[email protected]>; Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Eric,
Quoted 55 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 16 April 2020 08:45
To: Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; Shameerali Kolothum Thodi <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Zhangfei,
Quoted 39 lines · Zhangfei Gao
Quoted 24 lines · Eric Auger
This version fixes an issue observed by Shameer on an SMMU 3.2, when moving from dual stage config to stage 1 only config. The 2 high 64b of the STE now get reset. Otherwise, leaving the S2TTB set may cause a
C_BAD_STEerror.This series can be found at:
https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
(including the VFIO part)
The QEMU fellow series still can be found at:
https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6Users have expressed interest in that work and tested v9/v10:
- https://patchwork.kernel.org/cover/11039995/#23012381
- https://patchwork.kernel.org/cover/11039995/#23197235Background:
This series brings the IOMMU part of HW nested paging support in the SMMUv3. The VFIO part is submitted separately.
The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindingsQuoted 9 lines
Then those capabilities gets implemented in the SMMUv3 driver.
The virtualizer passes information through the VFIO user API which cascades them to the iommu subsystem. This allows the guest to own stage 1 tables and context descriptors (so-called PASID table) while the host owns stage 2 tables and main configuration structures (STE).
Thanks Eric
Quoted 7 lines
Quoted 2 lines
Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
1. no-sva works, where guest app directly use physical address via ioctl.Thank you for the testing. Glad it works for you.
Quoted 1 line
2. vSVA still not work, same as v10,
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Quoted 8 lines
Thanks for that. I have made an attempt to add the vSVA based on your v10 + JPBs sva patches. The host kernel and Qemu changes can be found here[1][2].
This basically adds multiple pasid support on top of your changes. I have done some basic sanity testing and we have some initial success with the zip vf dev on our D06 platform. Please note that the STALL event is not yet supported though, but works fine if we
mlock()guest usr mem.
Quoted 5 lines
I have added STALL support for our vSVA prototype and it seems to be working(on our hardware). I have updated the kernel and qemu branches with the same[1][2]. I should warn you though that these are prototype code and I am pretty much re-using the
VFIO_IOMMU_SET_PASID_TABLEinterface for almost everything. But thought of sharing, in case if it is useful somehow!.
Thank you again for sharing the POC. I looked at the kernel and QEMU branches.
Here are some preliminary comments:
- "arm-smmu-v3: Reset S2TTB while switching back from nested stage": as you mentionned S2TTB reset now is featured in v11
- "arm-smmu-v3: Add support for multiple pasid in nested mode": I could easily integrate this into my series. Update the iommu api first and pass multiple CD info in a separate patch
- "arm-smmu-v3: Add support to Invalidate CD": CD invalidation should be cascaded to host through the PASID cache invalidation uapi (no pb you warned us for the POC you simply used VFIO_IOMMU_SET_PASID_TABLE). I think I should add this support in my original series although it does not seem to trigger any issue up to now.
- "arm-smmu-v3: Remove duplication of fault propagation". I understand the transcode is done somewhere else with SVA but we still need to do it if a single CD is used, right? I will review the SVA code to better understand.
- for the STALL response injection I would tend to use a new VFIO region for responses. At the moment there is a single VFIO region for reporting the fault.
On QEMU side:
- I am currently working on 3.2 range invalidation support which is needed for DPDK/VFIO
- While at it I will look at how to incrementally introduce some of the features you need in this series.
Thanks
Eric
Hi Eric,
Quoted 13 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 13 May 2020 14:29
To: Shameerali Kolothum Thodi <[email protected]>; Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)
[...]
Quoted 20 lines
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Thanks for that. I have made an attempt to add the vSVA based on your v10 + JPBs sva patches. The host kernel and Qemu changes can be found here[1][2].
Quoted 7 lines
Quoted 4 lines
This basically adds multiple pasid support on top of your changes. I have done some basic sanity testing and we have some initial success with the zip vf dev on our D06 platform. Please note that the STALL event is not yet supported though, but works fine if we
mlock()guest usr mem.I have added STALL support for our vSVA prototype and it seems to be working(on our hardware). I have updated the kernel and qemu branches
with
Quoted 1 line
the same[1][2]. I should warn you though that these are prototype code and I
am pretty
Quoted 1 line
much re-using the
VFIO_IOMMU_SET_PASID_TABLEinterface for almosteverything.
Quoted 8 lines
Quoted 1 line
But thought of sharing, in case if it is useful somehow!.
Thank you again for sharing the POC. I looked at the kernel and QEMU branches.
Here are some preliminary comments:
- "arm-smmu-v3: Reset S2TTB while switching back from nested stage": as you mentionned S2TTB reset now is featured in v11
Yes.
Quoted 3 lines
- "arm-smmu-v3: Add support for multiple pasid in nested mode": I could easily integrate this into my series. Update the iommu api first and pass multiple CD info in a separate patch
Ok.
Quoted 5 lines
- "arm-smmu-v3: Add support to Invalidate CD": CD invalidation should be cascaded to host through the PASID cache invalidation uapi (no pb you warned us for the POC you simply used
VFIO_IOMMU_SET_PASID_TABLE). I think I should add this support in my original series although it does not seem to trigger any issue up to now.
Agree. Cache invalidation uapi is a better interface for this. Also I don’t think this matters for non-vsva cases as Guest kernel table/CD(pasid 0) will never get invalidated.
Quoted 4 lines
- "arm-smmu-v3: Remove duplication of fault propagation". I understand the transcode is done somewhere else with SVA but we still need to do it if a single CD is used, right? I will review the SVA code to better understand.
Hmm..not sure. Need to take another look to see whether we need a special handling for single CD or not.
Quoted 3 lines
- for the STALL response injection I would tend to use a new VFIO region for responses. At the moment there is a single VFIO region for reporting the fault.
Sure. That will be much cleaner and probably improve the context switch latency. Another thing I noted with STALL is that pasid_valid flag needs to be taken care in the SVA kernel path.
"iommu: Remove pasid validity check for STALL model page response msg" Not sure this one is a proper way to handle this.
Quoted 5 lines
On QEMU side:
- I am currently working on 3.2 range invalidation support which is needed for DPDK/VFIO
- While at it I will look at how to incrementally introduce some of the features you need in this series.
Ok.
Thanks for taking a look at the POC.
Cheers,
Shameer
Hi Shameer,
Quoted 46 lines · Shameerali Kolothum Thodi
Hi Eric,
Quoted 13 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 13 May 2020 14:29
To: Shameerali Kolothum Thodi <[email protected]>; Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)[...]
Quoted 20 lines
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Thanks for that. I have made an attempt to add the vSVA based on your v10 + JPBs sva patches. The host kernel and Qemu changes can be found here[1][2].
Quoted 7 lines
Quoted 4 lines
This basically adds multiple pasid support on top of your changes. I have done some basic sanity testing and we have some initial success with the zip vf dev on our D06 platform. Please note that the STALL event is not yet supported though, but works fine if we
mlock()guest usr mem.I have added STALL support for our vSVA prototype and it seems to be working(on our hardware). I have updated the kernel and qemu branches
with
Quoted 1 line
the same[1][2]. I should warn you though that these are prototype code and I
am pretty
Quoted 1 line
much re-using the
VFIO_IOMMU_SET_PASID_TABLEinterface for almosteverything.
Quoted 8 lines
Quoted 1 line
But thought of sharing, in case if it is useful somehow!.
Thank you again for sharing the POC. I looked at the kernel and QEMU branches.
Here are some preliminary comments:
- "arm-smmu-v3: Reset S2TTB while switching back from nested stage": as you mentionned S2TTB reset now is featured in v11
Quoted 7 lines
Yes.
Quoted 3 lines
- "arm-smmu-v3: Add support for multiple pasid in nested mode": I could easily integrate this into my series. Update the iommu api first and pass multiple CD info in a separate patch
Ok.
in v12, I added
[PATCH v12 14/15] iommu/smmuv3: Accept configs with more than one context descriptor
I don't think you need to add s1cdmax addition as we already have pasid_bits which should do the job.
Quoted 9 lines
Quoted 5 lines
- "arm-smmu-v3: Add support to Invalidate CD": CD invalidation should be cascaded to host through the PASID cache invalidation uapi (no pb you warned us for the POC you simply used
VFIO_IOMMU_SET_PASID_TABLE). I think I should add this support in my original series although it does not seem to trigger any issue up to now.Agree. Cache invalidation uapi is a better interface for this. Also I don’t think this matters for non-vsva cases as Guest kernel table/CD(pasid 0) will never get invalidated.
in v12 I added [PATCH v12 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID. I have not tested it though.
Quoted 5 lines
Quoted 5 lines
- "arm-smmu-v3: Remove duplication of fault propagation". I understand the transcode is done somewhere else with SVA but we still need to do it if a single CD is used, right? I will review the SVA code to better understand.
Since I have rebase on 5.10-rc4 you will still have this duplication to handle.
Quoted 7 lines
Hmm..not sure. Need to take another look to see whether we need a special handling for single CD or not.
Quoted 3 lines
- for the STALL response injection I would tend to use a new VFIO region for responses. At the moment there is a single VFIO region for reporting the fault.
in v12 I added a new VFIO region to inject your fault. This was tested with dummy event injection, this should work properly.
If we clearly identify all the public dependencies needed for vSVA/ARM I can help you respinning on top of them
Thanks
Eric
Quoted 14 lines
Sure. That will be much cleaner and probably improve the context switch latency. Another thing I noted with STALL is that
pasid_validflag needs to be taken care in the SVA kernel path."iommu: Remove pasid validity check for STALL model page response msg" Not sure this one is a proper way to handle this.
Quoted 5 lines
On QEMU side:
- I am currently working on 3.2 range invalidation support which is needed for DPDK/VFIO
- While at it I will look at how to incrementally introduce some of the features you need in this series.
Quoted 7 lines
Ok.
Thanks for taking a look at the POC.
Cheers,
Shameer
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi Eric,
First, many thanks for the respin. I will go through all of these(iommu/vfio/Qemu) and will do a thorough verification/tests on our hardware.
Quoted 72 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 17 November 2020 08:40
To: Shameerali Kolothum Thodi <[email protected]>; Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)Hi Shameer,
Quoted 12 lines · Shameerali Kolothum Thodi
Hi Eric,
Quoted 10 lines
-----Original Message-----
From: Auger Eric [mailto:[email protected]]
Sent: 13 May 2020 14:29
To: Shameerali Kolothum Thodi <[email protected]>; Zhangfei Gao <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected];Quoted 9 lines
Quoted 3 lines
[email protected]; [email protected]
Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)[...]
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Yes that's normal this series is not meant to support vSVM at this stage.
I intend to add the missing pieces during the next weeks.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Thanks for that. I have made an attempt to add the vSVA based on your v10 + JPBs sva patches. The host kernel and Qemu changes can be found here[1][2].
This basically adds multiple pasid support on top of your changes. I have done some basic sanity testing and we have some initial success with the zip vf dev on our D06 platform. Please note that the STALL event
is
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
not yet supported though, but works fine if we
mlock()guest usr mem.I have added STALL support for our vSVA prototype and it seems to be working(on our hardware). I have updated the kernel and qemu branches
with
Quoted 1 line
the same[1][2]. I should warn you though that these are prototype code
and I
Quoted 10 lines
Quoted 10 lines
am pretty
Quoted 1 line
much re-using the
VFIO_IOMMU_SET_PASID_TABLEinterface for almosteverything.
Quoted 1 line
But thought of sharing, in case if it is useful somehow!.
Thank you again for sharing the POC. I looked at the kernel and QEMU branches.
Here are some preliminary comments:
- "arm-smmu-v3: Reset S2TTB while switching back from nested stage":as
Quoted 8 lines
Quoted 1 line
you mentionned S2TTB reset now is featured in v11
Yes.
Quoted 3 lines
- "arm-smmu-v3: Add support for multiple pasid in nested mode": I could easily integrate this into my series. Update the iommu api first and pass multiple CD info in a separate patch
Quoted 7 lines
Quoted 1 line
Ok.
in v12, I added
[PATCH v12 14/15] iommu/smmuv3: Accept configs with more than one context descriptorI don't think you need to add s1cdmax addition as we already have
pasid_bitswhich should do the job.
Ok.
Quoted 11 lines
Quoted 9 lines
Quoted 5 lines
- "arm-smmu-v3: Add support to Invalidate CD": CD invalidation should be cascaded to host through the PASID cache invalidation uapi (no pb you warned us for the POC you simply used
VFIO_IOMMU_SET_PASID_TABLE). I think I should add this support in my original series although it does not seem to trigger any issue up to now.Agree. Cache invalidation uapi is a better interface for this. Also I don’t think this matters for non-vsva cases as Guest kernel table/CD(pasid 0) will never get invalidated.
in v12 I added [PATCH v12 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID. I have not tested it though.
Ok. Will verify this.
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
- "arm-smmu-v3: Remove duplication of fault propagation". I understand the transcode is done somewhere else with SVA but we still need to do it if a single CD is used, right? I will review the SVA code to better understand.
Since I have rebase on 5.10-rc4 you will still have this duplication to handle.
OK.
Quoted 9 lines
Quoted 6 lines
Hmm..not sure. Need to take another look to see whether we need a special handling for single CD or not.
Quoted 3 lines
- for the STALL response injection I would tend to use a new VFIO region for responses. At the moment there is a single VFIO region for reporting the fault.
in v12 I added a new VFIO region to inject your fault. This was tested with dummy event injection, this should work properly.
That’s cool. I will check this.
Quoted 2 lines
If we clearly identify all the public dependencies needed for vSVA/ARM I can help you respinning on top of them
Sure. I will rebase the vSVA related changes on top of your series and then we can take it from there. Thanks for your support.
Thanks,
Shameer
Quoted 24 lines
Thanks
Eric
Quoted 21 lines
Sure. That will be much cleaner and probably improve the context switch latency. Another thing I noted with STALL is that
pasid_validflag needs to be taken care in the SVA kernel path."iommu: Remove pasid validity check for STALL model page response msg" Not sure this one is a proper way to handle this.
Quoted 5 lines
On QEMU side:
- I am currently working on 3.2 range invalidation support which is needed for DPDK/VFIO
- While at it I will look at how to incrementally introduce some of the features you need in this series.Ok.
Thanks for taking a look at the POC.
Cheers,
Shameer