OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

target/arm/ptw: Pass an ARMSecuritySpace to arm_hcr_el2_eff_secstate()

Browse Source 
arm_hcr_el2_eff_secstate() takes a bool secure, which it uses to
determine whether EL2 is enabled in the current security state.
With the advent of FEAT_RME this is no longer sufficient, because
EL2 can be enabled for Secure state but not for Root, and both
of those will pass 'secure == true' in the callsites in ptw.c.

As it happens in all of our callsites in ptw.c we either avoid making
the call or else avoid using the returned value if we're doing a
translation for Root, so this is not a behaviour change even if the
experimental FEAT_RME is enabled.  But it is less confusing in the
ptw.c code if we avoid the use of a bool secure that duplicates some
of the information in the ArmSecuritySpace argument.

Make arm_hcr_el2_eff_secstate() take an ARMSecuritySpace argument
instead. Because we always want to know the HCR_EL2 for the
security state defined by the current effective value of
SCR_EL3.{NSE,NS}, it makes no sense to pass ARMSS_Root here,
and we assert that callers don't do that.

To avoid the assert(), we thus push the call to
arm_hcr_el2_eff_secstate() down into the cases in
regime_translation_disabled() that need it, rather than calling the
function and ignoring the result for the Root space translations.
All other calls to this function in ptw.c are already in places
where we have confirmed that the mmu_idx is a stage 2 translation
or that the regime EL is not 3.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20230807141514.19075-7-peter.maydell@linaro.org
This commit is contained in:
Peter Maydell 2023-08-22 17:31:07 +01:00
parent d1289140a0
commit 2d12bb96bd
3 changed files with 13 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
target/arm/cpu.h
View File

@ -2555,7 +2555,7 @@ static inline bool arm_is_el2_enabled(CPUARMState *env)
* "for all purposes other than a direct read or write access of HCR_EL2."
* Not included here is HCR_RW.
*/
uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure);
uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, ARMSecuritySpace space);
uint64_t arm_hcr_el2_eff(CPUARMState *env);
uint64_t arm_hcrx_el2_eff(CPUARMState *env);

8
target/arm/helper.c
View File

@ -5772,11 +5772,13 @@ static void hcr_writelow(CPUARMState *env, const ARMCPRegInfo *ri,
* Bits that are not included here:
* RW (read from SCR_EL3.RW as needed)
*/
uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure)
uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, ARMSecuritySpace space)
{
uint64_t ret = env->cp15.hcr_el2;
if (!arm_is_el2_enabled_secstate(env, secure)) {
assert(space != ARMSS_Root);
if (!arm_is_el2_enabled_secstate(env, arm_space_is_secure(space))) {
/*
* "This register has no effect if EL2 is not enabled in the
* current Security state". This is ARMv8.4-SecEL2 speak for
@ -5840,7 +5842,7 @@ uint64_t arm_hcr_el2_eff(CPUARMState *env)
if (arm_feature(env, ARM_FEATURE_M)) {
return 0;
}
return arm_hcr_el2_eff_secstate(env, arm_is_secure_below_el3(env));
return arm_hcr_el2_eff_secstate(env, arm_security_space_below_el3(env));
}
/*

15
target/arm/ptw.c
View File

@ -209,9 +209,9 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx,
ARMSecuritySpace space)
{
uint64_t hcr_el2;
bool is_secure = arm_space_is_secure(space);
if (arm_feature(env, ARM_FEATURE_M)) {
bool is_secure = arm_space_is_secure(space);
switch (env->v7m.mpu_ctrl[is_secure] &
(R_V7M_MPU_CTRL_ENABLE_MASK | R_V7M_MPU_CTRL_HFNMIENA_MASK)) {
case R_V7M_MPU_CTRL_ENABLE_MASK:
@ -230,18 +230,19 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx,
}
}
hcr_el2 = arm_hcr_el2_eff_secstate(env, is_secure);
switch (mmu_idx) {
case ARMMMUIdx_Stage2:
case ARMMMUIdx_Stage2_S:
/* HCR.DC means HCR.VM behaves as 1 */
hcr_el2 = arm_hcr_el2_eff_secstate(env, space);
return (hcr_el2 & (HCR_DC | HCR_VM)) == 0;
case ARMMMUIdx_E10_0:
case ARMMMUIdx_E10_1:
case ARMMMUIdx_E10_1_PAN:
/* TGE means that EL0/1 act as if SCTLR_EL1.M is zero */
hcr_el2 = arm_hcr_el2_eff_secstate(env, space);
if (hcr_el2 & HCR_TGE) {
return true;
}
@ -251,6 +252,7 @@ static bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx,
case ARMMMUIdx_Stage1_E1:
case ARMMMUIdx_Stage1_E1_PAN:
/* HCR.DC means SCTLR_EL1.M behaves as 0 */
hcr_el2 = arm_hcr_el2_eff_secstate(env, space);
if (hcr_el2 & HCR_DC) {
return true;
}
@ -530,7 +532,6 @@ static bool fault_s1ns(ARMSecuritySpace space, ARMMMUIdx s2_mmu_idx)
static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
hwaddr addr, ARMMMUFaultInfo *fi)
{
bool is_secure = ptw->in_secure;
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
ARMMMUIdx s2_mmu_idx = ptw->in_ptw_idx;
uint8_t pte_attrs;
@ -587,7 +588,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
}
if (regime_is_stage2(s2_mmu_idx)) {
uint64_t hcr = arm_hcr_el2_eff_secstate(env, is_secure);
uint64_t hcr = arm_hcr_el2_eff_secstate(env, ptw->in_space);
if ((hcr & HCR_PTW) && S2_attrs_are_device(hcr, pte_attrs)) {
/*
@ -3066,7 +3067,6 @@ static bool get_phys_addr_disabled(CPUARMState *env,
ARMMMUFaultInfo *fi)
{
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
bool is_secure = arm_space_is_secure(ptw->in_space);
uint8_t memattr = 0x00; /* Device nGnRnE */
uint8_t shareability = 0; /* non-shareable */
int r_el;
@ -3112,7 +3112,7 @@ static bool get_phys_addr_disabled(CPUARMState *env,
/* Fill in cacheattr a-la AArch64.TranslateAddressS1Off. */
if (r_el == 1) {
uint64_t hcr = arm_hcr_el2_eff_secstate(env, is_secure);
uint64_t hcr = arm_hcr_el2_eff_secstate(env, ptw->in_space);
if (hcr & HCR_DC) {
if (hcr & HCR_DCT) {
memattr = 0xf0; /* Tagged, Normal, WB, RWA */
@ -3149,7 +3149,6 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
{
hwaddr ipa;
int s1_prot, s1_lgpgsz;
bool is_secure = ptw->in_secure;
ARMSecuritySpace in_space = ptw->in_space;
bool ret, ipa_secure;
ARMCacheAttrs cacheattrs1;
@ -3212,7 +3211,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw,
}
/* Combine the S1 and S2 cache attributes. */
hcr = arm_hcr_el2_eff_secstate(env, is_secure);
hcr = arm_hcr_el2_eff_secstate(env, in_space);
if (hcr & HCR_DC) {
/*
* HCR.DC forces the first stage attributes to