* [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification
@ 2025-06-17 15:04 Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 1/8] amd_iommu: Fix Miscellaneous Information Register 0 encoding Alejandro Jimenez
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
Added two new changes based on observations from Ethan. Like the rest of
the fixes in this series, these do not trigger problems today given the
limited feature set supported. Re-tested the series with emulated devices,
VFIO passthrough usage with amd-iommu is not possible since merge of commit
31753d5a336f ("hw/i386/amd_iommu: Fix device setup failure when PT is on.")
regardless of guest kernel iommu mode.
Changes since v3:
- Made an additional change in PATCH 1 with correct encoding for
AMDVI_MAX_GVA_ADDR, adding Ethan as co-author. Dropped Vasant R-b.
- Added patch by Ethan fixing truncation bug.
Thank you,
Alejandro
v3: https://lore.kernel.org/all/20250529193023.3590780-1-alejandro.j.jimenez@oracle.com/
Alejandro Jimenez (7):
amd_iommu: Fix Miscellaneous Information Register 0 encoding
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
Ethan Milon (1):
amd_iommu: Fix truncation of oldval in amdvi_writeq
hw/i386/amd_iommu.c | 17 +++++++------
hw/i386/amd_iommu.h | 59 ++++++++++++++++++++++-----------------------
2 files changed, 38 insertions(+), 38 deletions(-)
base-commit: a6f02277595136832c9e9bcaf447ab574f7b1128
--
2.43.5
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 1/8] amd_iommu: Fix Miscellaneous Information Register 0 encoding
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-26 17:53 ` Vasant Hegde
2025-06-17 15:04 ` [PATCH v4 2/8] amd_iommu: Fix Device ID decoding for INVALIDATE_IOTLB_PAGES command Alejandro Jimenez
` (7 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
The definitions encoding the maximum Virtual, Physical, and Guest Virtual
Address sizes supported by the IOMMU are using incorrect offsets i.e. the
VASize and GVASize offsets are switched. The value in the GVAsize field is
also modified, since it was incorrectly encoded.
Cc: qemu-stable@nongnu.org
Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
Co-developed-by: Ethan MILON <ethan.milon@eviden.com>
Signed-off-by: Ethan MILON <ethan.milon@eviden.com>
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
---
hw/i386/amd_iommu.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 5672bdef89071..3b1d2e9da5347 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -196,9 +196,9 @@
#define AMDVI_PAGE_SHIFT_4K 12
#define AMDVI_PAGE_MASK_4K (~((1ULL << AMDVI_PAGE_SHIFT_4K) - 1))
-#define AMDVI_MAX_VA_ADDR (48UL << 5)
-#define AMDVI_MAX_PH_ADDR (40UL << 8)
-#define AMDVI_MAX_GVA_ADDR (48UL << 15)
+#define AMDVI_MAX_GVA_ADDR (2UL << 5)
+#define AMDVI_MAX_PH_ADDR (40UL << 8)
+#define AMDVI_MAX_VA_ADDR (48UL << 15)
/* Completion Wait data size */
#define AMDVI_COMPLETION_DATA_SIZE 8
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 2/8] amd_iommu: Fix Device ID decoding for INVALIDATE_IOTLB_PAGES command
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 1/8] amd_iommu: Fix Miscellaneous Information Register 0 encoding Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 3/8] amd_iommu: Update bitmasks representing DTE reserved fields Alejandro Jimenez
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
The DeviceID bits are extracted using an incorrect offset in the call to
amdvi_iotlb_remove_page(). This field is read (correctly) earlier, so use
the value already retrieved for devid.
Cc: qemu-stable@nongnu.org
Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
hw/i386/amd_iommu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 963aa2450c997..c27efa504d19a 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -508,7 +508,7 @@ static void amdvi_inval_inttable(AMDVIState *s, uint64_t *cmd)
static void iommu_inval_iotlb(AMDVIState *s, uint64_t *cmd)
{
- uint16_t devid = extract64(cmd[0], 0, 16);
+ uint16_t devid = cpu_to_le16(extract64(cmd[0], 0, 16));
if (extract64(cmd[1], 1, 1) || extract64(cmd[1], 3, 1) ||
extract64(cmd[1], 6, 6)) {
amdvi_log_illegalcom_error(s, extract64(cmd[0], 60, 4),
@@ -521,7 +521,7 @@ static void iommu_inval_iotlb(AMDVIState *s, uint64_t *cmd)
&devid);
} else {
amdvi_iotlb_remove_page(s, cpu_to_le64(extract64(cmd[1], 12, 52)) << 12,
- cpu_to_le16(extract64(cmd[1], 0, 16)));
+ devid);
}
trace_amdvi_iotlb_inval();
}
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 3/8] amd_iommu: Update bitmasks representing DTE reserved fields
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 1/8] amd_iommu: Fix Miscellaneous Information Register 0 encoding Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 2/8] amd_iommu: Fix Device ID decoding for INVALIDATE_IOTLB_PAGES command Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 4/8] amd_iommu: Fix masks for various IOMMU MMIO Registers Alejandro Jimenez
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
The DTE validation method verifies that all bits in reserved DTE fields are
unset. Update them according to the latest definition available in AMD I/O
Virtualization Technology (IOMMU) Specification - Section 2.2.2.1 Device
Table Entry Format. Remove the magic numbers and use a macro helper to
generate bitmasks covering the specified ranges for better legibility.
Note that some reserved fields specify that events are generated when they
contain non-zero bits, or checks are skipped under certain configurations.
This change only updates the reserved masks, checks for special conditions
are not yet implemented.
Cc: qemu-stable@nongnu.org
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
hw/i386/amd_iommu.c | 7 ++++---
hw/i386/amd_iommu.h | 9 ++++++---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index c27efa504d19a..6e78047919cb6 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -848,9 +848,10 @@ static inline uint64_t amdvi_get_perms(uint64_t entry)
static bool amdvi_validate_dte(AMDVIState *s, uint16_t devid,
uint64_t *dte)
{
- if ((dte[0] & AMDVI_DTE_LOWER_QUAD_RESERVED)
- || (dte[1] & AMDVI_DTE_MIDDLE_QUAD_RESERVED)
- || (dte[2] & AMDVI_DTE_UPPER_QUAD_RESERVED) || dte[3]) {
+ if ((dte[0] & AMDVI_DTE_QUAD0_RESERVED) ||
+ (dte[1] & AMDVI_DTE_QUAD1_RESERVED) ||
+ (dte[2] & AMDVI_DTE_QUAD2_RESERVED) ||
+ (dte[3] & AMDVI_DTE_QUAD3_RESERVED)) {
amdvi_log_illegaldevtab_error(s, devid,
s->devtab +
devid * AMDVI_DEVTAB_ENTRY_SIZE, 0);
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 3b1d2e9da5347..aacb29b617aec 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -25,6 +25,8 @@
#include "hw/i386/x86-iommu.h"
#include "qom/object.h"
+#define GENMASK64(h, l) (((~0ULL) >> (63 - (h) + (l))) << (l))
+
/* Capability registers */
#define AMDVI_CAPAB_BAR_LOW 0x04
#define AMDVI_CAPAB_BAR_HIGH 0x08
@@ -162,9 +164,10 @@
#define AMDVI_FEATURE_PC (1ULL << 9) /* Perf counters */
/* reserved DTE bits */
-#define AMDVI_DTE_LOWER_QUAD_RESERVED 0x80300000000000fc
-#define AMDVI_DTE_MIDDLE_QUAD_RESERVED 0x0000000000000100
-#define AMDVI_DTE_UPPER_QUAD_RESERVED 0x08f0000000000000
+#define AMDVI_DTE_QUAD0_RESERVED (GENMASK64(6, 2) | GENMASK64(63, 63))
+#define AMDVI_DTE_QUAD1_RESERVED 0
+#define AMDVI_DTE_QUAD2_RESERVED GENMASK64(53, 52)
+#define AMDVI_DTE_QUAD3_RESERVED (GENMASK64(14, 0) | GENMASK64(53, 48))
/* AMDVI paging mode */
#define AMDVI_GATS_MODE (2ULL << 12)
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 4/8] amd_iommu: Fix masks for various IOMMU MMIO Registers
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
` (2 preceding siblings ...)
2025-06-17 15:04 ` [PATCH v4 3/8] amd_iommu: Update bitmasks representing DTE reserved fields Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 5/8] amd_iommu: Fix mask to retrieve Interrupt Table Root Pointer from DTE Alejandro Jimenez
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
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-stable@nongnu.org
Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Vasant Hegde <vasant.hegde@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 aacb29b617aec..988a485f808cc 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -68,34 +68,34 @@
#define AMDVI_MMIO_SIZE 0x4000
-#define AMDVI_MMIO_DEVTAB_SIZE_MASK ((1ULL << 12) - 1)
-#define AMDVI_MMIO_DEVTAB_BASE_MASK (((1ULL << 52) - 1) & ~ \
- AMDVI_MMIO_DEVTAB_SIZE_MASK)
+#define AMDVI_MMIO_DEVTAB_SIZE_MASK GENMASK64(8, 0)
+#define AMDVI_MMIO_DEVTAB_BASE_MASK GENMASK64(51, 12)
+
#define AMDVI_MMIO_DEVTAB_ENTRY_SIZE 32
#define AMDVI_MMIO_DEVTAB_SIZE_UNIT 4096
/* some of this are similar but just for readability */
#define AMDVI_MMIO_CMDBUF_SIZE_BYTE (AMDVI_MMIO_COMMAND_BASE + 7)
#define AMDVI_MMIO_CMDBUF_SIZE_MASK 0x0f
-#define AMDVI_MMIO_CMDBUF_BASE_MASK AMDVI_MMIO_DEVTAB_BASE_MASK
-#define AMDVI_MMIO_CMDBUF_HEAD_MASK (((1ULL << 19) - 1) & ~0x0f)
-#define AMDVI_MMIO_CMDBUF_TAIL_MASK AMDVI_MMIO_EVTLOG_HEAD_MASK
+#define AMDVI_MMIO_CMDBUF_BASE_MASK GENMASK64(51, 12)
+#define AMDVI_MMIO_CMDBUF_HEAD_MASK GENMASK64(18, 4)
+#define AMDVI_MMIO_CMDBUF_TAIL_MASK GENMASK64(18, 4)
#define AMDVI_MMIO_EVTLOG_SIZE_BYTE (AMDVI_MMIO_EVENT_BASE + 7)
-#define AMDVI_MMIO_EVTLOG_SIZE_MASK AMDVI_MMIO_CMDBUF_SIZE_MASK
-#define AMDVI_MMIO_EVTLOG_BASE_MASK AMDVI_MMIO_CMDBUF_BASE_MASK
-#define AMDVI_MMIO_EVTLOG_HEAD_MASK (((1ULL << 19) - 1) & ~0x0f)
-#define AMDVI_MMIO_EVTLOG_TAIL_MASK AMDVI_MMIO_EVTLOG_HEAD_MASK
+#define AMDVI_MMIO_EVTLOG_SIZE_MASK 0x0f
+#define AMDVI_MMIO_EVTLOG_BASE_MASK GENMASK64(51, 12)
+#define AMDVI_MMIO_EVTLOG_HEAD_MASK GENMASK64(18, 4)
+#define AMDVI_MMIO_EVTLOG_TAIL_MASK GENMASK64(18, 4)
-#define AMDVI_MMIO_PPRLOG_SIZE_BYTE (AMDVI_MMIO_EVENT_BASE + 7)
-#define AMDVI_MMIO_PPRLOG_HEAD_MASK AMDVI_MMIO_EVTLOG_HEAD_MASK
-#define AMDVI_MMIO_PPRLOG_TAIL_MASK AMDVI_MMIO_EVTLOG_HEAD_MASK
-#define AMDVI_MMIO_PPRLOG_BASE_MASK AMDVI_MMIO_EVTLOG_BASE_MASK
-#define AMDVI_MMIO_PPRLOG_SIZE_MASK AMDVI_MMIO_EVTLOG_SIZE_MASK
+#define AMDVI_MMIO_PPRLOG_SIZE_BYTE (AMDVI_MMIO_PPR_BASE + 7)
+#define AMDVI_MMIO_PPRLOG_SIZE_MASK 0x0f
+#define AMDVI_MMIO_PPRLOG_BASE_MASK GENMASK64(51, 12)
+#define AMDVI_MMIO_PPRLOG_HEAD_MASK GENMASK64(18, 4)
+#define AMDVI_MMIO_PPRLOG_TAIL_MASK GENMASK64(18, 4)
#define AMDVI_MMIO_EXCL_ENABLED_MASK (1ULL << 0)
#define AMDVI_MMIO_EXCL_ALLOW_MASK (1ULL << 1)
-#define AMDVI_MMIO_EXCL_LIMIT_MASK AMDVI_MMIO_DEVTAB_BASE_MASK
+#define AMDVI_MMIO_EXCL_LIMIT_MASK GENMASK64(51, 12)
#define AMDVI_MMIO_EXCL_LIMIT_LOW 0xfff
/* mmio control register flags */
@@ -132,14 +132,14 @@
#define AMDVI_DEV_TRANSLATION_VALID (1ULL << 1)
#define AMDVI_DEV_MODE_MASK 0x7
#define AMDVI_DEV_MODE_RSHIFT 9
-#define AMDVI_DEV_PT_ROOT_MASK 0xffffffffff000
+#define AMDVI_DEV_PT_ROOT_MASK GENMASK64(51, 12)
#define AMDVI_DEV_PT_ROOT_RSHIFT 12
#define AMDVI_DEV_PERM_SHIFT 61
#define AMDVI_DEV_PERM_READ (1ULL << 61)
#define AMDVI_DEV_PERM_WRITE (1ULL << 62)
/* Device table entry bits 64:127 */
-#define AMDVI_DEV_DOMID_ID_MASK ((1ULL << 16) - 1)
+#define AMDVI_DEV_DOMID_ID_MASK GENMASK64(15, 0)
/* Event codes and flags, as stored in the info field */
#define AMDVI_EVENT_ILLEGAL_DEVTAB_ENTRY (0x1U << 12)
@@ -197,7 +197,7 @@
#define AMDVI_PAGE_SIZE (1ULL << AMDVI_PAGE_SHIFT)
#define AMDVI_PAGE_SHIFT_4K 12
-#define AMDVI_PAGE_MASK_4K (~((1ULL << AMDVI_PAGE_SHIFT_4K) - 1))
+#define AMDVI_PAGE_MASK_4K GENMASK64(63, 12)
#define AMDVI_MAX_GVA_ADDR (2UL << 5)
#define AMDVI_MAX_PH_ADDR (40UL << 8)
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 5/8] amd_iommu: Fix mask to retrieve Interrupt Table Root Pointer from DTE
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
` (3 preceding siblings ...)
2025-06-17 15:04 ` [PATCH v4 4/8] amd_iommu: Fix masks for various IOMMU MMIO Registers Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 6/8] amd_iommu: Fix the calculation for Device Table size Alejandro Jimenez
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
Fix an off-by-one error in the definition of AMDVI_IR_PHYS_ADDR_MASK. The
current definition masks off the most significant bit of the Interrupt Table
Root ptr i.e. it only generates a mask with bits [50:6] set. See the AMD I/O
Virtualization Technology (IOMMU) Specification for the Interrupt Table
Root Pointer[51:6] field in the Device Table Entry format.
Cc: qemu-stable@nongnu.org
Fixes: b44159fe0078 ("x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled")
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
hw/i386/amd_iommu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 988a485f808cc..96fc5b621e609 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -231,7 +231,7 @@
#define AMDVI_IR_INTCTL_PASS 1
#define AMDVI_IR_INTCTL_REMAP 2
-#define AMDVI_IR_PHYS_ADDR_MASK (((1ULL << 45) - 1) << 6)
+#define AMDVI_IR_PHYS_ADDR_MASK GENMASK64(51, 6)
/* MSI data 10:0 bits (section 2.2.5.1 Fig 14) */
#define AMDVI_IRTE_OFFSET 0x7ff
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 6/8] amd_iommu: Fix the calculation for Device Table size
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
` (4 preceding siblings ...)
2025-06-17 15:04 ` [PATCH v4 5/8] amd_iommu: Fix mask to retrieve Interrupt Table Root Pointer from DTE Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 7/8] amd_iommu: Remove duplicated definitions Alejandro Jimenez
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
Correctly calculate the Device Table size using the format encoded in the
Device Table Base Address Register (MMIO Offset 0000h).
Cc: qemu-stable@nongnu.org
Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
hw/i386/amd_iommu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 6e78047919cb6..92f94dc788c3d 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -665,8 +665,8 @@ static inline void amdvi_handle_devtab_write(AMDVIState *s)
uint64_t val = amdvi_readq(s, AMDVI_MMIO_DEVICE_TABLE);
s->devtab = (val & AMDVI_MMIO_DEVTAB_BASE_MASK);
- /* set device table length */
- s->devtab_len = ((val & AMDVI_MMIO_DEVTAB_SIZE_MASK) + 1 *
+ /* set device table length (i.e. number of entries table can hold) */
+ s->devtab_len = (((val & AMDVI_MMIO_DEVTAB_SIZE_MASK) + 1) *
(AMDVI_MMIO_DEVTAB_SIZE_UNIT /
AMDVI_MMIO_DEVTAB_ENTRY_SIZE));
}
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 7/8] amd_iommu: Remove duplicated definitions
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
` (5 preceding siblings ...)
2025-06-17 15:04 ` [PATCH v4 6/8] amd_iommu: Fix the calculation for Device Table size Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 8/8] amd_iommu: Fix truncation of oldval in amdvi_writeq Alejandro Jimenez
2025-06-26 17:55 ` [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Vasant Hegde
8 siblings, 0 replies; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
No functional change.
Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
hw/i386/amd_iommu.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 96fc5b621e609..8b42913ed8dab 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -206,10 +206,6 @@
/* Completion Wait data size */
#define AMDVI_COMPLETION_DATA_SIZE 8
-#define AMDVI_COMMAND_SIZE 16
-/* Completion Wait data size */
-#define AMDVI_COMPLETION_DATA_SIZE 8
-
#define AMDVI_COMMAND_SIZE 16
#define AMDVI_INT_ADDR_FIRST 0xfee00000
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 8/8] amd_iommu: Fix truncation of oldval in amdvi_writeq
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
` (6 preceding siblings ...)
2025-06-17 15:04 ` [PATCH v4 7/8] amd_iommu: Remove duplicated definitions Alejandro Jimenez
@ 2025-06-17 15:04 ` Alejandro Jimenez
2025-06-26 12:36 ` Vasant Hegde
2025-06-26 17:55 ` [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Vasant Hegde
8 siblings, 1 reply; 12+ messages in thread
From: Alejandro Jimenez @ 2025-06-17 15:04 UTC (permalink / raw)
To: qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, vasant.hegde, suravee.suthikulpanit,
santosh.shukla, sarunkod, brijesh.singh, joao.m.martins,
boris.ostrovsky, alejandro.j.jimenez, philmd
From: Ethan Milon <ethan.milon@eviden.com>
The variable `oldval` was incorrectly declared as a 32-bit `uint32_t`.
This could lead to truncation and incorrect behavior where the upper
read-only 32 bits are significant.
Fix the type of `oldval` to match the return type of `ldq_le_p()`.
Cc: qemu-stable@nongnu.org
Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: Ethan Milon <ethan.milon@eviden.com>
---
hw/i386/amd_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 92f94dc788c3d..5a24c17548d45 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));
}
--
2.43.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 8/8] amd_iommu: Fix truncation of oldval in amdvi_writeq
2025-06-17 15:04 ` [PATCH v4 8/8] amd_iommu: Fix truncation of oldval in amdvi_writeq Alejandro Jimenez
@ 2025-06-26 12:36 ` Vasant Hegde
0 siblings, 0 replies; 12+ messages in thread
From: Vasant Hegde @ 2025-06-26 12:36 UTC (permalink / raw)
To: Alejandro Jimenez, qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, suravee.suthikulpanit, santosh.shukla,
sarunkod, brijesh.singh, joao.m.martins, boris.ostrovsky, philmd
On 6/17/2025 8:34 PM, Alejandro Jimenez wrote:
> From: Ethan Milon <ethan.milon@eviden.com>
>
> The variable `oldval` was incorrectly declared as a 32-bit `uint32_t`.
> This could lead to truncation and incorrect behavior where the upper
> read-only 32 bits are significant.
>
> Fix the type of `oldval` to match the return type of `ldq_le_p()`.
>
> Cc: qemu-stable@nongnu.org
> Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
> Signed-off-by: Ethan Milon <ethan.milon@eviden.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
-Vasant
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/8] amd_iommu: Fix Miscellaneous Information Register 0 encoding
2025-06-17 15:04 ` [PATCH v4 1/8] amd_iommu: Fix Miscellaneous Information Register 0 encoding Alejandro Jimenez
@ 2025-06-26 17:53 ` Vasant Hegde
0 siblings, 0 replies; 12+ messages in thread
From: Vasant Hegde @ 2025-06-26 17:53 UTC (permalink / raw)
To: Alejandro Jimenez, qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, suravee.suthikulpanit, santosh.shukla,
sarunkod, brijesh.singh, joao.m.martins, boris.ostrovsky, philmd
On 6/17/2025 8:34 PM, Alejandro Jimenez wrote:
> The definitions encoding the maximum Virtual, Physical, and Guest Virtual
> Address sizes supported by the IOMMU are using incorrect offsets i.e. the
> VASize and GVASize offsets are switched. The value in the GVAsize field is
> also modified, since it was incorrectly encoded.
>
> Cc: qemu-stable@nongnu.org
> Fixes: d29a09ca6842 ("hw/i386: Introduce AMD IOMMU")
> Co-developed-by: Ethan MILON <ethan.milon@eviden.com>
> Signed-off-by: Ethan MILON <ethan.milon@eviden.com>
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
-Vasant
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
` (7 preceding siblings ...)
2025-06-17 15:04 ` [PATCH v4 8/8] amd_iommu: Fix truncation of oldval in amdvi_writeq Alejandro Jimenez
@ 2025-06-26 17:55 ` Vasant Hegde
8 siblings, 0 replies; 12+ messages in thread
From: Vasant Hegde @ 2025-06-26 17:55 UTC (permalink / raw)
To: Alejandro Jimenez, qemu-devel
Cc: ethan.milon, mst, pbonzini, mjt, marcel.apfelbaum,
richard.henderson, eduardo, suravee.suthikulpanit, santosh.shukla,
sarunkod, brijesh.singh, joao.m.martins, boris.ostrovsky, philmd
Hi Michael,
On 6/17/2025 8:34 PM, Alejandro Jimenez wrote:
> Added two new changes based on observations from Ethan. Like the rest of
> the fixes in this series, these do not trigger problems today given the
> limited feature set supported. Re-tested the series with emulated devices,
> VFIO passthrough usage with amd-iommu is not possible since merge of commit
> 31753d5a336f ("hw/i386/amd_iommu: Fix device setup failure when PT is on.")
> regardless of guest kernel iommu mode.
I have reviewed this series. It looks good to me.
-Vasant
>
> Changes since v3:
> - Made an additional change in PATCH 1 with correct encoding for
> AMDVI_MAX_GVA_ADDR, adding Ethan as co-author. Dropped Vasant R-b.
> - Added patch by Ethan fixing truncation bug.
>
> Thank you,
> Alejandro
>
> v3: https://lore.kernel.org/all/20250529193023.3590780-1-alejandro.j.jimenez@oracle.com/
>
> Alejandro Jimenez (7):
> amd_iommu: Fix Miscellaneous Information Register 0 encoding
> 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
>
> Ethan Milon (1):
> amd_iommu: Fix truncation of oldval in amdvi_writeq
>
> hw/i386/amd_iommu.c | 17 +++++++------
> hw/i386/amd_iommu.h | 59 ++++++++++++++++++++++-----------------------
> 2 files changed, 38 insertions(+), 38 deletions(-)
>
>
> base-commit: a6f02277595136832c9e9bcaf447ab574f7b1128
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-06-26 17:55 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-17 15:04 [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 1/8] amd_iommu: Fix Miscellaneous Information Register 0 encoding Alejandro Jimenez
2025-06-26 17:53 ` Vasant Hegde
2025-06-17 15:04 ` [PATCH v4 2/8] amd_iommu: Fix Device ID decoding for INVALIDATE_IOTLB_PAGES command Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 3/8] amd_iommu: Update bitmasks representing DTE reserved fields Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 4/8] amd_iommu: Fix masks for various IOMMU MMIO Registers Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 5/8] amd_iommu: Fix mask to retrieve Interrupt Table Root Pointer from DTE Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 6/8] amd_iommu: Fix the calculation for Device Table size Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 7/8] amd_iommu: Remove duplicated definitions Alejandro Jimenez
2025-06-17 15:04 ` [PATCH v4 8/8] amd_iommu: Fix truncation of oldval in amdvi_writeq Alejandro Jimenez
2025-06-26 12:36 ` Vasant Hegde
2025-06-26 17:55 ` [PATCH v4 0/8] amd_iommu: Fixes to align with AMDVi specification Vasant Hegde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).