On 7/24/2025 2:20 PM, Philippe Mathieu-Daudé wrote:
On 24/7/25 08:47, Sairaj Kodilkar wrote:
amdvi_write*() function do not preserve the older values of W1C bits in
the MMIO register. This results in all W1C bits set to 0, when guest
tries to reset a single bit by writing 1 to it. Fix this by preserving
W1C bits in the old value of the MMIO register.
Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU")
Suggested-by: Ethan MILON <ethan.mi...@eviden.com>
Signed-off-by: Sairaj Kodilkar <sarun...@amd.com>
---
hw/i386/amd_iommu.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 2183510d227c..bbffd07b4e48 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -124,7 +124,8 @@ static void amdvi_writew(AMDVIState *s, hwaddr
addr, uint16_t val)
uint16_t w1cmask = lduw_le_p(&s->w1cmask[addr]);
uint16_t oldval = lduw_le_p(&s->mmior[addr]);
stw_le_p(&s->mmior[addr],
- ((oldval & romask) | (val & ~romask)) & ~(val & w1cmask));
+ ((oldval & (romask | w1cmask)) |
+ (val & ~romask)) & ~(val & w1cmask));
High code complexity, hard to review. Do you mind introducing
intermediate well-named variables for clarity?
Sure !
}
static void amdvi_writel(AMDVIState *s, hwaddr addr, uint32_t val)
@@ -133,7 +134,8 @@ static void amdvi_writel(AMDVIState *s, hwaddr
addr, uint32_t val)
uint32_t w1cmask = ldl_le_p(&s->w1cmask[addr]);
uint32_t oldval = ldl_le_p(&s->mmior[addr]);
stl_le_p(&s->mmior[addr],
- ((oldval & romask) | (val & ~romask)) & ~(val & w1cmask));
+ ((oldval & (romask | w1cmask)) |
+ (val & ~romask)) & ~(val & w1cmask));
}
static void amdvi_writeq(AMDVIState *s, hwaddr addr, uint64_t val)
@@ -142,7 +144,8 @@ static void amdvi_writeq(AMDVIState *s, hwaddr
addr, uint64_t val)
uint64_t w1cmask = ldq_le_p(&s->w1cmask[addr]);
uint64_t oldval = ldq_le_p(&s->mmior[addr]);
stq_le_p(&s->mmior[addr],
- ((oldval & romask) | (val & ~romask)) & ~(val & w1cmask));
+ ((oldval & (romask | w1cmask)) |
+ (val & ~romask)) & ~(val & w1cmask));
}
/* OR a 64-bit register with a 64-bit value */
