* [PATCH v6 01/14] target/arm: Introduce regime_is_stage2
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 5:18 ` [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate Richard Henderson
` (13 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée, Philippe Mathieu-Daudé
Reduce the amount of typing required for this check.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/internals.h | 5 +++++
target/arm/helper.c | 14 +++++---------
target/arm/ptw.c | 14 ++++++--------
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index c3c3920ded..2b6889073d 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -673,6 +673,11 @@ static inline bool regime_is_pan(CPUARMState *env, ARMMMUIdx mmu_idx)
}
}
+static inline bool regime_is_stage2(ARMMMUIdx mmu_idx)
+{
+ return mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S;
+}
+
/* Return the exception level which controls this address translation regime */
static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
{
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c672903f43..cbfaabbc09 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10352,7 +10352,7 @@ int aa64_va_parameter_tbi(uint64_t tcr, ARMMMUIdx mmu_idx)
{
if (regime_has_2_ranges(mmu_idx)) {
return extract64(tcr, 37, 2);
- } else if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+ } else if (regime_is_stage2(mmu_idx)) {
return 0; /* VTCR_EL2 */
} else {
/* Replicate the single TBI bit so we always have 2 bits. */
@@ -10364,7 +10364,7 @@ int aa64_va_parameter_tbid(uint64_t tcr, ARMMMUIdx mmu_idx)
{
if (regime_has_2_ranges(mmu_idx)) {
return extract64(tcr, 51, 2);
- } else if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+ } else if (regime_is_stage2(mmu_idx)) {
return 0; /* VTCR_EL2 */
} else {
/* Replicate the single TBID bit so we always have 2 bits. */
@@ -10474,7 +10474,7 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
ARMGranuleSize gran;
ARMCPU *cpu = env_archcpu(env);
- bool stage2 = mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S;
+ bool stage2 = regime_is_stage2(mmu_idx);
if (!regime_has_2_ranges(mmu_idx)) {
select = 0;
@@ -10532,22 +10532,18 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
}
ds = false;
} else if (ds) {
- switch (mmu_idx) {
- case ARMMMUIdx_Stage2:
- case ARMMMUIdx_Stage2_S:
+ if (regime_is_stage2(mmu_idx)) {
if (gran == Gran16K) {
ds = cpu_isar_feature(aa64_tgran16_2_lpa2, cpu);
} else {
ds = cpu_isar_feature(aa64_tgran4_2_lpa2, cpu);
}
- break;
- default:
+ } else {
if (gran == Gran16K) {
ds = cpu_isar_feature(aa64_tgran16_lpa2, cpu);
} else {
ds = cpu_isar_feature(aa64_tgran4_lpa2, cpu);
}
- break;
}
if (ds) {
min_tsz = 12;
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 6c5ed56a10..004375e02b 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -842,8 +842,7 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
bool have_wxn;
int wxn = 0;
- assert(mmu_idx != ARMMMUIdx_Stage2);
- assert(mmu_idx != ARMMMUIdx_Stage2_S);
+ assert(!regime_is_stage2(mmu_idx));
user_rw = simple_ap_to_rw_prot_is_user(ap, true);
if (is_user) {
@@ -1171,7 +1170,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
goto do_fault;
}
- if (mmu_idx != ARMMMUIdx_Stage2 && mmu_idx != ARMMMUIdx_Stage2_S) {
+ if (!regime_is_stage2(mmu_idx)) {
/*
* The starting level depends on the virtual address size (which can
* be up to 48 bits) and the translation granule size. It indicates
@@ -1342,7 +1341,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
attrs = extract64(descriptor, 2, 10)
| (extract64(descriptor, 52, 12) << 10);
- if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+ if (regime_is_stage2(mmu_idx)) {
/* Stage 2 table descriptors do not include any attribute fields */
break;
}
@@ -1374,7 +1373,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
ap = extract32(attrs, 4, 2);
- if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+ if (regime_is_stage2(mmu_idx)) {
ns = mmu_idx == ARMMMUIdx_Stage2;
xn = extract32(attrs, 11, 2);
result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
@@ -1404,7 +1403,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
result->f.guarded = guarded;
}
- if (mmu_idx == ARMMMUIdx_Stage2 || mmu_idx == ARMMMUIdx_Stage2_S) {
+ if (regime_is_stage2(mmu_idx)) {
result->cacheattrs.is_s2_format = true;
result->cacheattrs.attrs = extract32(attrs, 0, 4);
} else {
@@ -1435,8 +1434,7 @@ do_fault:
fi->type = fault_type;
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 ||
- mmu_idx == ARMMMUIdx_Stage2_S);
+ fi->stage2 = fi->s1ptw || regime_is_stage2(mmu_idx);
fi->s1ns = mmu_idx == ARMMMUIdx_Stage2;
return true;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
2022-10-24 5:18 ` [PATCH v6 01/14] target/arm: Introduce regime_is_stage2 Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 14:09 ` Alex Bennée
2022-10-24 5:18 ` [PATCH v6 03/14] target/arm: Add isar predicates for FEAT_HAFDBS Richard Henderson
` (12 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 | 71 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 54 insertions(+), 17 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 004375e02b..161b7922e3 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,33 +234,24 @@ 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;
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
* 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 {
+ if (regime_is_stage2(s2_mmu_idx)) {
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;
@@ -267,6 +259,11 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
ptw->out_phys = s2.f.phys_addr;
pte_attrs = s2.cacheattrs.attrs;
pte_secure = s2.f.attrs.secure;
+ } else {
+ /* Regime is physical. */
+ ptw->out_phys = addr;
+ pte_attrs = 0;
+ pte_secure = is_secure;
}
ptw->out_host = NULL;
} else {
@@ -287,7 +284,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
pte_secure = full->attrs.secure;
}
- if (!s2_phys) {
+ if (regime_is_stage2(s2_mmu_idx)) {
uint64_t hcr = arm_hcr_el2_eff_secstate(env, is_secure);
if ((hcr & HCR_PTW) && S2_attrs_are_device(hcr, pte_attrs)) {
@@ -1282,7 +1279,18 @@ 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
+ * Assert that the non-secure idx are even, and relative order.
+ */
+ QEMU_BUILD_BUG_ON((ARMMMUIdx_Phys_NS & 1) != 0);
+ QEMU_BUILD_BUG_ON((ARMMMUIdx_Stage2 & 1) != 0);
+ QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS + 1 != ARMMMUIdx_Phys_S);
+ QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2 + 1 != ARMMMUIdx_Stage2_S);
+ 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;
@@ -2468,6 +2476,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;
/*
@@ -2527,10 +2536,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.
@@ -2541,6 +2572,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] 25+ messages in thread* Re: [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate
2022-10-24 5:18 ` [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate Richard Henderson
@ 2022-10-24 14:09 ` Alex Bennée
0 siblings, 0 replies; 25+ messages in thread
From: Alex Bennée @ 2022-10-24 14:09 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> 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 | 71 ++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 54 insertions(+), 17 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 004375e02b..161b7922e3 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;
I could use a comment here to explain the difference between mmu and ptr
indexes here because...
<snip>
> @@ -2527,10 +2536,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.
> @@ -2541,6 +2572,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 */
following this I got confused as to what might be overwritten or
ignored. I assume for v8-A (ARM_FEATURE_EL2) we won't be falling through
anyway?
Anyway I think I understand it now:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
> +
> + default:
> + /* Single stage and second stage uses physical for ptw. */
> + ptw->in_ptw_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS;
> + break;
> }
>
> /*
--
Alex Bennée
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 03/14] target/arm: Add isar predicates for FEAT_HAFDBS
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
2022-10-24 5:18 ` [PATCH v6 01/14] target/arm: Introduce regime_is_stage2 Richard Henderson
2022-10-24 5:18 ` [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 5:18 ` [PATCH v6 04/14] target/arm: Extract HA and HD in aa64_va_parameters Richard Henderson
` (11 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 64fc03214c..0eeb4abe53 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] 25+ messages in thread* [PATCH v6 04/14] target/arm: Extract HA and HD in aa64_va_parameters
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (2 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 03/14] target/arm: Add isar predicates for FEAT_HAFDBS Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 14:19 ` Alex Bennée
2022-10-24 5:18 ` [PATCH v6 05/14] target/arm: Move S1_ptw_translate outside arm_ld[lq]_ptw Richard Henderson
` (10 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5: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/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 2b6889073d..16d7989604 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1046,6 +1046,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 cbfaabbc09..6c7a8beed6 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);
}
@@ -10577,6 +10581,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] 25+ messages in thread* [PATCH v6 05/14] target/arm: Move S1_ptw_translate outside arm_ld[lq]_ptw
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (3 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 04/14] target/arm: Extract HA and HD in aa64_va_parameters Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 5:18 ` [PATCH v6 06/14] target/arm: Add ARMFault_UnsuppAtomicUpdate Richard Henderson
` (9 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 161b7922e3..36524b35ef 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -319,18 +319,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) {
@@ -358,18 +352,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) {
@@ -526,7 +514,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;
}
@@ -564,7 +555,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;
}
@@ -649,7 +643,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;
}
@@ -702,7 +699,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;
}
@@ -1291,7 +1291,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] 25+ messages in thread* [PATCH v6 06/14] target/arm: Add ARMFault_UnsuppAtomicUpdate
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (4 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 05/14] target/arm: Move S1_ptw_translate outside arm_ld[lq]_ptw Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 14:53 ` Alex Bennée
2022-10-24 5:18 ` [PATCH v6 07/14] target/arm: Remove loop from get_phys_addr_lpae Richard Henderson
` (8 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 16d7989604..a5ef5f6054 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] 25+ messages in thread* [PATCH v6 07/14] target/arm: Remove loop from get_phys_addr_lpae
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (5 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 06/14] target/arm: Add ARMFault_UnsuppAtomicUpdate Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 5:18 ` [PATCH v6 08/14] target/arm: Fix fault reporting in get_phys_addr_lpae Richard Henderson
` (7 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 | 192 +++++++++++++++++++++++------------------------
1 file changed, 96 insertions(+), 96 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 36524b35ef..615471699e 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1080,6 +1080,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) {
@@ -1272,106 +1274,104 @@ 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
- * Assert that the non-secure idx are even, and relative order.
- */
- QEMU_BUILD_BUG_ON((ARMMMUIdx_Phys_NS & 1) != 0);
- QEMU_BUILD_BUG_ON((ARMMMUIdx_Stage2 & 1) != 0);
- QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS + 1 != ARMMMUIdx_Phys_S);
- QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2 + 1 != ARMMMUIdx_Stage2_S);
- 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;
+ next_level:
+ descaddr |= (address >> (stride * (4 - level))) & indexmask;
+ descaddr &= ~7ULL;
+ nstable = extract32(tableattrs, 4, 1);
+ if (!nstable) {
/*
- * 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.
+ * Stage2_S -> Stage2 or Phys_S -> Phys_NS
+ * Assert that the non-secure idx are even, and relative order.
*/
- 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 (regime_is_stage2(mmu_idx)) {
- /* 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;
+ QEMU_BUILD_BUG_ON((ARMMMUIdx_Phys_NS & 1) != 0);
+ QEMU_BUILD_BUG_ON((ARMMMUIdx_Stage2 & 1) != 0);
+ QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS + 1 != ARMMMUIdx_Phys_S);
+ QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2 + 1 != ARMMMUIdx_Stage2_S);
+ 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 (regime_is_stage2(mmu_idx)) {
+ /* 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] 25+ messages in thread* [PATCH v6 08/14] target/arm: Fix fault reporting in get_phys_addr_lpae
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (6 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 07/14] target/arm: Remove loop from get_phys_addr_lpae Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 14:55 ` Alex Bennée
2022-10-24 5:18 ` [PATCH v6 09/14] target/arm: Don't shift attrs " Richard Henderson
` (6 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 615471699e..cd16b42c96 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1063,8 +1063,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;
@@ -1101,8 +1099,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;
@@ -1139,8 +1136,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;
}
}
@@ -1166,7 +1162,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 (!regime_is_stage2(mmu_idx)) {
@@ -1197,8 +1193,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) {
@@ -1217,8 +1212,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;
}
@@ -1240,7 +1234,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;
}
@@ -1301,7 +1295,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;
@@ -1319,7 +1313,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;
}
@@ -1376,9 +1370,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;
}
@@ -1395,8 +1389,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;
}
@@ -1441,8 +1435,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 || regime_is_stage2(mmu_idx);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 09/14] target/arm: Don't shift attrs in get_phys_addr_lpae
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (7 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 08/14] target/arm: Fix fault reporting in get_phys_addr_lpae Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 11:52 ` Philippe Mathieu-Daudé
2022-10-24 5:18 ` [PATCH v6 10/14] target/arm: Consider GP an attribute " Richard Henderson
` (5 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 cd16b42c96..cc7751218c 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1069,7 +1069,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);
@@ -1343,49 +1343,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 (regime_is_stage2(mmu_idx)) {
/* 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 (regime_is_stage2(mmu_idx)) {
ns = mmu_idx == ARMMMUIdx_Stage2;
- xn = extract32(attrs, 11, 2);
+ xn = extract64(attrs, 53, 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);
}
@@ -1410,10 +1409,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
if (regime_is_stage2(mmu_idx)) {
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;
@@ -1428,7 +1427,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] 25+ messages in thread* Re: [PATCH v6 09/14] target/arm: Don't shift attrs in get_phys_addr_lpae
2022-10-24 5:18 ` [PATCH v6 09/14] target/arm: Don't shift attrs " Richard Henderson
@ 2022-10-24 11:52 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-24 11:52 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-arm, Peter Maydell
On 24/10/22 07:18, Richard Henderson wrote:
> 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(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 10/14] target/arm: Consider GP an attribute in get_phys_addr_lpae
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (8 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 09/14] target/arm: Don't shift attrs " Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 15:06 ` Alex Bennée
2022-10-24 5:18 ` [PATCH v6 11/14] target/arm: Tidy merging of attributes from descriptor and table Richard Henderson
` (4 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Philippe Mathieu-Daudé, 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: Philippe Mathieu-Daudé <philmd@linaro.org>
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 cc7751218c..8004ca86df 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1077,7 +1077,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;
@@ -1343,7 +1342,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 (regime_is_stage2(mmu_idx)) {
/* Stage 2 table descriptors do not include any attribute fields */
@@ -1351,7 +1350,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;
@@ -1404,7 +1402,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 (regime_is_stage2(mmu_idx)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 10/14] target/arm: Consider GP an attribute in get_phys_addr_lpae
2022-10-24 5:18 ` [PATCH v6 10/14] target/arm: Consider GP an attribute " Richard Henderson
@ 2022-10-24 15:06 ` Alex Bennée
0 siblings, 0 replies; 25+ messages in thread
From: Alex Bennée @ 2022-10-24 15:06 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-arm, Philippe Mathieu-Daudé, Peter Maydell, qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> 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: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 11/14] target/arm: Tidy merging of attributes from descriptor and table
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (9 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 10/14] target/arm: Consider GP an attribute " Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 15:20 ` Alex Bennée
2022-10-24 5:18 ` [PATCH v6 12/14] target/arm: Implement FEAT_HAFDBS, access flag portion Richard Henderson
` (3 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm
Replace some gotos with some nested if statements.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/ptw.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8004ca86df..282828992e 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1341,27 +1341,25 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
page_size = (1ULL << ((stride * (4 - level)) + 3));
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(50, 14));
- if (regime_is_stage2(mmu_idx)) {
- /* Stage 2 table descriptors do not include any attribute fields */
- goto skip_attrs;
- }
- /* Merge in attributes from table descriptors */
- attrs |= nstable << 5; /* NS */
- if (param.hpd) {
- /* HPD disables all the table attributes except NSTable. */
- goto skip_attrs;
- }
- 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.
+ * Extract attributes from the descriptor, and apply table descriptors.
+ * Stage 2 table descriptors do not include any attribute fields.
+ * HPD disables all the table attributes except NSTable.
*/
- attrs &= ~(extract64(tableattrs, 2, 1) << 6); /* !APT[0] => AP[1] */
- attrs |= extract32(tableattrs, 3, 1) << 7; /* APT[1] => AP[2] */
- skip_attrs:
+ attrs = descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50, 14));
+ if (!regime_is_stage2(mmu_idx)) {
+ attrs |= nstable << 5; /* NS */
+ if (!param.hpd) {
+ 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 &= ~(extract64(tableattrs, 2, 1) << 6); /* !APT[0] => AP[1] */
+ attrs |= extract32(tableattrs, 3, 1) << 7; /* APT[1] => AP[2] */
+ }
+ }
/*
* Here descaddr is the final physical address, and attributes
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 12/14] target/arm: Implement FEAT_HAFDBS, access flag portion
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (10 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 11/14] target/arm: Tidy merging of attributes from descriptor and table Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 5:18 ` [PATCH v6 13/14] target/arm: Implement FEAT_HAFDBS, dirty bit portion Richard Henderson
` (2 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Peter Maydell
Perform the atomic update for hardware management of the access flag.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
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.
v5: Fix s1ns typo; incorporate i_yzsvv comment.
Move the AF update before attributes are extracted and merged.
Do permission check before pte writeback, per AArch64.S1Translate.
---
docs/system/arm/emulation.rst | 1 +
target/arm/cpu64.c | 1 +
target/arm/ptw.c | 176 +++++++++++++++++++++++++++++-----
3 files changed, 156 insertions(+), 22 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 282828992e..d9f89b6105 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;
@@ -238,6 +240,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
@@ -266,6 +270,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
pte_secure = is_secure;
}
ptw->out_host = NULL;
+ ptw->out_rw = false;
} else {
CPUTLBEntryFull *full;
int flags;
@@ -280,6 +285,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;
}
@@ -323,14 +329,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. */
@@ -356,15 +364,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 };
@@ -385,6 +403,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)
{
@@ -1077,7 +1180,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. */
@@ -1291,7 +1394,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;
@@ -1337,17 +1442,36 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
* 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.
+ *
+ * Afterward, descaddr is the final physical address.
*/
page_size = (1ULL << ((stride * (4 - level)) + 3));
descaddr &= ~(hwaddr)(page_size - 1);
descaddr |= (address & (page_size - 1));
+ if (likely(!ptw->in_debug)) {
+ /*
+ * Access flag.
+ * If HA is enabled, prepare to update the descriptor below.
+ * Otherwise, pass the access fault on to software.
+ */
+ if (!(descriptor & (1 << 10))) {
+ if (param.ha) {
+ new_descriptor |= 1 << 10; /* AF */
+ } else {
+ fi->type = ARMFault_AccessFlag;
+ goto do_fault;
+ }
+ }
+ }
+
/*
- * Extract attributes from the descriptor, and apply table descriptors.
- * Stage 2 table descriptors do not include any attribute fields.
- * HPD disables all the table attributes except NSTable.
+ * Extract attributes from the (modified) descriptor, and apply
+ * table descriptors. Stage 2 table descriptors do not include
+ * any attribute fields. HPD disables all the table attributes
+ * except NSTable.
*/
- attrs = descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50, 14));
+ attrs = new_descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50, 14));
if (!regime_is_stage2(mmu_idx)) {
attrs |= nstable << 5; /* NS */
if (!param.hpd) {
@@ -1361,18 +1485,7 @@ 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;
- }
-
ap = extract32(attrs, 6, 2);
-
if (regime_is_stage2(mmu_idx)) {
ns = mmu_idx == ARMMMUIdx_Stage2;
xn = extract64(attrs, 53, 2);
@@ -1389,6 +1502,25 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
goto do_fault;
}
+ /* 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;
+ }
+ /*
+ * 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.
+ */
+ if (new_descriptor != descriptor) {
+ descriptor = new_descriptor;
+ goto restart_atomic_update;
+ }
+ }
+
if (ns) {
/*
* The NS bit will (as required by the architecture) have no effect if
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 13/14] target/arm: Implement FEAT_HAFDBS, dirty bit portion
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (11 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 12/14] target/arm: Implement FEAT_HAFDBS, access flag portion Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-10-24 5:18 ` [PATCH v6 14/14] target/arm: Use the max page size in a 2-stage ptw Richard Henderson
2022-10-25 15:16 ` [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Peter Maydell
14 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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>
---
v5: Move the DB update before attributes are extracted and merged.
---
target/arm/cpu64.c | 2 +-
target/arm/ptw.c | 16 ++++++++++++++++
2 files changed, 17 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 d9f89b6105..d87757a700 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1463,6 +1463,22 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
goto do_fault;
}
}
+
+ /*
+ * Dirty Bit.
+ * If HD is enabled, pre-emptively set/clear the appropriate AP/S2AP
+ * bit for writeback. The actual write protection test may still be
+ * overridden by tableattrs, to be merged below.
+ */
+ if (param.hd
+ && extract64(descriptor, 51, 1) /* DBM */
+ && access_type == MMU_DATA_STORE) {
+ if (regime_is_stage2(mmu_idx)) {
+ new_descriptor |= 1ull << 7; /* set S2AP[1] */
+ } else {
+ new_descriptor &= ~(1ull << 7); /* clear AP[2] */
+ }
+ }
}
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 14/14] target/arm: Use the max page size in a 2-stage ptw
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (12 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 13/14] target/arm: Implement FEAT_HAFDBS, dirty bit portion Richard Henderson
@ 2022-10-24 5:18 ` Richard Henderson
2022-12-05 16:50 ` Peter Maydell
2022-10-25 15:16 ` [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Peter Maydell
14 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2022-10-24 5:18 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 d87757a700..b80a5c68ae 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2589,7 +2589,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;
@@ -2625,6 +2625,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));
@@ -2639,6 +2640,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] 25+ messages in thread* Re: [PATCH v6 14/14] target/arm: Use the max page size in a 2-stage ptw
2022-10-24 5:18 ` [PATCH v6 14/14] target/arm: Use the max page size in a 2-stage ptw Richard Henderson
@ 2022-12-05 16:50 ` Peter Maydell
2022-12-05 19:06 ` Richard Henderson
0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2022-12-05 16:50 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm, Marc Zyngier
On Mon, 24 Oct 2022 at 06:19, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> 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>
So when I was reviewing the v8R patchset I re-found this
change in the code, and had some questions about it that
I wasn't thinking about the first time...
> @@ -2639,6 +2640,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) {
Firstly, what if the lg_page_size is < TARGET_PAGE_SIZE ?
I think this can't happen for VMSA, but for PMSA it will
when the region (in either S1 or S2) is less than the page size
(in which case lg_page_size is 0). Presumably in this case we
want to set the result's lg_page_size to also be 0 to
preserve the "don't put this in the TLB" effect.
Secondly, how does this work for VMSA? Suppose that stage 1
is using 4K pages and stage 2 is using 64K pages. We will then
claim here that the result lg_page_size is 64K, but the
attributes and mapping in the result are only valid for
the 4K page that we looked up in stage 1 -- the surrounding
4K pages could have entirely different permissions/mapping.
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 14/14] target/arm: Use the max page size in a 2-stage ptw
2022-12-05 16:50 ` Peter Maydell
@ 2022-12-05 19:06 ` Richard Henderson
0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2022-12-05 19:06 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, qemu-arm, Marc Zyngier
On 12/5/22 10:50, Peter Maydell wrote:
>> @@ -2639,6 +2640,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) {
>
> Firstly, what if the lg_page_size is < TARGET_PAGE_SIZE ?
> I think this can't happen for VMSA, but for PMSA it will
> when the region (in either S1 or S2) is less than the page size
> (in which case lg_page_size is 0). Presumably in this case we
> want to set the result's lg_page_size to also be 0 to
> preserve the "don't put this in the TLB" effect.
Hmm, I hadn't considered that -- probably because I assumed a-profile. You're right that
we should preserve the "don't cache the result" behaviour.
> Secondly, how does this work for VMSA? Suppose that stage 1
> is using 4K pages and stage 2 is using 64K pages. We will then
> claim here that the result lg_page_size is 64K, but the
> attributes and mapping in the result are only valid for
> the 4K page that we looked up in stage 1 -- the surrounding
> 4K pages could have entirely different permissions/mapping.
This only works because the middle-end only registers one page, at TARGET_PAGE_SIZE.
But we need to record this as a large page, so that a flush of the (64k) stage2 page
address affects all of the (4k) stage1 page entries that it covers.
Perhaps it would be less confusing in N target/ implementations if we have two
lg_page_size structure members, and handle the unioning in the middle-end? Soliciting
suggestions for what to name such a beast (considering RME adds a stage3 lookup, and
associated page/granule sizes), and how to signal that it is or isn't used (presumably 0,
meaning that stageN+1 can't have a "don't record" setting).
r~
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS
2022-10-24 5:18 [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS Richard Henderson
` (13 preceding siblings ...)
2022-10-24 5:18 ` [PATCH v6 14/14] target/arm: Use the max page size in a 2-stage ptw Richard Henderson
@ 2022-10-25 15:16 ` Peter Maydell
14 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2022-10-25 15:16 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, qemu-arm
On Mon, 24 Oct 2022 at 06:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Changes for v6:
> * Fix rebase error wrt xn bit extract.
>
> Changes for v5:
> * Rebase, including 12 patches.
> * Add regime_is_stage2, which I should have done ages ago.
> * Reorg attribute extraction/merging vs descriptor modifications.
>
> Patches lacking review:
> 02-target-arm-Add-ptw_idx-to-S1Translate.patch
> 11-target-arm-Tidy-merging-of-attributes-from-descri.patch
> 13-target-arm-Implement-FEAT_HAFDBS-dirty-bit-portio.patch
Applied to target-arm.next, thanks.
(I know Alex asked for a comment on one of the patches; I think
we can do that as a follow-up patch.)
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread