I've been working on the ITS to add support for the GICv4 functionality.
In the course of that I found a handful of bugs in it and also some
places where the code benefited from refactoring to make it a better
base to put in the GICv4 parts. This patchset is just the bugfixes
and cleanups, because there are enough patches here that I figured it
made sense to send them out now rather than holding on to them.
I've included Alex's "clean up error reporting" patch as patch 1 in
the series to avoid having to use a Based-on: tag.
Bug fixes:
* Most of the bounds checks on values provided by the guest in
command packets had off-by-one errors. In almost all cases this
was completely harmless since the tables being indexed are in
guest memory, but for rdbase we use it as an index into a C array,
so there a badly-behaved guest could probably crash QEMU
* the loop over the GITS_BASER<n> registers in extract_table_params()
returned early when it found a register with the Valid bit clear,
rather than continuing to process the other base registers
* We miscalculated the entry sizes for the Collection and Device
tables, with the effect that we could potentially corrupt
guest memory
* the MAPI command handling was missing an error check on EventID
* if the guest confgured a DTE with a size of 32 we would have
shifted off the end of a 32-bit value
* the calls to process_its_cmd() put the return value in the wrong
variable
* if the memory access to read the first word of the command packet
failed, our error-handling codepath wasn't quite right
* we weren't actually implementing the "memory access errors cause
the ITS to stall command processing, parameter errors cause it
to continue to the next command" logic that the comments claim
Refactorings:
* the ITS_CTLR_ENABLED define was a duplicate of R_GITS_CTLR_ENABLED_MASK
* extract_table_params() had unnecessarily duplicated code for
handling each table type
* some refactoring and renaming of variables and struct fields used
in the bounds-check tests so that we have a consistent convention
that hopefully reduces the risk of future off-by-one errors
* some parts of the code which were doing open-coded shift-and-mask
operations have been converted to use the FIELD macro
* the code for "find the address of an entry in an in-guest-memory
table" can be factored out into its own function
thanks
-- PMM
Alex Bennée (1):
hw/intc: clean-up error reporting for failed ITS cmd
Peter Maydell (25):
hw/intc/arm_gicv3_its: Correct off-by-one bounds check on rdbase
hw/intc/arm_gicv3_its: Remove redundant ITS_CTLR_ENABLED define
hw/intc/arm_gicv3_its: Remove maxids union from TableDesc
hw/intc/arm_gicv3_its: Don't return early in extract_table_params()
loop
hw/intc/arm_gicv3_its: Reduce code duplication in
extract_table_params()
hw/intc/arm_gicv3_its: Correct setting of TableDesc entry_sz
hw/intc/arm_gicv3_its: Don't misuse GITS_TYPE_PHYSICAL define
hw/intc/arm_gicv3_its: Correct handling of MAPI
hw/intc/arm_gicv3_its: Use FIELD macros for DTEs
hw/intc/arm_gicv3_its: Use 1ULL when shifting by (DTE.SIZE + 1)
hw/intc/arm_gicv3_its: Correct comment about CTE RDBase field size
hw/intc/arm_gicv3_its: Use FIELD macros for CTEs
hw/intc/arm_gicv3_its: Fix various off-by-one errors
hw/intc/arm_gicv3_its: Rename max_l2_entries to num_l2_entries
hw/intc/arm_gicv3_its: Fix event ID bounds checks
hw/intc/arm_gicv3_its: Convert int ID check to num_intids convention
hw/intc/arm_gicv3_its: Fix handling of process_its_cmd() return value
hw/intc/arm_gicv3_its: Don't use data if reading command failed
hw/intc/arm_gicv3_its: Use enum for return value of process_*
functions
hw/intc/arm_gicv3_its: Fix return codes in process_its_cmd()
hw/intc/arm_gicv3_its: Refactor process_its_cmd() to reduce nesting
hw/intc/arm_gicv3_its: Fix return codes in process_mapti()
hw/intc/arm_gicv3_its: Fix return codes in process_mapc()
hw/intc/arm_gicv3_its: Fix return codes in process_mapd()
hw/intc/arm_gicv3_its: Factor out "find address of table entry" code
hw/intc/gicv3_internal.h | 40 +-
include/hw/intc/arm_gicv3_its_common.h | 9 +-
hw/intc/arm_gicv3_its.c | 628 +++++++++++--------------
3 files changed, 303 insertions(+), 374 deletions(-)
--
2.25.1
