[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 29/38] hw/intc/arm_gicv3_its: Use enum for return value of process
From: |
Peter Maydell |
Subject: |
[PULL 29/38] hw/intc/arm_gicv3_its: Use enum for return value of process_* functions |
Date: |
Thu, 20 Jan 2022 12:36:21 +0000 |
When an ITS detects an error in a command, it has an
implementation-defined (CONSTRAINED UNPREDICTABLE) choice of whether
to ignore the command, proceeding to the next one in the queue, or to
stall the ITS command queue, processing nothing further. The
behaviour required when the read of the command packet from memory
fails is less clearly documented, but the same set of choices as for
command errors seem reasonable.
The intention of the QEMU implementation, as documented in the
comments, is that if we encounter a memory error reading the command
packet or one of the various data tables then we should stall, but
for command parameter errors we should ignore the queue and continue.
However, we don't actually do this. To get the desired behaviour,
the various process_* functions need to return true to cause
process_cmdq() to advance to the next command and keep processing,
and false to stall command processing. What they mostly do is return
false for any kind of error.
To make the code clearer, replace the 'bool' return from the process_
functions with an enum which may be either CMD_STALL or CMD_CONTINUE.
In this commit no behaviour changes; in subsequent commits we will
adjust the error-return paths for the process_ functions one by one.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20220111171048.3545974-6-peter.maydell@linaro.org
---
hw/intc/arm_gicv3_its.c | 59 ++++++++++++++++++++++++++---------------
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index c1f76682d04..10901a5e709 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -45,6 +45,23 @@ typedef struct {
uint64_t itel;
} IteEntry;
+/*
+ * The ITS spec permits a range of CONSTRAINED UNPREDICTABLE options
+ * if a command parameter is not correct. These include both "stall
+ * processing of the command queue" and "ignore this command, and
+ * keep processing the queue". In our implementation we choose that
+ * memory transaction errors reading the command packet provoke a
+ * stall, but errors in parameters cause us to ignore the command
+ * and continue processing.
+ * The process_* functions which handle individual ITS commands all
+ * return an ItsCmdResult which tells process_cmdq() whether it should
+ * stall or keep going.
+ */
+typedef enum ItsCmdResult {
+ CMD_STALL = 0,
+ CMD_CONTINUE = 1,
+} ItsCmdResult;
+
static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
{
uint64_t result = 0;
@@ -217,8 +234,8 @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid,
MemTxResult *res)
* 3. handling of ITS CLEAR command
* 4. handling of ITS DISCARD command
*/
-static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset,
- ItsCmdType cmd)
+static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value,
+ uint32_t offset, ItsCmdType cmd)
{
AddressSpace *as = &s->gicv3->dma_as;
uint32_t devid, eventid;
@@ -231,7 +248,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t
value, uint32_t offset,
bool ite_valid = false;
uint64_t cte = 0;
bool cte_valid = false;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
uint64_t rdbase;
if (cmd == NONE) {
@@ -324,15 +341,15 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t
value, uint32_t offset,
if (cmd == DISCARD) {
IteEntry ite = {};
/* remove mapping from interrupt translation table */
- result = update_ite(s, eventid, dte, ite);
+ result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE :
CMD_STALL;
}
}
return result;
}
-static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset,
- bool ignore_pInt)
+static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value,
+ uint32_t offset, bool ignore_pInt)
{
AddressSpace *as = &s->gicv3->dma_as;
uint32_t devid, eventid;
@@ -343,7 +360,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value,
uint32_t offset,
MemTxResult res = MEMTX_OK;
uint16_t icid = 0;
uint64_t dte = 0;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
offset += NUM_BYTES_IN_DW;
@@ -404,7 +421,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value,
uint32_t offset,
ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
- result = update_ite(s, eventid, dte, ite);
+ result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL;
}
return result;
@@ -472,14 +489,14 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid,
bool valid,
}
}
-static bool process_mapc(GICv3ITSState *s, uint32_t offset)
+static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset)
{
AddressSpace *as = &s->gicv3->dma_as;
uint16_t icid;
uint64_t rdbase;
bool valid;
MemTxResult res = MEMTX_OK;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
uint64_t value;
offset += NUM_BYTES_IN_DW;
@@ -509,7 +526,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset)
* command in the queue
*/
} else {
- result = update_cte(s, icid, valid, rdbase);
+ result = update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL;
}
return result;
@@ -578,7 +595,8 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid,
bool valid,
}
}
-static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset)
+static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value,
+ uint32_t offset)
{
AddressSpace *as = &s->gicv3->dma_as;
uint32_t devid;
@@ -586,7 +604,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value,
uint32_t offset)
uint64_t itt_addr;
bool valid;
MemTxResult res = MEMTX_OK;
- bool result = false;
+ ItsCmdResult result = CMD_STALL;
devid = ((value & DEVID_MASK) >> DEVID_SHIFT);
@@ -623,7 +641,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value,
uint32_t offset)
* command in the queue
*/
} else {
- result = update_dte(s, devid, valid, size, itt_addr);
+ result = update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE :
CMD_STALL;
}
return result;
@@ -641,7 +659,6 @@ static void process_cmdq(GICv3ITSState *s)
uint64_t data;
AddressSpace *as = &s->gicv3->dma_as;
MemTxResult res = MEMTX_OK;
- bool result = true;
uint8_t cmd;
int i;
@@ -668,6 +685,8 @@ static void process_cmdq(GICv3ITSState *s)
}
while (wr_offset != rd_offset) {
+ ItsCmdResult result = CMD_CONTINUE;
+
cq_offset = (rd_offset * GITS_CMDQ_ENTRY_SIZE);
data = address_space_ldq_le(as, s->cq.base_addr + cq_offset,
MEMTXATTRS_UNSPECIFIED, &res);
@@ -726,18 +745,16 @@ static void process_cmdq(GICv3ITSState *s)
default:
break;
}
- if (result) {
+ if (result == CMD_CONTINUE) {
rd_offset++;
rd_offset %= s->cq.num_entries;
s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset);
} else {
- /*
- * in this implementation, in case of dma read/write error
- * we stall the command processing
- */
+ /* CMD_STALL */
s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, STALLED, 1);
qemu_log_mask(LOG_GUEST_ERROR,
- "%s: %x cmd processing failed\n", __func__, cmd);
+ "%s: 0x%x cmd processing failed, stalling\n",
+ __func__, cmd);
break;
}
}
--
2.25.1
- [PULL 02/38] hw: Move MARVELL_88W8618 Kconfig from audio/ to arm/, (continued)
- [PULL 02/38] hw: Move MARVELL_88W8618 Kconfig from audio/ to arm/, Peter Maydell, 2022/01/20
- [PULL 12/38] virtio-mem: Correct default THP size for ARM64, Peter Maydell, 2022/01/20
- [PULL 14/38] hw/intc/arm_gic: Implement read of GICC_IIDR, Peter Maydell, 2022/01/20
- [PULL 23/38] hw/misc/aspeed_i3c.c: Introduce a dummy AST2600 I3C model., Peter Maydell, 2022/01/20
- [PULL 35/38] hw/intc/arm_gicv3_its: Factor out "find address of table entry" code, Peter Maydell, 2022/01/20
- [PULL 18/38] hw/arm/virt: Honor highmem setting when computing the memory map, Peter Maydell, 2022/01/20
- [PULL 34/38] hw/intc/arm_gicv3_its: Fix return codes in process_mapd(), Peter Maydell, 2022/01/20
- [PULL 31/38] hw/intc/arm_gicv3_its: Refactor process_its_cmd() to reduce nesting, Peter Maydell, 2022/01/20
- [PULL 20/38] hw/arm/virt: Disable highmem devices that don't fit in the PA range, Peter Maydell, 2022/01/20
- [PULL 22/38] hw/arm: kudo add lm75s behind bus 1 switch at 75, Peter Maydell, 2022/01/20
- [PULL 29/38] hw/intc/arm_gicv3_its: Use enum for return value of process_* functions,
Peter Maydell <=
- [PULL 07/38] hw/acpi/aml-build: Improve scalability of PPTT generation, Peter Maydell, 2022/01/20
- [PULL 06/38] hw/arm/virt: Support cluster level in DT cpu-map, Peter Maydell, 2022/01/20
- [PULL 04/38] hw/net: Move MV88W8618 network device out of hw/arm/ directory, Peter Maydell, 2022/01/20
- [PULL 03/38] hw/arm/musicpal: Fix coding style of code related to MV88W8618 device, Peter Maydell, 2022/01/20
- [PULL 16/38] hw/arm/virt: Add a control for the the highmem PCIe MMIO, Peter Maydell, 2022/01/20
- [PULL 13/38] hw/arm/virt: Support for virtio-mem-pci, Peter Maydell, 2022/01/20
- [PULL 10/38] tests/acpi/bios-table-test: Update expected virt/PPTT file, Peter Maydell, 2022/01/20
- [PULL 11/38] docs/can: convert to restructuredText, Peter Maydell, 2022/01/20
- [PULL 15/38] hw/intc/arm_gic: Allow reset of the running priority, Peter Maydell, 2022/01/20
- [PULL 21/38] hw/arm/virt: Drop superfluous checks against highmem, Peter Maydell, 2022/01/20