Hi Alejandro,
On 12/6/25 22:59, Alejandro Jimenez wrote:
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).
I agree with Ethan it is better to avoid hidden truncation, because it
just makes debugging experience harder.
If this is the expected behavior, better add a comment, or use
extract64() which makes the truncation explicit.
Regards,
Phil.
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
