This patch-series aims at enabling Nested stage translation in guests
using virtio-iommu as the paravirtualized iommu. The backend is supported
with Arm SMMU-v3 that provides nested stage-1 and stage-2 translation.
This series derives its purpose from various efforts happening to add
support for Shared Virtual Addressing (SVA) in host and guest. On Arm,
most of the support for SVA has already landed. The support for nested
stage translation and fault reporting to guest has been proposed [1].
The related changes required in VFIO [2] framework have also been put
forward.
This series proposes changes in virtio-iommu to program PASID tables
and related stage-1 page tables. A simple iommu-pasid-table library
is added for this purpose that interacts with vendor drivers to
allocate and populate PASID tables.
In Arm SMMUv3 we propose to pull the Context Descriptor (CD) management
code out of the arm-smmu-v3 driver and add that as a glue vendor layer
to support allocating CD tables, and populating them with right values.
These CD tables are essentially the PASID tables and contain stage-1
page table configurations too.
A request to setup these CD tables come from virtio-iommu driver using
the iommu-pasid-table library when running on Arm. The virtio-iommu
then pass these PASID tables to the host using the right virtio backend
and support in VMM.
For testing we have added necessary support in kvmtool. The changes in
kvmtool are based on virtio-iommu development branch by Jean-Philippe
Brucker [3].
The tested kernel branch contains following in the order bottom to top
on the git hash -
a) v5.11-rc3
b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
table support for Arm.
c) Smmu test engine patches from Jean-Philippe's branch [4]
d) This series
e) Domain nesting info patches [5][6][7].
f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
the list).
This kernel is tested on Neoverse reference software stack with
Fixed virtual platform. Public version of the software stack and
FVP is available here[8][9].
A big thanks to Jean-Philippe for his contributions towards this work
and for his valuable guidance.
[1] https://lore.kernel.org/linux-iommu/[email protected]/T/
[2] https://lore.kernel.org/kvmarm/[email protected]/T/
[3] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
[4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
[5] https://lore.kernel.org/kvm/[email protected]/
[6] https://lore.kernel.org/kvm/[email protected]/
[7] https://lore.kernel.org/kvm/[email protected]/
[8] https://developer.arm.com/tools-and-software/open-source-software/arm-platforms-software/arm-ecosystem-fvps
[9] https://git.linaro.org/landing-teams/working/arm/arm-reference-platforms.git/about/docs/rdn1edge/user-guide.rst
Jean-Philippe Brucker (6):
iommu/virtio: Add headers for table format probing
iommu/virtio: Add table format probing
iommu/virtio: Add headers for binding pasid table in iommu
iommu/virtio: Add support for INVALIDATE request
iommu/virtio: Attach Arm PASID tables when available
iommu/virtio: Add support for Arm LPAE page table format
Vivek Gautam (9):
iommu/arm-smmu-v3: Create a Context Descriptor library
iommu: Add a simple PASID table library
iommu/arm-smmu-v3: Update drivers to work with iommu-pasid-table
iommu/arm-smmu-v3: Update CD base address info for user-space
iommu/arm-smmu-v3: Set sync op from consumer driver of cd-lib
iommu: Add asid_bits to arm smmu-v3 stage1 table info
iommu/virtio: Update table format probing header
iommu/virtio: Prepare to add attach pasid table infrastructure
iommu/virtio: Update fault type and reason info for viommu fault
drivers/iommu/arm/arm-smmu-v3/Makefile | 2 +-
.../arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 283 +++++++
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 16 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 268 +------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 +-
drivers/iommu/iommu-pasid-table.h | 140 ++++
drivers/iommu/virtio-iommu.c | 692 +++++++++++++++++-
include/uapi/linux/iommu.h | 2 +-
include/uapi/linux/virtio_iommu.h | 158 +++-
9 files changed, 1303 insertions(+), 262 deletions(-)
create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
create mode 100644 drivers/iommu/iommu-pasid-table.h
--
2.17.1
Para-virtualized iommu drivers in guest may require to create and manage
context descriptor (CD) tables as part of PASID table allocations.
The PASID tables are passed to host to configure stage-1 tables in
hardware.
Make way for a library driver for CD management to allow para-
virtualized iommu driver call such code.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
drivers/iommu/arm/arm-smmu-v3/Makefile | 2 +-
.../arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 223 ++++++++++++++++++
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 216 +----------------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 +
4 files changed, 228 insertions(+), 216 deletions(-)
create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
diff --git a/drivers/iommu/arm/arm-smmu-v3/Makefile b/drivers/iommu/arm/arm-smmu-v3/Makefile
index 54feb1ecccad..ca1a05b8b8ad 100644
--- a/drivers/iommu/arm/arm-smmu-v3/Makefile
+++ b/drivers/iommu/arm/arm-smmu-v3/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_ARM_SMMU_V3) += arm_smmu_v3.o
-arm_smmu_v3-objs-y += arm-smmu-v3.o
+arm_smmu_v3-objs-y += arm-smmu-v3.o arm-smmu-v3-cd-lib.o
arm_smmu_v3-objs-$(CONFIG_ARM_SMMU_V3_SVA) += arm-smmu-v3-sva.o
arm_smmu_v3-objs := $(arm_smmu_v3-objs-y)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
new file mode 100644
index 000000000000..97d1786a8a70
--- /dev/null
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * arm-smmu-v3 context descriptor handling library driver
+ *
+ * Copyright (C) 2021 Arm Ltd.
+ */
+
+#include <linux/dma-iommu.h>
+
+#include "arm-smmu-v3.h"
+
+static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
+ struct arm_smmu_l1_ctx_desc *l1_desc)
+{
+ size_t size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
+
+ l1_desc->l2ptr = dmam_alloc_coherent(smmu->dev, size,
+ &l1_desc->l2ptr_dma, GFP_KERNEL);
+ if (!l1_desc->l2ptr) {
+ dev_warn(smmu->dev,
+ "failed to allocate context descriptor table\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+static void arm_smmu_write_cd_l1_desc(__le64 *dst,
+ struct arm_smmu_l1_ctx_desc *l1_desc)
+{
+ u64 val = (l1_desc->l2ptr_dma & CTXDESC_L1_DESC_L2PTR_MASK) |
+ CTXDESC_L1_DESC_V;
+
+ /* See comment in arm_smmu_write_ctx_desc() */
+ WRITE_ONCE(*dst, cpu_to_le64(val));
+}
+
+static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
+ u32 ssid)
+{
+ __le64 *l1ptr;
+ unsigned int idx;
+ struct arm_smmu_l1_ctx_desc *l1_desc;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+
+ if (smmu_domain->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
+ return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
+
+ idx = ssid >> CTXDESC_SPLIT;
+ l1_desc = &cdcfg->l1_desc[idx];
+ if (!l1_desc->l2ptr) {
+ if (arm_smmu_alloc_cd_leaf_table(smmu, l1_desc))
+ return NULL;
+
+ l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
+ arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
+ /* An invalid L1CD can be cached */
+ arm_smmu_sync_cd(smmu_domain, ssid, false);
+ }
+ idx = ssid & (CTXDESC_L2_ENTRIES - 1);
+ return l1_desc->l2ptr + idx * CTXDESC_CD_DWORDS;
+}
+
+int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
+ struct arm_smmu_ctx_desc *cd)
+{
+ /*
+ * This function handles the following cases:
+ *
+ * (1) Install primary CD, for normal DMA traffic (SSID = 0).
+ * (2) Install a secondary CD, for SID+SSID traffic.
+ * (3) Update ASID of a CD. Atomically write the first 64 bits of the
+ * CD, then invalidate the old entry and mappings.
+ * (4) Quiesce the context without clearing the valid bit. Disable
+ * translation, and ignore any translation fault.
+ * (5) Remove a secondary CD.
+ */
+ u64 val;
+ bool cd_live;
+ __le64 *cdptr;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+ if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax)))
+ return -E2BIG;
+
+ cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
+ if (!cdptr)
+ return -ENOMEM;
+
+ val = le64_to_cpu(cdptr[0]);
+ cd_live = !!(val & CTXDESC_CD_0_V);
+
+ if (!cd) { /* (5) */
+ val = 0;
+ } else if (cd == &quiet_cd) { /* (4) */
+ val |= CTXDESC_CD_0_TCR_EPD0;
+ } else if (cd_live) { /* (3) */
+ val &= ~CTXDESC_CD_0_ASID;
+ val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
+ /*
+ * Until CD+TLB invalidation, both ASIDs may be used for tagging
+ * this substream's traffic
+ */
+ } else { /* (1) and (2) */
+ cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
+ cdptr[2] = 0;
+ cdptr[3] = cpu_to_le64(cd->mair);
+
+ /*
+ * STE is live, and the SMMU might read dwords of this CD in any
+ * order. Ensure that it observes valid values before reading
+ * V=1.
+ */
+ arm_smmu_sync_cd(smmu_domain, ssid, true);
+
+ val = cd->tcr |
+#ifdef __BIG_ENDIAN
+ CTXDESC_CD_0_ENDI |
+#endif
+ CTXDESC_CD_0_R | CTXDESC_CD_0_A |
+ (cd->mm ? 0 : CTXDESC_CD_0_ASET) |
+ CTXDESC_CD_0_AA64 |
+ FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
+ CTXDESC_CD_0_V;
+
+ /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
+ if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
+ val |= CTXDESC_CD_0_S;
+ }
+
+ /*
+ * The SMMU accesses 64-bit values atomically. See IHI0070Ca 3.21.3
+ * "Configuration structures and configuration invalidation completion"
+ *
+ * The size of single-copy atomic reads made by the SMMU is
+ * IMPLEMENTATION DEFINED but must be at least 64 bits. Any single
+ * field within an aligned 64-bit span of a structure can be altered
+ * without first making the structure invalid.
+ */
+ WRITE_ONCE(cdptr[0], cpu_to_le64(val));
+ arm_smmu_sync_cd(smmu_domain, ssid, true);
+ return 0;
+}
+
+int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
+{
+ int ret;
+ size_t l1size;
+ size_t max_contexts;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg;
+
+ max_contexts = 1 << cfg->s1cdmax;
+
+ if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
+ max_contexts <= CTXDESC_L2_ENTRIES) {
+ cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
+ cdcfg->num_l1_ents = max_contexts;
+
+ l1size = max_contexts * (CTXDESC_CD_DWORDS << 3);
+ } else {
+ cfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
+ cdcfg->num_l1_ents = DIV_ROUND_UP(max_contexts,
+ CTXDESC_L2_ENTRIES);
+
+ cdcfg->l1_desc = devm_kcalloc(smmu->dev, cdcfg->num_l1_ents,
+ sizeof(*cdcfg->l1_desc),
+ GFP_KERNEL);
+ if (!cdcfg->l1_desc)
+ return -ENOMEM;
+
+ l1size = cdcfg->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
+ }
+
+ cdcfg->cdtab = dmam_alloc_coherent(smmu->dev, l1size, &cdcfg->cdtab_dma,
+ GFP_KERNEL);
+ if (!cdcfg->cdtab) {
+ dev_warn(smmu->dev, "failed to allocate context descriptor\n");
+ ret = -ENOMEM;
+ goto err_free_l1;
+ }
+
+ return 0;
+
+err_free_l1:
+ if (cdcfg->l1_desc) {
+ devm_kfree(smmu->dev, cdcfg->l1_desc);
+ cdcfg->l1_desc = NULL;
+ }
+ return ret;
+}
+
+void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
+{
+ int i;
+ size_t size, l1size;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+
+ if (cdcfg->l1_desc) {
+ size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
+
+ for (i = 0; i < cdcfg->num_l1_ents; i++) {
+ if (!cdcfg->l1_desc[i].l2ptr)
+ continue;
+
+ dmam_free_coherent(smmu->dev, size,
+ cdcfg->l1_desc[i].l2ptr,
+ cdcfg->l1_desc[i].l2ptr_dma);
+ }
+ devm_kfree(smmu->dev, cdcfg->l1_desc);
+ cdcfg->l1_desc = NULL;
+
+ l1size = cdcfg->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
+ } else {
+ l1size = cdcfg->num_l1_ents * (CTXDESC_CD_DWORDS << 3);
+ }
+
+ dmam_free_coherent(smmu->dev, l1size, cdcfg->cdtab, cdcfg->cdtab_dma);
+ cdcfg->cdtab_dma = 0;
+ cdcfg->cdtab = NULL;
+}
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 51da068df4e9..13513f2f651a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -894,8 +894,7 @@ void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
arm_smmu_cmdq_issue_sync(smmu);
}
-static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
- int ssid, bool leaf)
+void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain, int ssid, bool leaf)
{
size_t i;
unsigned long flags;
@@ -922,219 +921,6 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
arm_smmu_cmdq_batch_submit(smmu, &cmds);
}
-static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
- struct arm_smmu_l1_ctx_desc *l1_desc)
-{
- size_t size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
-
- l1_desc->l2ptr = dmam_alloc_coherent(smmu->dev, size,
- &l1_desc->l2ptr_dma, GFP_KERNEL);
- if (!l1_desc->l2ptr) {
- dev_warn(smmu->dev,
- "failed to allocate context descriptor table\n");
- return -ENOMEM;
- }
- return 0;
-}
-
-static void arm_smmu_write_cd_l1_desc(__le64 *dst,
- struct arm_smmu_l1_ctx_desc *l1_desc)
-{
- u64 val = (l1_desc->l2ptr_dma & CTXDESC_L1_DESC_L2PTR_MASK) |
- CTXDESC_L1_DESC_V;
-
- /* See comment in arm_smmu_write_ctx_desc() */
- WRITE_ONCE(*dst, cpu_to_le64(val));
-}
-
-static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
- u32 ssid)
-{
- __le64 *l1ptr;
- unsigned int idx;
- struct arm_smmu_l1_ctx_desc *l1_desc;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
-
- if (smmu_domain->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
- return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
-
- idx = ssid >> CTXDESC_SPLIT;
- l1_desc = &cdcfg->l1_desc[idx];
- if (!l1_desc->l2ptr) {
- if (arm_smmu_alloc_cd_leaf_table(smmu, l1_desc))
- return NULL;
-
- l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
- arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
- /* An invalid L1CD can be cached */
- arm_smmu_sync_cd(smmu_domain, ssid, false);
- }
- idx = ssid & (CTXDESC_L2_ENTRIES - 1);
- return l1_desc->l2ptr + idx * CTXDESC_CD_DWORDS;
-}
-
-int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
- struct arm_smmu_ctx_desc *cd)
-{
- /*
- * This function handles the following cases:
- *
- * (1) Install primary CD, for normal DMA traffic (SSID = 0).
- * (2) Install a secondary CD, for SID+SSID traffic.
- * (3) Update ASID of a CD. Atomically write the first 64 bits of the
- * CD, then invalidate the old entry and mappings.
- * (4) Quiesce the context without clearing the valid bit. Disable
- * translation, and ignore any translation fault.
- * (5) Remove a secondary CD.
- */
- u64 val;
- bool cd_live;
- __le64 *cdptr;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
-
- if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax)))
- return -E2BIG;
-
- cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
- if (!cdptr)
- return -ENOMEM;
-
- val = le64_to_cpu(cdptr[0]);
- cd_live = !!(val & CTXDESC_CD_0_V);
-
- if (!cd) { /* (5) */
- val = 0;
- } else if (cd == &quiet_cd) { /* (4) */
- val |= CTXDESC_CD_0_TCR_EPD0;
- } else if (cd_live) { /* (3) */
- val &= ~CTXDESC_CD_0_ASID;
- val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
- /*
- * Until CD+TLB invalidation, both ASIDs may be used for tagging
- * this substream's traffic
- */
- } else { /* (1) and (2) */
- cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
- cdptr[2] = 0;
- cdptr[3] = cpu_to_le64(cd->mair);
-
- /*
- * STE is live, and the SMMU might read dwords of this CD in any
- * order. Ensure that it observes valid values before reading
- * V=1.
- */
- arm_smmu_sync_cd(smmu_domain, ssid, true);
-
- val = cd->tcr |
-#ifdef __BIG_ENDIAN
- CTXDESC_CD_0_ENDI |
-#endif
- CTXDESC_CD_0_R | CTXDESC_CD_0_A |
- (cd->mm ? 0 : CTXDESC_CD_0_ASET) |
- CTXDESC_CD_0_AA64 |
- FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
- CTXDESC_CD_0_V;
-
- /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
- if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
- val |= CTXDESC_CD_0_S;
- }
-
- /*
- * The SMMU accesses 64-bit values atomically. See IHI0070Ca 3.21.3
- * "Configuration structures and configuration invalidation completion"
- *
- * The size of single-copy atomic reads made by the SMMU is
- * IMPLEMENTATION DEFINED but must be at least 64 bits. Any single
- * field within an aligned 64-bit span of a structure can be altered
- * without first making the structure invalid.
- */
- WRITE_ONCE(cdptr[0], cpu_to_le64(val));
- arm_smmu_sync_cd(smmu_domain, ssid, true);
- return 0;
-}
-
-static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
-{
- int ret;
- size_t l1size;
- size_t max_contexts;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg;
-
- max_contexts = 1 << cfg->s1cdmax;
-
- if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
- max_contexts <= CTXDESC_L2_ENTRIES) {
- cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
- cdcfg->num_l1_ents = max_contexts;
-
- l1size = max_contexts * (CTXDESC_CD_DWORDS << 3);
- } else {
- cfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
- cdcfg->num_l1_ents = DIV_ROUND_UP(max_contexts,
- CTXDESC_L2_ENTRIES);
-
- cdcfg->l1_desc = devm_kcalloc(smmu->dev, cdcfg->num_l1_ents,
- sizeof(*cdcfg->l1_desc),
- GFP_KERNEL);
- if (!cdcfg->l1_desc)
- return -ENOMEM;
-
- l1size = cdcfg->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
- }
-
- cdcfg->cdtab = dmam_alloc_coherent(smmu->dev, l1size, &cdcfg->cdtab_dma,
- GFP_KERNEL);
- if (!cdcfg->cdtab) {
- dev_warn(smmu->dev, "failed to allocate context descriptor\n");
- ret = -ENOMEM;
- goto err_free_l1;
- }
-
- return 0;
-
-err_free_l1:
- if (cdcfg->l1_desc) {
- devm_kfree(smmu->dev, cdcfg->l1_desc);
- cdcfg->l1_desc = NULL;
- }
- return ret;
-}
-
-static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
-{
- int i;
- size_t size, l1size;
- struct arm_smmu_device *smmu = smmu_domain->smmu;
- struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
-
- if (cdcfg->l1_desc) {
- size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
-
- for (i = 0; i < cdcfg->num_l1_ents; i++) {
- if (!cdcfg->l1_desc[i].l2ptr)
- continue;
-
- dmam_free_coherent(smmu->dev, size,
- cdcfg->l1_desc[i].l2ptr,
- cdcfg->l1_desc[i].l2ptr_dma);
- }
- devm_kfree(smmu->dev, cdcfg->l1_desc);
- cdcfg->l1_desc = NULL;
-
- l1size = cdcfg->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3);
- } else {
- l1size = cdcfg->num_l1_ents * (CTXDESC_CD_DWORDS << 3);
- }
-
- dmam_free_coherent(smmu->dev, l1size, cdcfg->cdtab, cdcfg->cdtab_dma);
- cdcfg->cdtab_dma = 0;
- cdcfg->cdtab = NULL;
-}
-
bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd)
{
bool free;
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 9cdccf5d79b3..a50a3e4874f9 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -784,6 +784,9 @@ extern struct arm_smmu_ctx_desc quiet_cd;
int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
struct arm_smmu_ctx_desc *cd);
+int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain);
+void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain);
+void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain, int ssid, bool leaf);
void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid);
bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd);
int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
--
2.17.1
Add a small API in iommu subsystem to handle PASID table allocation
requests from different consumer drivers, such as a paravirtualized
iommu driver. The API provides ops for allocating and freeing PASID
table, writing to it and managing the table caches.
This library also provides for registering a vendor API that attaches
to these ops. The vendor APIs would eventually perform arch level
implementations for these PASID tables.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
drivers/iommu/iommu-pasid-table.h | 134 ++++++++++++++++++++++++++++++
1 file changed, 134 insertions(+)
create mode 100644 drivers/iommu/iommu-pasid-table.h
diff --git a/drivers/iommu/iommu-pasid-table.h b/drivers/iommu/iommu-pasid-table.h
new file mode 100644
index 000000000000..bd4f57656f67
--- /dev/null
+++ b/drivers/iommu/iommu-pasid-table.h
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PASID table management for the IOMMU
+ *
+ * Copyright (C) 2021 Arm Ltd.
+ */
+#ifndef __IOMMU_PASID_TABLE_H
+#define __IOMMU_PASID_TABLE_H
+
+#include <linux/io-pgtable.h>
+
+#include "arm/arm-smmu-v3/arm-smmu-v3.h"
+
+enum pasid_table_fmt {
+ PASID_TABLE_ARM_SMMU_V3,
+ PASID_TABLE_NUM_FMTS,
+};
+
+/**
+ * struct arm_smmu_cfg_info - arm-smmu-v3 specific configuration data
+ *
+ * @s1_cfg: arm-smmu-v3 stage1 config data
+ * @feat_flag: features supported by arm-smmu-v3 implementation
+ */
+struct arm_smmu_cfg_info {
+ struct arm_smmu_s1_cfg *s1_cfg;
+ u32 feat_flag;
+};
+
+/**
+ * struct iommu_vendor_psdtable_cfg - Configuration data for PASID tables
+ *
+ * @iommu_dev: device performing the DMA table walks
+ * @fmt: The PASID table format
+ * @base: DMA address of the allocated table, set by the vendor driver
+ * @cfg: arm-smmu-v3 specific config data
+ */
+struct iommu_vendor_psdtable_cfg {
+ struct device *iommu_dev;
+ enum pasid_table_fmt fmt;
+ dma_addr_t base;
+ union {
+ struct arm_smmu_cfg_info cfg;
+ } vendor;
+};
+
+struct iommu_vendor_psdtable_ops;
+
+/**
+ * struct iommu_pasid_table - describes a set of PASID tables
+ *
+ * @cookie: An opaque token provided by the IOMMU driver and passed back to any
+ * callback routine.
+ * @cfg: A copy of the PASID table configuration
+ * @ops: The PASID table operations in use for this set of page tables
+ */
+struct iommu_pasid_table {
+ void *cookie;
+ struct iommu_vendor_psdtable_cfg cfg;
+ struct iommu_vendor_psdtable_ops *ops;
+};
+
+#define pasid_table_cfg_to_table(pst_cfg) \
+ container_of((pst_cfg), struct iommu_pasid_table, cfg)
+
+struct iommu_vendor_psdtable_ops {
+ int (*alloc)(struct iommu_vendor_psdtable_cfg *cfg);
+ void (*free)(struct iommu_vendor_psdtable_cfg *cfg);
+ void (*prepare)(struct iommu_vendor_psdtable_cfg *cfg,
+ struct io_pgtable_cfg *pgtbl_cfg, u32 asid);
+ int (*write)(struct iommu_vendor_psdtable_cfg *cfg, int ssid,
+ void *cookie);
+ void (*sync)(void *cookie, int ssid, bool leaf);
+};
+
+static inline int iommu_psdtable_alloc(struct iommu_pasid_table *tbl,
+ struct iommu_vendor_psdtable_cfg *cfg)
+{
+ if (!tbl->ops->alloc)
+ return -ENOSYS;
+
+ return tbl->ops->alloc(cfg);
+}
+
+static inline void iommu_psdtable_free(struct iommu_pasid_table *tbl,
+ struct iommu_vendor_psdtable_cfg *cfg)
+{
+ if (!tbl->ops->free)
+ return;
+
+ tbl->ops->free(cfg);
+}
+
+static inline int iommu_psdtable_prepare(struct iommu_pasid_table *tbl,
+ struct iommu_vendor_psdtable_cfg *cfg,
+ struct io_pgtable_cfg *pgtbl_cfg,
+ u32 asid)
+{
+ if (!tbl->ops->prepare)
+ return -ENOSYS;
+
+ tbl->ops->prepare(cfg, pgtbl_cfg, asid);
+ return 0;
+}
+
+static inline int iommu_psdtable_write(struct iommu_pasid_table *tbl,
+ struct iommu_vendor_psdtable_cfg *cfg,
+ int ssid, void *cookie)
+{
+ if (!tbl->ops->write)
+ return -ENOSYS;
+
+ return tbl->ops->write(cfg, ssid, cookie);
+}
+
+static inline int iommu_psdtable_sync(struct iommu_pasid_table *tbl,
+ void *cookie, int ssid, bool leaf)
+{
+ if (!tbl->ops->sync)
+ return -ENOSYS;
+
+ tbl->ops->sync(cookie, ssid, leaf);
+ return 0;
+}
+
+/* A placeholder to register vendor specific pasid layer */
+static inline struct iommu_pasid_table *
+iommu_register_pasid_table(enum pasid_table_fmt fmt,
+ struct device *dev, void *cookie)
+{
+ return NULL;
+}
+
+#endif /* __IOMMU_PASID_TABLE_H */
--
2.17.1
Update base address information in vendor pasid table info to pass that
to user-space for stage1 table management.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
index 8a7187534706..ec37476c8d09 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
@@ -55,6 +55,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct iommu_vendor_psdtable_cfg *pst_cfg,
if (arm_smmu_alloc_cd_leaf_table(dev, l1_desc))
return NULL;
+ if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
+ pst_cfg->base = l1_desc->l2ptr_dma;
+
l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
/* An invalid L1CD can be cached */
@@ -211,6 +214,9 @@ static int arm_smmu_alloc_cd_tables(struct iommu_vendor_psdtable_cfg *pst_cfg)
goto err_free_l1;
}
+ if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_64K_L2)
+ pst_cfg->base = cdcfg->cdtab_dma;
+
return 0;
err_free_l1:
--
2.17.1
Add info about asid_bits and additional flags to table format
probing header.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Michael S. Tsirkin <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
include/uapi/linux/virtio_iommu.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
index 43821e33e7af..8a0624bab4b2 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -169,7 +169,10 @@ struct virtio_iommu_probe_pasid_size {
struct virtio_iommu_probe_table_format {
struct virtio_iommu_probe_property head;
__le16 format;
- __u8 reserved[2];
+ __le16 asid_bits;
+
+ __le32 flags;
+ __u8 reserved[4];
};
struct virtio_iommu_req_probe {
--
2.17.1
From: Jean-Philippe Brucker <[email protected]>
Add required UAPI defines for probing table format for underlying
iommu hardware. The device may provide information about hardware
tables and additional capabilities for each device.
This allows guest to correctly fabricate stage-1 page tables.
Signed-off-by: Jean-Philippe Brucker <[email protected]>
[Vivek: Use a single "struct virtio_iommu_probe_table_format" rather
than separate structures for page table and pasid table format.
Also update commit message.]
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Michael S. Tsirkin <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
include/uapi/linux/virtio_iommu.h | 44 ++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
index 237e36a280cb..43821e33e7af 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -2,7 +2,7 @@
/*
* Virtio-iommu definition v0.12
*
- * Copyright (C) 2019 Arm Ltd.
+ * Copyright (C) 2019-2021 Arm Ltd.
*/
#ifndef _UAPI_LINUX_VIRTIO_IOMMU_H
#define _UAPI_LINUX_VIRTIO_IOMMU_H
@@ -111,6 +111,12 @@ struct virtio_iommu_req_unmap {
#define VIRTIO_IOMMU_PROBE_T_NONE 0
#define VIRTIO_IOMMU_PROBE_T_RESV_MEM 1
+#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK 2
+#define VIRTIO_IOMMU_PROBE_T_INPUT_RANGE 3
+#define VIRTIO_IOMMU_PROBE_T_OUTPUT_SIZE 4
+#define VIRTIO_IOMMU_PROBE_T_PASID_SIZE 5
+#define VIRTIO_IOMMU_PROBE_T_PAGE_TABLE_FMT 6
+#define VIRTIO_IOMMU_PROBE_T_PASID_TABLE_FMT 7
#define VIRTIO_IOMMU_PROBE_T_MASK 0xfff
@@ -130,6 +136,42 @@ struct virtio_iommu_probe_resv_mem {
__le64 end;
};
+struct virtio_iommu_probe_page_size_mask {
+ struct virtio_iommu_probe_property head;
+ __u8 reserved[4];
+ __le64 mask;
+};
+
+struct virtio_iommu_probe_input_range {
+ struct virtio_iommu_probe_property head;
+ __u8 reserved[4];
+ __le64 start;
+ __le64 end;
+};
+
+struct virtio_iommu_probe_output_size {
+ struct virtio_iommu_probe_property head;
+ __u8 bits;
+ __u8 reserved[3];
+};
+
+struct virtio_iommu_probe_pasid_size {
+ struct virtio_iommu_probe_property head;
+ __u8 bits;
+ __u8 reserved[3];
+};
+
+/* Arm LPAE page table format */
+#define VIRTIO_IOMMU_FOMRAT_PGTF_ARM_LPAE 1
+/* Arm smmu-v3 type PASID table format */
+#define VIRTIO_IOMMU_FORMAT_PSTF_ARM_SMMU_V3 2
+
+struct virtio_iommu_probe_table_format {
+ struct virtio_iommu_probe_property head;
+ __le16 format;
+ __u8 reserved[2];
+};
+
struct virtio_iommu_req_probe {
struct virtio_iommu_req_head head;
__le32 endpoint;
--
2.17.1
Te change allows different consumers of arm-smmu-v3-cd-lib to set
their respective sync op for pasid entries.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 1 -
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 +++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
index ec37476c8d09..acaa09acecdd 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
@@ -265,7 +265,6 @@ struct iommu_vendor_psdtable_ops arm_cd_table_ops = {
.free = arm_smmu_free_cd_tables,
.prepare = arm_smmu_prepare_cd,
.write = arm_smmu_write_ctx_desc,
- .sync = arm_smmu_sync_cd,
};
struct iommu_pasid_table *arm_smmu_register_cd_table(struct device *dev,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 2f86c6ac42b6..0c644be22b4b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1869,6 +1869,13 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (ret)
goto out_free_cd_tables;
+ /*
+ * Strange to setup an op here?
+ * cd-lib is the actual user of sync op, and therefore the platform
+ * drivers should assign this sync/maintenance ops as per need.
+ */
+ tbl->ops->sync = arm_smmu_sync_cd;
+
/*
* Note that this will end up calling arm_smmu_sync_cd() before
* the master has been added to the devices list for this domain.
--
2.17.1
From: Jean-Philippe Brucker <[email protected]>
The device may provide information about hardware tables and additional
capabilities for each device. Parse the new probe fields.
Signed-off-by: Jean-Philippe Brucker <[email protected]>
[Vivek: Refactor to use "struct virtio_iommu_probe_table_format" rather
than separate structures for page table and pasid table format.]
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Michael S. Tsirkin <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
drivers/iommu/virtio-iommu.c | 102 ++++++++++++++++++++++++++++++++++-
1 file changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 2bfdd5734844..12d73321dbf4 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -78,6 +78,17 @@ struct viommu_endpoint {
struct viommu_dev *viommu;
struct viommu_domain *vdomain;
struct list_head resv_regions;
+
+ /* properties of the physical IOMMU */
+ u64 pgsize_mask;
+ u64 input_start;
+ u64 input_end;
+ u8 output_bits;
+ u8 pasid_bits;
+ /* Preferred PASID table format */
+ void *pstf;
+ /* Preferred page table format */
+ void *pgtf;
};
struct viommu_request {
@@ -457,6 +468,72 @@ static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
return 0;
}
+static int viommu_add_pgsize_mask(struct viommu_endpoint *vdev,
+ struct virtio_iommu_probe_page_size_mask *prop,
+ size_t len)
+{
+ if (len < sizeof(*prop))
+ return -EINVAL;
+ vdev->pgsize_mask = le64_to_cpu(prop->mask);
+ return 0;
+}
+
+static int viommu_add_input_range(struct viommu_endpoint *vdev,
+ struct virtio_iommu_probe_input_range *prop,
+ size_t len)
+{
+ if (len < sizeof(*prop))
+ return -EINVAL;
+ vdev->input_start = le64_to_cpu(prop->start);
+ vdev->input_end = le64_to_cpu(prop->end);
+ return 0;
+}
+
+static int viommu_add_output_size(struct viommu_endpoint *vdev,
+ struct virtio_iommu_probe_output_size *prop,
+ size_t len)
+{
+ if (len < sizeof(*prop))
+ return -EINVAL;
+ vdev->output_bits = prop->bits;
+ return 0;
+}
+
+static int viommu_add_pasid_size(struct viommu_endpoint *vdev,
+ struct virtio_iommu_probe_pasid_size *prop,
+ size_t len)
+{
+ if (len < sizeof(*prop))
+ return -EINVAL;
+ vdev->pasid_bits = prop->bits;
+ return 0;
+}
+
+static int viommu_add_pgtf(struct viommu_endpoint *vdev, void *pgtf, size_t len)
+{
+ /* Select the first page table format available */
+ if (len < sizeof(struct virtio_iommu_probe_table_format) || vdev->pgtf)
+ return -EINVAL;
+
+ vdev->pgtf = kmemdup(pgtf, len, GFP_KERNEL);
+ if (!vdev->pgtf)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static int viommu_add_pstf(struct viommu_endpoint *vdev, void *pstf, size_t len)
+{
+ if (len < sizeof(struct virtio_iommu_probe_table_format) || vdev->pstf)
+ return -EINVAL;
+
+ vdev->pstf = kmemdup(pstf, len, GFP_KERNEL);
+ if (!vdev->pstf)
+ return -ENOMEM;
+
+ return 0;
+}
+
static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
{
int ret;
@@ -493,11 +570,30 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
while (type != VIRTIO_IOMMU_PROBE_T_NONE &&
cur < viommu->probe_size) {
+ void *value = prop;
len = le16_to_cpu(prop->length) + sizeof(*prop);
switch (type) {
case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
- ret = viommu_add_resv_mem(vdev, (void *)prop, len);
+ ret = viommu_add_resv_mem(vdev, value, len);
+ break;
+ case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK:
+ ret = viommu_add_pgsize_mask(vdev, value, len);
+ break;
+ case VIRTIO_IOMMU_PROBE_T_INPUT_RANGE:
+ ret = viommu_add_input_range(vdev, value, len);
+ break;
+ case VIRTIO_IOMMU_PROBE_T_OUTPUT_SIZE:
+ ret = viommu_add_output_size(vdev, value, len);
+ break;
+ case VIRTIO_IOMMU_PROBE_T_PASID_SIZE:
+ ret = viommu_add_pasid_size(vdev, value, len);
+ break;
+ case VIRTIO_IOMMU_PROBE_T_PAGE_TABLE_FMT:
+ ret = viommu_add_pgtf(vdev, value, len);
+ break;
+ case VIRTIO_IOMMU_PROBE_T_PASID_TABLE_FMT:
+ ret = viommu_add_pstf(vdev, value, len);
break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
@@ -899,6 +995,8 @@ static struct iommu_device *viommu_probe_device(struct device *dev)
err_free_dev:
generic_iommu_put_resv_regions(dev, &vdev->resv_regions);
+ kfree(vdev->pstf);
+ kfree(vdev->pgtf);
kfree(vdev);
return ERR_PTR(ret);
@@ -915,6 +1013,8 @@ static void viommu_release_device(struct device *dev)
vdev = dev_iommu_priv_get(dev);
generic_iommu_put_resv_regions(dev, &vdev->resv_regions);
+ kfree(vdev->pstf);
+ kfree(vdev->pgtf);
kfree(vdev);
}
--
2.17.1
aisd_bits data is required to prepare stage-1 tables for arm-smmu-v3.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
include/uapi/linux/iommu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 082d758dd016..96abbfc7c643 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -357,7 +357,7 @@ struct iommu_pasid_smmuv3 {
__u32 version;
__u8 s1fmt;
__u8 s1dss;
- __u8 padding[2];
+ __u16 asid_bits;
};
/**
--
2.17.1
From: Jean-Philippe Brucker <[email protected]>
Add support for tlb invalidation ops that can send invalidation
requests to back-end virtio-iommu when stage-1 page tables are
supported.
Signed-off-by: Jean-Philippe Brucker <[email protected]>
[Vivek: Refactoring the iommu_flush_ops, and adding only one pasid sync
op that's needed with current iommu-pasid-table infrastructure.
Also updating uapi defines as required by latest changes]
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Michael S. Tsirkin <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
drivers/iommu/virtio-iommu.c | 95 ++++++++++++++++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index ae5dfd3f8269..004ea94e3731 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -13,6 +13,7 @@
#include <linux/freezer.h>
#include <linux/interval_tree.h>
#include <linux/iommu.h>
+#include <linux/io-pgtable.h>
#include <linux/module.h>
#include <linux/of_iommu.h>
#include <linux/of_platform.h>
@@ -63,6 +64,8 @@ struct viommu_mapping {
};
struct viommu_mm {
+ int pasid;
+ u64 archid;
struct io_pgtable_ops *ops;
struct viommu_domain *domain;
};
@@ -692,6 +695,98 @@ static void viommu_event_handler(struct virtqueue *vq)
virtqueue_kick(vq);
}
+/* PASID and pgtable APIs */
+
+static void __viommu_flush_pasid_tlb_all(struct viommu_domain *vdomain,
+ int pasid, u64 arch_id, int type)
+{
+ struct virtio_iommu_req_invalidate req = {
+ .head.type = VIRTIO_IOMMU_T_INVALIDATE,
+ .inv_gran = cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
+ .flags = cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID),
+ .inv_type = cpu_to_le32(type),
+
+ .domain = cpu_to_le32(vdomain->id),
+ .pasid = cpu_to_le32(pasid),
+ .archid = cpu_to_le64(arch_id),
+ };
+
+ if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
+ pr_debug("could not send invalidate request\n");
+}
+
+static void viommu_flush_tlb_add(struct iommu_iotlb_gather *gather,
+ unsigned long iova, size_t granule,
+ void *cookie)
+{
+ struct viommu_mm *viommu_mm = cookie;
+ struct viommu_domain *vdomain = viommu_mm->domain;
+ struct iommu_domain *domain = &vdomain->domain;
+
+ iommu_iotlb_gather_add_page(domain, gather, iova, granule);
+}
+
+static void viommu_flush_tlb_walk(unsigned long iova, size_t size,
+ size_t granule, void *cookie)
+{
+ struct viommu_mm *viommu_mm = cookie;
+ struct viommu_domain *vdomain = viommu_mm->domain;
+ struct virtio_iommu_req_invalidate req = {
+ .head.type = VIRTIO_IOMMU_T_INVALIDATE,
+ .inv_gran = cpu_to_le32(VIRTIO_IOMMU_INVAL_G_VA),
+ .inv_type = cpu_to_le32(VIRTIO_IOMMU_INV_T_IOTLB),
+ .flags = cpu_to_le32(VIRTIO_IOMMU_INVAL_F_ARCHID),
+
+ .domain = cpu_to_le32(vdomain->id),
+ .pasid = cpu_to_le32(viommu_mm->pasid),
+ .archid = cpu_to_le64(viommu_mm->archid),
+ .virt_start = cpu_to_le64(iova),
+ .nr_pages = cpu_to_le64(size / granule),
+ .granule = ilog2(granule),
+ };
+
+ if (viommu_add_req(vdomain->viommu, &req, sizeof(req)))
+ pr_debug("could not add invalidate request\n");
+}
+
+static void viommu_flush_tlb_all(void *cookie)
+{
+ struct viommu_mm *viommu_mm = cookie;
+
+ if (!viommu_mm->archid)
+ return;
+
+ __viommu_flush_pasid_tlb_all(viommu_mm->domain, viommu_mm->pasid,
+ viommu_mm->archid,
+ VIRTIO_IOMMU_INV_T_IOTLB);
+}
+
+static struct iommu_flush_ops viommu_flush_ops = {
+ .tlb_flush_all = viommu_flush_tlb_all,
+ .tlb_flush_walk = viommu_flush_tlb_walk,
+ .tlb_add_page = viommu_flush_tlb_add,
+};
+
+static void viommu_flush_pasid(void *cookie, int pasid, bool leaf)
+{
+ struct viommu_domain *vdomain = cookie;
+ struct virtio_iommu_req_invalidate req = {
+ .head.type = VIRTIO_IOMMU_T_INVALIDATE,
+ .inv_gran = cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
+ .inv_type = cpu_to_le32(VIRTIO_IOMMU_INV_T_PASID),
+ .flags = cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID),
+
+ .domain = cpu_to_le32(vdomain->id),
+ .pasid = cpu_to_le32(pasid),
+ };
+
+ if (leaf)
+ req.flags |= cpu_to_le32(VIRTIO_IOMMU_INVAL_F_LEAF);
+
+ if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
+ pr_debug("could not send invalidate request\n");
+}
+
/* IOMMU API */
static struct iommu_domain *viommu_domain_alloc(unsigned type)
--
2.17.1
From: Jean-Philippe Brucker <[email protected]>
Add the required UAPI defines for binding pasid tables in virtio-iommu.
This mode allows to hand stage-1 page tables over to the guest.
Signed-off-by: Jean-Philippe Brucker <[email protected]>
[Vivek: Refactor to cleanup headers for invalidation]
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Michael S. Tsirkin <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Kevin Tian <[email protected]>
Cc: Jacob Pan <[email protected]>
Cc: Liu Yi L <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Shameerali Kolothum Thodi <[email protected]>
---
include/uapi/linux/virtio_iommu.h | 68 +++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
index 8a0624bab4b2..3481e4a3dd24 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -16,6 +16,7 @@
#define VIRTIO_IOMMU_F_BYPASS 3
#define VIRTIO_IOMMU_F_PROBE 4
#define VIRTIO_IOMMU_F_MMIO 5
+#define VIRTIO_IOMMU_F_ATTACH_TABLE 6
struct virtio_iommu_range_64 {
__le64 start;
@@ -44,6 +45,8 @@ struct virtio_iommu_config {
#define VIRTIO_IOMMU_T_MAP 0x03
#define VIRTIO_IOMMU_T_UNMAP 0x04
#define VIRTIO_IOMMU_T_PROBE 0x05
+#define VIRTIO_IOMMU_T_ATTACH_TABLE 0x06
+#define VIRTIO_IOMMU_T_INVALIDATE 0x07
/* Status types */
#define VIRTIO_IOMMU_S_OK 0x00
@@ -82,6 +85,37 @@ struct virtio_iommu_req_detach {
struct virtio_iommu_req_tail tail;
};
+struct virtio_iommu_req_attach_table {
+ struct virtio_iommu_req_head head;
+ __le32 domain;
+ __le32 endpoint;
+ __le16 format;
+ __u8 reserved[62];
+ struct virtio_iommu_req_tail tail;
+};
+
+#define VIRTIO_IOMMU_PSTF_ARM_SMMU_V3_LINEAR 0x0
+#define VIRTIO_IOMMU_PSTF_ARM_SMMU_V3_4KL2 0x1
+#define VIRTIO_IOMMU_PSTF_ARM_SMMU_V3_64KL2 0x2
+
+#define VIRTIO_IOMMU_PSTF_ARM_SMMU_V3_DSS_TERM 0x0
+#define VIRTIO_IOMMU_PSTF_ARM_SMMU_V3_DSS_BYPASS 0x1
+#define VIRTIO_IOMMU_PSTF_ARM_SMMU_V3_DSS_0 0x2
+
+/* Arm SMMUv3 PASID Table Descriptor */
+struct virtio_iommu_req_attach_pst_arm {
+ struct virtio_iommu_req_head head;
+ __le32 domain;
+ __le32 endpoint;
+ __le16 format;
+ __u8 s1fmt;
+ __u8 s1dss;
+ __le64 s1contextptr;
+ __le32 s1cdmax;
+ __u8 reserved[48];
+ struct virtio_iommu_req_tail tail;
+};
+
#define VIRTIO_IOMMU_MAP_F_READ (1 << 0)
#define VIRTIO_IOMMU_MAP_F_WRITE (1 << 1)
#define VIRTIO_IOMMU_MAP_F_MMIO (1 << 2)
@@ -188,6 +222,40 @@ struct virtio_iommu_req_probe {
*/
};
+#define VIRTIO_IOMMU_INVAL_G_DOMAIN (1 << 0)
+#define VIRTIO_IOMMU_INVAL_G_PASID (1 << 1)
+#define VIRTIO_IOMMU_INVAL_G_VA (1 << 2)
+
+#define VIRTIO_IOMMU_INV_T_IOTLB (1 << 0)
+#define VIRTIO_IOMMU_INV_T_DEV_IOTLB (1 << 1)
+#define VIRTIO_IOMMU_INV_T_PASID (1 << 2)
+
+#define VIRTIO_IOMMU_INVAL_F_PASID (1 << 0)
+#define VIRTIO_IOMMU_INVAL_F_ARCHID (1 << 1)
+#define VIRTIO_IOMMU_INVAL_F_LEAF (1 << 2)
+
+struct virtio_iommu_req_invalidate {
+ struct virtio_iommu_req_head head;
+ __le16 inv_gran;
+ __le16 inv_type;
+
+ __le16 flags;
+ __u8 reserved1[2];
+ __le32 domain;
+
+ __le32 pasid;
+ __u8 reserved2[4];
+
+ __le64 archid;
+ __le64 virt_start;
+ __le64 nr_pages;
+
+ /* Page size, in nr of bits, typically 12 for 4k, 30 for 2MB, etc.) */
+ __u8 granule;
+ __u8 reserved3[11];
+ struct virtio_iommu_req_tail tail;
+};
+
/* Fault types */
#define VIRTIO_IOMMU_FAULT_R_UNKNOWN 0
#define VIRTIO_IOMMU_FAULT_R_DOMAIN 1
--
2.17.1
Hi Vivek,
On 1/15/21 1:13 PM, Vivek Gautam wrote:
> This patch-series aims at enabling Nested stage translation in guests
> using virtio-iommu as the paravirtualized iommu. The backend is supported
> with Arm SMMU-v3 that provides nested stage-1 and stage-2 translation.
>
> This series derives its purpose from various efforts happening to add
> support for Shared Virtual Addressing (SVA) in host and guest. On Arm,
> most of the support for SVA has already landed. The support for nested
> stage translation and fault reporting to guest has been proposed [1].
> The related changes required in VFIO [2] framework have also been put
> forward.
>
> This series proposes changes in virtio-iommu to program PASID tables
> and related stage-1 page tables. A simple iommu-pasid-table library
> is added for this purpose that interacts with vendor drivers to
> allocate and populate PASID tables.
> In Arm SMMUv3 we propose to pull the Context Descriptor (CD) management
> code out of the arm-smmu-v3 driver and add that as a glue vendor layer
> to support allocating CD tables, and populating them with right values.
> These CD tables are essentially the PASID tables and contain stage-1
> page table configurations too.
> A request to setup these CD tables come from virtio-iommu driver using
> the iommu-pasid-table library when running on Arm. The virtio-iommu
> then pass these PASID tables to the host using the right virtio backend
> and support in VMM.
>
> For testing we have added necessary support in kvmtool. The changes in
> kvmtool are based on virtio-iommu development branch by Jean-Philippe
> Brucker [3].
>
> The tested kernel branch contains following in the order bottom to top
> on the git hash -
> a) v5.11-rc3
> b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
> table support for Arm.
> c) Smmu test engine patches from Jean-Philippe's branch [4]
> d) This series
> e) Domain nesting info patches [5][6][7].
> f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
> the list).
>
> This kernel is tested on Neoverse reference software stack with
> Fixed virtual platform. Public version of the software stack and
> FVP is available here[8][9].
>
> A big thanks to Jean-Philippe for his contributions towards this work
> and for his valuable guidance.
>
> [1] https://lore.kernel.org/linux-iommu/[email protected]/T/
> [2] https://lore.kernel.org/kvmarm/[email protected]/T/
> [3] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
> [4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
> [5] https://lore.kernel.org/kvm/[email protected]/
> [6] https://lore.kernel.org/kvm/[email protected]/
> [7] https://lore.kernel.org/kvm/[email protected]/
> [8] https://developer.arm.com/tools-and-software/open-source-software/arm-platforms-software/arm-ecosystem-fvps
> [9] https://git.linaro.org/landing-teams/working/arm/arm-reference-platforms.git/about/docs/rdn1edge/user-guide.rst
Could you share a public branch where we could find all the kernel pieces.
Thank you in advance
Best Regards
Eric
>
> Jean-Philippe Brucker (6):
> iommu/virtio: Add headers for table format probing
> iommu/virtio: Add table format probing
> iommu/virtio: Add headers for binding pasid table in iommu
> iommu/virtio: Add support for INVALIDATE request
> iommu/virtio: Attach Arm PASID tables when available
> iommu/virtio: Add support for Arm LPAE page table format
>
> Vivek Gautam (9):
> iommu/arm-smmu-v3: Create a Context Descriptor library
> iommu: Add a simple PASID table library
> iommu/arm-smmu-v3: Update drivers to work with iommu-pasid-table
> iommu/arm-smmu-v3: Update CD base address info for user-space
> iommu/arm-smmu-v3: Set sync op from consumer driver of cd-lib
> iommu: Add asid_bits to arm smmu-v3 stage1 table info
> iommu/virtio: Update table format probing header
> iommu/virtio: Prepare to add attach pasid table infrastructure
> iommu/virtio: Update fault type and reason info for viommu fault
>
> drivers/iommu/arm/arm-smmu-v3/Makefile | 2 +-
> .../arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 283 +++++++
> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 16 +-
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 268 +------
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 +-
> drivers/iommu/iommu-pasid-table.h | 140 ++++
> drivers/iommu/virtio-iommu.c | 692 +++++++++++++++++-
> include/uapi/linux/iommu.h | 2 +-
> include/uapi/linux/virtio_iommu.h | 158 +++-
> 9 files changed, 1303 insertions(+), 262 deletions(-)
> create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
> create mode 100644 drivers/iommu/iommu-pasid-table.h
>
Hi Eric,
On 1/19/21 2:33 PM, Auger Eric wrote:
> Hi Vivek,
>
> On 1/15/21 1:13 PM, Vivek Gautam wrote:
>> This patch-series aims at enabling Nested stage translation in guests
>> using virtio-iommu as the paravirtualized iommu. The backend is supported
>> with Arm SMMU-v3 that provides nested stage-1 and stage-2 translation.
>>
>> This series derives its purpose from various efforts happening to add
>> support for Shared Virtual Addressing (SVA) in host and guest. On Arm,
>> most of the support for SVA has already landed. The support for nested
>> stage translation and fault reporting to guest has been proposed [1].
>> The related changes required in VFIO [2] framework have also been put
>> forward.
>>
>> This series proposes changes in virtio-iommu to program PASID tables
>> and related stage-1 page tables. A simple iommu-pasid-table library
>> is added for this purpose that interacts with vendor drivers to
>> allocate and populate PASID tables.
>> In Arm SMMUv3 we propose to pull the Context Descriptor (CD) management
>> code out of the arm-smmu-v3 driver and add that as a glue vendor layer
>> to support allocating CD tables, and populating them with right values.
>> These CD tables are essentially the PASID tables and contain stage-1
>> page table configurations too.
>> A request to setup these CD tables come from virtio-iommu driver using
>> the iommu-pasid-table library when running on Arm. The virtio-iommu
>> then pass these PASID tables to the host using the right virtio backend
>> and support in VMM.
>>
>> For testing we have added necessary support in kvmtool. The changes in
>> kvmtool are based on virtio-iommu development branch by Jean-Philippe
>> Brucker [3].
>>
>> The tested kernel branch contains following in the order bottom to top
>> on the git hash -
>> a) v5.11-rc3
>> b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
>> table support for Arm.
>> c) Smmu test engine patches from Jean-Philippe's branch [4]
>> d) This series
>> e) Domain nesting info patches [5][6][7].
>> f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
>> the list).
>>
>> This kernel is tested on Neoverse reference software stack with
>> Fixed virtual platform. Public version of the software stack and
>> FVP is available here[8][9].
>>
>> A big thanks to Jean-Philippe for his contributions towards this work
>> and for his valuable guidance.
>>
>> [1] https://lore.kernel.org/linux-iommu/[email protected]/T/
>> [2] https://lore.kernel.org/kvmarm/[email protected]/T/
>> [3] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
>> [4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
>> [5] https://lore.kernel.org/kvm/[email protected]/
>> [6] https://lore.kernel.org/kvm/[email protected]/
>> [7] https://lore.kernel.org/kvm/[email protected]/
>> [8] https://developer.arm.com/tools-and-software/open-source-software/arm-platforms-software/arm-ecosystem-fvps
>> [9] https://git.linaro.org/landing-teams/working/arm/arm-reference-platforms.git/about/docs/rdn1edge/user-guide.rst
>
> Could you share a public branch where we could find all the kernel pieces.
>
> Thank you in advance
Apologies for the delay. It took a bit of time to sort things out for a
public branch.
The branch is available in my github now. Please have a look.
https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-virtio-iommu
Thanks and regards
Vivek
>
> Best Regards
>
> Eric
>>
>> Jean-Philippe Brucker (6):
>> iommu/virtio: Add headers for table format probing
>> iommu/virtio: Add table format probing
>> iommu/virtio: Add headers for binding pasid table in iommu
>> iommu/virtio: Add support for INVALIDATE request
>> iommu/virtio: Attach Arm PASID tables when available
>> iommu/virtio: Add support for Arm LPAE page table format
>>
>> Vivek Gautam (9):
>> iommu/arm-smmu-v3: Create a Context Descriptor library
>> iommu: Add a simple PASID table library
>> iommu/arm-smmu-v3: Update drivers to work with iommu-pasid-table
>> iommu/arm-smmu-v3: Update CD base address info for user-space
>> iommu/arm-smmu-v3: Set sync op from consumer driver of cd-lib
>> iommu: Add asid_bits to arm smmu-v3 stage1 table info
>> iommu/virtio: Update table format probing header
>> iommu/virtio: Prepare to add attach pasid table infrastructure
>> iommu/virtio: Update fault type and reason info for viommu fault
>>
>> drivers/iommu/arm/arm-smmu-v3/Makefile | 2 +-
>> .../arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 283 +++++++
>> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 16 +-
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 268 +------
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 +-
>> drivers/iommu/iommu-pasid-table.h | 140 ++++
>> drivers/iommu/virtio-iommu.c | 692 +++++++++++++++++-
>> include/uapi/linux/iommu.h | 2 +-
>> include/uapi/linux/virtio_iommu.h | 158 +++-
>> 9 files changed, 1303 insertions(+), 262 deletions(-)
>> create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>> create mode 100644 drivers/iommu/iommu-pasid-table.h
>>
>
Hi Vivek,
> -----Original Message-----
> From: Vivek Kumar Gautam [mailto:[email protected]]
> Sent: 21 January 2021 17:34
> To: Auger Eric <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Shameerali Kolothum Thodi <[email protected]>
> Subject: Re: [PATCH RFC v1 00/15] iommu/virtio: Nested stage support with
> Arm
>
> Hi Eric,
>
>
> On 1/19/21 2:33 PM, Auger Eric wrote:
> > Hi Vivek,
> >
> > On 1/15/21 1:13 PM, Vivek Gautam wrote:
> >> This patch-series aims at enabling Nested stage translation in guests
> >> using virtio-iommu as the paravirtualized iommu. The backend is
> >> supported with Arm SMMU-v3 that provides nested stage-1 and stage-2
> translation.
> >>
> >> This series derives its purpose from various efforts happening to add
> >> support for Shared Virtual Addressing (SVA) in host and guest. On
> >> Arm, most of the support for SVA has already landed. The support for
> >> nested stage translation and fault reporting to guest has been proposed [1].
> >> The related changes required in VFIO [2] framework have also been put
> >> forward.
> >>
> >> This series proposes changes in virtio-iommu to program PASID tables
> >> and related stage-1 page tables. A simple iommu-pasid-table library
> >> is added for this purpose that interacts with vendor drivers to
> >> allocate and populate PASID tables.
> >> In Arm SMMUv3 we propose to pull the Context Descriptor (CD)
> >> management code out of the arm-smmu-v3 driver and add that as a glue
> >> vendor layer to support allocating CD tables, and populating them with right
> values.
> >> These CD tables are essentially the PASID tables and contain stage-1
> >> page table configurations too.
> >> A request to setup these CD tables come from virtio-iommu driver
> >> using the iommu-pasid-table library when running on Arm. The
> >> virtio-iommu then pass these PASID tables to the host using the right
> >> virtio backend and support in VMM.
> >>
> >> For testing we have added necessary support in kvmtool. The changes
> >> in kvmtool are based on virtio-iommu development branch by
> >> Jean-Philippe Brucker [3].
> >>
> >> The tested kernel branch contains following in the order bottom to
> >> top on the git hash -
> >> a) v5.11-rc3
> >> b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
> >> table support for Arm.
> >> c) Smmu test engine patches from Jean-Philippe's branch [4]
> >> d) This series
> >> e) Domain nesting info patches [5][6][7].
> >> f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
> >> the list).
> >>
> >> This kernel is tested on Neoverse reference software stack with Fixed
> >> virtual platform. Public version of the software stack and FVP is
> >> available here[8][9].
> >>
> >> A big thanks to Jean-Philippe for his contributions towards this work
> >> and for his valuable guidance.
> >>
> >> [1]
> >> https://lore.kernel.org/linux-iommu/20201118112151.25412-1-eric.auger
> >> @redhat.com/T/ [2]
> >>
> https://lore.kernel.org/kvmarm/20201116110030.32335-12-eric.auger@red
> >> hat.com/T/ [3]
> >> https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
> >> [4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
> >> [5]
> >> https://lore.kernel.org/kvm/1599734733-6431-2-git-send-email-yi.l.liu
> >> @intel.com/ [6]
> >> https://lore.kernel.org/kvm/1599734733-6431-3-git-send-email-yi.l.liu
> >> @intel.com/ [7]
> >> https://lore.kernel.org/kvm/1599734733-6431-4-git-send-email-yi.l.liu
> >> @intel.com/ [8]
> >> https://developer.arm.com/tools-and-software/open-source-software/arm
> >> -platforms-software/arm-ecosystem-fvps
> >> [9]
> >> https://git.linaro.org/landing-teams/working/arm/arm-reference-platfo
> >> rms.git/about/docs/rdn1edge/user-guide.rst
> >
> > Could you share a public branch where we could find all the kernel pieces.
> >
> > Thank you in advance
>
> Apologies for the delay. It took a bit of time to sort things out for a public
> branch.
> The branch is available in my github now. Please have a look.
>
> https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-vi
> rtio-iommu
Thanks for this. Do you have a corresponding kvmtool branch mentioned above as public?
Thanks,
Shameer
Hi Shameer,
On 1/22/21 9:19 PM, Shameerali Kolothum Thodi wrote:
> Hi Vivek,
>
>> -----Original Message-----
>> From: Vivek Kumar Gautam [mailto:[email protected]]
>> Sent: 21 January 2021 17:34
>> To: Auger Eric <[email protected]>; [email protected];
>> [email protected]; [email protected];
>> [email protected]
>> Cc: [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> Shameerali Kolothum Thodi <[email protected]>
>> Subject: Re: [PATCH RFC v1 00/15] iommu/virtio: Nested stage support with
>> Arm
>>
>> Hi Eric,
>>
>>
>> On 1/19/21 2:33 PM, Auger Eric wrote:
>>> Hi Vivek,
>>>
>>> On 1/15/21 1:13 PM, Vivek Gautam wrote:
>>>> This patch-series aims at enabling Nested stage translation in guests
>>>> using virtio-iommu as the paravirtualized iommu. The backend is
>>>> supported with Arm SMMU-v3 that provides nested stage-1 and stage-2
>> translation.
>>>>
>>>> This series derives its purpose from various efforts happening to add
>>>> support for Shared Virtual Addressing (SVA) in host and guest. On
>>>> Arm, most of the support for SVA has already landed. The support for
>>>> nested stage translation and fault reporting to guest has been proposed [1].
>>>> The related changes required in VFIO [2] framework have also been put
>>>> forward.
>>>>
>>>> This series proposes changes in virtio-iommu to program PASID tables
>>>> and related stage-1 page tables. A simple iommu-pasid-table library
>>>> is added for this purpose that interacts with vendor drivers to
>>>> allocate and populate PASID tables.
>>>> In Arm SMMUv3 we propose to pull the Context Descriptor (CD)
>>>> management code out of the arm-smmu-v3 driver and add that as a glue
>>>> vendor layer to support allocating CD tables, and populating them with right
>> values.
>>>> These CD tables are essentially the PASID tables and contain stage-1
>>>> page table configurations too.
>>>> A request to setup these CD tables come from virtio-iommu driver
>>>> using the iommu-pasid-table library when running on Arm. The
>>>> virtio-iommu then pass these PASID tables to the host using the right
>>>> virtio backend and support in VMM.
>>>>
>>>> For testing we have added necessary support in kvmtool. The changes
>>>> in kvmtool are based on virtio-iommu development branch by
>>>> Jean-Philippe Brucker [3].
>>>>
>>>> The tested kernel branch contains following in the order bottom to
>>>> top on the git hash -
>>>> a) v5.11-rc3
>>>> b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
>>>> table support for Arm.
>>>> c) Smmu test engine patches from Jean-Philippe's branch [4]
>>>> d) This series
>>>> e) Domain nesting info patches [5][6][7].
>>>> f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
>>>> the list).
>>>>
>>>> This kernel is tested on Neoverse reference software stack with Fixed
>>>> virtual platform. Public version of the software stack and FVP is
>>>> available here[8][9].
>>>>
>>>> A big thanks to Jean-Philippe for his contributions towards this work
>>>> and for his valuable guidance.
>>>>
>>>> [1]
>>>> https://lore.kernel.org/linux-iommu/20201118112151.25412-1-eric.auger
>>>> @redhat.com/T/ [2]
>>>>
>> https://lore.kernel.org/kvmarm/20201116110030.32335-12-eric.auger@red
>>>> hat.com/T/ [3]
>>>> https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
>>>> [4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
>>>> [5]
>>>> https://lore.kernel.org/kvm/1599734733-6431-2-git-send-email-yi.l.liu
>>>> @intel.com/ [6]
>>>> https://lore.kernel.org/kvm/1599734733-6431-3-git-send-email-yi.l.liu
>>>> @intel.com/ [7]
>>>> https://lore.kernel.org/kvm/1599734733-6431-4-git-send-email-yi.l.liu
>>>> @intel.com/ [8]
>>>> https://developer.arm.com/tools-and-software/open-source-software/arm
>>>> -platforms-software/arm-ecosystem-fvps
>>>> [9]
>>>> https://git.linaro.org/landing-teams/working/arm/arm-reference-platfo
>>>> rms.git/about/docs/rdn1edge/user-guide.rst
>>>
>>> Could you share a public branch where we could find all the kernel pieces.
>>>
>>> Thank you in advance
>>
>> Apologies for the delay. It took a bit of time to sort things out for a public
>> branch.
>> The branch is available in my github now. Please have a look.
>>
>> https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-vi
>> rtio-iommu
> > Thanks for this. Do you have a corresponding kvmtool branch mentioned
above as public?
Thanks for showing interest. I will publish the kvmtool branch asap.
Though the current development is based on Jean's branch for
virtio-iommu [1], I plan to rebase the changes to master soon.
Thanks & regards
Vivek
[1] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
>
> Thanks,
> Shameer
>
Hi Vivek,
On 1/21/21 6:34 PM, Vivek Kumar Gautam wrote:
> Hi Eric,
>
>
> On 1/19/21 2:33 PM, Auger Eric wrote:
>> Hi Vivek,
>>
>> On 1/15/21 1:13 PM, Vivek Gautam wrote:
>>> This patch-series aims at enabling Nested stage translation in guests
>>> using virtio-iommu as the paravirtualized iommu. The backend is
>>> supported
>>> with Arm SMMU-v3 that provides nested stage-1 and stage-2 translation.
>>>
>>> This series derives its purpose from various efforts happening to add
>>> support for Shared Virtual Addressing (SVA) in host and guest. On Arm,
>>> most of the support for SVA has already landed. The support for nested
>>> stage translation and fault reporting to guest has been proposed [1].
>>> The related changes required in VFIO [2] framework have also been put
>>> forward.
>>>
>>> This series proposes changes in virtio-iommu to program PASID tables
>>> and related stage-1 page tables. A simple iommu-pasid-table library
>>> is added for this purpose that interacts with vendor drivers to
>>> allocate and populate PASID tables.
>>> In Arm SMMUv3 we propose to pull the Context Descriptor (CD) management
>>> code out of the arm-smmu-v3 driver and add that as a glue vendor layer
>>> to support allocating CD tables, and populating them with right values.
>>> These CD tables are essentially the PASID tables and contain stage-1
>>> page table configurations too.
>>> A request to setup these CD tables come from virtio-iommu driver using
>>> the iommu-pasid-table library when running on Arm. The virtio-iommu
>>> then pass these PASID tables to the host using the right virtio backend
>>> and support in VMM.
>>>
>>> For testing we have added necessary support in kvmtool. The changes in
>>> kvmtool are based on virtio-iommu development branch by Jean-Philippe
>>> Brucker [3].
>>>
>>> The tested kernel branch contains following in the order bottom to top
>>> on the git hash -
>>> a) v5.11-rc3
>>> b) arm-smmu-v3 [1] and vfio [2] changes from Eric to add nested page
>>> Â Â Â table support for Arm.
>>> c) Smmu test engine patches from Jean-Philippe's branch [4]
>>> d) This series
>>> e) Domain nesting info patches [5][6][7].
>>> f) Changes to add arm-smmu-v3 specific nesting info (to be sent to
>>> Â Â Â the list).
>>>
>>> This kernel is tested on Neoverse reference software stack with
>>> Fixed virtual platform. Public version of the software stack and
>>> FVP is available here[8][9].
>>>
>>> A big thanks to Jean-Philippe for his contributions towards this work
>>> and for his valuable guidance.
>>>
>>> [1]
>>> https://lore.kernel.org/linux-iommu/[email protected]/T/
>>>
>>> [2]
>>> https://lore.kernel.org/kvmarm/[email protected]/T/
>>>
>>> [3] https://jpbrucker.net/git/kvmtool/log/?h=virtio-iommu/devel
>>> [4] https://jpbrucker.net/git/linux/log/?h=sva/smmute
>>> [5]
>>> https://lore.kernel.org/kvm/[email protected]/
>>>
>>> [6]
>>> https://lore.kernel.org/kvm/[email protected]/
>>>
>>> [7]
>>> https://lore.kernel.org/kvm/[email protected]/
>>>
>>> [8]
>>> https://developer.arm.com/tools-and-software/open-source-software/arm-platforms-software/arm-ecosystem-fvps
>>>
>>> [9]
>>> https://git.linaro.org/landing-teams/working/arm/arm-reference-platforms.git/about/docs/rdn1edge/user-guide.rst
>>>
>>
>> Could you share a public branch where we could find all the kernel
>> pieces.
>>
>> Thank you in advance
>
> Apologies for the delay. It took a bit of time to sort things out for a
> public branch.
> The branch is available in my github now. Please have a look.
>
> https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-virtio-iommu
no problem. Thank you for the link.
Best Regards
Eric
>
>
>
> Thanks and regards
> Vivek
>
>>
>> Best Regards
>>
>> Eric
>>>
>>> Jean-Philippe Brucker (6):
>>> Â Â iommu/virtio: Add headers for table format probing
>>> Â Â iommu/virtio: Add table format probing
>>> Â Â iommu/virtio: Add headers for binding pasid table in iommu
>>> Â Â iommu/virtio: Add support for INVALIDATE request
>>> Â Â iommu/virtio: Attach Arm PASID tables when available
>>> Â Â iommu/virtio: Add support for Arm LPAE page table format
>>>
>>> Vivek Gautam (9):
>>> Â Â iommu/arm-smmu-v3: Create a Context Descriptor library
>>> Â Â iommu: Add a simple PASID table library
>>> Â Â iommu/arm-smmu-v3: Update drivers to work with iommu-pasid-table
>>> Â Â iommu/arm-smmu-v3: Update CD base address info for user-space
>>> Â Â iommu/arm-smmu-v3: Set sync op from consumer driver of cd-lib
>>> Â Â iommu: Add asid_bits to arm smmu-v3 stage1 table info
>>> Â Â iommu/virtio: Update table format probing header
>>> Â Â iommu/virtio: Prepare to add attach pasid table infrastructure
>>> Â Â iommu/virtio: Update fault type and reason info for viommu fault
>>>
>>>  drivers/iommu/arm/arm-smmu-v3/Makefile       |  2 +-
>>>  .../arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c     | 283 +++++++
>>>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c  | 16 +-
>>>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c  | 268 +------
>>>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h  |  4 +-
>>>  drivers/iommu/iommu-pasid-table.h            | 140 ++++
>>>  drivers/iommu/virtio-iommu.c                 | 692 +++++++++++++++++-
>>>  include/uapi/linux/iommu.h                   |  2 +-
>>>  include/uapi/linux/virtio_iommu.h            | 158 +++-
>>> Â 9 files changed, 1303 insertions(+), 262 deletions(-)
>>> Â create mode 100644 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>>> Â create mode 100644 drivers/iommu/iommu-pasid-table.h
>>>
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
On Fri, Jan 15, 2021 at 05:43:31PM +0530, Vivek Gautam wrote:
> Update base address information in vendor pasid table info to pass that
> to user-space for stage1 table management.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Kevin Tian <[email protected]>
> Cc: Jacob Pan <[email protected]>
> Cc: Liu Yi L <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Shameerali Kolothum Thodi <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
> index 8a7187534706..ec37476c8d09 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
> @@ -55,6 +55,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct iommu_vendor_psdtable_cfg *pst_cfg,
> if (arm_smmu_alloc_cd_leaf_table(dev, l1_desc))
> return NULL;
>
> + if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
> + pst_cfg->base = l1_desc->l2ptr_dma;
> +
This isn't the right place, because this path allocates second-level
tables for two-level tables. I don't think we need pst_cfg->base at all,
because for both linear and two-level tables, the base pointer is in
cdcfg->cdtab_dma, which can be read directly.
Thanks,
Jean
> l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
> arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
> /* An invalid L1CD can be cached */
> @@ -211,6 +214,9 @@ static int arm_smmu_alloc_cd_tables(struct iommu_vendor_psdtable_cfg *pst_cfg)
> goto err_free_l1;
> }
>
> + if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_64K_L2)
> + pst_cfg->base = cdcfg->cdtab_dma;
> +
> return 0;
>
> err_free_l1:
> --
> 2.17.1
>
On Fri, Jan 15, 2021 at 05:43:32PM +0530, Vivek Gautam wrote:
> Te change allows different consumers of arm-smmu-v3-cd-lib to set
> their respective sync op for pasid entries.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Kevin Tian <[email protected]>
> Cc: Jacob Pan <[email protected]>
> Cc: Liu Yi L <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Shameerali Kolothum Thodi <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 1 -
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 +++++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
> index ec37476c8d09..acaa09acecdd 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
> @@ -265,7 +265,6 @@ struct iommu_vendor_psdtable_ops arm_cd_table_ops = {
> .free = arm_smmu_free_cd_tables,
> .prepare = arm_smmu_prepare_cd,
> .write = arm_smmu_write_ctx_desc,
> - .sync = arm_smmu_sync_cd,
> };
>
> struct iommu_pasid_table *arm_smmu_register_cd_table(struct device *dev,
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 2f86c6ac42b6..0c644be22b4b 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1869,6 +1869,13 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> if (ret)
> goto out_free_cd_tables;
>
> + /*
> + * Strange to setup an op here?
> + * cd-lib is the actual user of sync op, and therefore the platform
> + * drivers should assign this sync/maintenance ops as per need.
> + */
> + tbl->ops->sync = arm_smmu_sync_cd;
> +
Modifying a static struct from here doesn't feel right. I think the
interface should be roughly similar to io-pgtable since the principle is
the same. So the sync() op should be separate from arm_cd_table_ops since
it's a callback into the driver. Maybe pass it to
iommu_register_pasid_table().
Thanks,
Jean
Hi Vivek,
Thanks again for working on this. I have a few comments but it looks
sensible overall.
Regarding the overall design, I was initially assigning page directories
instead of whole PASID tables, which would simplify the driver and host
implementation. A major complication, however, is SMMUv3 accesses PASID
tables using a guest-physical address, so there is a messy negotiation
needed between host and guest when the host needs to allocate PASID
tables. Plus vSMMU needs PASID table assignment, so that's what the host
driver will implement.
On Fri, Jan 15, 2021 at 05:43:29PM +0530, Vivek Gautam wrote:
> Add a small API in iommu subsystem to handle PASID table allocation
> requests from different consumer drivers, such as a paravirtualized
> iommu driver. The API provides ops for allocating and freeing PASID
> table, writing to it and managing the table caches.
>
> This library also provides for registering a vendor API that attaches
> to these ops. The vendor APIs would eventually perform arch level
> implementations for these PASID tables.
Although Arm might be the only vendor to ever use this, I think the
abstraction makes sense and isn't too invasive. Even if we called directly
into the SMMU driver from the virtio one, we'd still need patch 3 and
separate TLB invalidations ops.
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Kevin Tian <[email protected]>
> Cc: Jacob Pan <[email protected]>
> Cc: Liu Yi L <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Shameerali Kolothum Thodi <[email protected]>
> ---
> drivers/iommu/iommu-pasid-table.h | 134 ++++++++++++++++++++++++++++++
> 1 file changed, 134 insertions(+)
> create mode 100644 drivers/iommu/iommu-pasid-table.h
>
> diff --git a/drivers/iommu/iommu-pasid-table.h b/drivers/iommu/iommu-pasid-table.h
> new file mode 100644
> index 000000000000..bd4f57656f67
> --- /dev/null
> +++ b/drivers/iommu/iommu-pasid-table.h
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PASID table management for the IOMMU
> + *
> + * Copyright (C) 2021 Arm Ltd.
> + */
> +#ifndef __IOMMU_PASID_TABLE_H
> +#define __IOMMU_PASID_TABLE_H
> +
> +#include <linux/io-pgtable.h>
> +
> +#include "arm/arm-smmu-v3/arm-smmu-v3.h"
> +
> +enum pasid_table_fmt {
> + PASID_TABLE_ARM_SMMU_V3,
> + PASID_TABLE_NUM_FMTS,
> +};
> +
> +/**
> + * struct arm_smmu_cfg_info - arm-smmu-v3 specific configuration data
> + *
> + * @s1_cfg: arm-smmu-v3 stage1 config data
> + * @feat_flag: features supported by arm-smmu-v3 implementation
> + */
> +struct arm_smmu_cfg_info {
> + struct arm_smmu_s1_cfg *s1_cfg;
> + u32 feat_flag;
> +};
> +
> +/**
> + * struct iommu_vendor_psdtable_cfg - Configuration data for PASID tables
> + *
> + * @iommu_dev: device performing the DMA table walks
> + * @fmt: The PASID table format
> + * @base: DMA address of the allocated table, set by the vendor driver
> + * @cfg: arm-smmu-v3 specific config data
> + */
> +struct iommu_vendor_psdtable_cfg {
> + struct device *iommu_dev;
> + enum pasid_table_fmt fmt;
> + dma_addr_t base;
> + union {
> + struct arm_smmu_cfg_info cfg;
For the union to be extensible, that field should be called "arm" or
something like that.
Thanks,
Jean
> + } vendor;
> +};
> +
> +struct iommu_vendor_psdtable_ops;
> +
> +/**
> + * struct iommu_pasid_table - describes a set of PASID tables
> + *
> + * @cookie: An opaque token provided by the IOMMU driver and passed back to any
> + * callback routine.
> + * @cfg: A copy of the PASID table configuration
> + * @ops: The PASID table operations in use for this set of page tables
> + */
> +struct iommu_pasid_table {
> + void *cookie;
> + struct iommu_vendor_psdtable_cfg cfg;
> + struct iommu_vendor_psdtable_ops *ops;
> +};
> +
> +#define pasid_table_cfg_to_table(pst_cfg) \
> + container_of((pst_cfg), struct iommu_pasid_table, cfg)
> +
> +struct iommu_vendor_psdtable_ops {
> + int (*alloc)(struct iommu_vendor_psdtable_cfg *cfg);
> + void (*free)(struct iommu_vendor_psdtable_cfg *cfg);
> + void (*prepare)(struct iommu_vendor_psdtable_cfg *cfg,
> + struct io_pgtable_cfg *pgtbl_cfg, u32 asid);
> + int (*write)(struct iommu_vendor_psdtable_cfg *cfg, int ssid,
> + void *cookie);
> + void (*sync)(void *cookie, int ssid, bool leaf);
> +};
> +
> +static inline int iommu_psdtable_alloc(struct iommu_pasid_table *tbl,
> + struct iommu_vendor_psdtable_cfg *cfg)
> +{
> + if (!tbl->ops->alloc)
> + return -ENOSYS;
> +
> + return tbl->ops->alloc(cfg);
> +}
> +
> +static inline void iommu_psdtable_free(struct iommu_pasid_table *tbl,
> + struct iommu_vendor_psdtable_cfg *cfg)
> +{
> + if (!tbl->ops->free)
> + return;
> +
> + tbl->ops->free(cfg);
> +}
> +
> +static inline int iommu_psdtable_prepare(struct iommu_pasid_table *tbl,
> + struct iommu_vendor_psdtable_cfg *cfg,
> + struct io_pgtable_cfg *pgtbl_cfg,
> + u32 asid)
> +{
> + if (!tbl->ops->prepare)
> + return -ENOSYS;
> +
> + tbl->ops->prepare(cfg, pgtbl_cfg, asid);
> + return 0;
> +}
> +
> +static inline int iommu_psdtable_write(struct iommu_pasid_table *tbl,
> + struct iommu_vendor_psdtable_cfg *cfg,
> + int ssid, void *cookie)
> +{
> + if (!tbl->ops->write)
> + return -ENOSYS;
> +
> + return tbl->ops->write(cfg, ssid, cookie);
> +}
> +
> +static inline int iommu_psdtable_sync(struct iommu_pasid_table *tbl,
> + void *cookie, int ssid, bool leaf)
> +{
> + if (!tbl->ops->sync)
> + return -ENOSYS;
> +
> + tbl->ops->sync(cookie, ssid, leaf);
> + return 0;
> +}
> +
> +/* A placeholder to register vendor specific pasid layer */
> +static inline struct iommu_pasid_table *
> +iommu_register_pasid_table(enum pasid_table_fmt fmt,
> + struct device *dev, void *cookie)
> +{
> + return NULL;
> +}
> +
> +#endif /* __IOMMU_PASID_TABLE_H */
> --
> 2.17.1
>
On Fri, Jan 15, 2021 at 05:43:36PM +0530, Vivek Gautam wrote:
> Add info about asid_bits and additional flags to table format
> probing header.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Michael S. Tsirkin <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Kevin Tian <[email protected]>
> Cc: Jacob Pan <[email protected]>
> Cc: Liu Yi L <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Shameerali Kolothum Thodi <[email protected]>
> ---
> include/uapi/linux/virtio_iommu.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
> index 43821e33e7af..8a0624bab4b2 100644
> --- a/include/uapi/linux/virtio_iommu.h
> +++ b/include/uapi/linux/virtio_iommu.h
> @@ -169,7 +169,10 @@ struct virtio_iommu_probe_pasid_size {
> struct virtio_iommu_probe_table_format {
> struct virtio_iommu_probe_property head;
> __le16 format;
> - __u8 reserved[2];
> + __le16 asid_bits;
> +
> + __le32 flags;
This struct should only contain the head and format fields. asid and flags
should go in a specialized structure - virtio_iommu_probe_pgt_arm64 in the
latest spec draft, where I dropped the asid_bits field in favor of an
"ASID16" flag.
Thanks,
Jean
On Fri, Jan 15, 2021 at 05:43:35PM +0530, Vivek Gautam wrote:
> aisd_bits data is required to prepare stage-1 tables for arm-smmu-v3.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Kevin Tian <[email protected]>
> Cc: Jacob Pan <[email protected]>
> Cc: Liu Yi L <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Shameerali Kolothum Thodi <[email protected]>
> ---
> include/uapi/linux/iommu.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> index 082d758dd016..96abbfc7c643 100644
> --- a/include/uapi/linux/iommu.h
> +++ b/include/uapi/linux/iommu.h
> @@ -357,7 +357,7 @@ struct iommu_pasid_smmuv3 {
> __u32 version;
> __u8 s1fmt;
> __u8 s1dss;
> - __u8 padding[2];
> + __u16 asid_bits;
Is this used anywhere? This struct is passed from host userspace to host
kernel to attach the PASID table, so I don't think it needs an asid_bits
field.
Thanks,
Jean
Hi Vivek,
On Fri, 15 Jan 2021 17:43:39 +0530, Vivek Gautam <[email protected]>
wrote:
> From: Jean-Philippe Brucker <[email protected]>
>
> Add support for tlb invalidation ops that can send invalidation
> requests to back-end virtio-iommu when stage-1 page tables are
> supported.
>
Just curious if it possible to reuse the iommu uapi for invalidation and others.
When we started out designing the iommu uapi, the intention was to support
both emulated and virtio iommu.
> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> [Vivek: Refactoring the iommu_flush_ops, and adding only one pasid sync
> op that's needed with current iommu-pasid-table infrastructure.
> Also updating uapi defines as required by latest changes]
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Michael S. Tsirkin <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Kevin Tian <[email protected]>
> Cc: Jacob Pan <[email protected]>
> Cc: Liu Yi L <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Shameerali Kolothum Thodi <[email protected]>
> ---
> drivers/iommu/virtio-iommu.c | 95 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 95 insertions(+)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index ae5dfd3f8269..004ea94e3731 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -13,6 +13,7 @@
> #include <linux/freezer.h>
> #include <linux/interval_tree.h>
> #include <linux/iommu.h>
> +#include <linux/io-pgtable.h>
> #include <linux/module.h>
> #include <linux/of_iommu.h>
> #include <linux/of_platform.h>
> @@ -63,6 +64,8 @@ struct viommu_mapping {
> };
>
> struct viommu_mm {
> + int pasid;
> + u64 archid;
> struct io_pgtable_ops *ops;
> struct viommu_domain *domain;
> };
> @@ -692,6 +695,98 @@ static void viommu_event_handler(struct virtqueue
> *vq) virtqueue_kick(vq);
> }
>
> +/* PASID and pgtable APIs */
> +
> +static void __viommu_flush_pasid_tlb_all(struct viommu_domain *vdomain,
> + int pasid, u64 arch_id, int
> type) +{
> + struct virtio_iommu_req_invalidate req = {
> + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
> + .inv_gran =
> cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
> + .flags =
> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID),
> + .inv_type = cpu_to_le32(type),
> +
> + .domain = cpu_to_le32(vdomain->id),
> + .pasid = cpu_to_le32(pasid),
> + .archid = cpu_to_le64(arch_id),
> + };
> +
> + if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
> + pr_debug("could not send invalidate request\n");
> +}
> +
> +static void viommu_flush_tlb_add(struct iommu_iotlb_gather *gather,
> + unsigned long iova, size_t granule,
> + void *cookie)
> +{
> + struct viommu_mm *viommu_mm = cookie;
> + struct viommu_domain *vdomain = viommu_mm->domain;
> + struct iommu_domain *domain = &vdomain->domain;
> +
> + iommu_iotlb_gather_add_page(domain, gather, iova, granule);
> +}
> +
> +static void viommu_flush_tlb_walk(unsigned long iova, size_t size,
> + size_t granule, void *cookie)
> +{
> + struct viommu_mm *viommu_mm = cookie;
> + struct viommu_domain *vdomain = viommu_mm->domain;
> + struct virtio_iommu_req_invalidate req = {
> + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
> + .inv_gran = cpu_to_le32(VIRTIO_IOMMU_INVAL_G_VA),
> + .inv_type = cpu_to_le32(VIRTIO_IOMMU_INV_T_IOTLB),
> + .flags =
> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_ARCHID), +
> + .domain = cpu_to_le32(vdomain->id),
> + .pasid = cpu_to_le32(viommu_mm->pasid),
> + .archid = cpu_to_le64(viommu_mm->archid),
> + .virt_start = cpu_to_le64(iova),
> + .nr_pages = cpu_to_le64(size / granule),
> + .granule = ilog2(granule),
> + };
> +
> + if (viommu_add_req(vdomain->viommu, &req, sizeof(req)))
> + pr_debug("could not add invalidate request\n");
> +}
> +
> +static void viommu_flush_tlb_all(void *cookie)
> +{
> + struct viommu_mm *viommu_mm = cookie;
> +
> + if (!viommu_mm->archid)
> + return;
> +
> + __viommu_flush_pasid_tlb_all(viommu_mm->domain, viommu_mm->pasid,
> + viommu_mm->archid,
> + VIRTIO_IOMMU_INV_T_IOTLB);
> +}
> +
> +static struct iommu_flush_ops viommu_flush_ops = {
> + .tlb_flush_all = viommu_flush_tlb_all,
> + .tlb_flush_walk = viommu_flush_tlb_walk,
> + .tlb_add_page = viommu_flush_tlb_add,
> +};
> +
> +static void viommu_flush_pasid(void *cookie, int pasid, bool leaf)
> +{
> + struct viommu_domain *vdomain = cookie;
> + struct virtio_iommu_req_invalidate req = {
> + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
> + .inv_gran =
> cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
> + .inv_type = cpu_to_le32(VIRTIO_IOMMU_INV_T_PASID),
> + .flags =
> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID), +
> + .domain = cpu_to_le32(vdomain->id),
> + .pasid = cpu_to_le32(pasid),
> + };
> +
> + if (leaf)
> + req.flags |=
> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_LEAF); +
> + if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
> + pr_debug("could not send invalidate request\n");
> +}
> +
> /* IOMMU API */
>
> static struct iommu_domain *viommu_domain_alloc(unsigned type)
Thanks,
Jacob
> From: Jacob Pan <[email protected]>
> Sent: Thursday, March 4, 2021 2:29 AM
>
> Hi Vivek,
>
> On Fri, 15 Jan 2021 17:43:39 +0530, Vivek Gautam <[email protected]>
> wrote:
>
> > From: Jean-Philippe Brucker <[email protected]>
> >
> > Add support for tlb invalidation ops that can send invalidation
> > requests to back-end virtio-iommu when stage-1 page tables are
> > supported.
> >
> Just curious if it possible to reuse the iommu uapi for invalidation and others.
> When we started out designing the iommu uapi, the intention was to support
> both emulated and virtio iommu.
IIUC this patch is about the protocol between virtio-iommu frontend and backend.
After the virtio-iommu backend receives invalidation ops, it then needs to
forward the request to the host IOMMU driver through the existing iommu
uapi that you referred to, as a emulated VT-d or SMMU would do.
Thanks
Kevin
>
> > Signed-off-by: Jean-Philippe Brucker <[email protected]>
> > [Vivek: Refactoring the iommu_flush_ops, and adding only one pasid sync
> > op that's needed with current iommu-pasid-table infrastructure.
> > Also updating uapi defines as required by latest changes]
> > Signed-off-by: Vivek Gautam <[email protected]>
> > Cc: Joerg Roedel <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Michael S. Tsirkin <[email protected]>
> > Cc: Robin Murphy <[email protected]>
> > Cc: Jean-Philippe Brucker <[email protected]>
> > Cc: Eric Auger <[email protected]>
> > Cc: Alex Williamson <[email protected]>
> > Cc: Kevin Tian <[email protected]>
> > Cc: Jacob Pan <[email protected]>
> > Cc: Liu Yi L <[email protected]>
> > Cc: Lorenzo Pieralisi <[email protected]>
> > Cc: Shameerali Kolothum Thodi <[email protected]>
> > ---
> > drivers/iommu/virtio-iommu.c | 95
> ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 95 insertions(+)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> > index ae5dfd3f8269..004ea94e3731 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -13,6 +13,7 @@
> > #include <linux/freezer.h>
> > #include <linux/interval_tree.h>
> > #include <linux/iommu.h>
> > +#include <linux/io-pgtable.h>
> > #include <linux/module.h>
> > #include <linux/of_iommu.h>
> > #include <linux/of_platform.h>
> > @@ -63,6 +64,8 @@ struct viommu_mapping {
> > };
> >
> > struct viommu_mm {
> > + int pasid;
> > + u64 archid;
> > struct io_pgtable_ops *ops;
> > struct viommu_domain *domain;
> > };
> > @@ -692,6 +695,98 @@ static void viommu_event_handler(struct
> virtqueue
> > *vq) virtqueue_kick(vq);
> > }
> >
> > +/* PASID and pgtable APIs */
> > +
> > +static void __viommu_flush_pasid_tlb_all(struct viommu_domain
> *vdomain,
> > + int pasid, u64 arch_id, int
> > type) +{
> > + struct virtio_iommu_req_invalidate req = {
> > + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
> > + .inv_gran =
> > cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
> > + .flags =
> > cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID),
> > + .inv_type = cpu_to_le32(type),
> > +
> > + .domain = cpu_to_le32(vdomain->id),
> > + .pasid = cpu_to_le32(pasid),
> > + .archid = cpu_to_le64(arch_id),
> > + };
> > +
> > + if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
> > + pr_debug("could not send invalidate request\n");
> > +}
> > +
> > +static void viommu_flush_tlb_add(struct iommu_iotlb_gather *gather,
> > + unsigned long iova, size_t granule,
> > + void *cookie)
> > +{
> > + struct viommu_mm *viommu_mm = cookie;
> > + struct viommu_domain *vdomain = viommu_mm->domain;
> > + struct iommu_domain *domain = &vdomain->domain;
> > +
> > + iommu_iotlb_gather_add_page(domain, gather, iova, granule);
> > +}
> > +
> > +static void viommu_flush_tlb_walk(unsigned long iova, size_t size,
> > + size_t granule, void *cookie)
> > +{
> > + struct viommu_mm *viommu_mm = cookie;
> > + struct viommu_domain *vdomain = viommu_mm->domain;
> > + struct virtio_iommu_req_invalidate req = {
> > + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
> > + .inv_gran = cpu_to_le32(VIRTIO_IOMMU_INVAL_G_VA),
> > + .inv_type =
> cpu_to_le32(VIRTIO_IOMMU_INV_T_IOTLB),
> > + .flags =
> > cpu_to_le32(VIRTIO_IOMMU_INVAL_F_ARCHID), +
> > + .domain = cpu_to_le32(vdomain->id),
> > + .pasid = cpu_to_le32(viommu_mm->pasid),
> > + .archid = cpu_to_le64(viommu_mm->archid),
> > + .virt_start = cpu_to_le64(iova),
> > + .nr_pages = cpu_to_le64(size / granule),
> > + .granule = ilog2(granule),
> > + };
> > +
> > + if (viommu_add_req(vdomain->viommu, &req, sizeof(req)))
> > + pr_debug("could not add invalidate request\n");
> > +}
> > +
> > +static void viommu_flush_tlb_all(void *cookie)
> > +{
> > + struct viommu_mm *viommu_mm = cookie;
> > +
> > + if (!viommu_mm->archid)
> > + return;
> > +
> > + __viommu_flush_pasid_tlb_all(viommu_mm->domain,
> viommu_mm->pasid,
> > + viommu_mm->archid,
> > + VIRTIO_IOMMU_INV_T_IOTLB);
> > +}
> > +
> > +static struct iommu_flush_ops viommu_flush_ops = {
> > + .tlb_flush_all = viommu_flush_tlb_all,
> > + .tlb_flush_walk = viommu_flush_tlb_walk,
> > + .tlb_add_page = viommu_flush_tlb_add,
> > +};
> > +
> > +static void viommu_flush_pasid(void *cookie, int pasid, bool leaf)
> > +{
> > + struct viommu_domain *vdomain = cookie;
> > + struct virtio_iommu_req_invalidate req = {
> > + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
> > + .inv_gran =
> > cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
> > + .inv_type =
> cpu_to_le32(VIRTIO_IOMMU_INV_T_PASID),
> > + .flags =
> > cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID), +
> > + .domain = cpu_to_le32(vdomain->id),
> > + .pasid = cpu_to_le32(pasid),
> > + };
> > +
> > + if (leaf)
> > + req.flags |=
> > cpu_to_le32(VIRTIO_IOMMU_INVAL_F_LEAF); +
> > + if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
> > + pr_debug("could not send invalidate request\n");
> > +}
> > +
> > /* IOMMU API */
> >
> > static struct iommu_domain *viommu_domain_alloc(unsigned type)
>
>
> Thanks,
>
> Jacob
Hi Jacob, Kevin,
On 3/4/21 11:28 AM, Tian, Kevin wrote:
>> From: Jacob Pan <[email protected]>
>> Sent: Thursday, March 4, 2021 2:29 AM
>>
>> Hi Vivek,
>>
>> On Fri, 15 Jan 2021 17:43:39 +0530, Vivek Gautam <[email protected]>
>> wrote:
>>
>>> From: Jean-Philippe Brucker <[email protected]>
>>>
>>> Add support for tlb invalidation ops that can send invalidation
>>> requests to back-end virtio-iommu when stage-1 page tables are
>>> supported.
>>>
>> Just curious if it possible to reuse the iommu uapi for invalidation and others.
>> When we started out designing the iommu uapi, the intention was to support
>> both emulated and virtio iommu.
>
> IIUC this patch is about the protocol between virtio-iommu frontend and backend.
> After the virtio-iommu backend receives invalidation ops, it then needs to
> forward the request to the host IOMMU driver through the existing iommu
> uapi that you referred to, as a emulated VT-d or SMMU would do.
Thanks a lot for looking at the patch.
Yes this patch is to provide the front-end virtio interface for
invalidation requests during map/unmap and when flushing the pasid
tables when virtio-iommu requested pasid table (in other words cd tables
for arm-smmu-v3) from the iommu-pasid-table library.
The kvmtool back-end virtio driver forwards these requests to vfio
driver which then makes use of iommu uapi to finally request host iommu
driver for handling these invalidations.
Regards
Vivek
>
> Thanks
> Kevin
>
>>
>>> Signed-off-by: Jean-Philippe Brucker <[email protected]>
>>> [Vivek: Refactoring the iommu_flush_ops, and adding only one pasid sync
>>> op that's needed with current iommu-pasid-table infrastructure.
>>> Also updating uapi defines as required by latest changes]
>>> Signed-off-by: Vivek Gautam <[email protected]>
>>> Cc: Joerg Roedel <[email protected]>
>>> Cc: Will Deacon <[email protected]>
>>> Cc: Michael S. Tsirkin <[email protected]>
>>> Cc: Robin Murphy <[email protected]>
>>> Cc: Jean-Philippe Brucker <[email protected]>
>>> Cc: Eric Auger <[email protected]>
>>> Cc: Alex Williamson <[email protected]>
>>> Cc: Kevin Tian <[email protected]>
>>> Cc: Jacob Pan <[email protected]>
>>> Cc: Liu Yi L <[email protected]>
>>> Cc: Lorenzo Pieralisi <[email protected]>
>>> Cc: Shameerali Kolothum Thodi <[email protected]>
>>> ---
>>> drivers/iommu/virtio-iommu.c | 95
>> ++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 95 insertions(+)
>>>
>>> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
>>> index ae5dfd3f8269..004ea94e3731 100644
>>> --- a/drivers/iommu/virtio-iommu.c
>>> +++ b/drivers/iommu/virtio-iommu.c
>>> @@ -13,6 +13,7 @@
>>> #include <linux/freezer.h>
>>> #include <linux/interval_tree.h>
>>> #include <linux/iommu.h>
>>> +#include <linux/io-pgtable.h>
>>> #include <linux/module.h>
>>> #include <linux/of_iommu.h>
>>> #include <linux/of_platform.h>
>>> @@ -63,6 +64,8 @@ struct viommu_mapping {
>>> };
>>>
>>> struct viommu_mm {
>>> + int pasid;
>>> + u64 archid;
>>> struct io_pgtable_ops *ops;
>>> struct viommu_domain *domain;
>>> };
>>> @@ -692,6 +695,98 @@ static void viommu_event_handler(struct
>> virtqueue
>>> *vq) virtqueue_kick(vq);
>>> }
>>>
>>> +/* PASID and pgtable APIs */
>>> +
>>> +static void __viommu_flush_pasid_tlb_all(struct viommu_domain
>> *vdomain,
>>> + int pasid, u64 arch_id, int
>>> type) +{
>>> + struct virtio_iommu_req_invalidate req = {
>>> + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
>>> + .inv_gran =
>>> cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
>>> + .flags =
>>> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID),
>>> + .inv_type = cpu_to_le32(type),
>>> +
>>> + .domain = cpu_to_le32(vdomain->id),
>>> + .pasid = cpu_to_le32(pasid),
>>> + .archid = cpu_to_le64(arch_id),
>>> + };
>>> +
>>> + if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
>>> + pr_debug("could not send invalidate request\n");
>>> +}
>>> +
>>> +static void viommu_flush_tlb_add(struct iommu_iotlb_gather *gather,
>>> + unsigned long iova, size_t granule,
>>> + void *cookie)
>>> +{
>>> + struct viommu_mm *viommu_mm = cookie;
>>> + struct viommu_domain *vdomain = viommu_mm->domain;
>>> + struct iommu_domain *domain = &vdomain->domain;
>>> +
>>> + iommu_iotlb_gather_add_page(domain, gather, iova, granule);
>>> +}
>>> +
>>> +static void viommu_flush_tlb_walk(unsigned long iova, size_t size,
>>> + size_t granule, void *cookie)
>>> +{
>>> + struct viommu_mm *viommu_mm = cookie;
>>> + struct viommu_domain *vdomain = viommu_mm->domain;
>>> + struct virtio_iommu_req_invalidate req = {
>>> + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
>>> + .inv_gran = cpu_to_le32(VIRTIO_IOMMU_INVAL_G_VA),
>>> + .inv_type =
>> cpu_to_le32(VIRTIO_IOMMU_INV_T_IOTLB),
>>> + .flags =
>>> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_ARCHID), +
>>> + .domain = cpu_to_le32(vdomain->id),
>>> + .pasid = cpu_to_le32(viommu_mm->pasid),
>>> + .archid = cpu_to_le64(viommu_mm->archid),
>>> + .virt_start = cpu_to_le64(iova),
>>> + .nr_pages = cpu_to_le64(size / granule),
>>> + .granule = ilog2(granule),
>>> + };
>>> +
>>> + if (viommu_add_req(vdomain->viommu, &req, sizeof(req)))
>>> + pr_debug("could not add invalidate request\n");
>>> +}
>>> +
>>> +static void viommu_flush_tlb_all(void *cookie)
>>> +{
>>> + struct viommu_mm *viommu_mm = cookie;
>>> +
>>> + if (!viommu_mm->archid)
>>> + return;
>>> +
>>> + __viommu_flush_pasid_tlb_all(viommu_mm->domain,
>> viommu_mm->pasid,
>>> + viommu_mm->archid,
>>> + VIRTIO_IOMMU_INV_T_IOTLB);
>>> +}
>>> +
>>> +static struct iommu_flush_ops viommu_flush_ops = {
>>> + .tlb_flush_all = viommu_flush_tlb_all,
>>> + .tlb_flush_walk = viommu_flush_tlb_walk,
>>> + .tlb_add_page = viommu_flush_tlb_add,
>>> +};
>>> +
>>> +static void viommu_flush_pasid(void *cookie, int pasid, bool leaf)
>>> +{
>>> + struct viommu_domain *vdomain = cookie;
>>> + struct virtio_iommu_req_invalidate req = {
>>> + .head.type = VIRTIO_IOMMU_T_INVALIDATE,
>>> + .inv_gran =
>>> cpu_to_le32(VIRTIO_IOMMU_INVAL_G_PASID),
>>> + .inv_type =
>> cpu_to_le32(VIRTIO_IOMMU_INV_T_PASID),
>>> + .flags =
>>> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_PASID), +
>>> + .domain = cpu_to_le32(vdomain->id),
>>> + .pasid = cpu_to_le32(pasid),
>>> + };
>>> +
>>> + if (leaf)
>>> + req.flags |=
>>> cpu_to_le32(VIRTIO_IOMMU_INVAL_F_LEAF); +
>>> + if (viommu_send_req_sync(vdomain->viommu, &req, sizeof(req)))
>>> + pr_debug("could not send invalidate request\n");
>>> +}
>>> +
>>> /* IOMMU API */
>>>
>>> static struct iommu_domain *viommu_domain_alloc(unsigned type)
>>
>>
>> Thanks,
>>
>> Jacob
On Fri, Jan 15, 2021 at 05:43:33PM +0530, Vivek Gautam wrote:
> From: Jean-Philippe Brucker <[email protected]>
>
> Add required UAPI defines for probing table format for underlying
> iommu hardware. The device may provide information about hardware
> tables and additional capabilities for each device.
> This allows guest to correctly fabricate stage-1 page tables.
>
> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> [Vivek: Use a single "struct virtio_iommu_probe_table_format" rather
> than separate structures for page table and pasid table format.
Makes sense. I've integrated that into the spec draft, added more precise
documentation and modified some of the definitions.
The current draft is here:
https://jpbrucker.net/virtio-iommu/spec/virtio-iommu-v0.13.pdf
Posted on the list here
https://lists.oasis-open.org/archives/virtio-dev/202102/msg00012.html
> Also update commit message.]
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Michael S. Tsirkin <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Kevin Tian <[email protected]>
> Cc: Jacob Pan <[email protected]>
> Cc: Liu Yi L <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Shameerali Kolothum Thodi <[email protected]>
> ---
> include/uapi/linux/virtio_iommu.h | 44 ++++++++++++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
> index 237e36a280cb..43821e33e7af 100644
> --- a/include/uapi/linux/virtio_iommu.h
> +++ b/include/uapi/linux/virtio_iommu.h
> @@ -2,7 +2,7 @@
> /*
> * Virtio-iommu definition v0.12
> *
> - * Copyright (C) 2019 Arm Ltd.
> + * Copyright (C) 2019-2021 Arm Ltd.
Not strictly necessary. But if you're modifying this comment please also
remove the "v0.12" above
> */
> #ifndef _UAPI_LINUX_VIRTIO_IOMMU_H
> #define _UAPI_LINUX_VIRTIO_IOMMU_H
> @@ -111,6 +111,12 @@ struct virtio_iommu_req_unmap {
>
> #define VIRTIO_IOMMU_PROBE_T_NONE 0
> #define VIRTIO_IOMMU_PROBE_T_RESV_MEM 1
> +#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK 2
> +#define VIRTIO_IOMMU_PROBE_T_INPUT_RANGE 3
> +#define VIRTIO_IOMMU_PROBE_T_OUTPUT_SIZE 4
> +#define VIRTIO_IOMMU_PROBE_T_PASID_SIZE 5
> +#define VIRTIO_IOMMU_PROBE_T_PAGE_TABLE_FMT 6
> +#define VIRTIO_IOMMU_PROBE_T_PASID_TABLE_FMT 7
Since there is a single struct we can have a single
VIRTIO_IOMMU_PROBE_T_TABLE_FORMAT.
>
> #define VIRTIO_IOMMU_PROBE_T_MASK 0xfff
>
> @@ -130,6 +136,42 @@ struct virtio_iommu_probe_resv_mem {
> __le64 end;
> };
>
> +struct virtio_iommu_probe_page_size_mask {
> + struct virtio_iommu_probe_property head;
> + __u8 reserved[4];
> + __le64 mask;
> +};
> +
> +struct virtio_iommu_probe_input_range {
> + struct virtio_iommu_probe_property head;
> + __u8 reserved[4];
> + __le64 start;
> + __le64 end;
> +};
> +
> +struct virtio_iommu_probe_output_size {
> + struct virtio_iommu_probe_property head;
> + __u8 bits;
> + __u8 reserved[3];
> +};
> +
> +struct virtio_iommu_probe_pasid_size {
> + struct virtio_iommu_probe_property head;
> + __u8 bits;
> + __u8 reserved[3];
> +};
> +
> +/* Arm LPAE page table format */
> +#define VIRTIO_IOMMU_FOMRAT_PGTF_ARM_LPAE 1
s/FOMRAT/FORMAT
> +/* Arm smmu-v3 type PASID table format */
> +#define VIRTIO_IOMMU_FORMAT_PSTF_ARM_SMMU_V3 2
These should be with the Arm-specific definitions patches 11 and 14
Thanks,
Jean
> +
> +struct virtio_iommu_probe_table_format {
> + struct virtio_iommu_probe_property head;
> + __le16 format;
> + __u8 reserved[2];
> +};
> +
> struct virtio_iommu_req_probe {
> struct virtio_iommu_req_head head;
> __le32 endpoint;
> --
> 2.17.1
>
Hi Jean,
On 3/3/21 10:44 PM, Jean-Philippe Brucker wrote:
> On Fri, Jan 15, 2021 at 05:43:31PM +0530, Vivek Gautam wrote:
>> Update base address information in vendor pasid table info to pass that
>> to user-space for stage1 table management.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Jean-Philippe Brucker <[email protected]>
>> Cc: Eric Auger <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> Cc: Kevin Tian <[email protected]>
>> Cc: Jacob Pan <[email protected]>
>> Cc: Liu Yi L <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Shameerali Kolothum Thodi <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>> index 8a7187534706..ec37476c8d09 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>> @@ -55,6 +55,9 @@ static __le64 *arm_smmu_get_cd_ptr(struct iommu_vendor_psdtable_cfg *pst_cfg,
>> if (arm_smmu_alloc_cd_leaf_table(dev, l1_desc))
>> return NULL;
>>
>> + if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
>> + pst_cfg->base = l1_desc->l2ptr_dma;
>> +
>
> This isn't the right place, because this path allocates second-level
> tables for two-level tables. I don't think we need pst_cfg->base at all,
> because for both linear and two-level tables, the base pointer is in
> cdcfg->cdtab_dma, which can be read directly.
Sure, will remove this.
>
> Thanks,
> Jean
>
>> l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
>> arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
>> /* An invalid L1CD can be cached */
>> @@ -211,6 +214,9 @@ static int arm_smmu_alloc_cd_tables(struct iommu_vendor_psdtable_cfg *pst_cfg)
>> goto err_free_l1;
>> }
>>
>> + if (s1cfg->s1fmt == STRTAB_STE_0_S1FMT_64K_L2)
>> + pst_cfg->base = cdcfg->cdtab_dma;
>> +
>> return 0;
>>
>> err_free_l1:
>> --
>> 2.17.1
>>
Hi Jean,
On 3/3/21 10:41 PM, Jean-Philippe Brucker wrote:
> Hi Vivek,
>
> Thanks again for working on this. I have a few comments but it looks
> sensible overall.
Thanks a lot for reviewing the patch-series. Please find my responses
inline below.
>
> Regarding the overall design, I was initially assigning page directories
> instead of whole PASID tables, which would simplify the driver and host
> implementation. A major complication, however, is SMMUv3 accesses PASID
> tables using a guest-physical address, so there is a messy negotiation
> needed between host and guest when the host needs to allocate PASID
> tables. Plus vSMMU needs PASID table assignment, so that's what the host
> driver will implement.
By assigning the page directories, you mean setting up just the stage-1
page table ops, and passing that information to the host using ATTACH_TABLE?
Right now when using kvmtool, the struct iommu_pasid_table_config is
populated with the correct information, and this whole memory is mapped
between host and guest by creating a mem bank using
kvm__for_each_mem_bank().
Did I get you or did I fail terribly in understanding the point you are
making here?
If it helps, I will publish my kvmtool branch.
>
> On Fri, Jan 15, 2021 at 05:43:29PM +0530, Vivek Gautam wrote:
>> Add a small API in iommu subsystem to handle PASID table allocation
>> requests from different consumer drivers, such as a paravirtualized
>> iommu driver. The API provides ops for allocating and freeing PASID
>> table, writing to it and managing the table caches.
>>
>> This library also provides for registering a vendor API that attaches
>> to these ops. The vendor APIs would eventually perform arch level
>> implementations for these PASID tables.
>
> Although Arm might be the only vendor to ever use this, I think the
> abstraction makes sense and isn't too invasive. Even if we called directly
> into the SMMU driver from the virtio one, we'd still need patch 3 and
> separate TLB invalidations ops.
Right, the idea was to make users of iommu-pasid-table - virtio-iommu or
the arm-smmu-v3 - consistent. I also noticed that the whole process of
allocating the pasid tables (or cd tables) and populating them with
stage-1 page tables in viommu is also in-line with how things are in
arm-smmu-v3 or atleast that's how the design can be in general -
allocate pasid_table, and program stage-1 information into it, and then
pass it across to host.
>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Jean-Philippe Brucker <[email protected]>
>> Cc: Eric Auger <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> Cc: Kevin Tian <[email protected]>
>> Cc: Jacob Pan <[email protected]>
>> Cc: Liu Yi L <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Shameerali Kolothum Thodi <[email protected]>
>> ---
>> drivers/iommu/iommu-pasid-table.h | 134 ++++++++++++++++++++++++++++++
>> 1 file changed, 134 insertions(+)
>> create mode 100644 drivers/iommu/iommu-pasid-table.h
>>
>> diff --git a/drivers/iommu/iommu-pasid-table.h b/drivers/iommu/iommu-pasid-table.h
>> new file mode 100644
>> index 000000000000..bd4f57656f67
>> --- /dev/null
>> +++ b/drivers/iommu/iommu-pasid-table.h
>> @@ -0,0 +1,134 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PASID table management for the IOMMU
>> + *
>> + * Copyright (C) 2021 Arm Ltd.
>> + */
>> +#ifndef __IOMMU_PASID_TABLE_H
>> +#define __IOMMU_PASID_TABLE_H
>> +
>> +#include <linux/io-pgtable.h>
>> +
>> +#include "arm/arm-smmu-v3/arm-smmu-v3.h"
>> +
>> +enum pasid_table_fmt {
>> + PASID_TABLE_ARM_SMMU_V3,
>> + PASID_TABLE_NUM_FMTS,
>> +};
>> +
>> +/**
>> + * struct arm_smmu_cfg_info - arm-smmu-v3 specific configuration data
>> + *
>> + * @s1_cfg: arm-smmu-v3 stage1 config data
>> + * @feat_flag: features supported by arm-smmu-v3 implementation
>> + */
>> +struct arm_smmu_cfg_info {
>> + struct arm_smmu_s1_cfg *s1_cfg;
>> + u32 feat_flag;
>> +};
>> +
>> +/**
>> + * struct iommu_vendor_psdtable_cfg - Configuration data for PASID tables
>> + *
>> + * @iommu_dev: device performing the DMA table walks
>> + * @fmt: The PASID table format
>> + * @base: DMA address of the allocated table, set by the vendor driver
>> + * @cfg: arm-smmu-v3 specific config data
>> + */
>> +struct iommu_vendor_psdtable_cfg {
>> + struct device *iommu_dev;
>> + enum pasid_table_fmt fmt;
>> + dma_addr_t base;
>> + union {
>> + struct arm_smmu_cfg_info cfg;
>
> For the union to be extensible, that field should be called "arm" or
> something like that.
Sure. Will amend this.
Thanks,
Vivek
>
> Thanks,
> Jean
>
>> + } vendor;
>> +};
>> +
>> +struct iommu_vendor_psdtable_ops;
>> +
>> +/**
>> + * struct iommu_pasid_table - describes a set of PASID tables
>> + *
>> + * @cookie: An opaque token provided by the IOMMU driver and passed back to any
>> + * callback routine.
>> + * @cfg: A copy of the PASID table configuration
>> + * @ops: The PASID table operations in use for this set of page tables
>> + */
>> +struct iommu_pasid_table {
>> + void *cookie;
>> + struct iommu_vendor_psdtable_cfg cfg;
>> + struct iommu_vendor_psdtable_ops *ops;
>> +};
>> +
>> +#define pasid_table_cfg_to_table(pst_cfg) \
>> + container_of((pst_cfg), struct iommu_pasid_table, cfg)
>> +
>> +struct iommu_vendor_psdtable_ops {
>> + int (*alloc)(struct iommu_vendor_psdtable_cfg *cfg);
>> + void (*free)(struct iommu_vendor_psdtable_cfg *cfg);
>> + void (*prepare)(struct iommu_vendor_psdtable_cfg *cfg,
>> + struct io_pgtable_cfg *pgtbl_cfg, u32 asid);
>> + int (*write)(struct iommu_vendor_psdtable_cfg *cfg, int ssid,
>> + void *cookie);
>> + void (*sync)(void *cookie, int ssid, bool leaf);
>> +};
>> +
>> +static inline int iommu_psdtable_alloc(struct iommu_pasid_table *tbl,
>> + struct iommu_vendor_psdtable_cfg *cfg)
>> +{
>> + if (!tbl->ops->alloc)
>> + return -ENOSYS;
>> +
>> + return tbl->ops->alloc(cfg);
>> +}
>> +
>> +static inline void iommu_psdtable_free(struct iommu_pasid_table *tbl,
>> + struct iommu_vendor_psdtable_cfg *cfg)
>> +{
>> + if (!tbl->ops->free)
>> + return;
>> +
>> + tbl->ops->free(cfg);
>> +}
>> +
>> +static inline int iommu_psdtable_prepare(struct iommu_pasid_table *tbl,
>> + struct iommu_vendor_psdtable_cfg *cfg,
>> + struct io_pgtable_cfg *pgtbl_cfg,
>> + u32 asid)
>> +{
>> + if (!tbl->ops->prepare)
>> + return -ENOSYS;
>> +
>> + tbl->ops->prepare(cfg, pgtbl_cfg, asid);
>> + return 0;
>> +}
>> +
>> +static inline int iommu_psdtable_write(struct iommu_pasid_table *tbl,
>> + struct iommu_vendor_psdtable_cfg *cfg,
>> + int ssid, void *cookie)
>> +{
>> + if (!tbl->ops->write)
>> + return -ENOSYS;
>> +
>> + return tbl->ops->write(cfg, ssid, cookie);
>> +}
>> +
>> +static inline int iommu_psdtable_sync(struct iommu_pasid_table *tbl,
>> + void *cookie, int ssid, bool leaf)
>> +{
>> + if (!tbl->ops->sync)
>> + return -ENOSYS;
>> +
>> + tbl->ops->sync(cookie, ssid, leaf);
>> + return 0;
>> +}
>> +
>> +/* A placeholder to register vendor specific pasid layer */
>> +static inline struct iommu_pasid_table *
>> +iommu_register_pasid_table(enum pasid_table_fmt fmt,
>> + struct device *dev, void *cookie)
>> +{
>> + return NULL;
>> +}
>> +
>> +#endif /* __IOMMU_PASID_TABLE_H */
>> --
>> 2.17.1
>>
On 3/3/21 10:45 PM, Jean-Philippe Brucker wrote:
> On Fri, Jan 15, 2021 at 05:43:32PM +0530, Vivek Gautam wrote:
>> Te change allows different consumers of arm-smmu-v3-cd-lib to set
>> their respective sync op for pasid entries.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Jean-Philippe Brucker <[email protected]>
>> Cc: Eric Auger <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> Cc: Kevin Tian <[email protected]>
>> Cc: Jacob Pan <[email protected]>
>> Cc: Liu Yi L <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Shameerali Kolothum Thodi <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c | 1 -
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 +++++++
>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>> index ec37476c8d09..acaa09acecdd 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-cd-lib.c
>> @@ -265,7 +265,6 @@ struct iommu_vendor_psdtable_ops arm_cd_table_ops = {
>> .free = arm_smmu_free_cd_tables,
>> .prepare = arm_smmu_prepare_cd,
>> .write = arm_smmu_write_ctx_desc,
>> - .sync = arm_smmu_sync_cd,
>> };
>>
>> struct iommu_pasid_table *arm_smmu_register_cd_table(struct device *dev,
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index 2f86c6ac42b6..0c644be22b4b 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -1869,6 +1869,13 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
>> if (ret)
>> goto out_free_cd_tables;
>>
>> + /*
>> + * Strange to setup an op here?
>> + * cd-lib is the actual user of sync op, and therefore the platform
>> + * drivers should assign this sync/maintenance ops as per need.
>> + */
>> + tbl->ops->sync = arm_smmu_sync_cd;
>> +
>
> Modifying a static struct from here doesn't feel right. I think the
> interface should be roughly similar to io-pgtable since the principle is
> the same. So the sync() op should be separate from arm_cd_table_ops since
> it's a callback into the driver. Maybe pass it to
> iommu_register_pasid_table().
Sure, will take care of this.
Thanks & regards
Vivek
On 3/3/21 10:47 PM, Jean-Philippe Brucker wrote:
> On Fri, Jan 15, 2021 at 05:43:33PM +0530, Vivek Gautam wrote:
>> From: Jean-Philippe Brucker <[email protected]>
>>
>> Add required UAPI defines for probing table format for underlying
>> iommu hardware. The device may provide information about hardware
>> tables and additional capabilities for each device.
>> This allows guest to correctly fabricate stage-1 page tables.
>>
>> Signed-off-by: Jean-Philippe Brucker <[email protected]>
>> [Vivek: Use a single "struct virtio_iommu_probe_table_format" rather
>> than separate structures for page table and pasid table format.
>
> Makes sense. I've integrated that into the spec draft, added more precise
> documentation and modified some of the definitions.
>
> The current draft is here:
> https://jpbrucker.net/virtio-iommu/spec/virtio-iommu-v0.13.pdf
> Posted on the list here
> https://lists.oasis-open.org/archives/virtio-dev/202102/msg00012.html
Thanks, I took an initial look, will review it this week.
>
>> Also update commit message.]
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Michael S. Tsirkin <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Jean-Philippe Brucker <[email protected]>
>> Cc: Eric Auger <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> Cc: Kevin Tian <[email protected]>
>> Cc: Jacob Pan <[email protected]>
>> Cc: Liu Yi L <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Shameerali Kolothum Thodi <[email protected]>
>> ---
>> include/uapi/linux/virtio_iommu.h | 44 ++++++++++++++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
>> index 237e36a280cb..43821e33e7af 100644
>> --- a/include/uapi/linux/virtio_iommu.h
>> +++ b/include/uapi/linux/virtio_iommu.h
>> @@ -2,7 +2,7 @@
>> /*
>> * Virtio-iommu definition v0.12
>> *
>> - * Copyright (C) 2019 Arm Ltd.
>> + * Copyright (C) 2019-2021 Arm Ltd.
>
> Not strictly necessary. But if you're modifying this comment please also
> remove the "v0.12" above
Sure, let me keep the copyright year unchanged until we finalize the
changes in draft spec.
>
>> */
>> #ifndef _UAPI_LINUX_VIRTIO_IOMMU_H
>> #define _UAPI_LINUX_VIRTIO_IOMMU_H
>> @@ -111,6 +111,12 @@ struct virtio_iommu_req_unmap {
>>
>> #define VIRTIO_IOMMU_PROBE_T_NONE 0
>> #define VIRTIO_IOMMU_PROBE_T_RESV_MEM 1
>> +#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK 2
>> +#define VIRTIO_IOMMU_PROBE_T_INPUT_RANGE 3
>> +#define VIRTIO_IOMMU_PROBE_T_OUTPUT_SIZE 4
>> +#define VIRTIO_IOMMU_PROBE_T_PASID_SIZE 5
>> +#define VIRTIO_IOMMU_PROBE_T_PAGE_TABLE_FMT 6
>> +#define VIRTIO_IOMMU_PROBE_T_PASID_TABLE_FMT 7
>
> Since there is a single struct we can have a single
> VIRTIO_IOMMU_PROBE_T_TABLE_FORMAT.
Right, that would make sense.
>
>>
>> #define VIRTIO_IOMMU_PROBE_T_MASK 0xfff
>>
>> @@ -130,6 +136,42 @@ struct virtio_iommu_probe_resv_mem {
>> __le64 end;
>> };
>>
>> +struct virtio_iommu_probe_page_size_mask {
>> + struct virtio_iommu_probe_property head;
>> + __u8 reserved[4];
>> + __le64 mask;
>> +};
>> +
>> +struct virtio_iommu_probe_input_range {
>> + struct virtio_iommu_probe_property head;
>> + __u8 reserved[4];
>> + __le64 start;
>> + __le64 end;
>> +};
>> +
>> +struct virtio_iommu_probe_output_size {
>> + struct virtio_iommu_probe_property head;
>> + __u8 bits;
>> + __u8 reserved[3];
>> +};
>> +
>> +struct virtio_iommu_probe_pasid_size {
>> + struct virtio_iommu_probe_property head;
>> + __u8 bits;
>> + __u8 reserved[3];
>> +};
>> +
>> +/* Arm LPAE page table format */
>> +#define VIRTIO_IOMMU_FOMRAT_PGTF_ARM_LPAE 1
>
> s/FOMRAT/FORMAT
Sure.
>
>> +/* Arm smmu-v3 type PASID table format */
>> +#define VIRTIO_IOMMU_FORMAT_PSTF_ARM_SMMU_V3 2
>
> These should be with the Arm-specific definitions patches 11 and 14
Right, will add these definitions with Arm specific patches.
Best regards
Vivek
[snip]
On 3/3/21 10:48 PM, Jean-Philippe Brucker wrote:
> On Fri, Jan 15, 2021 at 05:43:35PM +0530, Vivek Gautam wrote:
>> aisd_bits data is required to prepare stage-1 tables for arm-smmu-v3.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Jean-Philippe Brucker <[email protected]>
>> Cc: Eric Auger <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> Cc: Kevin Tian <[email protected]>
>> Cc: Jacob Pan <[email protected]>
>> Cc: Liu Yi L <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Shameerali Kolothum Thodi <[email protected]>
>> ---
>> include/uapi/linux/iommu.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
>> index 082d758dd016..96abbfc7c643 100644
>> --- a/include/uapi/linux/iommu.h
>> +++ b/include/uapi/linux/iommu.h
>> @@ -357,7 +357,7 @@ struct iommu_pasid_smmuv3 {
>> __u32 version;
>> __u8 s1fmt;
>> __u8 s1dss;
>> - __u8 padding[2];
>> + __u16 asid_bits;
>
> Is this used anywhere? This struct is passed from host userspace to host
> kernel to attach the PASID table, so I don't think it needs an asid_bits
> field.
Yea, must have missed removing it from the WIP work. Will remove it.
Thanks
Vivek
On 3/3/21 10:51 PM, Jean-Philippe Brucker wrote:
> On Fri, Jan 15, 2021 at 05:43:36PM +0530, Vivek Gautam wrote:
>> Add info about asid_bits and additional flags to table format
>> probing header.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Michael S. Tsirkin <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Jean-Philippe Brucker <[email protected]>
>> Cc: Eric Auger <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> Cc: Kevin Tian <[email protected]>
>> Cc: Jacob Pan <[email protected]>
>> Cc: Liu Yi L <[email protected]>
>> Cc: Lorenzo Pieralisi <[email protected]>
>> Cc: Shameerali Kolothum Thodi <[email protected]>
>> ---
>> include/uapi/linux/virtio_iommu.h | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
>> index 43821e33e7af..8a0624bab4b2 100644
>> --- a/include/uapi/linux/virtio_iommu.h
>> +++ b/include/uapi/linux/virtio_iommu.h
>> @@ -169,7 +169,10 @@ struct virtio_iommu_probe_pasid_size {
>> struct virtio_iommu_probe_table_format {
>> struct virtio_iommu_probe_property head;
>> __le16 format;
>> - __u8 reserved[2];
>> + __le16 asid_bits;
>> +
>> + __le32 flags;
>
> This struct should only contain the head and format fields. asid and flags
> should go in a specialized structure - virtio_iommu_probe_pgt_arm64 in the
> latest spec draft, where I dropped the asid_bits field in favor of an
> "ASID16" flag.
Right, will take care of this looking at the spec draft.
Best regards
Vivek
>
> Thanks,
> Jean
>
On Fri, Mar 12, 2021 at 06:17:55PM +0530, Vivek Kumar Gautam wrote:
> > Regarding the overall design, I was initially assigning page directories
> > instead of whole PASID tables, which would simplify the driver and host
> > implementation. A major complication, however, is SMMUv3 accesses PASID
> > tables using a guest-physical address, so there is a messy negotiation
> > needed between host and guest when the host needs to allocate PASID
> > tables. Plus vSMMU needs PASID table assignment, so that's what the host
> > driver will implement.
>
> By assigning the page directories, you mean setting up just the stage-1 page
> table ops, and passing that information to the host using ATTACH_TABLE?
Yes. And we can support nested translation with SMMUv2 that way. But with
SMMUv3 the guest has to manage the whole PASID table.
> Right now when using kvmtool, the struct iommu_pasid_table_config is
> populated with the correct information, and this whole memory is mapped
> between host and guest by creating a mem bank using
> kvm__for_each_mem_bank().
> Did I get you or did I fail terribly in understanding the point you are
> making here?
Makes sense
Thanks,
Jean