Re: [PATCH RFC v1 15/15] iommu/virtio: Update fault type and reason info for viommu fault

2021-04-05 Thread Vivek Kumar Gautam




On 3/29/21 9:53 PM, Jean-Philippe Brucker wrote:

On Fri, Mar 12, 2021 at 06:39:05PM +0530, Vivek Kumar Gautam wrote:

To complete the page request we would also need to send the response back to
the host from virtio backend when handling page request. So the virtio
command should also be accompanied with a vfio api to send the page request
response back to the host. Isn't it?
This is where the host smmuv3 can send PRI_RESP command to the device to
complete the page fault.


It looks like Eric already has this in the VFIO series:
https://lore.kernel.org/linux-iommu/20210223210625.604517-14-eric.au...@redhat.com/


Right, I have taken this change to work on getting the vSVA with 
virtio-iommu.
For this I am adding a new request for virtio-iomm - 
VIRTIO_IOMMU_T_PAGE_RESP, and related struct virtio_iommu_req_page_resp 
that would contain information such as, pasid, grpid, response_code, 
flags, and endpoint. This is inline with struct iommu_page_response.

I will post out the patches for this soon.

Thanks & regards
Vivek



Thanks,
Jean



Re: [PATCH RFC v1 13/15] iommu/virtio: Attach Arm PASID tables when available

2021-03-12 Thread Vivek Kumar Gautam




On 3/3/21 10:55 PM, Jean-Philippe Brucker wrote:

On Fri, Jan 15, 2021 at 05:43:40PM +0530, Vivek Gautam wrote:
[...]

