qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 06/13] hw/intc/arm_gicv3_its: Fix address calculation in get_


From: Richard Henderson
Subject: Re: [PATCH 06/13] hw/intc/arm_gicv3_its: Fix address calculation in get_ite() and update_ite()
Date: Thu, 3 Feb 2022 14:59:13 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0

On 2/2/22 06:32, Peter Maydell wrote:
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>
---
  hw/intc/gicv3_internal.h | 19 ++++++++++---------
  hw/intc/arm_gicv3_its.c  | 28 +++++++++++-----------------
  2 files changed, 21 insertions(+), 26 deletions(-)

This is confusing: 5-3 is titled "example of the number of bits that might be stored in an ITE"? Surely there must be a true architected format for this table, the one real hardware uses. Surely tcg will simply have to suck it up and break migration to fix this properly.


r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]