On 5/29/25 1:23 AM, Vasant Hegde wrote:
On 5/29/2025 3:47 AM, Alejandro Jimenez wrote:
Address various issues with definitions of the MMIO registers e.g. for the
Device Table Address Register, the size mask currently encompasses reserved
bits [11:9], so change it to only extract the bits [8:0] encoding size.
Convert masks to use GENMASK64 for consistency, and make unrelated
definitions independent.
Cc: qemu-sta...@nongnu.org
Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: Alejandro Jimenez <alejandro.j.jime...@oracle.com>
This patch makes it easy to read macros! Thanks.
Easier is the goal :). I was tempted to try to remove duplication since
these registers all use similar offsets for their fields, but I figured
it is better to keep them independent in case they diverge in future
revisions.
Reviewed-by: Vasant Hegde <vasant.he...@amd.com>
---
hw/i386/amd_iommu.h | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 45a997af861e6..09352672bdcc2 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -68,34 +68,34 @@
-#define AMDVI_MMIO_PPRLOG_SIZE_BYTE (AMDVI_MMIO_EVENT_BASE + 7)
Unrelated to this patch.. I think we should just read the AMDVI_MMIO_EVENT_BASE
register and extract the length. Same for rest of the buffer. We can do that as
improvement later.
Agree. The current approach is technically a bug since it is also
reading data from the next MMIO offset, and breaking the requirement
from 3.4 IOMMU MMIO Registers that says:
"Accesses must be aligned to the size of the access..."
I looked into that area because I was initially confused by the
*_SIZE_BYTE definitions (it didn't help that the one above is incorrect
since it uses the wrong starting register).
I considered renaming them to use something like *_LEN_OFFSET instead
since that would match the naming in the spec more closely, but decided
to limit changes to the header. And with the correct fix as you mention
those definitions won't be needed anyways.
Thank you,
Alejandro
-Vasant
