On Sat, 20 Aug 2022 at 15:19, <tobias.roeh...@rwth-aachen.de> wrote: > > From: Tobias Röhmel <tobias.roeh...@rwth-aachen.de> > > The Cortex-R52 has a 2 stage MPU translation process but doesn't have the > FEAT_S2FWB feature. This makes it neccessary to allow for the old cache > attribut combination. This is facilitated by changing the control path > of combine_cacheattrs instead of failing if the second cache attributes > struct is not in that format.
I see what you mean but I think I would phrase it differently: The v8R PMSAv8 has a two-stage MPU translation process, but, unlike VMSAv8, the stage 2 attributes are in the same format as the stage 1 attributes (8-bit MAIR format). Rather than converting the MAIR format to the format used for VMSA stage 2 (bits [5:2] of a VMSA stage 2 descriptor) and then converting back to do the attribute combination, allow combined_attrs_nofwb() to accept s2 attributes that are already in the MAIR format. We move the assert() to combined_attrs_fwb(), because that function really does require a VMSA stage 2 attribute format. (We will never get there for v8R, because PMSAv8 does not implement FEAT_S2FWB.) > Signed-off-by: Tobias Röhmel <tobias.roeh...@rwth-aachen.de> > --- > target/arm/ptw.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index 4d97a24808..8b037c1f55 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -2108,7 +2108,11 @@ static uint8_t combined_attrs_nofwb(CPUARMState *env, > { > uint8_t s1lo, s2lo, s1hi, s2hi, s2_mair_attrs, ret_attrs; > > - s2_mair_attrs = convert_stage2_attrs(env, s2.attrs); > + if (s2.is_s2_format) { > + s2_mair_attrs = convert_stage2_attrs(env, s2.attrs); > + } else { > + s2_mair_attrs = s2.attrs; > + } > > s1lo = extract32(s1.attrs, 0, 4); > s2lo = extract32(s2_mair_attrs, 0, 4); > @@ -2166,6 +2170,8 @@ static uint8_t force_cacheattr_nibble_wb(uint8_t attr) > static uint8_t combined_attrs_fwb(CPUARMState *env, > ARMCacheAttrs s1, ARMCacheAttrs s2) > { > + assert(s2.is_s2_format && !s1.is_s2_format); > + > switch (s2.attrs) { > case 7: > /* Use stage 1 attributes */ > @@ -2215,7 +2221,6 @@ static ARMCacheAttrs combine_cacheattrs(CPUARMState > *env, > ARMCacheAttrs ret; > bool tagged = false; > > - assert(s2.is_s2_format && !s1.is_s2_format); > ret.is_s2_format = false; > > if (s1.attrs == 0xf0) { With the commit message tweaks, Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM