2021-04-11 11:14:40

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)

SMMUv3 Nested Stage Setup (IOMMU part)

This series brings the IOMMU part of HW nested paging support
in the SMMUv3. The VFIO part is submitted separately.

This is based on Jean-Philippe's
[PATCH v14 00/10] iommu: I/O page faults for SMMUv3
https://www.spinics.net/lists/arm-kernel/msg886518.html
(including the patches that were not pulled for 5.13)

The IOMMU API is extended to support 2 new API functionalities:
1) pass the guest stage 1 configuration
2) pass stage 1 MSI bindings

Then those capabilities gets implemented in the SMMUv3 driver.

The virtualizer passes information through the VFIO user API
which cascades them to the iommu subsystem. This allows the guest
to own stage 1 tables and context descriptors (so-called PASID
table) while the host owns stage 2 tables and main configuration
structures (STE).

Best Regards

Eric

This series can be found at:
v5.12-rc6-jean-iopf-14-2stage-v15
(including the VFIO part in its last version: v13)

The VFIO series is sent separately.

History:

Previous version:
https://github.com/eauger/linux/tree/v5.11-stallv12-2stage-v14

v14 -> v15:
- on S1 invalidation, always use CMDQ_OP_TLBI_NH_VA
independently on host ARM_SMMU_FEAT_E2H support (Zenghui)
- remove iommu/smmuv3: Accept configs with more than one
context descriptor
- Remove spurious arm_smmu_cmdq_issue_sync in
IOMMU_INV_GRANU_ADDR cache invalidation (Zenghui)
- dma-iommu.c changes induced by Zenghui's comments
including the locking rework
- fix cache invalidation when guest uses RIL
and host does not support it (Chenxiang)
- removed iommu/smmuv3: Accept configs with more than one
context descriptor (Zenghui, Shameer)
- At this point I have kept the MSI binding API.

v13 -> v14:
- Took into account all received comments I think. Great
thanks to all the testers for their effort and sometimes
tentative fixes. I am really grateful to you!
- numerous fixes including guest running in
noiommu, iommu.strict=0, iommu.passthrough=on,
enable_unsafe_noiommu_mode

v12 -> v13:
- fixed compilation issue with CONFIG_ARM_SMMU_V3_SVA
reported by Shameer. This urged me to revisit patch 4 into
iommu/smmuv3: Allow s1 and s2 configs to coexist where
s1_cfg and s2_cfg are not dynamically allocated anymore.
Instead I use a new set field in existing structs
- fixed 2 others config checks
- Updated "iommu/arm-smmu-v3: Maintain a SID->device structure"
according to the last version

v11 -> v12:
- rebase on top of v5.10-rc4


Eric Auger (12):
iommu: Introduce attach/detach_pasid_table API
iommu: Introduce bind/unbind_guest_msi
iommu/smmuv3: Allow s1 and s2 configs to coexist
iommu/smmuv3: Get prepared for nested stage support
iommu/smmuv3: Implement attach/detach_pasid_table
iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs
iommu/smmuv3: Implement cache_invalidate
dma-iommu: Implement NESTED_MSI cookie
iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement
iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI
regions
iommu/smmuv3: Implement bind/unbind_guest_msi
iommu/smmuv3: report additional recoverable faults

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 463 ++++++++++++++++++--
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 14 +-
drivers/iommu/dma-iommu.c | 180 +++++++-
drivers/iommu/iommu.c | 106 +++++
include/linux/dma-iommu.h | 16 +
include/linux/iommu.h | 47 ++
include/uapi/linux/iommu.h | 54 +++
7 files changed, 838 insertions(+), 42 deletions(-)

--
2.26.3


2021-04-11 11:15:41

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 05/12] iommu/smmuv3: Implement attach/detach_pasid_table

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 <[email protected]>

---
v14 -> v15:
- add a comment before arm_smmu_get_cd_ptr to warn the
developper this function must not be used in case of nested
(Keqian)

v13 -> v14:
- on PASID table detach, reset the abort flag (Keqian)

v7 -> v8:
- remove smmu->features check, now done on domain finalize

v6 -> v7:
- check versions and comment the fact we don't need to take
into account s1dss and s1fmt
v3 -> v4:
- adapt to changes in iommu_pasid_table_config
- different programming convention at s1_cfg/s2_cfg/ste.abort

v2 -> v3:
- callback now is named set_pasid_table and struct fields
are laid out differently.

v1 -> v2:
- invalidate the STE before changing them
- hold init_mutex
- handle new fields
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 93 +++++++++++++++++++++
1 file changed, 93 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 90763bc0c590..44cdc6df09c1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -988,6 +988,10 @@ static void arm_smmu_write_cd_l1_desc(__le64 *dst,
WRITE_ONCE(*dst, cpu_to_le64(val));
}

+/*
+ * Must not be used in case of nested mode where the CD table is owned
+ * by the guest
+ */
static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
u32 ssid)
{
@@ -2846,6 +2850,93 @@ static void arm_smmu_get_resv_regions(struct device *dev,
iommu_dma_get_resv_regions(dev, head);
}

+static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_master *master;
+ struct arm_smmu_device *smmu;
+ unsigned long flags;
+ int ret = -EINVAL;
+
+ if (cfg->format != IOMMU_PASID_FORMAT_SMMUV3)
+ return -EINVAL;
+
+ if (cfg->version != PASID_TABLE_CFG_VERSION_1 ||
+ cfg->vendor_data.smmuv3.version != PASID_TABLE_SMMUV3_CFG_VERSION_1)
+ return -EINVAL;
+
+ mutex_lock(&smmu_domain->init_mutex);
+
+ smmu = smmu_domain->smmu;
+
+ if (!smmu)
+ goto out;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto out;
+
+ switch (cfg->config) {
+ case IOMMU_PASID_CONFIG_ABORT:
+ smmu_domain->s1_cfg.set = false;
+ smmu_domain->abort = true;
+ break;
+ case IOMMU_PASID_CONFIG_BYPASS:
+ smmu_domain->s1_cfg.set = false;
+ smmu_domain->abort = false;
+ break;
+ case IOMMU_PASID_CONFIG_TRANSLATE:
+ /* we do not support S1 <-> S1 transitions */
+ if (smmu_domain->s1_cfg.set)
+ goto out;
+
+ /*
+ * we currently support a single CD so s1fmt and s1dss
+ * fields are also ignored
+ */
+ if (cfg->pasid_bits)
+ goto out;
+
+ smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
+ smmu_domain->s1_cfg.set = true;
+ smmu_domain->abort = false;
+ break;
+ default:
+ goto out;
+ }
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head)
+ arm_smmu_install_ste_for_dev(master);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+ ret = 0;
+out:
+ mutex_unlock(&smmu_domain->init_mutex);
+ return ret;
+}
+
+static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_master *master;
+ unsigned long flags;
+
+ mutex_lock(&smmu_domain->init_mutex);
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto unlock;
+
+ smmu_domain->s1_cfg.set = false;
+ smmu_domain->abort = false;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head)
+ arm_smmu_install_ste_for_dev(master);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+unlock:
+ mutex_unlock(&smmu_domain->init_mutex);
+}
+
static bool arm_smmu_dev_has_feature(struct device *dev,
enum iommu_dev_features feat)
{
@@ -2943,6 +3034,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,
--
2.26.3

2021-04-11 11:16:16

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 08/12] dma-iommu: Implement NESTED_MSI cookie

Up to now, when the type was UNMANAGED, we used to
allocate IOVA pages within a reserved IOVA MSI range.

If both the host and the guest are exposed with SMMUs, each
would allocate an IOVA. The guest allocates an IOVA (gIOVA)
to map onto the guest MSI doorbell (gDB). The Host allocates
another IOVA (hIOVA) to map onto the physical doorbell (hDB).

So we end up with 2 unrelated mappings, at S1 and S2:
S1 S2
gIOVA -> gDB
hIOVA -> hDB

The PCI device would be programmed with hIOVA.
No stage 1 mapping would existing, causing the MSIs to fault.

iommu_dma_bind_guest_msi() allows to pass gIOVA/gDB
to the host so that gIOVA can be used by the host instead of
re-allocating a new hIOVA.

S1 S2
gIOVA -> gDB -> hDB

this time, the PCI device can be programmed with the gIOVA MSI
doorbell which is correctly mapped through both stages.

Nested mode is not compatible with HW MSI regions as in that
case gDB and hDB should have a 1-1 mapping. This check will
be done when attaching each device to the IOMMU domain.

Signed-off-by: Eric Auger <[email protected]>

---

v14 -> v15:
Took into account Zenghui's comments
- remove duplicated mutex.h inclusion
- introduce iommu_dma_get_nested_msi_page(), take the spinlock there
- add a comment saying the msi_lock only is used in nested mode
- take the msi_lock in other places
- fix prot
- check the S1 granule is smaller than S2 one
- remove s2_unamp
- do not init msi_iova in nested mode

v10 -> v11:
- fix compilation if !CONFIG_IOMMU_DMA

v7 -> v8:
- correct iommu_dma_(un)bind_guest_msi when
!CONFIG_IOMMU_DMA
- Mentioned nested mode is not compatible with HW MSI regions
in commit message
- protect with msi_lock on unbind

v6 -> v7:
- removed device handle

v3 -> v4:
- change function names; add unregister
- protect with msi_lock

v2 -> v3:
- also store the device handle on S1 mapping registration.
This garantees we associate the associated S2 mapping binds
to the correct physical MSI controller.

v1 -> v2:
- unmap stage2 on put()
---
drivers/iommu/dma-iommu.c | 180 +++++++++++++++++++++++++++++++++++++-
include/linux/dma-iommu.h | 16 ++++
2 files changed, 192 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index af765c813cc8..9d77c62208bd 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -29,12 +29,15 @@
struct iommu_dma_msi_page {
struct list_head list;
dma_addr_t iova;
+ dma_addr_t gpa;
phys_addr_t phys;
+ size_t s1_granule;
};

