mbox series

[00/26] arm gicv3 ITS: Various bug fixes and refactorings

Message ID 20211211191135.1764649-1-peter.maydell@linaro.org
Headers show
Series arm gicv3 ITS: Various bug fixes and refactorings | expand

Message

Peter Maydell Dec. 11, 2021, 7:11 p.m. UTC 
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(-)