* [PATCH v5 00/18] SMMUv3 nested translation support
@ 2024-07-15 8:45 Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 01/18] hw/arm/smmu-common: Add missing size check for stage-1 Mostafa Saleh
` (18 more replies)
0 siblings, 19 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
but not nested instances.
This patch series adds support for nested translation in SMMUv3,
this is controlled by property “arm-smmuv3.stage=nested”, and
advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
Main changes(architecture):
============================
1) CDs are considered IPA and translated with stage-2.
2) TTBx and tables for stage-1 are considered IPA and translated
with stage-2.
3) Translate the IPA address with stage-2.
TLBs:
======
TLBs are the most tricky part.
1) General design
Unified(Combined) design is used, where entries with ASID=-1 are
IPAs(cached from stage-2 config)
TLBs are also modified to cache 2 permissions, a new permission added
"parent_perm."
For non-nested configuration, perm == parent_perm and nothing
changes. This is used to know which stage to use in case there is
a permission fault from a TLB entry.
2) Caching in TLB
Stage-1 and stage-2 are inserted in the TLB as is.
For nested translation, both entries are combined into one TLB
entry. The size (level and granule) are chosen from the smallest entries.
That means that a stage-1 translation can be cached with sage-2
granule in key, this is taken into account for lookup.
3) TLB Lookup
TLB lookup already uses ASID in key, so it can distinguish between
stage-1 and stage-2.
And as mentioned above, the granule for stage-1 can be different,
If stage-1 lookup failed, we try again with the stage-2 granule.
4) TLB invalidation
- Address invalidation is split, for IOVA(CMD_TLBI_NH_VA
/CMD_TLBI_NH_VAA) and IPA(CMD_TLBI_S2_IPA) based on ASID value
- CMD_TLBI_NH_ASID/CMD_TLBI_NH_ALL: Consider VMID if stage-2 is
supported, and invalidate stage-1 only by VMIDs
As far as I understand, this is compliant with the ARM architecture:
- ARM ARM DDI 0487J.a: RLGSCG, RTVTYQ, RGNJPZ
- ARM IHI 0070F.b: 16.2 Caching
An alternative approach would be to instantiate 2 TLBs, one per each
stage. I haven’t investigated that.
Others
=======
- OAS: A typical setup with nesting is to share CPU stage-2 with the
SMMU, and according to the user manual, SMMU OAS must match the
system physical address.
This was discussed before in
https://lore.kernel.org/all/20230226220650.1480786-11-smostafa@google.com/
This series doesn’t implement that, but reworks OAS to make it easier
to configure in the future.
- For nested configuration, IOVA notifier only notifies for stage-1
invalidations (as far as I understand this is the intended
behaviour as it notifies for IOVA).
- Rework class in events.
- Stop ignoring VMID for stage-1 if stage-2 is also supported.
Future improvements:
=====================
1) One small improvement, that I don’t think it’s worth the extra
complexity, is in case of Stage-1 TLB miss for nested translation,
we can do stage-1 walk and lookup for stage-2 TLBs, instead of
doing the full walk.
Testing
========
1) IOMMUFD + VFIO
Kernel: https://lore.kernel.org/all/cover.1683688960.git.nicolinc@nvidia.com/
VMM: https://qemu-devel.nongnu.narkive.com/o815DqpI/rfc-v5-0-8-arm-smmuv3-emulation-support
By assigning “virtio-net-pci,netdev=net0,disable-legacy=on,iommu_platform=on,ats=on”,
to a guest VM (on top of QEMU guest) with VIFO and IOMMUFD.
2) Work in progress prototype I am hacking on for nesting on KVM
(this is nowhere near complete, and misses many stuff but it
doesn't require VMs/VFIO) also with virtio-net-pci and git
cloning a bunch of stuff and also observing traces.
https://android-kvm.googlesource.com/linux/+log/refs/heads/smostafa/android15-6.6-smmu-nesting-wip
Overall I tested the following configurations
(S1 = 4k, S2 = 4k):
- S1 level = 1 and S2 level = 1
- S1 level = 1 and S2 level = 2
- S1 level = 1 and S2 level = 3
- S1 level = 2 and S2 level = 1
- S1 level = 2 and S2 level = 2
- S1 level = 2 and S2 level = 3
- S1 level = 3 and S2 level = 2
- S1 level = 3 and S2 level = 3
(S1 = 16k, S2 = 4k)
- S1 level = 2 and S2 level = 1
- S1 level = 2 and S2 level = 2
- S1 level = 2 and S2 level = 3
- S1 level = 3 and S2 level = 1
- S1 level = 3 and S2 level = 2
- S1 level = 3 and S2 level = 3
(S1 = 64K, S2 = 4K)
- S1 level = 2 and S2 level = 1
- S1 level = 2 and S2 level = 2
- S1 level = 2 and S2 level = 3
- S1 level = 3 and S2 level = 1
- S1 level = 3 and S2 level = 2
- S1 level = 3 and S2 level = 3
(S1 = 4K, S2 = 16k)
- S1 level = 1 and S2 level = 2
- S1 level = 1 and S2 level = 3
- S1 level = 2 and S2 level = 2
- S1 level = 2 and S2 level = 3
- S1 level = 3 and S2 level = 2
- S1 level = 3 and S2 level = 3
(S1 = 4K, S2 = 64K)
- S1 level = 1 and S2 level = 2
- S1 level = 1 and S2 level = 3
- S1 level = 2 and S2 level = 2
- S1 level = 2 and S2 level = 3
- S1 level = 3 and S2 level = 2
- S1 level = 3 and S2 level = 3
hw/arm/smmuv3: Split smmuv3_translate() better viewed with --color-moved
The first 3 patches are fixes.
Changes in v5:
v4: https://lore.kernel.org/qemu-devel/20240701110241.2005222-1-smostafa@google.com/
- Collect Eric and Jean Rbs
- Fix a bug with nested lookup granule and iova mask
- Fix InputAddr for events for cd and ttbx translation faults
- Fix class in translation fault events
- Fix smmuv3_notify_iova
- Fix CACHED_ENTRY_TO_ADDR macro
- Drop FWB patch
- Fix bisectability by moving smmu_iotlb_inv_asid_vmid
Changes in v4:
v3: https://lore.kernel.org/qemu-devel/20240429032403.74910-1-smostafa@google.com/
- Collected Eric and Alex Rbs
- Rebased on master
- Dropped RFC tag
- Dropped last 2 patches about oas changes to avoid blocking this series
and I will post them after as RFC
- Split patch 7, and introduce CACHED_ENTRY_TO_ADDR in a separate patch
- Reorder patch 8 and 9 (combine tlb and tlb lookup)
- Split patch 12, and introduce smmu_iotlb_inv_asid_vmid in a separate patch
- Split patch 14, to have fault changes in a separate patch
- Update commit messages and include Fixes sha
- Minor updates, renames and a lot of comments based on review
Changes in v3
v2: https://lore.kernel.org/qemu-devel/20240408140818.3799590-1-smostafa@google.com/
- Collected Eric Rbs.
- Rebased on master.
- Fix an existing bug in class encoding.
- Fix an existing bug in S2 events missing IPA.
- Fix nesting event population (missing class and wrong events)
- Remove CALL_FUNC_CFG_S2.
- Rework TLB combination logic to cache the largest possible entries.
- Refactor nested translation code to be more clear.
- Split patch 05 to 4 patches.
- Convert asid/vmid in trace events to int also.
- Remove some extra traces as it was not needed.
- Improve commit messages.
Changes in v2:
v1: https://lore.kernel.org/qemu-devel/20240325101442.1306300-1-smostafa@google.com/
- Collected Eric Rbs
- Rework TLB to rely on VMID/ASID instead of an extra key.
- Fixed TLB issue with large stage-1 reported by Julian.
- Cap the OAS to 48 bits as PTW doesn’t support 52 bits.
- Fix ASID/VMID representation in some contexts as 16 bits while
they can be -1
- Increase visibility in trace points
Mostafa Saleh (18):
hw/arm/smmu-common: Add missing size check for stage-1
hw/arm/smmu: Fix IPA for stage-2 events
hw/arm/smmuv3: Fix encoding of CLASS in events
hw/arm/smmu: Use enum for SMMU stage
hw/arm/smmu: Split smmuv3_translate()
hw/arm/smmu: Consolidate ASID and VMID types
hw/arm/smmu: Introduce CACHED_ENTRY_TO_ADDR
hw/arm/smmuv3: Translate CD and TT using stage-2 table
hw/arm/smmu-common: Rework TLB lookup for nesting
hw/arm/smmu-common: Add support for nested TLB
hw/arm/smmu-common: Support nested translation
hw/arm/smmu: Support nesting in smmuv3_range_inval()
hw/arm/smmu: Introduce smmu_iotlb_inv_asid_vmid
hw/arm/smmu: Support nesting in the rest of commands
hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova()
hw/arm/smmuv3: Handle translation faults according to SMMUPTWEventInfo
hw/arm/smmuv3: Support and advertise nesting
hw/arm/smmu: Refactor SMMU OAS
hw/arm/smmu-common.c | 312 ++++++++++++++++++++---
hw/arm/smmuv3-internal.h | 19 +-
hw/arm/smmuv3.c | 467 +++++++++++++++++++++++------------
hw/arm/trace-events | 26 +-
include/hw/arm/smmu-common.h | 46 +++-
5 files changed, 640 insertions(+), 230 deletions(-)
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v5 01/18] hw/arm/smmu-common: Add missing size check for stage-1
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 02/18] hw/arm/smmu: Fix IPA for stage-2 events Mostafa Saleh
` (17 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
According to the SMMU architecture specification (ARM IHI 0070 F.b),
in “3.4 Address sizes”
The address output from the translation causes a stage 1 Address Size
fault if it exceeds the range of the effective IPA size for the given CD.
However, this check was missing.
There is already a similar check for stage-2 against effective PA.
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 1ce706bf94..eb2356bc35 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -381,6 +381,16 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
goto error;
}
+ /*
+ * The address output from the translation causes a stage 1 Address
+ * Size fault if it exceeds the range of the effective IPA size for
+ * the given CD.
+ */
+ if (gpa >= (1ULL << cfg->oas)) {
+ info->type = SMMU_PTW_ERR_ADDR_SIZE;
+ goto error;
+ }
+
tlbe->entry.translated_addr = gpa;
tlbe->entry.iova = iova & ~mask;
tlbe->entry.addr_mask = mask;
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 02/18] hw/arm/smmu: Fix IPA for stage-2 events
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 01/18] hw/arm/smmu-common: Add missing size check for stage-1 Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events Mostafa Saleh
` (16 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
For the following events (ARM IHI 0070 F.b - 7.3 Event records):
- F_TRANSLATION
- F_ACCESS
- F_PERMISSION
- F_ADDR_SIZE
If fault occurs at stage 2, S2 == 1 and:
- If translating an IPA for a transaction (whether by input to
stage 2-only configuration, or after successful stage 1 translation),
CLASS == IN, and IPA is provided.
At the moment only CLASS == IN is used which indicates input
translation.
However, this was not implemented correctly, as for stage 2, the code
only sets the S2 bit but not the IPA.
This field has the same bits as FetchAddr in F_WALK_EABT which is
populated correctly, so we don’t change that.
The setting of this field should be done from the walker as the IPA address
wouldn't be known in case of nesting.
For stage 1, the spec says:
If fault occurs at stage 1, S2 == 0 and:
CLASS == IN, IPA is UNKNOWN.
So, no need to set it to for stage 1, as ptw_info is initialised by zero in
smmuv3_translate().
Fixes: e703f7076a “hw/arm/smmuv3: Add page table walk for stage-2”
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 10 ++++++----
hw/arm/smmuv3.c | 4 ++++
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index eb2356bc35..8a8c718e6b 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -448,7 +448,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
*/
if (ipa >= (1ULL << inputsize)) {
info->type = SMMU_PTW_ERR_TRANSLATION;
- goto error;
+ goto error_ipa;
}
while (level < VMSA_LEVELS) {
@@ -494,13 +494,13 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
*/
if (!PTE_AF(pte) && !cfg->s2cfg.affd) {
info->type = SMMU_PTW_ERR_ACCESS;
- goto error;
+ goto error_ipa;
}
s2ap = PTE_AP(pte);
if (is_permission_fault_s2(s2ap, perm)) {
info->type = SMMU_PTW_ERR_PERMISSION;
- goto error;
+ goto error_ipa;
}
/*
@@ -509,7 +509,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
*/
if (gpa >= (1ULL << cfg->s2cfg.eff_ps)) {
info->type = SMMU_PTW_ERR_ADDR_SIZE;
- goto error;
+ goto error_ipa;
}
tlbe->entry.translated_addr = gpa;
@@ -522,6 +522,8 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
}
info->type = SMMU_PTW_ERR_TRANSLATION;
+error_ipa:
+ info->addr = ipa;
error:
info->stage = 2;
tlbe->entry.perm = IOMMU_NONE;
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 2d1e0d55ec..9dd3ea48e4 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -949,6 +949,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
if (PTW_RECORD_FAULT(cfg)) {
event.type = SMMU_EVT_F_TRANSLATION;
event.u.f_translation.addr = addr;
+ event.u.f_translation.addr2 = ptw_info.addr;
event.u.f_translation.rnw = flag & 0x1;
}
break;
@@ -956,6 +957,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
if (PTW_RECORD_FAULT(cfg)) {
event.type = SMMU_EVT_F_ADDR_SIZE;
event.u.f_addr_size.addr = addr;
+ event.u.f_addr_size.addr2 = ptw_info.addr;
event.u.f_addr_size.rnw = flag & 0x1;
}
break;
@@ -963,6 +965,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
if (PTW_RECORD_FAULT(cfg)) {
event.type = SMMU_EVT_F_ACCESS;
event.u.f_access.addr = addr;
+ event.u.f_access.addr2 = ptw_info.addr;
event.u.f_access.rnw = flag & 0x1;
}
break;
@@ -970,6 +973,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
if (PTW_RECORD_FAULT(cfg)) {
event.type = SMMU_EVT_F_PERMISSION;
event.u.f_permission.addr = addr;
+ event.u.f_permission.addr2 = ptw_info.addr;
event.u.f_permission.rnw = flag & 0x1;
}
break;
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 01/18] hw/arm/smmu-common: Add missing size check for stage-1 Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 02/18] hw/arm/smmu: Fix IPA for stage-2 events Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-17 15:07 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 04/18] hw/arm/smmu: Use enum for SMMU stage Mostafa Saleh
` (15 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
The SMMUv3 spec (ARM IHI 0070 F.b - 7.3 Event records) defines the
class of events faults as:
CLASS: The class of the operation that caused the fault:
- 0b00: CD, CD fetch.
- 0b01: TTD, Stage 1 translation table fetch.
- 0b10: IN, Input address
However, this value was not set and left as 0 which means CD and not
IN (0b10).
Another problem was that stage-2 class is considered IN not TT for
EABT, according to the spec:
Translation of an IPA after successful stage 1 translation (or,
in stage 2-only configuration, an input IPA)
- S2 == 1 (stage 2), CLASS == IN (Input to stage)
This would change soon when nested translations are supported.
While at it, add an enum for class as it would be used for nesting.
However, at the moment stage-1 and stage-2 use the same class values,
except for EABT.
Fixes: 9bde7f0674 “hw/arm/smmuv3: Implement translate callback”
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmuv3-internal.h | 6 ++++++
hw/arm/smmuv3.c | 8 +++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index e4dd11e1e6..0f3ecec804 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -32,6 +32,12 @@ typedef enum SMMUTranslationStatus {
SMMU_TRANS_SUCCESS,
} SMMUTranslationStatus;
+typedef enum SMMUTranslationClass {
+ SMMU_CLASS_CD,
+ SMMU_CLASS_TT,
+ SMMU_CLASS_IN,
+} SMMUTranslationClass;
+
/* MMIO Registers */
REG32(IDR0, 0x0)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 9dd3ea48e4..3d214c9f57 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -942,7 +942,9 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
event.type = SMMU_EVT_F_WALK_EABT;
event.u.f_walk_eabt.addr = addr;
event.u.f_walk_eabt.rnw = flag & 0x1;
- event.u.f_walk_eabt.class = 0x1;
+ /* Stage-2 (only) is class IN while stage-1 is class TT */
+ event.u.f_walk_eabt.class = (ptw_info.stage == 2) ?
+ SMMU_CLASS_IN : SMMU_CLASS_TT;
event.u.f_walk_eabt.addr2 = ptw_info.addr;
break;
case SMMU_PTW_ERR_TRANSLATION:
@@ -950,6 +952,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
event.type = SMMU_EVT_F_TRANSLATION;
event.u.f_translation.addr = addr;
event.u.f_translation.addr2 = ptw_info.addr;
+ event.u.f_translation.class = SMMU_CLASS_IN;
event.u.f_translation.rnw = flag & 0x1;
}
break;
@@ -958,6 +961,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
event.type = SMMU_EVT_F_ADDR_SIZE;
event.u.f_addr_size.addr = addr;
event.u.f_addr_size.addr2 = ptw_info.addr;
+ event.u.f_translation.class = SMMU_CLASS_IN;
event.u.f_addr_size.rnw = flag & 0x1;
}
break;
@@ -966,6 +970,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
event.type = SMMU_EVT_F_ACCESS;
event.u.f_access.addr = addr;
event.u.f_access.addr2 = ptw_info.addr;
+ event.u.f_translation.class = SMMU_CLASS_IN;
event.u.f_access.rnw = flag & 0x1;
}
break;
@@ -974,6 +979,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
event.type = SMMU_EVT_F_PERMISSION;
event.u.f_permission.addr = addr;
event.u.f_permission.addr2 = ptw_info.addr;
+ event.u.f_translation.class = SMMU_CLASS_IN;
event.u.f_permission.rnw = flag & 0x1;
}
break;
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 04/18] hw/arm/smmu: Use enum for SMMU stage
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (2 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 05/18] hw/arm/smmu: Split smmuv3_translate() Mostafa Saleh
` (14 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
Currently, translation stage is represented as an int, where 1 is stage-1 and
2 is stage-2, when nested is added, 3 would be confusing to represent nesting,
so we use an enum instead.
While keeping the same values, this is useful for:
- Doing tricks with bit masks, where BIT(0) is stage-1 and BIT(1) is
stage-2 and both is nested.
- Tracing, as stage is printed as int.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 14 +++++++-------
hw/arm/smmuv3.c | 17 +++++++++--------
include/hw/arm/smmu-common.h | 11 +++++++++--
3 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 8a8c718e6b..8a5858f69f 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -304,7 +304,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
dma_addr_t baseaddr, indexmask;
- int stage = cfg->stage;
+ SMMUStage stage = cfg->stage;
SMMUTransTableInfo *tt = select_tt(cfg, iova);
uint8_t level, granule_sz, inputsize, stride;
@@ -402,7 +402,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
info->type = SMMU_PTW_ERR_TRANSLATION;
error:
- info->stage = 1;
+ info->stage = SMMU_STAGE_1;
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
@@ -425,7 +425,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
dma_addr_t ipa, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
- const int stage = 2;
+ const SMMUStage stage = SMMU_STAGE_2;
int granule_sz = cfg->s2cfg.granule_sz;
/* ARM DDI0487I.a: Table D8-7. */
int inputsize = 64 - cfg->s2cfg.tsz;
@@ -525,7 +525,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
error_ipa:
info->addr = ipa;
error:
- info->stage = 2;
+ info->stage = SMMU_STAGE_2;
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
@@ -544,9 +544,9 @@ error:
int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
- if (cfg->stage == 1) {
+ if (cfg->stage == SMMU_STAGE_1) {
return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
- } else if (cfg->stage == 2) {
+ } else if (cfg->stage == SMMU_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
@@ -555,7 +555,7 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
*/
if (iova >= (1ULL << cfg->oas)) {
info->type = SMMU_PTW_ERR_ADDR_SIZE;
- info->stage = 1;
+ info->stage = SMMU_STAGE_1;
tlbe->entry.perm = IOMMU_NONE;
return -EINVAL;
}
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 3d214c9f57..7e9874b4a6 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -34,7 +34,8 @@
#include "smmuv3-internal.h"
#include "smmu-internal.h"
-#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == 1) ? (cfg)->record_faults : \
+#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == SMMU_STAGE_1) ? \
+ (cfg)->record_faults : \
(cfg)->s2cfg.record_faults)
/**
@@ -402,7 +403,7 @@ static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran)
static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
{
- cfg->stage = 2;
+ cfg->stage = SMMU_STAGE_2;
if (STE_S2AA64(ste) == 0x0) {
qemu_log_mask(LOG_UNIMP,
@@ -678,7 +679,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event)
/* we support only those at the moment */
cfg->aa64 = true;
- cfg->stage = 1;
+ cfg->stage = SMMU_STAGE_1;
cfg->oas = oas2bits(CD_IPS(cd));
cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas);
@@ -762,7 +763,7 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg,
return ret;
}
- if (cfg->aborted || cfg->bypassed || (cfg->stage == 2)) {
+ if (cfg->aborted || cfg->bypassed || (cfg->stage == SMMU_STAGE_2)) {
return 0;
}
@@ -882,7 +883,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
goto epilogue;
}
- if (cfg->stage == 1) {
+ if (cfg->stage == SMMU_STAGE_1) {
/* Select stage1 translation table. */
tt = select_tt(cfg, addr);
if (!tt) {
@@ -919,7 +920,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
* 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);
+ event.u.f_walk_eabt.s2 = (cfg->stage == SMMU_STAGE_2);
if (PTW_RECORD_FAULT(cfg)) {
event.type = SMMU_EVT_F_PERMISSION;
event.u.f_permission.addr = addr;
@@ -935,7 +936,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
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);
+ event.u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
g_free(cached_entry);
switch (ptw_info.type) {
case SMMU_PTW_ERR_WALK_EABT:
@@ -943,7 +944,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
event.u.f_walk_eabt.addr = addr;
event.u.f_walk_eabt.rnw = flag & 0x1;
/* Stage-2 (only) is class IN while stage-1 is class TT */
- event.u.f_walk_eabt.class = (ptw_info.stage == 2) ?
+ event.u.f_walk_eabt.class = (ptw_info.stage == SMMU_STAGE_2) ?
SMMU_CLASS_IN : SMMU_CLASS_TT;
event.u.f_walk_eabt.addr2 = ptw_info.addr;
break;
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 5ec2e6c1a4..b3c881f0ee 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -49,8 +49,15 @@ typedef enum {
SMMU_PTW_ERR_PERMISSION, /* Permission fault */
} SMMUPTWEventType;
+/* SMMU Stage */
+typedef enum {
+ SMMU_STAGE_1 = 1,
+ SMMU_STAGE_2,
+ SMMU_NESTED,
+} SMMUStage;
+
typedef struct SMMUPTWEventInfo {
- int stage;
+ SMMUStage stage;
SMMUPTWEventType type;
dma_addr_t addr; /* fetched address that induced an abort, if any */
} SMMUPTWEventInfo;
@@ -88,7 +95,7 @@ typedef struct SMMUS2Cfg {
*/
typedef struct SMMUTransCfg {
/* Shared fields between stage-1 and stage-2. */
- int stage; /* translation stage */
+ SMMUStage stage; /* translation stage */
bool disabled; /* smmu is disabled */
bool bypassed; /* translation is bypassed */
bool aborted; /* translation is aborted */
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 05/18] hw/arm/smmu: Split smmuv3_translate()
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (3 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 04/18] hw/arm/smmu: Use enum for SMMU stage Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 06/18] hw/arm/smmu: Consolidate ASID and VMID types Mostafa Saleh
` (13 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
smmuv3_translate() does everything from STE/CD parsing to TLB lookup
and PTW.
Soon, when nesting is supported, stage-1 data (tt, CD) needs to be
translated using stage-2.
Split smmuv3_translate() to 3 functions:
- smmu_translate(): in smmu-common.c, which does the TLB lookup, PTW,
TLB insertion, all the functions are already there, this just puts
them together.
This also simplifies the code as it consolidates event generation
in case of TLB lookup permission failure or in TT selection.
- smmuv3_do_translate(): in smmuv3.c, Calls smmu_translate() and does
the event population in case of errors.
- smmuv3_translate(), now calls smmuv3_do_translate() for
translation while the rest is the same.
Also, add stage in trace_smmuv3_translate_success()
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 59 +++++++++++
hw/arm/smmuv3.c | 194 +++++++++++++----------------------
hw/arm/trace-events | 2 +-
include/hw/arm/smmu-common.h | 8 ++
4 files changed, 142 insertions(+), 121 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 8a5858f69f..d94db6b34f 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -566,6 +566,65 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
g_assert_not_reached();
}
+SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
+ IOMMUAccessFlags flag, SMMUPTWEventInfo *info)
+{
+ uint64_t page_mask, aligned_addr;
+ SMMUTLBEntry *cached_entry = NULL;
+ SMMUTransTableInfo *tt;
+ int status;
+
+ /*
+ * Combined attributes used for TLB lookup, as only one stage is supported,
+ * it will hold attributes based on the enabled stage.
+ */
+ SMMUTransTableInfo tt_combined;
+
+ if (cfg->stage == SMMU_STAGE_1) {
+ /* Select stage1 translation table. */
+ tt = select_tt(cfg, addr);
+ if (!tt) {
+ info->type = SMMU_PTW_ERR_TRANSLATION;
+ info->stage = SMMU_STAGE_1;
+ return NULL;
+ }
+ tt_combined.granule_sz = tt->granule_sz;
+ tt_combined.tsz = tt->tsz;
+
+ } 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_combined, aligned_addr);
+ if (cached_entry) {
+ if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
+ info->type = SMMU_PTW_ERR_PERMISSION;
+ info->stage = cfg->stage;
+ return NULL;
+ }
+ return cached_entry;
+ }
+
+ cached_entry = g_new0(SMMUTLBEntry, 1);
+ status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info);
+ if (status) {
+ g_free(cached_entry);
+ return NULL;
+ }
+ smmu_iotlb_insert(bs, cfg, cached_entry);
+ return cached_entry;
+}
+
/**
* The bus number is used for lookup when SID based invalidation occurs.
* In that case we lazily populate the SMMUPciBus array from the bus hash
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 7e9874b4a6..85a3efd357 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -827,6 +827,76 @@ static void smmuv3_flush_config(SMMUDevice *sdev)
g_hash_table_remove(bc->configs, sdev);
}
+/* Do translation with TLB lookup. */
+static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
+ SMMUTransCfg *cfg,
+ SMMUEventInfo *event,
+ IOMMUAccessFlags flag,
+ SMMUTLBEntry **out_entry)
+{
+ SMMUPTWEventInfo ptw_info = {};
+ SMMUState *bs = ARM_SMMU(s);
+ SMMUTLBEntry *cached_entry = NULL;
+
+ cached_entry = smmu_translate(bs, cfg, addr, flag, &ptw_info);
+ if (!cached_entry) {
+ /* All faults from PTW has S2 field. */
+ event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
+ switch (ptw_info.type) {
+ case SMMU_PTW_ERR_WALK_EABT:
+ event->type = SMMU_EVT_F_WALK_EABT;
+ event->u.f_walk_eabt.addr = addr;
+ event->u.f_walk_eabt.rnw = flag & 0x1;
+ event->u.f_walk_eabt.class = (ptw_info.stage == SMMU_STAGE_2) ?
+ SMMU_CLASS_IN : SMMU_CLASS_TT;
+ event->u.f_walk_eabt.addr2 = ptw_info.addr;
+ break;
+ case SMMU_PTW_ERR_TRANSLATION:
+ if (PTW_RECORD_FAULT(cfg)) {
+ event->type = SMMU_EVT_F_TRANSLATION;
+ event->u.f_translation.addr = addr;
+ event->u.f_translation.addr2 = ptw_info.addr;
+ event->u.f_translation.class = SMMU_CLASS_IN;
+ event->u.f_translation.rnw = flag & 0x1;
+ }
+ break;
+ case SMMU_PTW_ERR_ADDR_SIZE:
+ if (PTW_RECORD_FAULT(cfg)) {
+ event->type = SMMU_EVT_F_ADDR_SIZE;
+ event->u.f_addr_size.addr = addr;
+ event->u.f_addr_size.addr2 = ptw_info.addr;
+ event->u.f_addr_size.class = SMMU_CLASS_IN;
+ event->u.f_addr_size.rnw = flag & 0x1;
+ }
+ break;
+ case SMMU_PTW_ERR_ACCESS:
+ if (PTW_RECORD_FAULT(cfg)) {
+ event->type = SMMU_EVT_F_ACCESS;
+ event->u.f_access.addr = addr;
+ event->u.f_access.addr2 = ptw_info.addr;
+ event->u.f_access.class = SMMU_CLASS_IN;
+ event->u.f_access.rnw = flag & 0x1;
+ }
+ break;
+ case SMMU_PTW_ERR_PERMISSION:
+ if (PTW_RECORD_FAULT(cfg)) {
+ event->type = SMMU_EVT_F_PERMISSION;
+ event->u.f_permission.addr = addr;
+ event->u.f_permission.addr2 = ptw_info.addr;
+ event->u.f_permission.class = SMMU_CLASS_IN;
+ event->u.f_permission.rnw = flag & 0x1;
+ }
+ break;
+ default:
+ g_assert_not_reached();
+ }
+ return SMMU_TRANS_ERROR;
+ }
+ *out_entry = cached_entry;
+ return SMMU_TRANS_SUCCESS;
+}
+
+/* Entry point to SMMU, does everything. */
static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
IOMMUAccessFlags flag, int iommu_idx)
{
@@ -836,12 +906,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
SMMUEventInfo event = {.type = SMMU_EVT_NONE,
.sid = sid,
.inval_ste_allowed = false};
- SMMUPTWEventInfo ptw_info = {};
SMMUTranslationStatus status;
- SMMUState *bs = ARM_SMMU(s);
- uint64_t page_mask, aligned_addr;
- SMMUTLBEntry *cached_entry = NULL;
- SMMUTransTableInfo *tt;
SMMUTransCfg *cfg = NULL;
IOMMUTLBEntry entry = {
.target_as = &address_space_memory,
@@ -850,11 +915,7 @@ 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;
+ SMMUTLBEntry *cached_entry = NULL;
qemu_mutex_lock(&s->mutex);
@@ -883,115 +944,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
goto epilogue;
}
- if (cfg->stage == SMMU_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;
- }
- tt_combined.granule_sz = tt->granule_sz;
- tt_combined.tsz = tt->tsz;
-
- } 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_combined, aligned_addr);
- if (cached_entry) {
- if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
- status = SMMU_TRANS_ERROR;
- /*
- * 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 == SMMU_STAGE_2);
- if (PTW_RECORD_FAULT(cfg)) {
- event.type = SMMU_EVT_F_PERMISSION;
- event.u.f_permission.addr = addr;
- event.u.f_permission.rnw = flag & 0x1;
- }
- } else {
- status = SMMU_TRANS_SUCCESS;
- }
- goto epilogue;
- }
-
- 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 == SMMU_STAGE_2);
- g_free(cached_entry);
- switch (ptw_info.type) {
- case SMMU_PTW_ERR_WALK_EABT:
- event.type = SMMU_EVT_F_WALK_EABT;
- event.u.f_walk_eabt.addr = addr;
- event.u.f_walk_eabt.rnw = flag & 0x1;
- /* Stage-2 (only) is class IN while stage-1 is class TT */
- event.u.f_walk_eabt.class = (ptw_info.stage == SMMU_STAGE_2) ?
- SMMU_CLASS_IN : SMMU_CLASS_TT;
- event.u.f_walk_eabt.addr2 = ptw_info.addr;
- break;
- case SMMU_PTW_ERR_TRANSLATION:
- if (PTW_RECORD_FAULT(cfg)) {
- event.type = SMMU_EVT_F_TRANSLATION;
- event.u.f_translation.addr = addr;
- event.u.f_translation.addr2 = ptw_info.addr;
- event.u.f_translation.class = SMMU_CLASS_IN;
- event.u.f_translation.rnw = flag & 0x1;
- }
- break;
- case SMMU_PTW_ERR_ADDR_SIZE:
- if (PTW_RECORD_FAULT(cfg)) {
- event.type = SMMU_EVT_F_ADDR_SIZE;
- event.u.f_addr_size.addr = addr;
- event.u.f_addr_size.addr2 = ptw_info.addr;
- event.u.f_translation.class = SMMU_CLASS_IN;
- event.u.f_addr_size.rnw = flag & 0x1;
- }
- break;
- case SMMU_PTW_ERR_ACCESS:
- if (PTW_RECORD_FAULT(cfg)) {
- event.type = SMMU_EVT_F_ACCESS;
- event.u.f_access.addr = addr;
- event.u.f_access.addr2 = ptw_info.addr;
- event.u.f_translation.class = SMMU_CLASS_IN;
- event.u.f_access.rnw = flag & 0x1;
- }
- break;
- case SMMU_PTW_ERR_PERMISSION:
- if (PTW_RECORD_FAULT(cfg)) {
- event.type = SMMU_EVT_F_PERMISSION;
- event.u.f_permission.addr = addr;
- event.u.f_permission.addr2 = ptw_info.addr;
- event.u.f_translation.class = SMMU_CLASS_IN;
- event.u.f_permission.rnw = flag & 0x1;
- }
- break;
- default:
- g_assert_not_reached();
- }
- status = SMMU_TRANS_ERROR;
- } else {
- smmu_iotlb_insert(bs, cfg, cached_entry);
- status = SMMU_TRANS_SUCCESS;
- }
+ status = smmuv3_do_translate(s, addr, cfg, &event, flag, &cached_entry);
epilogue:
qemu_mutex_unlock(&s->mutex);
@@ -1002,7 +955,8 @@ epilogue:
(addr & cached_entry->entry.addr_mask);
entry.addr_mask = cached_entry->entry.addr_mask;
trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr,
- entry.translated_addr, entry.perm);
+ entry.translated_addr, entry.perm,
+ cfg->stage);
break;
case SMMU_TRANS_DISABLE:
entry.perm = flag;
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index f1a54a02df..cc12924a84 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -37,7 +37,7 @@ smmuv3_get_ste(uint64_t addr) "STE addr: 0x%"PRIx64
smmuv3_translate_disable(const char *n, uint16_t sid, uint64_t addr, bool is_write) "%s sid=0x%x bypass (smmu disabled) iova:0x%"PRIx64" is_write=%d"
smmuv3_translate_bypass(const char *n, uint16_t sid, uint64_t addr, bool is_write) "%s sid=0x%x STE bypass iova:0x%"PRIx64" is_write=%d"
smmuv3_translate_abort(const char *n, uint16_t sid, uint64_t addr, bool is_write) "%s sid=0x%x abort on iova:0x%"PRIx64" is_write=%d"
-smmuv3_translate_success(const char *n, uint16_t sid, uint64_t iova, uint64_t translated, int perm) "%s sid=0x%x iova=0x%"PRIx64" translated=0x%"PRIx64" perm=0x%x"
+smmuv3_translate_success(const char *n, uint16_t sid, uint64_t iova, uint64_t translated, int perm, int stage) "%s sid=0x%x iova=0x%"PRIx64" translated=0x%"PRIx64" perm=0x%x stage=%d"
smmuv3_get_cd(uint64_t addr) "CD addr: 0x%"PRIx64
smmuv3_decode_cd(uint32_t oas) "oas=%d"
smmuv3_decode_cd_tt(int i, uint32_t tsz, uint64_t ttb, uint32_t granule_sz, bool had) "TT[%d]:tsz:%d ttb:0x%"PRIx64" granule_sz:%d had:%d"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index b3c881f0ee..5944735632 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -183,6 +183,14 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev)
int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info);
+
+/*
+ * smmu_translate - Look for a translation in TLB, if not, do a PTW.
+ * Returns NULL on PTW error or incase of TLB permission errors.
+ */
+SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
+ IOMMUAccessFlags flag, SMMUPTWEventInfo *info);
+
/**
* select_tt - compute which translation table shall be used according to
* the input iova and translation config and return the TT specific info
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 06/18] hw/arm/smmu: Consolidate ASID and VMID types
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (4 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 05/18] hw/arm/smmu: Split smmuv3_translate() Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 07/18] hw/arm/smmu: Introduce CACHED_ENTRY_TO_ADDR Mostafa Saleh
` (12 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
ASID and VMID used to be uint16_t in the translation config, however,
in other contexts they can be int as -1 in case of TLB invalidation,
to represent all (don’t care).
When stage-2 was added asid was set to -1 in stage-2 and vmid to -1
in stage-1 configs. However, that meant they were set as (65536),
this was not an issue as nesting was not supported and no
commands/lookup uses both.
With nesting, it’s critical to get this right as translation must be
tagged correctly with ASID/VMID, and with ASID=-1 meaning stage-2.
Represent ASID/VMID everywhere as int.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 10 +++++-----
hw/arm/smmuv3.c | 4 ++--
hw/arm/trace-events | 18 +++++++++---------
include/hw/arm/smmu-common.h | 14 +++++++-------
4 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index d94db6b34f..21982621c0 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -57,7 +57,7 @@ static gboolean smmu_iotlb_key_equal(gconstpointer v1, gconstpointer v2)
(k1->vmid == k2->vmid);
}
-SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
+SMMUIOTLBKey smmu_get_iotlb_key(int asid, int vmid, uint64_t iova,
uint8_t tg, uint8_t level)
{
SMMUIOTLBKey key = {.asid = asid, .vmid = vmid, .iova = iova,
@@ -130,7 +130,7 @@ void smmu_iotlb_inv_all(SMMUState *s)
static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
gpointer user_data)
{
- uint16_t asid = *(uint16_t *)user_data;
+ int asid = *(int *)user_data;
SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
return SMMU_IOTLB_ASID(*iotlb_key) == asid;
@@ -139,7 +139,7 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
gpointer user_data)
{
- uint16_t vmid = *(uint16_t *)user_data;
+ int vmid = *(int *)user_data;
SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
@@ -191,13 +191,13 @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
&info);
}
-void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
+void smmu_iotlb_inv_asid(SMMUState *s, int asid)
{
trace_smmu_iotlb_inv_asid(asid);
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
}
-void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
+void smmu_iotlb_inv_vmid(SMMUState *s, int vmid)
{
trace_smmu_iotlb_inv_vmid(vmid);
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 85a3efd357..11cd12e32f 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1244,7 +1244,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
}
case SMMU_CMD_TLBI_NH_ASID:
{
- uint16_t asid = CMD_ASID(&cmd);
+ int asid = CMD_ASID(&cmd);
if (!STAGE1_SUPPORTED(s)) {
cmd_error = SMMU_CERROR_ILL;
@@ -1277,7 +1277,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
break;
case SMMU_CMD_TLBI_S12_VMALL:
{
- uint16_t vmid = CMD_VMID(&cmd);
+ int vmid = CMD_VMID(&cmd);
if (!STAGE2_SUPPORTED(s)) {
cmd_error = SMMU_CERROR_ILL;
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index cc12924a84..09ccd39548 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -11,13 +11,13 @@ smmu_ptw_page_pte(int stage, int level, uint64_t iova, uint64_t baseaddr, uint6
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"
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_iotlb_inv_asid(int asid) "IOTLB invalidate asid=%d"
+smmu_iotlb_inv_vmid(int vmid) "IOTLB invalidate vmid=%d"
+smmu_iotlb_inv_iova(int 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"
-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"
+smmu_iotlb_lookup_hit(int asid, int 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(int asid, int 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(int asid, int 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)"
@@ -48,12 +48,12 @@ smmuv3_config_cache_hit(uint32_t sid, uint32_t hits, uint32_t misses, uint32_t p
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_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_cmdq_tlbi_nh_asid(int asid) "asid=%d"
+smmuv3_cmdq_tlbi_s12_vmid(int 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, 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
+smmuv3_inv_notifiers_iova(const char *name, int asid, int 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
# strongarm.c
strongarm_uart_update_parameters(const char *label, int speed, char parity, int data_bits, int stop_bits) "%s speed=%d parity=%c data=%d stop=%d"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 5944735632..96eb017e50 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -84,7 +84,7 @@ typedef struct SMMUS2Cfg {
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) */
+ int vmid; /* Virtual Machine ID (S2VMID) */
uint64_t vttb; /* Address of translation table base (S2TTB) */
} SMMUS2Cfg;
@@ -108,7 +108,7 @@ typedef struct SMMUTransCfg {
uint64_t ttb; /* TT base address */
uint8_t oas; /* output address width */
uint8_t tbi; /* Top Byte Ignore */
- uint16_t asid;
+ int asid;
SMMUTransTableInfo tt[2];
/* Used by stage-2 only. */
struct SMMUS2Cfg s2cfg;
@@ -132,8 +132,8 @@ typedef struct SMMUPciBus {
typedef struct SMMUIOTLBKey {
uint64_t iova;
- uint16_t asid;
- uint16_t vmid;
+ int asid;
+ int vmid;
uint8_t tg;
uint8_t level;
} SMMUIOTLBKey;
@@ -205,11 +205,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, uint16_t vmid, uint64_t iova,
+SMMUIOTLBKey smmu_get_iotlb_key(int asid, int 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_asid(SMMUState *s, int asid);
+void smmu_iotlb_inv_vmid(SMMUState *s, int 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.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 07/18] hw/arm/smmu: Introduce CACHED_ENTRY_TO_ADDR
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (5 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 06/18] hw/arm/smmu: Consolidate ASID and VMID types Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 08/18] hw/arm/smmuv3: Translate CD and TT using stage-2 table Mostafa Saleh
` (11 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
Soon, smmuv3_do_translate() will be used to translate the CD and the
TTBx, instead of re-writting the same logic to convert the returned
cached entry to an address, add a new macro CACHED_ENTRY_TO_ADDR.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmuv3.c | 3 +--
include/hw/arm/smmu-common.h | 3 +++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 11cd12e32f..3f2dfada44 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -951,8 +951,7 @@ epilogue:
switch (status) {
case SMMU_TRANS_SUCCESS:
entry.perm = cached_entry->entry.perm;
- entry.translated_addr = cached_entry->entry.translated_addr +
- (addr & cached_entry->entry.addr_mask);
+ entry.translated_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr);
entry.addr_mask = cached_entry->entry.addr_mask;
trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr,
entry.translated_addr, entry.perm,
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 96eb017e50..eecbebaaac 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -37,6 +37,9 @@
#define VMSA_IDXMSK(isz, strd, lvl) ((1ULL << \
VMSA_BIT_LVL(isz, strd, lvl)) - 1)
+#define CACHED_ENTRY_TO_ADDR(ent, addr) ((ent)->entry.translated_addr + \
+ ((addr) & (ent)->entry.addr_mask))
+
/*
* Page table walk error types
*/
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 08/18] hw/arm/smmuv3: Translate CD and TT using stage-2 table
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (6 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 07/18] hw/arm/smmu: Introduce CACHED_ENTRY_TO_ADDR Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-17 15:18 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 09/18] hw/arm/smmu-common: Rework TLB lookup for nesting Mostafa Saleh
` (10 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
According to ARM SMMU architecture specification (ARM IHI 0070 F.b),
In "5.2 Stream Table Entry":
[51:6] S1ContextPtr
If Config[1] == 1 (stage 2 enabled), this pointer is an IPA translated by
stage 2 and the programmed value must be within the range of the IAS.
In "5.4.1 CD notes":
The translation table walks performed from TTB0 or TTB1 are always performed
in IPA space if stage 2 translations are enabled.
This patch implements translation of the S1 context descriptor pointer and
TTBx base addresses through the S2 stage (IPA -> PA)
smmuv3_do_translate() is updated to have one arg which is translation
class, this is useful to:
- Decide wether a translation is stage-2 only or use the STE config.
- Populate the class in case of faults, WALK_EABT is left unchanged
for stage-1 as it is always IN, while stage-2 would match the
used class (TT, IN, CD), this will change slightly when the ptw
supports nested translation as it can also issue TT event with
class IN.
In case for stage-2 only translation, used in the context of nested
translation, the stage and asid are saved and restored before and
after calling smmu_translate().
Translating CD or TTBx can fail for the following reasons:
1) Large address size: This is described in
(3.4.3 Address sizes of SMMU-originated accesses)
- For CD ptr larger than IAS, for SMMUv3.1, it can trigger either
C_BAD_STE or Translation fault, we implement the latter as it
requires no extra code.
- For TTBx, if larger than the effective stage 1 output address size, it
triggers C_BAD_CD.
2) Faults from PTWs (7.3 Event records)
- F_ADDR_SIZE: large address size after first level causes stage 2 Address
Size fault (Also in 3.4.3 Address sizes of SMMU-originated accesses)
- F_PERMISSION: Same as an address translation. However, when
CLASS == CD, the access is implicitly Data and a read.
- F_ACCESS: Same as an address translation.
- F_TRANSLATION: Same as an address translation.
- F_WALK_EABT: Same as an address translation.
These are already implemented in the PTW logic, so no extra handling
required.
As in CD and TTBx translation context, the iova is not known, setting
the InputAddr was removed from "smmuv3_do_translate" and set after
from "smmuv3_translate" with the new function "smmuv3_fixup_event"
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmuv3.c | 120 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 103 insertions(+), 17 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 3f2dfada44..73d5a25705 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -337,14 +337,35 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf,
}
+static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
+ SMMUTransCfg *cfg,
+ SMMUEventInfo *event,
+ IOMMUAccessFlags flag,
+ SMMUTLBEntry **out_entry,
+ SMMUTranslationClass class);
/* @ssid > 0 not supported yet */
-static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid,
- CD *buf, SMMUEventInfo *event)
+static int smmu_get_cd(SMMUv3State *s, STE *ste, SMMUTransCfg *cfg,
+ uint32_t ssid, CD *buf, SMMUEventInfo *event)
{
dma_addr_t addr = STE_CTXPTR(ste);
int ret, i;
+ SMMUTranslationStatus status;
+ SMMUTLBEntry *entry;
trace_smmuv3_get_cd(addr);
+
+ if (cfg->stage == SMMU_NESTED) {
+ status = smmuv3_do_translate(s, addr, cfg, event,
+ IOMMU_RO, &entry, SMMU_CLASS_CD);
+
+ /* Same PTW faults are reported but with CLASS = CD. */
+ if (status != SMMU_TRANS_SUCCESS) {
+ return -EINVAL;
+ }
+
+ addr = CACHED_ENTRY_TO_ADDR(entry, addr);
+ }
+
/* TODO: guarantee 64-bit single-copy atomicity */
ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf),
MEMTXATTRS_UNSPECIFIED);
@@ -659,10 +680,13 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
return 0;
}
-static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event)
+static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
+ CD *cd, SMMUEventInfo *event)
{
int ret = -EINVAL;
int i;
+ SMMUTranslationStatus status;
+ SMMUTLBEntry *entry;
if (!CD_VALID(cd) || !CD_AARCH64(cd)) {
goto bad_cd;
@@ -713,9 +737,26 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event)
tt->tsz = tsz;
tt->ttb = CD_TTB(cd, i);
+
if (tt->ttb & ~(MAKE_64BIT_MASK(0, cfg->oas))) {
goto bad_cd;
}
+
+ /* Translate the TTBx, from IPA to PA if nesting is enabled. */
+ if (cfg->stage == SMMU_NESTED) {
+ status = smmuv3_do_translate(s, tt->ttb, cfg, event, IOMMU_RO,
+ &entry, SMMU_CLASS_TT);
+ /*
+ * Same PTW faults are reported but with CLASS = TT.
+ * If TTBx is larger than the effective stage 1 output addres
+ * size, it reports C_BAD_CD, which is handled by the above case.
+ */
+ if (status != SMMU_TRANS_SUCCESS) {
+ return -EINVAL;
+ }
+ tt->ttb = CACHED_ENTRY_TO_ADDR(entry, tt->ttb);
+ }
+
tt->had = CD_HAD(cd, i);
trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb, tt->granule_sz, tt->had);
}
@@ -767,12 +808,12 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg,
return 0;
}
- ret = smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event);
+ ret = smmu_get_cd(s, &ste, cfg, 0 /* ssid */, &cd, event);
if (ret) {
return ret;
}
- return decode_cd(cfg, &cd, event);
+ return decode_cd(s, cfg, &cd, event);
}
/**
@@ -832,58 +873,80 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
SMMUTransCfg *cfg,
SMMUEventInfo *event,
IOMMUAccessFlags flag,
- SMMUTLBEntry **out_entry)
+ SMMUTLBEntry **out_entry,
+ SMMUTranslationClass class)
{
SMMUPTWEventInfo ptw_info = {};
SMMUState *bs = ARM_SMMU(s);
SMMUTLBEntry *cached_entry = NULL;
+ int asid, stage;
+ bool desc_s2_translation = class != SMMU_CLASS_IN;
+
+ /*
+ * The function uses the argument class to identify which stage is used:
+ * - CLASS = IN: Means an input translation, determine the stage from STE.
+ * - CLASS = CD: Means the addr is an IPA of the CD, and it would be
+ * translated using the stage-2.
+ * - CLASS = TT: Means the addr is an IPA of the stage-1 translation table
+ * and it would be translated using the stage-2.
+ * For the last 2 cases instead of having intrusive changes in the common
+ * logic, we modify the cfg to be a stage-2 translation only in case of
+ * nested, and then restore it after.
+ */
+ if (desc_s2_translation) {
+ asid = cfg->asid;
+ stage = cfg->stage;
+ cfg->asid = -1;
+ cfg->stage = SMMU_STAGE_2;
+ }
cached_entry = smmu_translate(bs, cfg, addr, flag, &ptw_info);
+
+ if (desc_s2_translation) {
+ cfg->asid = asid;
+ cfg->stage = stage;
+ }
+
if (!cached_entry) {
/* All faults from PTW has S2 field. */
event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
switch (ptw_info.type) {
case SMMU_PTW_ERR_WALK_EABT:
event->type = SMMU_EVT_F_WALK_EABT;
- event->u.f_walk_eabt.addr = addr;
event->u.f_walk_eabt.rnw = flag & 0x1;
event->u.f_walk_eabt.class = (ptw_info.stage == SMMU_STAGE_2) ?
- SMMU_CLASS_IN : SMMU_CLASS_TT;
+ class : SMMU_CLASS_TT;
event->u.f_walk_eabt.addr2 = ptw_info.addr;
break;
case SMMU_PTW_ERR_TRANSLATION:
if (PTW_RECORD_FAULT(cfg)) {
event->type = SMMU_EVT_F_TRANSLATION;
- event->u.f_translation.addr = addr;
event->u.f_translation.addr2 = ptw_info.addr;
- event->u.f_translation.class = SMMU_CLASS_IN;
+ event->u.f_translation.class = class;
event->u.f_translation.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_ADDR_SIZE:
if (PTW_RECORD_FAULT(cfg)) {
event->type = SMMU_EVT_F_ADDR_SIZE;
- event->u.f_addr_size.addr = addr;
event->u.f_addr_size.addr2 = ptw_info.addr;
- event->u.f_addr_size.class = SMMU_CLASS_IN;
+ event->u.f_addr_size.class = class;
event->u.f_addr_size.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_ACCESS:
if (PTW_RECORD_FAULT(cfg)) {
event->type = SMMU_EVT_F_ACCESS;
- event->u.f_access.addr = addr;
event->u.f_access.addr2 = ptw_info.addr;
- event->u.f_access.class = SMMU_CLASS_IN;
+ event->u.f_access.class = class;
event->u.f_access.rnw = flag & 0x1;
}
break;
case SMMU_PTW_ERR_PERMISSION:
if (PTW_RECORD_FAULT(cfg)) {
event->type = SMMU_EVT_F_PERMISSION;
- event->u.f_permission.addr = addr;
event->u.f_permission.addr2 = ptw_info.addr;
- event->u.f_permission.class = SMMU_CLASS_IN;
+ event->u.f_permission.class = class;
event->u.f_permission.rnw = flag & 0x1;
}
break;
@@ -896,6 +959,27 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
return SMMU_TRANS_SUCCESS;
}
+/*
+ * Sets the InputAddr for an SMMU_TRANS_ERROR, as it can't be
+ * set from all contexts, as smmuv3_get_config() can return
+ * translation faults in case of nested translation (for CD
+ * and TTBx). But in that case the iova is not known.
+ */
+static void smmuv3_fixup_event(SMMUEventInfo *event, hwaddr iova)
+{
+ switch (event->type) {
+ case SMMU_EVT_F_WALK_EABT:
+ case SMMU_EVT_F_TRANSLATION:
+ case SMMU_EVT_F_ADDR_SIZE:
+ case SMMU_EVT_F_ACCESS:
+ case SMMU_EVT_F_PERMISSION:
+ event->u.f_walk_eabt.addr = iova;
+ break;
+ default:
+ break;
+ }
+}
+
/* Entry point to SMMU, does everything. */
static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
IOMMUAccessFlags flag, int iommu_idx)
@@ -944,7 +1028,8 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
goto epilogue;
}
- status = smmuv3_do_translate(s, addr, cfg, &event, flag, &cached_entry);
+ status = smmuv3_do_translate(s, addr, cfg, &event, flag,
+ &cached_entry, SMMU_CLASS_IN);
epilogue:
qemu_mutex_unlock(&s->mutex);
@@ -975,6 +1060,7 @@ epilogue:
entry.perm);
break;
case SMMU_TRANS_ERROR:
+ smmuv3_fixup_event(&event, addr);
qemu_log_mask(LOG_GUEST_ERROR,
"%s translation failed for iova=0x%"PRIx64" (%s)\n",
mr->parent_obj.name, addr, smmu_event_string(event.type));
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 09/18] hw/arm/smmu-common: Rework TLB lookup for nesting
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (7 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 08/18] hw/arm/smmuv3: Translate CD and TT using stage-2 table Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-17 15:28 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 10/18] hw/arm/smmu-common: Add support for nested TLB Mostafa Saleh
` (9 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
In the next patch, combine_tlb() will be added which combines 2 TLB
entries into one for nested translations, which chooses the granule
and level from the smallest entry.
This means that with nested translation, an entry can be cached with
the granule of stage-2 and not stage-1.
However, currently, the lookup for an IOVA is done with input stage
granule, which is stage-1 for nested configuration, which will not
work with the above logic.
This patch reworks lookup in that case, so it falls back to stage-2
granule if no entry is found using stage-1 granule.
Also, drop aligning the iova to avoid over-aligning in case the iova
is cached with a smaller granule, the TLB lookup will align the iova
anyway for each granule and level, and the page table walker doesn't
consider the page offset bits.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 64 +++++++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 21 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 21982621c0..f224e9c1e0 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -66,8 +66,10 @@ SMMUIOTLBKey smmu_get_iotlb_key(int asid, int vmid, uint64_t iova,
return key;
}
-SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
- SMMUTransTableInfo *tt, hwaddr iova)
+static SMMUTLBEntry *smmu_iotlb_lookup_all_levels(SMMUState *bs,
+ SMMUTransCfg *cfg,
+ SMMUTransTableInfo *tt,
+ hwaddr iova)
{
uint8_t tg = (tt->granule_sz - 10) / 2;
uint8_t inputsize = 64 - tt->tsz;
@@ -88,6 +90,36 @@ SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
}
level++;
}
+ return entry;
+}
+
+/**
+ * smmu_iotlb_lookup - Look up for a TLB entry.
+ * @bs: SMMU state which includes the TLB instance
+ * @cfg: Configuration of the translation
+ * @tt: Translation table info (granule and tsz)
+ * @iova: IOVA address to lookup
+ *
+ * returns a valid entry on success, otherwise NULL.
+ * In case of nested translation, tt can be updated to include
+ * the granule of the found entry as it might different from
+ * the IOVA granule.
+ */
+SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
+ SMMUTransTableInfo *tt, hwaddr iova)
+{
+ SMMUTLBEntry *entry = NULL;
+
+ entry = smmu_iotlb_lookup_all_levels(bs, cfg, tt, iova);
+ /*
+ * For nested translation also try the s2 granule, as the TLB will insert
+ * it if the size of s2 tlb entry was smaller.
+ */
+ if (!entry && (cfg->stage == SMMU_NESTED) &&
+ (cfg->s2cfg.granule_sz != tt->granule_sz)) {
+ tt->granule_sz = cfg->s2cfg.granule_sz;
+ entry = smmu_iotlb_lookup_all_levels(bs, cfg, tt, iova);
+ }
if (entry) {
cfg->iotlb_hits++;
@@ -569,18 +601,21 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
IOMMUAccessFlags flag, SMMUPTWEventInfo *info)
{
- uint64_t page_mask, aligned_addr;
SMMUTLBEntry *cached_entry = NULL;
SMMUTransTableInfo *tt;
int status;
/*
- * Combined attributes used for TLB lookup, as only one stage is supported,
- * it will hold attributes based on the enabled stage.
+ * Combined attributes used for TLB lookup, holds the attributes for
+ * the input stage.
*/
SMMUTransTableInfo tt_combined;
- if (cfg->stage == SMMU_STAGE_1) {
+ if (cfg->stage == SMMU_STAGE_2) {
+ /* Stage2. */
+ tt_combined.granule_sz = cfg->s2cfg.granule_sz;
+ tt_combined.tsz = cfg->s2cfg.tsz;
+ } else {
/* Select stage1 translation table. */
tt = select_tt(cfg, addr);
if (!tt) {
@@ -590,22 +625,9 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
}
tt_combined.granule_sz = tt->granule_sz;
tt_combined.tsz = tt->tsz;
-
- } 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_combined, aligned_addr);
+ cached_entry = smmu_iotlb_lookup(bs, cfg, &tt_combined, addr);
if (cached_entry) {
if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
info->type = SMMU_PTW_ERR_PERMISSION;
@@ -616,7 +638,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
}
cached_entry = g_new0(SMMUTLBEntry, 1);
- status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info);
+ status = smmu_ptw(cfg, addr, flag, cached_entry, info);
if (status) {
g_free(cached_entry);
return NULL;
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 10/18] hw/arm/smmu-common: Add support for nested TLB
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (8 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 09/18] hw/arm/smmu-common: Rework TLB lookup for nesting Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 11/18] hw/arm/smmu-common: Support nested translation Mostafa Saleh
` (8 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
This patch adds support for nested (combined) TLB entries.
The main function combine_tlb() is not used here but in the next
patches, but to simplify the patches it is introduced first.
Main changes:
1) New field added in the SMMUTLBEntry struct: parent_perm, for
nested TLB, holds the stage-2 permission, this can be used to know
the origin of a permission fault from a cached entry as caching
the “and” of the permissions loses this information.
SMMUPTWEventInfo is used to hold information about PTW faults so
the event can be populated, the value of stage used to be set
based on the current stage for TLB permission faults, however
with the parent_perm, it is now set based on which perm has
the missing permission
When nesting is not enabled it has the same value as perm which
doesn't change the logic.
2) As combined TLB implementation is used, the combination logic
chooses:
- tg and level from the entry which has the smallest addr_mask.
- Based on that the iova that would be cached is recalculated.
- Translated_addr is chosen from stage-2.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 37 ++++++++++++++++++++++++++++++++----
include/hw/arm/smmu-common.h | 1 +
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index f224e9c1e0..c894c4c621 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -426,7 +426,8 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
tlbe->entry.translated_addr = gpa;
tlbe->entry.iova = iova & ~mask;
tlbe->entry.addr_mask = mask;
- tlbe->entry.perm = PTE_AP_TO_PERM(ap);
+ tlbe->parent_perm = PTE_AP_TO_PERM(ap);
+ tlbe->entry.perm = tlbe->parent_perm;
tlbe->level = level;
tlbe->granule = granule_sz;
return 0;
@@ -547,7 +548,8 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
tlbe->entry.translated_addr = gpa;
tlbe->entry.iova = ipa & ~mask;
tlbe->entry.addr_mask = mask;
- tlbe->entry.perm = s2ap;
+ tlbe->parent_perm = s2ap;
+ tlbe->entry.perm = tlbe->parent_perm;
tlbe->level = level;
tlbe->granule = granule_sz;
return 0;
@@ -562,6 +564,30 @@ error:
return -EINVAL;
}
+/*
+ * combine S1 and S2 TLB entries into a single entry.
+ * As a result the S1 entry is overriden with combined data.
+ */
+static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe,
+ SMMUTLBEntry *tlbe_s2,
+ dma_addr_t iova,
+ SMMUTransCfg *cfg)
+{
+ if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) {
+ tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask;
+ tlbe->granule = tlbe_s2->granule;
+ tlbe->level = tlbe_s2->level;
+ }
+
+ tlbe->entry.translated_addr = CACHED_ENTRY_TO_ADDR(tlbe_s2,
+ tlbe->entry.translated_addr);
+
+ tlbe->entry.iova = iova & ~tlbe->entry.addr_mask;
+ /* parent_perm has s2 perm while perm keeps s1 perm. */
+ tlbe->parent_perm = tlbe_s2->entry.perm;
+ return;
+}
+
/**
* smmu_ptw - Walk the page tables for an IOVA, according to @cfg
*
@@ -629,9 +655,12 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
cached_entry = smmu_iotlb_lookup(bs, cfg, &tt_combined, addr);
if (cached_entry) {
- if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
+ if ((flag & IOMMU_WO) && !(cached_entry->entry.perm &
+ cached_entry->parent_perm & IOMMU_WO)) {
info->type = SMMU_PTW_ERR_PERMISSION;
- info->stage = cfg->stage;
+ info->stage = !(cached_entry->entry.perm & IOMMU_WO) ?
+ SMMU_STAGE_1 :
+ SMMU_STAGE_2;
return NULL;
}
return cached_entry;
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index eecbebaaac..d84de64122 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -77,6 +77,7 @@ typedef struct SMMUTLBEntry {
IOMMUTLBEntry entry;
uint8_t level;
uint8_t granule;
+ IOMMUAccessFlags parent_perm;
} SMMUTLBEntry;
/* Stage-2 configuration. */
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 11/18] hw/arm/smmu-common: Support nested translation
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (9 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 10/18] hw/arm/smmu-common: Add support for nested TLB Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-17 15:28 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 12/18] hw/arm/smmu: Support nesting in smmuv3_range_inval() Mostafa Saleh
` (7 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
When nested translation is requested, do the following:
- Translate stage-1 table address IPA into PA through stage-2.
- Translate stage-1 table walk output (IPA) through stage-2.
- Create a single TLB entry from stage-1 and stage-2 translations
using logic introduced before.
smmu_ptw() has a new argument SMMUState which include the TLB as
stage-1 table address can be cached in there.
Also in smmu_ptw(), a separate path used for nesting to simplify the
code, although some logic can be combined.
With nested translation class of translation fault can be different,
from the class of the translation, as faults from translating stage-1
tables are considered as CLASS_TT and not CLASS_IN, a new member
"is_ipa_descriptor" added to "SMMUPTWEventInfo" to differ faults
from walking stage 1 translation table and faults from translating
an IPA for a transaction.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 74 +++++++++++++++++++++++++++++++-----
hw/arm/smmuv3.c | 14 +++++++
include/hw/arm/smmu-common.h | 7 ++--
3 files changed, 82 insertions(+), 13 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index c894c4c621..8ed53f5b1d 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -318,8 +318,41 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
return NULL;
}
+/* Translate stage-1 table address using stage-2 page table. */
+static inline int translate_table_addr_ipa(SMMUState *bs,
+ dma_addr_t *table_addr,
+ SMMUTransCfg *cfg,
+ SMMUPTWEventInfo *info)
+{
+ dma_addr_t addr = *table_addr;
+ SMMUTLBEntry *cached_entry;
+ int asid;
+
+ /*
+ * The translation table walks performed from TTB0 or TTB1 are always
+ * performed in IPA space if stage 2 translations are enabled.
+ */
+ asid = cfg->asid;
+ cfg->stage = SMMU_STAGE_2;
+ cfg->asid = -1;
+ cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info);
+ cfg->asid = asid;
+ cfg->stage = SMMU_NESTED;
+
+ if (cached_entry) {
+ *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr);
+ return 0;
+ }
+
+ info->stage = SMMU_STAGE_2;
+ info->addr = addr;
+ info->is_ipa_descriptor = true;
+ return -EINVAL;
+}
+
/**
* smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA
+ * @bs: smmu state which includes TLB instance
* @cfg: translation config
* @iova: iova to translate
* @perm: access type
@@ -331,7 +364,7 @@ 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_s1(SMMUTransCfg *cfg,
+static int smmu_ptw_64_s1(SMMUState *bs, SMMUTransCfg *cfg,
dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
@@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
goto error;
}
baseaddr = get_table_pte_address(pte, granule_sz);
+ if (cfg->stage == SMMU_NESTED) {
+ if (translate_table_addr_ipa(bs, &baseaddr, cfg, info)) {
+ goto error;
+ }
+ }
level++;
continue;
} else if (is_page_pte(pte, level)) {
@@ -568,10 +606,8 @@ error:
* combine S1 and S2 TLB entries into a single entry.
* As a result the S1 entry is overriden with combined data.
*/
-static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe,
- SMMUTLBEntry *tlbe_s2,
- dma_addr_t iova,
- SMMUTransCfg *cfg)
+static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2,
+ dma_addr_t iova, SMMUTransCfg *cfg)
{
if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) {
tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask;
@@ -591,6 +627,7 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe,
/**
* smmu_ptw - Walk the page tables for an IOVA, according to @cfg
*
+ * @bs: smmu state which includes TLB instance
* @cfg: translation configuration
* @iova: iova to translate
* @perm: tentative access type
@@ -599,11 +636,15 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe,
*
* return 0 on success
*/
-int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
- SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
+int smmu_ptw(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t iova,
+ IOMMUAccessFlags perm, SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
+ int ret;
+ SMMUTLBEntry tlbe_s2;
+ dma_addr_t ipa;
+
if (cfg->stage == SMMU_STAGE_1) {
- return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
+ return smmu_ptw_64_s1(bs, cfg, iova, perm, tlbe, info);
} else if (cfg->stage == SMMU_STAGE_2) {
/*
* If bypassing stage 1(or unimplemented), the input address is passed
@@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info);
}
- g_assert_not_reached();
+ /* SMMU_NESTED. */
+ ret = smmu_ptw_64_s1(bs, cfg, iova, perm, tlbe, info);
+ if (ret) {
+ return ret;
+ }
+
+ ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova);
+ ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info);
+ if (ret) {
+ return ret;
+ }
+
+ combine_tlb(tlbe, &tlbe_s2, iova, cfg);
+ return 0;
}
SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
@@ -667,7 +721,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
}
cached_entry = g_new0(SMMUTLBEntry, 1);
- status = smmu_ptw(cfg, addr, flag, cached_entry, info);
+ status = smmu_ptw(bs, cfg, addr, flag, cached_entry, info);
if (status) {
g_free(cached_entry);
return NULL;
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 73d5a25705..06a96c65eb 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -910,6 +910,20 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
if (!cached_entry) {
/* All faults from PTW has S2 field. */
event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
+ /*
+ * Fault class is set as follows based on "class" input to
+ * the function and to "ptw_info" from "smmu_translate()"
+ * For stage-1:
+ * - EABT => CLASS_TT (hardcoded)
+ * - other events => CLASS_IN (input to function)
+ * For stage-2 => CLASS_IN (input to function)
+ * For nested, for all events:
+ * - CD fetch => CLASS_CD (input to function)
+ * - walking stage 1 translation table => CLASS_TT (from
+ * is_ipa_descriptor or input in case of TTBx)
+ * - s2 translation => CLASS_IN (input to function)
+ */
+ class = ptw_info.is_ipa_descriptor ? SMMU_CLASS_TT : class;
switch (ptw_info.type) {
case SMMU_PTW_ERR_WALK_EABT:
event->type = SMMU_EVT_F_WALK_EABT;
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index d84de64122..a3e6ab1b36 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -63,6 +63,7 @@ typedef struct SMMUPTWEventInfo {
SMMUStage stage;
SMMUPTWEventType type;
dma_addr_t addr; /* fetched address that induced an abort, if any */
+ bool is_ipa_descriptor; /* src for fault in nested translation. */
} SMMUPTWEventInfo;
typedef struct SMMUTransTableInfo {
@@ -184,9 +185,9 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev)
* smmu_ptw - Perform the page table walk for a given iova / access flags
* pair, according to @cfg translation config
*/
-int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
- SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info);
-
+int smmu_ptw(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t iova,
+ IOMMUAccessFlags perm, SMMUTLBEntry *tlbe,
+ SMMUPTWEventInfo *info);
/*
* smmu_translate - Look for a translation in TLB, if not, do a PTW.
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 12/18] hw/arm/smmu: Support nesting in smmuv3_range_inval()
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (10 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 11/18] hw/arm/smmu-common: Support nested translation Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 13/18] hw/arm/smmu: Introduce smmu_iotlb_inv_asid_vmid Mostafa Saleh
` (6 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
With nesting, we would need to invalidate IPAs without
over-invalidating stage-1 IOVAs. This can be done by
distinguishing IPAs in the TLBs by having ASID=-1.
To achieve that, rework the invalidation for IPAs to have a
separate function, while for IOVA invalidation ASID=-1 means
invalidate for all ASIDs.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 47 ++++++++++++++++++++++++++++++++++++
hw/arm/smmuv3.c | 23 ++++++++++++------
hw/arm/trace-events | 2 +-
include/hw/arm/smmu-common.h | 3 ++-
4 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 8ed53f5b1d..a100700497 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -195,6 +195,25 @@ static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
((entry->iova & ~info->mask) == info->iova);
}
+static gboolean smmu_hash_remove_by_vmid_ipa(gpointer key, gpointer value,
+ gpointer user_data)
+{
+ SMMUTLBEntry *iter = (SMMUTLBEntry *)value;
+ IOMMUTLBEntry *entry = &iter->entry;
+ SMMUIOTLBPageInvInfo *info = (SMMUIOTLBPageInvInfo *)user_data;
+ SMMUIOTLBKey iotlb_key = *(SMMUIOTLBKey *)key;
+
+ if (SMMU_IOTLB_ASID(iotlb_key) >= 0) {
+ /* This is a stage-1 address. */
+ return false;
+ }
+ if (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, int vmid, dma_addr_t iova,
uint8_t tg, uint64_t num_pages, uint8_t ttl)
{
@@ -223,6 +242,34 @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
&info);
}
+/*
+ * Similar to smmu_iotlb_inv_iova(), but for Stage-2, ASID is always -1,
+ * in Stage-1 invalidation ASID = -1, means don't care.
+ */
+void smmu_iotlb_inv_ipa(SMMUState *s, int vmid, dma_addr_t ipa, uint8_t tg,
+ uint64_t num_pages, uint8_t ttl)
+{
+ uint8_t granule = tg ? tg * 2 + 10 : 12;
+ int asid = -1;
+
+ if (ttl && (num_pages == 1)) {
+ SMMUIOTLBKey key = smmu_get_iotlb_key(asid, vmid, ipa, tg, ttl);
+
+ if (g_hash_table_remove(s->iotlb, &key)) {
+ return;
+ }
+ }
+
+ SMMUIOTLBPageInvInfo info = {
+ .iova = ipa,
+ .vmid = vmid,
+ .mask = (num_pages << granule) - 1};
+
+ g_hash_table_foreach_remove(s->iotlb,
+ smmu_hash_remove_by_vmid_ipa,
+ &info);
+}
+
void smmu_iotlb_inv_asid(SMMUState *s, int asid)
{
trace_smmu_iotlb_inv_asid(asid);
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 06a96c65eb..a0979f15a0 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1168,7 +1168,7 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, int vmid,
}
}
-static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
+static void smmuv3_range_inval(SMMUState *s, Cmd *cmd, SMMUStage stage)
{
dma_addr_t end, addr = CMD_ADDR(cmd);
uint8_t type = CMD_TYPE(cmd);
@@ -1193,9 +1193,13 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
}
if (!tg) {
- trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
+ trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf, stage);
smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
- smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
+ if (stage == SMMU_STAGE_1) {
+ smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
+ } else {
+ smmu_iotlb_inv_ipa(s, vmid, addr, tg, 1, ttl);
+ }
return;
}
@@ -1211,9 +1215,14 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd)
uint64_t mask = dma_aligned_pow2_mask(addr, end, 64);
num_pages = (mask + 1) >> granule;
- trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages, ttl, leaf);
+ trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages,
+ ttl, leaf, stage);
smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
- smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
+ if (stage == SMMU_STAGE_1) {
+ smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
+ } else {
+ smmu_iotlb_inv_ipa(s, vmid, addr, tg, num_pages, ttl);
+ }
addr += mask + 1;
}
}
@@ -1372,7 +1381,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
cmd_error = SMMU_CERROR_ILL;
break;
}
- smmuv3_range_inval(bs, &cmd);
+ smmuv3_range_inval(bs, &cmd, SMMU_STAGE_1);
break;
case SMMU_CMD_TLBI_S12_VMALL:
{
@@ -1397,7 +1406,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
* As currently only either s1 or s2 are supported
* we can reuse same function for s2.
*/
- smmuv3_range_inval(bs, &cmd);
+ smmuv3_range_inval(bs, &cmd, SMMU_STAGE_2);
break;
case SMMU_CMD_TLBI_EL3_ALL:
case SMMU_CMD_TLBI_EL3_VA:
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 09ccd39548..7d9c1703da 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -46,7 +46,7 @@ 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_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, int stage) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d stage=%d"
smmuv3_cmdq_tlbi_nh(void) ""
smmuv3_cmdq_tlbi_nh_asid(int asid) "asid=%d"
smmuv3_cmdq_tlbi_s12_vmid(int vmid) "vmid=%d"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index a3e6ab1b36..f9c8b00c9d 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -217,7 +217,8 @@ void smmu_iotlb_inv_asid(SMMUState *s, int asid);
void smmu_iotlb_inv_vmid(SMMUState *s, int 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);
-
+void smmu_iotlb_inv_ipa(SMMUState *s, int vmid, dma_addr_t ipa, uint8_t tg,
+ uint64_t num_pages, uint8_t ttl);
/* Unmap the range of all the notifiers registered to any IOMMU mr */
void smmu_inv_notifiers_all(SMMUState *s);
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 13/18] hw/arm/smmu: Introduce smmu_iotlb_inv_asid_vmid
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (11 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 12/18] hw/arm/smmu: Support nesting in smmuv3_range_inval() Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 14/18] hw/arm/smmu: Support nesting in the rest of commands Mostafa Saleh
` (5 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
Soon, Instead of doing TLB invalidation by ASID only, VMID will be
also required.
Add smmu_iotlb_inv_asid_vmid() which invalidates by both ASID and VMID.
However, at the moment this function is only used in SMMU_CMD_TLBI_NH_ASID
which is a stage-1 command, so passing VMID = -1 keeps the original
behaviour.
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 20 +++++++++++++-------
hw/arm/smmuv3.c | 2 +-
hw/arm/trace-events | 2 +-
include/hw/arm/smmu-common.h | 2 +-
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index a100700497..bf35806b02 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -159,13 +159,14 @@ void smmu_iotlb_inv_all(SMMUState *s)
g_hash_table_remove_all(s->iotlb);
}
-static gboolean smmu_hash_remove_by_asid(gpointer key, gpointer value,
- gpointer user_data)
+static gboolean smmu_hash_remove_by_asid_vmid(gpointer key, gpointer value,
+ gpointer user_data)
{
- int asid = *(int *)user_data;
+ SMMUIOTLBPageInvInfo *info = (SMMUIOTLBPageInvInfo *)user_data;
SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
- return SMMU_IOTLB_ASID(*iotlb_key) == asid;
+ return (SMMU_IOTLB_ASID(*iotlb_key) == info->asid) &&
+ (SMMU_IOTLB_VMID(*iotlb_key) == info->vmid);
}
static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
@@ -270,10 +271,15 @@ void smmu_iotlb_inv_ipa(SMMUState *s, int vmid, dma_addr_t ipa, uint8_t tg,
&info);
}
-void smmu_iotlb_inv_asid(SMMUState *s, int asid)
+void smmu_iotlb_inv_asid_vmid(SMMUState *s, int asid, int vmid)
{
- trace_smmu_iotlb_inv_asid(asid);
- g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
+ SMMUIOTLBPageInvInfo info = {
+ .asid = asid,
+ .vmid = vmid,
+ };
+
+ trace_smmu_iotlb_inv_asid_vmid(asid, vmid);
+ g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid_vmid, &info);
}
void smmu_iotlb_inv_vmid(SMMUState *s, int vmid)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index a0979f15a0..cfee42add4 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1361,7 +1361,7 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
trace_smmuv3_cmdq_tlbi_nh_asid(asid);
smmu_inv_notifiers_all(&s->smmu_state);
- smmu_iotlb_inv_asid(bs, asid);
+ smmu_iotlb_inv_asid_vmid(bs, asid, -1);
break;
}
case SMMU_CMD_TLBI_NH_ALL:
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 7d9c1703da..4aa71b1b19 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -11,7 +11,7 @@ smmu_ptw_page_pte(int stage, int level, uint64_t iova, uint64_t baseaddr, uint6
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"
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(int asid) "IOTLB invalidate asid=%d"
+smmu_iotlb_inv_asid_vmid(int asid, int vmid) "IOTLB invalidate asid=%d vmid=%d"
smmu_iotlb_inv_vmid(int vmid) "IOTLB invalidate vmid=%d"
smmu_iotlb_inv_iova(int asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64
smmu_inv_notifiers_mr(const char *name) "iommu mr=%s"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index f9c8b00c9d..b3a937190b 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -213,7 +213,7 @@ void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg, SMMUTLBEntry *entry);
SMMUIOTLBKey smmu_get_iotlb_key(int asid, int vmid, uint64_t iova,
uint8_t tg, uint8_t level);
void smmu_iotlb_inv_all(SMMUState *s);
-void smmu_iotlb_inv_asid(SMMUState *s, int asid);
+void smmu_iotlb_inv_asid_vmid(SMMUState *s, int asid, int vmid);
void smmu_iotlb_inv_vmid(SMMUState *s, int 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.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 14/18] hw/arm/smmu: Support nesting in the rest of commands
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (12 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 13/18] hw/arm/smmu: Introduce smmu_iotlb_inv_asid_vmid Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 15/18] hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova() Mostafa Saleh
` (4 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
Some commands need rework for nesting, as they used to assume S1
and S2 are mutually exclusive:
- CMD_TLBI_NH_ASID: Consider VMID if stage-2 is supported
- CMD_TLBI_NH_ALL: Consider VMID if stage-2 is supported, otherwise
invalidate everything, this required a new vmid invalidation
function for stage-1 only (ASID >= 0)
Also, rework trace events to reflect the new implementation.
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 16 ++++++++++++++++
hw/arm/smmuv3.c | 28 ++++++++++++++++++++++++++--
hw/arm/trace-events | 4 +++-
include/hw/arm/smmu-common.h | 1 +
4 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index bf35806b02..67cb134d23 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -178,6 +178,16 @@ static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
}
+static gboolean smmu_hash_remove_by_vmid_s1(gpointer key, gpointer value,
+ gpointer user_data)
+{
+ int vmid = *(int *)user_data;
+ SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
+
+ return (SMMU_IOTLB_VMID(*iotlb_key) == vmid) &&
+ (SMMU_IOTLB_ASID(*iotlb_key) >= 0);
+}
+
static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer value,
gpointer user_data)
{
@@ -288,6 +298,12 @@ void smmu_iotlb_inv_vmid(SMMUState *s, int vmid)
g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
}
+inline void smmu_iotlb_inv_vmid_s1(SMMUState *s, int vmid)
+{
+ trace_smmu_iotlb_inv_vmid_s1(vmid);
+ g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid_s1, &vmid);
+}
+
/* VMSAv8-64 Translation */
/**
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index cfee42add4..9a88b83511 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1353,25 +1353,49 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
case SMMU_CMD_TLBI_NH_ASID:
{
int asid = CMD_ASID(&cmd);
+ int vmid = -1;
if (!STAGE1_SUPPORTED(s)) {
cmd_error = SMMU_CERROR_ILL;
break;
}
+ /*
+ * VMID is only matched when stage 2 is supported, otherwise set it
+ * to -1 as the value used for stage-1 only VMIDs.
+ */
+ if (STAGE2_SUPPORTED(s)) {
+ vmid = CMD_VMID(&cmd);
+ }
+
trace_smmuv3_cmdq_tlbi_nh_asid(asid);
smmu_inv_notifiers_all(&s->smmu_state);
- smmu_iotlb_inv_asid_vmid(bs, asid, -1);
+ smmu_iotlb_inv_asid_vmid(bs, asid, vmid);
break;
}
case SMMU_CMD_TLBI_NH_ALL:
+ {
+ int vmid = -1;
+
if (!STAGE1_SUPPORTED(s)) {
cmd_error = SMMU_CERROR_ILL;
break;
}
+
+ /*
+ * If stage-2 is supported, invalidate for this VMID only, otherwise
+ * invalidate the whole thing.
+ */
+ if (STAGE2_SUPPORTED(s)) {
+ vmid = CMD_VMID(&cmd);
+ trace_smmuv3_cmdq_tlbi_nh(vmid);
+ smmu_iotlb_inv_vmid_s1(bs, vmid);
+ break;
+ }
QEMU_FALLTHROUGH;
+ }
case SMMU_CMD_TLBI_NSNH_ALL:
- trace_smmuv3_cmdq_tlbi_nh();
+ trace_smmuv3_cmdq_tlbi_nsnh();
smmu_inv_notifiers_all(&s->smmu_state);
smmu_iotlb_inv_all(bs);
break;
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 4aa71b1b19..593cc571da 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -13,6 +13,7 @@ smmu_get_pte(uint64_t baseaddr, int index, uint64_t pteaddr, uint64_t pte) "base
smmu_iotlb_inv_all(void) "IOTLB invalidate all"
smmu_iotlb_inv_asid_vmid(int asid, int vmid) "IOTLB invalidate asid=%d vmid=%d"
smmu_iotlb_inv_vmid(int vmid) "IOTLB invalidate vmid=%d"
+smmu_iotlb_inv_vmid_s1(int vmid) "IOTLB invalidate vmid=%d"
smmu_iotlb_inv_iova(int 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(int asid, int 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"
@@ -47,7 +48,8 @@ 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_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t num_pages, uint8_t ttl, bool leaf, int stage) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" ttl=%d leaf=%d stage=%d"
-smmuv3_cmdq_tlbi_nh(void) ""
+smmuv3_cmdq_tlbi_nh(int vmid) "vmid=%d"
+smmuv3_cmdq_tlbi_nsnh(void) ""
smmuv3_cmdq_tlbi_nh_asid(int asid) "asid=%d"
smmuv3_cmdq_tlbi_s12_vmid(int vmid) "vmid=%d"
smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index b3a937190b..d38687581b 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -215,6 +215,7 @@ SMMUIOTLBKey smmu_get_iotlb_key(int asid, int vmid, uint64_t iova,
void smmu_iotlb_inv_all(SMMUState *s);
void smmu_iotlb_inv_asid_vmid(SMMUState *s, int asid, int vmid);
void smmu_iotlb_inv_vmid(SMMUState *s, int vmid);
+void smmu_iotlb_inv_vmid_s1(SMMUState *s, int 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);
void smmu_iotlb_inv_ipa(SMMUState *s, int vmid, dma_addr_t ipa, uint8_t tg,
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 15/18] hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova()
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (13 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 14/18] hw/arm/smmu: Support nesting in the rest of commands Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-17 15:30 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 16/18] hw/arm/smmuv3: Handle translation faults according to SMMUPTWEventInfo Mostafa Saleh
` (3 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
IOMMUTLBEvent only understands IOVA, for stage-1 or stage-2
SMMU instances we consider the input address as the IOVA, but when
nesting is used, we can't mix stage-1 and stage-2 addresses, so for
nesting only stage-1 is considered the IOVA and would be notified.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmuv3.c | 39 +++++++++++++++++++++++++--------------
hw/arm/trace-events | 2 +-
2 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 9a88b83511..84cd314b33 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1096,27 +1096,38 @@ epilogue:
* @iova: iova
* @tg: translation granule (if communicated through range invalidation)
* @num_pages: number of @granule sized pages (if tg != 0), otherwise 1
+ * @stage: Which stage(1 or 2) is used
*/
static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
IOMMUNotifier *n,
int asid, int vmid,
dma_addr_t iova, uint8_t tg,
- uint64_t num_pages)
+ uint64_t num_pages, int stage)
{
SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
+ SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
+ SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
IOMMUTLBEvent event;
uint8_t granule;
- SMMUv3State *s = sdev->smmu;
+
+ if (!cfg) {
+ return;
+ }
+
+ /*
+ * stage is passed from TLB invalidation commands which can be either
+ * stage-1 or stage-2.
+ * However, IOMMUTLBEvent only understands IOVA, for stage-1 or stage-2
+ * SMMU instances we consider the input address as the IOVA, but when
+ * nesting is used, we can't mix stage-1 and stage-2 addresses, so for
+ * nesting only stage-1 is considered the IOVA and would be notified.
+ */
+ if ((stage == SMMU_STAGE_2) && (cfg->stage == SMMU_NESTED))
+ return;
if (!tg) {
- SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
- SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
SMMUTransTableInfo *tt;
- if (!cfg) {
- return;
- }
-
if (asid >= 0 && cfg->asid != asid) {
return;
}
@@ -1125,7 +1136,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
return;
}
- if (STAGE1_SUPPORTED(s)) {
+ if (stage == SMMU_STAGE_1) {
tt = select_tt(cfg, iova);
if (!tt) {
return;
@@ -1151,7 +1162,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
/* 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)
+ uint64_t num_pages, int stage)
{
SMMUDevice *sdev;
@@ -1160,10 +1171,10 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, int vmid,
IOMMUNotifier *n;
trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, vmid,
- iova, tg, num_pages);
+ iova, tg, num_pages, stage);
IOMMU_NOTIFIER_FOREACH(n, mr) {
- smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages);
+ smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages, stage);
}
}
}
@@ -1194,7 +1205,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd, SMMUStage stage)
if (!tg) {
trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf, stage);
- smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
+ smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1, stage);
if (stage == SMMU_STAGE_1) {
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
} else {
@@ -1217,7 +1228,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd, SMMUStage stage)
num_pages = (mask + 1) >> granule;
trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages,
ttl, leaf, stage);
- smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
+ smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages, stage);
if (stage == SMMU_STAGE_1) {
smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
} else {
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 593cc571da..be6c8f720b 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -55,7 +55,7 @@ smmuv3_cmdq_tlbi_s12_vmid(int 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, int asid, int 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
+smmuv3_inv_notifiers_iova(const char *name, int asid, int vmid, uint64_t iova, uint8_t tg, uint64_t num_pages, int stage) "iommu mr=%s asid=%d vmid=%d iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" stage=%d"
# strongarm.c
strongarm_uart_update_parameters(const char *label, int speed, char parity, int data_bits, int stop_bits) "%s speed=%d parity=%c data=%d stop=%d"
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 16/18] hw/arm/smmuv3: Handle translation faults according to SMMUPTWEventInfo
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (14 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 15/18] hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova() Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-17 15:31 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 17/18] hw/arm/smmuv3: Support and advertise nesting Mostafa Saleh
` (2 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
Previously, to check if faults are enabled, it was sufficient to check
the current stage of translation and check the corresponding
record_faults flag.
However, with nesting, it is possible for stage-1 (nested) translation
to trigger a stage-2 fault, so we check SMMUPTWEventInfo as it would
have the correct stage set from the page table walk.
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmuv3.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 84cd314b33..d052a2ba24 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -34,9 +34,10 @@
#include "smmuv3-internal.h"
#include "smmu-internal.h"
-#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == SMMU_STAGE_1) ? \
- (cfg)->record_faults : \
- (cfg)->s2cfg.record_faults)
+#define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1 && \
+ (cfg)->record_faults) || \
+ ((ptw_info).stage == SMMU_STAGE_2 && \
+ (cfg)->s2cfg.record_faults))
/**
* smmuv3_trigger_irq - pulse @irq if enabled and update
@@ -933,7 +934,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
event->u.f_walk_eabt.addr2 = ptw_info.addr;
break;
case SMMU_PTW_ERR_TRANSLATION:
- if (PTW_RECORD_FAULT(cfg)) {
+ if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_TRANSLATION;
event->u.f_translation.addr2 = ptw_info.addr;
event->u.f_translation.class = class;
@@ -941,7 +942,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
}
break;
case SMMU_PTW_ERR_ADDR_SIZE:
- if (PTW_RECORD_FAULT(cfg)) {
+ if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_ADDR_SIZE;
event->u.f_addr_size.addr2 = ptw_info.addr;
event->u.f_addr_size.class = class;
@@ -949,7 +950,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
}
break;
case SMMU_PTW_ERR_ACCESS:
- if (PTW_RECORD_FAULT(cfg)) {
+ if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_ACCESS;
event->u.f_access.addr2 = ptw_info.addr;
event->u.f_access.class = class;
@@ -957,7 +958,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
}
break;
case SMMU_PTW_ERR_PERMISSION:
- if (PTW_RECORD_FAULT(cfg)) {
+ if (PTW_RECORD_FAULT(ptw_info, cfg)) {
event->type = SMMU_EVT_F_PERMISSION;
event->u.f_permission.addr2 = ptw_info.addr;
event->u.f_permission.class = class;
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 17/18] hw/arm/smmuv3: Support and advertise nesting
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (15 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 16/18] hw/arm/smmuv3: Handle translation faults according to SMMUPTWEventInfo Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 18/18] hw/arm/smmu: Refactor SMMU OAS Mostafa Saleh
2024-07-17 15:09 ` [PATCH v5 00/18] SMMUv3 nested translation support Jean-Philippe Brucker
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
Everything is in place, consolidate parsing of STE cfg and setting
translation stage.
Advertise nesting if stage requested is "nested".
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmuv3.c | 35 ++++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index d052a2ba24..32b1f4cb75 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -261,6 +261,9 @@ static void smmuv3_init_regs(SMMUv3State *s)
/* Based on sys property, the stages supported in smmu will be advertised.*/
if (s->stage && !strcmp("2", s->stage)) {
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S2P, 1);
+ } else if (s->stage && !strcmp("nested", s->stage)) {
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1);
+ s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S2P, 1);
} else {
s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1);
}
@@ -425,8 +428,6 @@ static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran)
static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
{
- cfg->stage = SMMU_STAGE_2;
-
if (STE_S2AA64(ste) == 0x0) {
qemu_log_mask(LOG_UNIMP,
"SMMUv3 AArch32 tables not supported\n");
@@ -509,6 +510,27 @@ bad_ste:
return -EINVAL;
}
+static void decode_ste_config(SMMUTransCfg *cfg, uint32_t config)
+{
+
+ if (STE_CFG_ABORT(config)) {
+ cfg->aborted = true;
+ return;
+ }
+ if (STE_CFG_BYPASS(config)) {
+ cfg->bypassed = true;
+ return;
+ }
+
+ if (STE_CFG_S1_ENABLED(config)) {
+ cfg->stage = SMMU_STAGE_1;
+ }
+
+ if (STE_CFG_S2_ENABLED(config)) {
+ cfg->stage |= SMMU_STAGE_2;
+ }
+}
+
/* Returns < 0 in case of invalid STE, 0 otherwise */
static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
STE *ste, SMMUEventInfo *event)
@@ -525,13 +547,9 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
config = STE_CONFIG(ste);
- if (STE_CFG_ABORT(config)) {
- cfg->aborted = true;
- return 0;
- }
+ decode_ste_config(cfg, config);
- if (STE_CFG_BYPASS(config)) {
- cfg->bypassed = true;
+ if (cfg->aborted || cfg->bypassed) {
return 0;
}
@@ -704,7 +722,6 @@ static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
/* we support only those at the moment */
cfg->aa64 = true;
- cfg->stage = SMMU_STAGE_1;
cfg->oas = oas2bits(CD_IPS(cd));
cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas);
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v5 18/18] hw/arm/smmu: Refactor SMMU OAS
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (16 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 17/18] hw/arm/smmuv3: Support and advertise nesting Mostafa Saleh
@ 2024-07-15 8:45 ` Mostafa Saleh
2024-07-17 15:09 ` [PATCH v5 00/18] SMMUv3 nested translation support Jean-Philippe Brucker
18 siblings, 0 replies; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-15 8:45 UTC (permalink / raw)
To: qemu-arm, eric.auger, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz, Mostafa Saleh
SMMUv3 OAS is currently hardcoded in the code to 44 bits, for nested
configurations that can be a problem, as stage-2 might be shared with
the CPU which might have different PARANGE, and according to SMMU manual
ARM IHI 0070F.b:
6.3.6 SMMU_IDR5, OAS must match the system physical address size.
This patch doesn't change the SMMU OAS, but refactors the code to
make it easier to do that:
- Rely everywhere on IDR5 for reading OAS instead of using the
SMMU_IDR5_OAS macro, so, it is easier just to change IDR5 and
it propagages correctly.
- Add additional checks when OAS is greater than 48bits.
- Remove unused functions/macros: pa_range/MAX_PA.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
hw/arm/smmu-common.c | 7 ++++---
hw/arm/smmuv3-internal.h | 13 -------------
hw/arm/smmuv3.c | 35 ++++++++++++++++++++++++++++-------
3 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 67cb134d23..7d8a353956 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -452,7 +452,8 @@ static int smmu_ptw_64_s1(SMMUState *bs, SMMUTransCfg *cfg,
inputsize = 64 - tt->tsz;
level = 4 - (inputsize - 4) / stride;
indexmask = VMSA_IDXMSK(inputsize, stride, level);
- baseaddr = extract64(tt->ttb, 0, 48);
+
+ baseaddr = extract64(tt->ttb, 0, cfg->oas);
baseaddr &= ~indexmask;
while (level < VMSA_LEVELS) {
@@ -576,8 +577,8 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
* 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);
+ uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, cfg->s2cfg.eff_ps) +
+ (1 << stride) * idx * sizeof(uint64_t);
dma_addr_t indexmask = VMSA_IDXMSK(inputsize, stride, level);
baseaddr &= ~indexmask;
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 0f3ecec804..0ebf2eebcf 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -602,19 +602,6 @@ static inline int oas2bits(int oas_field)
return -1;
}
-static inline int pa_range(STE *ste)
-{
- int oas_field = MIN(STE_S2PS(ste), SMMU_IDR5_OAS);
-
- if (!STE_S2AA64(ste)) {
- return 40;
- }
-
- return oas2bits(oas_field);
-}
-
-#define MAX_PA(ste) ((1 << pa_range(ste)) - 1)
-
/* CD fields */
#define CD_VALID(x) extract32((x)->word[0], 31, 1)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 32b1f4cb75..d119a8026f 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -402,10 +402,10 @@ static bool s2t0sz_valid(SMMUTransCfg *cfg)
}
if (cfg->s2cfg.granule_sz == 16) {
- return (cfg->s2cfg.tsz >= 64 - oas2bits(SMMU_IDR5_OAS));
+ return (cfg->s2cfg.tsz >= 64 - cfg->s2cfg.eff_ps);
}
- return (cfg->s2cfg.tsz >= MAX(64 - oas2bits(SMMU_IDR5_OAS), 16));
+ return (cfg->s2cfg.tsz >= MAX(64 - cfg->s2cfg.eff_ps, 16));
}
/*
@@ -426,8 +426,11 @@ static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran)
return nr_concat <= VMSA_MAX_S2_CONCAT;
}
-static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
+static int decode_ste_s2_cfg(SMMUv3State *s, SMMUTransCfg *cfg,
+ STE *ste)
{
+ uint8_t oas = FIELD_EX32(s->idr[5], IDR5, OAS);
+
if (STE_S2AA64(ste) == 0x0) {
qemu_log_mask(LOG_UNIMP,
"SMMUv3 AArch32 tables not supported\n");
@@ -460,7 +463,15 @@ static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
}
/* For AA64, The effective S2PS size is capped to the OAS. */
- cfg->s2cfg.eff_ps = oas2bits(MIN(STE_S2PS(ste), SMMU_IDR5_OAS));
+ cfg->s2cfg.eff_ps = oas2bits(MIN(STE_S2PS(ste), oas));
+ /*
+ * For SMMUv3.1 and later, when OAS == IAS == 52, the stage 2 input
+ * range is further limited to 48 bits unless STE.S2TG indicates a
+ * 64KB granule.
+ */
+ if (cfg->s2cfg.granule_sz != 16) {
+ cfg->s2cfg.eff_ps = MIN(cfg->s2cfg.eff_ps, 48);
+ }
/*
* It is ILLEGAL for the address in S2TTB to be outside the range
* described by the effective S2PS value.
@@ -536,6 +547,7 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
STE *ste, SMMUEventInfo *event)
{
uint32_t config;
+ uint8_t oas = FIELD_EX32(s->idr[5], IDR5, OAS);
int ret;
if (!STE_VALID(ste)) {
@@ -579,8 +591,8 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
* 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);
+ cfg->oas = oas2bits(oas);
+ ret = decode_ste_s2_cfg(s, cfg, ste);
if (ret) {
goto bad_ste;
}
@@ -706,6 +718,7 @@ static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
int i;
SMMUTranslationStatus status;
SMMUTLBEntry *entry;
+ uint8_t oas = FIELD_EX32(s->idr[5], IDR5, OAS);
if (!CD_VALID(cd) || !CD_AARCH64(cd)) {
goto bad_cd;
@@ -724,7 +737,7 @@ static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
cfg->aa64 = true;
cfg->oas = oas2bits(CD_IPS(cd));
- cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas);
+ cfg->oas = MIN(oas2bits(oas), cfg->oas);
cfg->tbi = CD_TBI(cd);
cfg->asid = CD_ASID(cd);
cfg->affd = CD_AFFD(cd);
@@ -753,6 +766,14 @@ static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
goto bad_cd;
}
+ /*
+ * An address greater than 48 bits in size can only be output from a
+ * TTD when, in SMMUv3.1 and later, the effective IPS is 52 and a 64KB
+ * granule is in use for that translation table
+ */
+ if (tt->granule_sz != 16) {
+ cfg->oas = MIN(cfg->oas, 48);
+ }
tt->tsz = tsz;
tt->ttb = CD_TTB(cd, i);
--
2.45.2.993.g49e7a77208-goog
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events
2024-07-15 8:45 ` [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events Mostafa Saleh
@ 2024-07-17 15:07 ` Eric Auger
2024-07-17 15:58 ` Jean-Philippe Brucker
0 siblings, 1 reply; 35+ messages in thread
From: Eric Auger @ 2024-07-17 15:07 UTC (permalink / raw)
To: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz
Hi Jean,
On 7/15/24 10:45, Mostafa Saleh wrote:
> The SMMUv3 spec (ARM IHI 0070 F.b - 7.3 Event records) defines the
> class of events faults as:
>
> CLASS: The class of the operation that caused the fault:
> - 0b00: CD, CD fetch.
> - 0b01: TTD, Stage 1 translation table fetch.
> - 0b10: IN, Input address
>
> However, this value was not set and left as 0 which means CD and not
> IN (0b10).
>
> Another problem was that stage-2 class is considered IN not TT for
> EABT, according to the spec:
> Translation of an IPA after successful stage 1 translation (or,
> in stage 2-only configuration, an input IPA)
> - S2 == 1 (stage 2), CLASS == IN (Input to stage)
>
> This would change soon when nested translations are supported.
>
> While at it, add an enum for class as it would be used for nesting.
> However, at the moment stage-1 and stage-2 use the same class values,
> except for EABT.
>
> Fixes: 9bde7f0674 “hw/arm/smmuv3: Implement translate callback”
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> hw/arm/smmuv3-internal.h | 6 ++++++
> hw/arm/smmuv3.c | 8 +++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index e4dd11e1e6..0f3ecec804 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -32,6 +32,12 @@ typedef enum SMMUTranslationStatus {
> SMMU_TRANS_SUCCESS,
> } SMMUTranslationStatus;
>
> +typedef enum SMMUTranslationClass {
> + SMMU_CLASS_CD,
> + SMMU_CLASS_TT,
> + SMMU_CLASS_IN,
> +} SMMUTranslationClass;
> +
> /* MMIO Registers */
>
> REG32(IDR0, 0x0)
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 9dd3ea48e4..3d214c9f57 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -942,7 +942,9 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> event.type = SMMU_EVT_F_WALK_EABT;
> event.u.f_walk_eabt.addr = addr;
> event.u.f_walk_eabt.rnw = flag & 0x1;
> - event.u.f_walk_eabt.class = 0x1;
> + /* Stage-2 (only) is class IN while stage-1 is class TT */
> + event.u.f_walk_eabt.class = (ptw_info.stage == 2) ?
> + SMMU_CLASS_IN : SMMU_CLASS_TT;
does it match your expectations. While reading your previous comment I
have the impression what you had in mind was more complicated than that
* s2 walk that encounters EABT on S2 descriptor while translating
non-descriptor IPA is reported as class=IN, even when doing s2-only.
Thanks
Eric
> event.u.f_walk_eabt.addr2 = ptw_info.addr;
> break;
> case SMMU_PTW_ERR_TRANSLATION:
> @@ -950,6 +952,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> event.type = SMMU_EVT_F_TRANSLATION;
> event.u.f_translation.addr = addr;
> event.u.f_translation.addr2 = ptw_info.addr;
> + event.u.f_translation.class = SMMU_CLASS_IN;
> event.u.f_translation.rnw = flag & 0x1;
> }
> break;
> @@ -958,6 +961,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> event.type = SMMU_EVT_F_ADDR_SIZE;
> event.u.f_addr_size.addr = addr;
> event.u.f_addr_size.addr2 = ptw_info.addr;
> + event.u.f_translation.class = SMMU_CLASS_IN;
> event.u.f_addr_size.rnw = flag & 0x1;
> }
> break;
> @@ -966,6 +970,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> event.type = SMMU_EVT_F_ACCESS;
> event.u.f_access.addr = addr;
> event.u.f_access.addr2 = ptw_info.addr;
> + event.u.f_translation.class = SMMU_CLASS_IN;
> event.u.f_access.rnw = flag & 0x1;
> }
> break;
> @@ -974,6 +979,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> event.type = SMMU_EVT_F_PERMISSION;
> event.u.f_permission.addr = addr;
> event.u.f_permission.addr2 = ptw_info.addr;
> + event.u.f_translation.class = SMMU_CLASS_IN;
> event.u.f_permission.rnw = flag & 0x1;
> }
> break;
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
` (17 preceding siblings ...)
2024-07-15 8:45 ` [PATCH v5 18/18] hw/arm/smmu: Refactor SMMU OAS Mostafa Saleh
@ 2024-07-17 15:09 ` Jean-Philippe Brucker
2024-07-17 17:43 ` Eric Auger
18 siblings, 1 reply; 35+ messages in thread
From: Jean-Philippe Brucker @ 2024-07-17 15:09 UTC (permalink / raw)
To: Mostafa Saleh
Cc: qemu-arm, eric.auger, peter.maydell, qemu-devel, alex.bennee, maz,
nicolinc, julien, richard.henderson, marcin.juszkiewicz
On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
> but not nested instances.
> This patch series adds support for nested translation in SMMUv3,
> this is controlled by property “arm-smmuv3.stage=nested”, and
> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
For the whole series (3-9, 11, 12, 15, 16, 18):
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
(and I think patch 16 is missing Eric's R-b)
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 08/18] hw/arm/smmuv3: Translate CD and TT using stage-2 table
2024-07-15 8:45 ` [PATCH v5 08/18] hw/arm/smmuv3: Translate CD and TT using stage-2 table Mostafa Saleh
@ 2024-07-17 15:18 ` Eric Auger
0 siblings, 0 replies; 35+ messages in thread
From: Eric Auger @ 2024-07-17 15:18 UTC (permalink / raw)
To: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz
On 7/15/24 10:45, Mostafa Saleh wrote:
> According to ARM SMMU architecture specification (ARM IHI 0070 F.b),
> In "5.2 Stream Table Entry":
> [51:6] S1ContextPtr
> If Config[1] == 1 (stage 2 enabled), this pointer is an IPA translated by
> stage 2 and the programmed value must be within the range of the IAS.
>
> In "5.4.1 CD notes":
> The translation table walks performed from TTB0 or TTB1 are always performed
> in IPA space if stage 2 translations are enabled.
>
> This patch implements translation of the S1 context descriptor pointer and
> TTBx base addresses through the S2 stage (IPA -> PA)
>
> smmuv3_do_translate() is updated to have one arg which is translation
> class, this is useful to:
> - Decide wether a translation is stage-2 only or use the STE config.
> - Populate the class in case of faults, WALK_EABT is left unchanged
> for stage-1 as it is always IN, while stage-2 would match the
> used class (TT, IN, CD), this will change slightly when the ptw
> supports nested translation as it can also issue TT event with
> class IN.
>
> In case for stage-2 only translation, used in the context of nested
> translation, the stage and asid are saved and restored before and
> after calling smmu_translate().
>
> Translating CD or TTBx can fail for the following reasons:
> 1) Large address size: This is described in
> (3.4.3 Address sizes of SMMU-originated accesses)
> - For CD ptr larger than IAS, for SMMUv3.1, it can trigger either
> C_BAD_STE or Translation fault, we implement the latter as it
> requires no extra code.
> - For TTBx, if larger than the effective stage 1 output address size, it
> triggers C_BAD_CD.
>
> 2) Faults from PTWs (7.3 Event records)
> - F_ADDR_SIZE: large address size after first level causes stage 2 Address
> Size fault (Also in 3.4.3 Address sizes of SMMU-originated accesses)
> - F_PERMISSION: Same as an address translation. However, when
> CLASS == CD, the access is implicitly Data and a read.
> - F_ACCESS: Same as an address translation.
> - F_TRANSLATION: Same as an address translation.
> - F_WALK_EABT: Same as an address translation.
> These are already implemented in the PTW logic, so no extra handling
> required.
>
> As in CD and TTBx translation context, the iova is not known, setting
> the InputAddr was removed from "smmuv3_do_translate" and set after
> from "smmuv3_translate" with the new function "smmuv3_fixup_event"
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/arm/smmuv3.c | 120 +++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 103 insertions(+), 17 deletions(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 3f2dfada44..73d5a25705 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -337,14 +337,35 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf,
>
> }
>
> +static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> + SMMUTransCfg *cfg,
> + SMMUEventInfo *event,
> + IOMMUAccessFlags flag,
> + SMMUTLBEntry **out_entry,
> + SMMUTranslationClass class);
> /* @ssid > 0 not supported yet */
> -static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid,
> - CD *buf, SMMUEventInfo *event)
> +static int smmu_get_cd(SMMUv3State *s, STE *ste, SMMUTransCfg *cfg,
> + uint32_t ssid, CD *buf, SMMUEventInfo *event)
> {
> dma_addr_t addr = STE_CTXPTR(ste);
> int ret, i;
> + SMMUTranslationStatus status;
> + SMMUTLBEntry *entry;
>
> trace_smmuv3_get_cd(addr);
> +
> + if (cfg->stage == SMMU_NESTED) {
> + status = smmuv3_do_translate(s, addr, cfg, event,
> + IOMMU_RO, &entry, SMMU_CLASS_CD);
> +
> + /* Same PTW faults are reported but with CLASS = CD. */
> + if (status != SMMU_TRANS_SUCCESS) {
> + return -EINVAL;
> + }
> +
> + addr = CACHED_ENTRY_TO_ADDR(entry, addr);
> + }
> +
> /* TODO: guarantee 64-bit single-copy atomicity */
> ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf),
> MEMTXATTRS_UNSPECIFIED);
> @@ -659,10 +680,13 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
> return 0;
> }
>
> -static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event)
> +static int decode_cd(SMMUv3State *s, SMMUTransCfg *cfg,
> + CD *cd, SMMUEventInfo *event)
> {
> int ret = -EINVAL;
> int i;
> + SMMUTranslationStatus status;
> + SMMUTLBEntry *entry;
>
> if (!CD_VALID(cd) || !CD_AARCH64(cd)) {
> goto bad_cd;
> @@ -713,9 +737,26 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event)
>
> tt->tsz = tsz;
> tt->ttb = CD_TTB(cd, i);
> +
> if (tt->ttb & ~(MAKE_64BIT_MASK(0, cfg->oas))) {
> goto bad_cd;
> }
> +
> + /* Translate the TTBx, from IPA to PA if nesting is enabled. */
> + if (cfg->stage == SMMU_NESTED) {
> + status = smmuv3_do_translate(s, tt->ttb, cfg, event, IOMMU_RO,
> + &entry, SMMU_CLASS_TT);
> + /*
> + * Same PTW faults are reported but with CLASS = TT.
> + * If TTBx is larger than the effective stage 1 output addres
> + * size, it reports C_BAD_CD, which is handled by the above case.
> + */
> + if (status != SMMU_TRANS_SUCCESS) {
> + return -EINVAL;
> + }
> + tt->ttb = CACHED_ENTRY_TO_ADDR(entry, tt->ttb);
> + }
> +
> tt->had = CD_HAD(cd, i);
> trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb, tt->granule_sz, tt->had);
> }
> @@ -767,12 +808,12 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg,
> return 0;
> }
>
> - ret = smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event);
> + ret = smmu_get_cd(s, &ste, cfg, 0 /* ssid */, &cd, event);
> if (ret) {
> return ret;
> }
>
> - return decode_cd(cfg, &cd, event);
> + return decode_cd(s, cfg, &cd, event);
> }
>
> /**
> @@ -832,58 +873,80 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> SMMUTransCfg *cfg,
> SMMUEventInfo *event,
> IOMMUAccessFlags flag,
> - SMMUTLBEntry **out_entry)
> + SMMUTLBEntry **out_entry,
> + SMMUTranslationClass class)
> {
> SMMUPTWEventInfo ptw_info = {};
> SMMUState *bs = ARM_SMMU(s);
> SMMUTLBEntry *cached_entry = NULL;
> + int asid, stage;
> + bool desc_s2_translation = class != SMMU_CLASS_IN;
> +
> + /*
> + * The function uses the argument class to identify which stage is used:
> + * - CLASS = IN: Means an input translation, determine the stage from STE.
> + * - CLASS = CD: Means the addr is an IPA of the CD, and it would be
> + * translated using the stage-2.
> + * - CLASS = TT: Means the addr is an IPA of the stage-1 translation table
> + * and it would be translated using the stage-2.
> + * For the last 2 cases instead of having intrusive changes in the common
> + * logic, we modify the cfg to be a stage-2 translation only in case of
> + * nested, and then restore it after.
> + */
> + if (desc_s2_translation) {
> + asid = cfg->asid;
> + stage = cfg->stage;
> + cfg->asid = -1;
> + cfg->stage = SMMU_STAGE_2;
> + }
>
> cached_entry = smmu_translate(bs, cfg, addr, flag, &ptw_info);
> +
> + if (desc_s2_translation) {
> + cfg->asid = asid;
> + cfg->stage = stage;
> + }
> +
> if (!cached_entry) {
> /* All faults from PTW has S2 field. */
> event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
> switch (ptw_info.type) {
> case SMMU_PTW_ERR_WALK_EABT:
> event->type = SMMU_EVT_F_WALK_EABT;
> - event->u.f_walk_eabt.addr = addr;
> event->u.f_walk_eabt.rnw = flag & 0x1;
> event->u.f_walk_eabt.class = (ptw_info.stage == SMMU_STAGE_2) ?
> - SMMU_CLASS_IN : SMMU_CLASS_TT;
> + class : SMMU_CLASS_TT;
> event->u.f_walk_eabt.addr2 = ptw_info.addr;
> break;
> case SMMU_PTW_ERR_TRANSLATION:
> if (PTW_RECORD_FAULT(cfg)) {
> event->type = SMMU_EVT_F_TRANSLATION;
> - event->u.f_translation.addr = addr;
> event->u.f_translation.addr2 = ptw_info.addr;
> - event->u.f_translation.class = SMMU_CLASS_IN;
> + event->u.f_translation.class = class;
> event->u.f_translation.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_ADDR_SIZE:
> if (PTW_RECORD_FAULT(cfg)) {
> event->type = SMMU_EVT_F_ADDR_SIZE;
> - event->u.f_addr_size.addr = addr;
> event->u.f_addr_size.addr2 = ptw_info.addr;
> - event->u.f_addr_size.class = SMMU_CLASS_IN;
> + event->u.f_addr_size.class = class;
> event->u.f_addr_size.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_ACCESS:
> if (PTW_RECORD_FAULT(cfg)) {
> event->type = SMMU_EVT_F_ACCESS;
> - event->u.f_access.addr = addr;
> event->u.f_access.addr2 = ptw_info.addr;
> - event->u.f_access.class = SMMU_CLASS_IN;
> + event->u.f_access.class = class;
> event->u.f_access.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_PERMISSION:
> if (PTW_RECORD_FAULT(cfg)) {
> event->type = SMMU_EVT_F_PERMISSION;
> - event->u.f_permission.addr = addr;
> event->u.f_permission.addr2 = ptw_info.addr;
> - event->u.f_permission.class = SMMU_CLASS_IN;
> + event->u.f_permission.class = class;
> event->u.f_permission.rnw = flag & 0x1;
> }
> break;
> @@ -896,6 +959,27 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> return SMMU_TRANS_SUCCESS;
> }
>
> +/*
> + * Sets the InputAddr for an SMMU_TRANS_ERROR, as it can't be
> + * set from all contexts, as smmuv3_get_config() can return
> + * translation faults in case of nested translation (for CD
> + * and TTBx). But in that case the iova is not known.
> + */
> +static void smmuv3_fixup_event(SMMUEventInfo *event, hwaddr iova)
> +{
> + switch (event->type) {
> + case SMMU_EVT_F_WALK_EABT:
> + case SMMU_EVT_F_TRANSLATION:
> + case SMMU_EVT_F_ADDR_SIZE:
> + case SMMU_EVT_F_ACCESS:
> + case SMMU_EVT_F_PERMISSION:
> + event->u.f_walk_eabt.addr = iova;
> + break;
> + default:
> + break;
> + }
> +}
> +
> /* Entry point to SMMU, does everything. */
> static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> IOMMUAccessFlags flag, int iommu_idx)
> @@ -944,7 +1028,8 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> goto epilogue;
> }
>
> - status = smmuv3_do_translate(s, addr, cfg, &event, flag, &cached_entry);
> + status = smmuv3_do_translate(s, addr, cfg, &event, flag,
> + &cached_entry, SMMU_CLASS_IN);
>
> epilogue:
> qemu_mutex_unlock(&s->mutex);
> @@ -975,6 +1060,7 @@ epilogue:
> entry.perm);
> break;
> case SMMU_TRANS_ERROR:
> + smmuv3_fixup_event(&event, addr);
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s translation failed for iova=0x%"PRIx64" (%s)\n",
> mr->parent_obj.name, addr, smmu_event_string(event.type));
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 09/18] hw/arm/smmu-common: Rework TLB lookup for nesting
2024-07-15 8:45 ` [PATCH v5 09/18] hw/arm/smmu-common: Rework TLB lookup for nesting Mostafa Saleh
@ 2024-07-17 15:28 ` Eric Auger
0 siblings, 0 replies; 35+ messages in thread
From: Eric Auger @ 2024-07-17 15:28 UTC (permalink / raw)
To: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz
On 7/15/24 10:45, Mostafa Saleh wrote:
> In the next patch, combine_tlb() will be added which combines 2 TLB
> entries into one for nested translations, which chooses the granule
> and level from the smallest entry.
>
> This means that with nested translation, an entry can be cached with
> the granule of stage-2 and not stage-1.
>
> However, currently, the lookup for an IOVA is done with input stage
> granule, which is stage-1 for nested configuration, which will not
> work with the above logic.
> This patch reworks lookup in that case, so it falls back to stage-2
> granule if no entry is found using stage-1 granule.
>
> Also, drop aligning the iova to avoid over-aligning in case the iova
> is cached with a smaller granule, the TLB lookup will align the iova
> anyway for each granule and level, and the page table walker doesn't
> consider the page offset bits.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/arm/smmu-common.c | 64 +++++++++++++++++++++++++++++---------------
> 1 file changed, 43 insertions(+), 21 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 21982621c0..f224e9c1e0 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -66,8 +66,10 @@ SMMUIOTLBKey smmu_get_iotlb_key(int asid, int vmid, uint64_t iova,
> return key;
> }
>
> -SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
> - SMMUTransTableInfo *tt, hwaddr iova)
> +static SMMUTLBEntry *smmu_iotlb_lookup_all_levels(SMMUState *bs,
> + SMMUTransCfg *cfg,
> + SMMUTransTableInfo *tt,
> + hwaddr iova)
> {
> uint8_t tg = (tt->granule_sz - 10) / 2;
> uint8_t inputsize = 64 - tt->tsz;
> @@ -88,6 +90,36 @@ SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
> }
> level++;
> }
> + return entry;
> +}
> +
> +/**
> + * smmu_iotlb_lookup - Look up for a TLB entry.
> + * @bs: SMMU state which includes the TLB instance
> + * @cfg: Configuration of the translation
> + * @tt: Translation table info (granule and tsz)
> + * @iova: IOVA address to lookup
> + *
> + * returns a valid entry on success, otherwise NULL.
> + * In case of nested translation, tt can be updated to include
> + * the granule of the found entry as it might different from
> + * the IOVA granule.
> + */
> +SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg *cfg,
> + SMMUTransTableInfo *tt, hwaddr iova)
> +{
> + SMMUTLBEntry *entry = NULL;
> +
> + entry = smmu_iotlb_lookup_all_levels(bs, cfg, tt, iova);
> + /*
> + * For nested translation also try the s2 granule, as the TLB will insert
> + * it if the size of s2 tlb entry was smaller.
> + */
> + if (!entry && (cfg->stage == SMMU_NESTED) &&
> + (cfg->s2cfg.granule_sz != tt->granule_sz)) {
> + tt->granule_sz = cfg->s2cfg.granule_sz;
> + entry = smmu_iotlb_lookup_all_levels(bs, cfg, tt, iova);
> + }
>
> if (entry) {
> cfg->iotlb_hits++;
> @@ -569,18 +601,21 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
> IOMMUAccessFlags flag, SMMUPTWEventInfo *info)
> {
> - uint64_t page_mask, aligned_addr;
> SMMUTLBEntry *cached_entry = NULL;
> SMMUTransTableInfo *tt;
> int status;
>
> /*
> - * Combined attributes used for TLB lookup, as only one stage is supported,
> - * it will hold attributes based on the enabled stage.
> + * Combined attributes used for TLB lookup, holds the attributes for
> + * the input stage.
> */
> SMMUTransTableInfo tt_combined;
>
> - if (cfg->stage == SMMU_STAGE_1) {
> + if (cfg->stage == SMMU_STAGE_2) {
> + /* Stage2. */
> + tt_combined.granule_sz = cfg->s2cfg.granule_sz;
> + tt_combined.tsz = cfg->s2cfg.tsz;
> + } else {
> /* Select stage1 translation table. */
> tt = select_tt(cfg, addr);
> if (!tt) {
> @@ -590,22 +625,9 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
> }
> tt_combined.granule_sz = tt->granule_sz;
> tt_combined.tsz = tt->tsz;
> -
> - } 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_combined, aligned_addr);
> + cached_entry = smmu_iotlb_lookup(bs, cfg, &tt_combined, addr);
> if (cached_entry) {
> if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
> info->type = SMMU_PTW_ERR_PERMISSION;
> @@ -616,7 +638,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
> }
>
> cached_entry = g_new0(SMMUTLBEntry, 1);
> - status = smmu_ptw(cfg, aligned_addr, flag, cached_entry, info);
> + status = smmu_ptw(cfg, addr, flag, cached_entry, info);
> if (status) {
> g_free(cached_entry);
> return NULL;
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 11/18] hw/arm/smmu-common: Support nested translation
2024-07-15 8:45 ` [PATCH v5 11/18] hw/arm/smmu-common: Support nested translation Mostafa Saleh
@ 2024-07-17 15:28 ` Eric Auger
0 siblings, 0 replies; 35+ messages in thread
From: Eric Auger @ 2024-07-17 15:28 UTC (permalink / raw)
To: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz
On 7/15/24 10:45, Mostafa Saleh wrote:
> When nested translation is requested, do the following:
> - Translate stage-1 table address IPA into PA through stage-2.
> - Translate stage-1 table walk output (IPA) through stage-2.
> - Create a single TLB entry from stage-1 and stage-2 translations
> using logic introduced before.
>
> smmu_ptw() has a new argument SMMUState which include the TLB as
> stage-1 table address can be cached in there.
>
> Also in smmu_ptw(), a separate path used for nesting to simplify the
> code, although some logic can be combined.
>
> With nested translation class of translation fault can be different,
> from the class of the translation, as faults from translating stage-1
> tables are considered as CLASS_TT and not CLASS_IN, a new member
> "is_ipa_descriptor" added to "SMMUPTWEventInfo" to differ faults
> from walking stage 1 translation table and faults from translating
> an IPA for a transaction.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/arm/smmu-common.c | 74 +++++++++++++++++++++++++++++++-----
> hw/arm/smmuv3.c | 14 +++++++
> include/hw/arm/smmu-common.h | 7 ++--
> 3 files changed, 82 insertions(+), 13 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index c894c4c621..8ed53f5b1d 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -318,8 +318,41 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t iova)
> return NULL;
> }
>
> +/* Translate stage-1 table address using stage-2 page table. */
> +static inline int translate_table_addr_ipa(SMMUState *bs,
> + dma_addr_t *table_addr,
> + SMMUTransCfg *cfg,
> + SMMUPTWEventInfo *info)
> +{
> + dma_addr_t addr = *table_addr;
> + SMMUTLBEntry *cached_entry;
> + int asid;
> +
> + /*
> + * The translation table walks performed from TTB0 or TTB1 are always
> + * performed in IPA space if stage 2 translations are enabled.
> + */
> + asid = cfg->asid;
> + cfg->stage = SMMU_STAGE_2;
> + cfg->asid = -1;
> + cached_entry = smmu_translate(bs, cfg, addr, IOMMU_RO, info);
> + cfg->asid = asid;
> + cfg->stage = SMMU_NESTED;
> +
> + if (cached_entry) {
> + *table_addr = CACHED_ENTRY_TO_ADDR(cached_entry, addr);
> + return 0;
> + }
> +
> + info->stage = SMMU_STAGE_2;
> + info->addr = addr;
> + info->is_ipa_descriptor = true;
> + return -EINVAL;
> +}
> +
> /**
> * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA
> + * @bs: smmu state which includes TLB instance
> * @cfg: translation config
> * @iova: iova to translate
> * @perm: access type
> @@ -331,7 +364,7 @@ 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_s1(SMMUTransCfg *cfg,
> +static int smmu_ptw_64_s1(SMMUState *bs, SMMUTransCfg *cfg,
> dma_addr_t iova, IOMMUAccessFlags perm,
> SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> @@ -381,6 +414,11 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
> goto error;
> }
> baseaddr = get_table_pte_address(pte, granule_sz);
> + if (cfg->stage == SMMU_NESTED) {
> + if (translate_table_addr_ipa(bs, &baseaddr, cfg, info)) {
> + goto error;
> + }
> + }
> level++;
> continue;
> } else if (is_page_pte(pte, level)) {
> @@ -568,10 +606,8 @@ error:
> * combine S1 and S2 TLB entries into a single entry.
> * As a result the S1 entry is overriden with combined data.
> */
> -static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe,
> - SMMUTLBEntry *tlbe_s2,
> - dma_addr_t iova,
> - SMMUTransCfg *cfg)
> +static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2,
> + dma_addr_t iova, SMMUTransCfg *cfg)
> {
> if (tlbe_s2->entry.addr_mask < tlbe->entry.addr_mask) {
> tlbe->entry.addr_mask = tlbe_s2->entry.addr_mask;
> @@ -591,6 +627,7 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe,
> /**
> * smmu_ptw - Walk the page tables for an IOVA, according to @cfg
> *
> + * @bs: smmu state which includes TLB instance
> * @cfg: translation configuration
> * @iova: iova to translate
> * @perm: tentative access type
> @@ -599,11 +636,15 @@ static void __attribute__((unused)) combine_tlb(SMMUTLBEntry *tlbe,
> *
> * return 0 on success
> */
> -int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> +int smmu_ptw(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t iova,
> + IOMMUAccessFlags perm, SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> + int ret;
> + SMMUTLBEntry tlbe_s2;
> + dma_addr_t ipa;
> +
> if (cfg->stage == SMMU_STAGE_1) {
> - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
> + return smmu_ptw_64_s1(bs, cfg, iova, perm, tlbe, info);
> } else if (cfg->stage == SMMU_STAGE_2) {
> /*
> * If bypassing stage 1(or unimplemented), the input address is passed
> @@ -621,7 +662,20 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info);
> }
>
> - g_assert_not_reached();
> + /* SMMU_NESTED. */
> + ret = smmu_ptw_64_s1(bs, cfg, iova, perm, tlbe, info);
> + if (ret) {
> + return ret;
> + }
> +
> + ipa = CACHED_ENTRY_TO_ADDR(tlbe, iova);
> + ret = smmu_ptw_64_s2(cfg, ipa, perm, &tlbe_s2, info);
> + if (ret) {
> + return ret;
> + }
> +
> + combine_tlb(tlbe, &tlbe_s2, iova, cfg);
> + return 0;
> }
>
> SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
> @@ -667,7 +721,7 @@ SMMUTLBEntry *smmu_translate(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t addr,
> }
>
> cached_entry = g_new0(SMMUTLBEntry, 1);
> - status = smmu_ptw(cfg, addr, flag, cached_entry, info);
> + status = smmu_ptw(bs, cfg, addr, flag, cached_entry, info);
> if (status) {
> g_free(cached_entry);
> return NULL;
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 73d5a25705..06a96c65eb 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -910,6 +910,20 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> if (!cached_entry) {
> /* All faults from PTW has S2 field. */
> event->u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
> + /*
> + * Fault class is set as follows based on "class" input to
> + * the function and to "ptw_info" from "smmu_translate()"
> + * For stage-1:
> + * - EABT => CLASS_TT (hardcoded)
> + * - other events => CLASS_IN (input to function)
> + * For stage-2 => CLASS_IN (input to function)
> + * For nested, for all events:
> + * - CD fetch => CLASS_CD (input to function)
> + * - walking stage 1 translation table => CLASS_TT (from
> + * is_ipa_descriptor or input in case of TTBx)
> + * - s2 translation => CLASS_IN (input to function)
> + */
> + class = ptw_info.is_ipa_descriptor ? SMMU_CLASS_TT : class;
> switch (ptw_info.type) {
> case SMMU_PTW_ERR_WALK_EABT:
> event->type = SMMU_EVT_F_WALK_EABT;
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index d84de64122..a3e6ab1b36 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -63,6 +63,7 @@ typedef struct SMMUPTWEventInfo {
> SMMUStage stage;
> SMMUPTWEventType type;
> dma_addr_t addr; /* fetched address that induced an abort, if any */
> + bool is_ipa_descriptor; /* src for fault in nested translation. */
> } SMMUPTWEventInfo;
>
> typedef struct SMMUTransTableInfo {
> @@ -184,9 +185,9 @@ static inline uint16_t smmu_get_sid(SMMUDevice *sdev)
> * smmu_ptw - Perform the page table walk for a given iova / access flags
> * pair, according to @cfg translation config
> */
> -int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> - SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info);
> -
> +int smmu_ptw(SMMUState *bs, SMMUTransCfg *cfg, dma_addr_t iova,
> + IOMMUAccessFlags perm, SMMUTLBEntry *tlbe,
> + SMMUPTWEventInfo *info);
>
> /*
> * smmu_translate - Look for a translation in TLB, if not, do a PTW.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 15/18] hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova()
2024-07-15 8:45 ` [PATCH v5 15/18] hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova() Mostafa Saleh
@ 2024-07-17 15:30 ` Eric Auger
0 siblings, 0 replies; 35+ messages in thread
From: Eric Auger @ 2024-07-17 15:30 UTC (permalink / raw)
To: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz
On 7/15/24 10:45, Mostafa Saleh wrote:
> IOMMUTLBEvent only understands IOVA, for stage-1 or stage-2
> SMMU instances we consider the input address as the IOVA, but when
> nesting is used, we can't mix stage-1 and stage-2 addresses, so for
> nesting only stage-1 is considered the IOVA and would be notified.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/arm/smmuv3.c | 39 +++++++++++++++++++++++++--------------
> hw/arm/trace-events | 2 +-
> 2 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 9a88b83511..84cd314b33 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1096,27 +1096,38 @@ epilogue:
> * @iova: iova
> * @tg: translation granule (if communicated through range invalidation)
> * @num_pages: number of @granule sized pages (if tg != 0), otherwise 1
> + * @stage: Which stage(1 or 2) is used
> */
> static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
> IOMMUNotifier *n,
> int asid, int vmid,
> dma_addr_t iova, uint8_t tg,
> - uint64_t num_pages)
> + uint64_t num_pages, int stage)
> {
> SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
> + SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
> + SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
> IOMMUTLBEvent event;
> uint8_t granule;
> - SMMUv3State *s = sdev->smmu;
> +
> + if (!cfg) {
> + return;
> + }
> +
> + /*
> + * stage is passed from TLB invalidation commands which can be either
> + * stage-1 or stage-2.
> + * However, IOMMUTLBEvent only understands IOVA, for stage-1 or stage-2
> + * SMMU instances we consider the input address as the IOVA, but when
> + * nesting is used, we can't mix stage-1 and stage-2 addresses, so for
> + * nesting only stage-1 is considered the IOVA and would be notified.
> + */
> + if ((stage == SMMU_STAGE_2) && (cfg->stage == SMMU_NESTED))
> + return;
>
> if (!tg) {
> - SMMUEventInfo eventinfo = {.inval_ste_allowed = true};
> - SMMUTransCfg *cfg = smmuv3_get_config(sdev, &eventinfo);
> SMMUTransTableInfo *tt;
>
> - if (!cfg) {
> - return;
> - }
> -
> if (asid >= 0 && cfg->asid != asid) {
> return;
> }
> @@ -1125,7 +1136,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
> return;
> }
>
> - if (STAGE1_SUPPORTED(s)) {
> + if (stage == SMMU_STAGE_1) {
> tt = select_tt(cfg, iova);
> if (!tt) {
> return;
> @@ -1151,7 +1162,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
> /* 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)
> + uint64_t num_pages, int stage)
> {
> SMMUDevice *sdev;
>
> @@ -1160,10 +1171,10 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, int vmid,
> IOMMUNotifier *n;
>
> trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, vmid,
> - iova, tg, num_pages);
> + iova, tg, num_pages, stage);
>
> IOMMU_NOTIFIER_FOREACH(n, mr) {
> - smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages);
> + smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages, stage);
> }
> }
> }
> @@ -1194,7 +1205,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd, SMMUStage stage)
>
> if (!tg) {
> trace_smmuv3_range_inval(vmid, asid, addr, tg, 1, ttl, leaf, stage);
> - smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
> + smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1, stage);
> if (stage == SMMU_STAGE_1) {
> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
> } else {
> @@ -1217,7 +1228,7 @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd, SMMUStage stage)
> num_pages = (mask + 1) >> granule;
> trace_smmuv3_range_inval(vmid, asid, addr, tg, num_pages,
> ttl, leaf, stage);
> - smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
> + smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages, stage);
> if (stage == SMMU_STAGE_1) {
> smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
> } else {
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index 593cc571da..be6c8f720b 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -55,7 +55,7 @@ smmuv3_cmdq_tlbi_s12_vmid(int 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, int asid, int 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
> +smmuv3_inv_notifiers_iova(const char *name, int asid, int vmid, uint64_t iova, uint8_t tg, uint64_t num_pages, int stage) "iommu mr=%s asid=%d vmid=%d iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" stage=%d"
>
> # strongarm.c
> strongarm_uart_update_parameters(const char *label, int speed, char parity, int data_bits, int stop_bits) "%s speed=%d parity=%c data=%d stop=%d"
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 16/18] hw/arm/smmuv3: Handle translation faults according to SMMUPTWEventInfo
2024-07-15 8:45 ` [PATCH v5 16/18] hw/arm/smmuv3: Handle translation faults according to SMMUPTWEventInfo Mostafa Saleh
@ 2024-07-17 15:31 ` Eric Auger
0 siblings, 0 replies; 35+ messages in thread
From: Eric Auger @ 2024-07-17 15:31 UTC (permalink / raw)
To: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel
Cc: jean-philippe, alex.bennee, maz, nicolinc, julien,
richard.henderson, marcin.juszkiewicz
On 7/15/24 10:45, Mostafa Saleh wrote:
> Previously, to check if faults are enabled, it was sufficient to check
> the current stage of translation and check the corresponding
> record_faults flag.
>
> However, with nesting, it is possible for stage-1 (nested) translation
> to trigger a stage-2 fault, so we check SMMUPTWEventInfo as it would
> have the correct stage set from the page table walk.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/arm/smmuv3.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 84cd314b33..d052a2ba24 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -34,9 +34,10 @@
> #include "smmuv3-internal.h"
> #include "smmu-internal.h"
>
> -#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == SMMU_STAGE_1) ? \
> - (cfg)->record_faults : \
> - (cfg)->s2cfg.record_faults)
> +#define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1 && \
> + (cfg)->record_faults) || \
> + ((ptw_info).stage == SMMU_STAGE_2 && \
> + (cfg)->s2cfg.record_faults))
>
> /**
> * smmuv3_trigger_irq - pulse @irq if enabled and update
> @@ -933,7 +934,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> event->u.f_walk_eabt.addr2 = ptw_info.addr;
> break;
> case SMMU_PTW_ERR_TRANSLATION:
> - if (PTW_RECORD_FAULT(cfg)) {
> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
> event->type = SMMU_EVT_F_TRANSLATION;
> event->u.f_translation.addr2 = ptw_info.addr;
> event->u.f_translation.class = class;
> @@ -941,7 +942,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> }
> break;
> case SMMU_PTW_ERR_ADDR_SIZE:
> - if (PTW_RECORD_FAULT(cfg)) {
> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
> event->type = SMMU_EVT_F_ADDR_SIZE;
> event->u.f_addr_size.addr2 = ptw_info.addr;
> event->u.f_addr_size.class = class;
> @@ -949,7 +950,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> }
> break;
> case SMMU_PTW_ERR_ACCESS:
> - if (PTW_RECORD_FAULT(cfg)) {
> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
> event->type = SMMU_EVT_F_ACCESS;
> event->u.f_access.addr2 = ptw_info.addr;
> event->u.f_access.class = class;
> @@ -957,7 +958,7 @@ static SMMUTranslationStatus smmuv3_do_translate(SMMUv3State *s, hwaddr addr,
> }
> break;
> case SMMU_PTW_ERR_PERMISSION:
> - if (PTW_RECORD_FAULT(cfg)) {
> + if (PTW_RECORD_FAULT(ptw_info, cfg)) {
> event->type = SMMU_EVT_F_PERMISSION;
> event->u.f_permission.addr2 = ptw_info.addr;
> event->u.f_permission.class = class;
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events
2024-07-17 15:07 ` Eric Auger
@ 2024-07-17 15:58 ` Jean-Philippe Brucker
2024-07-17 15:59 ` Eric Auger
0 siblings, 1 reply; 35+ messages in thread
From: Jean-Philippe Brucker @ 2024-07-17 15:58 UTC (permalink / raw)
To: Eric Auger
Cc: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel, alex.bennee,
maz, nicolinc, julien, richard.henderson, marcin.juszkiewicz
Hi Eric,
On Wed, Jul 17, 2024 at 05:07:57PM +0200, Eric Auger wrote:
> Hi Jean,
>
> On 7/15/24 10:45, Mostafa Saleh wrote:
> > The SMMUv3 spec (ARM IHI 0070 F.b - 7.3 Event records) defines the
> > class of events faults as:
> >
> > CLASS: The class of the operation that caused the fault:
> > - 0b00: CD, CD fetch.
> > - 0b01: TTD, Stage 1 translation table fetch.
> > - 0b10: IN, Input address
> >
> > However, this value was not set and left as 0 which means CD and not
> > IN (0b10).
> >
> > Another problem was that stage-2 class is considered IN not TT for
> > EABT, according to the spec:
> > Translation of an IPA after successful stage 1 translation (or,
> > in stage 2-only configuration, an input IPA)
> > - S2 == 1 (stage 2), CLASS == IN (Input to stage)
> >
> > This would change soon when nested translations are supported.
> >
> > While at it, add an enum for class as it would be used for nesting.
> > However, at the moment stage-1 and stage-2 use the same class values,
> > except for EABT.
> >
> > Fixes: 9bde7f0674 “hw/arm/smmuv3: Implement translate callback”
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> > hw/arm/smmuv3-internal.h | 6 ++++++
> > hw/arm/smmuv3.c | 8 +++++++-
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> > index e4dd11e1e6..0f3ecec804 100644
> > --- a/hw/arm/smmuv3-internal.h
> > +++ b/hw/arm/smmuv3-internal.h
> > @@ -32,6 +32,12 @@ typedef enum SMMUTranslationStatus {
> > SMMU_TRANS_SUCCESS,
> > } SMMUTranslationStatus;
> >
> > +typedef enum SMMUTranslationClass {
> > + SMMU_CLASS_CD,
> > + SMMU_CLASS_TT,
> > + SMMU_CLASS_IN,
> > +} SMMUTranslationClass;
> > +
> > /* MMIO Registers */
> >
> > REG32(IDR0, 0x0)
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index 9dd3ea48e4..3d214c9f57 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -942,7 +942,9 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> > event.type = SMMU_EVT_F_WALK_EABT;
> > event.u.f_walk_eabt.addr = addr;
> > event.u.f_walk_eabt.rnw = flag & 0x1;
> > - event.u.f_walk_eabt.class = 0x1;
> > + /* Stage-2 (only) is class IN while stage-1 is class TT */
> > + event.u.f_walk_eabt.class = (ptw_info.stage == 2) ?
> > + SMMU_CLASS_IN : SMMU_CLASS_TT;
> does it match your expectations. While reading your previous comment I
> have the impression what you had in mind was more complicated than that
>
> * s2 walk that encounters EABT on S2 descriptor while translating
> non-descriptor IPA is reported as class=IN, even when doing s2-only.
At this point we only support single-stage, so I believe this is correct:
"A stage 1-only table walk that encounters EABT ... CLASS == TT"
"translation of ... in stage 2-only configuration, an input IPA...
CLASS == IN"
Later in the series this code changes in order to support nesting, but I
think it's still correct, because the EABT class works similarly to
translation errors, except for stage-1 faults which have CLASS==TT
Thanks,
Jean
>
> Thanks
>
> Eric
>
> > event.u.f_walk_eabt.addr2 = ptw_info.addr;
> > break;
> > case SMMU_PTW_ERR_TRANSLATION:
> > @@ -950,6 +952,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> > event.type = SMMU_EVT_F_TRANSLATION;
> > event.u.f_translation.addr = addr;
> > event.u.f_translation.addr2 = ptw_info.addr;
> > + event.u.f_translation.class = SMMU_CLASS_IN;
> > event.u.f_translation.rnw = flag & 0x1;
> > }
> > break;
> > @@ -958,6 +961,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> > event.type = SMMU_EVT_F_ADDR_SIZE;
> > event.u.f_addr_size.addr = addr;
> > event.u.f_addr_size.addr2 = ptw_info.addr;
> > + event.u.f_translation.class = SMMU_CLASS_IN;
> > event.u.f_addr_size.rnw = flag & 0x1;
> > }
> > break;
> > @@ -966,6 +970,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> > event.type = SMMU_EVT_F_ACCESS;
> > event.u.f_access.addr = addr;
> > event.u.f_access.addr2 = ptw_info.addr;
> > + event.u.f_translation.class = SMMU_CLASS_IN;
> > event.u.f_access.rnw = flag & 0x1;
> > }
> > break;
> > @@ -974,6 +979,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> > event.type = SMMU_EVT_F_PERMISSION;
> > event.u.f_permission.addr = addr;
> > event.u.f_permission.addr2 = ptw_info.addr;
> > + event.u.f_translation.class = SMMU_CLASS_IN;
> > event.u.f_permission.rnw = flag & 0x1;
> > }
> > break;
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events
2024-07-17 15:58 ` Jean-Philippe Brucker
@ 2024-07-17 15:59 ` Eric Auger
0 siblings, 0 replies; 35+ messages in thread
From: Eric Auger @ 2024-07-17 15:59 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: Mostafa Saleh, qemu-arm, peter.maydell, qemu-devel, alex.bennee,
maz, nicolinc, julien, richard.henderson, marcin.juszkiewicz
On 7/17/24 17:58, Jean-Philippe Brucker wrote:
> Hi Eric,
>
> On Wed, Jul 17, 2024 at 05:07:57PM +0200, Eric Auger wrote:
>> Hi Jean,
>>
>> On 7/15/24 10:45, Mostafa Saleh wrote:
>>> The SMMUv3 spec (ARM IHI 0070 F.b - 7.3 Event records) defines the
>>> class of events faults as:
>>>
>>> CLASS: The class of the operation that caused the fault:
>>> - 0b00: CD, CD fetch.
>>> - 0b01: TTD, Stage 1 translation table fetch.
>>> - 0b10: IN, Input address
>>>
>>> However, this value was not set and left as 0 which means CD and not
>>> IN (0b10).
>>>
>>> Another problem was that stage-2 class is considered IN not TT for
>>> EABT, according to the spec:
>>> Translation of an IPA after successful stage 1 translation (or,
>>> in stage 2-only configuration, an input IPA)
>>> - S2 == 1 (stage 2), CLASS == IN (Input to stage)
>>>
>>> This would change soon when nested translations are supported.
>>>
>>> While at it, add an enum for class as it would be used for nesting.
>>> However, at the moment stage-1 and stage-2 use the same class values,
>>> except for EABT.
>>>
>>> Fixes: 9bde7f0674 “hw/arm/smmuv3: Implement translate callback”
>>> Signed-off-by: Mostafa Saleh <smostafa@google.com>
>>> ---
>>> hw/arm/smmuv3-internal.h | 6 ++++++
>>> hw/arm/smmuv3.c | 8 +++++++-
>>> 2 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
>>> index e4dd11e1e6..0f3ecec804 100644
>>> --- a/hw/arm/smmuv3-internal.h
>>> +++ b/hw/arm/smmuv3-internal.h
>>> @@ -32,6 +32,12 @@ typedef enum SMMUTranslationStatus {
>>> SMMU_TRANS_SUCCESS,
>>> } SMMUTranslationStatus;
>>>
>>> +typedef enum SMMUTranslationClass {
>>> + SMMU_CLASS_CD,
>>> + SMMU_CLASS_TT,
>>> + SMMU_CLASS_IN,
>>> +} SMMUTranslationClass;
>>> +
>>> /* MMIO Registers */
>>>
>>> REG32(IDR0, 0x0)
>>> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
>>> index 9dd3ea48e4..3d214c9f57 100644
>>> --- a/hw/arm/smmuv3.c
>>> +++ b/hw/arm/smmuv3.c
>>> @@ -942,7 +942,9 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>> event.type = SMMU_EVT_F_WALK_EABT;
>>> event.u.f_walk_eabt.addr = addr;
>>> event.u.f_walk_eabt.rnw = flag & 0x1;
>>> - event.u.f_walk_eabt.class = 0x1;
>>> + /* Stage-2 (only) is class IN while stage-1 is class TT */
>>> + event.u.f_walk_eabt.class = (ptw_info.stage == 2) ?
>>> + SMMU_CLASS_IN : SMMU_CLASS_TT;
>> does it match your expectations. While reading your previous comment I
>> have the impression what you had in mind was more complicated than that
>>
>> * s2 walk that encounters EABT on S2 descriptor while translating
>> non-descriptor IPA is reported as class=IN, even when doing s2-only.
> At this point we only support single-stage, so I believe this is correct:
> "A stage 1-only table walk that encounters EABT ... CLASS == TT"
> "translation of ... in stage 2-only configuration, an input IPA...
> CLASS == IN"
>
> Later in the series this code changes in order to support nesting, but I
> think it's still correct, because the EABT class works similarly to
> translation errors, except for stage-1 faults which have CLASS==TT
OK thank you for the confirmation!
Eric
>
> Thanks,
> Jean
>
>> Thanks
>>
>> Eric
>>
>>> event.u.f_walk_eabt.addr2 = ptw_info.addr;
>>> break;
>>> case SMMU_PTW_ERR_TRANSLATION:
>>> @@ -950,6 +952,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>> event.type = SMMU_EVT_F_TRANSLATION;
>>> event.u.f_translation.addr = addr;
>>> event.u.f_translation.addr2 = ptw_info.addr;
>>> + event.u.f_translation.class = SMMU_CLASS_IN;
>>> event.u.f_translation.rnw = flag & 0x1;
>>> }
>>> break;
>>> @@ -958,6 +961,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>> event.type = SMMU_EVT_F_ADDR_SIZE;
>>> event.u.f_addr_size.addr = addr;
>>> event.u.f_addr_size.addr2 = ptw_info.addr;
>>> + event.u.f_translation.class = SMMU_CLASS_IN;
>>> event.u.f_addr_size.rnw = flag & 0x1;
>>> }
>>> break;
>>> @@ -966,6 +970,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>> event.type = SMMU_EVT_F_ACCESS;
>>> event.u.f_access.addr = addr;
>>> event.u.f_access.addr2 = ptw_info.addr;
>>> + event.u.f_translation.class = SMMU_CLASS_IN;
>>> event.u.f_access.rnw = flag & 0x1;
>>> }
>>> break;
>>> @@ -974,6 +979,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>>> event.type = SMMU_EVT_F_PERMISSION;
>>> event.u.f_permission.addr = addr;
>>> event.u.f_permission.addr2 = ptw_info.addr;
>>> + event.u.f_translation.class = SMMU_CLASS_IN;
>>> event.u.f_permission.rnw = flag & 0x1;
>>> }
>>> break;
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-17 15:09 ` [PATCH v5 00/18] SMMUv3 nested translation support Jean-Philippe Brucker
@ 2024-07-17 17:43 ` Eric Auger
2024-07-17 19:06 ` Peter Maydell
2024-07-18 9:43 ` Julien Grall
0 siblings, 2 replies; 35+ messages in thread
From: Eric Auger @ 2024-07-17 17:43 UTC (permalink / raw)
To: Jean-Philippe Brucker, Mostafa Saleh
Cc: qemu-arm, peter.maydell, qemu-devel, alex.bennee, maz, nicolinc,
julien, richard.henderson, marcin.juszkiewicz
Hi Peter, Richard,
On 7/17/24 17:09, Jean-Philippe Brucker wrote:
> On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
>> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
>> but not nested instances.
>> This patch series adds support for nested translation in SMMUv3,
>> this is controlled by property “arm-smmuv3.stage=nested”, and
>> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
> For the whole series (3-9, 11, 12, 15, 16, 18):
>
> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>
> (and I think patch 16 is missing Eric's R-b)
Jean-Philippe and I have followed up the progress of this series,
Mostafa took into account all our comments and all the patches were
reviewed. It seems to be in a pretty decent state so if you don't have
any objection, please consider pulling it for 9.1.
On my end I did some testing in non nesting mode with virtio-net/vhost
and I have not noticed any regression.
Would be nice if someone could send his T-b for the nested part though
(Julien?).
Thanks
Eric
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-17 17:43 ` Eric Auger
@ 2024-07-17 19:06 ` Peter Maydell
2024-07-18 9:43 ` Julien Grall
1 sibling, 0 replies; 35+ messages in thread
From: Peter Maydell @ 2024-07-17 19:06 UTC (permalink / raw)
To: eric.auger
Cc: Jean-Philippe Brucker, Mostafa Saleh, qemu-arm, qemu-devel,
alex.bennee, maz, nicolinc, julien, richard.henderson,
marcin.juszkiewicz
On Wed, 17 Jul 2024 at 18:44, Eric Auger <eric.auger@redhat.com> wrote:
>
> Hi Peter, Richard,
>
> On 7/17/24 17:09, Jean-Philippe Brucker wrote:
> > On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
> >> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
> >> but not nested instances.
> >> This patch series adds support for nested translation in SMMUv3,
> >> this is controlled by property “arm-smmuv3.stage=nested”, and
> >> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
> > For the whole series (3-9, 11, 12, 15, 16, 18):
> >
> > Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> >
> > (and I think patch 16 is missing Eric's R-b)
>
> Jean-Philippe and I have followed up the progress of this series,
> Mostafa took into account all our comments and all the patches were
> reviewed. It seems to be in a pretty decent state so if you don't have
> any objection, please consider pulling it for 9.1.
Yep, I've had this series on my radar since you said in a
comment on the previous revision that it was close to
being ready. Thanks to both you and Jean-Philippe for doing
the heavy lifting on the code review here.
Applied to target-arm.next for 9.1, thanks.
-- PMM
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-17 17:43 ` Eric Auger
2024-07-17 19:06 ` Peter Maydell
@ 2024-07-18 9:43 ` Julien Grall
2024-07-19 15:36 ` Julien Grall
1 sibling, 1 reply; 35+ messages in thread
From: Julien Grall @ 2024-07-18 9:43 UTC (permalink / raw)
To: eric.auger, Jean-Philippe Brucker, Mostafa Saleh
Cc: qemu-arm, peter.maydell, qemu-devel, alex.bennee, maz, nicolinc,
richard.henderson, marcin.juszkiewicz
Hi Eric,
On 17/07/2024 18:43, Eric Auger wrote:
> Hi Peter, Richard,
>
> On 7/17/24 17:09, Jean-Philippe Brucker wrote:
>> On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
>>> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
>>> but not nested instances.
>>> This patch series adds support for nested translation in SMMUv3,
>>> this is controlled by property “arm-smmuv3.stage=nested”, and
>>> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
>> For the whole series (3-9, 11, 12, 15, 16, 18):
>>
>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>
>> (and I think patch 16 is missing Eric's R-b)
>
> Jean-Philippe and I have followed up the progress of this series,
> Mostafa took into account all our comments and all the patches were
> reviewed. It seems to be in a pretty decent state so if you don't have
> any objection, please consider pulling it for 9.1.
>
> On my end I did some testing in non nesting mode with virtio-net/vhost
> and I have not noticed any regression.
> Would be nice if someone could send his T-b for the nested part though
> (Julien?).
I haven't yet tried the latest version. I will do that in the next
couple of days.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-18 9:43 ` Julien Grall
@ 2024-07-19 15:36 ` Julien Grall
2024-07-19 15:57 ` Peter Maydell
0 siblings, 1 reply; 35+ messages in thread
From: Julien Grall @ 2024-07-19 15:36 UTC (permalink / raw)
To: eric.auger, Jean-Philippe Brucker, Mostafa Saleh
Cc: qemu-arm, peter.maydell, qemu-devel, alex.bennee, maz, nicolinc,
richard.henderson, marcin.juszkiewicz
Hi,
On 18/07/2024 10:43, Julien Grall wrote:
> Hi Eric,
>
> On 17/07/2024 18:43, Eric Auger wrote:
>> Hi Peter, Richard,
>>
>> On 7/17/24 17:09, Jean-Philippe Brucker wrote:
>>> On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
>>>> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
>>>> but not nested instances.
>>>> This patch series adds support for nested translation in SMMUv3,
>>>> this is controlled by property “arm-smmuv3.stage=nested”, and
>>>> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
>>> For the whole series (3-9, 11, 12, 15, 16, 18):
>>>
>>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>>
>>> (and I think patch 16 is missing Eric's R-b)
>>
>> Jean-Philippe and I have followed up the progress of this series,
>> Mostafa took into account all our comments and all the patches were
>> reviewed. It seems to be in a pretty decent state so if you don't have
>> any objection, please consider pulling it for 9.1.
>>
>> On my end I did some testing in non nesting mode with virtio-net/vhost
>> and I have not noticed any regression.
>> Would be nice if someone could send his T-b for the nested part though
>> (Julien?).
>
> I haven't yet tried the latest version. I will do that in the next
> couple of days.
I see this is already merged. If this still matters:
Tested-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-19 15:36 ` Julien Grall
@ 2024-07-19 15:57 ` Peter Maydell
2024-07-20 22:11 ` Mostafa Saleh
0 siblings, 1 reply; 35+ messages in thread
From: Peter Maydell @ 2024-07-19 15:57 UTC (permalink / raw)
To: Julien Grall
Cc: eric.auger, Jean-Philippe Brucker, Mostafa Saleh, qemu-arm,
qemu-devel, alex.bennee, maz, nicolinc, richard.henderson,
marcin.juszkiewicz
On Fri, 19 Jul 2024 at 16:36, Julien Grall <julien@xen.org> wrote:
>
> Hi,
>
> On 18/07/2024 10:43, Julien Grall wrote:
> > Hi Eric,
> >
> > On 17/07/2024 18:43, Eric Auger wrote:
> >> Hi Peter, Richard,
> >>
> >> On 7/17/24 17:09, Jean-Philippe Brucker wrote:
> >>> On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
> >>>> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
> >>>> but not nested instances.
> >>>> This patch series adds support for nested translation in SMMUv3,
> >>>> this is controlled by property “arm-smmuv3.stage=nested”, and
> >>>> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
> >>> For the whole series (3-9, 11, 12, 15, 16, 18):
> >>>
> >>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> >>>
> >>> (and I think patch 16 is missing Eric's R-b)
> >>
> >> Jean-Philippe and I have followed up the progress of this series,
> >> Mostafa took into account all our comments and all the patches were
> >> reviewed. It seems to be in a pretty decent state so if you don't have
> >> any objection, please consider pulling it for 9.1.
> >>
> >> On my end I did some testing in non nesting mode with virtio-net/vhost
> >> and I have not noticed any regression.
> >> Would be nice if someone could send his T-b for the nested part though
> >> (Julien?).
> >
> > I haven't yet tried the latest version. I will do that in the next
> > couple of days.
> I see this is already merged. If this still matters:
>
> Tested-by: Julien Grall <jgrall@amazon.com>
We can't retrospectively add the tag, but the testing itself
is still important -- thanks for doing it.
Q: is there any reason not to:
(a) change the default to "nested" rather than "1"
(b) make the virt board (for new virt machine versions) use
"nested"?
AIUI "nested" should be a superset of "stage-1 only", the guest
can just ignore stage-2 if it doesn't care about it. Or is
there a performance hit from having stage-2 around even if the
guest doesn't enable it?
thanks
-- PMM
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-19 15:57 ` Peter Maydell
@ 2024-07-20 22:11 ` Mostafa Saleh
2024-07-22 9:35 ` Peter Maydell
0 siblings, 1 reply; 35+ messages in thread
From: Mostafa Saleh @ 2024-07-20 22:11 UTC (permalink / raw)
To: Peter Maydell
Cc: Julien Grall, eric.auger, Jean-Philippe Brucker, qemu-arm,
qemu-devel, alex.bennee, maz, nicolinc, richard.henderson,
marcin.juszkiewicz
Hi Peter,
On Fri, Jul 19, 2024 at 04:57:18PM +0100, Peter Maydell wrote:
> On Fri, 19 Jul 2024 at 16:36, Julien Grall <julien@xen.org> wrote:
> >
> > Hi,
> >
> > On 18/07/2024 10:43, Julien Grall wrote:
> > > Hi Eric,
> > >
> > > On 17/07/2024 18:43, Eric Auger wrote:
> > >> Hi Peter, Richard,
> > >>
> > >> On 7/17/24 17:09, Jean-Philippe Brucker wrote:
> > >>> On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
> > >>>> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
> > >>>> but not nested instances.
> > >>>> This patch series adds support for nested translation in SMMUv3,
> > >>>> this is controlled by property “arm-smmuv3.stage=nested”, and
> > >>>> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
> > >>> For the whole series (3-9, 11, 12, 15, 16, 18):
> > >>>
> > >>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > >>>
> > >>> (and I think patch 16 is missing Eric's R-b)
> > >>
> > >> Jean-Philippe and I have followed up the progress of this series,
> > >> Mostafa took into account all our comments and all the patches were
> > >> reviewed. It seems to be in a pretty decent state so if you don't have
> > >> any objection, please consider pulling it for 9.1.
> > >>
> > >> On my end I did some testing in non nesting mode with virtio-net/vhost
> > >> and I have not noticed any regression.
> > >> Would be nice if someone could send his T-b for the nested part though
> > >> (Julien?).
> > >
> > > I haven't yet tried the latest version. I will do that in the next
> > > couple of days.
> > I see this is already merged. If this still matters:
> >
> > Tested-by: Julien Grall <jgrall@amazon.com>
>
> We can't retrospectively add the tag, but the testing itself
> is still important -- thanks for doing it.
>
> Q: is there any reason not to:
> (a) change the default to "nested" rather than "1"
> (b) make the virt board (for new virt machine versions) use
> "nested"?
>
> AIUI "nested" should be a superset of "stage-1 only", the guest
> can just ignore stage-2 if it doesn't care about it. Or is
> there a performance hit from having stage-2 around even if the
> guest doesn't enable it?
I didn’t do benchmarks, but from the code, I don’t think there
would be a difference from using stage-1 only or nested stages
with stage-1 config.
I didn’t make “nested” the default stage or used it for the virt
board, as I was worried about compatibility issues (I think that
breaks backward migration), but otherwise I don’t see issues.
But if I understand correctly, setting that for virt board 9.1
(virt_machine_9_1_options) would be fine?
Thanks,
Mostafa
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v5 00/18] SMMUv3 nested translation support
2024-07-20 22:11 ` Mostafa Saleh
@ 2024-07-22 9:35 ` Peter Maydell
0 siblings, 0 replies; 35+ messages in thread
From: Peter Maydell @ 2024-07-22 9:35 UTC (permalink / raw)
To: Mostafa Saleh
Cc: Julien Grall, eric.auger, Jean-Philippe Brucker, qemu-arm,
qemu-devel, alex.bennee, maz, nicolinc, richard.henderson,
marcin.juszkiewicz
On Sat, 20 Jul 2024 at 23:11, Mostafa Saleh <smostafa@google.com> wrote:
>
> Hi Peter,
>
> On Fri, Jul 19, 2024 at 04:57:18PM +0100, Peter Maydell wrote:
> > On Fri, 19 Jul 2024 at 16:36, Julien Grall <julien@xen.org> wrote:
> > >
> > > Hi,
> > >
> > > On 18/07/2024 10:43, Julien Grall wrote:
> > > > Hi Eric,
> > > >
> > > > On 17/07/2024 18:43, Eric Auger wrote:
> > > >> Hi Peter, Richard,
> > > >>
> > > >> On 7/17/24 17:09, Jean-Philippe Brucker wrote:
> > > >>> On Mon, Jul 15, 2024 at 08:45:00AM +0000, Mostafa Saleh wrote:
> > > >>>> Currently, QEMU supports emulating either stage-1 or stage-2 SMMUs
> > > >>>> but not nested instances.
> > > >>>> This patch series adds support for nested translation in SMMUv3,
> > > >>>> this is controlled by property “arm-smmuv3.stage=nested”, and
> > > >>>> advertised to guests as (IDR0.S1P == 1 && IDR0.S2P == 2)
> > > >>> For the whole series (3-9, 11, 12, 15, 16, 18):
> > > >>>
> > > >>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > >>>
> > > >>> (and I think patch 16 is missing Eric's R-b)
> > > >>
> > > >> Jean-Philippe and I have followed up the progress of this series,
> > > >> Mostafa took into account all our comments and all the patches were
> > > >> reviewed. It seems to be in a pretty decent state so if you don't have
> > > >> any objection, please consider pulling it for 9.1.
> > > >>
> > > >> On my end I did some testing in non nesting mode with virtio-net/vhost
> > > >> and I have not noticed any regression.
> > > >> Would be nice if someone could send his T-b for the nested part though
> > > >> (Julien?).
> > > >
> > > > I haven't yet tried the latest version. I will do that in the next
> > > > couple of days.
> > > I see this is already merged. If this still matters:
> > >
> > > Tested-by: Julien Grall <jgrall@amazon.com>
> >
> > We can't retrospectively add the tag, but the testing itself
> > is still important -- thanks for doing it.
> >
> > Q: is there any reason not to:
> > (a) change the default to "nested" rather than "1"
> > (b) make the virt board (for new virt machine versions) use
> > "nested"?
> >
> > AIUI "nested" should be a superset of "stage-1 only", the guest
> > can just ignore stage-2 if it doesn't care about it. Or is
> > there a performance hit from having stage-2 around even if the
> > guest doesn't enable it?
>
> I didn’t do benchmarks, but from the code, I don’t think there
> would be a difference from using stage-1 only or nested stages
> with stage-1 config.
> I didn’t make “nested” the default stage or used it for the virt
> board, as I was worried about compatibility issues (I think that
> breaks backward migration), but otherwise I don’t see issues.
>
> But if I understand correctly, setting that for virt board 9.1
> (virt_machine_9_1_options) would be fine?
Yes, we would need to retain the old stage-1-only config on
the older machine version types, but we could switch to
nested by default on the newer ones. It's a little late
I think to make that change for the virt board at this
point in the 9.1 release cycle, but we could do it for 9.2.
thanks
-- PMM
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2024-07-22 9:36 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-15 8:45 [PATCH v5 00/18] SMMUv3 nested translation support Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 01/18] hw/arm/smmu-common: Add missing size check for stage-1 Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 02/18] hw/arm/smmu: Fix IPA for stage-2 events Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 03/18] hw/arm/smmuv3: Fix encoding of CLASS in events Mostafa Saleh
2024-07-17 15:07 ` Eric Auger
2024-07-17 15:58 ` Jean-Philippe Brucker
2024-07-17 15:59 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 04/18] hw/arm/smmu: Use enum for SMMU stage Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 05/18] hw/arm/smmu: Split smmuv3_translate() Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 06/18] hw/arm/smmu: Consolidate ASID and VMID types Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 07/18] hw/arm/smmu: Introduce CACHED_ENTRY_TO_ADDR Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 08/18] hw/arm/smmuv3: Translate CD and TT using stage-2 table Mostafa Saleh
2024-07-17 15:18 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 09/18] hw/arm/smmu-common: Rework TLB lookup for nesting Mostafa Saleh
2024-07-17 15:28 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 10/18] hw/arm/smmu-common: Add support for nested TLB Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 11/18] hw/arm/smmu-common: Support nested translation Mostafa Saleh
2024-07-17 15:28 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 12/18] hw/arm/smmu: Support nesting in smmuv3_range_inval() Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 13/18] hw/arm/smmu: Introduce smmu_iotlb_inv_asid_vmid Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 14/18] hw/arm/smmu: Support nesting in the rest of commands Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 15/18] hw/arm/smmuv3: Support nested SMMUs in smmuv3_notify_iova() Mostafa Saleh
2024-07-17 15:30 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 16/18] hw/arm/smmuv3: Handle translation faults according to SMMUPTWEventInfo Mostafa Saleh
2024-07-17 15:31 ` Eric Auger
2024-07-15 8:45 ` [PATCH v5 17/18] hw/arm/smmuv3: Support and advertise nesting Mostafa Saleh
2024-07-15 8:45 ` [PATCH v5 18/18] hw/arm/smmu: Refactor SMMU OAS Mostafa Saleh
2024-07-17 15:09 ` [PATCH v5 00/18] SMMUv3 nested translation support Jean-Philippe Brucker
2024-07-17 17:43 ` Eric Auger
2024-07-17 19:06 ` Peter Maydell
2024-07-18 9:43 ` Julien Grall
2024-07-19 15:36 ` Julien Grall
2024-07-19 15:57 ` Peter Maydell
2024-07-20 22:11 ` Mostafa Saleh
2024-07-22 9:35 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).