Re: [PATCH] target/arm: honor HCR_E2H for AT S1E2R and AT S1E2W address translation

[ake]

On Tue, 25 Oct 2022 13:25:22 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Mon, 17 Oct 2022 at 10:30, Ake Koomsin <ake@igel.co.jp> wrote:
> >
> > When HCR_E2H is set, AT S1E2R and AT S1E2W should translate an
> > address based on both TTBR0_EL2 and TTBR1_EL2.
> >
> > Signed-off-by: Ake Koomsin <ake@igel.co.jp>
> > ---
> >  target/arm/helper.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index dde64a487a..147f96e752 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -3486,7 +3486,12 @@ static void ats_write64(CPUARMState *env,
> > const ARMCPRegInfo *ri, }
> >              break;
> >          case 4: /* AT S1E2R, AT S1E2W */
> > -            mmu_idx = ARMMMUIdx_E2;
> > +            if (arm_hcr_el2_eff(env) & HCR_E2H) {
> > +                mmu_idx = env->pstate & PSTATE_PAN ?
> > +                    ARMMMUIdx_E20_2_PAN : ARMMMUIdx_E20_2;
> > +            } else {
> > +                mmu_idx = ARMMMUIdx_E2;
> > +            }
> >              break;
> 
> I agree that the AT insns should be handling E2H, but I'm not sure
> this is the right fix, and with Richard's recent refactorings I've
> kind of lost track of what all our MMUIdxes do.
> 
> In the pseudocode, E2H is handled by changing the behaviour not
> just of the S1E2 ops, but also of the S1E1 ops. If E2H is set:
>  * the S1E2 ops use the EL2&0 regime, but continue to ignore
>    PSTATE.PAN
>  * the S1E1 ops also use the EL2&0 regime, with the S1E1RP and
>    S1E1WP ops looking at PSTATE.PAN and the others not
> 
> Richard -- do we want to just do the same thing, or do
> our MMUIdx uses differ from the architectural translation
> regimes in a way that means we need to do something else?
> 
> thanks
> -- PMM

I will re-read the AT instruction pseudocode and will come back with a
better fix. It might take a while as I am not familiar with the code
base. Thank you very much for comments.

---
Ake Koomsin