mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

hw/intc/arm_gicv3_its: Fix address calculation in get_ite() and update_ite()

Browse Source 
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
This commit is contained in:
Peter Maydell 2022-02-01 19:32:00 +00:00
parent 06985cc3fe
commit a1ce993da6
2 changed files with 21 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
hw/intc/arm_gicv3_its.c
View File

@ -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;
}

19
hw/intc/gicv3_internal.h
View File

@ -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