On Tue, 24 Jan 2023 at 00:06, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> With Realm security state, bit 55 of a block or page descriptor during
> the stage2 walk becomes the NS bit; during the stage1 walk the bit 5
> NS bit is RES0. With Root security state, bit 11 of the block or page
> descriptor during the stage1 walk becomes the NSE bit.
>
> Rather than collecting an NS bit and applying it later, compute the
> output pa space from the input pa space and unconditionally assign.
> This means that we no longer need to adjust the output space earlier
> for the NSTable bit.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> target/arm/ptw.c | 74 +++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 60 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index ddafb1f329..849f5e89ca 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1250,11 +1250,12 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> S1Translate *ptw,
> int32_t stride;
> int addrsize, inputsize, outputsize;
> uint64_t tcr = regime_tcr(env, mmu_idx);
> - int ap, ns, xn, pxn;
> + int ap, xn, pxn;
> uint32_t el = regime_el(env, mmu_idx);
> uint64_t descaddrmask;
> bool aarch64 = arm_el_is_aa64(env, el);
> uint64_t descriptor, new_descriptor;
> + ARMSecuritySpace out_space;
>
> /* TODO: This code does not support shareability levels. */
> if (aarch64) {
> @@ -1434,8 +1435,6 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> S1Translate *ptw,
> ptw->in_ptw_idx += 1;
> ptw->in_secure = false;
> ptw->in_space = ARMSS_NonSecure;
> - result->f.attrs.secure = false;
> - result->f.attrs.space = ARMSS_NonSecure;
> }
>
> if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
> @@ -1552,12 +1551,66 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> S1Translate *ptw,
> }
>
> ap = extract32(attrs, 6, 2);
> + out_space = ptw->in_space;
> if (regime_is_stage2(mmu_idx)) {
> - ns = mmu_idx == ARMMMUIdx_Stage2;
> + /*
> + * R_GYNXY: For stage2 in Realm security state, bit 55 is NS.
> + * The bit remains ignored for other security states.
> + */
> + if (out_space == ARMSS_Realm && extract64(attrs, 55, 1)) {
> + out_space = ARMSS_NonSecure;
> + }
> xn = extract64(attrs, 53, 2);
> result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
> } else {
> - ns = extract32(attrs, 5, 1);
> + int ns = extract32(attrs, 5, 1);
> + switch (out_space) {
> + case ARMSS_Root:
> + /*
> + * R_GVZML: Bit 11 becomes the NSE field in the EL3 regime.
> + * R_XTYPW: NSE and NS together select the output pa space.
> + */
> + int nse = extract32(attrs, 11, 1);
> + out_space = (nse << 1) | ns;
> + if (out_space == ARMSS_Secure &&
> + !cpu_isar_feature(aa64_sel2, cpu)) {
> + out_space = ARMSS_NonSecure;
> + }
> + break;
> + case ARMSS_Secure:
> + if (ns) {
> + out_space = ARMSS_NonSecure;
> + }
> + break;
> + case ARMSS_Realm:
> + switch (mmu_idx) {
> + case ARMMMUIdx_Stage1_E0:
> + case ARMMMUIdx_Stage1_E1:
> + case ARMMMUIdx_Stage1_E1_PAN:
> + /* I_CZPRF: For Realm EL1&0 stage1, NS bit is RES0. */
> + break;
> + case ARMMMUIdx_E2:
> + case ARMMMUIdx_E20_0:
> + case ARMMMUIdx_E20_2:
> + case ARMMMUIdx_E20_2_PAN:
> + /*
> + * R_LYKFZ, R_WGRZN: For Realm EL2 and EL2&1,
> + * NS changes the output to non-secure space.
> + */
> + if (ns) {
> + out_space = ARMSS_NonSecure;
> + }
> + break;
> + default:
> + g_assert_not_reached();
> + }
> + break;
> + case ARMSS_NonSecure:
> + /* R_QRMFF: For NonSecure state, the NS bit is RES0. */
> + break;
> + default:
> + g_assert_not_reached();
> + }
> xn = extract64(attrs, 54, 1);
> pxn = extract64(attrs, 53, 1);
> result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
Various of these cases say "NS is RES0", but the code is
still extracting it from the attrs bit 5 and passing it
to get_S1prot(), which relies on being passed 1 for any case
where the translation table indicates that the memory is
Non-secure, whether the NS bit in the final block or page descriptor
was 1 or not (it uses it to implement the SCR_EL3.SIF bit).
This used to happen automatically because we set tableattrs to
1 << 4 if the initial access was non-secure, and then that bit
would always end up in attrs bit 5 regardless of what the various
NSTable and NS bits in the descriptors were, but the refactoring
in the previous patch changes that and so we need to do something
else I think.
thanks
-- PMM
