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

[Ben Dooks <ben.dooks@codethink.co.uk>]

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