Re: [Qemu-arm] [Qemu-devel] [PATCH v2 11/22] hw/intc/arm_gicv3: Implement GICv3 distributor registers

[Peter Maydell]

On 13 June 2016 at 07:27, Shannon Zhao <address@hidden> wrote:
>
>
> On 2016/5/26 22:55, Peter Maydell wrote:
>> +static uint8_t gicd_read_ipriorityr(GICv3State *s, MemTxAttrs attrs, int 
>> irq)
>> +{
>> +    /* Read the value of GICD_IPRIORITYR<n> for the specified interrupt,
>> +     * honouring security state (these are RAZ/WI for Group 0 or Secure
>> +     * Group 1 interrupts).
>> +     */
>> +    uint32_t prio;
>> +
>> +    if (irq < GIC_INTERNAL || irq >= s->num_irq) {
>> +        return 0;
>> +    }
>> +
>> +    prio = s->gicd_ipriority[irq];
>> +
>> +    if (!attrs.secure && !(s->gicd_ctlr & GICD_CTLR_DS)) {
>> +        if (!gicv3_gicd_group_test(s, irq)) {
>> +            /* Fields for Group 0 or Secure Group 1 interrupts are RAZ/WI */
> Here this check assure this interrupt belongs to Group 0 and NS access
> is not permitted, so it should return 0. But it doesn't say anything
> about Secure Group 1.

We're testing the GICD_IGROUPR bit here. If DS is zero (security
enabled), then IGROUPR == 0 means "Group 0 or Secure Group 1", which
is what the comment says we're testing. (If you care which of 0 and S1
it is then you look at IGRPMODR, but for security checks like these
we don't need to.)

>> +            return 0;
>> +        }
>> +        /* NS view of the interrupt priority */
>> +        prio = (prio << 1) & 0xff;
>> +    }
> So maybe here it should check if attrs.secure is true and the Group is
> 1, then return 0.

I'm confused. If attrs.secure is true this is a secure access which
should be allowed to see anything, shouldn't it?

>> +    return prio;
>> +}

thanks
-- PMM