+static int viommu_setup_pgtable(struct viommu_endpoint *vdev,
+   struct viommu_domain *vdomain)
+{
+   int ret, id;
+   u32 asid;
+   enum io_pgtable_fmt fmt;
+   struct io_pgtable_ops *ops = NULL;
+   struct viommu_dev *viommu = vdev->viommu;
+   struct virtio_iommu_probe_table_format *desc = vdev->pgtf;
+   struct iommu_pasid_table *tbl = vdomain->pasid_tbl;
+   struct iommu_vendor_psdtable_cfg *pst_cfg;
+   struct arm_smmu_cfg_info *cfgi;
+   struct io_pgtable_cfg cfg = {
+   .iommu_dev  = viommu->dev->parent,
+   .tlb= &viommu_flush_ops,
+   .pgsize_bitmap  = vdev->pgsize_mask ? vdev->pgsize_mask :
+ vdomain->domain.pgsize_bitmap,
+   .ias= (vdev->input_end ? ilog2(vdev->input_end) :
+  
ilog2(vdomain->domain.geometry.aperture_end)) + 1,
+   .oas= vdev->output_bits,
+   };
+
+   if (!desc)
+   return -EINVAL;
+
+   if (!vdev->output_bits)
+   return -ENODEV;
+
+   switch (le16_to_cpu(desc->format)) {
+   case VIRTIO_IOMMU_FOMRAT_PGTF_ARM_LPAE:
+   fmt = ARM_64_LPAE_S1;
+   break;
+   default:
+   dev_err(vdev->dev, "unsupported page table format 0x%x\n",
+   le16_to_cpu(desc->format));
+   return -EINVAL;
+   }
+
+   if (vdomain->mm.ops) {
+   /*
+* TODO: attach additional endpoint to the domain. Check that
+* the config is sane.
+*/
+   return -EEXIST;
+   }
+
+   vdomain->mm.domain = vdomain;
+   ops = alloc_io_pgtable_ops(fmt, &cfg, &vdomain->mm);
+   if (!ops)
+   return -ENOMEM;
+
+   pst_cfg = &tbl->cfg;
+   cfgi = &pst_cfg->vendor.cfg;
+   id = ida_simple_get(&asid_ida, 1, 1 << desc->asid_bits, GFP_KERNEL);
+   if (id < 0) {
+   ret = id;
+   goto err_free_pgtable;
+   }
+
+   asid = id;
+   ret = iommu_psdtable_prepare(tbl, pst_cfg, &cfg, asid);
+   if (ret)
+   goto err_free_asid;
+
+   /*
+* Strange to setup an op here?
+* cd-lib is the actual user of sync op, and therefore the platform
+* drivers should assign this sync/maintenance ops as per need.
+*/
+   tbl->ops->sync = viommu_flush_pasid;


But this function deals with page tables, not pasid tables. As said on an
earlier patch, the TLB flush ops should probably be passed during table
registration - those ops are global so should really be const.


Right, will amend it.




+
+   /* Right now only PASID 0 supported ?? */
+   ret = iommu_psdtable_write(tbl, pst_cfg, 0, &cfgi->s1_cfg->cd);
+   if (ret)
+   goto err_free_asid;
+
+   vdomain->mm.ops = ops;
+   dev_dbg(vdev->dev, "using page table format 0x%x\n", fmt);
+
+   return 0;
+
+err_free_asid:
+   ida_simple_remove(&asid_ida, asid);
+err_free_pgtable:
+   free_io_pgtable_ops(ops);
+   return ret;
+}
+
+static int viommu_config_arm_pst(struct iommu_vendor_psdtable_cfg *pst_cfg,
+struct virtio_iommu_req_attach_pst_arm *req)
+{
+   struct arm_smmu_s1_cfg *s1_cfg = pst_cfg->vendor.cfg.s1_cfg;
+
+   if (!s1_cfg)
+   return -ENODEV;
+
+   req->format  = cpu_to_le16(VIRTIO_IOMMU_FORMAT_PSTF_ARM_SMMU_V3);
+   req->s1fmt   = s1_cfg->s1fmt;
+   req->s1dss   = VIRTIO_IOMMU_PSTF_ARM_SMMU_V3_DSS_0;
+   req->s1contextptr = cpu_to_le64(pst_cfg->base);
+   req->s1cdmax = cpu_to_le32(s1_cfg->s1cdmax);
+
+   return 0;
+}
+
+static int viommu_config_pst(struct iommu_vendor_psdtable_cfg *pst_cfg,
+void *req, enum pasid_table_fmt fmt)
+{
+   int ret;
+
+   switch (fmt) {
+   case PASID_TABLE_ARM_SMMU_V3:
+   ret = viommu_config_arm_pst(pst_cfg, req);
+   break;
+   default:
+   ret = -EINVAL;
+   WARN_ON(1);
+   }
+
+   return ret;
+}
+
+static int viommu_prepare_arm_pst(struct viommu_endpoint *vdev,
+ struct iommu_vendor_psdtable_cfg *pst_cfg)
+{
+   struct virtio_iommu_probe_table_format *pgtf = vdev->pgtf;
+   struct arm_smmu_cfg_info *cfgi = &pst_cfg->vendor.cfg;
+   struct arm_smmu_s1_cfg *cfg;
+
+   /* Some sanity checks */
+   if (pgtf->asid_bits != 8 && pgtf->asid_bits != 16)
+   return -EINVAL;


No need for this, next patch cheks asid size in viommu_config_arm_pgt()


Right, thanks for catching.




+
+   cfg = devm_kzalloc(pst_cfg->iommu_dev, sizeof(cfg), GFP_KERNEL);
+   if (!cfg)
+   return -ENOMEM;
+
+   cfgi-

Re: [PATCH RFC v1 15/15] iommu/virtio: Update fault type and reason info for viommu fault

2021-03-12 Thread Vivek Kumar Gautam




On 3/3/21 10:55 PM, Jean-Philippe Brucker wrote:

On Fri, Jan 15, 2021 at 05:43:42PM +0530, Vivek Gautam wrote:

Fault type information can tell about a page request fault or
an unreceoverable fault, and further additions to fault reasons
and the related PASID information can help in handling faults
efficiently.

Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Michael S. Tsirkin 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  drivers/iommu/virtio-iommu.c  | 27 +--
  include/uapi/linux/virtio_iommu.h | 13 -
  2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 9cc3d35125e9..10ef9e98214a 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -652,9 +652,16 @@ static int viommu_fault_handler(struct viommu_dev *viommu,
char *reason_str;
  
  	u8 reason	= fault->reason;

+   u16 type= fault->flt_type;
u32 flags   = le32_to_cpu(fault->flags);
u32 endpoint= le32_to_cpu(fault->endpoint);
u64 address = le64_to_cpu(fault->address);
+   u32 pasid   = le32_to_cpu(fault->pasid);
+
+   if (type == VIRTIO_IOMMU_FAULT_F_PAGE_REQ) {
+   dev_info(viommu->dev, "Page request fault - unhandled\n");
+   return 0;
+   }
  
  	switch (reason) {

case VIRTIO_IOMMU_FAULT_R_DOMAIN:
@@ -663,6 +670,21 @@ static int viommu_fault_handler(struct viommu_dev *viommu,
case VIRTIO_IOMMU_FAULT_R_MAPPING:
reason_str = "page";
break;
+   case VIRTIO_IOMMU_FAULT_R_WALK_EABT:
+   reason_str = "page walk external abort";
+   break;
+   case VIRTIO_IOMMU_FAULT_R_PTE_FETCH:
+   reason_str = "pte fetch";
+   break;
+   case VIRTIO_IOMMU_FAULT_R_PERMISSION:
+   reason_str = "permission";
+   break;
+   case VIRTIO_IOMMU_FAULT_R_ACCESS:
+   reason_str = "access";
+   break;
+   case VIRTIO_IOMMU_FAULT_R_OOR_ADDRESS:
+   reason_str = "output address";
+   break;
case VIRTIO_IOMMU_FAULT_R_UNKNOWN:
default:
reason_str = "unknown";
@@ -671,8 +693,9 @@ static int viommu_fault_handler(struct viommu_dev *viommu,
  
  	/* TODO: find EP by ID and report_iommu_fault */

if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
-   dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx 
[%s%s%s]\n",
-   reason_str, endpoint, address,
+   dev_err_ratelimited(viommu->dev,
+   "%s fault from EP %u PASID %u at %#llx 
[%s%s%s]\n",
+   reason_str, endpoint, pasid, address,
flags & VIRTIO_IOMMU_FAULT_F_READ ? "R" : 
"",
flags & VIRTIO_IOMMU_FAULT_F_WRITE ? "W" : 
"",
flags & VIRTIO_IOMMU_FAULT_F_EXEC ? "X" : 
"");
diff --git a/include/uapi/linux/virtio_iommu.h 
b/include/uapi/linux/virtio_iommu.h
index 608c8d642e1f..a537d82777f7 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -290,19 +290,30 @@ struct virtio_iommu_req_invalidate {
  #define VIRTIO_IOMMU_FAULT_R_UNKNOWN  0
  #define VIRTIO_IOMMU_FAULT_R_DOMAIN   1
  #define VIRTIO_IOMMU_FAULT_R_MAPPING  2
+#define VIRTIO_IOMMU_FAULT_R_WALK_EABT 3
+#define VIRTIO_IOMMU_FAULT_R_PTE_FETCH 4
+#define VIRTIO_IOMMU_FAULT_R_PERMISSION5
+#define VIRTIO_IOMMU_FAULT_R_ACCESS6
+#define VIRTIO_IOMMU_FAULT_R_OOR_ADDRESS   7
  
  #define VIRTIO_IOMMU_FAULT_F_READ		(1 << 0)

  #define VIRTIO_IOMMU_FAULT_F_WRITE(1 << 1)
  #define VIRTIO_IOMMU_FAULT_F_EXEC (1 << 2)
  #define VIRTIO_IOMMU_FAULT_F_ADDRESS  (1 << 8)
  
+#define VIRTIO_IOMMU_FAULT_F_DMA_UNRECOV	1

+#define VIRTIO_IOMMU_FAULT_F_PAGE_REQ  2


Currently all reported faults are unrecoverable, so to be consistent
DMA_UNRECOV should be 0. But I'd prefer having just a new "page request"
flag in the flags field, instead of the flt_type field.


Yea, looking at what I am currently trying as well - handle page-request 
and leave all other faults as unrecoverable - I will add the page 
request flag in the structure.




For page requests we'll also need a 16-bit fault ID field to store the PRI
"page request group index" or the stall "stag". "last" and "privileged"
flags as well, to match the PRI page request. And a new command to
complete a page fault.


Right, will add the fields as suggested.
To complete the page request we would also need to send the response 
back to the host from virtio backend 

Re: [PATCH RFC v1 09/15] iommu/virtio: Update table format probing header

2021-03-12 Thread Vivek Kumar Gautam




On 3/3/21 10:51 PM, Jean-Philippe Brucker wrote:

On Fri, Jan 15, 2021 at 05:43:36PM +0530, Vivek Gautam wrote:

Add info about asid_bits and additional flags to table format
probing header.

Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Michael S. Tsirkin 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  include/uapi/linux/virtio_iommu.h | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_iommu.h 
b/include/uapi/linux/virtio_iommu.h
index 43821e33e7af..8a0624bab4b2 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -169,7 +169,10 @@ struct virtio_iommu_probe_pasid_size {
  struct virtio_iommu_probe_table_format {
struct virtio_iommu_probe_property  head;
__le16  format;
-   __u8reserved[2];
+   __le16  asid_bits;
+
+   __le32  flags;


This struct should only contain the head and format fields. asid and flags
should go in a specialized structure - virtio_iommu_probe_pgt_arm64 in the
latest spec draft, where I dropped the asid_bits field in favor of an
"ASID16" flag.


Right, will take care of this looking at the spec draft.

Best regards
Vivek



Thanks,
Jean



Re: [PATCH RFC v1 08/15] iommu: Add asid_bits to arm smmu-v3 stage1 table info

2021-03-12 Thread Vivek Kumar Gautam




On 3/3/21 10:48 PM, Jean-Philippe Brucker wrote:

On Fri, Jan 15, 2021 at 05:43:35PM +0530, Vivek Gautam wrote:

aisd_bits data is required to prepare stage-1 tables for arm-smmu-v3.

Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  include/uapi/linux/iommu.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 082d758dd016..96abbfc7c643 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -357,7 +357,7 @@ struct iommu_pasid_smmuv3 {
__u32   version;
__u8s1fmt;
__u8s1dss;
-   __u8padding[2];
+   __u16   asid_bits;


Is this used anywhere?  This struct is passed from host userspace to host
kernel to attach the PASID table, so I don't think it needs an asid_bits
field.


Yea, must have missed removing it from the WIP work. Will remove it.

Thanks
Vivek


Re: [PATCH RFC v1 06/15] iommu/virtio: Add headers for table format probing

2021-03-12 Thread Vivek Kumar Gautam




On 3/3/21 10:47 PM, Jean-Philippe Brucker wrote:

On Fri, Jan 15, 2021 at 05:43:33PM +0530, Vivek Gautam wrote:

From: Jean-Philippe Brucker 

Add required UAPI defines for probing table format for underlying
iommu hardware. The device may provide information about hardware
tables and additional capabilities for each device.
This allows guest to correctly fabricate stage-1 page tables.

Signed-off-by: Jean-Philippe Brucker 
[Vivek: Use a single "struct virtio_iommu_probe_table_format" rather
 than separate structures for page table and pasid table format.


Makes sense. I've integrated that into the spec draft, added more precise
documentation and modified some of the definitions.

The current draft is here:
https://jpbrucker.net/virtio-iommu/spec/virtio-iommu-v0.13.pdf
Posted on the list here
https://lists.oasis-open.org/archives/virtio-dev/202102/msg00012.html


Thanks, I took an initial look, will review it this week.




Also update commit message.]
Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Michael S. Tsirkin 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  include/uapi/linux/virtio_iommu.h | 44 ++-
  1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_iommu.h 
b/include/uapi/linux/virtio_iommu.h
index 237e36a280cb..43821e33e7af 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -2,7 +2,7 @@
  /*
   * Virtio-iommu definition v0.12
   *
- * Copyright (C) 2019 Arm Ltd.
+ * Copyright (C) 2019-2021 Arm Ltd.


Not strictly necessary. But if you're modifying this comment please also
remove the "v0.12" above


Sure, let me keep the copyright year unchanged until we finalize the 
changes in draft spec.





   */
  #ifndef _UAPI_LINUX_VIRTIO_IOMMU_H
  #define _UAPI_LINUX_VIRTIO_IOMMU_H
@@ -111,6 +111,12 @@ struct virtio_iommu_req_unmap {
  
  #define VIRTIO_IOMMU_PROBE_T_NONE		0

  #define VIRTIO_IOMMU_PROBE_T_RESV_MEM 1
+#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK2
+#define VIRTIO_IOMMU_PROBE_T_INPUT_RANGE   3
+#define VIRTIO_IOMMU_PROBE_T_OUTPUT_SIZE   4
+#define VIRTIO_IOMMU_PROBE_T_PASID_SIZE5
+#define VIRTIO_IOMMU_PROBE_T_PAGE_TABLE_FMT6
+#define VIRTIO_IOMMU_PROBE_T_PASID_TABLE_FMT   7


Since there is a single struct we can have a single
VIRTIO_IOMMU_PROBE_T_TABLE_FORMAT.


Right, that would make sense.



  
  #define VIRTIO_IOMMU_PROBE_T_MASK		0xfff
  
@@ -130,6 +136,42 @@ struct virtio_iommu_probe_resv_mem {

__le64  end;
  };
  
+struct virtio_iommu_probe_page_size_mask {

+   struct virtio_iommu_probe_property  head;
+   __u8reserved[4];
+   __le64  mask;
+};
+
+struct virtio_iommu_probe_input_range {
+   struct virtio_iommu_probe_property  head;
+   __u8reserved[4];
+   __le64  start;
+   __le64  end;
+};
+
+struct virtio_iommu_probe_output_size {
+   struct virtio_iommu_probe_property  head;
+   __u8bits;
+   __u8reserved[3];
+};
+
+struct virtio_iommu_probe_pasid_size {
+   struct virtio_iommu_probe_property  head;
+   __u8bits;
+   __u8reserved[3];
+};
+
+/* Arm LPAE page table format */
+#define VIRTIO_IOMMU_FOMRAT_PGTF_ARM_LPAE  1


s/FOMRAT/FORMAT


Sure.




+/* Arm smmu-v3 type PASID table format */
+#define VIRTIO_IOMMU_FORMAT_PSTF_ARM_SMMU_V3   2


These should be with the Arm-specific definitions patches 11 and 14


Right, will add these definitions with Arm specific patches.

Best regards
Vivek

[snip]


Re: [PATCH RFC v1 05/15] iommu/arm-smmu-v3: Set sync op from consumer driver of cd-lib

2021-03-12 Thread Vivek Kumar Gautam




On 3/3/21 10:45 PM, Jean-Philippe Brucker wrote:

On Fri, Jan 15, 2021 at 05:43:32PM +0530, Vivek Gautam wrote:

Te change allows different consumers of arm-smmu-v3-cd-lib to set
their respective sync op for pasid entries.

Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 1 -
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c| 7 +++
  2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
index ec37476c8d09..acaa09acecdd 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
@@ -265,7 +265,6 @@ struct iommu_vendor_psdtable_ops arm_cd_table_ops = {
.free= arm_smmu_free_cd_tables,
.prepare = arm_smmu_prepare_cd,
.write   = arm_smmu_write_ctx_desc,
-   .sync= arm_smmu_sync_cd,
  };
  
  struct iommu_pasid_table *arm_smmu_register_cd_table(struct device *dev,

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 2f86c6ac42b6..0c644be22b4b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1869,6 +1869,13 @@ static int arm_smmu_domain_finalise_s1(struct 
arm_smmu_domain *smmu_domain,
if (ret)
goto out_free_cd_tables;
  
+	/*

+* Strange to setup an op here?
+* cd-lib is the actual user of sync op, and therefore the platform
+* drivers should assign this sync/maintenance ops as per need.
+*/
+   tbl->ops->sync = arm_smmu_sync_cd;
+


Modifying a static struct from here doesn't feel right. I think the
interface should be roughly similar to io-pgtable since the principle is
the same. So the sync() op should be separate from arm_cd_table_ops since
it's a callback into the driver. Maybe pass it to
iommu_register_pasid_table().


Sure, will take care of this.

Thanks & regards
Vivek


Re: [PATCH RFC v1 02/15] iommu: Add a simple PASID table library

2021-03-12 Thread Vivek Kumar Gautam

Hi Jean,


On 3/3/21 10:41 PM, Jean-Philippe Brucker wrote:

Hi Vivek,

Thanks again for working on this. I have a few comments but it looks
sensible overall.


Thanks a lot for reviewing the patch-series. Please find my responses 
inline below.




Regarding the overall design, I was initially assigning page directories
instead of whole PASID tables, which would simplify the driver and host
implementation. A major complication, however, is SMMUv3 accesses PASID
tables using a guest-physical address, so there is a messy negotiation
needed between host and guest when the host needs to allocate PASID
tables. Plus vSMMU needs PASID table assignment, so that's what the host
driver will implement.


By assigning the page directories, you mean setting up just the stage-1 
page table ops, and passing that information to the host using ATTACH_TABLE?
Right now when using kvmtool, the struct iommu_pasid_table_config is 
populated with the correct information, and this whole memory is mapped 
between host and guest by creating a mem bank using 
kvm__for_each_mem_bank().
Did I get you or did I fail terribly in understanding the point you are 
making here?

If it helps, I will publish my kvmtool branch.



On Fri, Jan 15, 2021 at 05:43:29PM +0530, Vivek Gautam wrote:

Add a small API in iommu subsystem to handle PASID table allocation
requests from different consumer drivers, such as a paravirtualized
iommu driver. The API provides ops for allocating and freeing PASID
table, writing to it and managing the table caches.

This library also provides for registering a vendor API that attaches
to these ops. The vendor APIs would eventually perform arch level
implementations for these PASID tables.


Although Arm might be the only vendor to ever use this, I think the
abstraction makes sense and isn't too invasive. Even if we called directly
into the SMMU driver from the virtio one, we'd still need patch 3 and
separate TLB invalidations ops.


Right, the idea was to make users of iommu-pasid-table - virtio-iommu or 
the arm-smmu-v3 - consistent. I also noticed that the whole process of 
allocating the pasid tables (or cd tables) and populating them with 
stage-1 page tables in viommu is also in-line with how things are in 
arm-smmu-v3 or atleast that's how the design can be in general - 
allocate pasid_table, and program stage-1 information into it, and then 
pass it across to host.





Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  drivers/iommu/iommu-pasid-table.h | 134 ++
  1 file changed, 134 insertions(+)
  create mode 100644 drivers/iommu/iommu-pasid-table.h

diff --git a/drivers/iommu/iommu-pasid-table.h 
b/drivers/iommu/iommu-pasid-table.h
new file mode 100644
index ..bd4f57656f67
--- /dev/null
+++ b/drivers/iommu/iommu-pasid-table.h
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PASID table management for the IOMMU
+ *
+ * Copyright (C) 2021 Arm Ltd.
+ */
+#ifndef __IOMMU_PASID_TABLE_H
+#define __IOMMU_PASID_TABLE_H
+
+#include 
+
+#include "arm/arm-smmu-v3/arm-smmu-v3.h"
+
+enum pasid_table_fmt {
+   PASID_TABLE_ARM_SMMU_V3,
+   PASID_TABLE_NUM_FMTS,
+};
+
+/**
+ * struct arm_smmu_cfg_info - arm-smmu-v3 specific configuration data
+ *
+ * @s1_cfg: arm-smmu-v3 stage1 config data
+ * @feat_flag: features supported by arm-smmu-v3 implementation
+ */
+struct arm_smmu_cfg_info {
+   struct arm_smmu_s1_cfg  *s1_cfg;
+   u32 feat_flag;
+};
+
+/**
+ * struct iommu_vendor_psdtable_cfg - Configuration data for PASID tables
+ *
+ * @iommu_dev: device performing the DMA table walks
+ * @fmt: The PASID table format
+ * @base: DMA address of the allocated table, set by the vendor driver
+ * @cfg: arm-smmu-v3 specific config data
+ */
+struct iommu_vendor_psdtable_cfg {
+   struct device   *iommu_dev;
+   enum pasid_table_fmtfmt;
+   dma_addr_t  base;
+   union {
+   struct arm_smmu_cfg_infocfg;


For the union to be extensible, that field should be called "arm" or
something like that.


Sure. Will amend this.

Thanks,
Vivek



Thanks,
Jean


+   } vendor;
+};
+
+struct iommu_vendor_psdtable_ops;
+
+/**
+ * struct iommu_pasid_table - describes a set of PASID tables
+ *
+ * @cookie: An opaque token provided by the IOMMU driver and passed back to any
+ * callback routine.
+ * @cfg: A copy of the PASID table configuration
+ * @ops: The PASID table operations in use for this set of page tables
+ */
+struct iommu_pasid_table {
+   void*cookie;
+   struct iommu_vendor_psdtable_cfgcfg;
+   struct iommu_vendor_psdtable_ops*ops;
+};
+
+#define pasid_table_cfg_to_table(pst_cfg) \
+   container_of((pst_cfg), struct iomm

Re: [PATCH RFC v1 04/15] iommu/arm-smmu-v3: Update CD base address info for user-space

2021-03-12 Thread Vivek Kumar Gautam

Hi Jean,

On 3/3/21 10:44 PM, Jean-Philippe Brucker wrote:

On Fri, Jan 15, 2021 at 05:43:31PM +0530, Vivek Gautam wrote:

Update base address information in vendor pasid table info to pass that
to user-space for stage1 table management.

Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
index 8a7187534706..ec37476c8d09 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
@@ -55,6 +55,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct 
iommu_vendor_psdtable_cfg *pst_cfg,
if (arm_smmu_alloc_cd_leaf_table(dev, l1_desc))
return NULL;
  
+		if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)

+   pst_cfg->base = l1_desc->l2ptr_dma;
+


This isn't the right place, because this path allocates second-level
tables for two-level tables. I don't think we need pst_cfg->base at all,
because for both linear and two-level tables, the base pointer is in
cdcfg->cdtab_dma, which can be read directly.


Sure, will remove this.



Thanks,
Jean


l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
/* An invalid L1CD can be cached */
@@ -211,6 +214,9 @@ static int arm_smmu_alloc_cd_tables(struct 
iommu_vendor_psdtable_cfg *pst_cfg)
goto err_free_l1;
}
  
+	if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_64K_L2)

+   pst_cfg->base = cdcfg->cdtab_dma;
+
return 0;
  
  err_free_l1:

--
2.17.1



Re: [PATCH RFC v1 12/15] iommu/virtio: Add support for INVALIDATE request

2021-03-03 Thread Vivek Kumar Gautam

Hi Jacob, Kevin,


On 3/4/21 11:28 AM, Tian, Kevin wrote:

From: Jacob Pan 
Sent: Thursday, March 4, 2021 2:29 AM

Hi Vivek,

On Fri, 15 Jan 2021 17:43:39 +0530, Vivek Gautam 
wrote:


From: Jean-Philippe Brucker 

Add support for tlb invalidation ops that can send invalidation
requests to back-end virtio-iommu when stage-1 page tables are
supported.


Just curious if it possible to reuse the iommu uapi for invalidation and others.
When we started out designing the iommu uapi, the intention was to support
both emulated and virtio iommu.


IIUC this patch is about the protocol between virtio-iommu frontend and backend.
After the virtio-iommu backend receives invalidation ops, it then needs to
forward the request to the host IOMMU driver through the existing iommu
uapi that you referred to, as a emulated VT-d or SMMU would do.


Thanks a lot for looking at the patch.

Yes this patch is to provide the front-end virtio interface for 
invalidation requests during map/unmap and when flushing the pasid 
tables when virtio-iommu requested pasid table (in other words cd tables 
for arm-smmu-v3) from the iommu-pasid-table library.
The kvmtool back-end virtio driver forwards these requests to vfio 
driver which then makes use of iommu uapi to finally request host iommu 
driver for handling these invalidations.


Regards
Vivek



Thanks
Kevin




Signed-off-by: Jean-Philippe Brucker 
[Vivek: Refactoring the iommu_flush_ops, and adding only one pasid sync
 op that's needed with current iommu-pasid-table infrastructure.
Also updating uapi defines as required by latest changes]
Signed-off-by: Vivek Gautam 
Cc: Joerg Roedel 
Cc: Will Deacon 
Cc: Michael S. Tsirkin 
Cc: Robin Murphy 
Cc: Jean-Philippe Brucker 
Cc: Eric Auger 
Cc: Alex Williamson 
Cc: Kevin Tian 
Cc: Jacob Pan 
Cc: Liu Yi L 
Cc: Lorenzo Pieralisi 
Cc: Shameerali Kolothum Thodi 
---
  drivers/iommu/virtio-iommu.c | 95



  1 file changed, 95 insertions(+)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index ae5dfd3f8269..004ea94e3731 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -13,6 +13,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -63,6 +64,8 @@ struct viommu_mapping {
  };

  struct viommu_mm {
+   int pasid;
+   u64 archid;
struct io_pgtable_ops   *ops;
struct viommu_domain*domain;
  };
@@ -692,6 +695,98 @@ static void viommu_event_handler(struct

virtqueue

*vq) virtqueue_kick(vq);
  }

+/* PASID and pgtable APIs */
+
+static void __viommu_flush_pasid_tlb_all(struct viommu_domain

*vdomain,

+int pasid, u64 arch_id, int
type) +{
+   struct virtio_iommu_req_invalidate req = {
+   .head.type  = VIRTIO_IOMMU_T_INVALIDATE,
+   .inv_gran   =
cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
+   .flags  =
cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID),
+   .inv_type   = cpu_to_le32(type),
+
+   .domain = cpu_to_le32(vdomain->id),
+   .pasid  = cpu_to_le32(pasid),
+   .archid = cpu_to_le64(arch_id),
+   };
+
+   if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
+   pr_debug("could not send invalidate request\n");
+}
+
+static void viommu_flush_tlb_add(struct iommu_iotlb_gather *gather,
+unsigned long iova, size_t granule,
+void *cookie)
+{
+   struct viommu_mm *viommu_mm = cookie;
+   struct viommu_domain *vdomain = viommu_mm->domain;
+   struct iommu_domain *domain = &vdomain->domain;
+
+   iommu_iotlb_gather_add_page(domain, gather, iova, granule);
+}
+
+static void viommu_flush_tlb_walk(unsigned long iova, size_t size,
+ size_t granule, void *cookie)
+{
+   struct viommu_mm *viommu_mm = cookie;
+   struct viommu_domain *vdomain = viommu_mm->domain;
+   struct virtio_iommu_req_invalidate req = {
+   .head.type  = VIRTIO_IOMMU_T_INVALIDATE,
+   .inv_gran   = cpu_to_le32(VIRTIO_IOMMU_INVAL_G_VA),
+   .inv_type   =

cpu_to_le32(VIRTIO_IOMMU_INV_T_IOTLB),

+   .flags  =
cpu_to_le32(VIRTIO_IOMMU_INVAL_F_ARCHID), +
+   .domain = cpu_to_le32(vdomain->id),
+   .pasid  = cpu_to_le32(viommu_mm->pasid),
+   .archid = cpu_to_le64(viommu_mm->archid),
+   .virt_start = cpu_to_le64(iova),
+   .nr_pages   = cpu_to_le64(size / granule),
+   .granule= ilog2(granule),
+   };
+
+   if (viommu_add_req(vdomain->viommu, &req, sizeof(req)))
+   pr_debug("could not add invalidate request\n");
+}
+
+static void viommu_flush_tl

Re: [PATCH 2/2] iommu: arm-smmu-v3: Report domain nesting info reuqired for stage1

2021-03-03 Thread Vivek Kumar Gautam

Hi Eric,


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

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


My bad! Will correct it.



Signed-off-by: Vivek Gautam 
---
  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.


+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.


return 0;
  }
  





Thanks
Vivek


Re: [PATCH 1/2] iommu: Report domain nesting info for arm-smmu-v3

2021-03-03 Thread Vivek Kumar Gautam

Hi Eric,


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

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

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 
---
  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_LAST2
+#define IOMMU_PASID_FORMAT_ARM_SMMU_V3 2
+#define IOMMU_PASID_FORMAT_LAST3
__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].


+   __u8pgtbl_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.gau...@arm.com/


[snip]


Re: [PATCH RFC v1 00/15] iommu/virtio: Nested stage support with Arm

2021-01-25 Thread Vivek Kumar Gautam

Hi Shameer,


On 1/22/21 9:19 PM, Shameerali Kolothum Thodi wrote:

Hi Vivek,


-Original Message-
From: Vivek Kumar Gautam [mailto:vivek.gau...@arm.com]
Sent: 21 January 2021 17:34
To: Auger Eric ; linux-kernel@vger.kernel.org;
linux-arm-ker...@lists.infradead.org; io...@lists.linux-foundation.org;
virtualizat...@lists.linux-foundation.org
Cc: j...@8bytes.org; will.dea...@arm.com; m...@redhat.com;
robin.mur...@arm.com; jean-phili...@linaro.org;
alex.william...@redhat.com; kevin.t...@intel.com;
jacob.jun@linux.intel.com; yi.l@intel.com; lorenzo.pieral...@arm.com;
Shameerali Kolothum Thodi 
Subject: Re: [PATCH RFC v1 00/15] iommu/virtio: Nested stage support with
Arm

Hi Eric,


On 1/19/21 2:33 PM, Auger Eric wrote:

Hi Vivek,

On 1/15/21 1:13 PM, Vivek Gautam wrote:

This patch-series aims at enabling Nested stage translation in guests
using virtio-iommu as the paravirtualized iommu. The backend is
supported with Arm SMMU-v3 that provides nested stage-1 and stage-2

translation.


This series derives its purpose from various efforts happening to add
support for Shared Virtual Addressing (SVA) in host and guest. On
Arm, most of the support for SVA has already landed. The support for
nested stage translation and fault reporting to guest has been proposed [1].
The related changes required in VFIO [2] framework have also been put
forward.

This series proposes changes in virtio-iommu to program PASID tables
and related stage-1 page tables. A simple iommu-pasid-table library
is added for this purpose that interacts with vendor drivers to
allocate and populate PASID tables.
In Arm SMMUv3 we propose to pull the Context Descriptor (CD)
management code out of the arm-smmu-v3 driver and add that as a glue
vendor layer to support allocating CD tables, and populating them with right

values.

These CD tables are essentially the PASID tables and contain stage-1
page table configurations too.
A request to setup these CD tables come from virtio-iommu driver
using the iommu-pasid-table library when running on Arm. The
virtio-iommu then pass these PASID tables to the host using the right
virtio backend and support in VMM.

For testing we have added necessary support in kvmtool. The changes
in kvmtool are based on virtio-iommu development branch by
Jean-Philippe Brucker [3].

The tested kernel branch contains following in the order bottom to
top on the git hash -
a) v5.11-rc3
b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
 table support for Arm.
c) Smmu test engine patches from Jean-Philippe's branch [4]
d) This series
e) Domain nesting info patches [5][6][7].
f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
 the list).

This kernel is tested on Neoverse reference software stack with Fixed
virtual platform. Public version of the software stack and FVP is
available here[8][9].

A big thanks to Jean-Philippe for his contributions towards this work
and for his valuable guidance.

[1]
https://lore.kernel.org/linux-iommu/20201118112151.25412-1-eric.auger
@redhat.com/T/ [2]


https://lore.kernel.org/kvmarm/20201116110030.32335-12-eric.auger@red

hat.com/T/ [3]
https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
[4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
[5]
https://lore.kernel.org/kvm/1599734733-6431-2-git-send-email-yi.l.liu
@intel.com/ [6]
https://lore.kernel.org/kvm/1599734733-6431-3-git-send-email-yi.l.liu
@intel.com/ [7]
https://lore.kernel.org/kvm/1599734733-6431-4-git-send-email-yi.l.liu
@intel.com/ [8]
https://developer.arm.com/tools-and-software/open-source-software/arm
-platforms-software/arm-ecosystem-fvps
[9]
https://git.linaro.org/landing-teams/working/arm/arm-reference-platfo
rms.git/about/docs/rdn1edge/user-guide.rst


Could you share a public branch where we could find all the kernel pieces.

Thank you in advance


Apologies for the delay. It took a bit of time to sort things out for a public
branch.
The branch is available in my github now. Please have a look.

https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-vi
rtio-iommu
 > Thanks for this. Do you have a corresponding kvmtool branch mentioned 

above as public?

Thanks for showing interest. I will publish the kvmtool branch asap. 
Though the current development is based on Jean's branch for 
virtio-iommu [1], I plan to rebase the changes to master soon.


Thanks & regards
Vivek

[1] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel


Thanks,
Shameer



Re: [PATCH RFC v1 00/15] iommu/virtio: Nested stage support with Arm

2021-01-21 Thread Vivek Kumar Gautam

Hi Eric,


On 1/19/21 2:33 PM, Auger Eric wrote:

Hi Vivek,

On 1/15/21 1:13 PM, Vivek Gautam wrote:

This patch-series aims at enabling Nested stage translation in guests
using virtio-iommu as the paravirtualized iommu. The backend is supported
with Arm SMMU-v3 that provides nested stage-1 and stage-2 translation.

This series derives its purpose from various efforts happening to add
support for Shared Virtual Addressing (SVA) in host and guest. On Arm,
most of the support for SVA has already landed. The support for nested
stage translation and fault reporting to guest has been proposed [1].
The related changes required in VFIO [2] framework have also been put
forward.

This series proposes changes in virtio-iommu to program PASID tables
and related stage-1 page tables. A simple iommu-pasid-table library
is added for this purpose that interacts with vendor drivers to
allocate and populate PASID tables.
In Arm SMMUv3 we propose to pull the Context Descriptor (CD) management
code out of the arm-smmu-v3 driver and add that as a glue vendor layer
to support allocating CD tables, and populating them with right values.
These CD tables are essentially the PASID tables and contain stage-1
page table configurations too.
A request to setup these CD tables come from virtio-iommu driver using
the iommu-pasid-table library when running on Arm. The virtio-iommu
then pass these PASID tables to the host using the right virtio backend
and support in VMM.

For testing we have added necessary support in kvmtool. The changes in
kvmtool are based on virtio-iommu development branch by Jean-Philippe
Brucker [3].

The tested kernel branch contains following in the order bottom to top
on the git hash -
a) v5.11-rc3
b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
table support for Arm.
c) Smmu test engine patches from Jean-Philippe's branch [4]
d) This series
e) Domain nesting info patches [5][6][7].
f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
the list).

