[PATCH 2/2] iommu/mediatek: Alloc building as module

2021-03-22 Thread Yong Wu
The IOMMU in many SoC depends on the MM clocks and power-domain which
are device_initcall normally, thus the subsys_init here is not helpful.
This patch switches it to module_platform_driver which allow the
driver built as module.

Correspondingly switch the config to tristate.

Signed-off-by: Yong Wu 
---
 drivers/iommu/Kconfig |  2 +-
 drivers/iommu/mtk_iommu.c | 16 
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index bc93da48bed0..74f3e15edc14 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -349,7 +349,7 @@ config S390_AP_IOMMU
  is not implemented as it is not necessary for VFIO.
 
 config MTK_IOMMU
-   bool "MTK IOMMU Support"
+   tristate "MediaTek IOMMU Support"
depends on ARCH_MEDIATEK || COMPILE_TEST
select ARM_DMA_USE_IOMMU
select IOMMU_API
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6ecc007f07cd..a73ff3e20480 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1079,16 +1080,7 @@ static struct platform_driver mtk_iommu_driver = {
.pm = _iommu_pm_ops,
}
 };
+module_platform_driver(mtk_iommu_driver);
 
-static int __init mtk_iommu_init(void)
-{
-   int ret;
-
-   ret = platform_driver_register(_iommu_driver);
-   if (ret != 0)
-   pr_err("Failed to register MTK IOMMU driver\n");
-
-   return ret;
-}
-
-subsys_initcall(mtk_iommu_init)
+MODULE_DESCRIPTION("IOMMU API for MediaTek M4U implementations");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0

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


[PATCH 1/2] iommu/mediatek-v1: Alloc building as module

2021-03-22 Thread Yong Wu
This patch only adds support for building the IOMMU-v1 driver as module.
Correspondingly switch the config to tristate.

Signed-off-by: Yong Wu 
---
rebase on v5.12-rc2.
---
 drivers/iommu/Kconfig| 2 +-
 drivers/iommu/mtk_iommu_v1.c | 9 -
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 192ef8f61310..bc93da48bed0 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -364,7 +364,7 @@ config MTK_IOMMU
  If unsure, say N here.
 
 config MTK_IOMMU_V1
-   bool "MTK IOMMU Version 1 (M4U gen1) Support"
+   tristate "MediaTek IOMMU Version 1 (M4U gen1) Support"
depends on ARM
depends on ARCH_MEDIATEK || COMPILE_TEST
select ARM_DMA_USE_IOMMU
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 82ddfe9170d4..71370037083a 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -691,9 +692,7 @@ static struct platform_driver mtk_iommu_driver = {
.pm = _iommu_pm_ops,
}
 };
+module_platform_driver(mtk_iommu_driver);
 
-static int __init m4u_init(void)
-{
-   return platform_driver_register(_iommu_driver);
-}
-subsys_initcall(m4u_init);
+MODULE_DESCRIPTION("IOMMU API for MediaTek M4U v1 implementations");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0

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


Re: [PATCH 2/3] iommu/vt-d: Remove IOVA domain rcache flushing for CPU offlining

2021-03-22 Thread Lu Baolu

On 3/1/21 8:12 PM, John Garry wrote:

Now that the core code handles flushing per-IOVA domain CPU rcaches,
remove the handling here.

Signed-off-by: John Garry 
---
  drivers/iommu/intel/iommu.c | 31 ---
  include/linux/cpuhotplug.h  |  1 -
  2 files changed, 32 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..d1e66e1b07b8 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4065,35 +4065,6 @@ static struct notifier_block intel_iommu_memory_nb = {
.priority = 0
  };
  
-static void free_all_cpu_cached_iovas(unsigned int cpu)

-{
-   int i;
-
-   for (i = 0; i < g_num_of_iommus; i++) {
-   struct intel_iommu *iommu = g_iommus[i];
-   struct dmar_domain *domain;
-   int did;
-
-   if (!iommu)
-   continue;
-
-   for (did = 0; did < cap_ndoms(iommu->cap); did++) {
-   domain = get_iommu_domain(iommu, (u16)did);
-
-   if (!domain || domain->domain.type != IOMMU_DOMAIN_DMA)
-   continue;
-
-   iommu_dma_free_cpu_cached_iovas(cpu, >domain);
-   }
-   }
-}
-
-static int intel_iommu_cpu_dead(unsigned int cpu)
-{
-   free_all_cpu_cached_iovas(cpu);
-   return 0;
-}
-
  static void intel_disable_iommus(void)
  {
struct intel_iommu *iommu = NULL;
@@ -4388,8 +4359,6 @@ int __init intel_iommu_init(void)
bus_set_iommu(_bus_type, _iommu_ops);
if (si_domain && !hw_pass_through)
register_memory_notifier(_iommu_memory_nb);
-   cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD, "iommu/intel:dead", NULL,
- intel_iommu_cpu_dead);
  
  	down_read(_global_lock);

if (probe_acpi_namespace_devices())
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index cedac9986557..85996494bec1 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -57,7 +57,6 @@ enum cpuhp_state {
CPUHP_PAGE_ALLOC_DEAD,
CPUHP_NET_DEV_DEAD,
CPUHP_PCI_XGENE_DEAD,
-   CPUHP_IOMMU_INTEL_DEAD,
CPUHP_IOMMU_IOVA_DEAD,
CPUHP_LUSTRE_CFS_DEAD,
CPUHP_AP_ARM_CACHE_B15_RAC_DEAD,



Reviewed-by: Lu Baolu 

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v3] swiotlb: Make SWIOTLB_NO_FORCE perform no allocation

2021-03-22 Thread Florian Fainelli
When SWIOTLB_NO_FORCE is used, there should really be no allocations of
default_nslabs to occur since we are not going to use those slabs. If a
platform was somehow setting swiotlb_no_force and a later call to
swiotlb_init() was to be made we would still be proceeding with
allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
was set on the kernel command line we would have only allocated 2KB.

This would be inconsistent and the point of initializing default_nslabs
to 1, was intended to allocate the minimum amount of memory possible, so
simply remove that minimal allocation period.

Signed-off-by: Florian Fainelli 
---
Changes in v3:
- patch all call sites that can allocate SWIOTLB memory

Changes in v2:

- rebased against devel/for-linus-5.13
- updated commit message to reflect variable names

 kernel/dma/swiotlb.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 539c76beb52e..0a5b6f7e75bc 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -83,12 +83,10 @@ setup_io_tlb_npages(char *str)
}
if (*str == ',')
++str;
-   if (!strcmp(str, "force")) {
+   if (!strcmp(str, "force"))
swiotlb_force = SWIOTLB_FORCE;
-   } else if (!strcmp(str, "noforce")) {
+   else if (!strcmp(str, "noforce"))
swiotlb_force = SWIOTLB_NO_FORCE;
-   default_nslabs = 1;
-   }
 
return 0;
 }
@@ -174,6 +172,9 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long 
nslabs, int verbose)
struct io_tlb_mem *mem;
size_t alloc_size;
 
+   if (swiotlb_force == SWIOTLB_NO_FORCE)
+   return 0;
+
/* protect against double initialization */
if (WARN_ON_ONCE(io_tlb_default_mem))
return -ENOMEM;
@@ -211,6 +212,9 @@ swiotlb_init(int verbose)
size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
void *tlb;
 
+   if (swiotlb_force == SWIOTLB_NO_FORCE)
+   return;
+
/* Get IO TLB memory from the low pages */
tlb = memblock_alloc_low(bytes, PAGE_SIZE);
if (!tlb)
@@ -240,6 +244,9 @@ swiotlb_late_init_with_default_size(size_t default_size)
unsigned int order;
int rc = 0;
 
+   if (swiotlb_force == SWIOTLB_NO_FORCE)
+   return 0;
+
/*
 * Get IO TLB memory from the low pages
 */
@@ -276,6 +283,9 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
struct io_tlb_mem *mem;
 
+   if (swiotlb_force == SWIOTLB_NO_FORCE)
+   return 0;
+
/* protect against double initialization */
if (WARN_ON_ONCE(io_tlb_default_mem))
return -ENOMEM;
-- 
2.25.1

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