enum iommu_dma_cookie_type {
IOMMU_DMA_IOVA_COOKIE,
IOMMU_DMA_MSI_COOKIE,
+ IOMMU_DMA_NESTED_MSI_COOKIE,
};

struct iommu_dma_cookie {
@@ -46,6 +49,8 @@ struct iommu_dma_cookie {
dma_addr_t msi_iova;
};
struct list_head msi_page_list;
+ /* used in nested mode only */
+ spinlock_t msi_lock;

/* Domain for flush queue callback; NULL if flush queue not in use */
struct iommu_domain *fq_domain;
@@ -87,6 +92,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)

cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
if (cookie) {
+ spin_lock_init(&cookie->msi_lock);
INIT_LIST_HEAD(&cookie->msi_page_list);
cookie->type = type;
}
@@ -120,14 +126,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
*
* Users who manage their own IOVA allocation and do not want DMA API support,
* but would still like to take advantage of automatic MSI remapping, can use
- * this to initialise their own domain appropriately. Users should reserve a
+ * this to initialise their own domain appropriately. Users may reserve a
* contiguous IOVA region, starting at @base, large enough to accommodate the
* number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
- * used by the devices attached to @domain.
+ * used by the devices attached to @domain. The other way round is to provide
+ * usable iova pages through the iommu_dma_bind_guest_msi API (nested stages
+ * use case)
*/
int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
{
struct iommu_dma_cookie *cookie;
+ int nesting, ret;

if (domain->type != IOMMU_DOMAIN_UNMANAGED)
return -EINVAL;
@@ -135,11 +144,17 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
if (domain->iova_cookie)
return -EEXIST;

- cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+ ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
+ if (!ret && nesting)
+ cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
+ else
+ cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+
if (!cookie)
return -ENOMEM;

- cookie->msi_iova = base;
+ if (!nesting)
+ cookie->msi_iova = base;
domain->iova_cookie = cookie;
return 0;
}
@@ -163,15 +178,116 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
put_iova_domain(&cookie->iovad);

+ spin_lock(&cookie->msi_lock);
list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
+ if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE && msi->phys) {
+ size_t size = cookie_msi_granule(cookie);
+
+ WARN_ON(iommu_unmap(domain, msi->gpa, size) != size);
+ }
list_del(&msi->list);
kfree(msi);
}
+ spin_unlock(&cookie->msi_lock);
kfree(cookie);
domain->iova_cookie = NULL;
}
EXPORT_SYMBOL(iommu_put_dma_cookie);

+/**
+ * iommu_dma_bind_guest_msi - Allows to pass the stage 1
+ * binding of a virtual MSI doorbell used by @dev.
+ *
+ * @domain: domain handle
+ * @giova: guest iova
+ * @gpa: gpa of the virtual doorbell
+ * @size: size of the granule used for the stage1 mapping
+ *
+ * In nested stage use case, the user can provide IOVA/IPA bindings
+ * corresponding to a guest MSI stage 1 mapping. When the host needs
+ * to map its own MSI doorbells, it can use @gpa as stage 2 input
+ * and map it onto the physical MSI doorbell.
+ */
+int iommu_dma_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *msi;
+ int ret = 0;
+
+ if (!cookie)
+ return -EINVAL;
+
+ if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
+ return -EINVAL;
+
+ /*
+ * we currently do not support S1 granule larger than S2 one
+ * as this would oblige to have multiple S2 mappings for a
+ * single S1 one
+ */
+ if (size > cookie_msi_granule(cookie))
+ return -EINVAL;
+
+ giova = giova & ~(dma_addr_t)(size - 1);
+ gpa = gpa & ~(phys_addr_t)(size - 1);
+
+ spin_lock(&cookie->msi_lock);
+
+ list_for_each_entry(msi, &cookie->msi_page_list, list) {
+ if (msi->iova == giova)
+ goto unlock; /* this page is already registered */
+ }
+
+ msi = kzalloc(sizeof(*msi), GFP_ATOMIC);
+ if (!msi) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ msi->iova = giova;
+ msi->gpa = gpa;
+ msi->s1_granule = size;
+ list_add(&msi->list, &cookie->msi_page_list);
+unlock:
+ spin_unlock(&cookie->msi_lock);
+ return ret;
+}
+EXPORT_SYMBOL(iommu_dma_bind_guest_msi);
+
+void iommu_dma_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *msi;
+
+ if (!cookie)
+ return;
+
+ if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
+ return;
+
+ spin_lock(&cookie->msi_lock);
+
+ list_for_each_entry(msi, &cookie->msi_page_list, list) {
+ dma_addr_t aligned_giova =
+ giova & ~(dma_addr_t)(msi->s1_granule - 1);
+
+ if (msi->iova == aligned_giova) {
+ if (msi->phys) {
+ /* unmap the stage 2 */
+ size_t size = cookie_msi_granule(cookie);
+
+ WARN_ON(iommu_unmap(domain, msi->gpa, size) != size);
+ }
+ list_del(&msi->list);
+ kfree(msi);
+ break;
+ }
+ }
+ spin_unlock(&cookie->msi_lock);
+}
+EXPORT_SYMBOL(iommu_dma_unbind_guest_msi);
+
/**
* iommu_dma_get_resv_regions - Reserved region driver helper
* @dev: Device from iommu_get_resv_regions()
@@ -1300,6 +1416,58 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size)
dev_name(dev));
}

+/*
+ * iommu_dma_get_nested_msi_page - Returns a nested stage MSI page
+ * mapping translating into the physical doorbell address @msi_addr
+ *
+ * In nested mode, the userspace provides the guest
+ * gIOVA - gDB stage 1 mappings. When we need to build a stage 2
+ * mapping for a physical doorbell (@msi_addr), we look up
+ * for an unused S1 mapping and map the gDB onto @msi_addr
+ */
+static struct iommu_dma_msi_page *
+iommu_dma_get_nested_msi_page(struct iommu_domain *domain,
+ phys_addr_t msi_addr)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *iter, *msi_page = NULL;
+ size_t size = cookie_msi_granule(cookie);
+ int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+ spin_lock(&cookie->msi_lock);
+ list_for_each_entry(iter, &cookie->msi_page_list, list)
+ if (iter->phys == msi_addr) {
+ msi_page = iter;
+ goto unlock;
+ }
+
+ /*
+ * No nested mapping exists for the physical doorbell,
+ * look for an unused S1 mapping
+ */
+ list_for_each_entry(iter, &cookie->msi_page_list, list) {
+ int ret;
+
+ if (iter->phys)
+ continue;
+
+ /* do the stage 2 mapping */
+ ret = iommu_map_atomic(domain, iter->gpa, msi_addr, size, prot);
+ if (ret) {
+ pr_warn_once("MSI S2 mapping 0x%llx -> 0x%llx failed (%d)\n",
+ iter->gpa, msi_addr, ret);
+ goto unlock;
+ }
+ iter->phys = msi_addr;
+ msi_page = iter;
+ goto unlock;
+ }
+ pr_warn_once("No usable S1 MSI mapping found\n");
+unlock:
+ spin_unlock(&cookie->msi_lock);
+ return msi_page;
+}
+
static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
phys_addr_t msi_addr, struct iommu_domain *domain)
{
@@ -1310,6 +1478,10 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
size_t size = cookie_msi_granule(cookie);

msi_addr &= ~(phys_addr_t)(size - 1);
+
+ if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE)
+ return iommu_dma_get_nested_msi_page(domain, msi_addr);
+
list_for_each_entry(msi_page, &cookie->msi_page_list, list)
if (msi_page->phys == msi_addr)
return msi_page;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 706b68d1359b..7bd785e68477 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -12,6 +12,7 @@
#include <linux/dma-mapping.h>
#include <linux/iommu.h>
#include <linux/msi.h>
+#include <uapi/linux/iommu.h>

/* Domain management interface for IOMMU drivers */
int iommu_get_dma_cookie(struct iommu_domain *domain);
@@ -36,6 +37,9 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc,
struct msi_msg *msg);

void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
+int iommu_dma_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t iova, phys_addr_t gpa, size_t size);
+void iommu_dma_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova);

void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
struct iommu_domain *domain);
@@ -77,6 +81,18 @@ static inline void iommu_dma_compose_msi_msg(struct msi_desc *desc,
{
}

+static inline int
+iommu_dma_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t iova, phys_addr_t gpa, size_t size)
+{
+ return -ENODEV;
+}
+
+static inline void
+iommu_dma_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova)
+{
+}
+
static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
{
}
--
2.26.3

2021-04-11 11:16:28

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 09/12] iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement

In nested mode we enforce the rule that all devices belonging
to the same iommu_domain share the same msi_domain.

Indeed if there were several physical MSI doorbells being used
within a single iommu_domain, it becomes really difficult to
resolve the nested stage mapping translating into the correct
physical doorbell. So let's forbid this situation.

Signed-off-by: Eric Auger <[email protected]>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 41 +++++++++++++++++++++
1 file changed, 41 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 bfc112cc0d38..c4794c21c35f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2441,6 +2441,37 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
arm_smmu_install_ste_for_dev(master);
}

+static bool arm_smmu_share_msi_domain(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct irq_domain *irqd = dev_get_msi_domain(dev);
+ struct arm_smmu_master *master;
+ unsigned long flags;
+ bool share = false;
+
+ if (!irqd)
+ return true;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+ struct irq_domain *d = dev_get_msi_domain(master->dev);
+
+ if (!d)
+ continue;
+ if (irqd != d) {
+ dev_info(dev, "Nested mode forbids to attach devices "
+ "using different physical MSI doorbells "
+ "to the same iommu_domain");
+ goto unlock;
+ }
+ }
+ share = true;
+unlock:
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+ return share;
+}
+
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret = 0;
@@ -2498,6 +2529,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
ret = -EINVAL;
goto out_unlock;
}
+ /*
+ * In nested mode we must check all devices belonging to the
+ * domain share the same physical MSI doorbell. Otherwise nested
+ * stage MSI binding is not supported.
+ */
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED &&
+ !arm_smmu_share_msi_domain(domain, dev)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }

master->domain = smmu_domain;

--
2.26.3

