[PATCH 0/2] Domain nesting info for arm-smmu

These couple of patches are adding nesting information for arm
and are based on the domain nesting info patches by Yi [1,2,3].

Based on the discussion in the thread [4], sending these out as
I have been using in my tree [5] for nested translation based
on virtio-iommu on Arm reference platforms.

Thanks & regards
Vivek

[1] https://lore.kernel.org/kvm/1599734733-6431-2-git-send-email-yi.l.liu@intel.com/
[2] https://lore.kernel.org/kvm/1599734733-6431-3-git-send-email-yi.l.liu@intel.com/
[3] https://lore.kernel.org/kvm/1599734733-6431-4-git-send-email-yi.l.liu@intel.com/
[4] https://lore.kernel.org/kvm/306e7dd2-9eb2-0ca3-6a93-7c9aa0821ce9@arm.com/
[5] https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-virtio-iommu

Vivek Gautam (2):
  iommu: Report domain nesting info for arm-smmu-v3
  iommu: arm-smmu-v3: Report domain nesting info reuqired for stage1

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 +++++++++--
 include/uapi/linux/iommu.h                  | 31 +++++++++++++++++----
 2 files changed, 39 insertions(+), 8 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Add a vendor specific structure for domain nesting info for
arm smmu-v3, and necessary info fields required to populate
stage1 page tables.

Signed-off-by: Vivek Gautam <redacted>
---
 include/uapi/linux/iommu.h | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 4d3d988fa353..5f059bcf7720 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h

@@ -323,7 +323,8 @@ struct iommu_gpasid_bind_data {
 #define IOMMU_GPASID_BIND_VERSION_1	1
 	__u32 version;
 #define IOMMU_PASID_FORMAT_INTEL_VTD	1
-#define IOMMU_PASID_FORMAT_LAST		2
+#define IOMMU_PASID_FORMAT_ARM_SMMU_V3	2
+#define IOMMU_PASID_FORMAT_LAST		3
 	__u32 format;
 	__u32 addr_width;
 #define IOMMU_SVA_GPASID_VAL	(1 << 0) /* guest PASID valid */

@@ -409,6 +410,21 @@ struct iommu_nesting_info_vtd {
 	__u64	ecap_reg;
 };
 
+/*
+ * struct iommu_nesting_info_arm_smmuv3 - Arm SMMU-v3 nesting info.
+ */
+struct iommu_nesting_info_arm_smmuv3 {
+	__u32	flags;
+	__u16	asid_bits;
+
+	/* Arm LPAE page table format as per kernel */
+#define ARM_PGTBL_32_LPAE_S1		(0x0)
+#define ARM_PGTBL_64_LPAE_S1		(0x2)
+	__u8	pgtbl_fmt;
+
+	__u8	padding[9];
+};
+
 /*
  * struct iommu_nesting_info - Information for nesting-capable IOMMU.
  *			       userspace should check it before using

@@ -445,11 +461,13 @@ struct iommu_nesting_info_vtd {
  * +---------------+------------------------------------------------------+
  *
  * data struct types defined for @format:
- * +================================+=====================================+
- * | @format                        | data struct                         |
- * +================================+=====================================+
- * | IOMMU_PASID_FORMAT_INTEL_VTD   | struct iommu_nesting_info_vtd       |
- * +--------------------------------+-------------------------------------+
+ * +================================+======================================+
+ * | @format                        | data struct                          |
+ * +================================+======================================+
+ * | IOMMU_PASID_FORMAT_INTEL_VTD   | struct iommu_nesting_info_vtd        |
+ * +---------------+-------------------------------------------------------+
+ * | IOMMU_PASID_FORMAT_ARM_SMMU_V3 | struct iommu_nesting_info_arm_smmuv3 |
+ * +--------------------------------+--------------------------------------+
  *
  */
 struct iommu_nesting_info {

@@ -466,6 +484,7 @@ struct iommu_nesting_info {
 	/* Vendor specific data */
 	union {
 		struct iommu_nesting_info_vtd vtd;
+		struct iommu_nesting_info_arm_smmuv3 smmuv3;
 	} vendor;
 };
 

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Update nested domain information required for stage1 page table.

Signed-off-by: Vivek Gautam <redacted>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index c11dd3940583..728018921fae 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -2555,6 +2555,7 @@ static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
 					void *data)
 {
 	struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	unsigned int size;
 
 	if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)

@@ -2571,9 +2572,20 @@ static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
 		return 0;
 	}
 
-	/* report an empty iommu_nesting_info for now */
-	memset(info, 0x0, size);
+	/* Update the nesting info as required for stage1 page tables */
+	info->addr_width = smmu->ias;
+	info->format = IOMMU_PASID_FORMAT_ARM_SMMU_V3;
+	info->features = IOMMU_NESTING_FEAT_BIND_PGTBL |
+			 IOMMU_NESTING_FEAT_PAGE_RESP |
+			 IOMMU_NESTING_FEAT_CACHE_INVLD;
+	info->pasid_bits = smmu->ssid_bits;
+	info->vendor.smmuv3.asid_bits = smmu->asid_bits;
+	info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
+	memset(&info->padding, 0x0, 12);
+	memset(&info->vendor.smmuv3.padding, 0x0, 9);
+
 	info->argsz = size;
