Re: [PATCH 10/14] target/arm: do S1_ptw_translate() before address space lookup

["Rémi Denis-Courmont" <remi.denis.courmont@huawei.com>]

Le tiistaina 3. marraskuuta 2020, 21.54.48 EET Richard Henderson a écrit :
> On 11/2/20 2:57 AM, remi.denis.courmont@huawei.com wrote:
> > From: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > 
> > In the secure stage 2 translation regime, the VSTCR.SW and VTCR.NSW
> > bits can invert the secure flag for pagetable walks. This patchset
> > allows S1_ptw_translate() to change the non-secure bit.
> > 
> > Signed-off-by: Rémi Denis-Courmont <remi.denis.courmont@huawei.com>
> > ---
> > 
> >  target/arm/helper.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index 4c86e4f57c..7c70460e65 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -10403,7 +10403,7 @@ static bool get_level1_table_address(CPUARMState
> > *env, ARMMMUIdx mmu_idx,> 
> >  /* Translate a S1 pagetable walk through S2 if needed.  */
> >  static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
> > 
> > -                               hwaddr addr, MemTxAttrs txattrs,
> > +                               hwaddr addr, bool *is_secure,
> > 
> >                                 ARMMMUFaultInfo *fi)
> >  
> >  {
> >  
> >      ARMMMUIdx s2_mmu_idx;
> > 
> > @@ -10415,6 +10415,9 @@ static hwaddr S1_ptw_translate(CPUARMState *env,
> > ARMMMUIdx mmu_idx,> 
> >          int s2prot;
> >          int ret;
> >          ARMCacheAttrs cacheattrs = {};
> > 
> > +        MemTxAttrs txattrs = {};
> > +
> > +        assert(!*is_secure); /* TODO: S-EL2 */
> 
> Are you sure that you don't want to pass in txattrs via pointer instead?

That's possible too, and more like the existing code. Though I thought it 
clearer to pass only a pointer to the secure bit in/out, seen as that's the 
only in/out parameter.

> This change by itself looks questionable.  I guess I'll have to look
> forward to the other patch...


-- 
Rémi Denis-Courmont