2021-04-11 11:17:16

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 11/12] iommu/smmuv3: Implement bind/unbind_guest_msi

The bind/unbind_guest_msi() callbacks check the domain
is NESTED and redirect to the dma-iommu implementation.

Signed-off-by: Eric Auger <[email protected]>

---

v6 -> v7:
- remove device handle argument
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 43 +++++++++++++++++++++
1 file changed, 43 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 ec75219d6a52..3b0a67434f7d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2934,6 +2934,47 @@ static void arm_smmu_get_resv_regions(struct device *dev,
iommu_dma_get_resv_regions(dev, head);
}

+static int
+arm_smmu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu;
+ int ret = -EINVAL;
+
+ mutex_lock(&smmu_domain->init_mutex);
+ smmu = smmu_domain->smmu;
+ if (!smmu)
+ goto out;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto out;
+
+ ret = iommu_dma_bind_guest_msi(domain, giova, gpa, size);
+out:
+ mutex_unlock(&smmu_domain->init_mutex);
+ return ret;
+}
+
+static void
+arm_smmu_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu;
+
+ mutex_lock(&smmu_domain->init_mutex);
+ smmu = smmu_domain->smmu;
+ if (!smmu)
+ goto unlock;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto unlock;
+
+ iommu_dma_unbind_guest_msi(domain, giova);
+unlock:
+ mutex_unlock(&smmu_domain->init_mutex);
+}
+
static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
struct iommu_pasid_table_config *cfg)
{
@@ -3209,6 +3250,8 @@ static struct iommu_ops arm_smmu_ops = {
.attach_pasid_table = arm_smmu_attach_pasid_table,
.detach_pasid_table = arm_smmu_detach_pasid_table,
.cache_invalidate = arm_smmu_cache_invalidate,
+ .bind_guest_msi = arm_smmu_bind_guest_msi,
+ .unbind_guest_msi = arm_smmu_unbind_guest_msi,
.dev_has_feat = arm_smmu_dev_has_feature,
.dev_feat_enabled = arm_smmu_dev_feature_enabled,
.dev_enable_feat = arm_smmu_dev_enable_feature,
--
2.26.3

2021-04-11 11:17:46

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 10/12] iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI regions

Nested mode currently is not compatible with HW MSI reserved regions.
Indeed MSI transactions targeting this MSI doorbells bypass the SMMU.

Let's check nested mode is not attempted in such configuration.

Signed-off-by: Eric Auger <[email protected]>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 23 +++++++++++++++++++--
1 file changed, 21 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 c4794c21c35f..ec75219d6a52 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2472,6 +2472,23 @@ static bool arm_smmu_share_msi_domain(struct iommu_domain *domain,
return share;
}

+static bool arm_smmu_has_hw_msi_resv_region(struct device *dev)
+{
+ struct iommu_resv_region *region;
+ bool has_msi_resv_region = false;
+ LIST_HEAD(resv_regions);
+
+ iommu_get_resv_regions(dev, &resv_regions);
+ list_for_each_entry(region, &resv_regions, list) {
+ if (region->type == IOMMU_RESV_MSI) {
+ has_msi_resv_region = true;
+ break;
+ }
+ }
+ iommu_put_resv_regions(dev, &resv_regions);
+ return has_msi_resv_region;
+}
+
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret = 0;
@@ -2532,10 +2549,12 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
/*
* In nested mode we must check all devices belonging to the
* domain share the same physical MSI doorbell. Otherwise nested
- * stage MSI binding is not supported.
+ * stage MSI binding is not supported. Also nested mode is not
+ * compatible with MSI HW reserved regions.
*/
if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED &&
- !arm_smmu_share_msi_domain(domain, dev)) {
+ (!arm_smmu_share_msi_domain(domain, dev) ||
+ arm_smmu_has_hw_msi_resv_region(dev))) {
ret = -EINVAL;
goto out_unlock;
}
--
2.26.3

2021-04-11 11:17:47

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 07/12] iommu/smmuv3: Implement cache_invalidate

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

Signed-off-by: Eric Auger <[email protected]>

---
v4 -> v15:
- remove the redundant arm_smmu_cmdq_issue_sync(smmu)
in IOMMU_INV_GRANU_ADDR case (Zenghui)
- if RIL is not supported by the host, make sure the granule_size
that is passed by the userspace is supported or fix it
(Chenxiang)

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 | 89 +++++++++++++++++++++
1 file changed, 89 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 56a301fbe75a..bfc112cc0d38 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2961,6 +2961,94 @@ static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
mutex_unlock(&smmu_domain->init_mutex);
}

+static int
+arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info)
+{
+ struct arm_smmu_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 =
+ &inv_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 = &inv_info->granu.addr_info;
+ size_t granule_size = info->granule_size;
+ size_t size = info->nb_granules * info->granule_size;
+ bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
+ int tg;
+
+ if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
+ return -ENOENT;
+
+ if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
+ break;
+
+ tg = __ffs(granule_size);
+ if (granule_size & ~(1 << tg))
+ return -EINVAL;
+ /*
+ * When RIL is not supported, make sure the granule size that is
+ * passed is supported. In RIL mode, this is enforced in
+ * __arm_smmu_tlb_inv_range()
+ */
+ if (!(smmu->features & ARM_SMMU_FEAT_RANGE_INV) &&
+ !(granule_size & smmu_domain->domain.pgsize_bitmap)) {
+ tg = __ffs(smmu_domain->domain.pgsize_bitmap);
+ granule_size = 1 << tg;
+ size = size >> tg;
+ }
+
+ arm_smmu_tlb_inv_range_domain(info->addr, size,
+ granule_size, leaf,
+ info->archid, smmu_domain);
+ return 0;
+ }
+ case IOMMU_INV_GRANU_DOMAIN:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Global S1 invalidation */
+ cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
+ arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+ arm_smmu_cmdq_issue_sync(smmu);
+ return 0;
+}
+
static bool arm_smmu_dev_has_feature(struct device *dev,
enum iommu_dev_features feat)
{
@@ -3060,6 +3148,7 @@ static struct iommu_ops arm_smmu_ops = {
.put_resv_regions = generic_iommu_put_resv_regions,
.attach_pasid_table = arm_smmu_attach_pasid_table,
.detach_pasid_table = arm_smmu_detach_pasid_table,
+ .cache_invalidate = arm_smmu_cache_invalidate,
.dev_has_feat = arm_smmu_dev_has_feature,
.dev_feat_enabled = arm_smmu_dev_feature_enabled,
.dev_enable_feat = arm_smmu_dev_enable_feature,
--
2.26.3

2021-04-11 11:18:06

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 12/12] iommu/smmuv3: report additional recoverable faults

Up to now we have only reported translation faults. Now that
the guest can induce some configuration faults, let's report them
too. Add propagation for BAD_SUBSTREAMID, CD_FETCH, BAD_CD, WALK_EABT.
We also fix the transcoding for some existing translation faults.

Signed-off-by: Eric Auger <[email protected]>

---

v14 -> v15:
- adapt to removal of IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID
in [PATCH v13 10/10] iommu/arm-smmu-v3: Add stall support for
platform devices
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 40 +++++++++++++++++++--
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 +++
2 files changed, 42 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 3b0a67434f7d..d5c39c8e95b0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1473,6 +1473,7 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
u32 perm = 0;
struct arm_smmu_master *master;
bool ssid_valid = evt[0] & EVTQ_0_SSV;
+ u8 type = FIELD_GET(EVTQ_0_ID, evt[0]);
u32 sid = FIELD_GET(EVTQ_0_SID, evt[0]);
struct iommu_fault_event fault_evt = { };
struct iommu_fault *flt = &fault_evt.fault;
@@ -1525,8 +1526,6 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
} else {
flt->type = IOMMU_FAULT_DMA_UNRECOV;
flt->event = (struct iommu_fault_unrecoverable) {
- .reason = reason,
- .flags = IOMMU_FAULT_UNRECOV_ADDR_VALID,
.perm = perm,
.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
};
@@ -1535,6 +1534,43 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
flt->event.flags |= IOMMU_FAULT_UNRECOV_PASID_VALID;
flt->event.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
}
+
+ switch (type) {
+ case EVT_ID_TRANSLATION_FAULT:
+ flt->event.reason = IOMMU_FAULT_REASON_PTE_FETCH;
+ flt->event.flags |= IOMMU_FAULT_UNRECOV_ADDR_VALID;
+ break;
+ case EVT_ID_ADDR_SIZE_FAULT:
+ flt->event.reason = IOMMU_FAULT_REASON_OOR_ADDRESS;
+ flt->event.flags |= IOMMU_FAULT_UNRECOV_ADDR_VALID;
+ break;
+ case EVT_ID_ACCESS_FAULT:
+ flt->event.reason = IOMMU_FAULT_REASON_ACCESS;
+ flt->event.flags |= IOMMU_FAULT_UNRECOV_ADDR_VALID;
+ break;
+ case EVT_ID_PERMISSION_FAULT:
+ flt->event.reason = IOMMU_FAULT_REASON_PERMISSION;
+ flt->event.flags |= IOMMU_FAULT_UNRECOV_ADDR_VALID;
+ break;
+ case EVT_ID_BAD_SUBSTREAMID:
+ flt->event.reason = IOMMU_FAULT_REASON_PASID_INVALID;
+ break;
+ case EVT_ID_CD_FETCH:
+ flt->event.reason = IOMMU_FAULT_REASON_PASID_FETCH;
+ flt->event.flags |= IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID;
+ break;
+ case EVT_ID_BAD_CD:
+ flt->event.reason = IOMMU_FAULT_REASON_BAD_PASID_ENTRY;
+ break;
+ case EVT_ID_WALK_EABT:
+ flt->event.reason = IOMMU_FAULT_REASON_WALK_EABT;
+ flt->event.flags |= IOMMU_FAULT_UNRECOV_ADDR_VALID |
+ IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID;
+ break;
+ default:
+ /* TODO: report other unrecoverable faults. */
+ return -EFAULT;
+ }
}