Re: [Linuxarm] Re: [PATCH v14 07/13] iommu/smmuv3: Implement cache_invalidate

2021-03-22 Thread chenxiang (M)

Hi Eric,


在 2021/3/22 17:05, Auger Eric 写道:

Hi Chenxiang,

On 3/22/21 7:40 AM, chenxiang (M) wrote:

Hi Eric,


在 2021/3/20 1:36, Auger Eric 写道:

Hi Chenxiang,

On 3/4/21 8:55 AM, chenxiang (M) wrote:

Hi Eric,


在 2021/2/24 4:56, Eric Auger 写道:

Implement domain-selective, pasid selective and page-selective
IOTLB invalidations.

Signed-off-by: Eric Auger 

---

v13 -> v14:
- Add domain invalidation
- do global inval when asid is not provided with addr
granularity

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/arm-smmu-v3/arm-smmu-v3.c | 74
+
   1 file changed, 74 insertions(+)

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 4c19a1114de4..df3adc49111c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2949,6 +2949,79 @@ static void
arm_smmu_detach_pasid_table(struct iommu_domain *domain)
   mutex_unlock(_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_cmdq_ent cmd = {.opcode = CMDQ_OP_TLBI_NSNH_ALL};
+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_PASID ||
+inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
+return -ENOENT;
+}
+
+if (!(inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB))
+return -EINVAL;
+
+/* IOTLB invalidation */
+
+switch (inv_info->granularity) {
+case IOMMU_INV_GRANU_PASID:
+{
+struct iommu_inv_pasid_info *info =
+_info->granu.pasid_info;
+
+if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
+return -ENOENT;
+if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID))
+return -EINVAL;
+
+__arm_smmu_tlb_inv_context(smmu_domain, info->archid);
+return 0;
+}
+case IOMMU_INV_GRANU_ADDR:
+{
+struct iommu_inv_addr_info *info = _info->granu.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_PASID)
+return -ENOENT;
+
+if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
+break;
+
+arm_smmu_tlb_inv_range_domain(info->addr, size,
+  info->granule_size, leaf,
+  info->archid, smmu_domain);

Is it possible to add a check before the function to make sure that
info->granule_size can be recognized by SMMU?
There is a scenario which will cause TLBI issue: RIL feature is enabled
on guest but is disabled on host, and then on
host it just invalidate 4K/2M/1G once a time, but from QEMU,
info->nb_granules is always 1 and info->granule_size = size,
if size is not equal to 4K or 2M or 1G (for example size = granule_size
is 5M), it will only part of the size it wants to invalidate.

Do you have any idea about this issue?

At the QEMU VFIO notifier level, when I fill the struct
iommu_cache_invalidate_info, I currently miss the granule info, hence
this weird choice of setting setting info->nb_granules is always 1 and
info->granule_size = size. I think in arm_smmu_cache_invalidate() I need
to convert this info back to a the leaf page size, in case the host does
not support RIL. Just as it is done in  __arm_smmu_tlb_inv_range if RIL
is supported.

Does it makes sense to you?


Yes, it makes sense to me.
I am glad to test them if the patchset are ready.




Thank you for spotting the issue.

Eric

I think maybe we can add a check here: if RIL is not enabled and also
size is not the granule_size (4K/2M/1G) supported by
SMMU hardware, can we just simply use the smallest granule_size
supported by hardware all the time?


+
+arm_smmu_cmdq_issue_sync(smmu);
+return 0;
+}
+case IOMMU_INV_GRANU_DOMAIN:
+break;

I check the qemu code
(https://github.com/eauger/qemu/tree/v5.2.0-2stage-rfcv8), for opcode
CMD_TLBI_NH_ALL or CMD_TLBI_NSNH_ALL from guest OS
it calls smmu_inv_notifiers_all() to unamp all notifiers of all mr's,
but it seems not set 

[PATCH 5/5] iommu/vt-d: Make unnecessarily global functions static

2021-03-22 Thread Lu Baolu
Make some functions static as they are only used inside pasid.c.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel/pasid.c | 4 ++--
 drivers/iommu/intel/pasid.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index f2c747e62c6a..c896aef7db40 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -230,7 +230,7 @@ struct pasid_table *intel_pasid_get_table(struct device 
*dev)
return info->pasid_table;
 }
 
-int intel_pasid_get_dev_max_id(struct device *dev)
+static int intel_pasid_get_dev_max_id(struct device *dev)
 {
struct device_domain_info *info;
 
@@ -241,7 +241,7 @@ int intel_pasid_get_dev_max_id(struct device *dev)
return info->pasid_table->max_pasid;
 }
 
-struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid)
+static struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid)
 {
struct device_domain_info *info;
struct pasid_table *pasid_table;
diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index 90a3268d7a77..079534fcf55d 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -102,8 +102,6 @@ extern unsigned int intel_pasid_max_id;
 int intel_pasid_alloc_table(struct device *dev);
 void intel_pasid_free_table(struct device *dev);
 struct pasid_table *intel_pasid_get_table(struct device *dev);
-int intel_pasid_get_dev_max_id(struct device *dev);
-struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid);
 int intel_pasid_setup_first_level(struct intel_iommu *iommu,
  struct device *dev, pgd_t *pgd,
  u32 pasid, u16 did, int flags);
-- 
2.25.1

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


[PATCH 4/5] iommu/vt-d: Remove unused function declarations

2021-03-22 Thread Lu Baolu
Some functions have been deprecated. Remove the remaining function
delarations.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel/pasid.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index 444c0bec221a..90a3268d7a77 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -99,9 +99,6 @@ static inline bool pasid_pte_is_present(struct pasid_entry 
*pte)
 }
 
 extern unsigned int intel_pasid_max_id;
-int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp);
-void intel_pasid_free_id(u32 pasid);
-void *intel_pasid_lookup_id(u32 pasid);
 int intel_pasid_alloc_table(struct device *dev);
 void intel_pasid_free_table(struct device *dev);
 struct pasid_table *intel_pasid_get_table(struct device *dev);
-- 
2.25.1

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


[PATCH 3/5] iommu/vt-d: Remove SVM_FLAG_PRIVATE_PASID

2021-03-22 Thread Lu Baolu
The SVM_FLAG_PRIVATE_PASID has never been referenced in the tree, and
there's no plan to have anything to use it. So cleanup it.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel/svm.c | 40 ++-
 include/linux/intel-svm.h | 16 +++-
 2 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 875ee74d8c41..5165cea90421 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -465,9 +465,9 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
  struct mm_struct *mm, struct intel_svm_dev **sd)
 {
struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+   struct intel_svm *svm = NULL, *t;
struct device_domain_info *info;
struct intel_svm_dev *sdev;
-   struct intel_svm *svm = NULL;
unsigned long iflags;
int pasid_max;
int ret;
@@ -493,30 +493,26 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
}
}
 
