* [PATCH v4 01/24] target/arm: Enable TARGET_PAGE_ENTRY_EXTRA
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 02/24] target/arm: Use probe_access_full for MTE Richard Henderson
` (23 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Copy attrs and shareability, into the TLB. This will eventually
be used by S1_ptw_translate to report stage1 translation failures,
and by do_ats_write to fill in PAR_EL1.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu-param.h | 12 ++++++++++++
target/arm/tlb_helper.c | 3 +++
2 files changed, 15 insertions(+)
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 08681828ac..38347b0d20 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -30,6 +30,18 @@
*/
# define TARGET_PAGE_BITS_VARY
# define TARGET_PAGE_BITS_MIN 10
+
+/*
+ * Cache the attrs and shareability fields from the page table entry.
+ *
+ * For ARMMMUIdx_Stage2*, pte_attrs is the S2 descriptor bits [5:2].
+ * Otherwise, pte_attrs is the same as the MAIR_EL1 8-bit format.
+ * For shareability, as in the SH field of the VMSAv8-64 PTEs.
+ */
+# define TARGET_PAGE_ENTRY_EXTRA \
+ uint8_t pte_attrs; \
+ uint8_t shareability;
+
#endif
#define NB_MMU_MODES 8
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 49601394ec..353edbeb1d 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -236,6 +236,9 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
arm_tlb_mte_tagged(&res.f.attrs) = true;
}
+ res.f.pte_attrs = res.cacheattrs.attrs;
+ res.f.shareability = res.cacheattrs.shareability;
+
tlb_set_page_full(cs, mmu_idx, address, &res.f);
return true;
} else if (probe) {
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 02/24] target/arm: Use probe_access_full for MTE
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
2022-10-11 3:18 ` [PATCH v4 01/24] target/arm: Enable TARGET_PAGE_ENTRY_EXTRA Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 03/24] target/arm: Use probe_access_full for BTI Richard Henderson
` (22 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
The CPUTLBEntryFull structure now stores the original pte attributes, as
well as the physical address. Therefore, we no longer need a separate
bit in MemTxAttrs, nor do we need to walk the tree of memory regions.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 1 -
target/arm/sve_ldst_internal.h | 1 +
target/arm/mte_helper.c | 62 ++++++++++------------------------
target/arm/sve_helper.c | 54 ++++++++++-------------------
target/arm/tlb_helper.c | 4 ---
5 files changed, 36 insertions(+), 86 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1a909a1b43..f09ec8aa03 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3400,7 +3400,6 @@ static inline MemTxAttrs *typecheck_memtxattrs(MemTxAttrs *x)
* generic target bits directly.
*/
#define arm_tlb_bti_gp(x) (typecheck_memtxattrs(x)->target_tlb_bit0)
-#define arm_tlb_mte_tagged(x) (typecheck_memtxattrs(x)->target_tlb_bit1)
/*
* AArch64 usage of the PAGE_TARGET_* bits for linux-user.
diff --git a/target/arm/sve_ldst_internal.h b/target/arm/sve_ldst_internal.h
index b5c473fc48..4f159ec4ad 100644
--- a/target/arm/sve_ldst_internal.h
+++ b/target/arm/sve_ldst_internal.h
@@ -134,6 +134,7 @@ typedef struct {
void *host;
int flags;
MemTxAttrs attrs;
+ bool tagged;
} SVEHostPage;
bool sve_probe_page(SVEHostPage *info, bool nofault, CPUARMState *env,
diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index fdd23ab3f8..e85208339e 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -105,10 +105,9 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
TARGET_PAGE_BITS - LOG2_TAG_GRANULE - 1);
return tags + index;
#else
- uintptr_t index;
CPUTLBEntryFull *full;
+ MemTxAttrs attrs;
int in_page, flags;
- ram_addr_t ptr_ra;
hwaddr ptr_paddr, tag_paddr, xlat;
MemoryRegion *mr;
ARMASIdx tag_asi;
@@ -124,30 +123,12 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
* valid. Indicate to probe_access_flags no-fault, then assert that
* we received a valid page.
*/
- flags = probe_access_flags(env, ptr, ptr_access, ptr_mmu_idx,
- ra == 0, &host, ra);
+ flags = probe_access_full(env, ptr, ptr_access, ptr_mmu_idx,
+ ra == 0, &host, &full, ra);
assert(!(flags & TLB_INVALID_MASK));
- /*
- * Find the CPUTLBEntryFull for ptr. This *must* be present in the TLB
- * because we just found the mapping.
- * TODO: Perhaps there should be a cputlb helper that returns a
- * matching tlb entry + iotlb entry.
- */
- index = tlb_index(env, ptr_mmu_idx, ptr);
-# ifdef CONFIG_DEBUG_TCG
- {
- CPUTLBEntry *entry = tlb_entry(env, ptr_mmu_idx, ptr);
- target_ulong comparator = (ptr_access == MMU_DATA_LOAD
- ? entry->addr_read
- : tlb_addr_write(entry));
- g_assert(tlb_hit(comparator, ptr));
- }
-# endif
- full = &env_tlb(env)->d[ptr_mmu_idx].fulltlb[index];
-
/* If the virtual page MemAttr != Tagged, access unchecked. */
- if (!arm_tlb_mte_tagged(&full->attrs)) {
+ if (full->pte_attrs != 0xf0) {
return NULL;
}
@@ -162,6 +143,14 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
return NULL;
}
+ /*
+ * Remember these values across the second lookup below,
+ * which may invalidate this pointer via tlb resize.
+ */
+ ptr_paddr = full->phys_addr;
+ attrs = full->attrs;
+ full = NULL;
+
/*
* The Normal memory access can extend to the next page. E.g. a single
* 8-byte access to the last byte of a page will check only the last
@@ -170,9 +159,8 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
*/
in_page = -(ptr | TARGET_PAGE_MASK);
if (unlikely(ptr_size > in_page)) {
- void *ignore;
- flags |= probe_access_flags(env, ptr + in_page, ptr_access,
- ptr_mmu_idx, ra == 0, &ignore, ra);
+ flags |= probe_access_full(env, ptr + in_page, ptr_access,
+ ptr_mmu_idx, ra == 0, &host, &full, ra);
assert(!(flags & TLB_INVALID_MASK));
}
@@ -180,33 +168,17 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
if (unlikely(flags & TLB_WATCHPOINT)) {
int wp = ptr_access == MMU_DATA_LOAD ? BP_MEM_READ : BP_MEM_WRITE;
assert(ra != 0);
- cpu_check_watchpoint(env_cpu(env), ptr, ptr_size,
- full->attrs, wp, ra);
+ cpu_check_watchpoint(env_cpu(env), ptr, ptr_size, attrs, wp, ra);
}
- /*
- * Find the physical address within the normal mem space.
- * The memory region lookup must succeed because TLB_MMIO was
- * not set in the cputlb lookup above.
- */
- mr = memory_region_from_host(host, &ptr_ra);
- tcg_debug_assert(mr != NULL);
- tcg_debug_assert(memory_region_is_ram(mr));
- ptr_paddr = ptr_ra;
- do {
- ptr_paddr += mr->addr;
- mr = mr->container;
- } while (mr);
-
/* Convert to the physical address in tag space. */
tag_paddr = ptr_paddr >> (LOG2_TAG_GRANULE + 1);
/* Look up the address in tag space. */
- tag_asi = full->attrs.secure ? ARMASIdx_TagS : ARMASIdx_TagNS;
+ tag_asi = attrs.secure ? ARMASIdx_TagS : ARMASIdx_TagNS;
tag_as = cpu_get_address_space(env_cpu(env), tag_asi);
mr = address_space_translate(tag_as, tag_paddr, &xlat, NULL,
- tag_access == MMU_DATA_STORE,
- full->attrs);
+ tag_access == MMU_DATA_STORE, attrs);
/*
* Note that @mr will never be NULL. If there is nothing in the address
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index 9cae8fd352..3d0d2987cd 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -5351,8 +5351,19 @@ bool sve_probe_page(SVEHostPage *info, bool nofault, CPUARMState *env,
*/
addr = useronly_clean_ptr(addr);
+#ifdef CONFIG_USER_ONLY
flags = probe_access_flags(env, addr, access_type, mmu_idx, nofault,
&info->host, retaddr);
+ memset(&info->attrs, 0, sizeof(info->attrs));
+ /* Require both ANON and MTE; see allocation_tag_mem(). */
+ info->tagged = (flags & PAGE_ANON) && (flags & PAGE_MTE);
+#else
+ CPUTLBEntryFull *full;
+ flags = probe_access_full(env, addr, access_type, mmu_idx, nofault,
+ &info->host, &full, retaddr);
+ info->attrs = full->attrs;
+ info->tagged = full->pte_attrs == 0xf0;
+#endif
info->flags = flags;
if (flags & TLB_INVALID_MASK) {
@@ -5362,33 +5373,6 @@ bool sve_probe_page(SVEHostPage *info, bool nofault, CPUARMState *env,
/* Ensure that info->host[] is relative to addr, not addr + mem_off. */
info->host -= mem_off;
-
-#ifdef CONFIG_USER_ONLY
- memset(&info->attrs, 0, sizeof(info->attrs));
- /* Require both MAP_ANON and PROT_MTE -- see allocation_tag_mem. */
- arm_tlb_mte_tagged(&info->attrs) =
- (flags & PAGE_ANON) && (flags & PAGE_MTE);
-#else
- /*
- * Find the iotlbentry for addr and return the transaction attributes.
- * This *must* be present in the TLB because we just found the mapping.
- */
- {
- uintptr_t index = tlb_index(env, mmu_idx, addr);
-
-# ifdef CONFIG_DEBUG_TCG
- CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
- target_ulong comparator = (access_type == MMU_DATA_LOAD
- ? entry->addr_read
- : tlb_addr_write(entry));
- g_assert(tlb_hit(comparator, addr));
-# endif
-
- CPUTLBEntryFull *full = &env_tlb(env)->d[mmu_idx].fulltlb[index];
- info->attrs = full->attrs;
- }
-#endif
-
return true;
}
@@ -5617,7 +5601,7 @@ void sve_cont_ldst_mte_check(SVEContLdSt *info, CPUARMState *env,
intptr_t mem_off, reg_off, reg_last;
/* Process the page only if MemAttr == Tagged. */
- if (arm_tlb_mte_tagged(&info->page[0].attrs)) {
+ if (info->page[0].tagged) {
mem_off = info->mem_off_first[0];
reg_off = info->reg_off_first[0];
reg_last = info->reg_off_split;
@@ -5638,7 +5622,7 @@ void sve_cont_ldst_mte_check(SVEContLdSt *info, CPUARMState *env,
}
mem_off = info->mem_off_first[1];
- if (mem_off >= 0 && arm_tlb_mte_tagged(&info->page[1].attrs)) {
+ if (mem_off >= 0 && info->page[1].tagged) {
reg_off = info->reg_off_first[1];
reg_last = info->reg_off_last[1];
@@ -6017,7 +6001,7 @@ void sve_ldnfff1_r(CPUARMState *env, void *vg, const target_ulong addr,
* Disable MTE checking if the Tagged bit is not set. Since TBI must
* be set within MTEDESC for MTE, !mtedesc => !mte_active.
*/
- if (!arm_tlb_mte_tagged(&info.page[0].attrs)) {
+ if (!info.page[0].tagged) {
mtedesc = 0;
}
@@ -6568,7 +6552,7 @@ void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
cpu_check_watchpoint(env_cpu(env), addr, msize,
info.attrs, BP_MEM_READ, retaddr);
}
- if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
+ if (mtedesc && info.tagged) {
mte_check(env, mtedesc, addr, retaddr);
}
if (unlikely(info.flags & TLB_MMIO)) {
@@ -6585,7 +6569,7 @@ void sve_ld1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
msize, info.attrs,
BP_MEM_READ, retaddr);
}
- if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
+ if (mtedesc && info.tagged) {
mte_check(env, mtedesc, addr, retaddr);
}
tlb_fn(env, &scratch, reg_off, addr, retaddr);
@@ -6786,9 +6770,7 @@ void sve_ldff1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
(env_cpu(env), addr, msize) & BP_MEM_READ)) {
goto fault;
}
- if (mtedesc &&
- arm_tlb_mte_tagged(&info.attrs) &&
- !mte_probe(env, mtedesc, addr)) {
+ if (mtedesc && info.tagged && !mte_probe(env, mtedesc, addr)) {
goto fault;
}
@@ -6974,7 +6956,7 @@ void sve_st1_z(CPUARMState *env, void *vd, uint64_t *vg, void *vm,
info.attrs, BP_MEM_WRITE, retaddr);
}
- if (mtedesc && arm_tlb_mte_tagged(&info.attrs)) {
+ if (mtedesc && info.tagged) {
mte_check(env, mtedesc, addr, retaddr);
}
}
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 353edbeb1d..3462a6ea14 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -231,10 +231,6 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
res.f.phys_addr &= TARGET_PAGE_MASK;
address &= TARGET_PAGE_MASK;
}
- /* Notice and record tagged memory. */
- if (cpu_isar_feature(aa64_mte, cpu) && res.cacheattrs.attrs == 0xf0) {
- arm_tlb_mte_tagged(&res.f.attrs) = true;
- }
res.f.pte_attrs = res.cacheattrs.attrs;
res.f.shareability = res.cacheattrs.shareability;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 03/24] target/arm: Use probe_access_full for BTI
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
2022-10-11 3:18 ` [PATCH v4 01/24] target/arm: Enable TARGET_PAGE_ENTRY_EXTRA Richard Henderson
2022-10-11 3:18 ` [PATCH v4 02/24] target/arm: Use probe_access_full for MTE Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 04/24] target/arm: Add ARMMMUIdx_Phys_{S,NS} Richard Henderson
` (21 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Add a field to TARGET_PAGE_ENTRY_EXTRA to hold the guarded bit.
In is_guarded_page, use probe_access_full instead of just guessing
that the tlb entry is still present. Also handles the FIXME about
executing from device memory.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu-param.h | 9 +++++----
target/arm/cpu.h | 13 -------------
target/arm/internals.h | 1 +
target/arm/ptw.c | 7 ++++---
target/arm/translate-a64.c | 21 ++++++++++-----------
5 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 38347b0d20..f4338fd10e 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -36,12 +36,13 @@
*
* For ARMMMUIdx_Stage2*, pte_attrs is the S2 descriptor bits [5:2].
* Otherwise, pte_attrs is the same as the MAIR_EL1 8-bit format.
- * For shareability, as in the SH field of the VMSAv8-64 PTEs.
+ * For shareability and guarded, as in the SH and GP fields respectively
+ * of the VMSAv8-64 PTEs.
*/
# define TARGET_PAGE_ENTRY_EXTRA \
- uint8_t pte_attrs; \
- uint8_t shareability;
-
+ uint8_t pte_attrs; \
+ uint8_t shareability; \
+ bool guarded;
#endif
#define NB_MMU_MODES 8
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f09ec8aa03..a34d496c5b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3388,19 +3388,6 @@ static inline uint64_t *aa64_vfp_qreg(CPUARMState *env, unsigned regno)
/* Shared between translate-sve.c and sve_helper.c. */
extern const uint64_t pred_esz_masks[5];
-/* Helper for the macros below, validating the argument type. */
-static inline MemTxAttrs *typecheck_memtxattrs(MemTxAttrs *x)
-{
- return x;
-}
-
-/*
- * Lvalue macros for ARM TLB bits that we must cache in the TCG TLB.
- * Using these should be a bit more self-documenting than using the
- * generic target bits directly.
- */
-#define arm_tlb_bti_gp(x) (typecheck_memtxattrs(x)->target_tlb_bit0)
-
/*
* AArch64 usage of the PAGE_TARGET_* bits for linux-user.
* Note that with the Linux kernel, PROT_MTE may not be cleared by mprotect
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 9566364dca..c3c3920ded 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1095,6 +1095,7 @@ typedef struct ARMCacheAttrs {
unsigned int attrs:8;
unsigned int shareability:2; /* as in the SH field of the VMSAv8-64 PTEs */
bool is_s2_format:1;
+ bool guarded:1; /* guarded bit of the v8-64 PTE */
} ARMCacheAttrs;
/* Fields that are valid upon success. */
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 23f16f4ff7..2d182d62e5 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1313,9 +1313,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
*/
result->f.attrs.secure = false;
}
- /* When in aarch64 mode, and BTI is enabled, remember GP in the IOTLB. */
- if (aarch64 && guarded && cpu_isar_feature(aa64_bti, cpu)) {
- arm_tlb_bti_gp(&result->f.attrs) = true;
+
+ /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
+ if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
+ result->f.guarded = guarded;
}
if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 5b67375f4e..60ff753d81 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14601,22 +14601,21 @@ static bool is_guarded_page(CPUARMState *env, DisasContext *s)
#ifdef CONFIG_USER_ONLY
return page_get_flags(addr) & PAGE_BTI;
#else
+ CPUTLBEntryFull *full;
+ void *host;
int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx);
- unsigned int index = tlb_index(env, mmu_idx, addr);
- CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
+ int flags;
/*
* We test this immediately after reading an insn, which means
- * that any normal page must be in the TLB. The only exception
- * would be for executing from flash or device memory, which
- * does not retain the TLB entry.
- *
- * FIXME: Assume false for those, for now. We could use
- * arm_cpu_get_phys_page_attrs_debug to re-read the page
- * table entry even for that case.
+ * that the TLB entry must be present and valid, and thus this
+ * access will never raise an exception.
*/
- return (tlb_hit(entry->addr_code, addr) &&
- arm_tlb_bti_gp(&env_tlb(env)->d[mmu_idx].fulltlb[index].attrs));
+ flags = probe_access_full(env, addr, MMU_INST_FETCH, mmu_idx,
+ false, &host, &full, 0);
+ assert(!(flags & TLB_INVALID_MASK));
+
+ return full->guarded;
#endif
}
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 04/24] target/arm: Add ARMMMUIdx_Phys_{S,NS}
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (2 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 03/24] target/arm: Use probe_access_full for BTI Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 05/24] target/arm: Move ARMMMUIdx_Stage2 to a real tlb mmu_idx Richard Henderson
` (20 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Not yet used, but add mmu indexes for 1-1 mapping
to physical addresses.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu-param.h | 2 +-
target/arm/cpu.h | 7 ++++++-
target/arm/ptw.c | 19 +++++++++++++++++--
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index f4338fd10e..a5b27db275 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -45,6 +45,6 @@
bool guarded;
#endif
-#define NB_MMU_MODES 8
+#define NB_MMU_MODES 10
#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a34d496c5b..f93060e6d6 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2905,8 +2905,9 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
* EL2 EL2&0 +PAN
* EL2 (aka NS PL2)
* EL3 (aka S PL1)
+ * Physical (NS & S)
*
- * for a total of 8 different mmu_idx.
+ * for a total of 10 different mmu_idx.
*
* R profile CPUs have an MPU, but can use the same set of MMU indexes
* as A profile. They only need to distinguish EL0 and EL1 (and
@@ -2971,6 +2972,10 @@ typedef enum ARMMMUIdx {
ARMMMUIdx_E2 = 6 | ARM_MMU_IDX_A,
ARMMMUIdx_E3 = 7 | ARM_MMU_IDX_A,
+ /* TLBs with 1-1 mapping to the physical address spaces. */
+ ARMMMUIdx_Phys_NS = 8 | ARM_MMU_IDX_A,
+ ARMMMUIdx_Phys_S = 9 | ARM_MMU_IDX_A,
+
/*
* These are not allocated TLBs and are used only for AT system
* instructions or for the first stage of an S12 page table walk.
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 2d182d62e5..a977d09c6d 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -179,6 +179,11 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx,
case ARMMMUIdx_E3:
break;
+ case ARMMMUIdx_Phys_NS:
+ case ARMMMUIdx_Phys_S:
+ /* No translation for physical address spaces. */
+ return true;
+
default:
g_assert_not_reached();
}
@@ -2280,10 +2285,17 @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address,
{
uint8_t memattr = 0x00; /* Device nGnRnE */
uint8_t shareability = 0; /* non-sharable */
+ int r_el;
- if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) {
- int r_el = regime_el(env, mmu_idx);
+ switch (mmu_idx) {
+ case ARMMMUIdx_Stage2:
+ case ARMMMUIdx_Stage2_S:
+ case ARMMMUIdx_Phys_NS:
+ case ARMMMUIdx_Phys_S:
+ break;
+ default:
+ r_el = regime_el(env, mmu_idx);
if (arm_el_is_aa64(env, r_el)) {
int pamax = arm_pamax(env_archcpu(env));
uint64_t tcr = env->cp15.tcr_el[r_el];
@@ -2332,6 +2344,7 @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address,
shareability = 2; /* outer sharable */
}
result->cacheattrs.is_s2_format = false;
+ break;
}
result->f.phys_addr = address;
@@ -2536,6 +2549,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
is_secure = arm_is_secure_below_el3(env);
break;
case ARMMMUIdx_Stage2:
+ case ARMMMUIdx_Phys_NS:
case ARMMMUIdx_MPrivNegPri:
case ARMMMUIdx_MUserNegPri:
case ARMMMUIdx_MPriv:
@@ -2544,6 +2558,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
break;
case ARMMMUIdx_E3:
case ARMMMUIdx_Stage2_S:
+ case ARMMMUIdx_Phys_S:
case ARMMMUIdx_MSPrivNegPri:
case ARMMMUIdx_MSUserNegPri:
case ARMMMUIdx_MSPriv:
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 05/24] target/arm: Move ARMMMUIdx_Stage2 to a real tlb mmu_idx
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (3 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 04/24] target/arm: Add ARMMMUIdx_Phys_{S,NS} Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-14 18:09 ` Peter Maydell
2022-10-11 3:18 ` [PATCH v4 06/24] target/arm: Restrict tlb flush from vttbr_write to vmid change Richard Henderson
` (19 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
We had been marking this ARM_MMU_IDX_NOTLB, move it to a real tlb.
Flush the tlb when invalidating stage 1+2 translations. Re-use
alle1_tlbmask() for other instances of EL1&0 + Stage2.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v4: Implement the IPAS2 and RIPAS2 tlb flushing insns;
Reuse alle1_tlbmask to fix aa32 and vttbr flushing.
---
target/arm/cpu-param.h | 2 +-
target/arm/cpu.h | 23 ++++---
target/arm/helper.c | 151 ++++++++++++++++++++++++++++++-----------
3 files changed, 127 insertions(+), 49 deletions(-)
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index a5b27db275..b7bde18986 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -45,6 +45,6 @@
bool guarded;
#endif
-#define NB_MMU_MODES 10
+#define NB_MMU_MODES 12
#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f93060e6d6..c94e289012 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2906,8 +2906,9 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
* EL2 (aka NS PL2)
* EL3 (aka S PL1)
* Physical (NS & S)
+ * Stage2 (NS & S)
*
- * for a total of 10 different mmu_idx.
+ * for a total of 12 different mmu_idx.
*
* R profile CPUs have an MPU, but can use the same set of MMU indexes
* as A profile. They only need to distinguish EL0 and EL1 (and
@@ -2976,6 +2977,15 @@ typedef enum ARMMMUIdx {
ARMMMUIdx_Phys_NS = 8 | ARM_MMU_IDX_A,
ARMMMUIdx_Phys_S = 9 | ARM_MMU_IDX_A,
+ /*
+ * Used for second stage of an S12 page table walk, or for descriptor
+ * loads during first stage of an S1 page table walk. Note that both
+ * are in use simultaneously for SecureEL2: the security state for
+ * the S2 ptw is selected by the NS bit from the S1 ptw.
+ */
+ ARMMMUIdx_Stage2 = 10 | ARM_MMU_IDX_A,
+ ARMMMUIdx_Stage2_S = 11 | ARM_MMU_IDX_A,
+
/*
* These are not allocated TLBs and are used only for AT system
* instructions or for the first stage of an S12 page table walk.
@@ -2983,15 +2993,6 @@ typedef enum ARMMMUIdx {
ARMMMUIdx_Stage1_E0 = 0 | ARM_MMU_IDX_NOTLB,
ARMMMUIdx_Stage1_E1 = 1 | ARM_MMU_IDX_NOTLB,
ARMMMUIdx_Stage1_E1_PAN = 2 | ARM_MMU_IDX_NOTLB,
- /*
- * Not allocated a TLB: used only for second stage of an S12 page
- * table walk, or for descriptor loads during first stage of an S1
- * page table walk. Note that if we ever want to have a TLB for this
- * then various TLB flush insns which currently are no-ops or flush
- * only stage 1 MMU indexes will need to change to flush stage 2.
- */
- ARMMMUIdx_Stage2 = 3 | ARM_MMU_IDX_NOTLB,
- ARMMMUIdx_Stage2_S = 4 | ARM_MMU_IDX_NOTLB,
/*
* M-profile.
@@ -3022,6 +3023,8 @@ typedef enum ARMMMUIdxBit {
TO_CORE_BIT(E20_2),
TO_CORE_BIT(E20_2_PAN),
TO_CORE_BIT(E3),
+ TO_CORE_BIT(Stage2),
+ TO_CORE_BIT(Stage2_S),
TO_CORE_BIT(MUser),
TO_CORE_BIT(MPriv),
diff --git a/target/arm/helper.c b/target/arm/helper.c
index dde64a487a..18c51bb777 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -399,6 +399,21 @@ static void contextidr_write(CPUARMState *env, const ARMCPRegInfo *ri,
raw_write(env, ri, value);
}
+static int alle1_tlbmask(CPUARMState *env)
+{
+ /*
+ * Note that the 'ALL' scope must invalidate both stage 1 and
+ * stage 2 translations, whereas most other scopes only invalidate
+ * stage 1 translations.
+ */
+ return (ARMMMUIdxBit_E10_1 |
+ ARMMMUIdxBit_E10_1_PAN |
+ ARMMMUIdxBit_E10_0 |
+ ARMMMUIdxBit_Stage2 |
+ ARMMMUIdxBit_Stage2_S);
+}
+
+
/* IS variants of TLB operations must affect all cores */
static void tlbiall_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
@@ -501,10 +516,7 @@ static void tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = env_cpu(env);
- tlb_flush_by_mmuidx(cs,
- ARMMMUIdxBit_E10_1 |
- ARMMMUIdxBit_E10_1_PAN |
- ARMMMUIdxBit_E10_0);
+ tlb_flush_by_mmuidx(cs, alle1_tlbmask(env));
}
static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -512,10 +524,7 @@ static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
{
CPUState *cs = env_cpu(env);
- tlb_flush_by_mmuidx_all_cpus_synced(cs,
- ARMMMUIdxBit_E10_1 |
- ARMMMUIdxBit_E10_1_PAN |
- ARMMMUIdxBit_E10_0);
+ tlb_flush_by_mmuidx_all_cpus_synced(cs, alle1_tlbmask(env));
}
@@ -554,6 +563,24 @@ static void tlbimva_hyp_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
ARMMMUIdxBit_E2);
}
+static void tlbiipas2_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ CPUState *cs = env_cpu(env);
+ uint64_t pageaddr = (value & MAKE_64BIT_MASK(0, 28)) << 12;
+
+ tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_Stage2);
+}
+
+static void tlbiipas2is_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ CPUState *cs = env_cpu(env);
+ uint64_t pageaddr = (value & MAKE_64BIT_MASK(0, 28)) << 12;
+
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, ARMMMUIdxBit_Stage2);
+}
+
static const ARMCPRegInfo cp_reginfo[] = {
/* Define the secure and non-secure FCSE identifier CP registers
* separately because there is no secure bank in V8 (no _EL3). This allows
@@ -3786,13 +3813,10 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
/*
* A change in VMID to the stage2 page table (Stage2) invalidates
- * the combined stage 1&2 tlbs (EL10_1 and EL10_0).
+ * the stage2 and combined stage 1&2 tlbs (EL10_1 and EL10_0).
*/
if (raw_read(env, ri) != value) {
- uint16_t mask = ARMMMUIdxBit_E10_1 |
- ARMMMUIdxBit_E10_1_PAN |
- ARMMMUIdxBit_E10_0;
- tlb_flush_by_mmuidx(cs, mask);
+ tlb_flush_by_mmuidx(cs, alle1_tlbmask(env));
raw_write(env, ri, value);
}
}
@@ -4313,18 +4337,6 @@ static void tlbi_aa64_vmalle1_write(CPUARMState *env, const ARMCPRegInfo *ri,
}
}
-static int alle1_tlbmask(CPUARMState *env)
-{
- /*
- * Note that the 'ALL' scope must invalidate both stage 1 and
- * stage 2 translations, whereas most other scopes only invalidate
- * stage 1 translations.
- */
- return (ARMMMUIdxBit_E10_1 |
- ARMMMUIdxBit_E10_1_PAN |
- ARMMMUIdxBit_E10_0);
-}
-
static int e2_tlbmask(CPUARMState *env)
{
return (ARMMMUIdxBit_E20_0 |
@@ -4467,6 +4479,43 @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri,
ARMMMUIdxBit_E3, bits);
}
+static int ipas2e1_tlbmask(CPUARMState *env, int64_t value)
+{
+ /*
+ * The MSB of value is the NS field, which only applies if SEL2
+ * is implemented and SCR_EL3.NS is not set (i.e. in secure mode).
+ */
+ return (value >= 0
+ && cpu_isar_feature(aa64_sel2, env_archcpu(env))
+ && arm_is_secure_below_el3(env)
+ ? ARMMMUIdxBit_Stage2_S
+ : ARMMMUIdxBit_Stage2);
+}
+
+static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ CPUState *cs = env_cpu(env);
+ int mask = ipas2e1_tlbmask(env, value);
+ uint64_t pageaddr = sextract64(value << 12, 0, 56);
+
+ if (tlb_force_broadcast(env)) {
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, mask);
+ } else {
+ tlb_flush_page_by_mmuidx(cs, pageaddr, mask);
+ }
+}
+
+static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ CPUState *cs = env_cpu(env);
+ int mask = ipas2e1_tlbmask(env, value);
+ uint64_t pageaddr = sextract64(value << 12, 0, 56);
+
+ tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, mask);
+}
+
#ifdef TARGET_AARCH64
typedef struct {
uint64_t base;
@@ -4652,6 +4701,20 @@ static void tlbi_aa64_rvae3is_write(CPUARMState *env,
do_rvae_write(env, value, ARMMMUIdxBit_E3, true);
}
+
+static void tlbi_aa64_ripas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ do_rvae_write(env, value, ipas2e1_tlbmask(env, value),
+ tlb_force_broadcast(env));
+}
+
+static void tlbi_aa64_ripas2e1is_write(CPUARMState *env,
+ const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+ do_rvae_write(env, value, ipas2e1_tlbmask(env, value), true);
+}
#endif
static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -4930,10 +4993,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbi_aa64_vae1_write },
{ .name = "TLBI_IPAS2E1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1,
- .access = PL2_W, .type = ARM_CP_NOP },
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ipas2e1is_write },
{ .name = "TLBI_IPAS2LE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5,
- .access = PL2_W, .type = ARM_CP_NOP },
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ipas2e1is_write },
{ .name = "TLBI_ALLE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 4,
.access = PL2_W, .type = ARM_CP_NO_RAW,
@@ -4944,10 +5009,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbi_aa64_alle1is_write },
{ .name = "TLBI_IPAS2E1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1,
- .access = PL2_W, .type = ARM_CP_NOP },
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ipas2e1_write },
{ .name = "TLBI_IPAS2LE1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 5,
- .access = PL2_W, .type = ARM_CP_NOP },
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ipas2e1_write },
{ .name = "TLBI_ALLE1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 7, .opc2 = 4,
.access = PL2_W, .type = ARM_CP_NO_RAW,
@@ -5028,16 +5095,20 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.writefn = tlbimva_hyp_is_write },
{ .name = "TLBIIPAS2",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1,
- .type = ARM_CP_NOP, .access = PL2_W },
+ .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .writefn = tlbiipas2_hyp_write },
{ .name = "TLBIIPAS2IS",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1,
- .type = ARM_CP_NOP, .access = PL2_W },
+ .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .writefn = tlbiipas2is_hyp_write },
{ .name = "TLBIIPAS2L",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 5,
- .type = ARM_CP_NOP, .access = PL2_W },
+ .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .writefn = tlbiipas2_hyp_write },
{ .name = "TLBIIPAS2LIS",
.cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5,
- .type = ARM_CP_NOP, .access = PL2_W },
+ .type = ARM_CP_NO_RAW, .access = PL2_W,
+ .writefn = tlbiipas2is_hyp_write },
/* 32 bit cache operations */
{ .name = "ICIALLUIS", .cp = 15, .opc1 = 0, .crn = 7, .crm = 1, .opc2 = 0,
.type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_pou_access },
@@ -6694,10 +6765,12 @@ static const ARMCPRegInfo tlbirange_reginfo[] = {
.writefn = tlbi_aa64_rvae1_write },
{ .name = "TLBI_RIPAS2E1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 2,
- .access = PL2_W, .type = ARM_CP_NOP },
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ripas2e1is_write },
{ .name = "TLBI_RIPAS2LE1IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 6,
- .access = PL2_W, .type = ARM_CP_NOP },
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ripas2e1is_write },
{ .name = "TLBI_RVAE2IS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 2, .opc2 = 1,
.access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EL3_NO_EL2_UNDEF,
@@ -6708,10 +6781,12 @@ static const ARMCPRegInfo tlbirange_reginfo[] = {
.writefn = tlbi_aa64_rvae2is_write },
{ .name = "TLBI_RIPAS2E1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 2,
- .access = PL2_W, .type = ARM_CP_NOP },
- { .name = "TLBI_RIPAS2LE1", .state = ARM_CP_STATE_AA64,
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ripas2e1_write },
+ { .name = "TLBI_RIPAS2LE1", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 6,
- .access = PL2_W, .type = ARM_CP_NOP },
+ .access = PL2_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_ripas2e1_write },
{ .name = "TLBI_RVAE2OS", .state = ARM_CP_STATE_AA64,
.opc0 = 1, .opc1 = 4, .crn = 8, .crm = 5, .opc2 = 1,
.access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_EL3_NO_EL2_UNDEF,
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 06/24] target/arm: Restrict tlb flush from vttbr_write to vmid change
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (4 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 05/24] target/arm: Move ARMMMUIdx_Stage2 to a real tlb mmu_idx Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-14 18:12 ` Peter Maydell
2022-10-11 3:18 ` [PATCH v4 07/24] target/arm: Split out S1Translate type Richard Henderson
` (18 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Compare only the VMID field when considering whether we need to flush.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 18c51bb777..c672903f43 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3815,10 +3815,10 @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
* A change in VMID to the stage2 page table (Stage2) invalidates
* the stage2 and combined stage 1&2 tlbs (EL10_1 and EL10_0).
*/
- if (raw_read(env, ri) != value) {
+ if (extract64(raw_read(env, ri) ^ value, 48, 16) != 0) {
tlb_flush_by_mmuidx(cs, alle1_tlbmask(env));
- raw_write(env, ri, value);
}
+ raw_write(env, ri, value);
}
static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 07/24] target/arm: Split out S1Translate type
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (5 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 06/24] target/arm: Restrict tlb flush from vttbr_write to vmid change Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-17 9:53 ` Peter Maydell
2022-10-11 3:18 ` [PATCH v4 08/24] target/arm: Plumb debug into S1Translate Richard Henderson
` (17 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Consolidate most of the inputs and outputs of S1_ptw_translate
into a single structure. Plumb this through arm_ld*_ptw from
the controlling get_phys_addr_* routine.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v4: Replaces a different S1TranslateResult patch, and plumbs the
structure further out in the function call tree.
---
target/arm/ptw.c | 140 ++++++++++++++++++++++++++---------------------
1 file changed, 79 insertions(+), 61 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index a977d09c6d..dee69ee743 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -14,9 +14,16 @@
#include "idau.h"
-static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
- MMUAccessType access_type, ARMMMUIdx mmu_idx,
- bool is_secure, bool s1_is_el0,
+typedef struct S1Translate {
+ ARMMMUIdx in_mmu_idx;
+ bool in_secure;
+ bool out_secure;
+ hwaddr out_phys;
+} S1Translate;
+
+static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
+ uint64_t address,
+ MMUAccessType access_type, bool s1_is_el0,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
__attribute__((nonnull));
@@ -211,28 +218,31 @@ static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs)
}
/* Translate a S1 pagetable walk through S2 if needed. */
-static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
- hwaddr addr, bool *is_secure_ptr,
- ARMMMUFaultInfo *fi)
+static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
+ hwaddr addr, ARMMMUFaultInfo *fi)
{
- bool is_secure = *is_secure_ptr;
+ bool is_secure = ptw->in_secure;
ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
- if (arm_mmu_idx_is_stage1_of_2(mmu_idx) &&
+ if (arm_mmu_idx_is_stage1_of_2(ptw->in_mmu_idx) &&
!regime_translation_disabled(env, s2_mmu_idx, is_secure)) {
GetPhysAddrResult s2 = {};
+ S1Translate s2ptw = {
+ .in_mmu_idx = s2_mmu_idx,
+ .in_secure = is_secure,
+ };
uint64_t hcr;
int ret;
- ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, s2_mmu_idx,
- is_secure, false, &s2, fi);
+ ret = get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
+ false, &s2, fi);
if (ret) {
assert(fi->type != ARMFault_None);
fi->s2addr = addr;
fi->stage2 = true;
fi->s1ptw = true;
fi->s1ns = !is_secure;
- return ~0;
+ return false;
}
hcr = arm_hcr_el2_eff_secstate(env, is_secure);
@@ -246,7 +256,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
fi->stage2 = true;
fi->s1ptw = true;
fi->s1ns = !is_secure;
- return ~0;
+ return false;
}
if (arm_is_secure_below_el3(env)) {
@@ -256,19 +266,21 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
} else {
is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
}
- *is_secure_ptr = is_secure;
} else {
assert(!is_secure);
}
addr = s2.f.phys_addr;
}
- return addr;
+
+ ptw->out_secure = is_secure;
+ ptw->out_phys = addr;
+ return true;
}
/* All loads done in the course of a page table walk go through here. */
-static uint32_t arm_ldl_ptw(CPUARMState *env, hwaddr addr, bool is_secure,
- ARMMMUIdx mmu_idx, ARMMMUFaultInfo *fi)
+static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
+ ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
MemTxAttrs attrs = {};
@@ -276,13 +288,13 @@ static uint32_t arm_ldl_ptw(CPUARMState *env, hwaddr addr, bool is_secure,
AddressSpace *as;
uint32_t data;
- addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi);
- attrs.secure = is_secure;
- as = arm_addressspace(cs, attrs);
- if (fi->s1ptw) {
+ if (!S1_ptw_translate(env, ptw, addr, fi)) {
return 0;
}
- if (regime_translation_big_endian(env, mmu_idx)) {
+ addr = ptw->out_phys;
+ attrs.secure = ptw->out_secure;
+ as = arm_addressspace(cs, attrs);
+ if (regime_translation_big_endian(env, ptw->in_mmu_idx)) {
data = address_space_ldl_be(as, addr, attrs, &result);
} else {
data = address_space_ldl_le(as, addr, attrs, &result);
@@ -295,8 +307,8 @@ static uint32_t arm_ldl_ptw(CPUARMState *env, hwaddr addr, bool is_secure,
return 0;
}
-static uint64_t arm_ldq_ptw(CPUARMState *env, hwaddr addr, bool is_secure,
- ARMMMUIdx mmu_idx, ARMMMUFaultInfo *fi)
+static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
+ ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
MemTxAttrs attrs = {};
@@ -304,13 +316,13 @@ static uint64_t arm_ldq_ptw(CPUARMState *env, hwaddr addr, bool is_secure,
AddressSpace *as;
uint64_t data;
- addr = S1_ptw_translate(env, mmu_idx, addr, &is_secure, fi);
- attrs.secure = is_secure;
- as = arm_addressspace(cs, attrs);
- if (fi->s1ptw) {
+ if (!S1_ptw_translate(env, ptw, addr, fi)) {
return 0;
}
- if (regime_translation_big_endian(env, mmu_idx)) {
+ addr = ptw->out_phys;
+ attrs.secure = ptw->out_secure;
+ as = arm_addressspace(cs, attrs);
+ if (regime_translation_big_endian(env, ptw->in_mmu_idx)) {
data = address_space_ldq_be(as, addr, attrs, &result);
} else {
data = address_space_ldq_le(as, addr, attrs, &result);
@@ -431,10 +443,9 @@ static int simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx, int ap)
return simple_ap_to_rw_prot_is_user(ap, regime_is_user(env, mmu_idx));
}
-static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,
- MMUAccessType access_type, ARMMMUIdx mmu_idx,
- bool is_secure, GetPhysAddrResult *result,
- ARMMMUFaultInfo *fi)
+static bool get_phys_addr_v5(CPUARMState *env, S1Translate *ptw,
+ uint32_t address, MMUAccessType access_type,
+ GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
{
int level = 1;
uint32_t table;
@@ -448,18 +459,18 @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,
/* Pagetable walk. */
/* Lookup l1 descriptor. */
- if (!get_level1_table_address(env, mmu_idx, &table, address)) {
+ if (!get_level1_table_address(env, ptw->in_mmu_idx, &table, address)) {
/* Section translation fault if page walk is disabled by PD0 or PD1 */
fi->type = ARMFault_Translation;
goto do_fault;
}
- desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi);
+ desc = arm_ldl_ptw(env, ptw, table, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
type = (desc & 3);
domain = (desc >> 5) & 0x0f;
- if (regime_el(env, mmu_idx) == 1) {
+ if (regime_el(env, ptw->in_mmu_idx) == 1) {
dacr = env->cp15.dacr_ns;
} else {
dacr = env->cp15.dacr_s;
@@ -491,7 +502,7 @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,
/* Fine pagetable. */
table = (desc & 0xfffff000) | ((address >> 8) & 0xffc);
}
- desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi);
+ desc = arm_ldl_ptw(env, ptw, table, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -535,7 +546,7 @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,
g_assert_not_reached();
}
}
- result->f.prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot);
+ result->f.prot = ap_to_rw_prot(env, ptw->in_mmu_idx, ap, domain_prot);
result->f.prot |= result->f.prot ? PAGE_EXEC : 0;
if (!(result->f.prot & (1 << access_type))) {
/* Access permission fault. */
@@ -550,12 +561,12 @@ do_fault:
return true;
}
-static bool get_phys_addr_v6(CPUARMState *env, uint32_t address,
- MMUAccessType access_type, ARMMMUIdx mmu_idx,
- bool is_secure, GetPhysAddrResult *result,
- ARMMMUFaultInfo *fi)
+static bool get_phys_addr_v6(CPUARMState *env, S1Translate *ptw,
+ uint32_t address, MMUAccessType access_type,
+ GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
{
ARMCPU *cpu = env_archcpu(env);
+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
int level = 1;
uint32_t table;
uint32_t desc;
@@ -576,7 +587,7 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address,
fi->type = ARMFault_Translation;
goto do_fault;
}
- desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi);
+ desc = arm_ldl_ptw(env, ptw, table, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -629,7 +640,7 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address,
ns = extract32(desc, 3, 1);
/* Lookup l2 entry. */
table = (desc & 0xfffffc00) | ((address >> 10) & 0x3fc);
- desc = arm_ldl_ptw(env, table, is_secure, mmu_idx, fi);
+ desc = arm_ldl_ptw(env, ptw, table, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -972,22 +983,25 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level,
* the WnR bit is never set (the caller must do this).
*
* @env: CPUARMState
+ * @ptw: Current and next stage parameters for the walk.
* @address: virtual address to get physical address for
* @access_type: MMU_DATA_LOAD, MMU_DATA_STORE or MMU_INST_FETCH
- * @mmu_idx: MMU index indicating required translation regime
- * @s1_is_el0: if @mmu_idx is ARMMMUIdx_Stage2 (so this is a stage 2 page
- * table walk), must be true if this is stage 2 of a stage 1+2
+ * @s1_is_el0: if @ptw->in_mmu_idx is ARMMMUIdx_Stage2
+ * (so this is a stage 2 page table walk),
+ * must be true if this is stage 2 of a stage 1+2
* walk for an EL0 access. If @mmu_idx is anything else,
* @s1_is_el0 is ignored.
* @result: set on translation success,
* @fi: set to fault info if the translation fails
*/
-static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
- MMUAccessType access_type, ARMMMUIdx mmu_idx,
- bool is_secure, bool s1_is_el0,
+static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
+ uint64_t address,
+ MMUAccessType access_type, bool s1_is_el0,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
{
ARMCPU *cpu = env_archcpu(env);
+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
+ bool is_secure = ptw->in_secure;
/* Read an LPAE long-descriptor translation table. */
ARMFaultType fault_type = ARMFault_Translation;
uint32_t level;
@@ -1204,7 +1218,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
descaddr |= (address >> (stride * (4 - level))) & indexmask;
descaddr &= ~7ULL;
nstable = extract32(tableattrs, 4, 1);
- descriptor = arm_ldq_ptw(env, descaddr, !nstable, mmu_idx, fi);
+ ptw->in_secure = !nstable;
+ descriptor = arm_ldq_ptw(env, ptw, descaddr, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -2361,6 +2376,7 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
ARMMMUFaultInfo *fi)
{
ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx);
+ S1Translate ptw;
if (mmu_idx != s1_mmu_idx) {
/*
@@ -2373,7 +2389,6 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
int ret;
bool ipa_secure, s2walk_secure;
ARMCacheAttrs cacheattrs1;
- ARMMMUIdx s2_mmu_idx;
bool is_el0;
uint64_t hcr;
@@ -2398,8 +2413,9 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
s2walk_secure = false;
}
- s2_mmu_idx = (s2walk_secure
- ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2);
+ ptw.in_mmu_idx =
+ s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
+ ptw.in_secure = s2walk_secure;
is_el0 = mmu_idx == ARMMMUIdx_E10_0;
/*
@@ -2411,8 +2427,8 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
cacheattrs1 = result->cacheattrs;
memset(result, 0, sizeof(*result));
- ret = get_phys_addr_lpae(env, ipa, access_type, s2_mmu_idx,
- s2walk_secure, is_el0, result, fi);
+ ret = get_phys_addr_lpae(env, &ptw, ipa, access_type,
+ is_el0, result, fi);
fi->s2addr = ipa;
/* Combine the S1 and S2 perms. */
@@ -2517,15 +2533,17 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
return get_phys_addr_disabled(env, address, access_type, mmu_idx,
is_secure, result, fi);
}
+
+ ptw.in_mmu_idx = mmu_idx;
+ ptw.in_secure = is_secure;
+
if (regime_using_lpae_format(env, mmu_idx)) {
- return get_phys_addr_lpae(env, address, access_type, mmu_idx,
- is_secure, false, result, fi);
+ return get_phys_addr_lpae(env, &ptw, address, access_type, false,
+ result, fi);
} else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) {
- return get_phys_addr_v6(env, address, access_type, mmu_idx,
- is_secure, result, fi);
+ return get_phys_addr_v6(env, &ptw, address, access_type, result, fi);
} else {
- return get_phys_addr_v5(env, address, access_type, mmu_idx,
- is_secure, result, fi);
+ return get_phys_addr_v5(env, &ptw, address, access_type, result, fi);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v4 07/24] target/arm: Split out S1Translate type
2022-10-11 3:18 ` [PATCH v4 07/24] target/arm: Split out S1Translate type Richard Henderson
@ 2022-10-17 9:53 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2022-10-17 9:53 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 11 Oct 2022 at 04:27, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Consolidate most of the inputs and outputs of S1_ptw_translate
> into a single structure. Plumb this through arm_ld*_ptw from
> the controlling get_phys_addr_* routine.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v4: Replaces a different S1TranslateResult patch, and plumbs the
> structure further out in the function call tree.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 08/24] target/arm: Plumb debug into S1Translate
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (6 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 07/24] target/arm: Split out S1Translate type Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 09/24] target/arm: Move be test for regime into S1TranslateResult Richard Henderson
` (16 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Before using softmmu page tables for the ptw, plumb down
a debug parameter so that we can query page table entries
from gdbstub without modifying cpu state.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v4: Add debug to S1Translate, and plumb the S1Translate structure down
from the very outside. It means that S1Translate is now perhaps
mis-named, but it also eliminates the "secure_debug" function name.
---
target/arm/ptw.c | 55 ++++++++++++++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 18 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index dee69ee743..8fa0088d98 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -17,6 +17,7 @@
typedef struct S1Translate {
ARMMMUIdx in_mmu_idx;
bool in_secure;
+ bool in_debug;
bool out_secure;
hwaddr out_phys;
} S1Translate;
@@ -230,6 +231,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
S1Translate s2ptw = {
.in_mmu_idx = s2_mmu_idx,
.in_secure = is_secure,
+ .in_debug = ptw->in_debug,
};
uint64_t hcr;
int ret;
@@ -2370,13 +2372,15 @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address,
return 0;
}
-bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
- MMUAccessType access_type, ARMMMUIdx mmu_idx,
- bool is_secure, GetPhysAddrResult *result,
- ARMMMUFaultInfo *fi)
+static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
+ target_ulong address,
+ MMUAccessType access_type,
+ GetPhysAddrResult *result,
+ ARMMMUFaultInfo *fi)
{
+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx);
- S1Translate ptw;
+ bool is_secure = ptw->in_secure;
if (mmu_idx != s1_mmu_idx) {
/*
@@ -2392,8 +2396,9 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
bool is_el0;
uint64_t hcr;
- ret = get_phys_addr_with_secure(env, address, access_type,
- s1_mmu_idx, is_secure, result, fi);
+ ptw->in_mmu_idx = s1_mmu_idx;
+ ret = get_phys_addr_with_struct(env, ptw, address, access_type,
+ result, fi);
/* If S1 fails or S2 is disabled, return early. */
if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2,
@@ -2413,9 +2418,9 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
s2walk_secure = false;
}
- ptw.in_mmu_idx =
+ ptw->in_mmu_idx =
s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
- ptw.in_secure = s2walk_secure;
+ ptw->in_secure = s2walk_secure;
is_el0 = mmu_idx == ARMMMUIdx_E10_0;
/*
@@ -2427,7 +2432,7 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
cacheattrs1 = result->cacheattrs;
memset(result, 0, sizeof(*result));
- ret = get_phys_addr_lpae(env, &ptw, ipa, access_type,
+ ret = get_phys_addr_lpae(env, ptw, ipa, access_type,
is_el0, result, fi);
fi->s2addr = ipa;
@@ -2534,19 +2539,29 @@ bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
is_secure, result, fi);
}
- ptw.in_mmu_idx = mmu_idx;
- ptw.in_secure = is_secure;
-
if (regime_using_lpae_format(env, mmu_idx)) {
- return get_phys_addr_lpae(env, &ptw, address, access_type, false,
+ return get_phys_addr_lpae(env, ptw, address, access_type, false,
result, fi);
} else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) {
- return get_phys_addr_v6(env, &ptw, address, access_type, result, fi);
+ return get_phys_addr_v6(env, ptw, address, access_type, result, fi);
} else {
- return get_phys_addr_v5(env, &ptw, address, access_type, result, fi);
+ return get_phys_addr_v5(env, ptw, address, access_type, result, fi);
}
}
+bool get_phys_addr_with_secure(CPUARMState *env, target_ulong address,
+ MMUAccessType access_type, ARMMMUIdx mmu_idx,
+ bool is_secure, GetPhysAddrResult *result,
+ ARMMMUFaultInfo *fi)
+{
+ S1Translate ptw = {
+ .in_mmu_idx = mmu_idx,
+ .in_secure = is_secure,
+ };
+ return get_phys_addr_with_struct(env, &ptw, address, access_type,
+ result, fi);
+}
+
bool get_phys_addr(CPUARMState *env, target_ulong address,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
@@ -2595,12 +2610,16 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
+ S1Translate ptw = {
+ .in_mmu_idx = arm_mmu_idx(env),
+ .in_secure = arm_is_secure(env),
+ .in_debug = true,
+ };
GetPhysAddrResult res = {};
ARMMMUFaultInfo fi = {};
- ARMMMUIdx mmu_idx = arm_mmu_idx(env);
bool ret;
- ret = get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &res, &fi);
+ ret = get_phys_addr_with_struct(env, &ptw, addr, MMU_DATA_LOAD, &res, &fi);
*attrs = res.f.attrs;
if (ret) {
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 09/24] target/arm: Move be test for regime into S1TranslateResult
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (7 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 08/24] target/arm: Plumb debug into S1Translate Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 10/24] target/arm: Use softmmu tlbs for page table walking Richard Henderson
` (15 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Hoist this test out of arm_ld[lq]_ptw into S1_ptw_translate.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8fa0088d98..c58788ac69 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -19,6 +19,7 @@ typedef struct S1Translate {
bool in_secure;
bool in_debug;
bool out_secure;
+ bool out_be;
hwaddr out_phys;
} S1Translate;
@@ -277,6 +278,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
ptw->out_secure = is_secure;
ptw->out_phys = addr;
+ ptw->out_be = regime_translation_big_endian(env, ptw->in_mmu_idx);
return true;
}
@@ -296,7 +298,7 @@ static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
addr = ptw->out_phys;
attrs.secure = ptw->out_secure;
as = arm_addressspace(cs, attrs);
- if (regime_translation_big_endian(env, ptw->in_mmu_idx)) {
+ if (ptw->out_be) {
data = address_space_ldl_be(as, addr, attrs, &result);
} else {
data = address_space_ldl_le(as, addr, attrs, &result);
@@ -324,7 +326,7 @@ static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
addr = ptw->out_phys;
attrs.secure = ptw->out_secure;
as = arm_addressspace(cs, attrs);
- if (regime_translation_big_endian(env, ptw->in_mmu_idx)) {
+ if (ptw->out_be) {
data = address_space_ldq_be(as, addr, attrs, &result);
} else {
data = address_space_ldq_le(as, addr, attrs, &result);
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 10/24] target/arm: Use softmmu tlbs for page table walking
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (8 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 09/24] target/arm: Move be test for regime into S1TranslateResult Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 11/24] target/arm: Split out get_phys_addr_twostage Richard Henderson
` (14 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
So far, limit the change to S1_ptw_translate, arm_ldl_ptw, and
arm_ldq_ptw. Use probe_access_full to find the host address,
and if so use a host load. If the probe fails, we've got our
fault info already. On the off chance that page tables are not
in RAM, continue to use the address_space_ld* functions.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v4: Put the host address into S1Translate immediately.
---
target/arm/cpu.h | 5 +
target/arm/ptw.c | 196 +++++++++++++++++++++++++---------------
target/arm/tlb_helper.c | 17 +++-
3 files changed, 144 insertions(+), 74 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index c94e289012..e9e77b7563 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -225,6 +225,8 @@ typedef struct CPUARMTBFlags {
target_ulong flags2;
} CPUARMTBFlags;
+typedef struct ARMMMUFaultInfo ARMMMUFaultInfo;
+
typedef struct CPUArchState {
/* Regs for current mode. */
uint32_t regs[16];
@@ -715,6 +717,9 @@ typedef struct CPUArchState {
struct CPUBreakpoint *cpu_breakpoint[16];
struct CPUWatchpoint *cpu_watchpoint[16];
+ /* Optional fault info across tlb lookup. */
+ ARMMMUFaultInfo *tlb_fi;
+
/* Fields up to this point are cleared by a CPU reset */
struct {} end_reset_fields;
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index c58788ac69..8f41d285b7 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -9,6 +9,7 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
#include "qemu/range.h"
+#include "exec/exec-all.h"
#include "cpu.h"
#include "internals.h"
#include "idau.h"
@@ -21,6 +22,7 @@ typedef struct S1Translate {
bool out_secure;
bool out_be;
hwaddr out_phys;
+ void *out_host;
} S1Translate;
static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
@@ -200,7 +202,7 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx,
return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0;
}
-static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs)
+static bool S2_attrs_are_device(uint64_t hcr, uint8_t attrs)
{
/*
* For an S1 page table walk, the stage 1 attributes are always
@@ -211,11 +213,10 @@ static bool ptw_attrs_are_device(uint64_t hcr, ARMCacheAttrs cacheattrs)
* With HCR_EL2.FWB == 1 this is when descriptor bit [4] is 0, ie
* when cacheattrs.attrs bit [2] is 0.
*/
- assert(cacheattrs.is_s2_format);
if (hcr & HCR_FWB) {
- return (cacheattrs.attrs & 0x4) == 0;
+ return (attrs & 0x4) == 0;
} else {
- return (cacheattrs.attrs & 0xc) == 0;
+ return (attrs & 0xc) == 0;
}
}
@@ -224,32 +225,65 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
hwaddr addr, ARMMMUFaultInfo *fi)
{
bool is_secure = ptw->in_secure;
+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
+ bool s2_phys = false;
+ uint8_t pte_attrs;
+ bool pte_secure;
- if (arm_mmu_idx_is_stage1_of_2(ptw->in_mmu_idx) &&
- !regime_translation_disabled(env, s2_mmu_idx, is_secure)) {
- GetPhysAddrResult s2 = {};
- S1Translate s2ptw = {
- .in_mmu_idx = s2_mmu_idx,
- .in_secure = is_secure,
- .in_debug = ptw->in_debug,
- };
- uint64_t hcr;
- int ret;
+ if (!arm_mmu_idx_is_stage1_of_2(mmu_idx)
+ || regime_translation_disabled(env, s2_mmu_idx, is_secure)) {
+ s2_mmu_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
+ s2_phys = true;
+ }
- ret = get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
- false, &s2, fi);
- if (ret) {
- assert(fi->type != ARMFault_None);
- fi->s2addr = addr;
- fi->stage2 = true;
- fi->s1ptw = true;
- fi->s1ns = !is_secure;
- return false;
+ if (unlikely(ptw->in_debug)) {
+ /*
+ * From gdbstub, do not use softmmu so that we don't modify the
+ * state of the cpu at all, including softmmu tlb contents.
+ */
+ if (s2_phys) {
+ ptw->out_phys = addr;
+ pte_attrs = 0;
+ pte_secure = is_secure;
+ } else {
+ S1Translate s2ptw = {
+ .in_mmu_idx = s2_mmu_idx,
+ .in_secure = is_secure,
+ .in_debug = true,
+ };
+ GetPhysAddrResult s2 = { };
+ if (!get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
+ false, &s2, fi)) {
+ goto fail;
+ }
+ ptw->out_phys = s2.f.phys_addr;
+ pte_attrs = s2.cacheattrs.attrs;
+ pte_secure = s2.f.attrs.secure;
}
+ ptw->out_host = NULL;
+ } else {
+ CPUTLBEntryFull *full;
+ int flags;
- hcr = arm_hcr_el2_eff_secstate(env, is_secure);
- if ((hcr & HCR_PTW) && ptw_attrs_are_device(hcr, s2.cacheattrs)) {
+ env->tlb_fi = fi;
+ flags = probe_access_full(env, addr, MMU_DATA_LOAD,
+ arm_to_core_mmu_idx(s2_mmu_idx),
+ true, &ptw->out_host, &full, 0);
+ env->tlb_fi = NULL;
+
+ if (unlikely(flags & TLB_INVALID_MASK)) {
+ goto fail;
+ }
+ ptw->out_phys = full->phys_addr;
+ pte_attrs = full->pte_attrs;
+ pte_secure = full->attrs.secure;
+ }
+
+ if (!s2_phys) {
+ uint64_t hcr = arm_hcr_el2_eff_secstate(env, is_secure);
+
+ if ((hcr & HCR_PTW) && S2_attrs_are_device(hcr, pte_attrs)) {
/*
* PTW set and S1 walk touched S2 Device memory:
* generate Permission fault.
@@ -261,25 +295,23 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
fi->s1ns = !is_secure;
return false;
}
-
- if (arm_is_secure_below_el3(env)) {
- /* Check if page table walk is to secure or non-secure PA space. */
- if (is_secure) {
- is_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
- } else {
- is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
- }
- } else {
- assert(!is_secure);
- }
-
- addr = s2.f.phys_addr;
}
- ptw->out_secure = is_secure;
- ptw->out_phys = addr;
- ptw->out_be = regime_translation_big_endian(env, ptw->in_mmu_idx);
+ /* Check if page table walk is to secure or non-secure PA space. */
+ ptw->out_secure = (is_secure
+ && !(pte_secure
+ ? env->cp15.vstcr_el2 & VSTCR_SW
+ : env->cp15.vtcr_el2 & VTCR_NSW));
+ ptw->out_be = regime_translation_big_endian(env, mmu_idx);
return true;
+
+ fail:
+ assert(fi->type != ARMFault_None);
+ fi->s2addr = addr;
+ fi->stage2 = true;
+ fi->s1ptw = true;
+ fi->s1ns = !is_secure;
+ return false;
}
/* All loads done in the course of a page table walk go through here. */
@@ -287,56 +319,78 @@ static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
- MemTxAttrs attrs = {};
- MemTxResult result = MEMTX_OK;
- AddressSpace *as;
uint32_t data;
if (!S1_ptw_translate(env, ptw, addr, fi)) {
+ /* Failure. */
+ assert(fi->s1ptw);
return 0;
}
- addr = ptw->out_phys;
- attrs.secure = ptw->out_secure;
- as = arm_addressspace(cs, attrs);
- if (ptw->out_be) {
- data = address_space_ldl_be(as, addr, attrs, &result);
+
+ if (likely(ptw->out_host)) {
+ /* Page tables are in RAM, and we have the host address. */
+ if (ptw->out_be) {
+ data = ldl_be_p(ptw->out_host);
+ } else {
+ data = ldl_le_p(ptw->out_host);
+ }
} else {
- data = address_space_ldl_le(as, addr, attrs, &result);
+ /* Page tables are in MMIO. */
+ MemTxAttrs attrs = { .secure = ptw->out_secure };
+ AddressSpace *as = arm_addressspace(cs, attrs);
+ MemTxResult result = MEMTX_OK;
+
+ if (ptw->out_be) {
+ data = address_space_ldl_be(as, ptw->out_phys, attrs, &result);
+ } else {
+ data = address_space_ldl_le(as, ptw->out_phys, attrs, &result);
+ }
+ if (unlikely(result != MEMTX_OK)) {
+ fi->type = ARMFault_SyncExternalOnWalk;
+ fi->ea = arm_extabort_type(result);
+ return 0;
+ }
}
- if (result == MEMTX_OK) {
- return data;
- }
- fi->type = ARMFault_SyncExternalOnWalk;
- fi->ea = arm_extabort_type(result);
- return 0;
+ return data;
}
static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
- MemTxAttrs attrs = {};
- MemTxResult result = MEMTX_OK;
- AddressSpace *as;
uint64_t data;
if (!S1_ptw_translate(env, ptw, addr, fi)) {
+ /* Failure. */
+ assert(fi->s1ptw);
return 0;
}
- addr = ptw->out_phys;
- attrs.secure = ptw->out_secure;
- as = arm_addressspace(cs, attrs);
- if (ptw->out_be) {
- data = address_space_ldq_be(as, addr, attrs, &result);
+
+ if (likely(ptw->out_host)) {
+ /* Page tables are in RAM, and we have the host address. */
+ if (ptw->out_be) {
+ data = ldq_be_p(ptw->out_host);
+ } else {
+ data = ldq_le_p(ptw->out_host);
+ }
} else {
- data = address_space_ldq_le(as, addr, attrs, &result);
+ /* Page tables are in MMIO. */
+ MemTxAttrs attrs = { .secure = ptw->out_secure };
+ AddressSpace *as = arm_addressspace(cs, attrs);
+ MemTxResult result = MEMTX_OK;
+
+ if (ptw->out_be) {
+ data = address_space_ldq_be(as, ptw->out_phys, attrs, &result);
+ } else {
+ data = address_space_ldq_le(as, ptw->out_phys, attrs, &result);
+ }
+ if (unlikely(result != MEMTX_OK)) {
+ fi->type = ARMFault_SyncExternalOnWalk;
+ fi->ea = arm_extabort_type(result);
+ return 0;
+ }
}
- if (result == MEMTX_OK) {
- return data;
- }
- fi->type = ARMFault_SyncExternalOnWalk;
- fi->ea = arm_extabort_type(result);
- return 0;
+ return data;
}
static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 3462a6ea14..69b0dc69df 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -208,10 +208,21 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
bool probe, uintptr_t retaddr)
{
ARMCPU *cpu = ARM_CPU(cs);
- ARMMMUFaultInfo fi = {};
GetPhysAddrResult res = {};
+ ARMMMUFaultInfo local_fi, *fi;
int ret;
+ /*
+ * Allow S1_ptw_translate to see any fault generated here.
+ * Since this may recurse, read and clear.
+ */
+ fi = cpu->env.tlb_fi;
+ if (fi) {
+ cpu->env.tlb_fi = NULL;
+ } else {
+ fi = memset(&local_fi, 0, sizeof(local_fi));
+ }
+
/*
* Walk the page table and (if the mapping exists) add the page
* to the TLB. On success, return true. Otherwise, if probing,
@@ -220,7 +231,7 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
*/
ret = get_phys_addr(&cpu->env, address, access_type,
core_to_arm_mmu_idx(&cpu->env, mmu_idx),
- &res, &fi);
+ &res, fi);
if (likely(!ret)) {
/*
* Map a single [sub]page. Regions smaller than our declared
@@ -242,7 +253,7 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
} else {
/* now we have a real cpu fault */
cpu_restore_state(cs, retaddr, true);
- arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi);
+ arm_deliver_fault(cpu, address, access_type, mmu_idx, fi);
}
}
#else
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 11/24] target/arm: Split out get_phys_addr_twostage
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (9 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 10/24] target/arm: Use softmmu tlbs for page table walking Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:18 ` [PATCH v4 12/24] target/arm: Use bool consistently for get_phys_addr subroutines Richard Henderson
` (13 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 191 +++++++++++++++++++++++++----------------------
1 file changed, 100 insertions(+), 91 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8f41d285b7..dd6556560a 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -31,6 +31,13 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
__attribute__((nonnull));
+static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
+ target_ulong address,
+ MMUAccessType access_type,
+ GetPhysAddrResult *result,
+ ARMMMUFaultInfo *fi)
+ __attribute__((nonnull));
+
/* This mapping is common between ID_AA64MMFR0.PARANGE and TCR_ELx.{I}PS. */
static const uint8_t pamax_map[] = {
[0] = 32,
@@ -2428,6 +2435,94 @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address,
return 0;
}
+static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
+ target_ulong address,
+ MMUAccessType access_type,
+ GetPhysAddrResult *result,
+ ARMMMUFaultInfo *fi)
+{
+ hwaddr ipa;
+ int s1_prot;
+ int ret;
+ bool is_secure = ptw->in_secure;
+ bool ipa_secure, s2walk_secure;
+ ARMCacheAttrs cacheattrs1;
+ bool is_el0;
+ uint64_t hcr;
+
+ ret = get_phys_addr_with_struct(env, ptw, address, access_type, result, fi);
+
+ /* If S1 fails or S2 is disabled, return early. */
+ if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2, is_secure)) {
+ return ret;
+ }
+
+ ipa = result->f.phys_addr;
+ ipa_secure = result->f.attrs.secure;
+ if (is_secure) {
+ /* Select TCR based on the NS bit from the S1 walk. */
+ s2walk_secure = !(ipa_secure
+ ? env->cp15.vstcr_el2 & VSTCR_SW
+ : env->cp15.vtcr_el2 & VTCR_NSW);
+ } else {
+ assert(!ipa_secure);
+ s2walk_secure = false;
+ }
+
+ is_el0 = ptw->in_mmu_idx == ARMMMUIdx_Stage1_E0;
+ ptw->in_mmu_idx = s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
+ ptw->in_secure = s2walk_secure;
+
+ /*
+ * S1 is done, now do S2 translation.
+ * Save the stage1 results so that we may merge prot and cacheattrs later.
+ */
+ s1_prot = result->f.prot;
+ cacheattrs1 = result->cacheattrs;
+ memset(result, 0, sizeof(*result));
+
+ ret = get_phys_addr_lpae(env, ptw, ipa, access_type, is_el0, result, fi);
+ fi->s2addr = ipa;
+
+ /* Combine the S1 and S2 perms. */
+ result->f.prot &= s1_prot;
+
+ /* If S2 fails, return early. */
+ if (ret) {
+ return ret;
+ }
+
+ /* Combine the S1 and S2 cache attributes. */
+ hcr = arm_hcr_el2_eff_secstate(env, is_secure);
+ if (hcr & HCR_DC) {
+ /*
+ * HCR.DC forces the first stage attributes to
+ * Normal Non-Shareable,
+ * Inner Write-Back Read-Allocate Write-Allocate,
+ * Outer Write-Back Read-Allocate Write-Allocate.
+ * Do not overwrite Tagged within attrs.
+ */
+ if (cacheattrs1.attrs != 0xf0) {
+ cacheattrs1.attrs = 0xff;
+ }
+ cacheattrs1.shareability = 0;
+ }
+ result->cacheattrs = combine_cacheattrs(hcr, cacheattrs1,
+ result->cacheattrs);
+
+ /*
+ * Check if IPA translates to secure or non-secure PA space.
+ * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA.
+ */
+ result->f.attrs.secure =
+ (is_secure
+ && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))
+ && (ipa_secure
+ || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))));
+
+ return 0;
+}
+
static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
target_ulong address,
MMUAccessType access_type,
@@ -2441,99 +2536,13 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
if (mmu_idx != s1_mmu_idx) {
/*
* Call ourselves recursively to do the stage 1 and then stage 2
- * translations if mmu_idx is a two-stage regime.
+ * translations if mmu_idx is a two-stage regime, and EL2 present.
+ * Otherwise, a stage1+stage2 translation is just stage 1.
*/
+ ptw->in_mmu_idx = mmu_idx = s1_mmu_idx;
if (arm_feature(env, ARM_FEATURE_EL2)) {
- hwaddr ipa;
- int s1_prot;
- int ret;
- bool ipa_secure, s2walk_secure;
- ARMCacheAttrs cacheattrs1;
- bool is_el0;
- uint64_t hcr;
-
- ptw->in_mmu_idx = s1_mmu_idx;
- ret = get_phys_addr_with_struct(env, ptw, address, access_type,
- result, fi);
-
- /* If S1 fails or S2 is disabled, return early. */
- if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2,
- is_secure)) {
- return ret;
- }
-
- ipa = result->f.phys_addr;
- ipa_secure = result->f.attrs.secure;
- if (is_secure) {
- /* Select TCR based on the NS bit from the S1 walk. */
- s2walk_secure = !(ipa_secure
- ? env->cp15.vstcr_el2 & VSTCR_SW
- : env->cp15.vtcr_el2 & VTCR_NSW);
- } else {
- assert(!ipa_secure);
- s2walk_secure = false;
- }
-
- ptw->in_mmu_idx =
- s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
- ptw->in_secure = s2walk_secure;
- is_el0 = mmu_idx == ARMMMUIdx_E10_0;
-
- /*
- * S1 is done, now do S2 translation.
- * Save the stage1 results so that we may merge
- * prot and cacheattrs later.
- */
- s1_prot = result->f.prot;
- cacheattrs1 = result->cacheattrs;
- memset(result, 0, sizeof(*result));
-
- ret = get_phys_addr_lpae(env, ptw, ipa, access_type,
- is_el0, result, fi);
- fi->s2addr = ipa;
-
- /* Combine the S1 and S2 perms. */
- result->f.prot &= s1_prot;
-
- /* If S2 fails, return early. */
- if (ret) {
- return ret;
- }
-
- /* Combine the S1 and S2 cache attributes. */
- hcr = arm_hcr_el2_eff_secstate(env, is_secure);
- if (hcr & HCR_DC) {
- /*
- * HCR.DC forces the first stage attributes to
- * Normal Non-Shareable,
- * Inner Write-Back Read-Allocate Write-Allocate,
- * Outer Write-Back Read-Allocate Write-Allocate.
- * Do not overwrite Tagged within attrs.
- */
- if (cacheattrs1.attrs != 0xf0) {
- cacheattrs1.attrs = 0xff;
- }
- cacheattrs1.shareability = 0;
- }
- result->cacheattrs = combine_cacheattrs(hcr, cacheattrs1,
- result->cacheattrs);
-
- /*
- * Check if IPA translates to secure or non-secure PA space.
- * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA.
- */
- result->f.attrs.secure =
- (is_secure
- && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))
- && (ipa_secure
- || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))));
-
- return 0;
- } else {
- /*
- * For non-EL2 CPUs a stage1+stage2 translation is just stage 1.
- */
- mmu_idx = stage_1_mmu_idx(mmu_idx);
+ return get_phys_addr_twostage(env, ptw, address, access_type,
+ result, fi);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 12/24] target/arm: Use bool consistently for get_phys_addr subroutines
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (10 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 11/24] target/arm: Split out get_phys_addr_twostage Richard Henderson
@ 2022-10-11 3:18 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 13/24] target/arm: Add ptw_idx to S1Translate Richard Henderson
` (12 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
The return type of the functions is already bool, but in a few
instances we used an integer type with the return statement.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index dd6556560a..6c5ed56a10 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2432,7 +2432,7 @@ static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address,
result->f.lg_page_size = TARGET_PAGE_BITS;
result->cacheattrs.shareability = shareability;
result->cacheattrs.attrs = memattr;
- return 0;
+ return false;
}
static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
@@ -2443,9 +2443,8 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
{
hwaddr ipa;
int s1_prot;
- int ret;
bool is_secure = ptw->in_secure;
- bool ipa_secure, s2walk_secure;
+ bool ret, ipa_secure, s2walk_secure;
ARMCacheAttrs cacheattrs1;
bool is_el0;
uint64_t hcr;
@@ -2520,7 +2519,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
&& (ipa_secure
|| !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))));
- return 0;
+ return false;
}
static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 13/24] target/arm: Add ptw_idx to S1Translate
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (11 preceding siblings ...)
2022-10-11 3:18 ` [PATCH v4 12/24] target/arm: Use bool consistently for get_phys_addr subroutines Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-17 10:01 ` Peter Maydell
2022-10-11 3:19 ` [PATCH v4 14/24] target/arm: Add isar predicates for FEAT_HAFDBS Richard Henderson
` (11 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Hoist the computation of the mmu_idx for the ptw up to
get_phys_addr_with_struct and get_phys_addr_twostage.
This removes the duplicate check for stage2 disabled
from the middle of the walk, performing it only once.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 53 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 6c5ed56a10..b2bfcfde9a 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -17,6 +17,7 @@
typedef struct S1Translate {
ARMMMUIdx in_mmu_idx;
+ ARMMMUIdx in_ptw_idx;
bool in_secure;
bool in_debug;
bool out_secure;
@@ -233,17 +234,12 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
{
bool is_secure = ptw->in_secure;
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
- ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
- bool s2_phys = false;
+ ARMMMUIdx s2_mmu_idx = ptw->in_ptw_idx;
+ bool s2_phys = s2_mmu_idx == ARMMMUIdx_Phys_S ||
+ s2_mmu_idx == ARMMMUIdx_Phys_NS;
uint8_t pte_attrs;
bool pte_secure;
- if (!arm_mmu_idx_is_stage1_of_2(mmu_idx)
- || regime_translation_disabled(env, s2_mmu_idx, is_secure)) {
- s2_mmu_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
- s2_phys = true;
- }
-
if (unlikely(ptw->in_debug)) {
/*
* From gdbstub, do not use softmmu so that we don't modify the
@@ -256,10 +252,12 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
} else {
S1Translate s2ptw = {
.in_mmu_idx = s2_mmu_idx,
+ .in_ptw_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS,
.in_secure = is_secure,
.in_debug = true,
};
GetPhysAddrResult s2 = { };
+
if (!get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
false, &s2, fi)) {
goto fail;
@@ -1283,7 +1281,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
descaddr |= (address >> (stride * (4 - level))) & indexmask;
descaddr &= ~7ULL;
nstable = extract32(tableattrs, 4, 1);
- ptw->in_secure = !nstable;
+ if (!nstable) {
+ /* Stage2_S -> Stage2 or Phys_S -> Phys_NS */
+ ptw->in_ptw_idx &= ~1;
+ ptw->in_secure = false;
+ }
descriptor = arm_ldq_ptw(env, ptw, descaddr, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
@@ -2470,6 +2472,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
is_el0 = ptw->in_mmu_idx == ARMMMUIdx_Stage1_E0;
ptw->in_mmu_idx = s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
+ ptw->in_ptw_idx = s2walk_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
ptw->in_secure = s2walk_secure;
/*
@@ -2529,10 +2532,32 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
ARMMMUFaultInfo *fi)
{
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
- ARMMMUIdx s1_mmu_idx = stage_1_mmu_idx(mmu_idx);
bool is_secure = ptw->in_secure;
+ ARMMMUIdx s1_mmu_idx;
- if (mmu_idx != s1_mmu_idx) {
+ switch (mmu_idx) {
+ case ARMMMUIdx_Phys_S:
+ case ARMMMUIdx_Phys_NS:
+ /* Checking Phys early avoids special casing later vs regime_el. */
+ return get_phys_addr_disabled(env, address, access_type, mmu_idx,
+ is_secure, result, fi);
+
+ case ARMMMUIdx_Stage1_E0:
+ case ARMMMUIdx_Stage1_E1:
+ case ARMMMUIdx_Stage1_E1_PAN:
+ /* First stage lookup uses second stage for ptw. */
+ ptw->in_ptw_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
+ break;
+
+ case ARMMMUIdx_E10_0:
+ s1_mmu_idx = ARMMMUIdx_Stage1_E0;
+ goto do_twostage;
+ case ARMMMUIdx_E10_1:
+ s1_mmu_idx = ARMMMUIdx_Stage1_E1;
+ goto do_twostage;
+ case ARMMMUIdx_E10_1_PAN:
+ s1_mmu_idx = ARMMMUIdx_Stage1_E1_PAN;
+ do_twostage:
/*
* Call ourselves recursively to do the stage 1 and then stage 2
* translations if mmu_idx is a two-stage regime, and EL2 present.
@@ -2543,6 +2568,12 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
return get_phys_addr_twostage(env, ptw, address, access_type,
result, fi);
}
+ /* fall through */
+
+ default:
+ /* Single stage and second stage uses physical for ptw. */
+ ptw->in_ptw_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
+ break;
}
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v4 13/24] target/arm: Add ptw_idx to S1Translate
2022-10-11 3:19 ` [PATCH v4 13/24] target/arm: Add ptw_idx to S1Translate Richard Henderson
@ 2022-10-17 10:01 ` Peter Maydell
2022-10-20 3:16 ` Richard Henderson
0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2022-10-17 10:01 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 11 Oct 2022 at 04:30, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Hoist the computation of the mmu_idx for the ptw up to
> get_phys_addr_with_struct and get_phys_addr_twostage.
> This removes the duplicate check for stage2 disabled
> from the middle of the walk, performing it only once.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> + if (!nstable) {
> + /* Stage2_S -> Stage2 or Phys_S -> Phys_NS */
> + ptw->in_ptw_idx &= ~1;
> + ptw->in_secure = false;
> + }
I feel like this bitwise manipulation of the mmuidx values
is leaving a landmine for any future re-organization of
our mmuidx use. Can we do this just using the symbolic
constant values?
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v4 13/24] target/arm: Add ptw_idx to S1Translate
2022-10-17 10:01 ` Peter Maydell
@ 2022-10-20 3:16 ` Richard Henderson
0 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-20 3:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-arm
On 10/17/22 20:01, Peter Maydell wrote:
> On Tue, 11 Oct 2022 at 04:30, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Hoist the computation of the mmu_idx for the ptw up to
>> get_phys_addr_with_struct and get_phys_addr_twostage.
>> This removes the duplicate check for stage2 disabled
>> from the middle of the walk, performing it only once.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> + if (!nstable) {
>> + /* Stage2_S -> Stage2 or Phys_S -> Phys_NS */
>> + ptw->in_ptw_idx &= ~1;
>> + ptw->in_secure = false;
>> + }
>
> I feel like this bitwise manipulation of the mmuidx values
> is leaving a landmine for any future re-organization of
> our mmuidx use. Can we do this just using the symbolic
> constant values?
I can't think of a way with just symbolic values,
but I can add BUILD_BUG_ON to validate expectations,
so that it's not a *hidden* landmine. :-)
r~
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 14/24] target/arm: Add isar predicates for FEAT_HAFDBS
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (12 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 13/24] target/arm: Add ptw_idx to S1Translate Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 15/24] target/arm: Extract HA and HD in aa64_va_parameters Richard Henderson
` (10 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
The MMFR1 field may indicate support for hardware update of
access flag alone, or access flag and dirty bit.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e9e77b7563..cde4e86db2 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -4139,6 +4139,16 @@ static inline bool isar_feature_aa64_lva(const ARMISARegisters *id)
return FIELD_EX64(id->id_aa64mmfr2, ID_AA64MMFR2, VARANGE) != 0;
}
+static inline bool isar_feature_aa64_hafs(const ARMISARegisters *id)
+{
+ return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, HAFDBS) != 0;
+}
+
+static inline bool isar_feature_aa64_hdbs(const ARMISARegisters *id)
+{
+ return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, HAFDBS) >= 2;
+}
+
static inline bool isar_feature_aa64_tts2uxn(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, XNX) != 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 15/24] target/arm: Extract HA and HD in aa64_va_parameters
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (13 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 14/24] target/arm: Add isar predicates for FEAT_HAFDBS Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 16/24] target/arm: Move S1_ptw_translate outside arm_ld[lq]_ptw Richard Henderson
` (9 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 2 ++
target/arm/helper.c | 8 +++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index c3c3920ded..76ec7ee8cc 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1041,6 +1041,8 @@ typedef struct ARMVAParameters {
bool hpd : 1;
bool tsz_oob : 1; /* tsz has been clamped to legal range */
bool ds : 1;
+ bool ha : 1;
+ bool hd : 1;
ARMGranuleSize gran : 2;
} ARMVAParameters;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c672903f43..4487957e5d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10470,7 +10470,7 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
ARMMMUIdx mmu_idx, bool data)
{
uint64_t tcr = regime_tcr(env, mmu_idx);
- bool epd, hpd, tsz_oob, ds;
+ bool epd, hpd, tsz_oob, ds, ha, hd;
int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
ARMGranuleSize gran;
ARMCPU *cpu = env_archcpu(env);
@@ -10489,6 +10489,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
epd = false;
sh = extract32(tcr, 12, 2);
ps = extract32(tcr, 16, 3);
+ ha = extract32(tcr, 21, 1) && cpu_isar_feature(aa64_hafs, cpu);
+ hd = extract32(tcr, 22, 1) && cpu_isar_feature(aa64_hdbs, cpu);
ds = extract64(tcr, 32, 1);
} else {
/*
@@ -10510,6 +10512,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
hpd = extract64(tcr, 42, 1);
}
ps = extract64(tcr, 32, 3);
+ ha = extract64(tcr, 39, 1) && cpu_isar_feature(aa64_hafs, cpu);
+ hd = extract64(tcr, 40, 1) && cpu_isar_feature(aa64_hdbs, cpu);
ds = extract64(tcr, 59, 1);
}
@@ -10581,6 +10585,8 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
.hpd = hpd,
.tsz_oob = tsz_oob,
.ds = ds,
+ .ha = ha,
+ .hd = ha && hd,
.gran = gran,
};
}
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 16/24] target/arm: Move S1_ptw_translate outside arm_ld[lq]_ptw
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (14 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 15/24] target/arm: Extract HA and HD in aa64_va_parameters Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 17/24] target/arm: Add ARMFault_UnsuppAtomicUpdate Richard Henderson
` (8 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Separate S1 translation from the actual lookup.
Will enable lpae hardware updates.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 41 ++++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index b2bfcfde9a..d54e6ca938 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -320,18 +320,12 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
}
/* All loads done in the course of a page table walk go through here. */
-static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
+static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw,
ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
uint32_t data;
- if (!S1_ptw_translate(env, ptw, addr, fi)) {
- /* Failure. */
- assert(fi->s1ptw);
- return 0;
- }
-
if (likely(ptw->out_host)) {
/* Page tables are in RAM, and we have the host address. */
if (ptw->out_be) {
@@ -359,18 +353,12 @@ static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
return data;
}
-static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw, hwaddr addr,
+static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw,
ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
uint64_t data;
- if (!S1_ptw_translate(env, ptw, addr, fi)) {
- /* Failure. */
- assert(fi->s1ptw);
- return 0;
- }
-
if (likely(ptw->out_host)) {
/* Page tables are in RAM, and we have the host address. */
if (ptw->out_be) {
@@ -527,7 +515,10 @@ static bool get_phys_addr_v5(CPUARMState *env, S1Translate *ptw,
fi->type = ARMFault_Translation;
goto do_fault;
}
- desc = arm_ldl_ptw(env, ptw, table, fi);
+ if (!S1_ptw_translate(env, ptw, table, fi)) {
+ goto do_fault;
+ }
+ desc = arm_ldl_ptw(env, ptw, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -565,7 +556,10 @@ static bool get_phys_addr_v5(CPUARMState *env, S1Translate *ptw,
/* Fine pagetable. */
table = (desc & 0xfffff000) | ((address >> 8) & 0xffc);
}
- desc = arm_ldl_ptw(env, ptw, table, fi);
+ if (!S1_ptw_translate(env, ptw, table, fi)) {
+ goto do_fault;
+ }
+ desc = arm_ldl_ptw(env, ptw, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -650,7 +644,10 @@ static bool get_phys_addr_v6(CPUARMState *env, S1Translate *ptw,
fi->type = ARMFault_Translation;
goto do_fault;
}
- desc = arm_ldl_ptw(env, ptw, table, fi);
+ if (!S1_ptw_translate(env, ptw, table, fi)) {
+ goto do_fault;
+ }
+ desc = arm_ldl_ptw(env, ptw, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -703,7 +700,10 @@ static bool get_phys_addr_v6(CPUARMState *env, S1Translate *ptw,
ns = extract32(desc, 3, 1);
/* Lookup l2 entry. */
table = (desc & 0xfffffc00) | ((address >> 10) & 0x3fc);
- desc = arm_ldl_ptw(env, ptw, table, fi);
+ if (!S1_ptw_translate(env, ptw, table, fi)) {
+ goto do_fault;
+ }
+ desc = arm_ldl_ptw(env, ptw, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
@@ -1286,7 +1286,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
ptw->in_ptw_idx &= ~1;
ptw->in_secure = false;
}
- descriptor = arm_ldq_ptw(env, ptw, descaddr, fi);
+ if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
+ goto do_fault;
+ }
+ descriptor = arm_ldq_ptw(env, ptw, fi);
if (fi->type != ARMFault_None) {
goto do_fault;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 17/24] target/arm: Add ARMFault_UnsuppAtomicUpdate
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (15 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 16/24] target/arm: Move S1_ptw_translate outside arm_ld[lq]_ptw Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 18/24] target/arm: Remove loop from get_phys_addr_lpae Richard Henderson
` (7 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
This fault type is to be used with FEAT_HAFDBS when
the guest enables hw updates, but places the tables
in memory where atomic updates are unsupported.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 76ec7ee8cc..e195d771e0 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -338,6 +338,7 @@ typedef enum ARMFaultType {
ARMFault_AsyncExternal,
ARMFault_Debug,
ARMFault_TLBConflict,
+ ARMFault_UnsuppAtomicUpdate,
ARMFault_Lockdown,
ARMFault_Exclusive,
ARMFault_ICacheMaint,
@@ -524,6 +525,9 @@ static inline uint32_t arm_fi_to_lfsc(ARMMMUFaultInfo *fi)
case ARMFault_TLBConflict:
fsc = 0x30;
break;
+ case ARMFault_UnsuppAtomicUpdate:
+ fsc = 0x31;
+ break;
case ARMFault_Lockdown:
fsc = 0x34;
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 18/24] target/arm: Remove loop from get_phys_addr_lpae
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (16 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 17/24] target/arm: Add ARMFault_UnsuppAtomicUpdate Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 19/24] target/arm: Fix fault reporting in get_phys_addr_lpae Richard Henderson
` (6 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
The unconditional loop was used both to iterate over levels
and to control parsing of attributes. Use an explicit goto
in both cases.
While this appears less clean for iterating over levels, we
will need to jump back into the middle of this loop for
atomic updates, which is even uglier.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 182 +++++++++++++++++++++++------------------------
1 file changed, 91 insertions(+), 91 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index d54e6ca938..9b767f8236 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1082,6 +1082,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
uint64_t descaddrmask;
bool aarch64 = arm_el_is_aa64(env, el);
bool guarded = false;
+ uint64_t descriptor;
+ bool nstable;
/* TODO: This code does not support shareability levels. */
if (aarch64) {
@@ -1274,99 +1276,97 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
* bits at each step.
*/
tableattrs = is_secure ? 0 : (1 << 4);
- for (;;) {
- uint64_t descriptor;
- bool nstable;
- descaddr |= (address >> (stride * (4 - level))) & indexmask;
- descaddr &= ~7ULL;
- nstable = extract32(tableattrs, 4, 1);
- if (!nstable) {
- /* Stage2_S -> Stage2 or Phys_S -> Phys_NS */
- ptw->in_ptw_idx &= ~1;
- ptw->in_secure = false;
- }
- if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
- goto do_fault;
- }
- descriptor = arm_ldq_ptw(env, ptw, fi);
- if (fi->type != ARMFault_None) {
- goto do_fault;
- }
-
- if (!(descriptor & 1) ||
- (!(descriptor & 2) && (level == 3))) {
- /* Invalid, or the Reserved level 3 encoding */
- goto do_fault;
- }
-
- descaddr = descriptor & descaddrmask;
-
- /*
- * For FEAT_LPA and PS=6, bits [51:48] of descaddr are in [15:12]
- * of descriptor. For FEAT_LPA2 and effective DS, bits [51:50] of
- * descaddr are in [9:8]. Otherwise, if descaddr is out of range,
- * raise AddressSizeFault.
- */
- if (outputsize > 48) {
- if (param.ds) {
- descaddr |= extract64(descriptor, 8, 2) << 50;
- } else {
- descaddr |= extract64(descriptor, 12, 4) << 48;
- }
- } else if (descaddr >> outputsize) {
- fault_type = ARMFault_AddressSize;
- goto do_fault;
- }
-
- if ((descriptor & 2) && (level < 3)) {
- /*
- * Table entry. The top five bits are attributes which may
- * propagate down through lower levels of the table (and
- * which are all arranged so that 0 means "no effect", so
- * we can gather them up by ORing in the bits at each level).
- */
- tableattrs |= extract64(descriptor, 59, 5);
- level++;
- indexmask = indexmask_grainsize;
- continue;
- }
- /*
- * Block entry at level 1 or 2, or page entry at level 3.
- * These are basically the same thing, although the number
- * of bits we pull in from the vaddr varies. Note that although
- * descaddrmask masks enough of the low bits of the descriptor
- * to give a correct page or table address, the address field
- * in a block descriptor is smaller; so we need to explicitly
- * clear the lower bits here before ORing in the low vaddr bits.
- */
- page_size = (1ULL << ((stride * (4 - level)) + 3));
- descaddr &= ~(hwaddr)(page_size - 1);
- descaddr |= (address & (page_size - 1));
- /* Extract attributes from the descriptor */
- attrs = extract64(descriptor, 2, 10)
- | (extract64(descriptor, 52, 12) << 10);
-
- if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
- /* Stage 2 table descriptors do not include any attribute fields */
- break;
- }
- /* Merge in attributes from table descriptors */
- attrs |= nstable << 3; /* NS */
- guarded = extract64(descriptor, 50, 1); /* GP */
- if (param.hpd) {
- /* HPD disables all the table attributes except NSTable. */
- break;
- }
- attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
- /*
- * The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
- * means "force PL1 access only", which means forcing AP[1] to 0.
- */
- attrs &= ~(extract32(tableattrs, 2, 1) << 4); /* !APT[0] => AP[1] */
- attrs |= extract32(tableattrs, 3, 1) << 5; /* APT[1] => AP[2] */
- break;
+ next_level:
+ descaddr |= (address >> (stride * (4 - level))) & indexmask;
+ descaddr &= ~7ULL;
+ nstable = extract32(tableattrs, 4, 1);
+ if (!nstable) {
+ /* Stage2_S -> Stage2 or Phys_S -> Phys_NS */
+ ptw->in_ptw_idx &= ~1;
+ ptw->in_secure = false;
}
+ if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
+ goto do_fault;
+ }
+ descriptor = arm_ldq_ptw(env, ptw, fi);
+ if (fi->type != ARMFault_None) {
+ goto do_fault;
+ }
+
+ if (!(descriptor & 1) || (!(descriptor & 2) && (level == 3))) {
+ /* Invalid, or the Reserved level 3 encoding */
+ goto do_fault;
+ }
+
+ descaddr = descriptor & descaddrmask;
+
+ /*
+ * For FEAT_LPA and PS=6, bits [51:48] of descaddr are in [15:12]
+ * of descriptor. For FEAT_LPA2 and effective DS, bits [51:50] of
+ * descaddr are in [9:8]. Otherwise, if descaddr is out of range,
+ * raise AddressSizeFault.
+ */
+ if (outputsize > 48) {
+ if (param.ds) {
+ descaddr |= extract64(descriptor, 8, 2) << 50;
+ } else {
+ descaddr |= extract64(descriptor, 12, 4) << 48;
+ }
+ } else if (descaddr >> outputsize) {
+ fault_type = ARMFault_AddressSize;
+ goto do_fault;
+ }
+
+ if ((descriptor & 2) && (level < 3)) {
+ /*
+ * Table entry. The top five bits are attributes which may
+ * propagate down through lower levels of the table (and
+ * which are all arranged so that 0 means "no effect", so
+ * we can gather them up by ORing in the bits at each level).
+ */
+ tableattrs |= extract64(descriptor, 59, 5);
+ level++;
+ indexmask = indexmask_grainsize;
+ goto next_level;
+ }
+
+ /*
+ * Block entry at level 1 or 2, or page entry at level 3.
+ * These are basically the same thing, although the number
+ * of bits we pull in from the vaddr varies. Note that although
+ * descaddrmask masks enough of the low bits of the descriptor
+ * to give a correct page or table address, the address field
+ * in a block descriptor is smaller; so we need to explicitly
+ * clear the lower bits here before ORing in the low vaddr bits.
+ */
+ page_size = (1ULL << ((stride * (4 - level)) + 3));
+ descaddr &= ~(hwaddr)(page_size - 1);
+ descaddr |= (address & (page_size - 1));
+ /* Extract attributes from the descriptor */
+ attrs = extract64(descriptor, 2, 10)
+ | (extract64(descriptor, 52, 12) << 10);
+
+ if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+ /* Stage 2 table descriptors do not include any attribute fields */
+ goto skip_attrs;
+ }
+ /* Merge in attributes from table descriptors */
+ attrs |= nstable << 3; /* NS */
+ guarded = extract64(descriptor, 50, 1); /* GP */
+ if (param.hpd) {
+ /* HPD disables all the table attributes except NSTable. */
+ goto skip_attrs;
+ }
+ attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
+ /*
+ * The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
+ * means "force PL1 access only", which means forcing AP[1] to 0.
+ */
+ attrs &= ~(extract32(tableattrs, 2, 1) << 4); /* !APT[0] => AP[1] */
+ attrs |= extract32(tableattrs, 3, 1) << 5; /* APT[1] => AP[2] */
+ skip_attrs:
+
/*
* Here descaddr is the final physical address, and attributes
* are all in attrs.
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 19/24] target/arm: Fix fault reporting in get_phys_addr_lpae
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (17 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 18/24] target/arm: Remove loop from get_phys_addr_lpae Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 20/24] target/arm: Don't shift attrs " Richard Henderson
` (5 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Always overriding fi->type was incorrect, as we would not properly
propagate the fault type from S1_ptw_translate, or arm_ldq_ptw.
Simplify things by providing a new label for a translation fault.
For other faults, store into fi directly.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 9b767f8236..acbf09cce8 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1065,8 +1065,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
ARMCPU *cpu = env_archcpu(env);
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
bool is_secure = ptw->in_secure;
- /* Read an LPAE long-descriptor translation table. */
- ARMFaultType fault_type = ARMFault_Translation;
uint32_t level;
ARMVAParameters param;
uint64_t ttbr;
@@ -1103,8 +1101,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
* so our choice is to always raise the fault.
*/
if (param.tsz_oob) {
- fault_type = ARMFault_Translation;
- goto do_fault;
+ goto do_translation_fault;
}
addrsize = 64 - 8 * param.tbi;
@@ -1141,8 +1138,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
addrsize - inputsize);
if (-top_bits != param.select) {
/* The gap between the two regions is a Translation fault */
- fault_type = ARMFault_Translation;
- goto do_fault;
+ goto do_translation_fault;
}
}
@@ -1168,7 +1164,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
* Translation table walk disabled => Translation fault on TLB miss
* Note: This is always 0 on 64-bit EL2 and EL3.
*/
- goto do_fault;
+ goto do_translation_fault;
}
if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) {
@@ -1199,8 +1195,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
if (param.ds && stride == 9 && sl2) {
if (sl0 != 0) {
level = 0;
- fault_type = ARMFault_Translation;
- goto do_fault;
+ goto do_translation_fault;
}
startlevel = -1;
} else if (!aarch64 || stride == 9) {
@@ -1219,8 +1214,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
ok = check_s2_mmu_setup(cpu, aarch64, startlevel,
inputsize, stride, outputsize);
if (!ok) {
- fault_type = ARMFault_Translation;
- goto do_fault;
+ goto do_translation_fault;
}
level = startlevel;
}
@@ -1242,7 +1236,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
descaddr |= extract64(ttbr, 2, 4) << 48;
} else if (descaddr >> outputsize) {
level = 0;
- fault_type = ARMFault_AddressSize;
+ fi->type = ARMFault_AddressSize;
goto do_fault;
}
@@ -1296,7 +1290,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
if (!(descriptor & 1) || (!(descriptor & 2) && (level == 3))) {
/* Invalid, or the Reserved level 3 encoding */
- goto do_fault;
+ goto do_translation_fault;
}
descaddr = descriptor & descaddrmask;
@@ -1314,7 +1308,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
descaddr |= extract64(descriptor, 12, 4) << 48;
}
} else if (descaddr >> outputsize) {
- fault_type = ARMFault_AddressSize;
+ fi->type = ARMFault_AddressSize;
goto do_fault;
}
@@ -1371,9 +1365,9 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
* Here descaddr is the final physical address, and attributes
* are all in attrs.
*/
- fault_type = ARMFault_AccessFlag;
if ((attrs & (1 << 8)) == 0) {
/* Access flag */
+ fi->type = ARMFault_AccessFlag;
goto do_fault;
}
@@ -1390,8 +1384,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
}
- fault_type = ARMFault_Permission;
if (!(result->f.prot & (1 << access_type))) {
+ fi->type = ARMFault_Permission;
goto do_fault;
}
@@ -1436,8 +1430,9 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
result->f.lg_page_size = ctz64(page_size);
return false;
-do_fault:
- fi->type = fault_type;
+ do_translation_fault:
+ fi->type = ARMFault_Translation;
+ do_fault:
fi->level = level;
/* Tag the error as S2 for failed S1 PTW at S2 or ordinary S2. */
fi->stage2 = fi->s1ptw || (mmu_idx == ARMMMUIdx_Stage2 ||
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 20/24] target/arm: Don't shift attrs in get_phys_addr_lpae
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (18 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 19/24] target/arm: Fix fault reporting in get_phys_addr_lpae Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 21/24] target/arm: Consider GP an attribute " Richard Henderson
` (4 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Leave the upper and lower attributes in the place they originate
from in the descriptor. Shifting them around is confusing, since
one cannot read the bit numbers out of the manual. Also, new
attributes have been added which would alter the shifts.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index acbf09cce8..2227d2a2fd 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1071,7 +1071,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
hwaddr descaddr, indexmask, indexmask_grainsize;
uint32_t tableattrs;
target_ulong page_size;
- uint32_t attrs;
+ uint64_t attrs;
int32_t stride;
int addrsize, inputsize, outputsize;
uint64_t tcr = regime_tcr(env, mmu_idx);
@@ -1338,49 +1338,48 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
descaddr &= ~(hwaddr)(page_size - 1);
descaddr |= (address & (page_size - 1));
/* Extract attributes from the descriptor */
- attrs = extract64(descriptor, 2, 10)
- | (extract64(descriptor, 52, 12) << 10);
+ attrs = descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(52, 12));
if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
/* Stage 2 table descriptors do not include any attribute fields */
goto skip_attrs;
}
/* Merge in attributes from table descriptors */
- attrs |= nstable << 3; /* NS */
+ attrs |= nstable << 5; /* NS */
guarded = extract64(descriptor, 50, 1); /* GP */
if (param.hpd) {
/* HPD disables all the table attributes except NSTable. */
goto skip_attrs;
}
- attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
+ attrs |= extract64(tableattrs, 0, 2) << 53; /* XN, PXN */
/*
* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
* means "force PL1 access only", which means forcing AP[1] to 0.
*/
- attrs &= ~(extract32(tableattrs, 2, 1) << 4); /* !APT[0] => AP[1] */
- attrs |= extract32(tableattrs, 3, 1) << 5; /* APT[1] => AP[2] */
+ attrs &= ~(extract64(tableattrs, 2, 1) << 6); /* !APT[0] => AP[1] */
+ attrs |= extract32(tableattrs, 3, 1) << 7; /* APT[1] => AP[2] */
skip_attrs:
/*
* Here descaddr is the final physical address, and attributes
* are all in attrs.
*/
- if ((attrs & (1 << 8)) == 0) {
+ if ((attrs & (1 << 10)) == 0) {
/* Access flag */
fi->type = ARMFault_AccessFlag;
goto do_fault;
}
- ap = extract32(attrs, 4, 2);
+ ap = extract32(attrs, 6, 2);
if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
ns = mmu_idx == ARMMMUIdx_Stage2;
- xn = extract32(attrs, 11, 2);
+ xn = extract64(attrs, 54, 2);
result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
} else {
- ns = extract32(attrs, 3, 1);
- xn = extract32(attrs, 12, 1);
- pxn = extract32(attrs, 11, 1);
+ ns = extract32(attrs, 5, 1);
+ xn = extract64(attrs, 54, 1);
+ pxn = extract64(attrs, 53, 1);
result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
}
@@ -1405,10 +1404,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
result->cacheattrs.is_s2_format = true;
- result->cacheattrs.attrs = extract32(attrs, 0, 4);
+ result->cacheattrs.attrs = extract32(attrs, 2, 4);
} else {
/* Index into MAIR registers for cache attributes */
- uint8_t attrindx = extract32(attrs, 0, 3);
+ uint8_t attrindx = extract32(attrs, 2, 3);
uint64_t mair = env->cp15.mair_el[regime_el(env, mmu_idx)];
assert(attrindx <= 7);
result->cacheattrs.is_s2_format = false;
@@ -1423,7 +1422,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
if (param.ds) {
result->cacheattrs.shareability = param.sh;
} else {
- result->cacheattrs.shareability = extract32(attrs, 6, 2);
+ result->cacheattrs.shareability = extract32(attrs, 8, 2);
}
result->f.phys_addr = descaddr;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 21/24] target/arm: Consider GP an attribute in get_phys_addr_lpae
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (19 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 20/24] target/arm: Don't shift attrs " Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-11 3:19 ` [PATCH v4 22/24] target/arm: Implement FEAT_HAFDBS, access flag portion Richard Henderson
` (3 subsequent siblings)
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Both GP and DBM are in the upper attribute block.
Extend the computation of attrs to include them,
then simplify the setting of guarded.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 2227d2a2fd..8db635ca98 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1079,7 +1079,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
uint32_t el = regime_el(env, mmu_idx);
uint64_t descaddrmask;
bool aarch64 = arm_el_is_aa64(env, el);
- bool guarded = false;
uint64_t descriptor;
bool nstable;
@@ -1338,7 +1337,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
descaddr &= ~(hwaddr)(page_size - 1);
descaddr |= (address & (page_size - 1));
/* Extract attributes from the descriptor */
- attrs = descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(52, 12));
+ attrs = descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50, 14));
if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
/* Stage 2 table descriptors do not include any attribute fields */
@@ -1346,7 +1345,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
}
/* Merge in attributes from table descriptors */
attrs |= nstable << 5; /* NS */
- guarded = extract64(descriptor, 50, 1); /* GP */
if (param.hpd) {
/* HPD disables all the table attributes except NSTable. */
goto skip_attrs;
@@ -1399,7 +1397,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
/* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {
- result->f.guarded = guarded;
+ result->f.guarded = extract64(attrs, 50, 1); /* GP */
}
if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 22/24] target/arm: Implement FEAT_HAFDBS, access flag portion
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (20 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 21/24] target/arm: Consider GP an attribute " Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-17 10:45 ` Peter Maydell
2022-10-11 3:19 ` [PATCH v4 23/24] target/arm: Implement FEAT_HAFDBS, dirty bit portion Richard Henderson
` (2 subsequent siblings)
24 siblings, 1 reply; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Perform the atomic update for hardware management of the access flag.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v4: Raise permission fault if pte read-only and atomic update reqd.
Split out dirty bit portion.
Prepare for a single update for AF + DB.
---
docs/system/arm/emulation.rst | 1 +
target/arm/cpu64.c | 1 +
target/arm/ptw.c | 147 +++++++++++++++++++++++++++++++---
3 files changed, 138 insertions(+), 11 deletions(-)
diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst
index cfb4b0768b..580e67b190 100644
--- a/docs/system/arm/emulation.rst
+++ b/docs/system/arm/emulation.rst
@@ -32,6 +32,7 @@ the following architecture extensions:
- FEAT_FlagM (Flag manipulation instructions v2)
- FEAT_FlagM2 (Enhancements to flag manipulation instructions)
- FEAT_GTG (Guest translation granule size)
+- FEAT_HAFDBS (Hardware management of the access flag and dirty bit state)
- FEAT_HCX (Support for the HCRX_EL2 register)
- FEAT_HPDS (Hierarchical permission disables)
- FEAT_I8MM (AArch64 Int8 matrix multiplication instructions)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 85e0d1daf1..fe1369fe96 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -1165,6 +1165,7 @@ static void aarch64_max_initfn(Object *obj)
cpu->isar.id_aa64mmfr0 = t;
t = cpu->isar.id_aa64mmfr1;
+ t = FIELD_DP64(t, ID_AA64MMFR1, HAFDBS, 1); /* FEAT_HAFDBS, AF only */
t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* FEAT_VMID16 */
t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1); /* FEAT_VHE */
t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* FEAT_HPDS */
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8db635ca98..82b6ab029e 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -21,7 +21,9 @@ typedef struct S1Translate {
bool in_secure;
bool in_debug;
bool out_secure;
+ bool out_rw;
bool out_be;
+ hwaddr out_virt;
hwaddr out_phys;
void *out_host;
} S1Translate;
@@ -240,6 +242,8 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
uint8_t pte_attrs;
bool pte_secure;
+ ptw->out_virt = addr;
+
if (unlikely(ptw->in_debug)) {
/*
* From gdbstub, do not use softmmu so that we don't modify the
@@ -267,6 +271,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
pte_secure = s2.f.attrs.secure;
}
ptw->out_host = NULL;
+ ptw->out_rw = false;
} else {
CPUTLBEntryFull *full;
int flags;
@@ -281,6 +286,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
goto fail;
}
ptw->out_phys = full->phys_addr;
+ ptw->out_rw = full->prot & PROT_WRITE;
pte_attrs = full->pte_attrs;
pte_secure = full->attrs.secure;
}
@@ -324,14 +330,16 @@ static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate *ptw,
ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
+ void *host = ptw->out_host;
uint32_t data;
- if (likely(ptw->out_host)) {
+ if (likely(host)) {
/* Page tables are in RAM, and we have the host address. */
+ data = qatomic_read((uint32_t *)host);
if (ptw->out_be) {
- data = ldl_be_p(ptw->out_host);
+ data = be32_to_cpu(data);
} else {
- data = ldl_le_p(ptw->out_host);
+ data = le32_to_cpu(data);
}
} else {
/* Page tables are in MMIO. */
@@ -357,15 +365,25 @@ static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw,
ARMMMUFaultInfo *fi)
{
CPUState *cs = env_cpu(env);
+ void *host = ptw->out_host;
uint64_t data;
- if (likely(ptw->out_host)) {
+ if (likely(host)) {
/* Page tables are in RAM, and we have the host address. */
+#ifdef CONFIG_ATOMIC64
+ data = qatomic_read__nocheck((uint64_t *)host);
if (ptw->out_be) {
- data = ldq_be_p(ptw->out_host);
+ data = be64_to_cpu(data);
} else {
- data = ldq_le_p(ptw->out_host);
+ data = le64_to_cpu(data);
}
+#else
+ if (ptw->out_be) {
+ data = ldq_be_p(host);
+ } else {
+ data = ldq_le_p(host);
+ }
+#endif
} else {
/* Page tables are in MMIO. */
MemTxAttrs attrs = { .secure = ptw->out_secure };
@@ -386,6 +404,91 @@ static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate *ptw,
return data;
}
+static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val,
+ uint64_t new_val, S1Translate *ptw,
+ ARMMMUFaultInfo *fi)
+{
+ uint64_t cur_val;
+ void *host = ptw->out_host;
+
+ if (unlikely(!host)) {
+ fi->type = ARMFault_UnsuppAtomicUpdate;
+ fi->s1ptw = true;
+ return 0;
+ }
+
+ /*
+ * Raising a stage2 Protection fault for an atomic update to a read-only
+ * page is delayed until it is certain that there is a change to make.
+ */
+ if (unlikely(!ptw->out_rw)) {
+ int flags;
+ void *discard;
+
+ env->tlb_fi = fi;
+ flags = probe_access_flags(env, ptw->out_virt, MMU_DATA_STORE,
+ arm_to_core_mmu_idx(ptw->in_ptw_idx),
+ true, &discard, 0);
+ env->tlb_fi = NULL;
+
+ if (unlikely(flags & TLB_INVALID_MASK)) {
+ assert(fi->type != ARMFault_None);
+ fi->s2addr = ptw->out_virt;
+ fi->stage2 = true;
+ fi->s1ptw = true;
+ fi->s1ns = ptw->in_secure;
+ return 0;
+ }
+
+ /* In case CAS mismatches and we loop, remember writability. */
+ ptw->out_rw = true;
+ }
+
+#ifdef CONFIG_ATOMIC64
+ if (ptw->out_be) {
+ old_val = cpu_to_be64(old_val);
+ new_val = cpu_to_be64(new_val);
+ cur_val = qatomic_cmpxchg__nocheck((uint64_t *)host, old_val, new_val);
+ cur_val = be64_to_cpu(cur_val);
+ } else {
+ old_val = cpu_to_le64(old_val);
+ new_val = cpu_to_le64(new_val);
+ cur_val = qatomic_cmpxchg__nocheck((uint64_t *)host, old_val, new_val);
+ cur_val = le64_to_cpu(cur_val);
+ }
+#else
+ /*
+ * We can't support the full 64-bit atomic cmpxchg on the host.
+ * Because this is only used for FEAT_HAFDBS, which is only for AA64,
+ * we know that TCG_OVERSIZED_GUEST is set, which means that we are
+ * running in round-robin mode and could only race with dma i/o.
+ */
+#ifndef TCG_OVERSIZED_GUEST
+# error "Unexpected configuration"
+#endif
+ bool locked = qemu_mutex_iothread_locked();
+ if (!locked) {
+ qemu_mutex_lock_iothread();
+ }
+ if (ptw->out_be) {
+ cur_val = ldq_be_p(host);
+ if (cur_val == old_val) {
+ stq_be_p(host, new_val);
+ }
+ } else {
+ cur_val = ldq_le_p(host);
+ if (cur_val == old_val) {
+ stq_le_p(host, new_val);
+ }
+ }
+ if (!locked) {
+ qemu_mutex_unlock_iothread();
+ }
+#endif
+
+ return cur_val;
+}
+
static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
uint32_t *table, uint32_t address)
{
@@ -1079,7 +1182,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
uint32_t el = regime_el(env, mmu_idx);
uint64_t descaddrmask;
bool aarch64 = arm_el_is_aa64(env, el);
- uint64_t descriptor;
+ uint64_t descriptor, new_descriptor;
bool nstable;
/* TODO: This code does not support shareability levels. */
@@ -1286,7 +1389,9 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
if (fi->type != ARMFault_None) {
goto do_fault;
}
+ new_descriptor = descriptor;
+ restart_atomic_update:
if (!(descriptor & 1) || (!(descriptor & 2) && (level == 3))) {
/* Invalid, or the Reserved level 3 encoding */
goto do_translation_fault;
@@ -1362,10 +1467,18 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
* Here descaddr is the final physical address, and attributes
* are all in attrs.
*/
- if ((attrs & (1 << 10)) == 0) {
- /* Access flag */
- fi->type = ARMFault_AccessFlag;
- goto do_fault;
+ if (!(attrs & (1 << 10)) && !ptw->in_debug) {
+ /*
+ * Access flag.
+ * If HA is enabled, prepare to update the descriptor below.
+ * Otherwise, pass the access fault on to software.
+ */
+ if (param.ha) {
+ new_descriptor |= 1 << 10; /* AF */
+ } else {
+ fi->type = ARMFault_AccessFlag;
+ goto do_fault;
+ }
}
ap = extract32(attrs, 6, 2);
@@ -1381,6 +1494,18 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
}
+ /* If FEAT_HAFDBS has made changes, update the PTE. */
+ if (new_descriptor != descriptor) {
+ new_descriptor = arm_casq_ptw(env, descriptor, new_descriptor, ptw, fi);
+ if (fi->type != ARMFault_None) {
+ goto do_fault;
+ }
+ if (new_descriptor != descriptor) {
+ descriptor = new_descriptor;
+ goto restart_atomic_update;
+ }
+ }
+
if (!(result->f.prot & (1 << access_type))) {
fi->type = ARMFault_Permission;
goto do_fault;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v4 22/24] target/arm: Implement FEAT_HAFDBS, access flag portion
2022-10-11 3:19 ` [PATCH v4 22/24] target/arm: Implement FEAT_HAFDBS, access flag portion Richard Henderson
@ 2022-10-17 10:45 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2022-10-17 10:45 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 11 Oct 2022 at 04:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Perform the atomic update for hardware management of the access flag.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v4: Raise permission fault if pte read-only and atomic update reqd.
> Split out dirty bit portion.
> Prepare for a single update for AF + DB.
> +static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val,
> + uint64_t new_val, S1Translate *ptw,
> + ARMMMUFaultInfo *fi)
> +{
> + uint64_t cur_val;
> + void *host = ptw->out_host;
> +
> + if (unlikely(!host)) {
> + fi->type = ARMFault_UnsuppAtomicUpdate;
> + fi->s1ptw = true;
> + return 0;
> + }
> +
> + /*
> + * Raising a stage2 Protection fault for an atomic update to a read-only
> + * page is delayed until it is certain that there is a change to make.
> + */
> + if (unlikely(!ptw->out_rw)) {
> + int flags;
> + void *discard;
> +
> + env->tlb_fi = fi;
> + flags = probe_access_flags(env, ptw->out_virt, MMU_DATA_STORE,
> + arm_to_core_mmu_idx(ptw->in_ptw_idx),
> + true, &discard, 0);
> + env->tlb_fi = NULL;
> +
> + if (unlikely(flags & TLB_INVALID_MASK)) {
> + assert(fi->type != ARMFault_None);
> + fi->s2addr = ptw->out_virt;
> + fi->stage2 = true;
> + fi->s1ptw = true;
> + fi->s1ns = ptw->in_secure;
Shouldn't there be a ! here ? LHS is true-for-NS and RHS is true-for-S, I think.
> + return 0;
> + }
> +
> + /* In case CAS mismatches and we loop, remember writability. */
> + ptw->out_rw = true;
> + }
> +
> +#ifdef CONFIG_ATOMIC64
> + if (ptw->out_be) {
> + old_val = cpu_to_be64(old_val);
> + new_val = cpu_to_be64(new_val);
> + cur_val = qatomic_cmpxchg__nocheck((uint64_t *)host, old_val, new_val);
> + cur_val = be64_to_cpu(cur_val);
> + } else {
> + old_val = cpu_to_le64(old_val);
> + new_val = cpu_to_le64(new_val);
> + cur_val = qatomic_cmpxchg__nocheck((uint64_t *)host, old_val, new_val);
> + cur_val = le64_to_cpu(cur_val);
> + }
> +#else
> + /*
> + * We can't support the full 64-bit atomic cmpxchg on the host.
> + * Because this is only used for FEAT_HAFDBS, which is only for AA64,
> + * we know that TCG_OVERSIZED_GUEST is set, which means that we are
> + * running in round-robin mode and could only race with dma i/o.
> + */
> +#ifndef TCG_OVERSIZED_GUEST
> +# error "Unexpected configuration"
> +#endif
> + bool locked = qemu_mutex_iothread_locked();
> + if (!locked) {
> + qemu_mutex_lock_iothread();
> + }
> + if (ptw->out_be) {
> + cur_val = ldq_be_p(host);
> + if (cur_val == old_val) {
> + stq_be_p(host, new_val);
> + }
> + } else {
> + cur_val = ldq_le_p(host);
> + if (cur_val == old_val) {
> + stq_le_p(host, new_val);
> + }
> + }
> + if (!locked) {
> + qemu_mutex_unlock_iothread();
> + }
> +#endif
> +
> + return cur_val;
> +}
> @@ -1286,7 +1389,9 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
> if (fi->type != ARMFault_None) {
> goto do_fault;
> }
> + new_descriptor = descriptor;
>
> + restart_atomic_update:
> if (!(descriptor & 1) || (!(descriptor & 2) && (level == 3))) {
> /* Invalid, or the Reserved level 3 encoding */
> goto do_translation_fault;
> @@ -1362,10 +1467,18 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
> * Here descaddr is the final physical address, and attributes
> * are all in attrs.
> */
> - if ((attrs & (1 << 10)) == 0) {
> - /* Access flag */
> - fi->type = ARMFault_AccessFlag;
> - goto do_fault;
> + if (!(attrs & (1 << 10)) && !ptw->in_debug) {
> + /*
> + * Access flag.
> + * If HA is enabled, prepare to update the descriptor below.
> + * Otherwise, pass the access fault on to software.
> + */
> + if (param.ha) {
> + new_descriptor |= 1 << 10; /* AF */
> + } else {
> + fi->type = ARMFault_AccessFlag;
> + goto do_fault;
> + }
> }
>
> ap = extract32(attrs, 6, 2);
> @@ -1381,6 +1494,18 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
> result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
> }
>
> + /* If FEAT_HAFDBS has made changes, update the PTE. */
> + if (new_descriptor != descriptor) {
> + new_descriptor = arm_casq_ptw(env, descriptor, new_descriptor, ptw, fi);
> + if (fi->type != ARMFault_None) {
> + goto do_fault;
> + }
> + if (new_descriptor != descriptor) {
I think we could probably usefully add a comment here:
/*
* I_YZSVV says that if the in-memory descriptor has changed, then we
* must use the information in that new value (which might include a
* different output address, different attributes, or generate a fault),
* Restart the handling of the descriptor value from scratch.
*/
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 23/24] target/arm: Implement FEAT_HAFDBS, dirty bit portion
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (21 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 22/24] target/arm: Implement FEAT_HAFDBS, access flag portion Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-17 11:01 ` Peter Maydell
2022-10-11 3:19 ` [PATCH v4 24/24] target/arm: Use the max page size in a 2-stage ptw Richard Henderson
2022-10-17 12:49 ` [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Peter Maydell
24 siblings, 1 reply; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Perform the atomic update for hardware management of the dirty bit.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/cpu64.c | 2 +-
target/arm/ptw.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index fe1369fe96..0732796559 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -1165,7 +1165,7 @@ static void aarch64_max_initfn(Object *obj)
cpu->isar.id_aa64mmfr0 = t;
t = cpu->isar.id_aa64mmfr1;
- t = FIELD_DP64(t, ID_AA64MMFR1, HAFDBS, 1); /* FEAT_HAFDBS, AF only */
+ t = FIELD_DP64(t, ID_AA64MMFR1, HAFDBS, 2); /* FEAT_HAFDBS */
t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* FEAT_VMID16 */
t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1); /* FEAT_VHE */
t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* FEAT_HPDS */
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 82b6ab029e..0dbbb7d4d4 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1484,10 +1484,30 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
ap = extract32(attrs, 6, 2);
if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+ if (param.hd
+ && extract64(attrs, 51, 1) /* DBM */
+ && access_type == MMU_DATA_STORE) {
+ /*
+ * Pre-emptively set S2AP[1], so that we compute EXEC properly.
+ * C.f. AArch64.S2ApplyOutputPerms, which does the same thing.
+ */
+ ap |= 2;
+ new_descriptor |= 1ull << 7;
+ }
ns = mmu_idx == ARMMMUIdx_Stage2;
xn = extract64(attrs, 54, 2);
result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
} else {
+ if (param.hd
+ && extract64(attrs, 51, 1) /* DBM */
+ && access_type == MMU_DATA_STORE) {
+ /*
+ * Pre-emptively clear AP[2], so that we compute EXEC properly.
+ * C.f. AArch64.S1ApplyOutputPerms, which does the same thing.
+ */
+ ap &= ~2;
+ new_descriptor &= ~(1ull << 7);
+ }
ns = extract32(attrs, 5, 1);
xn = extract64(attrs, 54, 1);
pxn = extract64(attrs, 53, 1);
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v4 23/24] target/arm: Implement FEAT_HAFDBS, dirty bit portion
2022-10-11 3:19 ` [PATCH v4 23/24] target/arm: Implement FEAT_HAFDBS, dirty bit portion Richard Henderson
@ 2022-10-17 11:01 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2022-10-17 11:01 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 11 Oct 2022 at 04:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Perform the atomic update for hardware management of the dirty bit.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/cpu64.c | 2 +-
> target/arm/ptw.c | 20 ++++++++++++++++++++
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index fe1369fe96..0732796559 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -1165,7 +1165,7 @@ static void aarch64_max_initfn(Object *obj)
> cpu->isar.id_aa64mmfr0 = t;
>
> t = cpu->isar.id_aa64mmfr1;
> - t = FIELD_DP64(t, ID_AA64MMFR1, HAFDBS, 1); /* FEAT_HAFDBS, AF only */
> + t = FIELD_DP64(t, ID_AA64MMFR1, HAFDBS, 2); /* FEAT_HAFDBS */
> t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* FEAT_VMID16 */
> t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1); /* FEAT_VHE */
> t = FIELD_DP64(t, ID_AA64MMFR1, HPDS, 1); /* FEAT_HPDS */
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 82b6ab029e..0dbbb7d4d4 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1484,10 +1484,30 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
> ap = extract32(attrs, 6, 2);
>
> if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
> + if (param.hd
> + && extract64(attrs, 51, 1) /* DBM */
> + && access_type == MMU_DATA_STORE) {
> + /*
> + * Pre-emptively set S2AP[1], so that we compute EXEC properly.
> + * C.f. AArch64.S2ApplyOutputPerms, which does the same thing.
> + */
> + ap |= 2;
> + new_descriptor |= 1ull << 7;
> + }
> ns = mmu_idx == ARMMMUIdx_Stage2;
> xn = extract64(attrs, 54, 2);
> result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
> } else {
> + if (param.hd
> + && extract64(attrs, 51, 1) /* DBM */
> + && access_type == MMU_DATA_STORE) {
> + /*
> + * Pre-emptively clear AP[2], so that we compute EXEC properly.
> + * C.f. AArch64.S1ApplyOutputPerms, which does the same thing.
> + */
> + ap &= ~2;
> + new_descriptor &= ~(1ull << 7);
> + }
At this point in the code we've already merged the table
attributes into attrs. If the APTable[] bits forbid write permission
then HAFDBS isn't allowed to grant write permission. I think we
need to do this a bit further up, before we have merged the table
attributes in (so that the table attributes merge can force our
attribute bit back to 1 if the table attrs forbid writes).
(In the pseudocode the Permissions struct keeps track of the ap_table
and ap permissions settings separately and doesn't combine them until
AArch64.S1HasPermissionsFault().)
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 24/24] target/arm: Use the max page size in a 2-stage ptw
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (22 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 23/24] target/arm: Implement FEAT_HAFDBS, dirty bit portion Richard Henderson
@ 2022-10-11 3:19 ` Richard Henderson
2022-10-17 12:49 ` [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Peter Maydell
24 siblings, 0 replies; 33+ messages in thread
From: Richard Henderson @ 2022-10-11 3:19 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Marc Zyngier, Peter Maydell
We had only been reporting the stage2 page size. This causes
problems if stage1 is using a larger page size (16k, 2M, etc),
but stage2 is using a smaller page size, because cputlb does
not set large_page_{addr,mask} properly.
Fix by using the max of the two page sizes.
Reported-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 0dbbb7d4d4..b8934765ec 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2584,7 +2584,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
ARMMMUFaultInfo *fi)
{
hwaddr ipa;
- int s1_prot;
+ int s1_prot, s1_lgpgsz;
bool is_secure = ptw->in_secure;
bool ret, ipa_secure, s2walk_secure;
ARMCacheAttrs cacheattrs1;
@@ -2620,6 +2620,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
* Save the stage1 results so that we may merge prot and cacheattrs later.
*/
s1_prot = result->f.prot;
+ s1_lgpgsz = result->f.lg_page_size;
cacheattrs1 = result->cacheattrs;
memset(result, 0, sizeof(*result));
@@ -2634,6 +2635,14 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
return ret;
}
+ /*
+ * Use the maximum of the S1 & S2 page size, so that invalidation
+ * of pages > TARGET_PAGE_SIZE works correctly.
+ */
+ if (result->f.lg_page_size < s1_lgpgsz) {
+ result->f.lg_page_size = s1_lgpgsz;
+ }
+
/* Combine the S1 and S2 cache attributes. */
hcr = arm_hcr_el2_eff_secstate(env, is_secure);
if (hcr & HCR_DC) {
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS
2022-10-11 3:18 [PATCH v4 00/24] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (23 preceding siblings ...)
2022-10-11 3:19 ` [PATCH v4 24/24] target/arm: Use the max page size in a 2-stage ptw Richard Henderson
@ 2022-10-17 12:49 ` Peter Maydell
24 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2022-10-17 12:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Tue, 11 Oct 2022 at 04:21, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Changes for v4:
> * Rebase on today's target-arm.next pull, including 21 patches.
> * Split AF and DB enablement into two patches.
> * Perform only one atomic update per PTE.
> * Raise Permission fault if atomic update reqd to read-only PTE.
> * More use of S1Translate struct, which is perhaps now mis-named but
> more generally useful/used; suggestions for better naming solicited.
> * Other minor updates per review.
I'm taking patches 1-12 into target-arm.next.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread