Hi Ethan,
On 6/12/25 4:36 AM, Ethan MILON wrote:
Hi,
Is this series the right place to include the following minor fix?
I would defer this change for two reasons:
1) This series has been reviewed and tested already. I was hoping it
would be included on the Jun 1st pull but I sent v3 too late for that. I
think it is ready so I would like to leave it as is unless there are any
objections ...
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 0775c..18d30e1 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -140,7 +140,7 @@ static void amdvi_writeq(AMDVIState *s, hwaddr addr,
uint64_t val)
{
uint64_t romask = ldq_le_p(&s->romask[addr]);
uint64_t w1cmask = ldq_le_p(&s->w1cmask[addr]);
- uint32_t oldval = ldq_le_p(&s->mmior[addr]);
+ uint64_t oldval = ldq_le_p(&s->mmior[addr]);
stq_le_p(&s->mmior[addr],
((oldval & romask) | (val & ~romask)) & ~(val & w1cmask));
}
This corrects the type of oldval to match the return type of ldq_le_p().
2) This fix is needed, but it is likely better as part of additional
changes that are needed to cleanup/fix the XTSup support. i.e. there are
unhandled writes to the 0x170, 0x178, and 0x180 MMIO offsets, and those
depend on MMIO 0x18[IntCapXTEn]=1. I think the truncation of oldval that
you found is causing XTEn and IntCapXTEn bits on the control registers
to be ignored, but ultimately things are not broken enough (yet). In
other words, I think there is a lot more work to do in here, and it is
something I am looking into.
I suspect Vasant might have spotted this problem already, so he might
even have some fixes queued up...
That being said, if you want to send a patch with your S-b I'll add it
to this series.
Alejandro
Thanks,
Ethan
On 5/29/25 9:30 PM, Alejandro Jimenez wrote:
Caution: External email. Do not open attachments or click links, unless this
email comes from a known sender and you know the content is safe.
The main reason for sending this new revision so soon is that v2 included a
duplicated [PATCH 5/7]. I fixed a typo in the commit subject and missed
removing the old patch. Apologies for the mistake.
Additional changes in v3:
- Fixed typo on [PATCH 1/7] subject line (s/Miscellanous/Miscellaneous/).
- Added 'Fixes:' tag to [PATCH 5/7].
- Added Vasant's R-b to patches 4,5,7.
Thank you,
Alejandro
v2:
https://lore.kernel.org/qemu-devel/20250528221725.3554040-1-alejandro.j.jime...@oracle.com/
v1:
https://lore.kernel.org/all/20250311152446.45086-1-alejandro.j.jime...@oracle.com/
Alejandro Jimenez (7):
amd_iommu: Fix Miscellaneous Information Register 0 offsets
amd_iommu: Fix Device ID decoding for INVALIDATE_IOTLB_PAGES command
amd_iommu: Update bitmasks representing DTE reserved fields
amd_iommu: Fix masks for various IOMMU MMIO Registers
amd_iommu: Fix mask to retrieve Interrupt Table Root Pointer from DTE
amd_iommu: Fix the calculation for Device Table size
amd_iommu: Remove duplicated definitions
hw/i386/amd_iommu.c | 15 ++++++------
hw/i386/amd_iommu.h | 59 ++++++++++++++++++++++-----------------------
2 files changed, 37 insertions(+), 37 deletions(-)
base-commit: 80db93b2b88f9b3ed8927ae7ac74ca30e643a83e
--
2.43.5
