In preparation for adding stage-2 support, rename smmu_ptw_64 to
smmu_ptw_64_s1 and refactor some of the code so it can be reused in
stage-2 page table walk.
Remove AA64 check from PTW as decode_cd already ensures that AA64 is
used, otherwise it faults with C_BAD_CD.
A stage member is added to SMMUPTWEventInfo to differentiate
between stage-1 and stage-2 ptw faults.
Add stage argument to trace_smmu_ptw_level be consistent with other
trace events.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in v2:
- Refactor common functions to be use in stage-2.
- Add stage to SMMUPTWEventInfo.
- Remove AA64 check.
---
hw/arm/smmu-common.c | 27 ++++++++++-----------------
hw/arm/smmuv3.c | 2 ++
hw/arm/trace-events | 2 +-
include/hw/arm/smmu-common.h | 15 ++++++++++++---
4 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 0a5a60ca1e..b49c1affdb 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -264,7 +264,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
}
/**
- * smmu_ptw_64 - VMSAv8-64 Walk of the page tables for a given IOVA
+ * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA
* @cfg: translation config
* @iova: iova to translate
* @perm: access type
@@ -276,9 +276,9 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
* Upon success, @tlbe is filled with translated_addr and entry
* permission rights.
*/
-static int smmu_ptw_64(SMMUTransCfg *cfg,
- dma_addr_t iova, IOMMUAccessFlags perm,
- SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
+static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
+ dma_addr_t iova, IOMMUAccessFlags perm,
+ SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
dma_addr_t baseaddr, indexmask;
int stage = cfg->stage;
@@ -291,14 +291,14 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
}
granule_sz = tt->granule_sz;
- stride = granule_sz - 3;
+ stride = SMMU_STRIDE(granule_sz);
inputsize = 64 - tt->tsz;
level = 4 - (inputsize - 4) / stride;
- indexmask = (1ULL << (inputsize - (stride * (4 - level)))) - 1;
+ indexmask = SMMU_IDXMSK(inputsize, stride, level);
baseaddr = extract64(tt->ttb, 0, 48);
baseaddr &= ~indexmask;
- while (level <= 3) {
+ while (level < SMMU_LEVELS) {
uint64_t subpage_size = 1ULL << level_shift(level, granule_sz);
uint64_t mask = subpage_size - 1;
uint32_t offset = iova_level_offset(iova, inputsize, level, granule_sz);
@@ -309,7 +309,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
if (get_pte(baseaddr, offset, &pte, info)) {
goto error;
}
- trace_smmu_ptw_level(level, iova, subpage_size,
+ trace_smmu_ptw_level(stage, level, iova, subpage_size,
baseaddr, offset, pte);
if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
@@ -358,6 +358,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
info->type = SMMU_PTW_ERR_TRANSLATION;
error:
+ info->stage = 1;
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
@@ -376,15 +377,7 @@ error:
int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
- if (!cfg->aa64) {
- /*
- * This code path is not entered as we check this while decoding
- * the configuration data in the derived SMMU model.
- */
- g_assert_not_reached();
- }
-
- return smmu_ptw_64(cfg, iova, perm, tlbe, info);
+ return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
}
/**
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 270c80b665..4e90343996 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -716,6 +716,8 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
cached_entry = g_new0(SMMUTLBEntry, 1);
if (smmu_ptw(cfg, aligned_addr, flag, cached_entry, &ptw_info)) {
+ /* All faults from PTW has S2 field. */
+ event.u.f_walk_eabt.s2 = (ptw_info.stage == 2);
g_free(cached_entry);
switch (ptw_info.type) {
case SMMU_PTW_ERR_WALK_EABT:
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 2dee296c8f..205ac04573 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -5,7 +5,7 @@ virt_acpi_setup(void) "No fw cfg or ACPI disabled. Bailing out."
# smmu-common.c
smmu_add_mr(const char *name) "%s"
-smmu_ptw_level(int level, uint64_t iova, size_t subpage_size, uint64_t baseaddr, uint32_t offset, uint64_t pte) "level=%d iova=0x%"PRIx64" subpage_sz=0x%zx baseaddr=0x%"PRIx64" offset=%d => pte=0x%"PRIx64
+smmu_ptw_level(int stage, int level, uint64_t iova, size_t subpage_size, uint64_t baseaddr, uint32_t offset, uint64_t pte) "stage=%d level=%d iova=0x%"PRIx64" subpage_sz=0x%zx baseaddr=0x%"PRIx64" offset=%d => pte=0x%"PRIx64
smmu_ptw_invalid_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, uint32_t offset, uint64_t pte) "stage=%d level=%d base@=0x%"PRIx64" pte@=0x%"PRIx64" offset=%d pte=0x%"PRIx64
smmu_ptw_page_pte(int stage, int level, uint64_t iova, uint64_t baseaddr, uint64_t pteaddr, uint64_t pte, uint64_t address) "stage=%d level=%d iova=0x%"PRIx64" base@=0x%"PRIx64" pte@=0x%"PRIx64" pte=0x%"PRIx64" page address = 0x%"PRIx64
smmu_ptw_block_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, uint64_t pte, uint64_t iova, uint64_t gpa, int bsize_mb) "stage=%d level=%d base@=0x%"PRIx64" pte@=0x%"PRIx64" pte=0x%"PRIx64" iova=0x%"PRIx64" block address = 0x%"PRIx64" block size = %d MiB"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 2deead08d6..1ada792122 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -23,9 +23,17 @@
#include "hw/pci/pci.h"
#include "qom/object.h"
-#define SMMU_PCI_BUS_MAX 256
-#define SMMU_PCI_DEVFN_MAX 256
-#define SMMU_PCI_DEVFN(sid) (sid & 0xFF)
+#define SMMU_PCI_BUS_MAX 256
+#define SMMU_PCI_DEVFN_MAX 256
+#define SMMU_PCI_DEVFN(sid) (sid & 0xFF)
+
+#define SMMU_LEVELS 4
+
+#define SMMU_STRIDE(gran) ((gran) - SMMU_LEVELS + 1)
+#define SMMU_BIT_LVL(isz, strd, lvl) ((isz) - (strd) * \
+ (SMMU_LEVELS - (lvl)))
+#define SMMU_IDXMSK(isz, strd, lvl) ((1ULL << \
+ SMMU_BIT_LVL(isz, strd, lvl)) - 1)
/*
* Page table walk error types
@@ -40,6 +48,7 @@ typedef enum {
} SMMUPTWEventType;
typedef struct SMMUPTWEventInfo {
+ int stage;
SMMUPTWEventType type;
dma_addr_t addr; /* fetched address that induced an abort, if any */
} SMMUPTWEventInfo;
--
2.39.2.637.g21b0678d19-googHi Mostafa,
On 2/26/23 23:06, Mostafa Saleh wrote:
> In preparation for adding stage-2 support, rename smmu_ptw_64 to
> smmu_ptw_64_s1 and refactor some of the code so it can be reused in
> stage-2 page table walk.
>
> Remove AA64 check from PTW as decode_cd already ensures that AA64 is
> used, otherwise it faults with C_BAD_CD.
>
> A stage member is added to SMMUPTWEventInfo to differentiate
> between stage-1 and stage-2 ptw faults.
>
> Add stage argument to trace_smmu_ptw_level be consistent with other
> trace events.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in v2:
> - Refactor common functions to be use in stage-2.
> - Add stage to SMMUPTWEventInfo.
> - Remove AA64 check.
> ---
> hw/arm/smmu-common.c | 27 ++++++++++-----------------
> hw/arm/smmuv3.c | 2 ++
> hw/arm/trace-events | 2 +-
> include/hw/arm/smmu-common.h | 15 ++++++++++++---
> 4 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 0a5a60ca1e..b49c1affdb 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -264,7 +264,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
> }
>
> /**
> - * smmu_ptw_64 - VMSAv8-64 Walk of the page tables for a given IOVA
> + * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA
> * @cfg: translation config
> * @iova: iova to translate
> * @perm: access type
> @@ -276,9 +276,9 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
> * Upon success, @tlbe is filled with translated_addr and entry
> * permission rights.
> */
> -static int smmu_ptw_64(SMMUTransCfg *cfg,
> - dma_addr_t iova, IOMMUAccessFlags perm,
> - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> +static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
> + dma_addr_t iova, IOMMUAccessFlags perm,
> + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> dma_addr_t baseaddr, indexmask;
> int stage = cfg->stage;
> @@ -291,14 +291,14 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
> }
>
> granule_sz = tt->granule_sz;
> - stride = granule_sz - 3;
> + stride = SMMU_STRIDE(granule_sz);
> inputsize = 64 - tt->tsz;
> level = 4 - (inputsize - 4) / stride;
> - indexmask = (1ULL << (inputsize - (stride * (4 - level)))) - 1;
> + indexmask = SMMU_IDXMSK(inputsize, stride, level);
> baseaddr = extract64(tt->ttb, 0, 48);
> baseaddr &= ~indexmask;
>
> - while (level <= 3) {
> + while (level < SMMU_LEVELS) {
> uint64_t subpage_size = 1ULL << level_shift(level, granule_sz);
> uint64_t mask = subpage_size - 1;
> uint32_t offset = iova_level_offset(iova, inputsize, level, granule_sz);
> @@ -309,7 +309,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
> if (get_pte(baseaddr, offset, &pte, info)) {
> goto error;
> }
> - trace_smmu_ptw_level(level, iova, subpage_size,
> + trace_smmu_ptw_level(stage, level, iova, subpage_size,
> baseaddr, offset, pte);
>
> if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
> @@ -358,6 +358,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
> info->type = SMMU_PTW_ERR_TRANSLATION;
>
> error:
> + info->stage = 1;
> tlbe->entry.perm = IOMMU_NONE;
> return -EINVAL;
> }
> @@ -376,15 +377,7 @@ error:
> int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> - if (!cfg->aa64) {
> - /*
> - * This code path is not entered as we check this while decoding
> - * the configuration data in the derived SMMU model.
> - */
> - g_assert_not_reached();
> - }
> -
> - return smmu_ptw_64(cfg, iova, perm, tlbe, info);
> + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
> }
>
> /**
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 270c80b665..4e90343996 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -716,6 +716,8 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> cached_entry = g_new0(SMMUTLBEntry, 1);
>
> if (smmu_ptw(cfg, aligned_addr, flag, cached_entry, &ptw_info)) {
> + /* All faults from PTW has S2 field. */
> + event.u.f_walk_eabt.s2 = (ptw_info.stage == 2);
> g_free(cached_entry);
> switch (ptw_info.type) {
> case SMMU_PTW_ERR_WALK_EABT:
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index 2dee296c8f..205ac04573 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -5,7 +5,7 @@ virt_acpi_setup(void) "No fw cfg or ACPI disabled. Bailing out."
>
> # smmu-common.c
> smmu_add_mr(const char *name) "%s"
> -smmu_ptw_level(int level, uint64_t iova, size_t subpage_size, uint64_t baseaddr, uint32_t offset, uint64_t pte) "level=%d iova=0x%"PRIx64" subpage_sz=0x%zx baseaddr=0x%"PRIx64" offset=%d => pte=0x%"PRIx64
> +smmu_ptw_level(int stage, int level, uint64_t iova, size_t subpage_size, uint64_t baseaddr, uint32_t offset, uint64_t pte) "stage=%d level=%d iova=0x%"PRIx64" subpage_sz=0x%zx baseaddr=0x%"PRIx64" offset=%d => pte=0x%"PRIx64
> smmu_ptw_invalid_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, uint32_t offset, uint64_t pte) "stage=%d level=%d base@=0x%"PRIx64" pte@=0x%"PRIx64" offset=%d pte=0x%"PRIx64
> smmu_ptw_page_pte(int stage, int level, uint64_t iova, uint64_t baseaddr, uint64_t pteaddr, uint64_t pte, uint64_t address) "stage=%d level=%d iova=0x%"PRIx64" base@=0x%"PRIx64" pte@=0x%"PRIx64" pte=0x%"PRIx64" page address = 0x%"PRIx64
> smmu_ptw_block_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, uint64_t pte, uint64_t iova, uint64_t gpa, int bsize_mb) "stage=%d level=%d base@=0x%"PRIx64" pte@=0x%"PRIx64" pte=0x%"PRIx64" iova=0x%"PRIx64" block address = 0x%"PRIx64" block size = %d MiB"
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 2deead08d6..1ada792122 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -23,9 +23,17 @@
> #include "hw/pci/pci.h"
> #include "qom/object.h"
>
> -#define SMMU_PCI_BUS_MAX 256
> -#define SMMU_PCI_DEVFN_MAX 256
> -#define SMMU_PCI_DEVFN(sid) (sid & 0xFF)
> +#define SMMU_PCI_BUS_MAX 256
> +#define SMMU_PCI_DEVFN_MAX 256
> +#define SMMU_PCI_DEVFN(sid) (sid & 0xFF)
> +
> +#define SMMU_LEVELS 4
> +
> +#define SMMU_STRIDE(gran) ((gran) - SMMU_LEVELS + 1)
> +#define SMMU_BIT_LVL(isz, strd, lvl) ((isz) - (strd) * \
> + (SMMU_LEVELS - (lvl)))
> +#define SMMU_IDXMSK(isz, strd, lvl) ((1ULL << \
> + SMMU_BIT_LVL(isz, strd, lvl)) - 1)
This looks good to me. Just a question about the BIT_LVL and IDXMSK
defines. Do they correspond to any documented pseudocode functions
documented somewhere in the ARM ARM?
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
>
> /*
> * Page table walk error types
> @@ -40,6 +48,7 @@ typedef enum {
> } SMMUPTWEventType;
>
> typedef struct SMMUPTWEventInfo {
> + int stage;
> SMMUPTWEventType type;
> dma_addr_t addr; /* fetched address that induced an abort, if any */
> } SMMUPTWEventInfo;
Hi Eric, On Fri, Mar 17, 2023 at 07:31:06PM +0100, Eric Auger wrote: > > +#define SMMU_LEVELS 4 > > + > > +#define SMMU_STRIDE(gran) ((gran) - SMMU_LEVELS + 1) > > +#define SMMU_BIT_LVL(isz, strd, lvl) ((isz) - (strd) * \ > > + (SMMU_LEVELS - (lvl))) > > +#define SMMU_IDXMSK(isz, strd, lvl) ((1ULL << \ > > + SMMU_BIT_LVL(isz, strd, lvl)) - 1) > This looks good to me. Just a question about the BIT_LVL and IDXMSK > defines. Do they correspond to any documented pseudocode functions > documented somewhere in the ARM ARM? I see they are not implemented as functions in ARM ARM, but as part of aarch64/translation/vmsa_addrcalc/AArch64.TTBaseAddress: constant integer FINAL_LEVEL = 3; levels = FINAL_LEVEL - startlevel; tsize = (iasize - (levels*stride + granulebits)) + 3; tablebase = Align(tablebase, 1 << tsize); This gives the same result, however the equations are a bit different as they use final level(3), while we use number of levels(4). Thanks, Mostafa
© 2016 - 2026 Red Hat, Inc.