+
 	return 0;
 }
 

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Vivek,
On 2/12/21 11:58 AM, Vivek Gautam wrote:

Shouldn't it be a bitfield instead as both can be supported (the actual
driver only supports 64b table format though). Does it match matches
IDR0.TTF?

+	__u8	pgtbl_fmt;

So I understand this API is supposed to allow VFIO to expose those info
early enough to the userspace to help configuring the viommu and avoid
errors later on. I wonder how far we want to go on this path. What about
those other caps that impact the STE/CD validity. There may be others...

SMMU_IDR0.CD2L (support of 2 stage CD)
SMMU_IDR0.TTENDIAN (endianness)
SMMU_IDR0.HTTU (if 0 forbids HA/HD setting in the CD)
SMMU_IDR3.STT (impacts T0SZ)

Thanks

Eric

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Vivek,

On 2/12/21 11:58 AM, Vivek Gautam wrote:

Update nested domain information required for stage1 page table.

s/reuqired/required in the commit title

I understood IOMMU_NESTING_FEAT_BIND_PGTBL advertises the requirement to
bind tables per PASID, ie. passing iommu_gpasid_bind_data.
In ARM case I guess you plan to use attach/detach_pasid_table API with
iommu_pasid_table_config struct. So I understood we should add a new
feature here.

+			 IOMMU_NESTING_FEAT_PAGE_RESP |
+			 IOMMU_NESTING_FEAT_CACHE_INVLD;
+	info->pasid_bits = smmu->ssid_bits;
+	info->vendor.smmuv3.asid_bits = smmu->asid_bits;
+	info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
+	memset(&info->padding, 0x0, 12);
+	memset(&info->vendor.smmuv3.padding, 0x0, 9);
+
 	info->argsz = size;
+

spurious new line

 	return 0;
 }
 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Vivek,

On 2/12/21 11:58 AM, Vivek Gautam wrote:

IOMMU_NESTING_FEAT_PAGE_RESP definition is missing too

Eric

+			 IOMMU_NESTING_FEAT_CACHE_INVLD;
+	info->pasid_bits = smmu->ssid_bits;
+	info->vendor.smmuv3.asid_bits = smmu->asid_bits;
+	info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
+	memset(&info->padding, 0x0, 12);
+	memset(&info->vendor.smmuv3.padding, 0x0, 9);
+
 	info->argsz = size;
+
 	return 0;
 }
 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Eric,

On Fri, Feb 12, 2021 at 11:44 PM Auger Eric [off-list ref] wrote:

Hi Vivek,

On 2/12/21 11:58 AM, Vivek Gautam wrote:

quoted

Update nested domain information required for stage1 page table.

s/reuqired/required in the commit title

Oh! my bad.

quoted

Signed-off-by: Vivek Gautam <redacted>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index c11dd3940583..728018921fae 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -2555,6 +2555,7 @@ static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
                                      void *data)
 {
      struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
+     struct arm_smmu_device *smmu = smmu_domain->smmu;
      unsigned int size;

      if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)

@@ -2571,9 +2572,20 @@ static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
              return 0;
      }

-     /* report an empty iommu_nesting_info for now */
-     memset(info, 0x0, size);
+     /* Update the nesting info as required for stage1 page tables */
+     info->addr_width = smmu->ias;
+     info->format = IOMMU_PASID_FORMAT_ARM_SMMU_V3;
+     info->features = IOMMU_NESTING_FEAT_BIND_PGTBL |

I understood IOMMU_NESTING_FEAT_BIND_PGTBL advertises the requirement to
bind tables per PASID, ie. passing iommu_gpasid_bind_data.
In ARM case I guess you plan to use attach/detach_pasid_table API with
iommu_pasid_table_config struct. So I understood we should add a new
feature here.

Right, the idea is to let vfio know that we support pasid table binding, and
I thought we could use the same flag. But clearly that's not the case.
Will add a new feature.

quoted

+                      IOMMU_NESTING_FEAT_PAGE_RESP |
+                      IOMMU_NESTING_FEAT_CACHE_INVLD;
+     info->pasid_bits = smmu->ssid_bits;
+     info->vendor.smmuv3.asid_bits = smmu->asid_bits;
+     info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
+     memset(&info->padding, 0x0, 12);
+     memset(&info->vendor.smmuv3.padding, 0x0, 9);
+
      info->argsz = size;
+

spurious new line

Sure, will remove it.

Best regards
Vivek

quoted

      return 0;
 }

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Eric,


On 2/12/21 11:43 PM, Auger Eric wrote:

Hi Vivek,
On 2/12/21 11:58 AM, Vivek Gautam wrote:

quoted

