Re: [PATCH] hw/intc/arm_gicv3: ICC_PMR_EL1 high bits should be RAZ

[Ben Dooks]

On 14/11/2023 17:14, Peter Maydell wrote:
> On Tue, 14 Nov 2023 at 16:54, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> > The ICC_PMR_ELx bit msak returned from icc_fullprio_mask
> > should technically also remove any bit above 7 as these
> > are marked reserved (read 0) and should therefore should
> > not be written as anything other than 0.
> >
> > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> > ---
> >   hw/intc/arm_gicv3_cpuif.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> > index d07b13eb27..986044df79 100644
> > --- a/hw/intc/arm_gicv3_cpuif.c
> > +++ b/hw/intc/arm_gicv3_cpuif.c
> > @@ -803,7 +803,7 @@ static uint32_t icc_fullprio_mask(GICv3CPUState *cs)
> >        * with the group priority, whose mask depends on the value of BPR
> >        * for the interrupt group.)
> >        */
> > -    return ~0U << (8 - cs->pribits);
> > +    return (~0U << (8 - cs->pribits)) & 0xff;
> >   }
>
> The upper bits of ICC_PMR_ELx are defined as RES0, which has a
> complicated technical definition which you can find in the GIC
> architecture specification glossary. It's valid for RES0 bits to
> be implemented as reads-as-written, which is the way our current
> implementation works. Valid guest code should never be writing
> any non-zero value into those bits.

Yeah, got some proprietary test code that is trying write-1 and
then assuming read-0.

> What problem are you running into that you're trying to fix
> with this patch? If our implementation misbehaves as a result
> of letting these high bits through into cs->icc_pmr_el1 that
> would be a good reason for making the change.

See above, local test code issue.

 > If we do want to change this, for consistency we'd want
> to change icv_fullprio_mask() too.

If this isn't useful then I'll keep it as a local patch for now.

--
Ben Dooks                               http://www.codethink.co.uk/
Senior Engineer                         Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html