On 2016/6/13 17:04, Peter Maydell wrote: > On 13 June 2016 at 07:27, Shannon Zhao <zhaoshengl...@huawei.com> 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