| Message ID | 20221115161736.2425584-3-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Arm GICv2 fixes | expand |
On 15/11/22 17:17, Alex Bennée wrote: > a66a24585f (hw/intc/arm_gic: Implement read of GICC_IIDR) implemented > this for the CPU interface register. The fact we don't implement it > shows up when running Xen with -d guest_error which is definitely > wrong because the guest is perfectly entitled to read it. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > --- > v2 > - checkpatch fixes. > v3 > - re-base on re-flow with if > v4 > - fix the commit message > --- > hw/intc/arm_gic.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 1a04144c38..7a34bc0998 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -973,8 +973,18 @@ static uint8_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) > /* GICD_TYPER byte 1 */ > return (s->security_extn << 2); > } > - if (offset < 0x08) > + if (offset == 8) { > + /* GICD_IIDR byte 0 */ > + return 0x3b; /* Arm JEP106 identity */ > + } > + if (offset == 9) { > + /* GICD_IIDR byte 1 */ > + return 0x04; /* Arm JEP106 identity */ Possible future cleanup, define JEP106_ID_ARM: $ git grep 0x43b hw/intc/arm_gic.c:1671: *data = (s->revision << 16) | 0x43b; hw/intc/gicv3_internal.h:743: return 0x43b; hw/misc/armv7m_ras.c:26: *data = 0x43b; > + } > + if (offset < 0x0c) { > + /* All other bytes in this range are RAZ */ > return 0; > + } Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On Tue, 15 Nov 2022 at 16:29, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > Possible future cleanup, define JEP106_ID_ARM: > > $ git grep 0x43b > hw/intc/arm_gic.c:1671: *data = (s->revision << 16) | 0x43b; > hw/intc/gicv3_internal.h:743: return 0x43b; > hw/misc/armv7m_ras.c:26: *data = 0x43b; I think that's probably not worthwhile, because the datasheet gives you the hex value of the ID register for the device, and if we hide that behind a #define then now to check that the value is right you need to go look at the #define line. Put another way, the important thing is "this register holds the value 0x43b", and it's just a point of information that that happens to correspond to a JEP-106 manufacturer ID code value. thanks -- PMM
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 1a04144c38..7a34bc0998 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -973,8 +973,18 @@ static uint8_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs) /* GICD_TYPER byte 1 */ return (s->security_extn << 2); } - if (offset < 0x08) + if (offset == 8) { + /* GICD_IIDR byte 0 */ + return 0x3b; /* Arm JEP106 identity */ + } + if (offset == 9) { + /* GICD_IIDR byte 1 */ + return 0x04; /* Arm JEP106 identity */ + } + if (offset < 0x0c) { + /* All other bytes in this range are RAZ */ return 0; + } if (offset >= 0x80) { /* Interrupt Group Registers: these RAZ/WI if this is an NS * access to a GIC with the security extensions, or if the GIC
a66a24585f (hw/intc/arm_gic: Implement read of GICC_IIDR) implemented this for the CPU interface register. The fact we don't implement it shows up when running Xen with -d guest_error which is definitely wrong because the guest is perfectly entitled to read it. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - checkpatch fixes. v3 - re-base on re-flow with if v4 - fix the commit message --- hw/intc/arm_gic.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)