[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 31/39] hw/intc/arm_gicv3_its: Fix address calculation in get_ite()
From: |
Peter Maydell |
Subject: |
[PULL 31/39] hw/intc/arm_gicv3_its: Fix address calculation in get_ite() and update_ite() |
Date: |
Tue, 8 Feb 2022 11:39:40 +0000 |
In get_ite() and update_ite() we work with a 12-byte in-guest-memory
table entry, which we intend to handle as an 8-byte value followed by
a 4-byte value. Unfortunately the calculation of the address of the
4-byte value is wrong, because we write it as:
table_base_address + (index * entrysize) + 4
(obfuscated by the way the expression has been written)
when it should be + 8. This bug meant that we overwrote the top
bytes of the 8-byte value with the 4-byte value. There are no
guest-visible effects because the top half of the 8-byte value
contains only the doorbell interrupt field, which is used only in
GICv4, and the two bugs in the "write ITE" and "read ITE" codepaths
cancel each other out.
We can't simply change the calculation, because this would break
migration of a (TCG) guest from the old version of QEMU which had
in-guest-memory interrupt tables written using the buggy version of
update_ite(). We must also at the same time change the layout of the
fields within the ITE_L and ITE_H values so that the in-memory
locations of the fields we care about (VALID, INTTYPE, INTID and
ICID) stay the same.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20220201193207.2771604-7-peter.maydell@linaro.org
---
hw/intc/gicv3_internal.h | 19 ++++++++++---------
hw/intc/arm_gicv3_its.c | 28 +++++++++++-----------------
2 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 60c8617e4e4..2bf1baef047 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -370,22 +370,23 @@ FIELD(MOVI_2, ICID, 0, 16)
* 12 bytes Interrupt translation Table Entry size
* as per Table 5.3 in GICv3 spec
* ITE Lower 8 Bytes
- * Bits: | 49 ... 26 | 25 ... 2 | 1 | 0 |
- * Values: | Doorbell | IntNum | IntType | Valid |
+ * Bits: | 63 ... 48 | 47 ... 32 | 31 ... 26 | 25 ... 2 | 1 | 0
|
+ * Values: | vPEID | ICID | unused | IntNum | IntType | Valid
|
* ITE Higher 4 Bytes
- * Bits: | 31 ... 16 | 15 ...0 |
- * Values: | vPEID | ICID |
- * (When Doorbell is unused, as it always is in GICv3, it is 1023)
+ * Bits: | 31 ... 25 | 24 ... 0 |
+ * Values: | unused | Doorbell |
+ * (When Doorbell is unused, as it always is for INTYPE_PHYSICAL,
+ * the value of that field in memory cannot be relied upon -- older
+ * versions of QEMU did not correctly write to that memory.)
*/
#define ITS_ITT_ENTRY_SIZE 0xC
FIELD(ITE_L, VALID, 0, 1)
FIELD(ITE_L, INTTYPE, 1, 1)
FIELD(ITE_L, INTID, 2, 24)
-FIELD(ITE_L, DOORBELL, 26, 24)
-
-FIELD(ITE_H, ICID, 0, 16)
-FIELD(ITE_H, VPEID, 16, 16)
+FIELD(ITE_L, ICID, 32, 16)
+FIELD(ITE_L, VPEID, 48, 16)
+FIELD(ITE_H, DOORBELL, 0, 24)
/* Possible values for ITE_L INTTYPE */
#define ITE_INTTYPE_VIRTUAL 0
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index b94775fd379..48eaf20a6c9 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -173,14 +173,12 @@ static bool update_ite(GICv3ITSState *s, uint32_t
eventid, const DTEntry *dte,
{
AddressSpace *as = &s->gicv3->dma_as;
MemTxResult res = MEMTX_OK;
+ hwaddr iteaddr = dte->ittaddr + eventid * ITS_ITT_ENTRY_SIZE;
- address_space_stq_le(as, dte->ittaddr + (eventid * (sizeof(uint64_t) +
- sizeof(uint32_t))), ite.itel, MEMTXATTRS_UNSPECIFIED,
- &res);
+ address_space_stq_le(as, iteaddr, ite.itel, MEMTXATTRS_UNSPECIFIED, &res);
if (res == MEMTX_OK) {
- address_space_stl_le(as, dte->ittaddr + (eventid * (sizeof(uint64_t) +
- sizeof(uint32_t))) + sizeof(uint32_t), ite.iteh,
+ address_space_stl_le(as, iteaddr + 8, ite.iteh,
MEMTXATTRS_UNSPECIFIED, &res);
}
if (res != MEMTX_OK) {
@@ -196,16 +194,12 @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid,
const DTEntry *dte,
AddressSpace *as = &s->gicv3->dma_as;
bool status = false;
IteEntry ite = {};
+ hwaddr iteaddr = dte->ittaddr + eventid * ITS_ITT_ENTRY_SIZE;
- ite.itel = address_space_ldq_le(as, dte->ittaddr +
- (eventid * (sizeof(uint64_t) +
- sizeof(uint32_t))), MEMTXATTRS_UNSPECIFIED,
- res);
+ ite.itel = address_space_ldq_le(as, iteaddr, MEMTXATTRS_UNSPECIFIED, res);
if (*res == MEMTX_OK) {
- ite.iteh = address_space_ldl_le(as, dte->ittaddr +
- (eventid * (sizeof(uint64_t) +
- sizeof(uint32_t))) + sizeof(uint32_t),
+ ite.iteh = address_space_ldl_le(as, iteaddr + 8,
MEMTXATTRS_UNSPECIFIED, res);
if (*res == MEMTX_OK) {
@@ -213,7 +207,7 @@ static bool get_ite(GICv3ITSState *s, uint32_t eventid,
const DTEntry *dte,
int inttype = FIELD_EX64(ite.itel, ITE_L, INTTYPE);
if (inttype == ITE_INTTYPE_PHYSICAL) {
*pIntid = FIELD_EX64(ite.itel, ITE_L, INTID);
- *icid = FIELD_EX32(ite.iteh, ITE_H, ICID);
+ *icid = FIELD_EX64(ite.itel, ITE_L, ICID);
status = true;
}
}
@@ -412,8 +406,8 @@ static ItsCmdResult process_mapti(GICv3ITSState *s, const
uint64_t *cmdpkt,
ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, true);
ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, pIntid);
- ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
- ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, ICID, icid);
+ ite.iteh = FIELD_DP32(ite.iteh, ITE_H, DOORBELL, INTID_SPURIOUS);
return update_ite(s, eventid, &dte, ite) ? CMD_CONTINUE : CMD_STALL;
}
@@ -688,8 +682,8 @@ static ItsCmdResult process_movi(GICv3ITSState *s, const
uint64_t *cmdpkt)
ite.itel = FIELD_DP64(ite.itel, ITE_L, VALID, 1);
ite.itel = FIELD_DP64(ite.itel, ITE_L, INTTYPE, ITE_INTTYPE_PHYSICAL);
ite.itel = FIELD_DP64(ite.itel, ITE_L, INTID, intid);
- ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS);
- ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, new_icid);
+ ite.itel = FIELD_DP64(ite.itel, ITE_L, ICID, new_icid);
+ ite.iteh = FIELD_DP32(ite.iteh, ITE_H, DOORBELL, INTID_SPURIOUS);
return update_ite(s, eventid, &dte, ite) ? CMD_CONTINUE : CMD_STALL;
}
--
2.25.1
- [PULL 09/39] hw/arm: imx: Don't enable PSCI conduit when booting guest in EL3, (continued)
- [PULL 09/39] hw/arm: imx: Don't enable PSCI conduit when booting guest in EL3, Peter Maydell, 2022/02/08
- [PULL 16/39] hw/arm/highbank: Drop use of secure_board_setup, Peter Maydell, 2022/02/08
- [PULL 23/39] arm: force flag recalculation when messing with DAIF, Peter Maydell, 2022/02/08
- [PULL 22/39] hw/arm: versal-virt: Always call arm_load_kernel(), Peter Maydell, 2022/02/08
- [PULL 20/39] hw/arm/boot: Drop nb_cpus field from arm_boot_info, Peter Maydell, 2022/02/08
- [PULL 21/39] hw/arm/boot: Drop existing dtb /psci node rather than retaining it, Peter Maydell, 2022/02/08
- [PULL 26/39] hw/intc/arm_gicv3_its: Use address_space_map() to access command queue packets, Peter Maydell, 2022/02/08
- [PULL 24/39] hw/timer/armv7m_systick: Update clock source before enabling timer, Peter Maydell, 2022/02/08
- [PULL 33/39] hw/intc/arm_gicv3_its: Pass ITE values back from get_ite() via a struct, Peter Maydell, 2022/02/08
- [PULL 30/39] hw/intc/arm_gicv3_its: Pass CTEntry to update_cte(), Peter Maydell, 2022/02/08
- [PULL 31/39] hw/intc/arm_gicv3_its: Fix address calculation in get_ite() and update_ite(),
Peter Maydell <=
- [PULL 18/39] hw/arm/boot: Don't write secondary boot stub if using PSCI, Peter Maydell, 2022/02/08
- [PULL 25/39] hw/arm/smmuv3: Fix device reset, Peter Maydell, 2022/02/08
- [PULL 36/39] hw/intc/arm_gicv3_its: In MAPC with V=0, don't check rdbase field, Peter Maydell, 2022/02/08
- [PULL 34/39] hw/intc/arm_gicv3_its: Make update_ite() use ITEntry, Peter Maydell, 2022/02/08
- [PULL 19/39] hw/arm/highbank: Drop unused secondary boot stub code, Peter Maydell, 2022/02/08
- [PULL 35/39] hw/intc/arm_gicv3_its: Drop TableDesc and CmdQDesc valid fields, Peter Maydell, 2022/02/08
- [PULL 37/39] hw/intc/arm_gicv3_its: Don't allow intid 1023 in MAPI/MAPTI, Peter Maydell, 2022/02/08
- [PULL 32/39] hw/intc/arm_gicv3_its: Avoid nested ifs in get_ite(), Peter Maydell, 2022/02/08
- [PULL 38/39] hw/intc/arm_gicv3_its: Split error checks, Peter Maydell, 2022/02/08
- Re: [PULL 00/39] target-arm queue, Peter Maydell, 2022/02/08