mutex_lock(&smmu->streams_mutex);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index eb0cc08e8240..9c37dbec75b2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -378,6 +378,10 @@

#define EVTQ_0_ID GENMASK_ULL(7, 0)

+#define EVT_ID_BAD_SUBSTREAMID 0x08
+#define EVT_ID_CD_FETCH 0x09
+#define EVT_ID_BAD_CD 0x0a
+#define EVT_ID_WALK_EABT 0x0b
#define EVT_ID_TRANSLATION_FAULT 0x10
#define EVT_ID_ADDR_SIZE_FAULT 0x11
#define EVT_ID_ACCESS_FAULT 0x12
--
2.26.3

2021-04-11 11:27:04

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 02/12] iommu: Introduce bind/unbind_guest_msi

On ARM, MSI are translated by the SMMU. An IOVA is allocated
for each MSI doorbell. If both the host and the guest are exposed
with SMMUs, we end up with 2 different IOVAs allocated by each.
guest allocates an IOVA (gIOVA) to map onto the guest MSI
doorbell (gDB). The Host allocates another IOVA (hIOVA) to map
onto the physical doorbell (hDB).

So we end up with 2 untied mappings:
S1 S2
gIOVA -> gDB
hIOVA -> hDB

Currently the PCI device is programmed by the host with hIOVA
as MSI doorbell. So this does not work.

This patch introduces an API to pass gIOVA/gDB to the host so
that gIOVA can be reused by the host instead of re-allocating
a new IOVA. So the goal is to create the following nested mapping:

S1 S2
gIOVA -> gDB -> hDB

and program the PCI device with gIOVA MSI doorbell.

In case we have several devices attached to this nested domain
(devices belonging to the same group), they cannot be isolated
on guest side either. So they should also end up in the same domain
on guest side. We will enforce that all the devices attached to
the host iommu domain use the same physical doorbell and similarly
a single virtual doorbell mapping gets registered (1 single
virtual doorbell is used on guest as well).

Signed-off-by: Eric Auger <[email protected]>

---

v13 -> v14:
- s/iova/giova in iommu_unbind_guest_msi proto (Kequian)

v7 -> v8:
- dummy iommu_unbind_guest_msi turned into a void function

v6 -> v7:
- remove the device handle parameter.
- Add comments saying there can only be a single MSI binding
registered per iommu_domain
v5 -> v6:
-fix compile issue when IOMMU_API is not set

v3 -> v4:
- add unbind

v2 -> v3:
- add a struct device handle
---
drivers/iommu/iommu.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 20 ++++++++++++++++++++
2 files changed, 57 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 90bacf000789..1853279216eb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2282,6 +2282,43 @@ static void __iommu_detach_device(struct iommu_domain *domain,
trace_detach_device_from_domain(dev);
}

+/**
+ * iommu_bind_guest_msi - Passes the stage1 GIOVA/GPA mapping of a
+ * virtual doorbell
+ *
+ * @domain: iommu domain the stage 1 mapping will be attached to
+ * @iova: iova allocated by the guest
+ * @gpa: guest physical address of the virtual doorbell
+ * @size: granule size used for the mapping
+ *
+ * The associated IOVA can be reused by the host to create a nested
+ * stage2 binding mapping translating into the physical doorbell used
+ * by the devices attached to the domain.
+ *
+ * All devices within the domain must share the same physical doorbell.
+ * A single MSI GIOVA/GPA mapping can be attached to an iommu_domain.
+ */
+
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+ if (unlikely(!domain->ops->bind_guest_msi))
+ return -ENODEV;
+
+ return domain->ops->bind_guest_msi(domain, giova, gpa, size);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
+
+void iommu_unbind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova)
+{
+ if (unlikely(!domain->ops->unbind_guest_msi))
+ return;
+
+ domain->ops->unbind_guest_msi(domain, giova);
+}
+EXPORT_SYMBOL_GPL(iommu_unbind_guest_msi);
+
void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
{
struct iommu_group *group;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index c4422975359e..72bda5d93951 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -241,6 +241,8 @@ struct iommu_iotlb_gather {
* @sva_unbind_gpasid: unbind guest pasid and mm
* @attach_pasid_table: attach a pasid table
* @detach_pasid_table: detach the pasid table
+ * @bind_guest_msi: provides a stage1 giova/gpa MSI doorbell mapping
+ * @unbind_guest_msi: withdraw a stage1 giova/gpa MSI doorbell mapping
* @def_domain_type: device default domain type, return value:
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
* - IOMMU_DOMAIN_DMA: must use a dma domain
@@ -322,6 +324,10 @@ struct iommu_ops {

int (*def_domain_type)(struct device *dev);

+ int (*bind_guest_msi)(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size);
+ void (*unbind_guest_msi)(struct iommu_domain *domain, dma_addr_t giova);
+
unsigned long pgsize_bitmap;
struct module *owner;
};
@@ -464,6 +470,10 @@ extern int iommu_attach_pasid_table(struct iommu_domain *domain,
extern int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
void __user *udata);
extern void iommu_detach_pasid_table(struct iommu_domain *domain);
+extern int iommu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size);
+extern void iommu_unbind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1101,6 +1111,16 @@ static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
}
+
+static inline
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+ dma_addr_t giova, phys_addr_t gpa, size_t size)
+{
+ return -ENODEV;
+}
+static inline
+void iommu_unbind_guest_msi(struct iommu_domain *domain, dma_addr_t giova) {}
+
#endif /* CONFIG_IOMMU_API */

/**
--
2.26.3

2021-04-11 11:29:02

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 04/12] iommu/smmuv3: Get prepared for nested stage support

When nested stage translation is setup, both s1_cfg and
s2_cfg are set.

We introduce a new smmu_domain abort field that will be set
upon guest stage1 configuration passing. If no guest stage1
config has been attached, it is ignored when writing the STE.

arm_smmu_write_strtab_ent() is modified to write both stage
fields in the STE and deal with the abort field.

In nested mode, only stage 2 is "finalized" as the host does
not own/configure the stage 1 context descriptor; guest does.

Signed-off-by: Eric Auger <[email protected]>

---
v13 -> v14:
- removed BUG_ON(ste_live && !nested) as this should never happen
- restored the old comment as there is always an abort in between
S2 -> S1 + S2 and S1 + S2 -> S2
- remove sparse warning

v10 -> v11:
- Fix an issue reported by Shameer when switching from with vSMMU
to without vSMMU. Despite the spec does not seem to mention it
seems to be needed to reset the 2 high 64b when switching from
S1+S2 cfg to S1 only. Especially dst[3] needs to be reset (S2TTB).
On some implementations, if the S2TTB is not reset, this causes
a C_BAD_STE error
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 55 ++++++++++++++++++---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +
2 files changed, 49 insertions(+), 8 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 22fb39a9ef31..90763bc0c590 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1237,7 +1237,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
* 3. Update Config, sync
*/
u64 val = le64_to_cpu(dst[0]);
- bool ste_live = false;
+ bool s1_live = false, s2_live = false, ste_live;
+ bool abort, translate = false;
struct arm_smmu_device *smmu = NULL;
struct arm_smmu_s1_cfg *s1_cfg;
struct arm_smmu_s2_cfg *s2_cfg;
@@ -1277,6 +1278,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
default:
break;
}
+ translate = s1_cfg->set || s2_cfg->set;
}

if (val & STRTAB_STE_0_V) {
@@ -1284,23 +1286,36 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
case STRTAB_STE_0_CFG_BYPASS:
break;
case STRTAB_STE_0_CFG_S1_TRANS:
+ s1_live = true;
+ break;
case STRTAB_STE_0_CFG_S2_TRANS:
- ste_live = true;
+ s2_live = true;
+ break;
+ case STRTAB_STE_0_CFG_NESTED:
+ s1_live = true;
+ s2_live = true;
break;
case STRTAB_STE_0_CFG_ABORT:
- BUG_ON(!disable_bypass);
break;
default:
BUG(); /* STE corruption */
}
}

+ ste_live = s1_live || s2_live;
+
/* Nuke the existing STE_0 value, as we're going to rewrite it */
val = STRTAB_STE_0_V;

/* Bypass/fault */
- if (!smmu_domain || !(s1_cfg->set || s2_cfg->set)) {
- if (!smmu_domain && disable_bypass)
+
+ if (!smmu_domain)
+ abort = disable_bypass;
+ else
+ abort = smmu_domain->abort;
+
+ if (abort || !translate) {
+ if (abort)
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
else
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
@@ -1318,11 +1333,17 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
return;
}

+ if (ste_live) {
+ /* First invalidate the live STE */
+ dst[0] = cpu_to_le64(STRTAB_STE_0_CFG_ABORT);
+ arm_smmu_sync_ste_for_sid(smmu, sid);
+ }
+
if (s1_cfg->set) {
u64 strw = smmu->features & ARM_SMMU_FEAT_E2H ?
STRTAB_STE_1_STRW_EL2 : STRTAB_STE_1_STRW_NSEL1;

- BUG_ON(ste_live);
+ BUG_ON(s1_live);
dst[1] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
@@ -1341,7 +1362,14 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
}

if (s2_cfg->set) {
- BUG_ON(ste_live);
+ u64 vttbr = s2_cfg->vttbr & STRTAB_STE_3_S2TTB_MASK;
+
+ if (s2_live) {
+ u64 s2ttb = le64_to_cpu(dst[3]) & STRTAB_STE_3_S2TTB_MASK;
+
+ BUG_ON(s2ttb != vttbr);
+ }
+
dst[2] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_2_S2VMID, s2_cfg->vmid) |
FIELD_PREP(STRTAB_STE_2_VTCR, s2_cfg->vtcr) |
@@ -1351,9 +1379,12 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
STRTAB_STE_2_S2PTW | STRTAB_STE_2_S2AA64 |
STRTAB_STE_2_S2R);

- dst[3] = cpu_to_le64(s2_cfg->vttbr & STRTAB_STE_3_S2TTB_MASK);
+ dst[3] = cpu_to_le64(vttbr);

val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S2_TRANS);
+ } else {
+ dst[2] = 0;
+ dst[3] = 0;
}

