On Tue, 14 Nov 2023 at 16:54, Ben Dooks <ben.do...@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.do...@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. 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. If we do want to change this, for consistency we'd want to change icv_fullprio_mask() too. thanks -- PMM