This kernel is tested on Neoverse reference software stack with
Fixed virtual platform. Public version of the software stack and
FVP is available here[8][9].

A big thanks to Jean-Philippe for his contributions towards this work
and for his valuable guidance.

[1] 
https://lore.kernel.org/linux-iommu/20201118112151.25412-1-eric.au...@redhat.com/T/
[2] 
https://lore.kernel.org/kvmarm/20201116110030.32335-12-eric.au...@redhat.com/T/
[3] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
[4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
[5] 
https://lore.kernel.org/kvm/1599734733-6431-2-git-send-email-yi.l@intel.com/
[6] 
https://lore.kernel.org/kvm/1599734733-6431-3-git-send-email-yi.l@intel.com/
[7] 
https://lore.kernel.org/kvm/1599734733-6431-4-git-send-email-yi.l@intel.com/
[8] 
https://developer.arm.com/tools-and-software/open-source-software/arm-platforms-software/arm-ecosystem-fvps
[9] 
https://git.linaro.org/landing-teams/working/arm/arm-reference-platforms.git/about/docs/rdn1edge/user-guide.rst


Could you share a public branch where we could find all the kernel pieces.

Thank you in advance


Apologies for the delay. It took a bit of time to sort things out for a 
public branch.

The branch is available in my github now. Please have a look.

https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-virtio-iommu


Thanks and regards
Vivek



Best Regards

Eric


Jean-Philippe Brucker (6):
   iommu/virtio: Add headers for table format probing
   iommu/virtio: Add table format probing
   iommu/virtio: Add headers for binding pasid table in iommu
   iommu/virtio: Add support for INVALIDATE request
   iommu/virtio: Attach Arm PASID tables when available
   iommu/virtio: Add support for Arm LPAE page table format

Vivek Gautam (9):
   iommu/arm-smmu-v3: Create a Context Descriptor library
   iommu: Add a simple PASID table library
   iommu/arm-smmu-v3: Update drivers to work with iommu-pasid-table
   iommu/arm-smmu-v3: Update CD base address info for user-space
   iommu/arm-smmu-v3: Set sync op from consumer driver of cd-lib
   iommu: Add asid_bits to arm smmu-v3 stage1 table info
   iommu/virtio: Update table format probing header
   iommu/virtio: Prepare to add attach pasid table infrastructure
   iommu/virtio: Update fault type and reason info for viommu fault

  drivers/iommu/arm/arm-smmu-v3/Makefile|   2 +-
  .../arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c  | 283 +++
  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  16 +-
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 268 +--
  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   4 +-
  drivers/iommu/iommu-pasid-table.h | 140 
  drivers/iommu/virtio-iommu.c  | 692 +-
  include/uapi/linux/iommu.h|   2 +-
  include/uapi/linux/virtio_iommu.h | 158 +++-
  9 files changed, 1303 insertions(+), 262 deletions(-)
  create mode 100644 drivers/iommu/a