* [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3
@ 2023-04-01 10:49 Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 01/10] hw/arm/smmuv3: Add missing fields for IDR0 Mostafa Saleh
` (10 more replies)
0 siblings, 11 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
This patch series adds stage-2 translation support for SMMUv3. It is
controlled by a new system property “arm-smmuv3.stage”.
- When set to “1”: Stage-1 only would be advertised and supported (default
behaviour)
- When set to “2”: Stage-2 only would be advertised and supported.
- Value “all” is reserved for nesting support. However it is not
implemented in this patch series (more about this in the end)
Features implemented in stage-2 are mostly synonymous with stage-1
- VMID16.
- Only AArch64 translation tables are supported.
- Only little endian translation table supported.
- Stall is not supported.
- HTTU is not supported, SW is expected to maintain the Access flag.
To make it easy to support nesting, a new structure(SMMUS2Cfg) is
embedded within SMMUTransCfg, to hold stage-2 configuration.
TLBs were updated to support VMID, where when stage-2 is used ASID is
set to -1 and ignored and when stage-1 is used VMID is set to -1 and
ignored.
As only one stage is supported at a time at the moment, TLB will
represent IPA=>PA translation with proper attributes(granularity and
t0sz) parsed from STEs for stage-2, and will represent VA=>PA
translation with proper attributes parsed from the CDs for stage-1.
New commands where added that are used with stage-2
- CMD_TLBI_S12_VMALL: Invalidate all translations for a VMID.
- CMD_TLBI_S2_IPA: Invalidate stage-2 by VMID and IPA
Some commands are illegal to be used from stage-2 were modified to
return CERROR_ILL.
This patch series can be used to run Linux pKVM SMMUv3 patches (currently on the list)
which controls stage-2 (from EL2) while providing a paravirtualized
interface the host(EL1)
https://lore.kernel.org/kvmarm/20230201125328.2186498-1-jean-philippe@linaro.org/
Looking forward, nesting is the next feature to go for, here are some
thoughts about this:
- TLB would require big changes for this, we can go either for a combined
implementation or per stage one. This would affect returns from PTW and
invalidation commands.
- Stage-1 data structures should be translated by stage-2 if enabled (as
context descriptors and ttb0/ttb1)
- Translated addresses from stage-1 should be translated by stage-2 if
enabled.
- Some existing commands(as CMD_TLBI_S2_IPA, CMD_TLBI_NH_ASID …) would be
modified and some of those would be based on the design of the TLBs.
- Currently, VMID is ignored when stage-1 is used as it can’t be used with
stage-2. However when nesting is advertised VMID shouldn’t be ignored
even if stage-2 is bypassed.
Changes in v3:
- Collected Reviewed-by tags
- Separate stage-2 record faults from stage-1
- Fix input address check in stage-2 walk
- Fix shift in STE_S2HD, STE_S2HA, STE_S2S, STE_S2R
- Add more logs for illegal configs and commands.
- Rename SMMU translation macros to VMSA as they are not part of SMMU spec
- Rename stage-2 variables and functions (iova=>ipa, ap=>s2ap, ...)
- Rename oas in SMMUS2Cfg to eff_ps
- Improve comments (mention user manuals versions, field names)
Changes in v2:
-Collected Reviewed-by tags
-Add oas to SMMUS2Cfg, and use it in PTW
-Add stage member to to SMMUPTWEventInfo to differentiate stage-1 and
stage-2 PTW faults
-Move stage-2 knob to the last patch
-Add all STE parsing in one patch
-Pares and use S2PS and S2ENDI
-Split S2AFF patch over PTW and STE patches.
-Fix TLB aliasing issue
-Renaming and refactoring and rewording commits.
-Populate OAS based on PARANGE
-Add checks for stage-1 only commands
-Update trace events to hold translation stage, vmid when possible
-Remove feature flags for supported stages as they were redundant with IDR0
Mostafa Saleh (10):
hw/arm/smmuv3: Add missing fields for IDR0
hw/arm/smmuv3: Update translation config to hold stage-2
hw/arm/smmuv3: Refactor stage-1 PTW
hw/arm/smmuv3: Add page table walk for stage-2
hw/arm/smmuv3: Parse STE config for stage-2
hw/arm/smmuv3: Make TLB lookup work for stage-2
hw/arm/smmuv3: Add VMID to TLB tagging
hw/arm/smmuv3: Add CMDs related to stage-2
hw/arm/smmuv3: Add stage-2 support in iova notifier
hw/arm/smmuv3: Add knob to choose translation stage and enable stage-2
hw/arm/smmu-common.c | 209 +++++++++++++++++---
hw/arm/smmu-internal.h | 37 ++++
hw/arm/smmuv3-internal.h | 12 +-
hw/arm/smmuv3.c | 364 ++++++++++++++++++++++++++++++-----
hw/arm/trace-events | 14 +-
include/hw/arm/smmu-common.h | 45 ++++-
include/hw/arm/smmuv3.h | 4 +
7 files changed, 595 insertions(+), 90 deletions(-)
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH v3 01/10] hw/arm/smmuv3: Add missing fields for IDR0
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 02/10] hw/arm/smmuv3: Update translation config to hold stage-2 Mostafa Saleh
` (9 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
In preparation for adding stage-2 support.
Add IDR0 fields related to stage-2.
VMID16: 16-bit VMID supported.
S2P: Stage-2 translation supported.
They are described in 6.3.1 SMMU_IDR0.
No functional change intended.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in V2:
- Collected Reviewed-by tags.
---
hw/arm/smmuv3-internal.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index e8f0ebf25e..183d5ac8dc 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -34,10 +34,12 @@ typedef enum SMMUTranslationStatus {
/* MMIO Registers */
REG32(IDR0, 0x0)
+ FIELD(IDR0, S2P, 0 , 1)
FIELD(IDR0, S1P, 1 , 1)
FIELD(IDR0, TTF, 2 , 2)
FIELD(IDR0, COHACC, 4 , 1)
FIELD(IDR0, ASID16, 12, 1)
+ FIELD(IDR0, VMID16, 18, 1)
FIELD(IDR0, TTENDIAN, 21, 2)
FIELD(IDR0, STALL_MODEL, 24, 2)
FIELD(IDR0, TERM_MODEL, 26, 1)
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 02/10] hw/arm/smmuv3: Update translation config to hold stage-2
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 01/10] hw/arm/smmuv3: Add missing fields for IDR0 Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-05-15 8:37 ` Eric Auger
2023-04-01 10:49 ` [RFC PATCH v3 03/10] hw/arm/smmuv3: Refactor stage-1 PTW Mostafa Saleh
` (8 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
In preparation for adding stage-2 support, add a S2 config
struct(SMMUS2Cfg), composed of the following fields and embedded in
the main SMMUTransCfg:
-tsz: Size of IPA input region (S2T0SZ)
-sl0: Start level of translation (S2SL0)
-affd: AF Fault Disable (S2AFFD)
-record_faults: Record fault events (S2R)
-granule_sz: Granule page shift (based on S2TG)
-vmid: Virtual Machine ID (S2VMID)
-vttb: Address of translation table base (S2TTB)
-eff_ps: Effective PA output range (based on S2PS)
They will be used in the next patches in stage-2 address translation.
The fields in SMMUS2Cfg, are reordered to make the shared and stage-1
fields next to each other, this reordering didn't change the struct
size (104 bytes before and after).
Stage-1 only fields: aa64, asid, tt, ttb, tbi, record_faults, oas.
oas is stage-1 output address size. However, it is used to check
input address in case stage-1 is unimplemented or bypassed according
to SMMUv3 manual IHI0070.E "3.4. Address sizes"
Shared fields: stage, disabled, bypassed, aborted, iotlb_*.
No functional change intended.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in v3:
-Add record_faults for stage-2
-Reorder and document fields in SMMUTransCfg based on stage
-Rename oas in SMMUS2Cfg to eff_ps
-Improve comments in SMMUS2Cfg
Changes in v2:
-Add oas
---
include/hw/arm/smmu-common.h | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 9fcff26357..9cf3f37929 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -58,25 +58,41 @@ typedef struct SMMUTLBEntry {
uint8_t granule;
} SMMUTLBEntry;
+/* Stage-2 configuration. */
+typedef struct SMMUS2Cfg {
+ uint8_t tsz; /* Size of IPA input region (S2T0SZ) */
+ uint8_t sl0; /* Start level of translation (S2SL0) */
+ bool affd; /* AF Fault Disable (S2AFFD) */
+ bool record_faults; /* Record fault events (S2R) */
+ uint8_t granule_sz; /* Granule page shift (based on S2TG) */
+ uint8_t eff_ps; /* Effective PA output range (based on S2PS) */
+ uint16_t vmid; /* Virtual Machine ID (S2VMID) */
+ uint64_t vttb; /* Address of translation table base (S2TTB) */
+} SMMUS2Cfg;
+
/*
* Generic structure populated by derived SMMU devices
* after decoding the configuration information and used as
* input to the page table walk
*/
typedef struct SMMUTransCfg {
+ /* Shared fields between stage-1 and stage-2. */
int stage; /* translation stage */
- bool aa64; /* arch64 or aarch32 translation table */
bool disabled; /* smmu is disabled */
bool bypassed; /* translation is bypassed */
bool aborted; /* translation is aborted */
+ uint32_t iotlb_hits; /* counts IOTLB hits */
+ uint32_t iotlb_misses; /* counts IOTLB misses*/
+ /* Used by stage-1 only. */
+ bool aa64; /* arch64 or aarch32 translation table */
bool record_faults; /* record fault events */
uint64_t ttb; /* TT base address */
uint8_t oas; /* output address width */
uint8_t tbi; /* Top Byte Ignore */
uint16_t asid;
SMMUTransTableInfo tt[2];
- uint32_t iotlb_hits; /* counts IOTLB hits for this asid */
- uint32_t iotlb_misses; /* counts IOTLB misses for this asid */
+ /* Used by stage-2 only. */
+ struct SMMUS2Cfg s2cfg;
} SMMUTransCfg;
typedef struct SMMUDevice {
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 03/10] hw/arm/smmuv3: Refactor stage-1 PTW
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 01/10] hw/arm/smmuv3: Add missing fields for IDR0 Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 02/10] hw/arm/smmuv3: Update translation config to hold stage-2 Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 04/10] hw/arm/smmuv3: Add page table walk for stage-2 Mostafa Saleh
` (7 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
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>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
Changes in v3:
- Collected Reviewed-by tag
- Rename translation consts and macros from SMMU_* to VMSA_*
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 | 16 +++++++++++++---
4 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index e7f1c1f219..50391a8c94 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 = VMSA_STRIDE(granule_sz);
inputsize = 64 - tt->tsz;
level = 4 - (inputsize - 4) / stride;
- indexmask = (1ULL << (inputsize - (stride * (4 - level)))) - 1;
+ indexmask = VMSA_IDXMSK(inputsize, stride, level);
baseaddr = extract64(tt->ttb, 0, 48);
baseaddr &= ~indexmask;
- while (level <= 3) {
+ while (level < VMSA_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 9cf3f37929..97cea8ea06 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -23,9 +23,18 @@
#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)
+
+/* VMSAv8-64 Translation constants and functions */
+#define VMSA_LEVELS 4
+
+#define VMSA_STRIDE(gran) ((gran) - VMSA_LEVELS + 1)
+#define VMSA_BIT_LVL(isz, strd, lvl) ((isz) - (strd) * \
+ (VMSA_LEVELS - (lvl)))
+#define VMSA_IDXMSK(isz, strd, lvl) ((1ULL << \
+ VMSA_BIT_LVL(isz, strd, lvl)) - 1)
/*
* Page table walk error types
@@ -40,6 +49,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.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 04/10] hw/arm/smmuv3: Add page table walk for stage-2
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (2 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 03/10] hw/arm/smmuv3: Refactor stage-1 PTW Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-05-15 8:37 ` Eric Auger
2023-04-01 10:49 ` [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config " Mostafa Saleh
` (6 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
In preparation for adding stage-2 support, add Stage-2 PTW code.
Only Aarch64 format is supported as stage-1.
Nesting stage-1 and stage-2 is not supported right now.
HTTU is not supported, SW is expected to maintain the Access flag.
This is described in the SMMUv3 manual(IHI 0070.E)
"5.2. Stream Table Entry" in "[181] S2AFFD".
This flag determines the behavior on access of a stage-2 page whose
descriptor has AF == 0:
- 0b0: An Access flag fault occurs (stall not supported).
- 0b1: An Access flag fault never occurs.
An Access fault takes priority over a Permission fault.
There are 3 address size checks for stage-2 according to "IHI 0070.E"
in "3.4. Address sizes".
- As nesting is not supported, input address is passed directly to
stage-2, and is checked against IAS.
We use cfg->oas to hold the OAS when stage-1 is not used, this is set
in the next patch.
This check is done outside of smmu_ptw_64_s2 as it is not part of
stage-2(it throws stage-1 fault), and the stage-2 function shouldn't
change it's behavior when nesting is supported.
When nesting is supported and we figure out how to combine TLB for
stage-1 and stage-2 we can move this check into the stage-1 function
as described in ARM DDI0487I.a in pseudocode
aarch64/translation/vmsa_translation/AArch64.S1Translate
aarch64/translation/vmsa_translation/AArch64.S1DisabledOutput
- Input to stage-2 is checked against s2t0sz, and throws stage-2
transaltion fault if exceeds it.
- Output of stage-2 is checked against effective PA output range.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in v3:
- Fix IPA address size check.
- s2cfg.oas renamed to s2cfg.eff_ps.
- s/iova/ipa
- s/ap/s2ap
- s/gran/granule_sz
- use level_shift instead of inline code.
- Add missing brackets in is_permission_fault_s2.
- Use new VMSA_* macros and functions instead of SMMU_*
- Rename pgd_idx to pgd_concat_idx.
- Move SMMU_MAX_S2_CONCAT to STE patch as it is not used here.
Changes in v2:
- Squash S2AFF PTW code.
- Use common functions between stage-1 and stage-2.
- Add checks for IPA and out PA.
---
hw/arm/smmu-common.c | 142 ++++++++++++++++++++++++++++++++++++++++-
hw/arm/smmu-internal.h | 35 ++++++++++
2 files changed, 176 insertions(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 50391a8c94..a4ebd004c5 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -363,6 +363,127 @@ error:
return -EINVAL;
}
+/**
+ * smmu_ptw_64_s2 - VMSAv8-64 Walk of the page tables for a given ipa
+ * for stage-2.
+ * @cfg: translation config
+ * @ipa: ipa to translate
+ * @perm: access type
+ * @tlbe: SMMUTLBEntry (out)
+ * @info: handle to an error info
+ *
+ * Return 0 on success, < 0 on error. In case of error, @info is filled
+ * and tlbe->perm is set to IOMMU_NONE.
+ * Upon success, @tlbe is filled with translated_addr and entry
+ * permission rights.
+ */
+static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
+ dma_addr_t ipa, IOMMUAccessFlags perm,
+ SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
+{
+ const int stage = 2;
+ int granule_sz = cfg->s2cfg.granule_sz;
+ /* ARM DDI0487I.a: Table D8-7. */
+ int inputsize = 64 - cfg->s2cfg.tsz;
+ int level = get_start_level(cfg->s2cfg.sl0, granule_sz);
+ int stride = VMSA_STRIDE(granule_sz);
+ int idx = pgd_concat_idx(level, granule_sz, ipa);
+ /*
+ * Get the ttb from concatenated structure.
+ * The offset is the idx * size of each ttb(number of ptes * (sizeof(pte))
+ */
+ uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, 48) + (1 << stride) *
+ idx * sizeof(uint64_t);
+ dma_addr_t indexmask = VMSA_IDXMSK(inputsize, stride, level);
+
+ baseaddr &= ~indexmask;
+
+ /*
+ * On input, a stage 2 Translation fault occurs if the IPA is outside the
+ * range configured by the relevant S2T0SZ field of the STE.
+ */
+ if (ipa >= (1ULL << inputsize)) {
+ info->type = SMMU_PTW_ERR_TRANSLATION;
+ goto error;
+ }
+
+ while (level < VMSA_LEVELS) {
+ uint64_t subpage_size = 1ULL << level_shift(level, granule_sz);
+ uint64_t mask = subpage_size - 1;
+ uint32_t offset = iova_level_offset(ipa, inputsize, level, granule_sz);
+ uint64_t pte, gpa;
+ dma_addr_t pte_addr = baseaddr + offset * sizeof(pte);
+ uint8_t s2ap;
+
+ if (get_pte(baseaddr, offset, &pte, info)) {
+ goto error;
+ }
+ trace_smmu_ptw_level(stage, level, ipa, subpage_size,
+ baseaddr, offset, pte);
+ if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
+ trace_smmu_ptw_invalid_pte(stage, level, baseaddr,
+ pte_addr, offset, pte);
+ break;
+ }
+
+ if (is_table_pte(pte, level)) {
+ baseaddr = get_table_pte_address(pte, granule_sz);
+ level++;
+ continue;
+ } else if (is_page_pte(pte, level)) {
+ gpa = get_page_pte_address(pte, granule_sz);
+ trace_smmu_ptw_page_pte(stage, level, ipa,
+ baseaddr, pte_addr, pte, gpa);
+ } else {
+ uint64_t block_size;
+
+ gpa = get_block_pte_address(pte, level, granule_sz,
+ &block_size);
+ trace_smmu_ptw_block_pte(stage, level, baseaddr,
+ pte_addr, pte, ipa, gpa,
+ block_size >> 20);
+ }
+
+ /*
+ * If S2AFFD and PTE.AF are 0 => fault. (5.2. Stream Table Entry)
+ * An Access fault takes priority over a Permission fault.
+ */
+ if (!PTE_AF(pte) && !cfg->s2cfg.affd) {
+ info->type = SMMU_PTW_ERR_ACCESS;
+ goto error;
+ }
+
+ s2ap = PTE_AP(pte);
+ if (is_permission_fault_s2(s2ap, perm)) {
+ info->type = SMMU_PTW_ERR_PERMISSION;
+ goto error;
+ }
+
+ /*
+ * The address output from the translation causes a stage 2 Address
+ * Size fault if it exceeds the effective PA output range.
+ */
+ if (gpa >= (1ULL << cfg->s2cfg.eff_ps)) {
+ info->type = SMMU_PTW_ERR_ADDR_SIZE;
+ goto error;
+ }
+
+ tlbe->entry.translated_addr = gpa;
+ tlbe->entry.iova = ipa & ~mask;
+ tlbe->entry.addr_mask = mask;
+ tlbe->entry.perm = s2ap;
+ tlbe->level = level;
+ tlbe->granule = granule_sz;
+ return 0;
+ }
+ info->type = SMMU_PTW_ERR_TRANSLATION;
+
+error:
+ info->stage = 2;
+ tlbe->entry.perm = IOMMU_NONE;
+ return -EINVAL;
+}
+
/**
* smmu_ptw - Walk the page tables for an IOVA, according to @cfg
*
@@ -377,7 +498,26 @@ error:
int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
- return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
+ if (cfg->stage == 1) {
+ return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
+ } else if (cfg->stage == 2) {
+ /*
+ * If bypassing stage 1(or unimplemented), the input address is passed
+ * directly to stage 2 as IPA. If the input address of a transaction
+ * exceeds the size of the IAS, a stage 1 Address Size fault occurs.
+ * For AA64, IAS = OAS according to (IHI 0070.E) "3.4 Address sizes"
+ */
+ if (iova >= (1ULL << cfg->oas)) {
+ info->type = SMMU_PTW_ERR_ADDR_SIZE;
+ info->stage = 1;
+ tlbe->entry.perm = IOMMU_NONE;
+ return -EINVAL;
+ }
+
+ return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info);
+ }
+
+ g_assert_not_reached();
}
/**
diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
index 2d75b31953..a9454f914e 100644
--- a/hw/arm/smmu-internal.h
+++ b/hw/arm/smmu-internal.h
@@ -66,6 +66,8 @@
#define PTE_APTABLE(pte) \
(extract64(pte, 61, 2))
+#define PTE_AF(pte) \
+ (extract64(pte, 10, 1))
/*
* TODO: At the moment all transactions are considered as privileged (EL1)
* as IOMMU translation callback does not pass user/priv attributes.
@@ -73,6 +75,9 @@
#define is_permission_fault(ap, perm) \
(((perm) & IOMMU_WO) && ((ap) & 0x2))
+#define is_permission_fault_s2(s2ap, perm) \
+ (!(((s2ap) & (perm)) == (perm)))
+
#define PTE_AP_TO_PERM(ap) \
(IOMMU_ACCESS_FLAG(true, !((ap) & 0x2)))
@@ -96,6 +101,36 @@ uint64_t iova_level_offset(uint64_t iova, int inputsize,
MAKE_64BIT_MASK(0, gsz - 3);
}
+/* FEAT_LPA2 and FEAT_TTST are not implemented. */
+static inline int get_start_level(int sl0 , int granule_sz)
+{
+ /* ARM DDI0487I.a: Table D8-12. */
+ if (granule_sz == 12) {
+ return 2 - sl0;
+ }
+ /* ARM DDI0487I.a: Table D8-22 and Table D8-31. */
+ return 3 - sl0;
+}
+
+/*
+ * Index in a concatenated first level stage-2 page table.
+ * ARM DDI0487I.a: D8.2.2 Concatenated translation tables.
+ */
+static inline int pgd_concat_idx(int start_level, int granule_sz,
+ dma_addr_t ipa)
+{
+ uint64_t ret;
+ /*
+ * Get the number of bits handled by next levels, then any extra bits in
+ * the address should index the concatenated tables. This relation can be
+ * deduced from tables in ARM DDI0487I.a: D8.2.7-9
+ */
+ int shift = level_shift(start_level - 1, granule_sz);
+
+ ret = ipa >> shift;
+ return ret;
+}
+
#define SMMU_IOTLB_ASID(key) ((key).asid)
typedef struct SMMUIOTLBPageInvInfo {
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config for stage-2
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (3 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 04/10] hw/arm/smmuv3: Add page table walk for stage-2 Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-05-15 13:03 ` Eric Auger
2023-04-01 10:49 ` [RFC PATCH v3 06/10] hw/arm/smmuv3: Make TLB lookup work " Mostafa Saleh
` (5 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
Parse stage-2 configuration from STE and populate it in SMMUS2Cfg.
Validity of field values are checked when possible.
Only AA64 tables are supported and Small Translation Tables (STT) are
not supported.
According to SMMUv3 UM(IHI0070E) "5.2 Stream Table Entry": All fields
with an S2 prefix (with the exception of S2VMID) are IGNORED when
stage-2 bypasses translation (Config[1] == 0).
Which means that VMID can be used(for TLB tagging) even if stage-2 is
bypassed, so we parse it unconditionally when S2P exists. Otherwise
it is set to -1.(only S1P)
As stall is not supported, if S2S is set the translation would abort.
For S2R, we reuse the same code used for stage-1 with flag
record_faults. However when nested translation is supported we would
need to separate stage-1 and stage-2 faults.
Fix wrong shift in STE_S2HD, STE_S2HA, STE_S2S.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in V3:
- Separate fault handling.
- Fix shift in STE_S2HD, STE_S2HA, STE_S2S, STE_S2R.
- Rename t0sz_valid to s2t0sz_valid.
- separate stage-2 STE parsing in decode_ste_s2_cfg.
- Add a log for invalid S2ENDI and S2TTB.
- Set default value for stage-1 OAS.
- Move and rename SMMU_MAX_S2_CONCAT to VMSA_MAX_S2_CONCAT.
Changes in V2:
- Parse S2PS and S2ENDI
- Squash with S2VMID parsing patch
- Squash with S2AFF parsing
- Squash with fault reporting patch
- Add check for S2T0SZ
- Renaming and refactoring code
---
hw/arm/smmuv3-internal.h | 10 +-
hw/arm/smmuv3.c | 188 +++++++++++++++++++++++++++++++++--
include/hw/arm/smmu-common.h | 1 +
include/hw/arm/smmuv3.h | 3 +
4 files changed, 192 insertions(+), 10 deletions(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 183d5ac8dc..6d1c1edab7 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -526,9 +526,13 @@ typedef struct CD {
#define STE_S2TG(x) extract32((x)->word[5], 14, 2)
#define STE_S2PS(x) extract32((x)->word[5], 16, 3)
#define STE_S2AA64(x) extract32((x)->word[5], 19, 1)
-#define STE_S2HD(x) extract32((x)->word[5], 24, 1)
-#define STE_S2HA(x) extract32((x)->word[5], 25, 1)
-#define STE_S2S(x) extract32((x)->word[5], 26, 1)
+#define STE_S2ENDI(x) extract32((x)->word[5], 20, 1)
+#define STE_S2AFFD(x) extract32((x)->word[5], 21, 1)
+#define STE_S2HD(x) extract32((x)->word[5], 23, 1)
+#define STE_S2HA(x) extract32((x)->word[5], 24, 1)
+#define STE_S2S(x) extract32((x)->word[5], 25, 1)
+#define STE_S2R(x) extract32((x)->word[5], 26, 1)
+
#define STE_CTXPTR(x) \
({ \
unsigned long addr; \
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 4e90343996..0f5429aed8 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -33,6 +33,16 @@
#include "smmuv3-internal.h"
#include "smmu-internal.h"
+/* If stage-1 fault enabled and ptw event targets it. */
+#define PTW_FAULT_S1(cfg, event) ((cfg)->record_faults && \
+ !(event).u.f_walk_eabt.s2)
+/* If stage-2 fault enabled and ptw event targets it. */
+#define PTW_FAULT_S2(cfg, event) ((cfg)->s2cfg.record_faults && \
+ (event).u.f_walk_eabt.s2)
+
+#define PTW_FAULT_ALLOWED(cfg, event) (PTW_FAULT_S1(cfg, event) || \
+ PTW_FAULT_S2(cfg, event))
+
/**
* smmuv3_trigger_irq - pulse @irq if enabled and update
* GERROR register in case of GERROR interrupt
@@ -329,11 +339,141 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid,
return 0;
}
+/*
+ * Max valid value is 39 when SMMU_IDR3.STT == 0.
+ * In architectures after SMMUv3.0:
+ * - If STE.S2TG selects a 4KB or 16KB granule, the minimum valid value for this
+ * field is MAX(16, 64-IAS)
+ * - If STE.S2TG selects a 64KB granule, the minimum valid value for this field
+ * is (64-IAS).
+ * As we only support AA64, IAS = OAS.
+ */
+static bool s2t0sz_valid(SMMUTransCfg *cfg)
+{
+ if (cfg->s2cfg.tsz > 39) {
+ return false;
+ }
+
+ if (cfg->s2cfg.granule_sz == 16) {
+ return (cfg->s2cfg.tsz >= 64 - oas2bits(SMMU_IDR5_OAS));
+ }
+
+ return (cfg->s2cfg.tsz >= MAX(64 - oas2bits(SMMU_IDR5_OAS), 16));
+}
+
+/*
+ * Return true if s2 page table config is valid.
+ * This checks with the configured start level, ias_bits and granularity we can
+ * have a valid page table as described in ARM ARM D8.2 Translation process.
+ * The idea here is to see for the highest possible number of IPA bits, how
+ * many concatenated tables we would need, if it is more than 16, then this is
+ * not possible.
+ */
+static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran)
+{
+ int level = get_start_level(sl0, gran);
+ uint64_t ipa_bits = 64 - t0sz;
+ uint64_t max_ipa = (1ULL << ipa_bits) - 1;
+ int nr_concat = pgd_concat_idx(level, gran, max_ipa) + 1;
+
+ return nr_concat <= VMSA_MAX_S2_CONCAT;
+}
+
+static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
+{
+ cfg->stage = 2;
+
+ if (STE_S2AA64(ste) == 0x0) {
+ qemu_log_mask(LOG_UNIMP,
+ "SMMUv3 AArch32 tables not supported\n");
+ g_assert_not_reached();
+ }
+
+ switch (STE_S2TG(ste)) {
+ case 0x0: /* 4KB */
+ cfg->s2cfg.granule_sz = 12;
+ break;
+ case 0x1: /* 64KB */
+ cfg->s2cfg.granule_sz = 16;
+ break;
+ case 0x2: /* 16KB */
+ cfg->s2cfg.granule_sz = 14;
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "SMMUv3 bad STE S2TG: %x\n", STE_S2TG(ste));
+ goto bad_ste;
+ }
+
+ cfg->s2cfg.vttb = STE_S2TTB(ste);
+
+ cfg->s2cfg.sl0 = STE_S2SL0(ste);
+ /* FEAT_TTST not supported. */
+ if (cfg->s2cfg.sl0 == 0x3) {
+ qemu_log_mask(LOG_UNIMP, "SMMUv3 S2SL0 = 0x3 has no meaning!\n");
+ goto bad_ste;
+ }
+
+ /* For AA64, The effective S2PS size is capped to the OAS. */
+ cfg->s2cfg.eff_ps = oas2bits(MIN(STE_S2PS(ste), SMMU_IDR5_OAS));
+ /*
+ * It is ILLEGAL for the address in S2TTB to be outside the range
+ * described by the effective S2PS value.
+ */
+ if (cfg->s2cfg.vttb & ~(MAKE_64BIT_MASK(0, cfg->s2cfg.eff_ps))) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "SMMUv3 S2TTB too large 0x%lx, effective PS %d bits\n",
+ cfg->s2cfg.vttb, cfg->s2cfg.eff_ps);
+ goto bad_ste;
+ }
+
+ cfg->s2cfg.tsz = STE_S2T0SZ(ste);
+
+ if (!s2t0sz_valid(cfg)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "SMMUv3 bad STE S2T0SZ = %d\n",
+ cfg->s2cfg.tsz);
+ goto bad_ste;
+ }
+
+ if (!s2_pgtable_config_valid(cfg->s2cfg.sl0, cfg->s2cfg.tsz,
+ cfg->s2cfg.granule_sz)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "SMMUv3 STE stage 2 config not valid!\n");
+ goto bad_ste;
+ }
+
+ /* Only LE supported(IDR0.TTENDIAN). */
+ if (STE_S2ENDI(ste)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "SMMUv3 STE_S2ENDI only supports LE!\n");
+ goto bad_ste;
+ }
+
+ cfg->s2cfg.affd = STE_S2AFFD(ste);
+
+ cfg->s2cfg.record_faults = STE_S2R(ste);
+ /* As stall is not supported. */
+ if (STE_S2S(ste)) {
+ qemu_log_mask(LOG_UNIMP, "SMMUv3 Stall not implemented!\n");
+ goto bad_ste;
+ }
+
+ /* This is still here as stage 2 has not been fully enabled yet. */
+ qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
+ goto bad_ste;
+
+ return 0;
+
+bad_ste:
+ return -EINVAL;
+}
+
/* Returns < 0 in case of invalid STE, 0 otherwise */
static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
STE *ste, SMMUEventInfo *event)
{
uint32_t config;
+ int ret;
if (!STE_VALID(ste)) {
if (!event->inval_ste_allowed) {
@@ -354,11 +494,39 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
return 0;
}
- if (STE_CFG_S2_ENABLED(config)) {
- qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
+ /*
+ * If a stage is enabled in SW while not advertised, throw bad ste
+ * according to user manual(IHI0070E) "5.2 Stream Table Entry".
+ */
+ if (!STAGE1_SUPPORTED(s) && STE_CFG_S1_ENABLED(config)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "SMMUv3 S1 used but not supported.\n");
+ goto bad_ste;
+ }
+ if (!STAGE2_SUPPORTED(s) && STE_CFG_S2_ENABLED(config)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "SMMUv3 S2 used but not supported.\n");
goto bad_ste;
}
+ if (STAGE2_SUPPORTED(s)) {
+ /* VMID is considered even if s2 is disabled. */
+ cfg->s2cfg.vmid = STE_S2VMID(ste);
+ } else {
+ /* Default to -1 */
+ cfg->s2cfg.vmid = -1;
+ }
+
+ if (STE_CFG_S2_ENABLED(config)) {
+ /*
+ * Stage-1 OAS defaults to OAS even if not enabled as it would be used
+ * in input address check for stage-2.
+ */
+ cfg->oas = oas2bits(SMMU_IDR5_OAS);
+ ret = decode_ste_s2_cfg(cfg, ste);
+ if (ret) {
+ goto bad_ste;
+ }
+ }
+
if (STE_S1CDMAX(ste) != 0) {
qemu_log_mask(LOG_UNIMP,
"SMMUv3 does not support multiple context descriptors yet\n");
@@ -702,7 +870,13 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
if (cached_entry) {
if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
status = SMMU_TRANS_ERROR;
- if (cfg->record_faults) {
+ /*
+ * We know that the TLB only contains either stage-1 or stage-2 as
+ * nesting is not supported. So it is sufficient to check the
+ * translation stage to know the TLB stage for now.
+ */
+ event.u.f_walk_eabt.s2 = (cfg->stage == 2);
+ if (PTW_FAULT_ALLOWED(cfg, event)) {
event.type = SMMU_EVT_F_PERMISSION;
event.u.f_permission.addr = addr;
event.u.f_permission.rnw = flag & 0x1;
@@ -728,28 +902,28 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
event.u.f_walk_eabt.addr2 = ptw_info.addr;
break;
case SMMU_PTW_ERR_TRANSLATION:
- if (cfg->record_faults) {
+ if (PTW_FAULT_ALLOWED(cfg, event)) {
event.type = SMMU_EVT_F_TRANSLATION;
event.u.f_translation.addr = addr;
event.u.f_translation.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_ADDR_SIZE:
- if (cfg->record_faults) {
+ if (PTW_FAULT_ALLOWED(cfg, event)) {
event.type = SMMU_EVT_F_ADDR_SIZE;
event.u.f_addr_size.addr = addr;
event.u.f_addr_size.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_ACCESS:
- if (cfg->record_faults) {
+ if (PTW_FAULT_ALLOWED(cfg, event)) {
event.type = SMMU_EVT_F_ACCESS;
event.u.f_access.addr = addr;
event.u.f_access.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_PERMISSION:
- if (cfg->record_faults) {
+ if (PTW_FAULT_ALLOWED(cfg, event)) {
event.type = SMMU_EVT_F_PERMISSION;
event.u.f_permission.addr = addr;
event.u.f_permission.rnw = flag & 0x1;
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 97cea8ea06..4f1405d4e4 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -29,6 +29,7 @@
/* VMSAv8-64 Translation constants and functions */
#define VMSA_LEVELS 4
+#define VMSA_MAX_S2_CONCAT 16
#define VMSA_STRIDE(gran) ((gran) - VMSA_LEVELS + 1)
#define VMSA_BIT_LVL(isz, strd, lvl) ((isz) - (strd) * \
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index a0c026402e..6031d7d325 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -83,4 +83,7 @@ struct SMMUv3Class {
#define TYPE_ARM_SMMUV3 "arm-smmuv3"
OBJECT_DECLARE_TYPE(SMMUv3State, SMMUv3Class, ARM_SMMUV3)
+#define STAGE1_SUPPORTED(s) FIELD_EX32(s->idr[0], IDR0, S1P)
+#define STAGE2_SUPPORTED(s) FIELD_EX32(s->idr[0], IDR0, S2P)
+
#endif
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 06/10] hw/arm/smmuv3: Make TLB lookup work for stage-2
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (4 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config " Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 07/10] hw/arm/smmuv3: Add VMID to TLB tagging Mostafa Saleh
` (4 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
Right now, either stage-1 or stage-2 are supported, this simplifies
how we can deal with TLBs.
This patch makes TLB lookup work if stage-2 is enabled instead of
stage-1.
TLB lookup is done before a PTW, if a valid entry is found we won't
do the PTW.
To be able to do TLB lookup, we need the correct tagging info, as
granularity and input size, so we get this based on the supported
translation stage. The TLB entries are added correctly from each
stage PTW.
When nested translation is supported, this would need to change, for
example if we go with a combined TLB implementation, we would need to
use the min of the granularities in TLB.
As stage-2 shouldn't be tagged by ASID, it will be set to -1 if S1P
is not enabled.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
Changes in v3:
- Rename temp to tt_combined and move to top.
- Collected Reviewed-by tag.
Changes in v2:
- check if S1 is enabled(not supported) when reading S1 TT.
---
hw/arm/smmuv3.c | 44 +++++++++++++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 11 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 0f5429aed8..a1f4a4f902 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -727,6 +727,9 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg,
STE ste;
CD cd;
+ /* ASID defaults to -1 (if s1 is not supported). */
+ cfg->asid = -1;
+
ret = smmu_find_ste(s, sid, &ste, event);
if (ret) {
return ret;
@@ -824,6 +827,11 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
.addr_mask = ~(hwaddr)0,
.perm = IOMMU_NONE,
};
+ /*
+ * Combined attributes used for TLB lookup, as only one stage is supported,
+ * it will hold attributes based on the enabled stage.
+ */
+ SMMUTransTableInfo tt_combined;
qemu_mutex_lock(&s->mutex);
@@ -852,21 +860,35 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
goto epilogue;
}
- tt = select_tt(cfg, addr);
- if (!tt) {
- if (cfg->record_faults) {
- event.type = SMMU_EVT_F_TRANSLATION;
- event.u.f_translation.addr = addr;
- event.u.f_translation.rnw = flag & 0x1;
+ if (cfg->stage == 1) {
+ /* Select stage1 translation table. */
+ tt = select_tt(cfg, addr);
+ if (!tt) {
+ if (cfg->record_faults) {
+ event.type = SMMU_EVT_F_TRANSLATION;
+ event.u.f_translation.addr = addr;
+ event.u.f_translation.rnw = flag & 0x1;
+ }
+ status = SMMU_TRANS_ERROR;
+ goto epilogue;
}
- status = SMMU_TRANS_ERROR;
- goto epilogue;
- }
+ tt_combined.granule_sz = tt->granule_sz;
+ tt_combined.tsz = tt->tsz;
- page_mask = (1ULL << (tt->granule_sz)) - 1;
+ } else {
+ /* Stage2. */
+ tt_combined.granule_sz = cfg->s2cfg.granule_sz;
+ tt_combined.tsz = cfg->s2cfg.tsz;
+ }
+ /*
+ * TLB lookup looks for granule and input size for a translation stage,
+ * as only one stage is supported right now, choose the right values
+ * from the configuration.
+ */
+ page_mask = (1ULL << tt_combined.granule_sz) - 1;
aligned_addr = addr & ~page_mask;
- cached_entry = smmu_iotlb_lookup(bs, cfg, tt, aligned_addr);
+ cached_entry = smmu_iotlb_lookup(bs, cfg, &tt_combined, aligned_addr);
if (cached_entry) {
if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
status = SMMU_TRANS_ERROR;
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 07/10] hw/arm/smmuv3: Add VMID to TLB tagging
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (5 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 06/10] hw/arm/smmuv3: Make TLB lookup work " Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2 Mostafa Saleh
` (3 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
Allow TLB to be tagged with VMID.
If stage-1 is only supported, VMID is set to -1 and ignored from STE
and CMD_TLBI_NH* cmds.
Update smmu_iotlb_insert trace event to have vmid.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
Changes in v3:
- Collected Reviewed-by tag.
Changes in v2:
-Fix TLB aliasing issue from missing check in smmu_iotlb_key_equal.
-Add vmid to traces smmu_iotlb_insert and smmu_iotlb_lookup_hit/miss.
-Add vmid to hash function.
---
hw/arm/smmu-common.c | 36 ++++++++++++++++++++++--------------
hw/arm/smmu-internal.h | 2 ++
hw/arm/smmuv3.c | 12 +++++++++---
hw/arm/trace-events | 6 +++---
include/hw/arm/smmu-common.h | 5 +++--
5 files changed, 39 insertions(+), 22 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index a4ebd004c5..72ed6edd48 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -38,7 +38,7 @@ static guint smmu_iotlb_key_hash(gconstpointer v)
/* Jenkins hash */
a = b = c = JHASH_INITVAL + sizeof(*key);
- a += key->asid + key->level + key->tg;
+ a += key->asid + key->vmid + key->level + key->tg;
b += extract64(key->iova, 0, 32);
c += extract64(key->iova, 32, 32);
@@ -53,13 +53,15 @@ static gboolean smmu_iotlb_key_equal(gconstpointer v1, gconstpointer v2)
SMMUIOTLBKey *k1 = (SMMUIOTLBKey *)v1, *k2 = (SMMUIOTLBKey *)v2;
return (k1->asid == k2->asid) && (k1->iova == k2->iova) &&
- (k1->level == k2->level) && (k1->tg == k2->tg);
+ (k1->level == k2->level) && (k1->tg == k2->tg) &&
+ (k1->vmid == k2->vmid);
}
-SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint64_t iova,
+SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
uint8_t tg, uint8_t level)
{
- SMMUIOTLBKey key = {.asid = asid, .iova = iova, .tg = tg, .level = level};
+ SMMUIOTLBKey key = {.asid = asid, .vmid = vmid, .iova = iova,
+ .tg = tg, .level = level};
return key;
}
@@ -78,7 +80,8 @@ SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
uint64_t mask = subpage_size - 1;
SMMUIOTLBKey key;
- key = smmu_get_iotlb_key(cfg->asid, iova & ~mask, tg, level);
+ key = smmu_get_iotlb_key(cfg->asid, cfg->s2cfg.vmid,
+ iova & ~mask, tg, level);
entry = g_hash_table_lookup(bs->iotlb, &key);
if (entry) {
break;
@@ -88,13 +91,13 @@ SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
if (entry) {
cfg->iotlb_hits++;
- trace_smmu_iotlb_lookup_hit(cfg->asid, iova,
+ trace_smmu_iotlb_lookup_hit(cfg->asid, cfg->s2cfg.vmid, iova,
cfg->iotlb_hits, cfg->iotlb_misses,
100 * cfg->iotlb_hits /
(cfg->iotlb_hits + cfg->iotlb_misses));
} else {
cfg->iotlb_misses++;
- trace_smmu_iotlb_lookup_miss(cfg->asid, iova,
+ trace_smmu_iotlb_lookup_miss(cfg->asid, cfg->s2cfg.vmid, iova,
cfg->iotlb_hits, cfg->iotlb_misses,
100 * cfg->iotlb_hits /
(cfg->iotlb_hits + cfg->iotlb_misses));
@@ -111,8 +114,10 @@ void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg, SMMUTLBEntry *new)
smmu_iotlb_inv_all(bs);
}
- *key = smmu_get_iotlb_key(cfg->asid, new->entry.iova, tg, new->level);
- trace_smmu_iotlb_insert(cfg->asid, new->entry.iova, tg, new->level);
+ *key = smmu_get_iotlb_key(cfg->asid, cfg->s2cfg.vmid, new->entry.iova,
+ tg, new->level);
+ trace_smmu_iotlb_insert(cfg->asid, cfg->s2cfg.vmid, new->entry.iova,
+ tg, new->level);
g_hash_table_insert(bs->iotlb, key, new);
}
@@ -130,8 +135,7 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
return SMMU_IOTLB_ASID(*iotlb_key) == asid;
}
-
-static gboolean smmu_hash_remove_by_asid_iova(gpointer key, gpointer value,
+static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
gpointer user_data)
{
SMMUTLBEntry *iter = (SMMUTLBEntry *)value;
@@ -142,18 +146,21 @@ static gboolean smmu_hash_remove_by_asid_iova(gpointer key, gpointer value,
if (info->asid >= 0 && info->asid != SMMU_IOTLB_ASID(iotlb_key)) {
return false;
}
+ if (info->vmid >= 0 && info->vmid != SMMU_IOTLB_VMID(iotlb_key)) {
+ return false;
+ }
return ((info->iova & ~entry->addr_mask) == entry->iova) ||
((entry->iova & ~info->mask) == info->iova);
}
-void smmu_iotlb_inv_iova(SMMUState *s, int asid, dma_addr_t iova,
+void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
uint8_t tg, uint64_t num_pages, uint8_t ttl)
{
/* if tg is not set we use 4KB range invalidation */
uint8_t granule = tg ? tg * 2 + 10 : 12;
if (ttl && (num_pages == 1) && (asid >= 0)) {
- SMMUIOTLBKey key = smmu_get_iotlb_key(asid, iova, tg, ttl);
+ SMMUIOTLBKey key = smmu_get_iotlb_key(asid, vmid, iova, tg, ttl);
if (g_hash_table_remove(s->iotlb, &key)) {
return;
@@ -166,10 +173,11 @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, dma_addr_t iova,
SMMUIOTLBPageInvInfo info = {
.asid = asid, .iova = iova,
+ .vmid = vmid,
.mask = (num_pages * 1 << granule) - 1};
g_hash_table_foreach_remove(s->iotlb,
- smmu_hash_remove_by_asid_iova,
+ smmu_hash_remove_by_asid_vmid_iova,
&info);
}
diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
index a9454f914e..843bebb185 100644
--- a/hw/arm/smmu-internal.h
+++ b/hw/arm/smmu-internal.h
@@ -132,9 +132,11 @@ static inline int pgd_concat_idx(int start_level, int granule_sz,
}
#define SMMU_IOTLB_ASID(key) ((key).asid)
+#define SMMU_IOTLB_VMID(key) ((key).vmid)
typedef struct SMMUIOTLBPageInvInfo {
int asid;
+ int vmid;
uint64_t iova;
uint64_t mask;
} SMMUIOTLBPageInvInfo;
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index a1f4a4f902..d7e7003da9 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1073,7 +1073,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
{
dma_addr_t end, addr = CMD_ADDR(cmd);
uint8_t type = CMD_TYPE(cmd);
- uint16_t vmid = CMD_VMID(cmd);
+ int vmid = -1;
uint8_t scale = CMD_SCALE(cmd);
uint8_t num = CMD_NUM(cmd);
uint8_t ttl = CMD_TTL(cmd);
@@ -1082,6 +1082,12 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
uint64_t num_pages;
uint8_t granule;
int asid = -1;
+ SMMUv3State *smmuv3 = ARM_SMMUV3(s);
+
+ /* Only consider VMID if stage-2 is supported. */
+ if (STAGE2_SUPPORTED(smmuv3)) {
+ vmid = CMD_VMID(cmd);
+ }
if (type == SMMU_CMD_TLBI_NH_VA) {
asid = CMD_ASID(cmd);
@@ -1090,7 +1096,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
if (!tg) {
trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
- smmu_iotlb_inv_iova(s, asid, addr, tg, 1, ttl);
+ smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
return;
}
@@ -1108,7 +1114,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
num_pages = (mask + 1) >> granule;
trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
- smmu_iotlb_inv_iova(s, asid, addr, tg, num_pages, ttl);
+ smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
addr += mask + 1;
}
}
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 205ac04573..705104e58b 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -14,9 +14,9 @@ smmu_iotlb_inv_all(void) "IOTLB invalidate all"
smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d"
smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64
smmu_inv_notifiers_mr(const char *name) "iommu mr=%s"
-smmu_iotlb_lookup_hit(uint16_t asid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache HIT asid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d"
-smmu_iotlb_lookup_miss(uint16_t asid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache MISS asid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d"
-smmu_iotlb_insert(uint16_t asid, uint64_t addr, uint8_t tg, uint8_t level) "IOTLB ++ asid=%d addr=0x%"PRIx64" tg=%d level=%d"
+smmu_iotlb_lookup_hit(uint16_t asid, uint16_t vmid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache HIT asid=%d vmid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d"
+smmu_iotlb_lookup_miss(uint16_t asid, uint16_t vmid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache MISS asid=%d vmid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d"
+smmu_iotlb_insert(uint16_t asid, uint16_t vmid, uint64_t addr, uint8_t tg, uint8_t level) "IOTLB ++ asid=%d vmid=%d addr=0x%"PRIx64" tg=%d level=%d"
# smmuv3.c
smmuv3_read_mmio(uint64_t addr, uint64_t val, unsigned size, uint32_t r) "addr: 0x%"PRIx64" val:0x%"PRIx64" size: 0x%x(%d)"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 4f1405d4e4..3cbb4998ad 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -125,6 +125,7 @@ typedef struct SMMUPciBus {
typedef struct SMMUIOTLBKey {
uint64_t iova;
uint16_t asid;
+ uint16_t vmid;
uint8_t tg;
uint8_t level;
} SMMUIOTLBKey;
@@ -188,11 +189,11 @@ IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t sid);
SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
SMMUTransTableInfo *tt, hwaddr iova);
void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg, SMMUTLBEntry *entry);
-SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint64_t iova,
+SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
uint8_t tg, uint8_t level);
void smmu_iotlb_inv_all(SMMUState *s);
void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid);
-void smmu_iotlb_inv_iova(SMMUState *s, int asid, dma_addr_t iova,
+void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
uint8_t tg, uint64_t num_pages, uint8_t ttl);
/* Unmap the range of all the notifiers registered to any IOMMU mr */
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (6 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 07/10] hw/arm/smmuv3: Add VMID to TLB tagging Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-05-15 13:17 ` Eric Auger
2023-05-15 14:14 ` Eric Auger
2023-04-01 10:49 ` [RFC PATCH v3 09/10] hw/arm/smmuv3: Add stage-2 support in iova notifier Mostafa Saleh
` (2 subsequent siblings)
10 siblings, 2 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
CMD_TLBI_S2_IPA: As S1+S2 is not enabled, for now this can be the
same as CMD_TLBI_NH_VAA.
CMD_TLBI_S12_VMALL: Added new function to invalidate TLB by VMID.
For stage-1 only commands, add a check to throw CERROR_ILL if used
when stage-1 is not supported.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in v3:
- Log guest error for all illegal commands.
Changes in v2:
- Add checks for stage-1 only commands
- Rename smmuv3_s1_range_inval to smmuv3_range_inval
---
hw/arm/smmu-common.c | 16 +++++++++++
hw/arm/smmuv3.c | 53 ++++++++++++++++++++++++++++++------
hw/arm/trace-events | 4 ++-
include/hw/arm/smmu-common.h | 1 +
4 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 72ed6edd48..45e9d7e752 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -135,6 +135,16 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
return SMMU_IOTLB_ASID(*iotlb_key) == asid;
}
+
+static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
+ gpointer user_data)
+{
+ uint16_t vmid = *(uint16_t *)user_data;
+ SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
+
+ return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
+}
+
static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
gpointer user_data)
{
@@ -187,6 +197,12 @@ void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
}
+inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
+{
+ trace_smmu_iotlb_inv_vmid(vmid);
+ g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
+}
+
/* VMSAv8-64 Translation */
/**
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index d7e7003da9..3b5b1fad1a 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1069,7 +1069,7 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
}
}
-static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
+static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
{
dma_addr_t end, addr = CMD_ADDR(cmd);
uint8_t type = CMD_TYPE(cmd);
@@ -1094,7 +1094,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
}
if (!tg) {
- trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
+ trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
return;
@@ -1112,7 +1112,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
uint64_t mask = dma_aligned_pow2_mask(addr, end, 64);
num_pages = (mask + 1) >> granule;
- trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
+ trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
addr += mask + 1;
@@ -1246,12 +1246,22 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
{
uint16_t asid = CMD_ASID(&cmd);
+ if (!STAGE1_SUPPORTED(s)) {
+ cmd_error = SMMU_CERROR_ILL;
+ break;
+ }
+
trace_smmuv3_cmdq_tlbi_nh_asid(asid);
smmu_inv_notifiers_all(&s->smmu_state);
smmu_iotlb_inv_asid(bs, asid);
break;
}
case SMMU_CMD_TLBI_NH_ALL:
+ if (!STAGE1_SUPPORTED(s)) {
+ cmd_error = SMMU_CERROR_ILL;
+ break;
+ }
+ QEMU_FALLTHROUGH;
case SMMU_CMD_TLBI_NSNH_ALL:
trace_smmuv3_cmdq_tlbi_nh();
smmu_inv_notifiers_all(&s->smmu_state);
@@ -1259,7 +1269,34 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
break;
case SMMU_CMD_TLBI_NH_VAA:
case SMMU_CMD_TLBI_NH_VA:
- smmuv3_s1_range_inval(bs, &cmd);
+ if (!STAGE1_SUPPORTED(s)) {
+ cmd_error = SMMU_CERROR_ILL;
+ break;
+ }
+ smmuv3_range_inval(bs, &cmd);
+ break;
+ case SMMU_CMD_TLBI_S12_VMALL:
+ uint16_t vmid = CMD_VMID(&cmd);
+
+ if (!STAGE2_SUPPORTED(s)) {
+ cmd_error = SMMU_CERROR_ILL;
+ break;
+ }
+
+ trace_smmuv3_cmdq_tlbi_s12_vmid(vmid);
+ smmu_inv_notifiers_all(&s->smmu_state);
+ smmu_iotlb_inv_vmid(bs, vmid);
+ break;
+ case SMMU_CMD_TLBI_S2_IPA:
+ if (!STAGE2_SUPPORTED(s)) {
+ cmd_error = SMMU_CERROR_ILL;
+ break;
+ }
+ /*
+ * As currently only either s1 or s2 are supported
+ * we can reuse same function for s2.
+ */
+ smmuv3_range_inval(bs, &cmd);
break;
case SMMU_CMD_TLBI_EL3_ALL:
case SMMU_CMD_TLBI_EL3_VA:
@@ -1267,8 +1304,6 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
case SMMU_CMD_TLBI_EL2_ASID:
case SMMU_CMD_TLBI_EL2_VA:
case SMMU_CMD_TLBI_EL2_VAA:
- case SMMU_CMD_TLBI_S12_VMALL:
- case SMMU_CMD_TLBI_S2_IPA:
case SMMU_CMD_ATC_INV:
case SMMU_CMD_PRI_RESP:
case SMMU_CMD_RESUME:
@@ -1277,12 +1312,14 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
break;
default:
cmd_error = SMMU_CERROR_ILL;
- qemu_log_mask(LOG_GUEST_ERROR,
- "Illegal command type: %d\n", CMD_TYPE(&cmd));
break;
}
qemu_mutex_unlock(&s->mutex);
if (cmd_error) {
+ if (cmd_error == SMMU_CERROR_ILL) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Illegal command type: %d\n", CMD_TYPE(&cmd));
+ }
break;
}
/*
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 705104e58b..f8fdf1ca9f 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -12,6 +12,7 @@ smmu_ptw_block_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, ui
smmu_get_pte(uint64_t baseaddr, int index, uint64_t pteaddr, uint64_t pte) "baseaddr=0x%"PRIx64" index=0x%x, pteaddr=0x%"PRIx64", pte=0x%"PRIx64
smmu_iotlb_inv_all(void) "IOTLB invalidate all"
smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d"
+smmu_iotlb_inv_vmid(uint16_t vmid) "IOTLB invalidate vmid=%d"
smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64
smmu_inv_notifiers_mr(const char *name) "iommu mr=%s"
smmu_iotlb_lookup_hit(uint16_t asid, uint16_t vmid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache HIT asid=%d vmid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d"
@@ -45,9 +46,10 @@ smmuv3_cmdq_cfgi_ste_range(int start, int end) "start=0x%x - end=0x%x"
smmuv3_cmdq_cfgi_cd(uint32_t sid) "sid=0x%x"
smmuv3_config_cache_hit(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache HIT for sid=0x%x (hits=%d, misses=%d, hit rate=%d)"
smmuv3_config_cache_miss(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache MISS for sid=0x%x (hits=%d, misses=%d, hit rate=%d)"
-smmuv3_s1_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t num_pages, uint8_t ttl, bool leaf) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d"
+smmuv3_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t num_pages, uint8_t ttl, bool leaf) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d"
smmuv3_cmdq_tlbi_nh(void) ""
smmuv3_cmdq_tlbi_nh_asid(uint16_t asid) "asid=%d"
+smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu mr=%s"
smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu mr=%s"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 3cbb4998ad..fd8d772da1 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -193,6 +193,7 @@ SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
uint8_t tg, uint8_t level);
void smmu_iotlb_inv_all(SMMUState *s);
void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid);
+void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid);
void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
uint8_t tg, uint64_t num_pages, uint8_t ttl);
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 09/10] hw/arm/smmuv3: Add stage-2 support in iova notifier
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (7 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2 Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 10/10] hw/arm/smmuv3: Add knob to choose translation stage and enable stage-2 Mostafa Saleh
2023-05-12 14:46 ` [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Peter Maydell
10 siblings, 0 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
In smmuv3_notify_iova, read the granule based on translation stage
and use VMID if valid value is sent.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
Changes in v3:
- Collected Reviewed-by tag.
---
hw/arm/smmuv3.c | 39 ++++++++++++++++++++++++++-------------
hw/arm/trace-events | 2 +-
2 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 3b5b1fad1a..826aacf8b1 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1006,18 +1006,21 @@ epilogue:
* @mr: IOMMU mr region handle
* @n: notifier to be called
* @asid: address space ID or negative value if we don't care
+ * @vmid: virtual machine ID or negative value if we don't care
* @iova: iova
* @tg: translation granule (if communicated through range invalidation)
* @num_pages: number of @granule sized pages (if tg != 0), otherwise 1
*/
static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
IOMMUNotifier *n,
- int asid, dma_addr_t iova,
- uint8_t tg, uint64_t num_pages)
+ int asid, int vmid,
+ dma_addr_t iova, uint8_t tg,
+ uint64_t num_pages)
{
SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
IOMMUTLBEvent event;
uint8_t granule;
+ SMMUv3State *s = sdev->smmu;
if (!tg) {
SMMUEventInfo event = {.inval_ste_allowed = true};
@@ -1032,11 +1035,20 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
return;
}
- tt = select_tt(cfg, iova);
- if (!tt) {
+ if (vmid >= 0 && cfg->s2cfg.vmid != vmid) {
return;
}
- granule = tt->granule_sz;
+
+ if (STAGE1_SUPPORTED(s)) {
+ tt = select_tt(cfg, iova);
+ if (!tt) {
+ return;
+ }
+ granule = tt->granule_sz;
+ } else {
+ granule = cfg->s2cfg.granule_sz;
+ }
+
} else {
granule = tg * 2 + 10;
}
@@ -1050,9 +1062,10 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
memory_region_notify_iommu_one(n, &event);
}
-/* invalidate an asid/iova range tuple in all mr's */
-static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
- uint8_t tg, uint64_t num_pages)
+/* invalidate an asid/vmid/iova range tuple in all mr's */
+static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, int vmid,
+ dma_addr_t iova, uint8_t tg,
+ uint64_t num_pages)
{
SMMUDevice *sdev;
@@ -1060,11 +1073,11 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
IOMMUMemoryRegion *mr = &sdev->iommu;
IOMMUNotifier *n;
- trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, iova,
- tg, num_pages);
+ trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, vmid,
+ iova, tg, num_pages);
IOMMU_NOTIFIER_FOREACH(n, mr) {
- smmuv3_notify_iova(mr, n, asid, iova, tg, num_pages);
+ smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages);
}
}
}
@@ -1095,7 +1108,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
if (!tg) {
trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
- smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
+ smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
return;
}
@@ -1113,7 +1126,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
num_pages = (mask + 1) >> granule;
trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
- smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
+ smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
addr += mask + 1;
}
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index f8fdf1ca9f..cdc1ea06a8 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -53,5 +53,5 @@ smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu mr=%s"
smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu mr=%s"
-smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint64_t iova, uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64
+smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint16_t vmid, uint64_t iova, uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d vmid=%d iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [RFC PATCH v3 10/10] hw/arm/smmuv3: Add knob to choose translation stage and enable stage-2
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (8 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 09/10] hw/arm/smmuv3: Add stage-2 support in iova notifier Mostafa Saleh
@ 2023-04-01 10:49 ` Mostafa Saleh
2023-05-15 13:15 ` Eric Auger
2023-05-12 14:46 ` [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Peter Maydell
10 siblings, 1 reply; 26+ messages in thread
From: Mostafa Saleh @ 2023-04-01 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: jean-philippe, eric.auger, peter.maydell, qemu-arm,
richard.henderson, Mostafa Saleh
As everything is in place, we can use a new system property to
advertise which stage is supported and remove bad_ste from STE
stage2 config.
The property added arm-smmuv3.stage can have 3 values:
- "1": Stage-1 only is advertised.
- "2": Stage-2 only is advertised.
- "all": Stage-1 + Stage-2 are supported, which is not implemented in
this patch series.
If not passed or an unsupported value is passed, it will default to
stage-1.
Advertise VMID16.
Don't try to decode CD, if stage-2 is configured.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in v2:
- Squash knob patch with stage-2 enable patch.
- Don't try to decode CD, if stage-2 is configured.
---
hw/arm/smmuv3.c | 34 +++++++++++++++++++++++++---------
include/hw/arm/smmuv3.h | 1 +
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 826aacf8b1..22b5613d4c 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -21,6 +21,7 @@
#include "hw/irq.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
#include "hw/qdev-core.h"
#include "hw/pci/pci.h"
#include "cpu.h"
@@ -248,14 +249,20 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *info)
static void smmuv3_init_regs(SMMUv3State *s)
{
- /**
- * IDR0: stage1 only, AArch64 only, coherent access, 16b ASID,
- * multi-level stream table
+ /*
+ * Based on sys property, the stages supported in smmu will be advertised.
+ * At the moment "all" is not supported and default to stage-1.
*/
- s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1); /* stage 1 supported */
+ if (s->stage && !strcmp("2", s->stage)) {
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S2P, 1);
+ } else {
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1);
+ }
+
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, TTF, 2); /* AArch64 PTW only */
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, COHACC, 1); /* IO coherent */
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ASID16, 1); /* 16-bit ASID */
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, VMID16, 1); /* 16-bit VMID */
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, TTENDIAN, 2); /* little endian */
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, STALL_MODEL, 1); /* No stall */
/* terminated transaction will always be aborted/error returned */
@@ -458,10 +465,6 @@ static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
goto bad_ste;
}
- /* This is still here as stage 2 has not been fully enabled yet. */
- qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
- goto bad_ste;
-
return 0;
bad_ste:
@@ -740,7 +743,7 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg,
return ret;
}
- if (cfg->aborted || cfg->bypassed) {
+ if (cfg->aborted || cfg->bypassed || (cfg->stage == 2)) {
return 0;
}
@@ -1809,6 +1812,18 @@ static const VMStateDescription vmstate_smmuv3 = {
}
};
+static Property smmuv3_properties[] = {
+ /*
+ * Stages of translation advertised.
+ * "1": Stage 1
+ * "2": Stage 2
+ * "all": Stage 1 + Stage 2
+ * Defaults to stage 1
+ */
+ DEFINE_PROP_STRING("stage", SMMUv3State, stage),
+ DEFINE_PROP_END_OF_LIST()
+};
+
static void smmuv3_instance_init(Object *obj)
{
/* Nothing much to do here as of now */
@@ -1825,6 +1840,7 @@ static void smmuv3_class_init(ObjectClass *klass, void *data)
&c->parent_phases);
c->parent_realize = dc->realize;
dc->realize = smmu_realize;
+ device_class_set_props(dc, smmuv3_properties);
}
static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index 6031d7d325..d183a62766 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -62,6 +62,7 @@ struct SMMUv3State {
qemu_irq irq[4];
QemuMutex mutex;
+ char *stage;
};
typedef enum {
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
` (9 preceding siblings ...)
2023-04-01 10:49 ` [RFC PATCH v3 10/10] hw/arm/smmuv3: Add knob to choose translation stage and enable stage-2 Mostafa Saleh
@ 2023-05-12 14:46 ` Peter Maydell
2023-05-12 15:26 ` Eric Auger
2023-05-15 13:03 ` Mostafa Saleh
10 siblings, 2 replies; 26+ messages in thread
From: Peter Maydell @ 2023-05-12 14:46 UTC (permalink / raw)
To: Mostafa Saleh
Cc: qemu-devel, jean-philippe, eric.auger, qemu-arm, richard.henderson
On Sat, 1 Apr 2023 at 11:49, Mostafa Saleh <smostafa@google.com> wrote:
>
> This patch series adds stage-2 translation support for SMMUv3. It is
> controlled by a new system property “arm-smmuv3.stage”.
> - When set to “1”: Stage-1 only would be advertised and supported (default
> behaviour)
> - When set to “2”: Stage-2 only would be advertised and supported.
> - Value “all” is reserved for nesting support. However it is not
> implemented in this patch series (more about this in the end)
>
> Features implemented in stage-2 are mostly synonymous with stage-1
> - VMID16.
> - Only AArch64 translation tables are supported.
> - Only little endian translation table supported.
> - Stall is not supported.
> - HTTU is not supported, SW is expected to maintain the Access flag.
Eric: are you planning to review this v3? I think only
patches 2, 4, 5, 8, 10 still need review.
Mostafa: is there anything in particular here that means this
patchset should stay an RFC and isn't ready to go into the tree?
thanks
-- PMM
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3
2023-05-12 14:46 ` [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Peter Maydell
@ 2023-05-12 15:26 ` Eric Auger
2023-05-15 13:03 ` Mostafa Saleh
1 sibling, 0 replies; 26+ messages in thread
From: Eric Auger @ 2023-05-12 15:26 UTC (permalink / raw)
To: Peter Maydell, Mostafa Saleh
Cc: qemu-devel, jean-philippe, qemu-arm, richard.henderson
Hi Peter,
On 5/12/23 16:46, Peter Maydell wrote:
> On Sat, 1 Apr 2023 at 11:49, Mostafa Saleh <smostafa@google.com> wrote:
>> This patch series adds stage-2 translation support for SMMUv3. It is
>> controlled by a new system property “arm-smmuv3.stage”.
>> - When set to “1”: Stage-1 only would be advertised and supported (default
>> behaviour)
>> - When set to “2”: Stage-2 only would be advertised and supported.
>> - Value “all” is reserved for nesting support. However it is not
>> implemented in this patch series (more about this in the end)
>>
>> Features implemented in stage-2 are mostly synonymous with stage-1
>> - VMID16.
>> - Only AArch64 translation tables are supported.
>> - Only little endian translation table supported.
>> - Stall is not supported.
>> - HTTU is not supported, SW is expected to maintain the Access flag.
> Eric: are you planning to review this v3? I think only
> patches 2, 4, 5, 8, 10 still need review.
yes I will do it beg of next week and will also test it against non
regression on S1 only.
Thanks
Eric
>
> Mostafa: is there anything in particular here that means this
> patchset should stay an RFC and isn't ready to go into the tree?
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 04/10] hw/arm/smmuv3: Add page table walk for stage-2
2023-04-01 10:49 ` [RFC PATCH v3 04/10] hw/arm/smmuv3: Add page table walk for stage-2 Mostafa Saleh
@ 2023-05-15 8:37 ` Eric Auger
0 siblings, 0 replies; 26+ messages in thread
From: Eric Auger @ 2023-05-15 8:37 UTC (permalink / raw)
To: Mostafa Saleh, qemu-devel
Cc: jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 4/1/23 12:49, Mostafa Saleh wrote:
> In preparation for adding stage-2 support, add Stage-2 PTW code.
> Only Aarch64 format is supported as stage-1.
>
> Nesting stage-1 and stage-2 is not supported right now.
>
> HTTU is not supported, SW is expected to maintain the Access flag.
> This is described in the SMMUv3 manual(IHI 0070.E)
nit the exact version might be E.a.
> "5.2. Stream Table Entry" in "[181] S2AFFD".
> This flag determines the behavior on access of a stage-2 page whose
> descriptor has AF == 0:
> - 0b0: An Access flag fault occurs (stall not supported).
> - 0b1: An Access flag fault never occurs.
> An Access fault takes priority over a Permission fault.
>
> There are 3 address size checks for stage-2 according to "IHI 0070.E"
> in "3.4. Address sizes".
> - As nesting is not supported, input address is passed directly to
> stage-2, and is checked against IAS.
> We use cfg->oas to hold the OAS when stage-1 is not used, this is set
> in the next patch.
> This check is done outside of smmu_ptw_64_s2 as it is not part of
> stage-2(it throws stage-1 fault), and the stage-2 function shouldn't
> change it's behavior when nesting is supported.
> When nesting is supported and we figure out how to combine TLB for
> stage-1 and stage-2 we can move this check into the stage-1 function
> as described in ARM DDI0487I.a in pseudocode
> aarch64/translation/vmsa_translation/AArch64.S1Translate
> aarch64/translation/vmsa_translation/AArch64.S1DisabledOutput
>
> - Input to stage-2 is checked against s2t0sz, and throws stage-2
> transaltion fault if exceeds it.
>
> - Output of stage-2 is checked against effective PA output range.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in v3:
> - Fix IPA address size check.
> - s2cfg.oas renamed to s2cfg.eff_ps.
> - s/iova/ipa
> - s/ap/s2ap
> - s/gran/granule_sz
> - use level_shift instead of inline code.
> - Add missing brackets in is_permission_fault_s2.
> - Use new VMSA_* macros and functions instead of SMMU_*
> - Rename pgd_idx to pgd_concat_idx.
> - Move SMMU_MAX_S2_CONCAT to STE patch as it is not used here.
> Changes in v2:
> - Squash S2AFF PTW code.
> - Use common functions between stage-1 and stage-2.
> - Add checks for IPA and out PA.
> ---
> hw/arm/smmu-common.c | 142 ++++++++++++++++++++++++++++++++++++++++-
> hw/arm/smmu-internal.h | 35 ++++++++++
> 2 files changed, 176 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 50391a8c94..a4ebd004c5 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -363,6 +363,127 @@ error:
> return -EINVAL;
> }
>
> +/**
> + * smmu_ptw_64_s2 - VMSAv8-64 Walk of the page tables for a given ipa
> + * for stage-2.
> + * @cfg: translation config
> + * @ipa: ipa to translate
> + * @perm: access type
> + * @tlbe: SMMUTLBEntry (out)
> + * @info: handle to an error info
> + *
> + * Return 0 on success, < 0 on error. In case of error, @info is filled
> + * and tlbe->perm is set to IOMMU_NONE.
> + * Upon success, @tlbe is filled with translated_addr and entry
> + * permission rights.
> + */
> +static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
> + dma_addr_t ipa, IOMMUAccessFlags perm,
> + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> +{
> + const int stage = 2;
> + int granule_sz = cfg->s2cfg.granule_sz;
> + /* ARM DDI0487I.a: Table D8-7. */
> + int inputsize = 64 - cfg->s2cfg.tsz;
> + int level = get_start_level(cfg->s2cfg.sl0, granule_sz);
> + int stride = VMSA_STRIDE(granule_sz);
> + int idx = pgd_concat_idx(level, granule_sz, ipa);
> + /*
> + * Get the ttb from concatenated structure.
> + * The offset is the idx * size of each ttb(number of ptes * (sizeof(pte))
> + */
> + uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, 48) + (1 << stride) *
> + idx * sizeof(uint64_t);
> + dma_addr_t indexmask = VMSA_IDXMSK(inputsize, stride, level);
> +
> + baseaddr &= ~indexmask;
> +
> + /*
> + * On input, a stage 2 Translation fault occurs if the IPA is outside the
> + * range configured by the relevant S2T0SZ field of the STE.
> + */
> + if (ipa >= (1ULL << inputsize)) {
> + info->type = SMMU_PTW_ERR_TRANSLATION;
> + goto error;
> + }
> +
> + while (level < VMSA_LEVELS) {
> + uint64_t subpage_size = 1ULL << level_shift(level, granule_sz);
> + uint64_t mask = subpage_size - 1;
> + uint32_t offset = iova_level_offset(ipa, inputsize, level, granule_sz);
> + uint64_t pte, gpa;
> + dma_addr_t pte_addr = baseaddr + offset * sizeof(pte);
> + uint8_t s2ap;
> +
> + if (get_pte(baseaddr, offset, &pte, info)) {
> + goto error;
> + }
> + trace_smmu_ptw_level(stage, level, ipa, subpage_size,
> + baseaddr, offset, pte);
> + if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
> + trace_smmu_ptw_invalid_pte(stage, level, baseaddr,
> + pte_addr, offset, pte);
> + break;
> + }
> +
> + if (is_table_pte(pte, level)) {
> + baseaddr = get_table_pte_address(pte, granule_sz);
> + level++;
> + continue;
> + } else if (is_page_pte(pte, level)) {
> + gpa = get_page_pte_address(pte, granule_sz);
> + trace_smmu_ptw_page_pte(stage, level, ipa,
> + baseaddr, pte_addr, pte, gpa);
> + } else {
> + uint64_t block_size;
> +
> + gpa = get_block_pte_address(pte, level, granule_sz,
> + &block_size);
> + trace_smmu_ptw_block_pte(stage, level, baseaddr,
> + pte_addr, pte, ipa, gpa,
> + block_size >> 20);
> + }
> +
> + /*
> + * If S2AFFD and PTE.AF are 0 => fault. (5.2. Stream Table Entry)
> + * An Access fault takes priority over a Permission fault.
> + */
> + if (!PTE_AF(pte) && !cfg->s2cfg.affd) {
> + info->type = SMMU_PTW_ERR_ACCESS;
> + goto error;
> + }
> +
> + s2ap = PTE_AP(pte);
> + if (is_permission_fault_s2(s2ap, perm)) {
> + info->type = SMMU_PTW_ERR_PERMISSION;
> + goto error;
> + }
> +
> + /*
> + * The address output from the translation causes a stage 2 Address
> + * Size fault if it exceeds the effective PA output range.
> + */
> + if (gpa >= (1ULL << cfg->s2cfg.eff_ps)) {
> + info->type = SMMU_PTW_ERR_ADDR_SIZE;
> + goto error;
> + }
> +
> + tlbe->entry.translated_addr = gpa;
> + tlbe->entry.iova = ipa & ~mask;
> + tlbe->entry.addr_mask = mask;
> + tlbe->entry.perm = s2ap;
> + tlbe->level = level;
> + tlbe->granule = granule_sz;
> + return 0;
> + }
> + info->type = SMMU_PTW_ERR_TRANSLATION;
> +
> +error:
> + info->stage = 2;
> + tlbe->entry.perm = IOMMU_NONE;
> + return -EINVAL;
> +}
> +
> /**
> * smmu_ptw - Walk the page tables for an IOVA, according to @cfg
> *
> @@ -377,7 +498,26 @@ error:
> int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
> + if (cfg->stage == 1) {
> + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
> + } else if (cfg->stage == 2) {
> + /*
> + * If bypassing stage 1(or unimplemented), the input address is passed
> + * directly to stage 2 as IPA. If the input address of a transaction
> + * exceeds the size of the IAS, a stage 1 Address Size fault occurs.
> + * For AA64, IAS = OAS according to (IHI 0070.E) "3.4 Address sizes"
> + */
> + if (iova >= (1ULL << cfg->oas)) {
> + info->type = SMMU_PTW_ERR_ADDR_SIZE;
> + info->stage = 1;
> + tlbe->entry.perm = IOMMU_NONE;
> + return -EINVAL;
> + }
> +
> + return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info);
> + }
> +
> + g_assert_not_reached();
> }
>
> /**
> diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
> index 2d75b31953..a9454f914e 100644
> --- a/hw/arm/smmu-internal.h
> +++ b/hw/arm/smmu-internal.h
> @@ -66,6 +66,8 @@
> #define PTE_APTABLE(pte) \
> (extract64(pte, 61, 2))
>
> +#define PTE_AF(pte) \
> + (extract64(pte, 10, 1))
> /*
> * TODO: At the moment all transactions are considered as privileged (EL1)
> * as IOMMU translation callback does not pass user/priv attributes.
> @@ -73,6 +75,9 @@
> #define is_permission_fault(ap, perm) \
> (((perm) & IOMMU_WO) && ((ap) & 0x2))
>
> +#define is_permission_fault_s2(s2ap, perm) \
> + (!(((s2ap) & (perm)) == (perm)))
> +
> #define PTE_AP_TO_PERM(ap) \
> (IOMMU_ACCESS_FLAG(true, !((ap) & 0x2)))
>
> @@ -96,6 +101,36 @@ uint64_t iova_level_offset(uint64_t iova, int inputsize,
> MAKE_64BIT_MASK(0, gsz - 3);
> }
>
> +/* FEAT_LPA2 and FEAT_TTST are not implemented. */
> +static inline int get_start_level(int sl0 , int granule_sz)
> +{
> + /* ARM DDI0487I.a: Table D8-12. */
> + if (granule_sz == 12) {
> + return 2 - sl0;
> + }
> + /* ARM DDI0487I.a: Table D8-22 and Table D8-31. */
> + return 3 - sl0;
> +}
> +
> +/*
> + * Index in a concatenated first level stage-2 page table.
> + * ARM DDI0487I.a: D8.2.2 Concatenated translation tables.
> + */
> +static inline int pgd_concat_idx(int start_level, int granule_sz,
> + dma_addr_t ipa)
> +{
> + uint64_t ret;
> + /*
> + * Get the number of bits handled by next levels, then any extra bits in
> + * the address should index the concatenated tables. This relation can be
> + * deduced from tables in ARM DDI0487I.a: D8.2.7-9
> + */
> + int shift = level_shift(start_level - 1, granule_sz);
> +
> + ret = ipa >> shift;
> + return ret;
> +}
> +
> #define SMMU_IOTLB_ASID(key) ((key).asid)
>
> typedef struct SMMUIOTLBPageInvInfo {
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 02/10] hw/arm/smmuv3: Update translation config to hold stage-2
2023-04-01 10:49 ` [RFC PATCH v3 02/10] hw/arm/smmuv3: Update translation config to hold stage-2 Mostafa Saleh
@ 2023-05-15 8:37 ` Eric Auger
0 siblings, 0 replies; 26+ messages in thread
From: Eric Auger @ 2023-05-15 8:37 UTC (permalink / raw)
To: Mostafa Saleh, qemu-devel
Cc: jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 4/1/23 12:49, Mostafa Saleh wrote:
> In preparation for adding stage-2 support, add a S2 config
> struct(SMMUS2Cfg), composed of the following fields and embedded in
> the main SMMUTransCfg:
> -tsz: Size of IPA input region (S2T0SZ)
> -sl0: Start level of translation (S2SL0)
> -affd: AF Fault Disable (S2AFFD)
> -record_faults: Record fault events (S2R)
> -granule_sz: Granule page shift (based on S2TG)
> -vmid: Virtual Machine ID (S2VMID)
> -vttb: Address of translation table base (S2TTB)
> -eff_ps: Effective PA output range (based on S2PS)
>
> They will be used in the next patches in stage-2 address translation.
>
> The fields in SMMUS2Cfg, are reordered to make the shared and stage-1
> fields next to each other, this reordering didn't change the struct
> size (104 bytes before and after).
>
> Stage-1 only fields: aa64, asid, tt, ttb, tbi, record_faults, oas.
> oas is stage-1 output address size. However, it is used to check
> input address in case stage-1 is unimplemented or bypassed according
> to SMMUv3 manual IHI0070.E "3.4. Address sizes"
>
> Shared fields: stage, disabled, bypassed, aborted, iotlb_*.
>
> No functional change intended.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in v3:
> -Add record_faults for stage-2
> -Reorder and document fields in SMMUTransCfg based on stage
> -Rename oas in SMMUS2Cfg to eff_ps
> -Improve comments in SMMUS2Cfg
> Changes in v2:
> -Add oas
> ---
> include/hw/arm/smmu-common.h | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 9fcff26357..9cf3f37929 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -58,25 +58,41 @@ typedef struct SMMUTLBEntry {
> uint8_t granule;
> } SMMUTLBEntry;
>
> +/* Stage-2 configuration. */
> +typedef struct SMMUS2Cfg {
> + uint8_t tsz; /* Size of IPA input region (S2T0SZ) */
> + uint8_t sl0; /* Start level of translation (S2SL0) */
> + bool affd; /* AF Fault Disable (S2AFFD) */
> + bool record_faults; /* Record fault events (S2R) */
> + uint8_t granule_sz; /* Granule page shift (based on S2TG) */
> + uint8_t eff_ps; /* Effective PA output range (based on S2PS) */
> + uint16_t vmid; /* Virtual Machine ID (S2VMID) */
> + uint64_t vttb; /* Address of translation table base (S2TTB) */
> +} SMMUS2Cfg;
> +
> /*
> * Generic structure populated by derived SMMU devices
> * after decoding the configuration information and used as
> * input to the page table walk
> */
> typedef struct SMMUTransCfg {
> + /* Shared fields between stage-1 and stage-2. */
> int stage; /* translation stage */
> - bool aa64; /* arch64 or aarch32 translation table */
> bool disabled; /* smmu is disabled */
> bool bypassed; /* translation is bypassed */
> bool aborted; /* translation is aborted */
> + uint32_t iotlb_hits; /* counts IOTLB hits */
> + uint32_t iotlb_misses; /* counts IOTLB misses*/
> + /* Used by stage-1 only. */
> + bool aa64; /* arch64 or aarch32 translation table */
> bool record_faults; /* record fault events */
> uint64_t ttb; /* TT base address */
> uint8_t oas; /* output address width */
> uint8_t tbi; /* Top Byte Ignore */
> uint16_t asid;
> SMMUTransTableInfo tt[2];
> - uint32_t iotlb_hits; /* counts IOTLB hits for this asid */
> - uint32_t iotlb_misses; /* counts IOTLB misses for this asid */
> + /* Used by stage-2 only. */
> + struct SMMUS2Cfg s2cfg;
> } SMMUTransCfg;
>
> typedef struct SMMUDevice {
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config for stage-2
2023-04-01 10:49 ` [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config " Mostafa Saleh
@ 2023-05-15 13:03 ` Eric Auger
2023-05-15 15:37 ` Mostafa Saleh
0 siblings, 1 reply; 26+ messages in thread
From: Eric Auger @ 2023-05-15 13:03 UTC (permalink / raw)
To: Mostafa Saleh, qemu-devel
Cc: jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 4/1/23 12:49, Mostafa Saleh wrote:
> Parse stage-2 configuration from STE and populate it in SMMUS2Cfg.
> Validity of field values are checked when possible.
>
> Only AA64 tables are supported and Small Translation Tables (STT) are
> not supported.
>
> According to SMMUv3 UM(IHI0070E) "5.2 Stream Table Entry": All fields
> with an S2 prefix (with the exception of S2VMID) are IGNORED when
> stage-2 bypasses translation (Config[1] == 0).
>
> Which means that VMID can be used(for TLB tagging) even if stage-2 is
> bypassed, so we parse it unconditionally when S2P exists. Otherwise
> it is set to -1.(only S1P)
>
> As stall is not supported, if S2S is set the translation would abort.
> For S2R, we reuse the same code used for stage-1 with flag
> record_faults. However when nested translation is supported we would
> need to separate stage-1 and stage-2 faults.
>
> Fix wrong shift in STE_S2HD, STE_S2HA, STE_S2S.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in V3:
> - Separate fault handling.
> - Fix shift in STE_S2HD, STE_S2HA, STE_S2S, STE_S2R.
> - Rename t0sz_valid to s2t0sz_valid.
> - separate stage-2 STE parsing in decode_ste_s2_cfg.
> - Add a log for invalid S2ENDI and S2TTB.
> - Set default value for stage-1 OAS.
> - Move and rename SMMU_MAX_S2_CONCAT to VMSA_MAX_S2_CONCAT.
> Changes in V2:
> - Parse S2PS and S2ENDI
> - Squash with S2VMID parsing patch
> - Squash with S2AFF parsing
> - Squash with fault reporting patch
> - Add check for S2T0SZ
> - Renaming and refactoring code
> ---
> hw/arm/smmuv3-internal.h | 10 +-
> hw/arm/smmuv3.c | 188 +++++++++++++++++++++++++++++++++--
> include/hw/arm/smmu-common.h | 1 +
> include/hw/arm/smmuv3.h | 3 +
> 4 files changed, 192 insertions(+), 10 deletions(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index 183d5ac8dc..6d1c1edab7 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -526,9 +526,13 @@ typedef struct CD {
> #define STE_S2TG(x) extract32((x)->word[5], 14, 2)
> #define STE_S2PS(x) extract32((x)->word[5], 16, 3)
> #define STE_S2AA64(x) extract32((x)->word[5], 19, 1)
> -#define STE_S2HD(x) extract32((x)->word[5], 24, 1)
> -#define STE_S2HA(x) extract32((x)->word[5], 25, 1)
> -#define STE_S2S(x) extract32((x)->word[5], 26, 1)
> +#define STE_S2ENDI(x) extract32((x)->word[5], 20, 1)
> +#define STE_S2AFFD(x) extract32((x)->word[5], 21, 1)
> +#define STE_S2HD(x) extract32((x)->word[5], 23, 1)
> +#define STE_S2HA(x) extract32((x)->word[5], 24, 1)
> +#define STE_S2S(x) extract32((x)->word[5], 25, 1)
> +#define STE_S2R(x) extract32((x)->word[5], 26, 1)
> +
> #define STE_CTXPTR(x) \
> ({ \
> unsigned long addr; \
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 4e90343996..0f5429aed8 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -33,6 +33,16 @@
> #include "smmuv3-internal.h"
> #include "smmu-internal.h"
>
> +/* If stage-1 fault enabled and ptw event targets it. */
> +#define PTW_FAULT_S1(cfg, event) ((cfg)->record_faults && \
> + !(event).u.f_walk_eabt.s2)
> +/* If stage-2 fault enabled and ptw event targets it. */
> +#define PTW_FAULT_S2(cfg, event) ((cfg)->s2cfg.record_faults && \
> + (event).u.f_walk_eabt.s2)
> +
> +#define PTW_FAULT_ALLOWED(cfg, event) (PTW_FAULT_S1(cfg, event) || \
> + PTW_FAULT_S2(cfg, event))
The name of the macro does not really reflect what it tests. I would
suggest PTW_RECORD_FAULT and PTW_RECORD_S1|S2_FAULT
I would also suggest (cfg->stage == 1) ? PTW_RECORD_S1_FAULT(cfg,
event) : PTW_RECORD_S2_FAULT(cfg, event)
PTW_FAULT_S1() and PTW_FAULT_S2() are not used anywhere else.
I would simplify PTW_RECORD_FAULT(cfg) (cfg->stage == 1) ?
(cfg)->record_faults: (cfg)->s2cfg.record_faults
> +
> /**
> * smmuv3_trigger_irq - pulse @irq if enabled and update
> * GERROR register in case of GERROR interrupt
> @@ -329,11 +339,141 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid,
> return 0;
> }
>
> +/*
> + * Max valid value is 39 when SMMU_IDR3.STT == 0.
> + * In architectures after SMMUv3.0:
> + * - If STE.S2TG selects a 4KB or 16KB granule, the minimum valid value for this
> + * field is MAX(16, 64-IAS)
nit: maybe indent with the if
> + * - If STE.S2TG selects a 64KB granule, the minimum valid value for this field
> + * is (64-IAS).
ditto
> + * As we only support AA64, IAS = OAS.
> + */
> +static bool s2t0sz_valid(SMMUTransCfg *cfg)
> +{
> + if (cfg->s2cfg.tsz > 39) {
> + return false;
> + }
> +
> + if (cfg->s2cfg.granule_sz == 16) {
> + return (cfg->s2cfg.tsz >= 64 - oas2bits(SMMU_IDR5_OAS));
> + }
> +
> + return (cfg->s2cfg.tsz >= MAX(64 - oas2bits(SMMU_IDR5_OAS), 16));
> +}
> +
> +/*
> + * Return true if s2 page table config is valid.
> + * This checks with the configured start level, ias_bits and granularity we can
> + * have a valid page table as described in ARM ARM D8.2 Translation process.
> + * The idea here is to see for the highest possible number of IPA bits, how
> + * many concatenated tables we would need, if it is more than 16, then this is
> + * not possible.
> + */
> +static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran)
> +{
> + int level = get_start_level(sl0, gran);
> + uint64_t ipa_bits = 64 - t0sz;
> + uint64_t max_ipa = (1ULL << ipa_bits) - 1;
> + int nr_concat = pgd_concat_idx(level, gran, max_ipa) + 1;
> +
> + return nr_concat <= VMSA_MAX_S2_CONCAT;
> +}
> +
> +static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
> +{
> + cfg->stage = 2;
> +
> + if (STE_S2AA64(ste) == 0x0) {
> + qemu_log_mask(LOG_UNIMP,
> + "SMMUv3 AArch32 tables not supported\n");
> + g_assert_not_reached();
> + }
> +
> + switch (STE_S2TG(ste)) {
> + case 0x0: /* 4KB */
> + cfg->s2cfg.granule_sz = 12;
> + break;
> + case 0x1: /* 64KB */
> + cfg->s2cfg.granule_sz = 16;
> + break;
> + case 0x2: /* 16KB */
> + cfg->s2cfg.granule_sz = 14;
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "SMMUv3 bad STE S2TG: %x\n", STE_S2TG(ste));
> + goto bad_ste;
> + }
> +
> + cfg->s2cfg.vttb = STE_S2TTB(ste);
> +
> + cfg->s2cfg.sl0 = STE_S2SL0(ste);
> + /* FEAT_TTST not supported. */
> + if (cfg->s2cfg.sl0 == 0x3) {
> + qemu_log_mask(LOG_UNIMP, "SMMUv3 S2SL0 = 0x3 has no meaning!\n");
> + goto bad_ste;
> + }
> +
> + /* For AA64, The effective S2PS size is capped to the OAS. */
> + cfg->s2cfg.eff_ps = oas2bits(MIN(STE_S2PS(ste), SMMU_IDR5_OAS));
> + /*
> + * It is ILLEGAL for the address in S2TTB to be outside the range
> + * described by the effective S2PS value.
> + */
> + if (cfg->s2cfg.vttb & ~(MAKE_64BIT_MASK(0, cfg->s2cfg.eff_ps))) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "SMMUv3 S2TTB too large 0x%lx, effective PS %d bits\n",
> + cfg->s2cfg.vttb, cfg->s2cfg.eff_ps);
> + goto bad_ste;
> + }
> +
> + cfg->s2cfg.tsz = STE_S2T0SZ(ste);
> +
> + if (!s2t0sz_valid(cfg)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "SMMUv3 bad STE S2T0SZ = %d\n",
> + cfg->s2cfg.tsz);
> + goto bad_ste;
> + }
> +
> + if (!s2_pgtable_config_valid(cfg->s2cfg.sl0, cfg->s2cfg.tsz,
> + cfg->s2cfg.granule_sz)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "SMMUv3 STE stage 2 config not valid!\n");
> + goto bad_ste;
> + }
> +
> + /* Only LE supported(IDR0.TTENDIAN). */
> + if (STE_S2ENDI(ste)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "SMMUv3 STE_S2ENDI only supports LE!\n");
> + goto bad_ste;
> + }
> +
> + cfg->s2cfg.affd = STE_S2AFFD(ste);
> +
> + cfg->s2cfg.record_faults = STE_S2R(ste);
> + /* As stall is not supported. */
> + if (STE_S2S(ste)) {
> + qemu_log_mask(LOG_UNIMP, "SMMUv3 Stall not implemented!\n");
> + goto bad_ste;
> + }
> +
> + /* This is still here as stage 2 has not been fully enabled yet. */
> + qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
> + goto bad_ste;
> +
> + return 0;
> +
> +bad_ste:
> + return -EINVAL;
> +}
> +
> /* Returns < 0 in case of invalid STE, 0 otherwise */
> static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
> STE *ste, SMMUEventInfo *event)
> {
> uint32_t config;
> + int ret;
>
> if (!STE_VALID(ste)) {
> if (!event->inval_ste_allowed) {
> @@ -354,11 +494,39 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
> return 0;
> }
>
> - if (STE_CFG_S2_ENABLED(config)) {
> - qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
> + /*
> + * If a stage is enabled in SW while not advertised, throw bad ste
> + * according to user manual(IHI0070E) "5.2 Stream Table Entry".
> + */
> + if (!STAGE1_SUPPORTED(s) && STE_CFG_S1_ENABLED(config)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "SMMUv3 S1 used but not supported.\n");
> + goto bad_ste;
> + }
> + if (!STAGE2_SUPPORTED(s) && STE_CFG_S2_ENABLED(config)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "SMMUv3 S2 used but not supported.\n");
> goto bad_ste;
> }
>
> + if (STAGE2_SUPPORTED(s)) {
> + /* VMID is considered even if s2 is disabled. */
> + cfg->s2cfg.vmid = STE_S2VMID(ste);
> + } else {
> + /* Default to -1 */
> + cfg->s2cfg.vmid = -1;
> + }
> +
> + if (STE_CFG_S2_ENABLED(config)) {
> + /*
> + * Stage-1 OAS defaults to OAS even if not enabled as it would be used
> + * in input address check for stage-2.
> + */
> + cfg->oas = oas2bits(SMMU_IDR5_OAS);
> + ret = decode_ste_s2_cfg(cfg, ste);
> + if (ret) {
> + goto bad_ste;
> + }
> + }
> +
> if (STE_S1CDMAX(ste) != 0) {
> qemu_log_mask(LOG_UNIMP,
> "SMMUv3 does not support multiple context descriptors yet\n");
> @@ -702,7 +870,13 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> if (cached_entry) {
> if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
> status = SMMU_TRANS_ERROR;
> - if (cfg->record_faults) {
> + /*
> + * We know that the TLB only contains either stage-1 or stage-2 as
> + * nesting is not supported. So it is sufficient to check the
> + * translation stage to know the TLB stage for now.
> + */
> + event.u.f_walk_eabt.s2 = (cfg->stage == 2);
> + if (PTW_FAULT_ALLOWED(cfg, event)) {
> event.type = SMMU_EVT_F_PERMISSION;
> event.u.f_permission.addr = addr;
> event.u.f_permission.rnw = flag & 0x1;
> @@ -728,28 +902,28 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> event.u.f_walk_eabt.addr2 = ptw_info.addr;
> break;
> case SMMU_PTW_ERR_TRANSLATION:
> - if (cfg->record_faults) {
> + if (PTW_FAULT_ALLOWED(cfg, event)) {
> event.type = SMMU_EVT_F_TRANSLATION;
> event.u.f_translation.addr = addr;
> event.u.f_translation.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_ADDR_SIZE:
> - if (cfg->record_faults) {
> + if (PTW_FAULT_ALLOWED(cfg, event)) {
> event.type = SMMU_EVT_F_ADDR_SIZE;
> event.u.f_addr_size.addr = addr;
> event.u.f_addr_size.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_ACCESS:
> - if (cfg->record_faults) {
> + if (PTW_FAULT_ALLOWED(cfg, event)) {
> event.type = SMMU_EVT_F_ACCESS;
> event.u.f_access.addr = addr;
> event.u.f_access.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_PERMISSION:
> - if (cfg->record_faults) {
> + if (PTW_FAULT_ALLOWED(cfg, event)) {
> event.type = SMMU_EVT_F_PERMISSION;
> event.u.f_permission.addr = addr;
> event.u.f_permission.rnw = flag & 0x1;
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 97cea8ea06..4f1405d4e4 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -29,6 +29,7 @@
>
> /* VMSAv8-64 Translation constants and functions */
> #define VMSA_LEVELS 4
> +#define VMSA_MAX_S2_CONCAT 16
>
> #define VMSA_STRIDE(gran) ((gran) - VMSA_LEVELS + 1)
> #define VMSA_BIT_LVL(isz, strd, lvl) ((isz) - (strd) * \
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index a0c026402e..6031d7d325 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -83,4 +83,7 @@ struct SMMUv3Class {
> #define TYPE_ARM_SMMUV3 "arm-smmuv3"
> OBJECT_DECLARE_TYPE(SMMUv3State, SMMUv3Class, ARM_SMMUV3)
>
> +#define STAGE1_SUPPORTED(s) FIELD_EX32(s->idr[0], IDR0, S1P)
> +#define STAGE2_SUPPORTED(s) FIELD_EX32(s->idr[0], IDR0, S2P)
> +
> #endif
Thanks
Eric
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3
2023-05-12 14:46 ` [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Peter Maydell
2023-05-12 15:26 ` Eric Auger
@ 2023-05-15 13:03 ` Mostafa Saleh
2023-05-15 14:34 ` Eric Auger
1 sibling, 1 reply; 26+ messages in thread
From: Mostafa Saleh @ 2023-05-15 13:03 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, jean-philippe, eric.auger, qemu-arm, richard.henderson
Hi Peter,
On Fri, May 12, 2023 at 03:46:43PM +0100, Peter Maydell wrote:
>
> Mostafa: is there anything in particular here that means this
> patchset should stay an RFC and isn't ready to go into the tree?
No, I believe the series is in a good shape now.
I will remove RFC when sending v4, if Eric has not major findings.
Thanks,
Mostafa
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 10/10] hw/arm/smmuv3: Add knob to choose translation stage and enable stage-2
2023-04-01 10:49 ` [RFC PATCH v3 10/10] hw/arm/smmuv3: Add knob to choose translation stage and enable stage-2 Mostafa Saleh
@ 2023-05-15 13:15 ` Eric Auger
0 siblings, 0 replies; 26+ messages in thread
From: Eric Auger @ 2023-05-15 13:15 UTC (permalink / raw)
To: Mostafa Saleh, qemu-devel
Cc: jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 4/1/23 12:49, Mostafa Saleh wrote:
> As everything is in place, we can use a new system property to
> advertise which stage is supported and remove bad_ste from STE
> stage2 config.
>
> The property added arm-smmuv3.stage can have 3 values:
> - "1": Stage-1 only is advertised.
> - "2": Stage-2 only is advertised.
> - "all": Stage-1 + Stage-2 are supported, which is not implemented in
> this patch series.
>
> If not passed or an unsupported value is passed, it will default to
> stage-1.
>
> Advertise VMID16.
>
> Don't try to decode CD, if stage-2 is configured.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in v2:
> - Squash knob patch with stage-2 enable patch.
> - Don't try to decode CD, if stage-2 is configured.
> ---
> hw/arm/smmuv3.c | 34 +++++++++++++++++++++++++---------
> include/hw/arm/smmuv3.h | 1 +
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 826aacf8b1..22b5613d4c 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -21,6 +21,7 @@
> #include "hw/irq.h"
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> +#include "hw/qdev-properties.h"
> #include "hw/qdev-core.h"
> #include "hw/pci/pci.h"
> #include "cpu.h"
> @@ -248,14 +249,20 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *info)
>
> static void smmuv3_init_regs(SMMUv3State *s)
> {
> - /**
> - * IDR0: stage1 only, AArch64 only, coherent access, 16b ASID,
> - * multi-level stream table
> + /*
> + * Based on sys property, the stages supported in smmu will be advertised.
> + * At the moment "all" is not supported and default to stage-1.
> */
> - s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1); /* stage 1 supported */
> + if (s->stage && !strcmp("2", s->stage)) {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S2P, 1);
> + } else {
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1);
> + }
> +
> s->idr[0] = FIELD_DP32(s->idr[0], IDR0, TTF, 2); /* AArch64 PTW only */
> s->idr[0] = FIELD_DP32(s->idr[0], IDR0, COHACC, 1); /* IO coherent */
> s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ASID16, 1); /* 16-bit ASID */
> + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, VMID16, 1); /* 16-bit VMID */
> s->idr[0] = FIELD_DP32(s->idr[0], IDR0, TTENDIAN, 2); /* little endian */
> s->idr[0] = FIELD_DP32(s->idr[0], IDR0, STALL_MODEL, 1); /* No stall */
> /* terminated transaction will always be aborted/error returned */
> @@ -458,10 +465,6 @@ static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
> goto bad_ste;
> }
>
> - /* This is still here as stage 2 has not been fully enabled yet. */
> - qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
> - goto bad_ste;
> -
> return 0;
>
> bad_ste:
> @@ -740,7 +743,7 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg,
> return ret;
> }
>
> - if (cfg->aborted || cfg->bypassed) {
> + if (cfg->aborted || cfg->bypassed || (cfg->stage == 2)) {
> return 0;
> }
>
> @@ -1809,6 +1812,18 @@ static const VMStateDescription vmstate_smmuv3 = {
> }
> };
>
> +static Property smmuv3_properties[] = {
> + /*
> + * Stages of translation advertised.
> + * "1": Stage 1
> + * "2": Stage 2
> + * "all": Stage 1 + Stage 2
I don't think you should talk about 'all' at the moment.
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> + * Defaults to stage 1
> + */
> + DEFINE_PROP_STRING("stage", SMMUv3State, stage),
> + DEFINE_PROP_END_OF_LIST()
> +};
> +
> static void smmuv3_instance_init(Object *obj)
> {
> /* Nothing much to do here as of now */
> @@ -1825,6 +1840,7 @@ static void smmuv3_class_init(ObjectClass *klass, void *data)
> &c->parent_phases);
> c->parent_realize = dc->realize;
> dc->realize = smmu_realize;
> + device_class_set_props(dc, smmuv3_properties);
> }
>
> static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 6031d7d325..d183a62766 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -62,6 +62,7 @@ struct SMMUv3State {
>
> qemu_irq irq[4];
> QemuMutex mutex;
> + char *stage;
> };
>
> typedef enum {
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2
2023-04-01 10:49 ` [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2 Mostafa Saleh
@ 2023-05-15 13:17 ` Eric Auger
2023-05-15 14:14 ` Eric Auger
1 sibling, 0 replies; 26+ messages in thread
From: Eric Auger @ 2023-05-15 13:17 UTC (permalink / raw)
To: Mostafa Saleh, qemu-devel
Cc: jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 4/1/23 12:49, Mostafa Saleh wrote:
> CMD_TLBI_S2_IPA: As S1+S2 is not enabled, for now this can be the
> same as CMD_TLBI_NH_VAA.
>
> CMD_TLBI_S12_VMALL: Added new function to invalidate TLB by VMID.
>
> For stage-1 only commands, add a check to throw CERROR_ILL if used
> when stage-1 is not supported.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in v3:
> - Log guest error for all illegal commands.
> Changes in v2:
> - Add checks for stage-1 only commands
> - Rename smmuv3_s1_range_inval to smmuv3_range_inval
> ---
> hw/arm/smmu-common.c | 16 +++++++++++
> hw/arm/smmuv3.c | 53 ++++++++++++++++++++++++++++++------
> hw/arm/trace-events | 4 ++-
> include/hw/arm/smmu-common.h | 1 +
> 4 files changed, 65 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 72ed6edd48..45e9d7e752 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -135,6 +135,16 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
>
> return SMMU_IOTLB_ASID(*iotlb_key) == asid;
> }
> +
> +static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
> + gpointer user_data)
> +{
> + uint16_t vmid = *(uint16_t *)user_data;
> + SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
> +
> + return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
> +}
> +
> static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
> gpointer user_data)
> {
> @@ -187,6 +197,12 @@ void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
> g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
> }
>
> +inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
> +{
> + trace_smmu_iotlb_inv_vmid(vmid);
> + g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
> +}
> +
> /* VMSAv8-64 Translation */
>
> /**
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index d7e7003da9..3b5b1fad1a 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1069,7 +1069,7 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
> }
> }
>
> -static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> +static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
> {
> dma_addr_t end, addr = CMD_ADDR(cmd);
> uint8_t type = CMD_TYPE(cmd);
> @@ -1094,7 +1094,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> }
>
> if (!tg) {
> - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
> + trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
> smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
> return;
> @@ -1112,7 +1112,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> uint64_t mask = dma_aligned_pow2_mask(addr, end, 64);
>
> num_pages = (mask + 1) >> granule;
> - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
> + trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
> smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
> addr += mask + 1;
> @@ -1246,12 +1246,22 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> {
> uint16_t asid = CMD_ASID(&cmd);
>
> + if (!STAGE1_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> +
> trace_smmuv3_cmdq_tlbi_nh_asid(asid);
> smmu_inv_notifiers_all(&s->smmu_state);
> smmu_iotlb_inv_asid(bs, asid);
> break;
> }
> case SMMU_CMD_TLBI_NH_ALL:
> + if (!STAGE1_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> + QEMU_FALLTHROUGH;
> case SMMU_CMD_TLBI_NSNH_ALL:
> trace_smmuv3_cmdq_tlbi_nh();
> smmu_inv_notifiers_all(&s->smmu_state);
> @@ -1259,7 +1269,34 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> break;
> case SMMU_CMD_TLBI_NH_VAA:
> case SMMU_CMD_TLBI_NH_VA:
> - smmuv3_s1_range_inval(bs, &cmd);
> + if (!STAGE1_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> + smmuv3_range_inval(bs, &cmd);
> + break;
> + case SMMU_CMD_TLBI_S12_VMALL:
> + uint16_t vmid = CMD_VMID(&cmd);
> +
> + if (!STAGE2_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> +
> + trace_smmuv3_cmdq_tlbi_s12_vmid(vmid);
> + smmu_inv_notifiers_all(&s->smmu_state);
> + smmu_iotlb_inv_vmid(bs, vmid);
> + break;
> + case SMMU_CMD_TLBI_S2_IPA:
> + if (!STAGE2_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> + /*
> + * As currently only either s1 or s2 are supported
> + * we can reuse same function for s2.
> + */
> + smmuv3_range_inval(bs, &cmd);
> break;
> case SMMU_CMD_TLBI_EL3_ALL:
> case SMMU_CMD_TLBI_EL3_VA:
> @@ -1267,8 +1304,6 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> case SMMU_CMD_TLBI_EL2_ASID:
> case SMMU_CMD_TLBI_EL2_VA:
> case SMMU_CMD_TLBI_EL2_VAA:
> - case SMMU_CMD_TLBI_S12_VMALL:
> - case SMMU_CMD_TLBI_S2_IPA:
> case SMMU_CMD_ATC_INV:
> case SMMU_CMD_PRI_RESP:
> case SMMU_CMD_RESUME:
> @@ -1277,12 +1312,14 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> break;
> default:
> cmd_error = SMMU_CERROR_ILL;
> - qemu_log_mask(LOG_GUEST_ERROR,
> - "Illegal command type: %d\n", CMD_TYPE(&cmd));
> break;
> }
> qemu_mutex_unlock(&s->mutex);
> if (cmd_error) {
> + if (cmd_error == SMMU_CERROR_ILL) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "Illegal command type: %d\n", CMD_TYPE(&cmd));
> + }
> break;
> }
> /*
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index 705104e58b..f8fdf1ca9f 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -12,6 +12,7 @@ smmu_ptw_block_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, ui
> smmu_get_pte(uint64_t baseaddr, int index, uint64_t pteaddr, uint64_t pte) "baseaddr=0x%"PRIx64" index=0x%x, pteaddr=0x%"PRIx64", pte=0x%"PRIx64
> smmu_iotlb_inv_all(void) "IOTLB invalidate all"
> smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d"
> +smmu_iotlb_inv_vmid(uint16_t vmid) "IOTLB invalidate vmid=%d"
> smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64
> smmu_inv_notifiers_mr(const char *name) "iommu mr=%s"
> smmu_iotlb_lookup_hit(uint16_t asid, uint16_t vmid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache HIT asid=%d vmid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d"
> @@ -45,9 +46,10 @@ smmuv3_cmdq_cfgi_ste_range(int start, int end) "start=0x%x - end=0x%x"
> smmuv3_cmdq_cfgi_cd(uint32_t sid) "sid=0x%x"
> smmuv3_config_cache_hit(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache HIT for sid=0x%x (hits=%d, misses=%d, hit rate=%d)"
> smmuv3_config_cache_miss(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache MISS for sid=0x%x (hits=%d, misses=%d, hit rate=%d)"
> -smmuv3_s1_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t num_pages, uint8_t ttl, bool leaf) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d"
> +smmuv3_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t num_pages, uint8_t ttl, bool leaf) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d"
> smmuv3_cmdq_tlbi_nh(void) ""
> smmuv3_cmdq_tlbi_nh_asid(uint16_t asid) "asid=%d"
> +smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
> smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
> smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu mr=%s"
> smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu mr=%s"
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 3cbb4998ad..fd8d772da1 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -193,6 +193,7 @@ SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
> uint8_t tg, uint8_t level);
> void smmu_iotlb_inv_all(SMMUState *s);
> void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid);
> +void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid);
> void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
> uint8_t tg, uint64_t num_pages, uint8_t ttl);
>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2
2023-04-01 10:49 ` [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2 Mostafa Saleh
2023-05-15 13:17 ` Eric Auger
@ 2023-05-15 14:14 ` Eric Auger
2023-05-15 15:32 ` Mostafa Saleh
1 sibling, 1 reply; 26+ messages in thread
From: Eric Auger @ 2023-05-15 14:14 UTC (permalink / raw)
To: Mostafa Saleh, qemu-devel
Cc: jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 4/1/23 12:49, Mostafa Saleh wrote:
> CMD_TLBI_S2_IPA: As S1+S2 is not enabled, for now this can be the
> same as CMD_TLBI_NH_VAA.
>
> CMD_TLBI_S12_VMALL: Added new function to invalidate TLB by VMID.
>
> For stage-1 only commands, add a check to throw CERROR_ILL if used
> when stage-1 is not supported.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in v3:
> - Log guest error for all illegal commands.
> Changes in v2:
> - Add checks for stage-1 only commands
> - Rename smmuv3_s1_range_inval to smmuv3_range_inval
> ---
> hw/arm/smmu-common.c | 16 +++++++++++
> hw/arm/smmuv3.c | 53 ++++++++++++++++++++++++++++++------
> hw/arm/trace-events | 4 ++-
> include/hw/arm/smmu-common.h | 1 +
> 4 files changed, 65 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 72ed6edd48..45e9d7e752 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -135,6 +135,16 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
>
> return SMMU_IOTLB_ASID(*iotlb_key) == asid;
> }
> +
> +static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
> + gpointer user_data)
> +{
> + uint16_t vmid = *(uint16_t *)user_data;
> + SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
> +
> + return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
> +}
> +
> static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
> gpointer user_data)
> {
> @@ -187,6 +197,12 @@ void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
> g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
> }
>
> +inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
> +{
> + trace_smmu_iotlb_inv_vmid(vmid);
> + g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
> +}
> +
> /* VMSAv8-64 Translation */
>
> /**
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index d7e7003da9..3b5b1fad1a 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1069,7 +1069,7 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
> }
> }
>
> -static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> +static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
> {
> dma_addr_t end, addr = CMD_ADDR(cmd);
> uint8_t type = CMD_TYPE(cmd);
> @@ -1094,7 +1094,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> }
>
> if (!tg) {
> - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
> + trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
> smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
> return;
> @@ -1112,7 +1112,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> uint64_t mask = dma_aligned_pow2_mask(addr, end, 64);
>
> num_pages = (mask + 1) >> granule;
> - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
> + trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
> smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
> addr += mask + 1;
> @@ -1246,12 +1246,22 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> {
> uint16_t asid = CMD_ASID(&cmd);
>
> + if (!STAGE1_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> +
> trace_smmuv3_cmdq_tlbi_nh_asid(asid);
> smmu_inv_notifiers_all(&s->smmu_state);
> smmu_iotlb_inv_asid(bs, asid);
> break;
> }
> case SMMU_CMD_TLBI_NH_ALL:
> + if (!STAGE1_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> + QEMU_FALLTHROUGH;
> case SMMU_CMD_TLBI_NSNH_ALL:
> trace_smmuv3_cmdq_tlbi_nh();
> smmu_inv_notifiers_all(&s->smmu_state);
> @@ -1259,7 +1269,34 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> break;
> case SMMU_CMD_TLBI_NH_VAA:
> case SMMU_CMD_TLBI_NH_VA:
> - smmuv3_s1_range_inval(bs, &cmd);
> + if (!STAGE1_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> + smmuv3_range_inval(bs, &cmd);
> + break;
> + case SMMU_CMD_TLBI_S12_VMALL:
> + uint16_t vmid = CMD_VMID(&cmd);
I get
../hw/arm/smmuv3.c: In function ‘smmuv3_cmdq_consume’:
../hw/arm/smmuv3.c:1295:13: error: a label can only be part of a
statement and a declaration is not a statement
uint16_t vmid = CMD_VMID(&cmd);
you should put the case into a block.
Eric
> +
> + if (!STAGE2_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> +
> + trace_smmuv3_cmdq_tlbi_s12_vmid(vmid);
> + smmu_inv_notifiers_all(&s->smmu_state);
> + smmu_iotlb_inv_vmid(bs, vmid);
> + break;
> + case SMMU_CMD_TLBI_S2_IPA:
> + if (!STAGE2_SUPPORTED(s)) {
> + cmd_error = SMMU_CERROR_ILL;
> + break;
> + }
> + /*
> + * As currently only either s1 or s2 are supported
> + * we can reuse same function for s2.
> + */
> + smmuv3_range_inval(bs, &cmd);
> break;
> case SMMU_CMD_TLBI_EL3_ALL:
> case SMMU_CMD_TLBI_EL3_VA:
> @@ -1267,8 +1304,6 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> case SMMU_CMD_TLBI_EL2_ASID:
> case SMMU_CMD_TLBI_EL2_VA:
> case SMMU_CMD_TLBI_EL2_VAA:
> - case SMMU_CMD_TLBI_S12_VMALL:
> - case SMMU_CMD_TLBI_S2_IPA:
> case SMMU_CMD_ATC_INV:
> case SMMU_CMD_PRI_RESP:
> case SMMU_CMD_RESUME:
> @@ -1277,12 +1312,14 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> break;
> default:
> cmd_error = SMMU_CERROR_ILL;
> - qemu_log_mask(LOG_GUEST_ERROR,
> - "Illegal command type: %d\n", CMD_TYPE(&cmd));
> break;
> }
> qemu_mutex_unlock(&s->mutex);
> if (cmd_error) {
> + if (cmd_error == SMMU_CERROR_ILL) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "Illegal command type: %d\n", CMD_TYPE(&cmd));
> + }
> break;
> }
> /*
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index 705104e58b..f8fdf1ca9f 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -12,6 +12,7 @@ smmu_ptw_block_pte(int stage, int level, uint64_t baseaddr, uint64_t pteaddr, ui
> smmu_get_pte(uint64_t baseaddr, int index, uint64_t pteaddr, uint64_t pte) "baseaddr=0x%"PRIx64" index=0x%x, pteaddr=0x%"PRIx64", pte=0x%"PRIx64
> smmu_iotlb_inv_all(void) "IOTLB invalidate all"
> smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d"
> +smmu_iotlb_inv_vmid(uint16_t vmid) "IOTLB invalidate vmid=%d"
> smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64
> smmu_inv_notifiers_mr(const char *name) "iommu mr=%s"
> smmu_iotlb_lookup_hit(uint16_t asid, uint16_t vmid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache HIT asid=%d vmid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d"
> @@ -45,9 +46,10 @@ smmuv3_cmdq_cfgi_ste_range(int start, int end) "start=0x%x - end=0x%x"
> smmuv3_cmdq_cfgi_cd(uint32_t sid) "sid=0x%x"
> smmuv3_config_cache_hit(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache HIT for sid=0x%x (hits=%d, misses=%d, hit rate=%d)"
> smmuv3_config_cache_miss(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t perc) "Config cache MISS for sid=0x%x (hits=%d, misses=%d, hit rate=%d)"
> -smmuv3_s1_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t num_pages, uint8_t ttl, bool leaf) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d"
> +smmuv3_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t num_pages, uint8_t ttl, bool leaf) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d"
> smmuv3_cmdq_tlbi_nh(void) ""
> smmuv3_cmdq_tlbi_nh_asid(uint16_t asid) "asid=%d"
> +smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
> smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
> smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu mr=%s"
> smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu mr=%s"
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 3cbb4998ad..fd8d772da1 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -193,6 +193,7 @@ SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
> uint8_t tg, uint8_t level);
> void smmu_iotlb_inv_all(SMMUState *s);
> void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid);
> +void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid);
> void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
> uint8_t tg, uint64_t num_pages, uint8_t ttl);
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3
2023-05-15 13:03 ` Mostafa Saleh
@ 2023-05-15 14:34 ` Eric Auger
0 siblings, 0 replies; 26+ messages in thread
From: Eric Auger @ 2023-05-15 14:34 UTC (permalink / raw)
To: Mostafa Saleh, Peter Maydell
Cc: qemu-devel, jean-philippe, qemu-arm, richard.henderson
Hi Mostafa, Peter
On 5/15/23 15:03, Mostafa Saleh wrote:
> Hi Peter,
>
> On Fri, May 12, 2023 at 03:46:43PM +0100, Peter Maydell wrote:
>> Mostafa: is there anything in particular here that means this
>> patchset should stay an RFC and isn't ready to go into the tree?
> No, I believe the series is in a good shape now.
> I will remove RFC when sending v4, if Eric has not major findings.
Yes I think you need to turn this into a PATCH set now. I had no big
comments to be taken into account. I tested this against non regressions
with S1 and I did not spot anything bad. Also I tested migration between
this and original implementation and it went gently.
Thanks
Eric
>
> Thanks,
> Mostafa
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2
2023-05-15 14:14 ` Eric Auger
@ 2023-05-15 15:32 ` Mostafa Saleh
2023-05-16 17:04 ` Eric Auger
0 siblings, 1 reply; 26+ messages in thread
From: Mostafa Saleh @ 2023-05-15 15:32 UTC (permalink / raw)
To: Eric Auger
Cc: qemu-devel, jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Eric,
On Mon, May 15, 2023 at 04:14:16PM +0200, Eric Auger wrote:
> Hi Mostafa,
> On 4/1/23 12:49, Mostafa Saleh wrote:
> > CMD_TLBI_S2_IPA: As S1+S2 is not enabled, for now this can be the
> > same as CMD_TLBI_NH_VAA.
> >
> > CMD_TLBI_S12_VMALL: Added new function to invalidate TLB by VMID.
> >
> > For stage-1 only commands, add a check to throw CERROR_ILL if used
> > when stage-1 is not supported.
> >
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> > Changes in v3:
> > - Log guest error for all illegal commands.
> > Changes in v2:
> > - Add checks for stage-1 only commands
> > - Rename smmuv3_s1_range_inval to smmuv3_range_inval
> > ---
> > hw/arm/smmu-common.c | 16 +++++++++++
> > hw/arm/smmuv3.c | 53 ++++++++++++++++++++++++++++++------
> > hw/arm/trace-events | 4 ++-
> > include/hw/arm/smmu-common.h | 1 +
> > 4 files changed, 65 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> > index 72ed6edd48..45e9d7e752 100644
> > --- a/hw/arm/smmu-common.c
> > +++ b/hw/arm/smmu-common.c
> > @@ -135,6 +135,16 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
> >
> > return SMMU_IOTLB_ASID(*iotlb_key) == asid;
> > }
> > +
> > +static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
> > + gpointer user_data)
> > +{
> > + uint16_t vmid = *(uint16_t *)user_data;
> > + SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
> > +
> > + return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
> > +}
> > +
> > static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
> > gpointer user_data)
> > {
> > @@ -187,6 +197,12 @@ void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
> > g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
> > }
> >
> > +inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
> > +{
> > + trace_smmu_iotlb_inv_vmid(vmid);
> > + g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
> > +}
> > +
> > /* VMSAv8-64 Translation */
> >
> > /**
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index d7e7003da9..3b5b1fad1a 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -1069,7 +1069,7 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
> > }
> > }
> >
> > -static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> > +static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
> > {
> > dma_addr_t end, addr = CMD_ADDR(cmd);
> > uint8_t type = CMD_TYPE(cmd);
> > @@ -1094,7 +1094,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> > }
> >
> > if (!tg) {
> > - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
> > + trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
> > smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
> > smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
> > return;
> > @@ -1112,7 +1112,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
> > uint64_t mask = dma_aligned_pow2_mask(addr, end, 64);
> >
> > num_pages = (mask + 1) >> granule;
> > - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
> > + trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
> > smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
> > smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
> > addr += mask + 1;
> > @@ -1246,12 +1246,22 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> > {
> > uint16_t asid = CMD_ASID(&cmd);
> >
> > + if (!STAGE1_SUPPORTED(s)) {
> > + cmd_error = SMMU_CERROR_ILL;
> > + break;
> > + }
> > +
> > trace_smmuv3_cmdq_tlbi_nh_asid(asid);
> > smmu_inv_notifiers_all(&s->smmu_state);
> > smmu_iotlb_inv_asid(bs, asid);
> > break;
> > }
> > case SMMU_CMD_TLBI_NH_ALL:
> > + if (!STAGE1_SUPPORTED(s)) {
> > + cmd_error = SMMU_CERROR_ILL;
> > + break;
> > + }
> > + QEMU_FALLTHROUGH;
> > case SMMU_CMD_TLBI_NSNH_ALL:
> > trace_smmuv3_cmdq_tlbi_nh();
> > smmu_inv_notifiers_all(&s->smmu_state);
> > @@ -1259,7 +1269,34 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> > break;
> > case SMMU_CMD_TLBI_NH_VAA:
> > case SMMU_CMD_TLBI_NH_VA:
> > - smmuv3_s1_range_inval(bs, &cmd);
> > + if (!STAGE1_SUPPORTED(s)) {
> > + cmd_error = SMMU_CERROR_ILL;
> > + break;
> > + }
> > + smmuv3_range_inval(bs, &cmd);
> > + break;
> > + case SMMU_CMD_TLBI_S12_VMALL:
> > + uint16_t vmid = CMD_VMID(&cmd);
> I get
> ../hw/arm/smmuv3.c: In function ‘smmuv3_cmdq_consume’:
> ../hw/arm/smmuv3.c:1295:13: error: a label can only be part of a
> statement and a declaration is not a statement
> uint16_t vmid = CMD_VMID(&cmd);
>
> you should put the case into a block.
Thanks for spotting this, I will fix it.
Can you please let me know your config/build commands?
as I didn't get errors when compiling it.
Thanks,
Mostafa
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config for stage-2
2023-05-15 13:03 ` Eric Auger
@ 2023-05-15 15:37 ` Mostafa Saleh
2023-05-16 17:19 ` Eric Auger
0 siblings, 1 reply; 26+ messages in thread
From: Mostafa Saleh @ 2023-05-15 15:37 UTC (permalink / raw)
To: Eric Auger
Cc: qemu-devel, jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Eric,
Thanks a lot for taking the time to review the patches.
On Mon, May 15, 2023 at 03:03:28PM +0200, Eric Auger wrote:
> >
> > +/* If stage-1 fault enabled and ptw event targets it. */
> > +#define PTW_FAULT_S1(cfg, event) ((cfg)->record_faults && \
> > + !(event).u.f_walk_eabt.s2)
> > +/* If stage-2 fault enabled and ptw event targets it. */
> > +#define PTW_FAULT_S2(cfg, event) ((cfg)->s2cfg.record_faults && \
> > + (event).u.f_walk_eabt.s2)
> > +
> > +#define PTW_FAULT_ALLOWED(cfg, event) (PTW_FAULT_S1(cfg, event) || \
> > + PTW_FAULT_S2(cfg, event))
> The name of the macro does not really reflect what it tests. I would
> suggest PTW_RECORD_FAULT and PTW_RECORD_S1|S2_FAULT
> I would also suggest (cfg->stage == 1) ? PTW_RECORD_S1_FAULT(cfg,
> event) : PTW_RECORD_S2_FAULT(cfg, event)
>
> PTW_FAULT_S1() and PTW_FAULT_S2() are not used anywhere else.
>
> I would simplify PTW_RECORD_FAULT(cfg) (cfg->stage == 1) ?
> (cfg)->record_faults: (cfg)->s2cfg.record_faults
Yes, this is much simpler.
I am wondering as stage-2 only SMMUs can have stage-1 faults as described in
(IHI 0070.E.a) "3.4 Address sizes".
If the input address of a transaction exceeds the size of the IAS.
I guess this means that the fault record in this case is still controlled by S2R
although it is stage-1 fault as there is no CD or stage-1 config.
Thanks,
Mostafa
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2
2023-05-15 15:32 ` Mostafa Saleh
@ 2023-05-16 17:04 ` Eric Auger
2023-05-16 19:32 ` Mostafa Saleh
0 siblings, 1 reply; 26+ messages in thread
From: Eric Auger @ 2023-05-16 17:04 UTC (permalink / raw)
To: Mostafa Saleh
Cc: qemu-devel, jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 5/15/23 17:32, Mostafa Saleh wrote:
> Hi Eric,
>
> On Mon, May 15, 2023 at 04:14:16PM +0200, Eric Auger wrote:
>> Hi Mostafa,
>> On 4/1/23 12:49, Mostafa Saleh wrote:
>>> CMD_TLBI_S2_IPA: As S1+S2 is not enabled, for now this can be the
>>> same as CMD_TLBI_NH_VAA.
>>>
>>> CMD_TLBI_S12_VMALL: Added new function to invalidate TLB by VMID.
>>>
>>> For stage-1 only commands, add a check to throw CERROR_ILL if used
>>> when stage-1 is not supported.
>>>
>>> Signed-off-by: Mostafa Saleh <smostafa@google.com>
>>> ---
>>> Changes in v3:
>>> - Log guest error for all illegal commands.
>>> Changes in v2:
>>> - Add checks for stage-1 only commands
>>> - Rename smmuv3_s1_range_inval to smmuv3_range_inval
>>> ---
>>> hw/arm/smmu-common.c | 16 +++++++++++
>>> hw/arm/smmuv3.c | 53 ++++++++++++++++++++++++++++++------
>>> hw/arm/trace-events | 4 ++-
>>> include/hw/arm/smmu-common.h | 1 +
>>> 4 files changed, 65 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>>> index 72ed6edd48..45e9d7e752 100644
>>> --- a/hw/arm/smmu-common.c
>>> +++ b/hw/arm/smmu-common.c
>>> @@ -135,6 +135,16 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
>>>
>>> return SMMU_IOTLB_ASID(*iotlb_key) == asid;
>>> }
>>> +
>>> +static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
>>> + gpointer user_data)
>>> +{
>>> + uint16_t vmid = *(uint16_t *)user_data;
>>> + SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
>>> +
>>> + return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
>>> +}
>>> +
>>> static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
>>> gpointer user_data)
>>> {
>>> @@ -187,6 +197,12 @@ void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
>>> g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
>>> }
>>>
>>> +inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
>>> +{
>>> + trace_smmu_iotlb_inv_vmid(vmid);
>>> + g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
>>> +}
>>> +
>>> /* VMSAv8-64 Translation */
>>>
>>> /**
>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>> index d7e7003da9..3b5b1fad1a 100644
>>> --- a/hw/arm/smmuv3.c
>>> +++ b/hw/arm/smmuv3.c
>>> @@ -1069,7 +1069,7 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
>>> }
>>> }
>>>
>>> -static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
>>> +static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
>>> {
>>> dma_addr_t end, addr = CMD_ADDR(cmd);
>>> uint8_t type = CMD_TYPE(cmd);
>>> @@ -1094,7 +1094,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
>>> }
>>>
>>> if (!tg) {
>>> - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
>>> + trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
>>> smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
>>> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
>>> return;
>>> @@ -1112,7 +1112,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
>>> uint64_t mask = dma_aligned_pow2_mask(addr, end, 64);
>>>
>>> num_pages = (mask + 1) >> granule;
>>> - trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
>>> + trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
>>> smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
>>> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
>>> addr += mask + 1;
>>> @@ -1246,12 +1246,22 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
>>> {
>>> uint16_t asid = CMD_ASID(&cmd);
>>>
>>> + if (!STAGE1_SUPPORTED(s)) {
>>> + cmd_error = SMMU_CERROR_ILL;
>>> + break;
>>> + }
>>> +
>>> trace_smmuv3_cmdq_tlbi_nh_asid(asid);
>>> smmu_inv_notifiers_all(&s->smmu_state);
>>> smmu_iotlb_inv_asid(bs, asid);
>>> break;
>>> }
>>> case SMMU_CMD_TLBI_NH_ALL:
>>> + if (!STAGE1_SUPPORTED(s)) {
>>> + cmd_error = SMMU_CERROR_ILL;
>>> + break;
>>> + }
>>> + QEMU_FALLTHROUGH;
>>> case SMMU_CMD_TLBI_NSNH_ALL:
>>> trace_smmuv3_cmdq_tlbi_nh();
>>> smmu_inv_notifiers_all(&s->smmu_state);
>>> @@ -1259,7 +1269,34 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
>>> break;
>>> case SMMU_CMD_TLBI_NH_VAA:
>>> case SMMU_CMD_TLBI_NH_VA:
>>> - smmuv3_s1_range_inval(bs, &cmd);
>>> + if (!STAGE1_SUPPORTED(s)) {
>>> + cmd_error = SMMU_CERROR_ILL;
>>> + break;
>>> + }
>>> + smmuv3_range_inval(bs, &cmd);
>>> + break;
>>> + case SMMU_CMD_TLBI_S12_VMALL:
>>> + uint16_t vmid = CMD_VMID(&cmd);
>> I get
>> ../hw/arm/smmuv3.c: In function ‘smmuv3_cmdq_consume’:
>> ../hw/arm/smmuv3.c:1295:13: error: a label can only be part of a
>> statement and a declaration is not a statement
>> uint16_t vmid = CMD_VMID(&cmd);
>>
>> you should put the case into a block.
> Thanks for spotting this, I will fix it.
> Can you please let me know your config/build commands?
> as I didn't get errors when compiling it.
I used a very basic config:
configure --target-list=aarch64-softmmu --enable-kvm
--enable-trace-backends=log
Eric
>
> Thanks,
> Mostafa
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config for stage-2
2023-05-15 15:37 ` Mostafa Saleh
@ 2023-05-16 17:19 ` Eric Auger
0 siblings, 0 replies; 26+ messages in thread
From: Eric Auger @ 2023-05-16 17:19 UTC (permalink / raw)
To: Mostafa Saleh
Cc: qemu-devel, jean-philippe, peter.maydell, qemu-arm, richard.henderson
Hi Mostafa,
On 5/15/23 17:37, Mostafa Saleh wrote:
> Hi Eric,
>
> Thanks a lot for taking the time to review the patches.
>
> On Mon, May 15, 2023 at 03:03:28PM +0200, Eric Auger wrote:
>>>
>>> +/* If stage-1 fault enabled and ptw event targets it. */
>>> +#define PTW_FAULT_S1(cfg, event) ((cfg)->record_faults && \
>>> + !(event).u.f_walk_eabt.s2)
>>> +/* If stage-2 fault enabled and ptw event targets it. */
>>> +#define PTW_FAULT_S2(cfg, event) ((cfg)->s2cfg.record_faults && \
>>> + (event).u.f_walk_eabt.s2)
>>> +
>>> +#define PTW_FAULT_ALLOWED(cfg, event) (PTW_FAULT_S1(cfg, event) || \
>>> + PTW_FAULT_S2(cfg, event))
>> The name of the macro does not really reflect what it tests. I would
>> suggest PTW_RECORD_FAULT and PTW_RECORD_S1|S2_FAULT
>> I would also suggest (cfg->stage == 1) ? PTW_RECORD_S1_FAULT(cfg,
>> event) : PTW_RECORD_S2_FAULT(cfg, event)
>>
>> PTW_FAULT_S1() and PTW_FAULT_S2() are not used anywhere else.
>>
>> I would simplify PTW_RECORD_FAULT(cfg) (cfg->stage == 1) ?
>> (cfg)->record_faults: (cfg)->s2cfg.record_faults
> Yes, this is much simpler.
> I am wondering as stage-2 only SMMUs can have stage-1 faults as described in
> (IHI 0070.E.a) "3.4 Address sizes".
> If the input address of a transaction exceeds the size of the IAS.
> I guess this means that the fault record in this case is still controlled by S2R
> although it is stage-1 fault as there is no CD or stage-1 config.
Yes this sounds sensible.
Eric
>
> Thanks,
> Mostafa
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2
2023-05-16 17:04 ` Eric Auger
@ 2023-05-16 19:32 ` Mostafa Saleh
0 siblings, 0 replies; 26+ messages in thread
From: Mostafa Saleh @ 2023-05-16 19:32 UTC (permalink / raw)
To: Eric Auger
Cc: qemu-devel, jean-philippe, peter.maydell, qemu-arm, richard.henderson
On Tue, May 16, 2023 at 07:04:34PM +0200, Eric Auger wrote:
> >>> + break;
> >>> + case SMMU_CMD_TLBI_S12_VMALL:
> >>> + uint16_t vmid = CMD_VMID(&cmd);
> >> I get
> >> ../hw/arm/smmuv3.c: In function ‘smmuv3_cmdq_consume’:
> >> ../hw/arm/smmuv3.c:1295:13: error: a label can only be part of a
> >> statement and a declaration is not a statement
> >> uint16_t vmid = CMD_VMID(&cmd);
> >>
> >> you should put the case into a block.
> > Thanks for spotting this, I will fix it.
> > Can you please let me know your config/build commands?
> > as I didn't get errors when compiling it.
> I used a very basic config:
>
> configure --target-list=aarch64-softmmu --enable-kvm
> --enable-trace-backends=log
Thanks Eric, this builds fine for me, it must be a toolchain thing then.
Thanks,
Mostafa
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2023-05-16 19:33 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-01 10:49 [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 01/10] hw/arm/smmuv3: Add missing fields for IDR0 Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 02/10] hw/arm/smmuv3: Update translation config to hold stage-2 Mostafa Saleh
2023-05-15 8:37 ` Eric Auger
2023-04-01 10:49 ` [RFC PATCH v3 03/10] hw/arm/smmuv3: Refactor stage-1 PTW Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 04/10] hw/arm/smmuv3: Add page table walk for stage-2 Mostafa Saleh
2023-05-15 8:37 ` Eric Auger
2023-04-01 10:49 ` [RFC PATCH v3 05/10] hw/arm/smmuv3: Parse STE config " Mostafa Saleh
2023-05-15 13:03 ` Eric Auger
2023-05-15 15:37 ` Mostafa Saleh
2023-05-16 17:19 ` Eric Auger
2023-04-01 10:49 ` [RFC PATCH v3 06/10] hw/arm/smmuv3: Make TLB lookup work " Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 07/10] hw/arm/smmuv3: Add VMID to TLB tagging Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 08/10] hw/arm/smmuv3: Add CMDs related to stage-2 Mostafa Saleh
2023-05-15 13:17 ` Eric Auger
2023-05-15 14:14 ` Eric Auger
2023-05-15 15:32 ` Mostafa Saleh
2023-05-16 17:04 ` Eric Auger
2023-05-16 19:32 ` Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 09/10] hw/arm/smmuv3: Add stage-2 support in iova notifier Mostafa Saleh
2023-04-01 10:49 ` [RFC PATCH v3 10/10] hw/arm/smmuv3: Add knob to choose translation stage and enable stage-2 Mostafa Saleh
2023-05-15 13:15 ` Eric Auger
2023-05-12 14:46 ` [RFC PATCH v3 00/10] Add stage-2 translation for SMMUv3 Peter Maydell
2023-05-12 15:26 ` Eric Auger
2023-05-15 13:03 ` Mostafa Saleh
2023-05-15 14:34 ` Eric Auger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.