-   if (!(flags & SVM_FLAG_PRIVATE_PASID)) {
-   struct intel_svm *t;
-
-   list_for_each_entry(t, _svm_list, list) {
-   if (t->mm != mm || (t->flags & SVM_FLAG_PRIVATE_PASID))
-   continue;
-
-   svm = t;
-   if (svm->pasid >= pasid_max) {
-   dev_warn(dev,
-"Limited PASID width. Cannot use 
existing PASID %d\n",
-svm->pasid);
-   ret = -ENOSPC;
-   goto out;
-   }
+   list_for_each_entry(t, _svm_list, list) {
+   if (t->mm != mm)
+   continue;
 
-   /* Find the matching device in svm list */
-   for_each_svm_dev(sdev, svm, dev) {
-   sdev->users++;
-   goto success;
-   }
+   svm = t;
+   if (svm->pasid >= pasid_max) {
+   dev_warn(dev,
+"Limited PASID width. Cannot use existing 
PASID %d\n",
+svm->pasid);
+   ret = -ENOSPC;
+   goto out;
+   }
 
-   break;
+   /* Find the matching device in svm list */
+   for_each_svm_dev(sdev, svm, dev) {
+   sdev->users++;
+   goto success;
}
+
+   break;
}
 
sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 6c9d10c0fb1e..10fa80eef13a 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -14,16 +14,6 @@
 #define SVM_REQ_EXEC   (1<<1)
 #define SVM_REQ_PRIV   (1<<0)
 
-/*
- * The SVM_FLAG_PRIVATE_PASID flag requests a PASID which is *not* the "main"
- * PASID for the current process. Even if a PASID already exists, a new one
- * will be allocated. And the PASID allocated with SVM_FLAG_PRIVATE_PASID
- * will not be given to subsequent callers. This facility allows a driver to
- * disambiguate between multiple device contexts which access the same MM,
- * if there is no other way to do so. It should be used sparingly, if at all.
- */
-#define SVM_FLAG_PRIVATE_PASID (1<<0)
-
 /*
  * The SVM_FLAG_SUPERVISOR_MODE flag requests a PASID which can be used only
  * for access to kernel addresses. No IOTLB flushes are automatically done
@@ -35,18 +25,18 @@
  * It is unlikely that we will ever hook into flush_tlb_kernel_range() to
  * do such IOTLB flushes automatically.
  */
-#define SVM_FLAG_SUPERVISOR_MODE   (1<<1)
+#define SVM_FLAG_SUPERVISOR_MODE   BIT(0)
 /*
  * The SVM_FLAG_GUEST_MODE flag is used when a PASID bind is for guest
  * processes. Compared to the host bind, the primary differences are:
  * 1. mm life cycle management
  * 2. fault reporting
  */
-#define SVM_FLAG_GUEST_MODE(1<<2)
+#define SVM_FLAG_GUEST_MODEBIT(1)
 /*
  * The SVM_FLAG_GUEST_PASID flag is used when a guest has its own PASID space,
  * which requires guest and host PASID translation at both directions.
  */
-#define SVM_FLAG_GUEST_PASID   (1<<3)
+#define SVM_FLAG_GUEST_PASID   BIT(2)
 
 #endif /* __INTEL_SVM_H__ */
-- 
2.25.1

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


[PATCH 2/5] iommu/vt-d: Remove svm_dev_ops

2021-03-22 Thread Lu Baolu
The svm_dev_ops has never been referenced in the tree, and there's no
plan to have anything to use it. Remove it to make the code neat.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel/svm.c   | 15 +--
 include/linux/intel-iommu.h |  3 ---
 include/linux/intel-svm.h   |  7 ---
 3 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 5d590d63ab52..875ee74d8c41 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -462,7 +462,6 @@ static void load_pasid(struct mm_struct *mm, u32 pasid)
 /* Caller must hold pasid_mutex, mm reference */
 static int
 intel_svm_bind_mm(struct device *dev, unsigned int flags,
- struct svm_dev_ops *ops,
  struct mm_struct *mm, struct intel_svm_dev **sd)
 {
struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
@@ -512,10 +511,6 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
 
/* Find the matching device in svm list */
for_each_svm_dev(sdev, svm, dev) {
-   if (sdev->ops != ops) {
-   ret = -EBUSY;
-   goto out;
-   }
sdev->users++;
goto success;
}
@@ -550,7 +545,6 @@ intel_svm_bind_mm(struct device *dev, unsigned int flags,
 
/* Finish the setup now we know we're keeping it */
sdev->users = 1;
-   sdev->ops = ops;
init_rcu_head(>rcu);
 
if (!svm) {
@@ -1006,13 +1000,6 @@ static irqreturn_t prq_event_thread(int irq, void *d)
mmap_read_unlock(svm->mm);
mmput(svm->mm);
 bad_req:
-   WARN_ON(!sdev);
-   if (sdev && sdev->ops && sdev->ops->fault_cb) {
-   int rwxp = (req->rd_req << 3) | (req->wr_req << 2) |
-   (req->exe_req << 1) | (req->pm_req);
-   sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr,
-   req->priv_data, rwxp, result);
-   }
/* We get here in the error case where the PASID lookup failed,
   and these can be NULL. Do not use them below this point! */
sdev = NULL;
@@ -1087,7 +1074,7 @@ intel_svm_bind(struct device *dev, struct mm_struct *mm, 
void *drvdata)
if (drvdata)
flags = *(unsigned int *)drvdata;
mutex_lock(_mutex);
-   ret = intel_svm_bind_mm(dev, flags, NULL, mm, );
+   ret = intel_svm_bind_mm(dev, flags, mm, );
if (ret)
sva = ERR_PTR(ret);
else if (sdev)
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 76f974da8ca4..03faf20a6817 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -770,14 +770,11 @@ u32 intel_svm_get_pasid(struct iommu_sva *handle);
 int intel_svm_page_response(struct device *dev, struct iommu_fault_event *evt,
struct iommu_page_response *msg);
 
-struct svm_dev_ops;
-
 struct intel_svm_dev {
struct list_head list;
struct rcu_head rcu;
struct device *dev;
struct intel_iommu *iommu;
-   struct svm_dev_ops *ops;
struct iommu_sva sva;
u32 pasid;
int users;
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 39d368a810b8..6c9d10c0fb1e 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -8,13 +8,6 @@
 #ifndef __INTEL_SVM_H__
 #define __INTEL_SVM_H__
 
-struct device;
-
-struct svm_dev_ops {
-   void (*fault_cb)(struct device *dev, u32 pasid, u64 address,
-void *private, int rwxp, int response);
-};
-
 /* Values for rxwp in fault_cb callback */
 #define SVM_REQ_READ   (1<<3)
 #define SVM_REQ_WRITE  (1<<2)
-- 
2.25.1

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


[PATCH 1/5] iommu/vt-d: Remove unused dma map/unmap trace events

2021-03-22 Thread Lu Baolu
With commit c588072bba6b5 ("iommu/vt-d: Convert intel iommu driver to
the iommu ops"), the trace events for dma map/unmap have no users any
more. Cleanup them to make the code neat.

Signed-off-by: Lu Baolu 
---
 include/trace/events/intel_iommu.h | 120 -
 1 file changed, 120 deletions(-)

diff --git a/include/trace/events/intel_iommu.h 
b/include/trace/events/intel_iommu.h
index e801f4910522..d233f2916584 100644
--- a/include/trace/events/intel_iommu.h
+++ b/include/trace/events/intel_iommu.h
@@ -15,126 +15,6 @@
 #include 
 #include 
 
-DECLARE_EVENT_CLASS(dma_map,
-   TP_PROTO(struct device *dev, dma_addr_t dev_addr, phys_addr_t phys_addr,
-size_t size),
-
-   TP_ARGS(dev, dev_addr, phys_addr, size),
-
-   TP_STRUCT__entry(
-   __string(dev_name, dev_name(dev))
-   __field(dma_addr_t, dev_addr)
-   __field(phys_addr_t, phys_addr)
-   __field(size_t, size)
-   ),
-
-   TP_fast_assign(
-   __assign_str(dev_name, dev_name(dev));
-   __entry->dev_addr = dev_addr;
-   __entry->phys_addr = phys_addr;
-   __entry->size = size;
-   ),
-
-   TP_printk("dev=%s dev_addr=0x%llx phys_addr=0x%llx size=%zu",
- __get_str(dev_name),
- (unsigned long long)__entry->dev_addr,
- (unsigned long long)__entry->phys_addr,
- __entry->size)
-);
-
-DEFINE_EVENT(dma_map, map_single,
-   TP_PROTO(struct device *dev, dma_addr_t dev_addr, phys_addr_t phys_addr,
-size_t size),
-   TP_ARGS(dev, dev_addr, phys_addr, size)
-);
-
-DEFINE_EVENT(dma_map, bounce_map_single,
-   TP_PROTO(struct device *dev, dma_addr_t dev_addr, phys_addr_t phys_addr,
-size_t size),
-   TP_ARGS(dev, dev_addr, phys_addr, size)
-);
-
-DECLARE_EVENT_CLASS(dma_unmap,
-   TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
-
-   TP_ARGS(dev, dev_addr, size),
-
-   TP_STRUCT__entry(
-   __string(dev_name, dev_name(dev))
-   __field(dma_addr_t, dev_addr)
-   __field(size_t, size)
-   ),
-
-   TP_fast_assign(
-   __assign_str(dev_name, dev_name(dev));
-   __entry->dev_addr = dev_addr;
-   __entry->size = size;
-   ),
-
-   TP_printk("dev=%s dev_addr=0x%llx size=%zu",
- __get_str(dev_name),
- (unsigned long long)__entry->dev_addr,
- __entry->size)
-);
-
-DEFINE_EVENT(dma_unmap, unmap_single,
-   TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
-   TP_ARGS(dev, dev_addr, size)
-);
-
-DEFINE_EVENT(dma_unmap, unmap_sg,
-   TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
-   TP_ARGS(dev, dev_addr, size)
-);
-
-DEFINE_EVENT(dma_unmap, bounce_unmap_single,
-   TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
-   TP_ARGS(dev, dev_addr, size)
-);
-
-DECLARE_EVENT_CLASS(dma_map_sg,
-   TP_PROTO(struct device *dev, int index, int total,
-struct scatterlist *sg),
-
-   TP_ARGS(dev, index, total, sg),
-
-   TP_STRUCT__entry(
-   __string(dev_name, dev_name(dev))
-   __field(dma_addr_t, dev_addr)
-   __field(phys_addr_t, phys_addr)
-   __field(size_t, size)
-   __field(int, index)
-   __field(int, total)
-   ),
-
-   TP_fast_assign(
-   __assign_str(dev_name, dev_name(dev));
-   __entry->dev_addr = sg->dma_address;
-   __entry->phys_addr = sg_phys(sg);
-   __entry->size = sg->dma_length;
-   __entry->index = index;
-   __entry->total = total;
-   ),
-
-   TP_printk("dev=%s [%d/%d] dev_addr=0x%llx phys_addr=0x%llx size=%zu",
- __get_str(dev_name), __entry->index, __entry->total,
- (unsigned long long)__entry->dev_addr,
- (unsigned long long)__entry->phys_addr,
- __entry->size)
-);
-
-DEFINE_EVENT(dma_map_sg, map_sg,
-   TP_PROTO(struct device *dev, int index, int total,
-struct scatterlist *sg),
-   TP_ARGS(dev, index, total, sg)
-);
-
-DEFINE_EVENT(dma_map_sg, bounce_map_sg,
-   TP_PROTO(struct device *dev, int index, int total,
-struct scatterlist *sg),
-   TP_ARGS(dev, index, total, sg)
-);
-
 TRACE_EVENT(qi_submit,
TP_PROTO(struct intel_iommu *iommu, u64 qw0, u64 qw1, u64 qw2, u64 qw3),
 
-- 
2.25.1

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


[PATCH 0/5] iommu/vt-d: Several misc cleanups

2021-03-22 Thread Lu Baolu
Hi Joerg et al,

This series includes several cleanups in the VT-d driver. Please help to
review.

Best regards,
baolu

Lu Baolu (5):
  iommu/vt-d: Remove unused dma map/unmap trace events
  iommu/vt-d: Remove svm_dev_ops
  iommu/vt-d: Remove SVM_FLAG_PRIVATE_PASID
  iommu/vt-d: Remove unused function declarations
  iommu/vt-d: Make unnecessarily global functions static

 drivers/iommu/intel/pasid.c|   4 +-
 drivers/iommu/intel/pasid.h|   5 --
 drivers/iommu/intel/svm.c  |  55 +
 include/linux/intel-iommu.h|   3 -
 include/linux/intel-svm.h  |  23 +-
 include/trace/events/intel_iommu.h | 120 -
 6 files changed, 24 insertions(+), 186 deletions(-)

-- 
2.25.1

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


Re: [PATCH 0/3] Apple M1 DART IOMMU driver

2021-03-22 Thread Sven Peter via iommu


Hi Mark,

On Sun, Mar 21, 2021, at 19:35, Mark Kettenis wrote:
>
> Guess we do need to understand a little bit better how the USB DART
> actually works.  My hypothesis (based on our discussion on #asahi) is
> that the XHCI host controller and the peripheral controller of the
> DWC3 block use different DMA "streams" that are handled by the
> different sub-DARTs.

I've done some more experiments and the situation is unfortunately more
complicated: Most DMA transfers are translated with the first DART.
But sometimes (and I have not been able to figure out the exact conditions)
transfers instead have to go through the second DART. 
This happens e.g. with one of my USB keyboards after a stop EP command
is issued: Suddenly the xhci_ep_ctx struct must be translated through the
second DART.

What this likely means is that we'll need to point both DARTs
to the same pagetables and just issue the TLB maintenance operations
as a group.

> 
> The Corellium folks use a DART + sub-DART model in their driver and a
> single node in the device tree that represents both.  That might sense
> since the error registers and interrupts are shared.  Maybe it would
> make sense to select the appropriate sub-DART based on the DMA stream
> ID?

dwc3 specifically seems to require stream id #1 from the DART
at <0x5 0x02f0> and stream id #0 from the DART at <0x5 0x02f8>.
Both of these only share a IRQ line but are otherwise completely independent.
Each has their own error registers, etc. and we need some way to
specify these two DARTs + the appropriate stream ID.

Essentially we have three options to represent this now:

1) Add both DARTs as separate regs, use #iommu-cells = <2> and have the
   first cell select the DART and the second one the stream ID.
   We could allow #iommu-cells = <1> in case only one reg is specified
   for the PCIe DART:

   usb_dart1@502f0 {
 compatible = "apple,t8103-dart";
 reg = <0x5 0x02f0 0x0 0x4000>, <0x5 0x02f8 0x0 0x4000>;
 #iommu-cells = <2>;
 ...
   };

   usb1 {
 iommus = <_dart1 0 1>, <_dart1 1 0>;
 ...
   };

   I prefer this option because we fully describe the DART in a single
   device node here. It also feels natural to group them like this because
   they need to share some properties (like dma-window and the interrupt)
   anyway. 

2) Create two DART nodes which share the same IRQ line and attach them
   both to the master node:

   usb_dart1a@502f0 {
 compatible = "apple,t8103-dart";
 reg = <0x5 0x02f0 0x0 0x4000>;
 #iommu-cells = <1>;
 ...
   };
   usb_dart1b@502f8 {
 compatible = "apple,t8103-dart";
 reg = <0x5 0x02f8 0x0 0x4000>;
 #iommu-cells = <1>;
 ...
   };

   usb1 {
 iommus = <_dart1a 1>, <_dart1b 0>;
 ...
   };

   I dislike this one because attaching those two DARTs to a single device
   seems rather unusual. We'd also have to duplicate the dma-window setting,
   make sure it's the same for both DARTs and there are probably even more
   complications I can't think of right now. It seems like this would also
   make the device tree very verbose and the implementation itself more
   complicated.

3) Introduce another property and let the DART driver take care of
   mirroring the pagetables. I believe this would be similar to
   the sid-remap property:

   usb_dart1@502f0 {
 compatible = "apple,t8103-dart";
 reg = <0x5 0x02f0 0x0 0x4000>, <0x5 0x02f8 0x0 0x4000>;
 #iommu-cells = <1>;
 sid-remap = <0 1>;
   };
   usb1 {
 iommus = <_dart1 0>;
   };

   I slightly dislike this one because we now specify which stream id 
   to use in two places: Once in the device node and another time in the
   new property in the DART node. I also don't think the binding is much
   simpler than the first one.


> > where #dma-address-cells and #dma-size-cells default to
> > #address-cells and #size-cells respectively if I understand
> > the code correctly. That way we could also just always use
> > a 64bit address and size in the DT, e.g.
> > 
> >   pcie_dart {
> >   [ ... ]
> >   dma-window = <0 0x10 0 0x3fe0>;
> >   [ ... ]
> >   };
> 
> That sounds like a serious contender to me!  Hopefully one of the
> Linux kernel developers can give this some sort of blessing.
> 
> I think it would make sense for us to just rely on the #address-cells
> and #size-cells defaults for the M1 device tree.
>

Agreed.


Best,

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


RE: [RFC PATCH v2 1/8] ACPICA: IORT: Update for revision E

2021-03-22 Thread Kaneda, Erik


> -Original Message-
> From: Shameerali Kolothum Thodi
> 
> Sent: Monday, March 22, 2021 3:36 AM
> To: Kaneda, Erik ; linux-arm-
> ker...@lists.infradead.org; linux-a...@vger.kernel.org; iommu@lists.linux-
> foundation.org; de...@acpica.org; Lorenzo Pieralisi
> ; Moore, Robert 
> Cc: Linuxarm ; steven.pr...@arm.com;
> sami.muja...@arm.com; robin.mur...@arm.com; wanghuiqiang
> 
> Subject: [Devel] Re: [RFC PATCH v2 1/8] ACPICA: IORT: Update for revision E
> 
> [+]
> 
> Hi Erik,
> 
> As this is now just merged ino acpica-master and based on the discussion we
> had here,
> 
> https://github.com/acpica/acpica/pull/638
> 
> I had a discussion with ARM folks(Lorenzo) in the linaro-open-discussions call
> and
> can confirm that the IORT Revision E is not the final specification and has
> some issues
> which is now corrected in the latest E.b revision[1]. Also there are no 
> current
> users
> for the Rev E and it may not be a good idea to push this version into the 
> Linux
> kernel
> or elsewhere.
> 
> So could you please revert the merge and I am planning to work on the E.b
> soon.
Hi,

> Please let me know if I need to explicitly send a revert pull request or not.

Please send a revert pull request and I'll remember to not submit Linux-ize the 
IORT patches.

From all of the activity that I've seen with the IORT specification, it looks 
like this table is nontrivial to design and maintain. The difficulty I have 
with the table is that I do not understand which table revisions are in active 
use.

So my question is this: which IORT revisions are being actively used?

Thanks,
Erik
> 
> Thanks,
> Shameer
> 
> 1. https://developer.arm.com/documentation/den0049/latest/
> 
> > -Original Message-
> > From: iommu [mailto:iommu-boun...@lists.linux-foundation.org] On
> Behalf Of
> > Shameer Kolothum
> > Sent: 19 November 2020 12:12
> > To: linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> > iommu@lists.linux-foundation.org; de...@acpica.org
> > Cc: Linuxarm ; steven.pr...@arm.com;
> Guohanjun
> > (Hanjun Guo) ; sami.muja...@arm.com;
> > robin.mur...@arm.com; wanghuiqiang 
> > Subject: [RFC PATCH v2 1/8] ACPICA: IORT: Update for revision E
> >
> > IORT revision E contains a few additions like,
> >     -Added an identifier field in the node descriptors to aid table
> >      cross-referencing.
> >     -Introduced the Reserved Memory Range(RMR) node. This is used
> >      to describe memory ranges that are used by endpoints and requires
> >      a unity mapping in SMMU.
> > -Introduced a flag in the RC node to express support for PRI.
> >
> > Signed-off-by: Shameer Kolothum
> 
> > ---
> >  include/acpi/actbl2.h | 25 +++--
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index
> > ec66779cb193..274fce7b5c01 100644
> > --- a/include/acpi/actbl2.h
> > +++ b/include/acpi/actbl2.h
> > @@ -68,7 +68,7 @@
> >   * IORT - IO Remapping Table
> >   *
> >   * Conforms to "IO Remapping Table System Software on ARM Platforms",
> > - * Document number: ARM DEN 0049D, March 2018
> > + * Document number: ARM DEN 0049E, June 2020
> >   *
> >
> >
> **
> **
> > **/
> >
> > @@ -86,7 +86,8 @@ struct acpi_iort_node {
> > u8 type;
> > u16 length;
> > u8 revision;
> > -   u32 reserved;
> > +   u16 reserved;
> > +   u16 identifier;
> > u32 mapping_count;
> > u32 mapping_offset;
> > char node_data[1];
> > @@ -100,7 +101,8 @@ enum acpi_iort_node_type {
> > ACPI_IORT_NODE_PCI_ROOT_COMPLEX = 0x02,
> > ACPI_IORT_NODE_SMMU = 0x03,
> > ACPI_IORT_NODE_SMMU_V3 = 0x04,
> > -   ACPI_IORT_NODE_PMCG = 0x05
> > +   ACPI_IORT_NODE_PMCG = 0x05,
> > +   ACPI_IORT_NODE_RMR = 0x06,
> >  };
> >
> >  struct acpi_iort_id_mapping {
> > @@ -167,10 +169,10 @@ struct acpi_iort_root_complex {
> > u8 reserved[3]; /* Reserved, must be zero */
> >  };
> >
> > -/* Values for ats_attribute field above */
> > +/* Masks for ats_attribute field above */
> >
> > -#define ACPI_IORT_ATS_SUPPORTED 0x0001 /* The root
> > complex supports ATS */
> > -#define ACPI_IORT_ATS_UNSUPPORTED   0x /* The root
> > complex doesn't support ATS */
> > +#define ACPI_IORT_ATS_SUPPORTED (1)/* The root complex
> > supports ATS */
> > +#define ACPI_IORT_PRI_SUPPORTED (1<<1) /* The root complex
> > supports PRI */
> >
> >  struct acpi_iort_smmu {
> > u64 base_address;   /* SMMU base address */
> > @@ -241,6 +243,17 @@ struct acpi_iort_pmcg {
> > u64 page1_base_address;
> >  };
> >
> > +struct acpi_iort_rmr {
> > +   u32 rmr_count;
> > +   u32 rmr_offset;
> > +};
> > +
> > +struct acpi_iort_rmr_desc {
> > +   u64 base_address;
> > +   u64 length;
> > +   u32 reserved;
> > +};
> > +
> >
> >
> /**
> *
> > 
> >   *
> >   * IVRS 

Re: [PATCH 2/3] dt-bindings: iommu: add DART iommu bindings

2021-03-22 Thread Sven Peter via iommu
Hi Rob,

On Mon, Mar 22, 2021, at 01:15, Rob Herring wrote:
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.

Sorry about that! It looks like I didn't have yamllint installed.
I have fixed the issues and will re-submit.


Thanks,

Sven

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


Re: [PATCH 0/3] iommu/iova: Add CPU hotplug handler to flush rcaches to core code

2021-03-22 Thread John Garry

On 01/03/2021 12:12, John Garry wrote:

The Intel IOMMU driver supports flushing the per-CPU rcaches when a CPU is
offlined.

Let's move it to core code, so everyone can take advantage.

Also correct a code comment.

Based on v5.12-rc1. Tested on arm64 only.


Hi guys,

Friendly reminder ...

Thanks
John



John Garry (3):
   iova: Add CPU hotplug handler to flush rcaches
   iommu/vt-d: Remove IOVA domain rcache flushing for CPU offlining
   iova: Correct comment for free_cpu_cached_iovas()

  drivers/iommu/intel/iommu.c | 31 ---
  drivers/iommu/iova.c| 32 ++--
  include/linux/cpuhotplug.h  |  2 +-
  include/linux/iova.h|  1 +
  4 files changed, 32 insertions(+), 34 deletions(-)



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


Re: [PATCH v2 1/4] iommu/vt-d: Enable write protect for supervisor SVM

2021-03-22 Thread Guenter Roeck
On Tue, Mar 02, 2021 at 02:13:57AM -0800, Jacob Pan wrote:
> Write protect bit, when set, inhibits supervisor writes to the read-only
> pages. In supervisor shared virtual addressing (SVA), where page tables
> are shared between CPU and DMA, IOMMU PASID entry WPE bit should match
> CR0.WP bit in the CPU.
> This patch sets WPE bit for supervisor PASIDs if CR0.WP is set.
> 
> Signed-off-by: Sanjay Kumar 
> Signed-off-by: Jacob Pan 
> ---

ia64:defconfig:

drivers/iommu/intel/pasid.c: In function 'pasid_enable_wpe':
drivers/iommu/intel/pasid.c:536:22: error: implicit declaration of function 
'read_cr0'
drivers/iommu/intel/pasid.c:539:23: error: 'X86_CR0_WP' undeclared

Maybe it _is_ time to retire ia64 ?

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


Re: [PATCH 1/6] iommu: Move IOVA power-of-2 roundup into allocator

2021-03-22 Thread John Garry

On 19/03/2021 19:20, Robin Murphy wrote:

Hi Robin,


So then we have the issue of how to dynamically increase this rcache
threshold. The problem is that we may have many devices associated with
the same domain. So, in theory, we can't assume that when we increase
the threshold that some other device will try to fast free an IOVA which
was allocated prior to the increase and was not rounded up.

I'm very open to better (or less bad) suggestions on how to do this ...

...but yes, regardless of exactly where it happens, rounding up or not
is the problem for rcaches in general. I've said several times that my
preferred approach is to not change it that dynamically at all, but
instead treat it more like we treat the default domain type.



Can you remind me of that idea? I don't remember you mentioning using 
default domain handling as a reference in any context.


Thanks,
John
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 0/2] VMD MSI Remapping Bypass

2021-03-22 Thread Lorenzo Pieralisi
On Wed, 10 Feb 2021 09:13:13 -0700, Jon Derrick wrote:
> The Intel Volume Management Device acts similar to a PCI-to-PCI bridge in that
> it changes downstream devices' requester-ids to its own. As VMD supports PCIe
> devices, it has its own MSI-X table and transmits child device MSI-X by
> remapping child device MSI-X and handling like a demultiplexer.
> 
> Some newer VMD devices (Icelake Server) have an option to bypass the VMD MSI-X
> remapping table. This allows for better performance scaling as the child 
> device
> MSI-X won't be limited by VMD's MSI-X count and IRQ handler.
> 
> [...]

Applied to pci/vmd, thanks!

[1/2] iommu/vt-d: Use Real PCI DMA device for IRTE
  https://git.kernel.org/lpieralisi/pci/c/9b4a824b88
[2/2] PCI: vmd: Disable MSI-X remapping when possible
  https://git.kernel.org/lpieralisi/pci/c/ee81ee84f8

Thanks,
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH V4 05/18] iommu/ioasid: Redefine IOASID set and allocation APIs

2021-03-22 Thread Jason Gunthorpe
On Fri, Mar 19, 2021 at 11:22:21AM -0700, Jacob Pan wrote:
> Hi Jason,
> 
> On Fri, 19 Mar 2021 10:54:32 -0300, Jason Gunthorpe  wrote:
> 
> > On Fri, Mar 19, 2021 at 02:41:32PM +0100, Jean-Philippe Brucker wrote:
> > > On Fri, Mar 19, 2021 at 09:46:45AM -0300, Jason Gunthorpe wrote:  
> > > > On Fri, Mar 19, 2021 at 10:58:41AM +0100, Jean-Philippe Brucker wrote:
> > > >   
> > > > > Although there is no use for it at the moment (only two upstream
> > > > > users and it looks like amdkfd always uses current too), I quite
> > > > > like the client-server model where the privileged process does
> > > > > bind() and programs the hardware queue on behalf of the client
> > > > > process.  
> > > > 
> > > > This creates a lot complexity, how do does process A get a secure
> > > > reference to B? How does it access the memory in B to setup the HW?  
> > > 
> > > mm_access() for example, and passing addresses via IPC  
> > 
> > I'd rather the source process establish its own PASID and then pass
> > the rights to use it to some other process via FD passing than try to
> > go the other way. There are lots of security questions with something
> > like mm_access.
> > 
> 
> Thank you all for the input, it sounds like we are OK to remove mm argument
> from iommu_sva_bind_device() and iommu_sva_alloc_pasid() for now?
> 
> Let me try to summarize PASID allocation as below:
> 
> Interfaces| Usage |  Limit| bind¹ |User visible
> /dev/ioasid²  | G-SVA/IOVA|  cgroup   | No|Yes
> char dev³ | SVA   |  cgroup   | Yes   |No
> iommu driver  | default PASID|  no| No|No
> kernel| super SVA | no| yes   |No
> 
> ¹ Allocated during SVA bind
> ² PASIDs allocated via /dev/ioasid are not bound to any mm. But its
>   ownership is assigned to the process that does the allocation.

What does "not bound to a mm" mean?

IMHO a use created PASID is either bound to a mm (current) at creation
time, or it will never be bound to a mm and its page table is under
user control via /dev/ioasid.

I thought the whole point of something like a /dev/ioasid was to get
away from each and every device creating its own PASID interface?

It maybe somewhat reasonable that some devices could have some easy
'make a SVA PASID on current' interface built in, but anything more
complicated should use /dev/ioasid, and anything consuming PASID
should also have an API to import and attach a PASID from /dev/ioasid.

> Currently, the proposed /dev/ioasid interface does not map individual PASID
> with an FD. The FD is at the ioasid_set granularity and bond to the current
> mm. We could extend the IOCTLs to cover individual PASID-FD passing case
> when use cases arise. Would this work?

Is it a good idea that the FD is per ioasid_set ? What is the set used
for?

Usually kernel interfaces work nicer with a one fd/one object model.

But even if it is a set, you could pass the set between co-operating
processes and the PASID can be created in the correct 'current'. But
there is all kinds of security questsions as soon as you start doing
anything like this - is there really a use case?

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

RE: [RFC PATCH v2 1/8] ACPICA: IORT: Update for revision E

2021-03-22 Thread Shameerali Kolothum Thodi
[+]

Hi Erik,

As this is now just merged ino acpica-master and based on the discussion we had 
here,

https://github.com/acpica/acpica/pull/638

I had a discussion with ARM folks(Lorenzo) in the linaro-open-discussions call 
and
can confirm that the IORT Revision E is not the final specification and has 
some issues
which is now corrected in the latest E.b revision[1]. Also there are no current 
users
for the Rev E and it may not be a good idea to push this version into the Linux 
kernel
or elsewhere.

So could you please revert the merge and I am planning to work on the E.b soon.
Please let me know if I need to explicitly send a revert pull request or not.

Thanks,
Shameer

1. https://developer.arm.com/documentation/den0049/latest/

> -Original Message-
> From: iommu [mailto:iommu-boun...@lists.linux-foundation.org] On Behalf Of
> Shameer Kolothum
> Sent: 19 November 2020 12:12
> To: linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org; de...@acpica.org
> Cc: Linuxarm ; steven.pr...@arm.com; Guohanjun
> (Hanjun Guo) ; sami.muja...@arm.com;
> robin.mur...@arm.com; wanghuiqiang 
> Subject: [RFC PATCH v2 1/8] ACPICA: IORT: Update for revision E
> 
> IORT revision E contains a few additions like,
>     -Added an identifier field in the node descriptors to aid table
>      cross-referencing.
>     -Introduced the Reserved Memory Range(RMR) node. This is used
>      to describe memory ranges that are used by endpoints and requires
>      a unity mapping in SMMU.
> -Introduced a flag in the RC node to express support for PRI.
> 
> Signed-off-by: Shameer Kolothum 
> ---
>  include/acpi/actbl2.h | 25 +++--
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index
> ec66779cb193..274fce7b5c01 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -68,7 +68,7 @@
>   * IORT - IO Remapping Table
>   *
>   * Conforms to "IO Remapping Table System Software on ARM Platforms",
> - * Document number: ARM DEN 0049D, March 2018
> + * Document number: ARM DEN 0049E, June 2020
>   *
> 
> 
> **/
> 
> @@ -86,7 +86,8 @@ struct acpi_iort_node {
>   u8 type;
>   u16 length;
>   u8 revision;
> - u32 reserved;
> + u16 reserved;
> + u16 identifier;
>   u32 mapping_count;
>   u32 mapping_offset;
>   char node_data[1];
> @@ -100,7 +101,8 @@ enum acpi_iort_node_type {
>   ACPI_IORT_NODE_PCI_ROOT_COMPLEX = 0x02,
>   ACPI_IORT_NODE_SMMU = 0x03,
>   ACPI_IORT_NODE_SMMU_V3 = 0x04,
> - ACPI_IORT_NODE_PMCG = 0x05
> + ACPI_IORT_NODE_PMCG = 0x05,
> + ACPI_IORT_NODE_RMR = 0x06,
>  };
> 
>  struct acpi_iort_id_mapping {
> @@ -167,10 +169,10 @@ struct acpi_iort_root_complex {
>   u8 reserved[3]; /* Reserved, must be zero */
>  };
> 
> -/* Values for ats_attribute field above */
> +/* Masks for ats_attribute field above */
> 
> -#define ACPI_IORT_ATS_SUPPORTED 0x0001   /* The root
> complex supports ATS */
> -#define ACPI_IORT_ATS_UNSUPPORTED   0x   /* The root
> complex doesn't support ATS */
> +#define ACPI_IORT_ATS_SUPPORTED (1)  /* The root complex
> supports ATS */
> +#define ACPI_IORT_PRI_SUPPORTED (1<<1)   /* The root complex
> supports PRI */
> 
>  struct acpi_iort_smmu {
>   u64 base_address;   /* SMMU base address */
> @@ -241,6 +243,17 @@ struct acpi_iort_pmcg {
>   u64 page1_base_address;
>  };
> 
> +struct acpi_iort_rmr {
> + u32 rmr_count;
> + u32 rmr_offset;
> +};
> +
> +struct acpi_iort_rmr_desc {
> + u64 base_address;
> + u64 length;
> + u32 reserved;
> +};
> +
> 
> /***
> 
>   *
>   * IVRS - I/O Virtualization Reporting Structure
> --
> 2.17.1
> 
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH V4 05/18] iommu/ioasid: Redefine IOASID set and allocation APIs

2021-03-22 Thread Jean-Philippe Brucker
On Fri, Mar 19, 2021 at 11:22:21AM -0700, Jacob Pan wrote:
> Hi Jason,
> 
> On Fri, 19 Mar 2021 10:54:32 -0300, Jason Gunthorpe  wrote:
> 
> > On Fri, Mar 19, 2021 at 02:41:32PM +0100, Jean-Philippe Brucker wrote:
> > > On Fri, Mar 19, 2021 at 09:46:45AM -0300, Jason Gunthorpe wrote:  
> > > > On Fri, Mar 19, 2021 at 10:58:41AM +0100, Jean-Philippe Brucker wrote:
> > > >   
> > > > > Although there is no use for it at the moment (only two upstream
> > > > > users and it looks like amdkfd always uses current too), I quite
> > > > > like the client-server model where the privileged process does
> > > > > bind() and programs the hardware queue on behalf of the client
> > > > > process.  
> > > > 
> > > > This creates a lot complexity, how do does process A get a secure
> > > > reference to B? How does it access the memory in B to setup the HW?  
> > > 
> > > mm_access() for example, and passing addresses via IPC  
> > 
> > I'd rather the source process establish its own PASID and then pass
> > the rights to use it to some other process via FD passing than try to
> > go the other way. There are lots of security questions with something
> > like mm_access.
> > 
> 
> Thank you all for the input, it sounds like we are OK to remove mm argument
> from iommu_sva_bind_device() and iommu_sva_alloc_pasid() for now?

Fine by me. By the way the IDXD currently missues the bind API for
supervisor PASID, and the drvdata parameter isn't otherwise used. This
would be a good occasion to clean both. The new bind prototype could be:

struct iommu_sva *iommu_sva_bind_device(struct device *dev, int flags)

And a flag IOMMU_SVA_BIND_SUPERVISOR (not that I plan to implement it in
the SMMU, but I think we need to clean the current usage)

> 
> Let me try to summarize PASID allocation as below:
> 
> Interfaces| Usage |  Limit| bind¹ |User visible
> 
> /dev/ioasid²  | G-SVA/IOVA|  cgroup   | No|Yes
> 
> char dev³ | SVA   |  cgroup   | Yes   |No
> 
> iommu driver  | default PASID|  no| No|No

Is this PASID #0?

> 
> kernel| super SVA | no| yes   |No
> 

Also wondering about device driver allocating auxiliary domains for their
private use, to do iommu_map/unmap on private PASIDs (a clean replacement
to super SVA, for example). Would that go through the same path as
/dev/ioasid and use the cgroup of current task?

Thanks,
Jean

> 
> ¹ Allocated during SVA bind
> ² PASIDs allocated via /dev/ioasid are not bound to any mm. But its
>   ownership is assigned to the process that does the allocation.
> ³ Include uacce, other private device driver char dev such as idxd
> 
> Currently, the proposed /dev/ioasid interface does not map individual PASID
> with an FD. The FD is at the ioasid_set granularity and bond to the current
> mm. We could extend the IOCTLs to cover individual PASID-FD passing case
> when use cases arise. Would this work?
> 
> Thanks,
> 
> Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [Linuxarm] Re: [PATCH v14 07/13] iommu/smmuv3: Implement cache_invalidate

2021-03-22 Thread Auger Eric
Hi Chenxiang,

On 3/22/21 7:40 AM, chenxiang (M) wrote:
> Hi Eric,
> 
> 
> 在 2021/3/20 1:36, Auger Eric 写道:
>> Hi Chenxiang,
>>
>> On 3/4/21 8:55 AM, chenxiang (M) wrote:
>>> Hi Eric,
>>>
>>>
>>> 在 2021/2/24 4:56, Eric Auger 写道:
 Implement domain-selective, pasid selective and page-selective
 IOTLB invalidations.

 Signed-off-by: Eric Auger 

 ---

 v13 -> v14:
 - Add domain invalidation
 - do global inval when asid is not provided with addr
    granularity

 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/arm-smmu-v3/arm-smmu-v3.c | 74
 +
   1 file changed, 74 insertions(+)

 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 4c19a1114de4..df3adc49111c 100644
 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
 +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
 @@ -2949,6 +2949,79 @@ static void
 arm_smmu_detach_pasid_table(struct iommu_domain *domain)
   mutex_unlock(_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_cmdq_ent cmd = {.opcode = CMDQ_OP_TLBI_NSNH_ALL};
 +    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_PASID ||
 +    inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
 +    return -ENOENT;
 +    }
 +
 +    if (!(inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB))
 +    return -EINVAL;
 +
 +    /* IOTLB invalidation */
 +
 +    switch (inv_info->granularity) {
 +    case IOMMU_INV_GRANU_PASID:
 +    {
 +    struct iommu_inv_pasid_info *info =
 +    _info->granu.pasid_info;
 +
 +    if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
 +    return -ENOENT;
 +    if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID))
 +    return -EINVAL;
 +
 +    __arm_smmu_tlb_inv_context(smmu_domain, info->archid);
 +    return 0;
 +    }
 +    case IOMMU_INV_GRANU_ADDR:
 +    {
 +    struct iommu_inv_addr_info *info = _info->granu.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_PASID)
 +    return -ENOENT;
 +
 +    if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
 +    break;
 +
 +    arm_smmu_tlb_inv_range_domain(info->addr, size,
 +  info->granule_size, leaf,
 +  info->archid, smmu_domain);
>>> Is it possible to add a check before the function to make sure that
>>> info->granule_size can be recognized by SMMU?
>>> There is a scenario which will cause TLBI issue: RIL feature is enabled
>>> on guest but is disabled on host, and then on
>>> host it just invalidate 4K/2M/1G once a time, but from QEMU,
>>> info->nb_granules is always 1 and info->granule_size = size,
>>> if size is not equal to 4K or 2M or 1G (for example size = granule_size
>>> is 5M), it will only part of the size it wants to invalidate.
> 
> Do you have any idea about this issue?

At the QEMU VFIO notifier level, when I fill the struct
iommu_cache_invalidate_info, I currently miss the granule info, hence
this weird choice of setting setting info->nb_granules is always 1 and
info->granule_size = size. I think in arm_smmu_cache_invalidate() I need
to convert this info back to a the leaf page size, in case the host does
not support RIL. Just as it is done in  __arm_smmu_tlb_inv_range if RIL
is supported.

Does it makes sense to you?

Thank you for spotting the issue.

Eric
> 
>>>
>>> I think maybe we can add a check here: if RIL is not enabled and also
>>> size is not the granule_size (4K/2M/1G) supported by
>>> SMMU hardware, can we just 

Re: [PATCH v2] swiotlb: Make SWIOTLB_NO_FORCE perform no allocation

2021-03-22 Thread Christoph Hellwig
On Sat, Mar 20, 2021 at 08:37:40PM -0700, Florian Fainelli wrote:
> - if (!strcmp(str, "force")) {
> + if (!strcmp(str, "force"))
>   swiotlb_force = SWIOTLB_FORCE;
> - } else if (!strcmp(str, "noforce")) {
> + else if (!strcmp(str, "noforce"))
>   swiotlb_force = SWIOTLB_NO_FORCE;
> - default_nslabs = 1;
> - }
>  
>   return 0;
>  }
> @@ -211,6 +209,9 @@ swiotlb_init(int verbose)
>   size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
>   void *tlb;
>  
> + if (swiotlb_force == SWIOTLB_NO_FORCE)
> + return;

We'll also need this in the other callers of swiotlb_init_with_tbl
and swiotlb_late_init_with_tbl.

I actually had a plan to mostly kill them, but that can better
way until more support for multiple io_tlb structures is merged.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [Linuxarm] Re: [PATCH v14 07/13] iommu/smmuv3: Implement cache_invalidate

2021-03-22 Thread chenxiang (M)

Hi Eric,


在 2021/3/20 1:36, Auger Eric 写道:

Hi Chenxiang,

On 3/4/21 8:55 AM, chenxiang (M) wrote:

Hi Eric,


在 2021/2/24 4:56, Eric Auger 写道:

Implement domain-selective, pasid selective and page-selective
IOTLB invalidations.

Signed-off-by: Eric Auger 

---

v13 -> v14:
- Add domain invalidation
- do global inval when asid is not provided with addr
   granularity

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/arm-smmu-v3/arm-smmu-v3.c | 74 +
  1 file changed, 74 insertions(+)

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 4c19a1114de4..df3adc49111c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2949,6 +2949,79 @@ static void arm_smmu_detach_pasid_table(struct 
iommu_domain *domain)
mutex_unlock(_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_cmdq_ent cmd = {.opcode = CMDQ_OP_TLBI_NSNH_ALL};
+   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_PASID ||
+   inv_info->cache & IOMMU_CACHE_INV_TYPE_DEV_IOTLB) {
+   return -ENOENT;
+   }
+
+   if (!(inv_info->cache & IOMMU_CACHE_INV_TYPE_IOTLB))
+   return -EINVAL;
+
+   /* IOTLB invalidation */
+
+   switch (inv_info->granularity) {
+   case IOMMU_INV_GRANU_PASID:
+   {
+   struct iommu_inv_pasid_info *info =
+   _info->granu.pasid_info;
+
+   if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
+   return -ENOENT;
+   if (!(info->flags & IOMMU_INV_PASID_FLAGS_ARCHID))
+   return -EINVAL;
+
+   __arm_smmu_tlb_inv_context(smmu_domain, info->archid);
+   return 0;
+   }
+   case IOMMU_INV_GRANU_ADDR:
+   {
+   struct iommu_inv_addr_info *info = _info->granu.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_PASID)
+   return -ENOENT;
+
+   if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
+   break;
+
+   arm_smmu_tlb_inv_range_domain(info->addr, size,
+ info->granule_size, leaf,
+ info->archid, smmu_domain);

Is it possible to add a check before the function to make sure that
info->granule_size can be recognized by SMMU?
There is a scenario which will cause TLBI issue: RIL feature is enabled
on guest but is disabled on host, and then on
host it just invalidate 4K/2M/1G once a time, but from QEMU,
info->nb_granules is always 1 and info->granule_size = size,
if size is not equal to 4K or 2M or 1G (for example size = granule_size
is 5M), it will only part of the size it wants to invalidate.


Do you have any idea about this issue?



I think maybe we can add a check here: if RIL is not enabled and also
size is not the granule_size (4K/2M/1G) supported by
SMMU hardware, can we just simply use the smallest granule_size
supported by hardware all the time?


+
+   arm_smmu_cmdq_issue_sync(smmu);
+   return 0;
+   }
+   case IOMMU_INV_GRANU_DOMAIN:
+   break;

I check the qemu code
(https://github.com/eauger/qemu/tree/v5.2.0-2stage-rfcv8), for opcode
CMD_TLBI_NH_ALL or CMD_TLBI_NSNH_ALL from guest OS
it calls smmu_inv_notifiers_all() to unamp all notifiers of all mr's,
but it seems not set event.entry.granularity which i think it should set
IOMMU_INV_GRAN_ADDR.

this is because IOMMU_INV_GRAN_ADDR = 0. But for clarity I should rather
set it explicitly ;-)


ok


BTW, for opcode CMD_TLBI_NH_ALL or CMD_TLBI_NSNH_ALL, it needs to unmap
size = 0x1 on 48bit system (it may spend much time),  maybe
it is better
to set it as IOMMU_INV_GRANU_DOMAIN, then in host OS, send TLBI_NH_ALL

Re: [PATCH v14 05/13] iommu/smmuv3: Implement attach/detach_pasid_table

2021-03-22 Thread Keqian Zhu
Hi Eric,

On 2021/3/19 21:15, Auger Eric wrote:
> Hi Keqian,
> 
> On 3/2/21 9:35 AM, Keqian Zhu wrote:
>> Hi Eric,
>>
>> On 2021/2/24 4:56, Eric Auger wrote:
>>> On 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.
>>>
>>> On detach, the stage 1 config is unset and the abort flag is
>>> unset.
>>>
>>> Signed-off-by: Eric Auger 
>>>
>> [...]
>>
>>> +
>>> +   /*
>>> +* we currently support a single CD so s1fmt and s1dss
>>> +* fields are also ignored
>>> +*/
>>> +   if (cfg->pasid_bits)
>>> +   goto out;
>>> +
>>> +   smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
>> only the "cdtab_dma" field of "cdcfg" is set, we are not able to locate a 
>> specific cd using arm_smmu_get_cd_ptr().
>>
>> Maybe we'd better use a specialized function to fill other fields of "cdcfg" 
>> or add a sanity check in arm_smmu_get_cd_ptr()
>> to prevent calling it under nested mode?
>>
>> As now we just call arm_smmu_get_cd_ptr() during finalise_s1(), no problem 
>> found. Just a suggestion ;-)
> 
> forgive me for the delay. yes I can indeed make sure that code is not
> called in nested mode. Please could you detail why you would need to
> call arm_smmu_get_cd_ptr()?
I accidentally called this function in nested mode when verify the smmu mpam 
feature. :)

Yes, in nested mode, context descriptor is owned by guest, hypervisor does not 
need to care about its content.
Maybe we'd better give an explicit comment for arm_smmu_get_cd_ptr() to let 
coder pay attention to this? :)

Thanks,
Keqian

> 
> Thanks
> 
> Eric
>>
>> Thanks,
>> Keqian
>>
>>
>>> +   smmu_domain->s1_cfg.set = true;
>>> +   smmu_domain->abort = false;
>>> +   break;
>>> +   default:
>>> +   goto out;
>>> +   }
>>> +   spin_lock_irqsave(_domain->devices_lock, flags);
>>> +   list_for_each_entry(master, _domain->devices, domain_head)
>>> +   arm_smmu_install_ste_for_dev(master);
>>> +   spin_unlock_irqrestore(_domain->devices_lock, flags);
>>> +   ret = 0;
>>> +out:
>>> +   mutex_unlock(_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(_domain->init_mutex);
>>> +
>>> +   if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
>>> +   goto unlock;
>>> +
>>> +   smmu_domain->s1_cfg.set = false;
>>> +   smmu_domain->abort = false;
>>> +
>>> +   spin_lock_irqsave(_domain->devices_lock, flags);
>>> +   list_for_each_entry(master, _domain->devices, domain_head)
>>> +   arm_smmu_install_ste_for_dev(master);
>>> +   spin_unlock_irqrestore(_domain->devices_lock, flags);
>>> +
>>> +unlock:
>>> +   mutex_unlock(_domain->init_mutex);
>>> +}
>>> +
>>>  static bool arm_smmu_dev_has_feature(struct device *dev,
>>>  enum iommu_dev_features feat)
>>>  {
>>> @@ -2939,6 +3026,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,
>>> .dev_has_feat   = arm_smmu_dev_has_feature,
>>> .dev_feat_enabled   = arm_smmu_dev_feature_enabled,
>>> .dev_enable_feat= arm_smmu_dev_enable_feature,
>>>
>>
> 
> .
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu