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

[Shannon Zhao]

On 2016/6/13 17:04, Peter Maydell wrote:
> 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.)
> 
Oh, right.

-- 
Shannon