| Message ID | 20220122182444.724087-11-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm_gicv3_its: Implement MOVI and MOVALL commands | expand |
On 22/1/22 19:24, Peter Maydell wrote: > The MemoryRegionOps gicv3_its_translation_ops currently provides only > a .write_with_attrs function, because the only register in this > region is the write-only GITS_TRANSLATER. However, if you don't > provide a read function and the guest tries reading from this memory > region, QEMU will crash because > memory_region_read_with_attrs_accessor() calls a NULL pointer. > > Add a read function which always returns 0, to cover both bogus > attempts to read GITS_TRANSLATER and also reads from the rest of the > region, which is documented to be reserved, RES0. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_its.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > +static MemTxResult gicv3_its_translation_read(void *opaque, hwaddr offset, > + uint64_t *data, unsigned size, > + MemTxAttrs attrs) > +{ > + /* > + * GITS_TRANSLATER is write-only, and all other addresses > + * in the interrupt translation space frame are RES0. > + */ > + *data = 0; Maybe log GUEST_ERROR? Otherwise, Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > + return MEMTX_OK; > +}
On 1/23/22 05:24, Peter Maydell wrote: > The MemoryRegionOps gicv3_its_translation_ops currently provides only > a .write_with_attrs function, because the only register in this > region is the write-only GITS_TRANSLATER. However, if you don't > provide a read function and the guest tries reading from this memory > region, QEMU will crash because > memory_region_read_with_attrs_accessor() calls a NULL pointer. > > Add a read function which always returns 0, to cover both bogus > attempts to read GITS_TRANSLATER and also reads from the rest of the > region, which is documented to be reserved, RES0. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_its.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index d9ff7b88492..b17f2631269 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -813,6 +813,18 @@ static void extract_cmdq_params(GICv3ITSState *s) } } +static MemTxResult gicv3_its_translation_read(void *opaque, hwaddr offset, + uint64_t *data, unsigned size, + MemTxAttrs attrs) +{ + /* + * GITS_TRANSLATER is write-only, and all other addresses + * in the interrupt translation space frame are RES0. + */ + *data = 0; + return MEMTX_OK; +} + static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset, uint64_t data, unsigned size, MemTxAttrs attrs) @@ -1168,6 +1180,7 @@ static const MemoryRegionOps gicv3_its_control_ops = { }; static const MemoryRegionOps gicv3_its_translation_ops = { + .read_with_attrs = gicv3_its_translation_read, .write_with_attrs = gicv3_its_translation_write, .valid.min_access_size = 2, .valid.max_access_size = 4,
The MemoryRegionOps gicv3_its_translation_ops currently provides only a .write_with_attrs function, because the only register in this region is the write-only GITS_TRANSLATER. However, if you don't provide a read function and the guest tries reading from this memory region, QEMU will crash because memory_region_read_with_attrs_accessor() calls a NULL pointer. Add a read function which always returns 0, to cover both bogus attempts to read GITS_TRANSLATER and also reads from the rest of the region, which is documented to be reserved, RES0. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_its.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)