* [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2
@ 2022-07-14 13:22 Peter Maydell
2022-07-14 13:22 ` [PATCH 1/7] target/arm: Define and use new regime_tcr_value() function Peter Maydell
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:22 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
In https://gitlab.com/qemu-project/qemu/-/issues/1103 Idan pointed
out that our regime_tcr() function doesn't handle the fact that some
of the config bits for the Secure stage 2 translation regime are
shared with NS EL2 and stored in VTCR_EL2 rather than VSTCR_EL2.
Currently the only visible effect of this is that if the guest tries
to enable LPA2 via VTCR_EL2.DS QEMU will incorrectly fail to honour
this for Secure stage 2 translations.
The final patch in this series fixes this, by synthesizing a
VTCR_EL2 format value for Secure stage 2 by merging together
the shared VTCR_EL2 fields into the VSTCR_EL2 value.
The first six patches get rid of a very longstanding
micro-optimization of our TCR register handling that gets in the way
of fixing the bug. Currently regime_tcr() returns a pointer to a TCR
struct, which contains both the 64-bit TCR value and also two
'base_mask' and 'mask' values, which are pre-calculated when the
register is written. Those values are used only in the
get_level1_table_address() function. That function is called once
per page table walk for 32-bit short-descriptor page tables only.
The pre-calculation saves only a handful of logical shifting and
masking instructions, which is a tiny amount of overhead compared to
everything else we need to do on a guest page table walk. Conversely
the microoptimization adds extra complexity on TCR register writes,
and code complexity handling the TCR struct. It therefore no longer
seems to be pulling its weight, and we're better off without it,
rather than jumping through hoops to come up with a fix to the
VSTCR_EL2 issue that lets us retain it.
thanks
-- PMM
Peter Maydell (7):
target/arm: Define and use new regime_tcr_value() function
target/arm: Calculate mask/base_mask in get_level1_table_address()
target/arm: Fold regime_tcr() and regime_tcr_value() together
target/arm: Fix big-endian host handling of VTCR
target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t
target/arm: Store TCR_EL* registers as uint64_t
target/arm: Honour VTCR_EL2 bits in Secure EL2
target/arm/cpu.h | 31 ++++++++++----
target/arm/internals.h | 34 +++++++++++----
target/arm/cpu.c | 2 +-
target/arm/debug_helper.c | 2 +-
target/arm/helper.c | 87 +++++++++++----------------------------
target/arm/ptw.c | 38 +++++++++--------
target/arm/tlb_helper.c | 2 +-
7 files changed, 96 insertions(+), 100 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/7] target/arm: Define and use new regime_tcr_value() function
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
@ 2022-07-14 13:22 ` Peter Maydell
2022-07-14 23:12 ` Richard Henderson
2022-07-14 13:22 ` [PATCH 2/7] target/arm: Calculate mask/base_mask in get_level1_table_address() Peter Maydell
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:22 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
The regime_tcr() function returns a pointer to a struct TCR
corresponding to the TCR controlling a translation regime. The
struct TCR has the raw value of the register, plus two fields mask
and base_mask which are used as a small optimization in the case of
32-bit short-descriptor lookups. Almost all callers of regime_tcr()
only want the raw register value. Define and use a new
regime_tcr_value() function which returns only the raw 64-bit
register value.
This is a preliminary to removing the 32-bit short descriptor
optimization -- it only saves a handful of bit operations, which is
tiny compared to the overhead of doing a page table walk at all, and
the TCR struct is awkward and makes fixing
https://gitlab.com/qemu-project/qemu/-/issues/1103 unnecessarily
difficult.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/internals.h | 6 ++++++
target/arm/helper.c | 6 +++---
target/arm/ptw.c | 8 ++++----
target/arm/tlb_helper.c | 2 +-
4 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 00e2e710f6c..fa046124fa8 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -793,6 +793,12 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
}
+/* Return the raw value of the TCR controlling this translation regime */
+static inline uint64_t regime_tcr_value(CPUARMState *env, ARMMMUIdx mmu_idx)
+{
+ return regime_tcr(env, mmu_idx)->raw_tcr;
+}
+
/**
* arm_num_brps: Return number of implemented breakpoints.
* Note that the ID register BRPS field is "number of bps - 1",
diff --git a/target/arm/helper.c b/target/arm/helper.c
index cfcad97ce07..b45c81c714c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4216,7 +4216,7 @@ static int vae1_tlbmask(CPUARMState *env)
static int tlbbits_for_regime(CPUARMState *env, ARMMMUIdx mmu_idx,
uint64_t addr)
{
- uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+ uint64_t tcr = regime_tcr_value(env, mmu_idx);
int tbi = aa64_va_parameter_tbi(tcr, mmu_idx);
int select = extract64(addr, 55, 1);
@@ -10158,7 +10158,7 @@ static int aa64_va_parameter_tcma(uint64_t tcr, ARMMMUIdx mmu_idx)
ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
ARMMMUIdx mmu_idx, bool data)
{
- uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+ uint64_t tcr = regime_tcr_value(env, mmu_idx);
bool epd, hpd, using16k, using64k, tsz_oob, ds;
int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
ARMCPU *cpu = env_archcpu(env);
@@ -10849,7 +10849,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
{
CPUARMTBFlags flags = {};
ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx);
- uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+ uint64_t tcr = regime_tcr_value(env, mmu_idx);
uint64_t sctlr;
int tbii, tbid;
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index e71fc1f4293..0d7e8ffa41b 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -820,7 +820,7 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
ARMMMUIdx mmu_idx)
{
- uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+ uint64_t tcr = regime_tcr_value(env, mmu_idx);
uint32_t el = regime_el(env, mmu_idx);
int select, tsz;
bool epd, hpd;
@@ -994,7 +994,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
uint32_t attrs;
int32_t stride;
int addrsize, inputsize, outputsize;
- TCR *tcr = regime_tcr(env, mmu_idx);
+ uint64_t tcr = regime_tcr_value(env, mmu_idx);
int ap, ns, xn, pxn;
uint32_t el = regime_el(env, mmu_idx);
uint64_t descaddrmask;
@@ -1112,8 +1112,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
* For stage 2 translations the starting level is specified by the
* VTCR_EL2.SL0 field (whose interpretation depends on the page size)
*/
- uint32_t sl0 = extract32(tcr->raw_tcr, 6, 2);
- uint32_t sl2 = extract64(tcr->raw_tcr, 33, 1);
+ uint32_t sl0 = extract32(tcr, 6, 2);
+ uint32_t sl2 = extract64(tcr, 33, 1);
uint32_t startlevel;
bool ok;
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 7d8a86b3c45..a2f87a5042d 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -20,7 +20,7 @@ bool regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
return true;
}
if (arm_feature(env, ARM_FEATURE_LPAE)
- && (regime_tcr(env, mmu_idx)->raw_tcr & TTBCR_EAE)) {
+ && (regime_tcr_value(env, mmu_idx) & TTBCR_EAE)) {
return true;
}
return false;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/7] target/arm: Calculate mask/base_mask in get_level1_table_address()
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
2022-07-14 13:22 ` [PATCH 1/7] target/arm: Define and use new regime_tcr_value() function Peter Maydell
@ 2022-07-14 13:22 ` Peter Maydell
2022-07-14 23:18 ` Richard Henderson
2022-07-14 13:22 ` [PATCH 3/7] target/arm: Fold regime_tcr() and regime_tcr_value() together Peter Maydell
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:22 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
In get_level1_table_address(), instead of using precalculated values
of mask and base_mask from the TCR struct, calculate them directly
(in the same way we currently do in vmsa_ttbcr_raw_write() to
populate the TCR struct fields).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/ptw.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 0d7e8ffa41b..16226d14233 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -315,20 +315,24 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
uint32_t *table, uint32_t address)
{
/* Note that we can only get here for an AArch32 PL0/PL1 lookup */
- TCR *tcr = regime_tcr(env, mmu_idx);
+ uint64_t tcr = regime_tcr_value(env, mmu_idx);
+ int maskshift = extract32(tcr, 0, 3);
+ uint32_t mask = ~(((uint32_t)0xffffffffu) >> maskshift);
+ uint32_t base_mask;
- if (address & tcr->mask) {
- if (tcr->raw_tcr & TTBCR_PD1) {
+ if (address & mask) {
+ if (tcr & TTBCR_PD1) {
/* Translation table walk disabled for TTBR1 */
return false;
}
*table = regime_ttbr(env, mmu_idx, 1) & 0xffffc000;
} else {
- if (tcr->raw_tcr & TTBCR_PD0) {
+ if (tcr & TTBCR_PD0) {
/* Translation table walk disabled for TTBR0 */
return false;
}
- *table = regime_ttbr(env, mmu_idx, 0) & tcr->base_mask;
+ base_mask = ~((uint32_t)0x3fffu >> maskshift);
+ *table = regime_ttbr(env, mmu_idx, 0) & base_mask;
}
*table |= (address >> 18) & 0x3ffc;
return true;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/7] target/arm: Fold regime_tcr() and regime_tcr_value() together
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
2022-07-14 13:22 ` [PATCH 1/7] target/arm: Define and use new regime_tcr_value() function Peter Maydell
2022-07-14 13:22 ` [PATCH 2/7] target/arm: Calculate mask/base_mask in get_level1_table_address() Peter Maydell
@ 2022-07-14 13:22 ` Peter Maydell
2022-07-14 23:18 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 4/7] target/arm: Fix big-endian host handling of VTCR Peter Maydell
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:22 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
The only caller of regime_tcr() is now regime_tcr_value(); fold the
two together, and use the shorter and more natural 'regime_tcr'
name for the new function.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/internals.h | 16 +++++-----------
target/arm/helper.c | 6 +++---
target/arm/ptw.c | 6 +++---
target/arm/tlb_helper.c | 2 +-
4 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index fa046124fa8..0a1eb20afce 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -777,26 +777,20 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
return env->cp15.sctlr_el[regime_el(env, mmu_idx)];
}
-/* Return the TCR controlling this translation regime */
-static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
+/* Return the value of the TCR controlling this translation regime */
+static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
{
if (mmu_idx == ARMMMUIdx_Stage2) {
- return &env->cp15.vtcr_el2;
+ return env->cp15.vtcr_el2.raw_tcr;
}
if (mmu_idx == ARMMMUIdx_Stage2_S) {
/*
* Note: Secure stage 2 nominally shares fields from VTCR_EL2, but
* those are not currently used by QEMU, so just return VSTCR_EL2.
*/
- return &env->cp15.vstcr_el2;
+ return env->cp15.vstcr_el2.raw_tcr;
}
- return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
-}
-
-/* Return the raw value of the TCR controlling this translation regime */
-static inline uint64_t regime_tcr_value(CPUARMState *env, ARMMMUIdx mmu_idx)
-{
- return regime_tcr(env, mmu_idx)->raw_tcr;
+ return env->cp15.tcr_el[regime_el(env, mmu_idx)].raw_tcr;
}
/**
diff --git a/target/arm/helper.c b/target/arm/helper.c
index b45c81c714c..3d4317c4c85 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4216,7 +4216,7 @@ static int vae1_tlbmask(CPUARMState *env)
static int tlbbits_for_regime(CPUARMState *env, ARMMMUIdx mmu_idx,
uint64_t addr)
{
- uint64_t tcr = regime_tcr_value(env, mmu_idx);
+ uint64_t tcr = regime_tcr(env, mmu_idx);
int tbi = aa64_va_parameter_tbi(tcr, mmu_idx);
int select = extract64(addr, 55, 1);
@@ -10158,7 +10158,7 @@ static int aa64_va_parameter_tcma(uint64_t tcr, ARMMMUIdx mmu_idx)
ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va,
ARMMMUIdx mmu_idx, bool data)
{
- uint64_t tcr = regime_tcr_value(env, mmu_idx);
+ uint64_t tcr = regime_tcr(env, mmu_idx);
bool epd, hpd, using16k, using64k, tsz_oob, ds;
int select, tsz, tbi, max_tsz, min_tsz, ps, sh;
ARMCPU *cpu = env_archcpu(env);
@@ -10849,7 +10849,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
{
CPUARMTBFlags flags = {};
ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx);
- uint64_t tcr = regime_tcr_value(env, mmu_idx);
+ uint64_t tcr = regime_tcr(env, mmu_idx);
uint64_t sctlr;
int tbii, tbid;
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 16226d14233..e9959848d88 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -315,7 +315,7 @@ static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
uint32_t *table, uint32_t address)
{
/* Note that we can only get here for an AArch32 PL0/PL1 lookup */
- uint64_t tcr = regime_tcr_value(env, mmu_idx);
+ uint64_t tcr = regime_tcr(env, mmu_idx);
int maskshift = extract32(tcr, 0, 3);
uint32_t mask = ~(((uint32_t)0xffffffffu) >> maskshift);
uint32_t base_mask;
@@ -824,7 +824,7 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va,
ARMMMUIdx mmu_idx)
{
- uint64_t tcr = regime_tcr_value(env, mmu_idx);
+ uint64_t tcr = regime_tcr(env, mmu_idx);
uint32_t el = regime_el(env, mmu_idx);
int select, tsz;
bool epd, hpd;
@@ -998,7 +998,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, uint64_t address,
uint32_t attrs;
int32_t stride;
int addrsize, inputsize, outputsize;
- uint64_t tcr = regime_tcr_value(env, mmu_idx);
+ uint64_t tcr = regime_tcr(env, mmu_idx);
int ap, ns, xn, pxn;
uint32_t el = regime_el(env, mmu_idx);
uint64_t descaddrmask;
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index a2f87a5042d..5a709eab56f 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -20,7 +20,7 @@ bool regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
return true;
}
if (arm_feature(env, ARM_FEATURE_LPAE)
- && (regime_tcr_value(env, mmu_idx) & TTBCR_EAE)) {
+ && (regime_tcr(env, mmu_idx) & TTBCR_EAE)) {
return true;
}
return false;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/7] target/arm: Fix big-endian host handling of VTCR
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
` (2 preceding siblings ...)
2022-07-14 13:22 ` [PATCH 3/7] target/arm: Fold regime_tcr() and regime_tcr_value() together Peter Maydell
@ 2022-07-14 13:23 ` Peter Maydell
2022-07-14 23:19 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 5/7] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t Peter Maydell
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:23 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
We have a bug in our handling of accesses to the AArch32 VTCR
register on big-endian hosts: we were not adjusting the part of the
uint64_t field within TCR that the generated code would access. That
can be done with offsetoflow32(), by using an ARM_CP_STATE_BOTH cpreg
struct, or by defining a full set of read/write/reset functions --
the various other TCR cpreg structs used one or another of those
strategies, but for VTCR we did not, so on a big-endian host VTCR
accesses would touch the wrong half of the register.
Use offsetoflow32() in the VTCR register struct. This works even
though the field in the CPU struct is currently a struct TCR, because
the first field in that struct is the uint64_t raw_tcr.
None of the other TCR registers have this bug -- either they are
AArch64 only, or else they define resetfn, writefn, etc, and
expect to be passed the full struct pointer.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Actually I'm not 100% sure that TTBCR is handled correctly for
big-endian hosts. But it's going to go away shortly anyway.
---
target/arm/helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3d4317c4c85..7eee2007a0e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5409,7 +5409,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
.cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
.type = ARM_CP_ALIAS,
.access = PL2_RW, .accessfn = access_el3_aa32ns,
- .fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
+ .fieldoffset = offsetoflow32(CPUARMState, cp15.vtcr_el2) },
{ .name = "VTCR_EL2", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
.access = PL2_RW,
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/7] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
` (3 preceding siblings ...)
2022-07-14 13:23 ` [PATCH 4/7] target/arm: Fix big-endian host handling of VTCR Peter Maydell
@ 2022-07-14 13:23 ` Peter Maydell
2022-07-14 23:21 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 6/7] target/arm: Store TCR_EL* " Peter Maydell
2022-07-14 13:23 ` [PATCH 7/7] target/arm: Honour VTCR_EL2 bits in Secure EL2 Peter Maydell
6 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:23 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
Change the representation of the VSTCR_EL2 and VTCR_EL2 registers in
the CPU state struct from struct TCR to uint64_t.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 4 ++--
target/arm/internals.h | 4 ++--
target/arm/helper.c | 4 +---
target/arm/ptw.c | 14 +++++++-------
4 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1e36a839ee4..445e477c710 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -340,8 +340,8 @@ typedef struct CPUArchState {
uint64_t vsttbr_el2; /* Secure Virtualization Translation Table. */
/* MMU translation table base control. */
TCR tcr_el[4];
- TCR vtcr_el2; /* Virtualization Translation Control. */
- TCR vstcr_el2; /* Secure Virtualization Translation Control. */
+ uint64_t vtcr_el2; /* Virtualization Translation Control. */
+ uint64_t vstcr_el2; /* Secure Virtualization Translation Control. */
uint32_t c2_data; /* MPU data cacheable bits. */
uint32_t c2_insn; /* MPU instruction cacheable bits. */
union { /* MMU domain access control register
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 0a1eb20afce..9f654b12cea 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -781,14 +781,14 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
{
if (mmu_idx == ARMMMUIdx_Stage2) {
- return env->cp15.vtcr_el2.raw_tcr;
+ return env->cp15.vtcr_el2;
}
if (mmu_idx == ARMMMUIdx_Stage2_S) {
/*
* Note: Secure stage 2 nominally shares fields from VTCR_EL2, but
* those are not currently used by QEMU, so just return VSTCR_EL2.
*/
- return env->cp15.vstcr_el2.raw_tcr;
+ return env->cp15.vstcr_el2;
}
return env->cp15.tcr_el[regime_el(env, mmu_idx)].raw_tcr;
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7eee2007a0e..eaf6521c615 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5413,9 +5413,7 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
{ .name = "VTCR_EL2", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
.access = PL2_RW,
- /* no .writefn needed as this can't cause an ASID change;
- * no .raw_writefn or .resetfn needed as we never use mask/base_mask
- */
+ /* no .writefn needed as this can't cause an ASID change */
.fieldoffset = offsetof(CPUARMState, cp15.vtcr_el2) },
{ .name = "VTTBR", .state = ARM_CP_STATE_AA32,
.cp = 15, .opc1 = 6, .crm = 2,
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index e9959848d88..8049c67f039 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -241,9 +241,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
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.raw_tcr & VSTCR_SW);
+ *is_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
} else {
- *is_secure = !(env->cp15.vtcr_el2.raw_tcr & VTCR_NSW);
+ *is_secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
}
} else {
assert(!*is_secure);
@@ -2341,9 +2341,9 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
ipa_secure = attrs->secure;
if (arm_is_secure_below_el3(env)) {
if (ipa_secure) {
- attrs->secure = !(env->cp15.vstcr_el2.raw_tcr & VSTCR_SW);
+ attrs->secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
} else {
- attrs->secure = !(env->cp15.vtcr_el2.raw_tcr & VTCR_NSW);
+ attrs->secure = !(env->cp15.vtcr_el2 & VTCR_NSW);
}
} else {
assert(!ipa_secure);
@@ -2385,11 +2385,11 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
if (arm_is_secure_below_el3(env)) {
if (ipa_secure) {
attrs->secure =
- !(env->cp15.vstcr_el2.raw_tcr & (VSTCR_SA | VSTCR_SW));
+ !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW));
} else {
attrs->secure =
- !((env->cp15.vtcr_el2.raw_tcr & (VTCR_NSA | VTCR_NSW))
- || (env->cp15.vstcr_el2.raw_tcr & (VSTCR_SA | VSTCR_SW)));
+ !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))
+ || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)));
}
}
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/7] target/arm: Store TCR_EL* registers as uint64_t
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
` (4 preceding siblings ...)
2022-07-14 13:23 ` [PATCH 5/7] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t Peter Maydell
@ 2022-07-14 13:23 ` Peter Maydell
2022-07-14 23:23 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 7/7] target/arm: Honour VTCR_EL2 bits in Secure EL2 Peter Maydell
6 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:23 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
Change the representation of the TCR_EL* registers in the CPU state
struct from struct TCR to uint64_t. This allows us to drop the
custom vmsa_ttbcr_raw_write() function, moving the "enforce RES0"
checks to their more usual location in the writefn
vmsa_ttbcr_write(). We also don't need the resetfn any more.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 8 +----
target/arm/internals.h | 6 ++--
target/arm/cpu.c | 2 +-
target/arm/debug_helper.c | 2 +-
target/arm/helper.c | 75 +++++++++++----------------------------
target/arm/ptw.c | 2 +-
6 files changed, 27 insertions(+), 68 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 445e477c710..bbd1afa6251 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -166,12 +166,6 @@ typedef struct ARMGenericTimer {
#define GTIMER_HYPVIRT 4
#define NUM_GTIMERS 5
-typedef struct {
- uint64_t raw_tcr;
- uint32_t mask;
- uint32_t base_mask;
-} TCR;
-
#define VTCR_NSW (1u << 29)
#define VTCR_NSA (1u << 30)
#define VSTCR_SW VTCR_NSW
@@ -339,7 +333,7 @@ typedef struct CPUArchState {
uint64_t vttbr_el2; /* Virtualization Translation Table Base. */
uint64_t vsttbr_el2; /* Secure Virtualization Translation Table. */
/* MMU translation table base control. */
- TCR tcr_el[4];
+ uint64_t tcr_el[4];
uint64_t vtcr_el2; /* Virtualization Translation Control. */
uint64_t vstcr_el2; /* Secure Virtualization Translation Control. */
uint32_t c2_data; /* MPU data cacheable bits. */
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 9f654b12cea..742135ef146 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -252,9 +252,9 @@ unsigned int arm_pamax(ARMCPU *cpu);
*/
static inline bool extended_addresses_enabled(CPUARMState *env)
{
- TCR *tcr = &env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1];
+ uint64_t tcr = env->cp15.tcr_el[arm_is_secure(env) ? 3 : 1];
return arm_el_is_aa64(env, 1) ||
- (arm_feature(env, ARM_FEATURE_LPAE) && (tcr->raw_tcr & TTBCR_EAE));
+ (arm_feature(env, ARM_FEATURE_LPAE) && (tcr & TTBCR_EAE));
}
/* Update a QEMU watchpoint based on the information the guest has set in the
@@ -790,7 +790,7 @@ static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
*/
return env->cp15.vstcr_el2;
}
- return env->cp15.tcr_el[regime_el(env, mmu_idx)].raw_tcr;
+ return env->cp15.tcr_el[regime_el(env, mmu_idx)];
}
/**
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5de7e097e9b..1b7b3d76bb3 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -226,7 +226,7 @@ static void arm_cpu_reset(DeviceState *dev)
* Enable TBI0 but not TBI1.
* Note that this must match useronly_clean_ptr.
*/
- env->cp15.tcr_el[1].raw_tcr = 5 | (1ULL << 37);
+ env->cp15.tcr_el[1] = 5 | (1ULL << 37);
/* Enable MTE */
if (cpu_isar_feature(aa64_mte, cpu)) {
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index d09fccb0a4f..c21739242c5 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -439,7 +439,7 @@ static uint32_t arm_debug_exception_fsr(CPUARMState *env)
using_lpae = true;
} else {
if (arm_feature(env, ARM_FEATURE_LPAE) &&
- (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) {
+ (env->cp15.tcr_el[target_el] & TTBCR_EAE)) {
using_lpae = true;
}
}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index eaf6521c615..51e58e08468 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3606,19 +3606,21 @@ static const ARMCPRegInfo pmsav5_cp_reginfo[] = {
.fieldoffset = offsetof(CPUARMState, cp15.c6_region[7]) },
};
-static void vmsa_ttbcr_raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
- uint64_t value)
+static void vmsa_ttbcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
{
- TCR *tcr = raw_ptr(env, ri);
- int maskshift = extract32(value, 0, 3);
+ ARMCPU *cpu = env_archcpu(env);
if (!arm_feature(env, ARM_FEATURE_V8)) {
if (arm_feature(env, ARM_FEATURE_LPAE) && (value & TTBCR_EAE)) {
- /* Pre ARMv8 bits [21:19], [15:14] and [6:3] are UNK/SBZP when
- * using Long-desciptor translation table format */
+ /*
+ * Pre ARMv8 bits [21:19], [15:14] and [6:3] are UNK/SBZP when
+ * using Long-descriptor translation table format
+ */
value &= ~((7 << 19) | (3 << 14) | (0xf << 3));
} else if (arm_feature(env, ARM_FEATURE_EL3)) {
- /* In an implementation that includes the Security Extensions
+ /*
+ * In an implementation that includes the Security Extensions
* TTBCR has additional fields PD0 [4] and PD1 [5] for
* Short-descriptor translation table format.
*/
@@ -3628,55 +3630,23 @@ static void vmsa_ttbcr_raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
}
}
- /* Update the masks corresponding to the TCR bank being written
- * Note that we always calculate mask and base_mask, but
- * they are only used for short-descriptor tables (ie if EAE is 0);
- * for long-descriptor tables the TCR fields are used differently
- * and the mask and base_mask values are meaningless.
- */
- tcr->raw_tcr = value;
- tcr->mask = ~(((uint32_t)0xffffffffu) >> maskshift);
- tcr->base_mask = ~((uint32_t)0x3fffu >> maskshift);
-}
-
-static void vmsa_ttbcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
- uint64_t value)
-{
- ARMCPU *cpu = env_archcpu(env);
- TCR *tcr = raw_ptr(env, ri);
-
if (arm_feature(env, ARM_FEATURE_LPAE)) {
/* With LPAE the TTBCR could result in a change of ASID
* via the TTBCR.A1 bit, so do a TLB flush.
*/
tlb_flush(CPU(cpu));
}
- /* Preserve the high half of TCR_EL1, set via TTBCR2. */
- value = deposit64(tcr->raw_tcr, 0, 32, value);
- vmsa_ttbcr_raw_write(env, ri, value);
-}
-
-static void vmsa_ttbcr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
-{
- TCR *tcr = raw_ptr(env, ri);
-
- /* Reset both the TCR as well as the masks corresponding to the bank of
- * the TCR being reset.
- */
- tcr->raw_tcr = 0;
- tcr->mask = 0;
- tcr->base_mask = 0xffffc000u;
+ raw_write(env, ri, value);
}
static void vmsa_tcr_el12_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
ARMCPU *cpu = env_archcpu(env);
- TCR *tcr = raw_ptr(env, ri);
/* For AArch64 the A1 bit could result in a change of ASID, so TLB flush. */
tlb_flush(CPU(cpu));
- tcr->raw_tcr = value;
+ raw_write(env, ri, value);
}
static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3780,15 +3750,15 @@ static const ARMCPRegInfo vmsa_cp_reginfo[] = {
.opc0 = 3, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 2,
.access = PL1_RW, .accessfn = access_tvm_trvm,
.writefn = vmsa_tcr_el12_write,
- .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
+ .raw_writefn = raw_write,
+ .resetvalue = 0,
.fieldoffset = offsetof(CPUARMState, cp15.tcr_el[1]) },
{ .name = "TTBCR", .cp = 15, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 2,
.access = PL1_RW, .accessfn = access_tvm_trvm,
.type = ARM_CP_ALIAS, .writefn = vmsa_ttbcr_write,
- .raw_writefn = vmsa_ttbcr_raw_write,
- /* No offsetoflow32 -- pass the entire TCR to writefn/raw_writefn. */
- .bank_fieldoffsets = { offsetof(CPUARMState, cp15.tcr_el[3]),
- offsetof(CPUARMState, cp15.tcr_el[1])} },
+ .raw_writefn = raw_write,
+ .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.tcr_el[3]),
+ offsetoflow32(CPUARMState, cp15.tcr_el[1])} },
};
/* Note that unlike TTBCR, writing to TTBCR2 does not require flushing
@@ -3799,8 +3769,8 @@ static const ARMCPRegInfo ttbcr2_reginfo = {
.access = PL1_RW, .accessfn = access_tvm_trvm,
.type = ARM_CP_ALIAS,
.bank_fieldoffsets = {
- offsetofhigh32(CPUARMState, cp15.tcr_el[3].raw_tcr),
- offsetofhigh32(CPUARMState, cp15.tcr_el[1].raw_tcr),
+ offsetofhigh32(CPUARMState, cp15.tcr_el[3]),
+ offsetofhigh32(CPUARMState, cp15.tcr_el[1]),
},
};
@@ -5403,7 +5373,6 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
{ .name = "TCR_EL2", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 4, .crn = 2, .crm = 0, .opc2 = 2,
.access = PL2_RW, .writefn = vmsa_tcr_el12_write,
- /* no .raw_writefn or .resetfn needed as we never use mask/base_mask */
.fieldoffset = offsetof(CPUARMState, cp15.tcr_el[2]) },
{ .name = "VTCR", .state = ARM_CP_STATE_AA32,
.cp = 15, .opc1 = 4, .crn = 2, .crm = 1, .opc2 = 2,
@@ -5643,12 +5612,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
{ .name = "TCR_EL3", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 6, .crn = 2, .crm = 0, .opc2 = 2,
.access = PL3_RW,
- /* no .writefn needed as this can't cause an ASID change;
- * we must provide a .raw_writefn and .resetfn because we handle
- * reset and migration for the AArch32 TTBCR(S), which might be
- * using mask and base_mask.
- */
- .resetfn = vmsa_ttbcr_reset, .raw_writefn = vmsa_ttbcr_raw_write,
+ /* no .writefn needed as this can't cause an ASID change */
+ .resetvalue = 0,
.fieldoffset = offsetof(CPUARMState, cp15.tcr_el[3]) },
{ .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
.type = ARM_CP_ALIAS,
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8049c67f039..3261039d93a 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -2466,7 +2466,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
int 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].raw_tcr;
+ uint64_t tcr = env->cp15.tcr_el[r_el];
int addrtop, tbi;
tbi = aa64_va_parameter_tbi(tcr, mmu_idx);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 7/7] target/arm: Honour VTCR_EL2 bits in Secure EL2
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
` (5 preceding siblings ...)
2022-07-14 13:23 ` [PATCH 6/7] target/arm: Store TCR_EL* " Peter Maydell
@ 2022-07-14 13:23 ` Peter Maydell
2022-07-14 23:25 ` Richard Henderson
6 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2022-07-14 13:23 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Idan Horowitz
In regime_tcr() we return the appropriate TCR register for the
translation regime. For Secure EL2, we return the VSTCR_EL2 value,
but in this translation regime some fields that control behaviour are
in VTCR_EL2. When this code was originally written (as the comment
notes), QEMU didn't care about any of those fields, but we have since
added support for features such as LPA2 which do need the values from
those fields.
Synthesize a TCR value by merging in the relevant VTCR_EL2 fields to
the VSTCR_EL2 value.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1103
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 19 +++++++++++++++++++
target/arm/internals.h | 22 +++++++++++++++++++---
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bbd1afa6251..57b5dd1f70b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1412,6 +1412,25 @@ FIELD(CPTR_EL3, TCPAC, 31, 1)
#define TTBCR_SH1 (1U << 28)
#define TTBCR_EAE (1U << 31)
+FIELD(VTCR, T0SZ, 0, 6)
+FIELD(VTCR, SL0, 6, 2)
+FIELD(VTCR, IRGN0, 8, 2)
+FIELD(VTCR, ORGN0, 10, 2)
+FIELD(VTCR, SH0, 12, 2)
+FIELD(VTCR, TG0, 14, 2)
+FIELD(VTCR, PS, 16, 3)
+FIELD(VTCR, VS, 19, 1)
+FIELD(VTCR, HA, 21, 1)
+FIELD(VTCR, HD, 22, 1)
+FIELD(VTCR, HWU59, 25, 1)
+FIELD(VTCR, HWU60, 26, 1)
+FIELD(VTCR, HWU61, 27, 1)
+FIELD(VTCR, HWU62, 28, 1)
+FIELD(VTCR, NSW, 29, 1)
+FIELD(VTCR, NSA, 30, 1)
+FIELD(VTCR, DS, 32, 1)
+FIELD(VTCR, SL2, 33, 1)
+
/* Bit definitions for ARMv8 SPSR (PSTATE) format.
* Only these are valid when in AArch64 mode; in
* AArch32 mode SPSRs are basically CPSR-format.
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 742135ef146..b8fefdff675 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -777,6 +777,16 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
return env->cp15.sctlr_el[regime_el(env, mmu_idx)];
}
+/*
+ * These are the fields in VTCR_EL2 which affect both the Secure stage 2
+ * and the Non-Secure stage 2 translation regimes (and hence which are
+ * not present in VSTCR_EL2).
+ */
+#define VTCR_SHARED_FIELD_MASK \
+ (R_VTCR_IRGN0_MASK | R_VTCR_ORGN0_MASK | R_VTCR_SH0_MASK | \
+ R_VTCR_PS_MASK | R_VTCR_VS_MASK | R_VTCR_HA_MASK | R_VTCR_HD_MASK | \
+ R_VTCR_DS_MASK)
+
/* Return the value of the TCR controlling this translation regime */
static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
{
@@ -785,10 +795,16 @@ static inline uint64_t regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
}
if (mmu_idx == ARMMMUIdx_Stage2_S) {
/*
- * Note: Secure stage 2 nominally shares fields from VTCR_EL2, but
- * those are not currently used by QEMU, so just return VSTCR_EL2.
+ * Secure stage 2 shares fields from VTCR_EL2. We merge those
+ * in with the VSTCR_EL2 value to synthesize a single VTCR_EL2 format
+ * value so the callers don't need to special case this.
+ *
+ * If a future architecture change defines bits in VSTCR_EL2 that
+ * overlap with these VTCR_EL2 fields we may need to revisit this.
*/
- return env->cp15.vstcr_el2;
+ uint64_t v = env->cp15.vstcr_el2 & ~VTCR_SHARED_FIELD_MASK;
+ v |= env->cp15.vtcr_el2 & VTCR_SHARED_FIELD_MASK;
+ return v;
}
return env->cp15.tcr_el[regime_el(env, mmu_idx)];
}
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/7] target/arm: Define and use new regime_tcr_value() function
2022-07-14 13:22 ` [PATCH 1/7] target/arm: Define and use new regime_tcr_value() function Peter Maydell
@ 2022-07-14 23:12 ` Richard Henderson
0 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2022-07-14 23:12 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Idan Horowitz
On 7/14/22 18:52, Peter Maydell wrote:
> The regime_tcr() function returns a pointer to a struct TCR
> corresponding to the TCR controlling a translation regime. The
> struct TCR has the raw value of the register, plus two fields mask
> and base_mask which are used as a small optimization in the case of
> 32-bit short-descriptor lookups. Almost all callers of regime_tcr()
> only want the raw register value. Define and use a new
> regime_tcr_value() function which returns only the raw 64-bit
> register value.
>
> This is a preliminary to removing the 32-bit short descriptor
> optimization -- it only saves a handful of bit operations, which is
> tiny compared to the overhead of doing a page table walk at all, and
> the TCR struct is awkward and makes fixing
> https://gitlab.com/qemu-project/qemu/-/issues/1103 unnecessarily
> difficult.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> target/arm/internals.h | 6 ++++++
> target/arm/helper.c | 6 +++---
> target/arm/ptw.c | 8 ++++----
> target/arm/tlb_helper.c | 2 +-
> 4 files changed, 14 insertions(+), 8 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/7] target/arm: Calculate mask/base_mask in get_level1_table_address()
2022-07-14 13:22 ` [PATCH 2/7] target/arm: Calculate mask/base_mask in get_level1_table_address() Peter Maydell
@ 2022-07-14 23:18 ` Richard Henderson
0 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2022-07-14 23:18 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Idan Horowitz
On 7/14/22 18:52, Peter Maydell wrote:
> In get_level1_table_address(), instead of using precalculated values
> of mask and base_mask from the TCR struct, calculate them directly
> (in the same way we currently do in vmsa_ttbcr_raw_write() to
> populate the TCR struct fields).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/7] target/arm: Fold regime_tcr() and regime_tcr_value() together
2022-07-14 13:22 ` [PATCH 3/7] target/arm: Fold regime_tcr() and regime_tcr_value() together Peter Maydell
@ 2022-07-14 23:18 ` Richard Henderson
0 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2022-07-14 23:18 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Idan Horowitz
On 7/14/22 18:52, Peter Maydell wrote:
> The only caller of regime_tcr() is now regime_tcr_value(); fold the
> two together, and use the shorter and more natural 'regime_tcr'
> name for the new function.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> target/arm/internals.h | 16 +++++-----------
> target/arm/helper.c | 6 +++---
> target/arm/ptw.c | 6 +++---
> target/arm/tlb_helper.c | 2 +-
> 4 files changed, 12 insertions(+), 18 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] target/arm: Fix big-endian host handling of VTCR
2022-07-14 13:23 ` [PATCH 4/7] target/arm: Fix big-endian host handling of VTCR Peter Maydell
@ 2022-07-14 23:19 ` Richard Henderson
0 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2022-07-14 23:19 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Idan Horowitz
On 7/14/22 18:53, Peter Maydell wrote:
> We have a bug in our handling of accesses to the AArch32 VTCR
> register on big-endian hosts: we were not adjusting the part of the
> uint64_t field within TCR that the generated code would access. That
> can be done with offsetoflow32(), by using an ARM_CP_STATE_BOTH cpreg
> struct, or by defining a full set of read/write/reset functions --
> the various other TCR cpreg structs used one or another of those
> strategies, but for VTCR we did not, so on a big-endian host VTCR
> accesses would touch the wrong half of the register.
>
> Use offsetoflow32() in the VTCR register struct. This works even
> though the field in the CPU struct is currently a struct TCR, because
> the first field in that struct is the uint64_t raw_tcr.
>
> None of the other TCR registers have this bug -- either they are
> AArch64 only, or else they define resetfn, writefn, etc, and
> expect to be passed the full struct pointer.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/7] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t
2022-07-14 13:23 ` [PATCH 5/7] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t Peter Maydell
@ 2022-07-14 23:21 ` Richard Henderson
0 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2022-07-14 23:21 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Idan Horowitz
On 7/14/22 18:53, Peter Maydell wrote:
> Change the representation of the VSTCR_EL2 and VTCR_EL2 registers in
> the CPU state struct from struct TCR to uint64_t.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> target/arm/cpu.h | 4 ++--
> target/arm/internals.h | 4 ++--
> target/arm/helper.c | 4 +---
> target/arm/ptw.c | 14 +++++++-------
> 4 files changed, 12 insertions(+), 14 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] target/arm: Store TCR_EL* registers as uint64_t
2022-07-14 13:23 ` [PATCH 6/7] target/arm: Store TCR_EL* " Peter Maydell
@ 2022-07-14 23:23 ` Richard Henderson
0 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2022-07-14 23:23 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Idan Horowitz
On 7/14/22 18:53, Peter Maydell wrote:
> Change the representation of the TCR_EL* registers in the CPU state
> struct from struct TCR to uint64_t. This allows us to drop the
> custom vmsa_ttbcr_raw_write() function, moving the "enforce RES0"
> checks to their more usual location in the writefn
> vmsa_ttbcr_write(). We also don't need the resetfn any more.
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> target/arm/cpu.h | 8 +----
> target/arm/internals.h | 6 ++--
> target/arm/cpu.c | 2 +-
> target/arm/debug_helper.c | 2 +-
> target/arm/helper.c | 75 +++++++++++----------------------------
> target/arm/ptw.c | 2 +-
> 6 files changed, 27 insertions(+), 68 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] target/arm: Honour VTCR_EL2 bits in Secure EL2
2022-07-14 13:23 ` [PATCH 7/7] target/arm: Honour VTCR_EL2 bits in Secure EL2 Peter Maydell
@ 2022-07-14 23:25 ` Richard Henderson
0 siblings, 0 replies; 15+ messages in thread
From: Richard Henderson @ 2022-07-14 23:25 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Idan Horowitz
On 7/14/22 18:53, Peter Maydell wrote:
> In regime_tcr() we return the appropriate TCR register for the
> translation regime. For Secure EL2, we return the VSTCR_EL2 value,
> but in this translation regime some fields that control behaviour are
> in VTCR_EL2. When this code was originally written (as the comment
> notes), QEMU didn't care about any of those fields, but we have since
> added support for features such as LPA2 which do need the values from
> those fields.
>
> Synthesize a TCR value by merging in the relevant VTCR_EL2 fields to
> the VSTCR_EL2 value.
>
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1103
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> target/arm/cpu.h | 19 +++++++++++++++++++
> target/arm/internals.h | 22 +++++++++++++++++++---
> 2 files changed, 38 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-07-14 23:26 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-14 13:22 [PATCH 0/7] target/arm: Handle VTCR_EL2 bits shared between S and NS EL2 Peter Maydell
2022-07-14 13:22 ` [PATCH 1/7] target/arm: Define and use new regime_tcr_value() function Peter Maydell
2022-07-14 23:12 ` Richard Henderson
2022-07-14 13:22 ` [PATCH 2/7] target/arm: Calculate mask/base_mask in get_level1_table_address() Peter Maydell
2022-07-14 23:18 ` Richard Henderson
2022-07-14 13:22 ` [PATCH 3/7] target/arm: Fold regime_tcr() and regime_tcr_value() together Peter Maydell
2022-07-14 23:18 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 4/7] target/arm: Fix big-endian host handling of VTCR Peter Maydell
2022-07-14 23:19 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 5/7] target/arm: Store VTCR_EL2, VSTCR_EL2 registers as uint64_t Peter Maydell
2022-07-14 23:21 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 6/7] target/arm: Store TCR_EL* " Peter Maydell
2022-07-14 23:23 ` Richard Henderson
2022-07-14 13:23 ` [PATCH 7/7] target/arm: Honour VTCR_EL2 bits in Secure EL2 Peter Maydell
2022-07-14 23:25 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).