if (master->ats_enabled)
@@ -2158,6 +2189,14 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
return 0;
}

+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED &&
+ (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1) ||
+ !(smmu->features & ARM_SMMU_FEAT_TRANS_S2))) {
+ dev_info(smmu_domain->smmu->dev,
+ "does not implement two stages\n");
+ return -EINVAL;
+ }
+
/* Restrict the stage to what we can actually support */
if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index ec2b77596b6a..eb0cc08e8240 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -206,6 +206,7 @@
#define STRTAB_STE_0_CFG_BYPASS 4
#define STRTAB_STE_0_CFG_S1_TRANS 5
#define STRTAB_STE_0_CFG_S2_TRANS 6
+#define STRTAB_STE_0_CFG_NESTED 7

#define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)
#define STRTAB_STE_0_S1FMT_LINEAR 0
@@ -722,6 +723,7 @@ struct arm_smmu_domain {
enum arm_smmu_domain_stage stage;
struct arm_smmu_s1_cfg s1_cfg;
struct arm_smmu_s2_cfg s2_cfg;
+ bool abort;

struct iommu_domain domain;

--
2.26.3

2021-04-11 11:35:05

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 01/12] iommu: Introduce attach/detach_pasid_table API

In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on the guest,
the physical IOMMU must be programmed to be consistent with
the guest mappings. If the physical IOMMU supports two
translation stages it makes sense to program guest mappings
onto the first stage/level (ARM/Intel terminology) while the host
owns the stage/level 2.

In that case, it is mandated to trap on guest configuration
settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows
to set/unset the pasid table information.

A generic iommu_pasid_table_config struct is introduced in
a new iommu.h uapi header. This is going to be used by the VFIO
user API.

Signed-off-by: Jean-Philippe Brucker <[email protected]>
Signed-off-by: Liu, Yi L <[email protected]>
Signed-off-by: Ashok Raj <[email protected]>
Signed-off-by: Jacob Pan <[email protected]>
Signed-off-by: Eric Auger <[email protected]>

---

v13 -> v14:
- export iommu_attach_pasid_table
- add dummy iommu_uapi_attach_pasid_table
- swap base_ptr and format in iommu_pasid_table_config

v12 -> v13:
- Fix config check

v11 -> v12:
- add argsz, name the union
---
drivers/iommu/iommu.c | 69 ++++++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 27 +++++++++++++++
include/uapi/linux/iommu.h | 54 +++++++++++++++++++++++++++++
3 files changed, 150 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d0b0a15dba84..90bacf000789 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2200,6 +2200,75 @@ int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev
}
EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid);

+int iommu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ if (unlikely(!domain->ops->attach_pasid_table))
+ return -ENODEV;
+
+ return domain->ops->attach_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_attach_pasid_table);
+
+int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
+ void __user *uinfo)
+{
+ struct iommu_pasid_table_config pasid_table_data = { 0 };
+ u32 minsz;
+
+ if (unlikely(!domain->ops->attach_pasid_table))
+ return -ENODEV;
+
+ /*
+ * No new spaces can be added before the variable sized union, the
+ * minimum size is the offset to the union.
+ */
+ minsz = offsetof(struct iommu_pasid_table_config, vendor_data);
+
+ /* Copy minsz from user to get flags and argsz */
+ if (copy_from_user(&pasid_table_data, uinfo, minsz))
+ return -EFAULT;
+
+ /* Fields before the variable size union are mandatory */
+ if (pasid_table_data.argsz < minsz)
+ return -EINVAL;
+
+ /* PASID and address granu require additional info beyond minsz */
+ if (pasid_table_data.version != PASID_TABLE_CFG_VERSION_1)
+ return -EINVAL;
+ if (pasid_table_data.format == IOMMU_PASID_FORMAT_SMMUV3 &&
+ pasid_table_data.argsz <
+ offsetofend(struct iommu_pasid_table_config, vendor_data.smmuv3))
+ return -EINVAL;
+
+ /*
+ * User might be using a newer UAPI header which has a larger data
+ * size, we shall support the existing flags within the current
+ * size. Copy the remaining user data _after_ minsz but not more
+ * than the current kernel supported size.
+ */
+ if (copy_from_user((void *)&pasid_table_data + minsz, uinfo + minsz,
+ min_t(u32, pasid_table_data.argsz, sizeof(pasid_table_data)) - minsz))
+ return -EFAULT;
+
+ /* Now the argsz is validated, check the content */
+ if (pasid_table_data.config < IOMMU_PASID_CONFIG_TRANSLATE ||
+ pasid_table_data.config > IOMMU_PASID_CONFIG_ABORT)
+ return -EINVAL;
+
+ return domain->ops->attach_pasid_table(domain, &pasid_table_data);
+}
+EXPORT_SYMBOL_GPL(iommu_uapi_attach_pasid_table);
+
+void iommu_detach_pasid_table(struct iommu_domain *domain)
+{
+ if (unlikely(!domain->ops->detach_pasid_table))
+ return;
+
+ domain->ops->detach_pasid_table(domain);
+}
+EXPORT_SYMBOL_GPL(iommu_detach_pasid_table);
+
static void __iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
{
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 86d688c4418f..c4422975359e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -239,6 +239,8 @@ struct iommu_iotlb_gather {
* @cache_invalidate: invalidate translation caches
* @sva_bind_gpasid: bind guest pasid and mm
* @sva_unbind_gpasid: unbind guest pasid and mm
+ * @attach_pasid_table: attach a pasid table
+ * @detach_pasid_table: detach the pasid table
* @def_domain_type: device default domain type, return value:
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
* - IOMMU_DOMAIN_DMA: must use a dma domain
@@ -304,6 +306,9 @@ struct iommu_ops {
void *drvdata);
void (*sva_unbind)(struct iommu_sva *handle);
u32 (*sva_get_pasid)(struct iommu_sva *handle);
+ int (*attach_pasid_table)(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg);
+ void (*detach_pasid_table)(struct iommu_domain *domain);

int (*page_response)(struct device *dev,
struct iommu_fault_event *evt,
@@ -454,6 +459,11 @@ extern int iommu_uapi_sva_unbind_gpasid(struct iommu_domain *domain,
struct device *dev, void __user *udata);
extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid);
+extern int iommu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg);
+extern int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
+ void __user *udata);
+extern void iommu_detach_pasid_table(struct iommu_domain *domain);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -1028,6 +1038,23 @@ iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
return -ENODEV;
}

+static inline
+int iommu_attach_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ return -ENODEV;
+}
+
+static inline
+int iommu_uapi_attach_pasid_table(struct iommu_domain *domain,
+ void __user *uinfo)
+{
+ return -ENODEV;
+}
+
+static inline
+void iommu_detach_pasid_table(struct iommu_domain *domain) {}
+
static inline struct iommu_sva *
iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
{
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index e1d9e75f2c94..40c28bb0e1bf 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -338,4 +338,58 @@ struct iommu_gpasid_bind_data {
} vendor;
};

+/**
+ * struct iommu_pasid_smmuv3 - ARM SMMUv3 Stream Table Entry stage 1 related
+ * information
+ * @version: API version of this structure
+ * @s1fmt: STE s1fmt (format of the CD table: single CD, linear table
+ * or 2-level table)
+ * @s1dss: STE s1dss (specifies the behavior when @pasid_bits != 0
+ * and no PASID is passed along with the incoming transaction)
+ * @padding: reserved for future use (should be zero)
+ *
+ * The PASID table is referred to as the Context Descriptor (CD) table on ARM
+ * SMMUv3. Please refer to the ARM SMMU 3.x spec (ARM IHI 0070A) for full
+ * details.
+ */
+struct iommu_pasid_smmuv3 {
+#define PASID_TABLE_SMMUV3_CFG_VERSION_1 1
+ __u32 version;
+ __u8 s1fmt;
+ __u8 s1dss;
+ __u8 padding[2];
+};
+
+/**
+ * struct iommu_pasid_table_config - PASID table data used to bind guest PASID
+ * table to the host IOMMU
+ * @argsz: User filled size of this data
+ * @version: API version to prepare for future extensions
+ * @base_ptr: guest physical address of the PASID table
+ * @format: format of the PASID table
+ * @pasid_bits: number of PASID bits used in the PASID table
+ * @config: indicates whether the guest translation stage must
+ * be translated, bypassed or aborted.
+ * @padding: reserved for future use (should be zero)
+ * @vendor_data.smmuv3: table information when @format is
+ * %IOMMU_PASID_FORMAT_SMMUV3
+ */
+struct iommu_pasid_table_config {
+ __u32 argsz;
+#define PASID_TABLE_CFG_VERSION_1 1
+ __u32 version;
+ __u64 base_ptr;
+#define IOMMU_PASID_FORMAT_SMMUV3 1
+ __u32 format;
+ __u8 pasid_bits;
+#define IOMMU_PASID_CONFIG_TRANSLATE 1
+#define IOMMU_PASID_CONFIG_BYPASS 2
+#define IOMMU_PASID_CONFIG_ABORT 3
+ __u8 config;
+ __u8 padding[2];
+ union {
+ struct iommu_pasid_smmuv3 smmuv3;
+ } vendor_data;
+};
+
#endif /* _UAPI_IOMMU_H */
--
2.26.3

2021-04-11 11:35:23

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 03/12] iommu/smmuv3: Allow s1 and s2 configs to coexist

In true nested mode, both s1_cfg and s2_cfg will coexist.
Let's remove the union and add a "set" field in each
config structure telling whether the config is set and needs
to be applied when writing the STE. In legacy nested mode,
only the second stage is used. In true nested mode, both stages
are used and the S1 config is "set" when the guest passes
its pasid table.

No functional change intended.

Signed-off-by: Eric Auger <[email protected]>

---

v13 -> v14:
- slight reword of the commit message

v12 -> v13:
- does not dynamically allocate s1-cfg and s2_cfg anymore. Add
the set field
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 43 +++++++++++++--------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 8 ++--
2 files changed, 31 insertions(+), 20 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 53abad8fdd91..22fb39a9ef31 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1239,8 +1239,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
u64 val = le64_to_cpu(dst[0]);
bool ste_live = false;
struct arm_smmu_device *smmu = NULL;
- struct arm_smmu_s1_cfg *s1_cfg = NULL;
- struct arm_smmu_s2_cfg *s2_cfg = NULL;
+ struct arm_smmu_s1_cfg *s1_cfg;
+ struct arm_smmu_s2_cfg *s2_cfg;
struct arm_smmu_domain *smmu_domain = NULL;
struct arm_smmu_cmdq_ent prefetch_cmd = {
.opcode = CMDQ_OP_PREFETCH_CFG,
@@ -1255,13 +1255,24 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
}

if (smmu_domain) {
+ s1_cfg = &smmu_domain->s1_cfg;
+ s2_cfg = &smmu_domain->s2_cfg;
+
switch (smmu_domain->stage) {
case ARM_SMMU_DOMAIN_S1:
- s1_cfg = &smmu_domain->s1_cfg;
+ s1_cfg->set = true;
+ s2_cfg->set = false;
break;
case ARM_SMMU_DOMAIN_S2:
+ s1_cfg->set = false;
+ s2_cfg->set = true;
+ break;
case ARM_SMMU_DOMAIN_NESTED:
- s2_cfg = &smmu_domain->s2_cfg;
+ /*
+ * Actual usage of stage 1 depends on nested mode:
+ * legacy (2d stage only) or true nested mode
+ */
+ s2_cfg->set = true;
break;
default:
break;
@@ -1288,7 +1299,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
val = STRTAB_STE_0_V;

/* Bypass/fault */
- if (!smmu_domain || !(s1_cfg || s2_cfg)) {
+ if (!smmu_domain || !(s1_cfg->set || s2_cfg->set)) {
if (!smmu_domain && disable_bypass)
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
else
@@ -1307,7 +1318,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
return;
}

- if (s1_cfg) {
+ if (s1_cfg->set) {
u64 strw = smmu->features & ARM_SMMU_FEAT_E2H ?
STRTAB_STE_1_STRW_EL2 : STRTAB_STE_1_STRW_NSEL1;

@@ -1329,7 +1340,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
}

- if (s2_cfg) {
+ if (s2_cfg->set) {
BUG_ON(ste_live);
dst[2] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_2_S2VMID, s2_cfg->vmid) |
@@ -2020,24 +2031,24 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
{
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_s1_cfg *s1_cfg = &smmu_domain->s1_cfg;
+ struct arm_smmu_s2_cfg *s2_cfg = &smmu_domain->s2_cfg;

iommu_put_dma_cookie(domain);
free_io_pgtable_ops(smmu_domain->pgtbl_ops);

/* Free the CD and ASID, if we allocated them */
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
-
+ if (s1_cfg->set) {
/* Prevent SVA from touching the CD while we're freeing it */
mutex_lock(&arm_smmu_asid_lock);
- if (cfg->cdcfg.cdtab)
+ if (s1_cfg->cdcfg.cdtab)
arm_smmu_free_cd_tables(smmu_domain);
- arm_smmu_free_asid(&cfg->cd);
+ arm_smmu_free_asid(&s1_cfg->cd);
mutex_unlock(&arm_smmu_asid_lock);
- } else {
- struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
- if (cfg->vmid)
- arm_smmu_bitmap_free(smmu->vmid_map, cfg->vmid);
+ }
+ if (s2_cfg->set) {
+ if (s2_cfg->vmid)
+ arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
}

kfree(smmu_domain);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 59af0bbd2f7b..ec2b77596b6a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -598,12 +598,14 @@ struct arm_smmu_s1_cfg {
struct arm_smmu_ctx_desc cd;
u8 s1fmt;
u8 s1cdmax;
+ bool set;
};

struct arm_smmu_s2_cfg {
u16 vmid;
u64 vttbr;
u64 vtcr;
+ bool set;
};

struct arm_smmu_strtab_cfg {
@@ -718,10 +720,8 @@ struct arm_smmu_domain {
atomic_t nr_ats_masters;

enum arm_smmu_domain_stage stage;
- union {
- struct arm_smmu_s1_cfg s1_cfg;
- struct arm_smmu_s2_cfg s2_cfg;
- };
+ struct arm_smmu_s1_cfg s1_cfg;
+ struct arm_smmu_s2_cfg s2_cfg;

struct iommu_domain domain;

--
2.26.3

2021-04-11 11:35:41

by Eric Auger

[permalink] [raw]
Subject: [PATCH v15 06/12] iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs

With nested stage support, soon we will need to invalidate
S1 contexts and ranges tagged with an unmanaged asid, this
latter being managed by the guest. So let's introduce 2 helpers
that allow to invalidate with externally managed ASIDs

Signed-off-by: Eric Auger <[email protected]>

---

v14 -> v15:
- Always send CMDQ_OP_TLBI_NH_VA and do not test
smmu_domain->smmu->features & ARM_SMMU_FEAT_E2H as the guest does
not run in hyp mode atm (Zenghui).

v13 -> v14
- Actually send the NH_ASID command (reported by Xingang Wang)
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 42 ++++++++++++++++-----
1 file changed, 33 insertions(+), 9 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 44cdc6df09c1..56a301fbe75a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1851,9 +1851,9 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
}

/* IO_PGTABLE API */
-static void arm_smmu_tlb_inv_context(void *cookie)
+static void __arm_smmu_tlb_inv_context(struct arm_smmu_domain *smmu_domain,
+ int ext_asid)
{
- struct arm_smmu_domain *smmu_domain = cookie;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cmdq_ent cmd;

@@ -1864,7 +1864,13 @@ static void arm_smmu_tlb_inv_context(void *cookie)
* insertion to guarantee those are observed before the TLBI. Do be
* careful, 007.
*/
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+ if (ext_asid >= 0) { /* guest stage 1 invalidation */
+ cmd.opcode = CMDQ_OP_TLBI_NH_ASID;
+ cmd.tlbi.asid = ext_asid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
+ arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+ arm_smmu_cmdq_issue_sync(smmu);
+ } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg.cd.asid);
} else {
cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
@@ -1875,6 +1881,13 @@ static void arm_smmu_tlb_inv_context(void *cookie)
arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
}

+static void arm_smmu_tlb_inv_context(void *cookie)
+{
+ struct arm_smmu_domain *smmu_domain = cookie;
+
+ __arm_smmu_tlb_inv_context(smmu_domain, -1);
+}
+
static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
unsigned long iova, size_t size,
size_t granule,
@@ -1934,9 +1947,10 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
arm_smmu_cmdq_batch_submit(smmu, &cmds);
}

-static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
- size_t granule, bool leaf,
- struct arm_smmu_domain *smmu_domain)
+static void
+arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
+ size_t granule, bool leaf, int ext_asid,
+ struct arm_smmu_domain *smmu_domain)
{
struct arm_smmu_cmdq_ent cmd = {
.tlbi = {
@@ -1944,7 +1958,16 @@ static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
},
};

- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+ if (ext_asid >= 0) { /* guest stage 1 invalidation */
+ /*
+ * At the moment the guest only uses NS-EL1, to be
+ * revisited when nested virt gets supported with E2H
+ * exposed.
+ */
+ cmd.opcode = CMDQ_OP_TLBI_NH_VA;
+ cmd.tlbi.asid = ext_asid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
+ } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
cmd.opcode = smmu_domain->smmu->features & ARM_SMMU_FEAT_E2H ?
CMDQ_OP_TLBI_EL2_VA : CMDQ_OP_TLBI_NH_VA;
cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
@@ -1952,6 +1975,7 @@ static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
cmd.opcode = CMDQ_OP_TLBI_S2_IPA;
cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
}
+
__arm_smmu_tlb_inv_range(&cmd, iova, size, granule, smmu_domain);

/*
@@ -1990,7 +2014,7 @@ static void arm_smmu_tlb_inv_page_nosync(struct iommu_iotlb_gather *gather,
static void arm_smmu_tlb_inv_walk(unsigned long iova, size_t size,
size_t granule, void *cookie)
{
- arm_smmu_tlb_inv_range_domain(iova, size, granule, false, cookie);
+ arm_smmu_tlb_inv_range_domain(iova, size, granule, false, -1, cookie);
}

static const struct iommu_flush_ops arm_smmu_flush_ops = {
@@ -2531,7 +2555,7 @@ static void arm_smmu_iotlb_sync(struct iommu_domain *domain,

arm_smmu_tlb_inv_range_domain(gather->start,
gather->end - gather->start + 1,
- gather->pgsize, true, smmu_domain);
+ gather->pgsize, true, -1, smmu_domain);
}

static phys_addr_t
--
2.26.3

2021-04-14 11:27:43

by Xingang Wang

[permalink] [raw]
Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)

Hi Eric, Jean-Philippe

On 2021/4/11 19:12, Eric Auger wrote:
> SMMUv3 Nested Stage Setup (IOMMU part)
>
> This series brings the IOMMU part of HW nested paging support
> in the SMMUv3. The VFIO part is submitted separately.
>
> This is based on Jean-Philippe's
> [PATCH v14 00/10] iommu: I/O page faults for SMMUv3
> https://www.spinics.net/lists/arm-kernel/msg886518.html
> (including the patches that were not pulled for 5.13)
>
> The IOMMU API is extended to support 2 new API functionalities:
> 1) pass the guest stage 1 configuration
> 2) pass stage 1 MSI bindings
>
> Then those capabilities gets implemented in the SMMUv3 driver.
>
> The virtualizer passes information through the VFIO user API
> which cascades them to the iommu subsystem. This allows the guest
> to own stage 1 tables and context descriptors (so-called PASID
> table) while the host owns stage 2 tables and main configuration
> structures (STE).
>
> Best Regards
>
> Eric
>
> This series can be found at:
> v5.12-rc6-jean-iopf-14-2stage-v15
> (including the VFIO part in its last version: v13)
>

I am testing the performance of an accelerator with/without SVA/vSVA,
and found there might be some potential performance loss risk for SVA/vSVA.

I use a Network and computing encryption device (SEC), and send 1MB
request for 10000 times.

I trigger mm fault before I send the request, so there should be no iopf.

Here's what I got:

physical scenario:
performance: SVA:9MB/s NOSVA:9MB/s
tlb_miss: SVA:302,651 NOSVA:1,223
trans_table_walk_access:SVA:302,276 NOSVA:1,237

VM scenario:
performance: vSVA:9MB/s NOvSVA:6MB/s about 30~40% loss
tlb_miss: vSVA:4,423,897 NOvSVA:1,907
trans_table_walk_access:vSVA:61,928,430 NOvSVA:21,948

In physical scenario, there's almost no performance loss, but the
tlb_miss and trans_table_walk_access of stage 1 for SVA is quite high,
comparing to NOSVA.

In VM scenario, there's about 30~40% performance loss, this is because
the two stage tlb_miss and trans_table_walk_access is even higher, and
impact the performance.

I compare the procedure of building page table of SVA and NOSVA, and
found that NOSVA uses 2MB mapping as far as possible, while SVA uses
only 4KB.

I retest with huge page, and huge page could solve this problem, the
performance of SVA/vSVA is almost the same as NOSVA.

I am wondering do you have any other solution for the performance loss
of vSVA, or any other method to reduce the tlb_miss/trans_table_walk.

Thanks

Xingang

.

Subject: RE: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)



> -----Original Message-----
> From: wangxingang
> Sent: 14 April 2021 03:36
> To: Eric Auger <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; zhukeqian <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Shameerali Kolothum
> Thodi <[email protected]>; yuzenghui
> <[email protected]>; [email protected]; lushenming
> <[email protected]>; [email protected]; chenxiang (M)
> <[email protected]>; [email protected]; jiangkunkun
> <[email protected]>
> Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)
>
> Hi Eric, Jean-Philippe
>
> On 2021/4/11 19:12, Eric Auger wrote:
> > SMMUv3 Nested Stage Setup (IOMMU part)
> >
> > This series brings the IOMMU part of HW nested paging support
> > in the SMMUv3. The VFIO part is submitted separately.
> >
> > This is based on Jean-Philippe's
> > [PATCH v14 00/10] iommu: I/O page faults for SMMUv3
> > https://www.spinics.net/lists/arm-kernel/msg886518.html
> > (including the patches that were not pulled for 5.13)
> >
> > The IOMMU API is extended to support 2 new API functionalities:
> > 1) pass the guest stage 1 configuration
> > 2) pass stage 1 MSI bindings
> >
> > Then those capabilities gets implemented in the SMMUv3 driver.
> >
> > The virtualizer passes information through the VFIO user API
> > which cascades them to the iommu subsystem. This allows the guest
> > to own stage 1 tables and context descriptors (so-called PASID
> > table) while the host owns stage 2 tables and main configuration
> > structures (STE).
> >
> > Best Regards
> >
> > Eric
> >
> > This series can be found at:
> > v5.12-rc6-jean-iopf-14-2stage-v15
> > (including the VFIO part in its last version: v13)
> >
>
> I am testing the performance of an accelerator with/without SVA/vSVA,
> and found there might be some potential performance loss risk for SVA/vSVA.
>
> I use a Network and computing encryption device (SEC), and send 1MB
> request for 10000 times.
>
> I trigger mm fault before I send the request, so there should be no iopf.
>
> Here's what I got:
>
> physical scenario:
> performance: SVA:9MB/s NOSVA:9MB/s
> tlb_miss: SVA:302,651 NOSVA:1,223
> trans_table_walk_access:SVA:302,276 NOSVA:1,237
>
> VM scenario:
> performance: vSVA:9MB/s NOvSVA:6MB/s about 30~40% loss
> tlb_miss: vSVA:4,423,897 NOvSVA:1,907
> trans_table_walk_access:vSVA:61,928,430 NOvSVA:21,948
>
> In physical scenario, there's almost no performance loss, but the
> tlb_miss and trans_table_walk_access of stage 1 for SVA is quite high,
> comparing to NOSVA.
>
> In VM scenario, there's about 30~40% performance loss, this is because
> the two stage tlb_miss and trans_table_walk_access is even higher, and
> impact the performance.
>
> I compare the procedure of building page table of SVA and NOSVA, and
> found that NOSVA uses 2MB mapping as far as possible, while SVA uses
> only 4KB.
>
> I retest with huge page, and huge page could solve this problem, the
> performance of SVA/vSVA is almost the same as NOSVA.
>
> I am wondering do you have any other solution for the performance loss
> of vSVA, or any other method to reduce the tlb_miss/trans_table_walk.

Hi Xingang,

Just curious, do you have DVM enabled on this board or does it use explicit
SMMU TLB invalidations?

Thanks,
Shameer

2021-04-14 19:55:37

by Xingang Wang

[permalink] [raw]
Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)

Hi Shameer,

On 2021/4/14 14:56, Shameerali Kolothum Thodi wrote:
>
>
>> -----Original Message-----
>> From: wangxingang
>> Sent: 14 April 2021 03:36
>> To: Eric Auger <[email protected]>; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; zhukeqian <[email protected]>
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; Shameerali Kolothum
>> Thodi <[email protected]>; yuzenghui
>> <[email protected]>; [email protected]; lushenming
>> <[email protected]>; [email protected]; chenxiang (M)
>> <[email protected]>; [email protected]; jiangkunkun
>> <[email protected]>
>> Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)
>>
>> Hi Eric, Jean-Philippe
>>
>> On 2021/4/11 19:12, Eric Auger wrote:
>>> SMMUv3 Nested Stage Setup (IOMMU part)
>>>
>>> This series brings the IOMMU part of HW nested paging support
>>> in the SMMUv3. The VFIO part is submitted separately.
>>>
>>> This is based on Jean-Philippe's
>>> [PATCH v14 00/10] iommu: I/O page faults for SMMUv3
>>> https://www.spinics.net/lists/arm-kernel/msg886518.html
>>> (including the patches that were not pulled for 5.13)
>>>
>>> The IOMMU API is extended to support 2 new API functionalities:
>>> 1) pass the guest stage 1 configuration
>>> 2) pass stage 1 MSI bindings
>>>
>>> Then those capabilities gets implemented in the SMMUv3 driver.
>>>
>>> The virtualizer passes information through the VFIO user API
>>> which cascades them to the iommu subsystem. This allows the guest
>>> to own stage 1 tables and context descriptors (so-called PASID
>>> table) while the host owns stage 2 tables and main configuration
>>> structures (STE).
>>>
>>> Best Regards
>>>
>>> Eric
>>>
>>> This series can be found at:
>>> v5.12-rc6-jean-iopf-14-2stage-v15
>>> (including the VFIO part in its last version: v13)
>>>
>>
>> I am testing the performance of an accelerator with/without SVA/vSVA,
>> and found there might be some potential performance loss risk for SVA/vSVA.
>>
>> I use a Network and computing encryption device (SEC), and send 1MB
>> request for 10000 times.
>>
>> I trigger mm fault before I send the request, so there should be no iopf.
>>
>> Here's what I got:
>>
>> physical scenario:
>> performance: SVA:9MB/s NOSVA:9MB/s
>> tlb_miss: SVA:302,651 NOSVA:1,223
>> trans_table_walk_access:SVA:302,276 NOSVA:1,237
>>
>> VM scenario:
>> performance: vSVA:9MB/s NOvSVA:6MB/s about 30~40% loss
>> tlb_miss: vSVA:4,423,897 NOvSVA:1,907
>> trans_table_walk_access:vSVA:61,928,430 NOvSVA:21,948
>>
>> In physical scenario, there's almost no performance loss, but the
>> tlb_miss and trans_table_walk_access of stage 1 for SVA is quite high,
>> comparing to NOSVA.
>>
>> In VM scenario, there's about 30~40% performance loss, this is because
>> the two stage tlb_miss and trans_table_walk_access is even higher, and
>> impact the performance.
>>
>> I compare the procedure of building page table of SVA and NOSVA, and
>> found that NOSVA uses 2MB mapping as far as possible, while SVA uses
>> only 4KB.
>>
>> I retest with huge page, and huge page could solve this problem, the
>> performance of SVA/vSVA is almost the same as NOSVA.
>>
>> I am wondering do you have any other solution for the performance loss
>> of vSVA, or any other method to reduce the tlb_miss/trans_table_walk.
>
> Hi Xingang,
>
> Just curious, do you have DVM enabled on this board or does it use explicit
> SMMU TLB invalidations?
>
> Thanks,
> Shameer
>

For now, DVM is enabled and TLBI is not explicit used.

And by the way the performance data above is
performance: vSVA:9GB/s(not 9MB/s) NOvSVA:6GB/s(not 6GB/s)

Thanks

Xingang

.

2021-04-21 12:55:15

by Vivek Kumar Gautam

[permalink] [raw]
Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)

Hi Eric,

On 4/11/21 4:42 PM, Eric Auger wrote:
> SMMUv3 Nested Stage Setup (IOMMU part)
>

[snip]

>
> Eric Auger (12):
> iommu: Introduce attach/detach_pasid_table API
> iommu: Introduce bind/unbind_guest_msi
> iommu/smmuv3: Allow s1 and s2 configs to coexist
> iommu/smmuv3: Get prepared for nested stage support
> iommu/smmuv3: Implement attach/detach_pasid_table
> iommu/smmuv3: Allow stage 1 invalidation with unmanaged ASIDs
> iommu/smmuv3: Implement cache_invalidate
> dma-iommu: Implement NESTED_MSI cookie
> iommu/smmuv3: Nested mode single MSI doorbell per domain enforcement
> iommu/smmuv3: Enforce incompatibility between nested mode and HW MSI
> regions
> iommu/smmuv3: Implement bind/unbind_guest_msi
> iommu/smmuv3: report additional recoverable faults

[snip]

I noticed that the patch[1]:
[PATCH v13 15/15] iommu/smmuv3: Add PASID cache invalidation per PASID
has been dropped in the v14 and v15 of
this series.

Is this planned to be part of any future series, or did I miss a
discussion about dropping the patch? :-)


[1]
https://patchwork.kernel.org/project/kvm/patch/[email protected]/


Best regards
Vivek
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

2021-04-23 18:24:35

by Sumit Gupta

[permalink] [raw]
Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)

Hi Eric,

I have validated v15 of the patch series from your branch "v5.12-rc6-jean-iopf-14-2stage-v15"
on top of Jean's current sva patches with Kernel-5.12.0-rc8.
Verfied nested translations with NVMe PCI device assigned to Guest VM.

Tested-by: Sumit Gupta <[email protected]>

2021-05-14 12:58:43

by Kunkun Jiang

[permalink] [raw]
Subject: Re: [PATCH v15 07/12] iommu/smmuv3: Implement cache_invalidate



On 2021/4/11 19:12, Eric Auger wrote:
> Implement domain-selective, pasid selective and page-selective
> IOTLB invalidations.
>
> Signed-off-by: Eric Auger <[email protected]>
>
> ---
> v4 -> v15:
> - remove the redundant arm_smmu_cmdq_issue_sync(smmu)
> in IOMMU_INV_GRANU_ADDR case (Zenghui)
> - if RIL is not supported by the host, make sure the granule_size
> that is passed by the userspace is supported or fix it
> (Chenxiang)
>
> 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 | 89 +++++++++++++++++++++
> 1 file changed, 89 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 56a301fbe75a..bfc112cc0d38 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2961,6 +2961,94 @@ static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
> mutex_unlock(&smmu_domain->init_mutex);
> }
>
> +static int
> +arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
> + struct iommu_cache_invalidate_info *inv_info)
> +{
> + struct arm_smmu_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 =
> + &inv_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 = &inv_info->granu.addr_info;
> + size_t granule_size = info->granule_size;
> + size_t size = info->nb_granules * info->granule_size;
> + bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
> + int tg;
> +
> + if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
> + return -ENOENT;
> +
> + if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
> + break;
> +
> + tg = __ffs(granule_size);
> + if (granule_size & ~(1 << tg))
> + return -EINVAL;
This check looks like to confirm the granule_size is a power of 2.
Does the granule_size have to be a power of 2?
I think it should also be handled correctly, even if the granule_size is
not a power of 2.
> + /*
> + * When RIL is not supported, make sure the granule size that is
> + * passed is supported. In RIL mode, this is enforced in
> + * __arm_smmu_tlb_inv_range()
> + */
> + if (!(smmu->features & ARM_SMMU_FEAT_RANGE_INV) &&
> + !(granule_size & smmu_domain->domain.pgsize_bitmap)) {
> + tg = __ffs(smmu_domain->domain.pgsize_bitmap);
> + granule_size = 1 << tg;
> + size = size >> tg;
Why does size need to be shifted tg bits to the right?

Thanks,
Kunkun Jiang
> + }
> +
> + arm_smmu_tlb_inv_range_domain(info->addr, size,
> + granule_size, leaf,
> + info->archid, smmu_domain);
> + return 0;
> + }
> + case IOMMU_INV_GRANU_DOMAIN:
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* Global S1 invalidation */
> + cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
> + arm_smmu_cmdq_issue_cmd(smmu, &cmd);
> + arm_smmu_cmdq_issue_sync(smmu);
> + return 0;
> +}
> +
> static bool arm_smmu_dev_has_feature(struct device *dev,
> enum iommu_dev_features feat)
> {
> @@ -3060,6 +3148,7 @@ static struct iommu_ops arm_smmu_ops = {
> .put_resv_regions = generic_iommu_put_resv_regions,
> .attach_pasid_table = arm_smmu_attach_pasid_table,
> .detach_pasid_table = arm_smmu_detach_pasid_table,
> + .cache_invalidate = arm_smmu_cache_invalidate,
> .dev_has_feat = arm_smmu_dev_has_feature,
> .dev_feat_enabled = arm_smmu_dev_feature_enabled,
> .dev_enable_feat = arm_smmu_dev_enable_feature,



2021-05-21 09:58:46

by Kunkun Jiang

[permalink] [raw]
Subject: Re: [PATCH v15 07/12] iommu/smmuv3: Implement cache_invalidate

Hi Eric,

On 2021/4/11 19:12, Eric Auger wrote:
> Implement domain-selective, pasid selective and page-selective
> IOTLB invalidations.
>
> Signed-off-by: Eric Auger <[email protected]>
>
> ---
> v4 -> v15:
> - remove the redundant arm_smmu_cmdq_issue_sync(smmu)
> in IOMMU_INV_GRANU_ADDR case (Zenghui)
> - if RIL is not supported by the host, make sure the granule_size
> that is passed by the userspace is supported or fix it
> (Chenxiang)
>
> 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 | 89 +++++++++++++++++++++
> 1 file changed, 89 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 56a301fbe75a..bfc112cc0d38 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2961,6 +2961,94 @@ static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
> mutex_unlock(&smmu_domain->init_mutex);
> }
>
> +static int
> +arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
> + struct iommu_cache_invalidate_info *inv_info)
> +{
> + struct arm_smmu_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 =
> + &inv_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 = &inv_info->granu.addr_info;
> + size_t granule_size = info->granule_size;
> + size_t size = info->nb_granules * info->granule_size;
> + bool leaf = info->flags & IOMMU_INV_ADDR_FLAGS_LEAF;
> + int tg;
> +
> + if (info->flags & IOMMU_INV_ADDR_FLAGS_PASID)
> + return -ENOENT;
> +
> + if (!(info->flags & IOMMU_INV_ADDR_FLAGS_ARCHID))
> + break;
> +
> + tg = __ffs(granule_size);
> + if (granule_size & ~(1 << tg))
> + return -EINVAL;
> + /*
> + * When RIL is not supported, make sure the granule size that is
> + * passed is supported. In RIL mode, this is enforced in
> + * __arm_smmu_tlb_inv_range()
> + */
> + if (!(smmu->features & ARM_SMMU_FEAT_RANGE_INV) &&
> + !(granule_size & smmu_domain->domain.pgsize_bitmap)) {
> + tg = __ffs(smmu_domain->domain.pgsize_bitmap);
> + granule_size = 1 << tg;
> + size = size >> tg;
> + }
> +
> + arm_smmu_tlb_inv_range_domain(info->addr, size,
> + granule_size, leaf,
> + info->archid, smmu_domain);
I encountered some errors when I tested the SMMU nested mode.

Test scenario description:
guest kernel: 4KB translation granule
host kernel: 16KB translation granule

errors:
1. encountered an endless loop in __arm_smmu_tlb_inv_range because
num_pages is 0
2. encountered CERROR_ILL because the fields of TLB invalidation
command are as follow: TG = 2, NUM = 0, SCALE = 0, TTL = 0. The
combination is exactly the kind of reserved combination pointed
out in the SMMUv3 spec(page 143-144, version D.a)

According to my analysis, we should do a bit more validation on the
'size' and 'granule_size' when SMMU supports RIL:
1. Align 'size' with the smallest granule size supported by SMMU upwards.
2. If the granule size isn't supported by SMMU, we set it to the smallest
granule size supported by SMMU

I sent two patches to fix them in theĀ  __arm_smmu_tlb_inv_range(). [1]
(These patches may better explain what I want to express.)
According to the reply, it seems that it is more appropriate to modify here.

Thanks,
Kunkun Jiang

[1] [RFC PATCH v1 0/2] iommu/arm-smmu-v3: Add some parameter check in
__arm_smmu_tlb_inv_range()
https://lore.kernel.org/linux-iommu/[email protected]/
> + return 0;
> + }
> + case IOMMU_INV_GRANU_DOMAIN:
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* Global S1 invalidation */
> + cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
> + arm_smmu_cmdq_issue_cmd(smmu, &cmd);
> + arm_smmu_cmdq_issue_sync(smmu);
> + return 0;
> +}
> +
> static bool arm_smmu_dev_has_feature(struct device *dev,
> enum iommu_dev_features feat)
> {
> @@ -3060,6 +3148,7 @@ static struct iommu_ops arm_smmu_ops = {
> .put_resv_regions = generic_iommu_put_resv_regions,
> .attach_pasid_table = arm_smmu_attach_pasid_table,
> .detach_pasid_table = arm_smmu_detach_pasid_table,
> + .cache_invalidate = arm_smmu_cache_invalidate,
> .dev_has_feat = arm_smmu_dev_has_feature,
> .dev_feat_enabled = arm_smmu_dev_feature_enabled,
> .dev_enable_feat = arm_smmu_dev_enable_feature,


2021-09-27 21:19:51

by Krishna Reddy

[permalink] [raw]
Subject: RE: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)

Hi Eric,
> This is based on Jean-Philippe's
> [PATCH v14 00/10] iommu: I/O page faults for SMMUv3
> https://www.spinics.net/lists/arm-kernel/msg886518.html
> (including the patches that were not pulled for 5.13)
>

Jean's patches have been merged to v5.14.
Do you anticipate IOMMU/VFIO part patches getting into upstream kernel soon?

-KR

2021-09-28 06:26:38

by Eric Auger

[permalink] [raw]
Subject: Re: [PATCH v15 00/12] SMMUv3 Nested Stage Setup (IOMMU part)

Hi Krishna,

On 9/27/21 11:17 PM, Krishna Reddy wrote:
> Hi Eric,
>> This is based on Jean-Philippe's
>> [PATCH v14 00/10] iommu: I/O page faults for SMMUv3
>> https://www.spinics.net/lists/arm-kernel/msg886518.html
>> (including the patches that were not pulled for 5.13)
>>
> Jean's patches have been merged to v5.14.
> Do you anticipate IOMMU/VFIO part patches getting into upstream kernel soon?

I am going to respin the smmu part rebased on v5.15. As for the VFIO
part, this needs to be totally redesigned based on /dev/iommu (see
[RFC 00/20] Introduce /dev/iommu for userspace I/O address space
management).

I will provide some updated kernel and qemu branches for testing purpose
only.

Thanks

Eric
>
> -KR
>