[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate
From: |
Alex Bennée |
Subject: |
Re: [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate |
Date: |
Mon, 24 Oct 2022 15:09:32 +0100 |
User-agent: |
mu4e 1.9.1; emacs 28.2.50 |
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
- [PATCH v6 00/14] target/arm: Implement FEAT_HAFDBS, Richard Henderson, 2022/10/24
- [PATCH v6 01/14] target/arm: Introduce regime_is_stage2, Richard Henderson, 2022/10/24
- [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate, Richard Henderson, 2022/10/24
- Re: [PATCH v6 02/14] target/arm: Add ptw_idx to S1Translate,
Alex Bennée <=
- [PATCH v6 05/14] target/arm: Move S1_ptw_translate outside arm_ld[lq]_ptw, Richard Henderson, 2022/10/24
- [PATCH v6 04/14] target/arm: Extract HA and HD in aa64_va_parameters, Richard Henderson, 2022/10/24
- [PATCH v6 03/14] target/arm: Add isar predicates for FEAT_HAFDBS, Richard Henderson, 2022/10/24
- [PATCH v6 13/14] target/arm: Implement FEAT_HAFDBS, dirty bit portion, Richard Henderson, 2022/10/24
- [PATCH v6 06/14] target/arm: Add ARMFault_UnsuppAtomicUpdate, Richard Henderson, 2022/10/24
- [PATCH v6 11/14] target/arm: Tidy merging of attributes from descriptor and table, Richard Henderson, 2022/10/24
- [PATCH v6 12/14] target/arm: Implement FEAT_HAFDBS, access flag portion, Richard Henderson, 2022/10/24