Add a vendor specific structure for domain nesting info for
arm smmu-v3, and necessary info fields required to populate
stage1 page tables.

Signed-off-by: Vivek Gautam <redacted>
---
  include/uapi/linux/iommu.h | 31 +++++++++++++++++++++++++------
  1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 4d3d988fa353..5f059bcf7720 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h

@@ -323,7 +323,8 @@ struct iommu_gpasid_bind_data {
  #define IOMMU_GPASID_BIND_VERSION_1	1
  	__u32 version;
  #define IOMMU_PASID_FORMAT_INTEL_VTD	1
-#define IOMMU_PASID_FORMAT_LAST		2
+#define IOMMU_PASID_FORMAT_ARM_SMMU_V3	2
+#define IOMMU_PASID_FORMAT_LAST		3
  	__u32 format;
  	__u32 addr_width;
  #define IOMMU_SVA_GPASID_VAL	(1 << 0) /* guest PASID valid */

@@ -409,6 +410,21 @@ struct iommu_nesting_info_vtd {
  	__u64	ecap_reg;
  };
  
+/*
+ * struct iommu_nesting_info_arm_smmuv3 - Arm SMMU-v3 nesting info.
+ */
+struct iommu_nesting_info_arm_smmuv3 {
+	__u32	flags;
+	__u16	asid_bits;
+
+	/* Arm LPAE page table format as per kernel */
+#define ARM_PGTBL_32_LPAE_S1		(0x0)
+#define ARM_PGTBL_64_LPAE_S1		(0x2)

Thanks for reviewing and I am terribly sorry for coming to it with delay.

Shouldn't it be a bitfield instead as both can be supported (the actual
driver only supports 64b table format though). Does it match matches
IDR0.TTF?

Yes, it should be a bitfield rather, and it doesn't match with IDR0.TTF. 
This is
  to hint the stage1 table allocations from viommu.
  Please see viommu_setup_pgtable() in the patch at [1].

quoted

+	__u8	pgtbl_fmt;

So I understand this API is supposed to allow VFIO to expose those info
early enough to the userspace to help configuring the viommu and avoid
errors later on. I wonder how far we want to go on this path. What about
those other caps that impact the STE/CD validity. There may be others...

SMMU_IDR0.CD2L (support of 2 stage CD)
SMMU_IDR0.TTENDIAN (endianness)
SMMU_IDR0.HTTU (if 0 forbids HA/HD setting in the CD)
SMMU_IDR3.STT (impacts T0SZ)

Right. The idea was to start with a minimal set of configuration.

But as you rightly pointed out we need a scalable solution to this problem

for arm-smmu-v3. I am now thinking if we could even use the nesting_info

for arm. We don't want to end up adding flags for all the feature bits.

Let me know if you have any suggestions.

Best regards
Vivek

[1] 
https://lore.kernel.org/linux-arm-kernel/20210115121342.15093-14-vivek.gautam@arm.com/

[snip]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Eric,


On 2/12/21 11:43 PM, Auger Eric wrote:

Hi Vivek,

On 2/12/21 11:58 AM, Vivek Gautam wrote:

quoted

Update nested domain information required for stage1 page table.

s/reuqired/required in the commit title

My bad! Will correct it.

quoted

Signed-off-by: Vivek Gautam <redacted>
---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++++++++++++++--
  1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index c11dd3940583..728018921fae 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -2555,6 +2555,7 @@ static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
  					void *data)
  {
  	struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
  	unsigned int size;
  
  	if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)

@@ -2571,9 +2572,20 @@ static int arm_smmu_domain_nesting_info(struct arm_smmu_domain *smmu_domain,
  		return 0;
  	}
  
-	/* report an empty iommu_nesting_info for now */
-	memset(info, 0x0, size);
+	/* Update the nesting info as required for stage1 page tables */
+	info->addr_width = smmu->ias;
+	info->format = IOMMU_PASID_FORMAT_ARM_SMMU_V3;
+	info->features = IOMMU_NESTING_FEAT_BIND_PGTBL |

I understood IOMMU_NESTING_FEAT_BIND_PGTBL advertises the requirement to
bind tables per PASID, ie. passing iommu_gpasid_bind_data.
In ARM case I guess you plan to use attach/detach_pasid_table API with
iommu_pasid_table_config struct. So I understood we should add a new
feature here.

Right, the idea is to let vfio know that we support pasid table binding, and
  I thought we could use the same flag. But clearly that's not the case.
  I will add a new feature.

quoted

+			 IOMMU_NESTING_FEAT_PAGE_RESP |
+			 IOMMU_NESTING_FEAT_CACHE_INVLD;
+	info->pasid_bits = smmu->ssid_bits;
+	info->vendor.smmuv3.asid_bits = smmu->asid_bits;
+	info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
+	memset(&info->padding, 0x0, 12);
+	memset(&info->vendor.smmuv3.padding, 0x0, 9);
+
  	info->argsz = size;
+

spurious new line

Sure, will correct this.

quoted

  	return 0;
  }
  

Thanks
Vivek

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel