* [Qemu-devel] [PATCH v2 0/2] hw/intc/arm_gicv3: Some simple bugfixes @ 2019-05-24 12:42 Peter Maydell 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 1/2] hw/intc/arm_gicv3: Fix decoding of ID register range Peter Maydell 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 2/2] hw/intc/arm_gicv3: GICD_TYPER.SecurityExtn is RAZ if GICD_CTLR.DS == 1 Peter Maydell 0 siblings, 2 replies; 5+ messages in thread From: Peter Maydell @ 2019-05-24 12:42 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé This patchset fixes a couple of simple bugs in our GICv3 implementation. Changes since v1: * patches 3 and 4 from the old patchset are now in master * patch 1 now covers both the read and write functions I've also just noticed (via grep for IDREGS) that we made the same decode mistake in the SMMUv3. I'll send that out as a separate patch. thanks -- PMM Peter Maydell (2): hw/intc/arm_gicv3: Fix decoding of ID register range hw/intc/arm_gicv3: GICD_TYPER.SecurityExtn is RAZ if GICD_CTLR.DS == 1 hw/intc/arm_gicv3_dist.c | 12 +++++++++--- hw/intc/arm_gicv3_redist.c | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] hw/intc/arm_gicv3: Fix decoding of ID register range 2019-05-24 12:42 [Qemu-devel] [PATCH v2 0/2] hw/intc/arm_gicv3: Some simple bugfixes Peter Maydell @ 2019-05-24 12:42 ` Peter Maydell 2019-05-24 13:13 ` Philippe Mathieu-Daudé 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 2/2] hw/intc/arm_gicv3: GICD_TYPER.SecurityExtn is RAZ if GICD_CTLR.DS == 1 Peter Maydell 1 sibling, 1 reply; 5+ messages in thread From: Peter Maydell @ 2019-05-24 12:42 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé The GIC ID registers cover an area 0x30 bytes in size (12 registers, 4 bytes each). We were incorrectly decoding only the first 0x20 bytes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_dist.c | 4 ++-- hw/intc/arm_gicv3_redist.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c index 53c55c57291..e6fe4905fd3 100644 --- a/hw/intc/arm_gicv3_dist.c +++ b/hw/intc/arm_gicv3_dist.c @@ -533,7 +533,7 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset, } return MEMTX_OK; } - case GICD_IDREGS ... GICD_IDREGS + 0x1f: + case GICD_IDREGS ... GICD_IDREGS + 0x2f: /* ID registers */ *data = gicv3_idreg(offset - GICD_IDREGS); return MEMTX_OK; @@ -744,7 +744,7 @@ static MemTxResult gicd_writel(GICv3State *s, hwaddr offset, gicd_write_irouter(s, attrs, irq, r); return MEMTX_OK; } - case GICD_IDREGS ... GICD_IDREGS + 0x1f: + case GICD_IDREGS ... GICD_IDREGS + 0x2f: case GICD_TYPER: case GICD_IIDR: /* RO registers, ignore the write */ diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c index 3b0ba6de1ab..8645220d618 100644 --- a/hw/intc/arm_gicv3_redist.c +++ b/hw/intc/arm_gicv3_redist.c @@ -233,7 +233,7 @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr offset, } *data = cs->gicr_nsacr; return MEMTX_OK; - case GICR_IDREGS ... GICR_IDREGS + 0x1f: + case GICR_IDREGS ... GICR_IDREGS + 0x2f: *data = gicv3_idreg(offset - GICR_IDREGS); return MEMTX_OK; default: @@ -363,7 +363,7 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr offset, return MEMTX_OK; case GICR_IIDR: case GICR_TYPER: - case GICR_IDREGS ... GICR_IDREGS + 0x1f: + case GICR_IDREGS ... GICR_IDREGS + 0x2f: /* RO registers, ignore the write */ qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid guest write to RO register at offset " -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] hw/intc/arm_gicv3: Fix decoding of ID register range 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 1/2] hw/intc/arm_gicv3: Fix decoding of ID register range Peter Maydell @ 2019-05-24 13:13 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2019-05-24 13:13 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel On 5/24/19 2:42 PM, Peter Maydell wrote: > The GIC ID registers cover an area 0x30 bytes in size > (12 registers, 4 bytes each). We were incorrectly decoding > only the first 0x20 bytes. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_dist.c | 4 ++-- > hw/intc/arm_gicv3_redist.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c > index 53c55c57291..e6fe4905fd3 100644 > --- a/hw/intc/arm_gicv3_dist.c > +++ b/hw/intc/arm_gicv3_dist.c > @@ -533,7 +533,7 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset, > } > return MEMTX_OK; > } > - case GICD_IDREGS ... GICD_IDREGS + 0x1f: > + case GICD_IDREGS ... GICD_IDREGS + 0x2f: > /* ID registers */ > *data = gicv3_idreg(offset - GICD_IDREGS); > return MEMTX_OK; > @@ -744,7 +744,7 @@ static MemTxResult gicd_writel(GICv3State *s, hwaddr offset, > gicd_write_irouter(s, attrs, irq, r); > return MEMTX_OK; > } > - case GICD_IDREGS ... GICD_IDREGS + 0x1f: > + case GICD_IDREGS ... GICD_IDREGS + 0x2f: > case GICD_TYPER: > case GICD_IIDR: > /* RO registers, ignore the write */ > diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c > index 3b0ba6de1ab..8645220d618 100644 > --- a/hw/intc/arm_gicv3_redist.c > +++ b/hw/intc/arm_gicv3_redist.c > @@ -233,7 +233,7 @@ static MemTxResult gicr_readl(GICv3CPUState *cs, hwaddr offset, > } > *data = cs->gicr_nsacr; > return MEMTX_OK; > - case GICR_IDREGS ... GICR_IDREGS + 0x1f: > + case GICR_IDREGS ... GICR_IDREGS + 0x2f: > *data = gicv3_idreg(offset - GICR_IDREGS); > return MEMTX_OK; > default: > @@ -363,7 +363,7 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr offset, > return MEMTX_OK; > case GICR_IIDR: > case GICR_TYPER: > - case GICR_IDREGS ... GICR_IDREGS + 0x1f: > + case GICR_IDREGS ... GICR_IDREGS + 0x2f: > /* RO registers, ignore the write */ > qemu_log_mask(LOG_GUEST_ERROR, > "%s: invalid guest write to RO register at offset " > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] hw/intc/arm_gicv3: GICD_TYPER.SecurityExtn is RAZ if GICD_CTLR.DS == 1 2019-05-24 12:42 [Qemu-devel] [PATCH v2 0/2] hw/intc/arm_gicv3: Some simple bugfixes Peter Maydell 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 1/2] hw/intc/arm_gicv3: Fix decoding of ID register range Peter Maydell @ 2019-05-24 12:42 ` Peter Maydell 2019-06-07 13:08 ` Peter Maydell 1 sibling, 1 reply; 5+ messages in thread From: Peter Maydell @ 2019-05-24 12:42 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Philippe Mathieu-Daudé The GICv3 specification says that the GICD_TYPER.SecurityExtn bit is RAZ if GICD_CTLR.DS is 1. We were incorrectly making it RAZ if the security extension is unsupported. "Security extension unsupported" always implies GICD_CTLR.DS == 1, but the guest can also set DS on a GIC which does support the security extension. Fix the condition to correctly check the GICD_CTLR.DS bit. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_dist.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c index e6fe4905fd3..b65f56f9035 100644 --- a/hw/intc/arm_gicv3_dist.c +++ b/hw/intc/arm_gicv3_dist.c @@ -378,8 +378,14 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset, * ITLinesNumber == (num external irqs / 32) - 1 */ int itlinesnumber = ((s->num_irq - GIC_INTERNAL) / 32) - 1; + /* + * SecurityExtn must be RAZ if GICD_CTLR.DS == 1, and + * "security extensions not supported" always implies DS == 1, + * so we only need to check the DS bit. + */ + bool sec_extn = !(s->gicd_ctlr & GICD_CTLR_DS); - *data = (1 << 25) | (1 << 24) | (s->security_extn << 10) | + *data = (1 << 25) | (1 << 24) | (sec_extn << 10) | (0xf << 19) | itlinesnumber; return MEMTX_OK; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw/intc/arm_gicv3: GICD_TYPER.SecurityExtn is RAZ if GICD_CTLR.DS == 1 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 2/2] hw/intc/arm_gicv3: GICD_TYPER.SecurityExtn is RAZ if GICD_CTLR.DS == 1 Peter Maydell @ 2019-06-07 13:08 ` Peter Maydell 0 siblings, 0 replies; 5+ messages in thread From: Peter Maydell @ 2019-06-07 13:08 UTC (permalink / raw) To: qemu-arm, QEMU Developers; +Cc: Philippe Mathieu-Daudé Ping for code review, please? thanks -- PMM On Fri, 24 May 2019 at 13:42, Peter Maydell <peter.maydell@linaro.org> wrote: > > The GICv3 specification says that the GICD_TYPER.SecurityExtn bit > is RAZ if GICD_CTLR.DS is 1. We were incorrectly making it RAZ > if the security extension is unsupported. "Security extension > unsupported" always implies GICD_CTLR.DS == 1, but the guest can > also set DS on a GIC which does support the security extension. > Fix the condition to correctly check the GICD_CTLR.DS bit. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_dist.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c > index e6fe4905fd3..b65f56f9035 100644 > --- a/hw/intc/arm_gicv3_dist.c > +++ b/hw/intc/arm_gicv3_dist.c > @@ -378,8 +378,14 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset, > * ITLinesNumber == (num external irqs / 32) - 1 > */ > int itlinesnumber = ((s->num_irq - GIC_INTERNAL) / 32) - 1; > + /* > + * SecurityExtn must be RAZ if GICD_CTLR.DS == 1, and > + * "security extensions not supported" always implies DS == 1, > + * so we only need to check the DS bit. > + */ > + bool sec_extn = !(s->gicd_ctlr & GICD_CTLR_DS); > > - *data = (1 << 25) | (1 << 24) | (s->security_extn << 10) | > + *data = (1 << 25) | (1 << 24) | (sec_extn << 10) | > (0xf << 19) | itlinesnumber; > return MEMTX_OK; > } > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-07 13:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-05-24 12:42 [Qemu-devel] [PATCH v2 0/2] hw/intc/arm_gicv3: Some simple bugfixes Peter Maydell 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 1/2] hw/intc/arm_gicv3: Fix decoding of ID register range Peter Maydell 2019-05-24 13:13 ` Philippe Mathieu-Daudé 2019-05-24 12:42 ` [Qemu-devel] [PATCH v2 2/2] hw/intc/arm_gicv3: GICD_TYPER.SecurityExtn is RAZ if GICD_CTLR.DS == 1 Peter Maydell 2019-06-07 13:08 ` Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).