From: Alejandro Jimenez <[email protected]>
Interrupt remapping table entries are data stored in guest memory in
little-endian format. Decoding them with bitfields depends on host bitfield
layout and the value returned from dma_memory_read() is not portable to
big-endian hosts.
Replace the legacy and GA IRTE bitfield definitions with explicit FIELD()
definitions. Convert the guest memory values returned from dma_memory_read()
with le32_to_cpu() or le64_to_cpu(), then extract relevant fields using
FIELD_EX32() or FIELD_EX64() as appropriate to match the IRTE format.
Fixes: b44159fe0078 ("x86_iommu/amd: Add interrupt remap support when VAPIC is
not enabled")
Fixes: 135f866e609c ("x86_iommu/amd: Add interrupt remap support when VAPIC is
enabled")
Reported-by: Peter Maydell <[email protected]>
Suggested-by: Peter Maydell <[email protected]>
Signed-off-by: Alejandro Jimenez <[email protected]>
Reviewed-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Reviewed-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Message-ID: <[email protected]>
---
hw/i386/amd_iommu.h | 49 ------------------------
hw/i386/amd_iommu.c | 93 +++++++++++++++++++++++++++++++++------------
2 files changed, 69 insertions(+), 73 deletions(-)
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index ca4440a4c1..687691ec1c 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -291,55 +291,6 @@
#define AMDVI_DEV_LINT0_PASS_MASK (1ULL << 62)
#define AMDVI_DEV_LINT1_PASS_MASK (1ULL << 63)
-/* Interrupt remapping table fields (Guest VAPIC not enabled) */
-union irte {
- uint32_t val;
- struct {
- uint32_t valid:1,
- no_fault:1,
- int_type:3,
- rq_eoi:1,
- dm:1,
- guest_mode:1,
- destination:8,
- vector:8,
- rsvd:8;
- } fields;
-};
-
-/* Interrupt remapping table fields (Guest VAPIC is enabled) */
-union irte_ga_lo {
- uint64_t val;
-
- /* For int remapping */
- struct {
- uint64_t valid:1,
- no_fault:1,
- /* ------ */
- int_type:3,
- rq_eoi:1,
- dm:1,
- /* ------ */
- guest_mode:1,
- destination:24,
- rsvd_1:32;
- } fields_remap;
-};
-
-union irte_ga_hi {
- uint64_t val;
- struct {
- uint64_t vector:8,
- rsvd_2:48,
- destination_hi:8;
- } fields;
-};
-
-struct irte_ga {
- union irte_ga_lo lo;
- union irte_ga_hi hi;
-};
-
#define TYPE_AMD_IOMMU_DEVICE "amd-iommu"
OBJECT_DECLARE_SIMPLE_TYPE(AMDVIState, AMD_IOMMU_DEVICE)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 291cb368a9..c7bf21b762 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -89,6 +89,11 @@ typedef struct AMDVIIOTLBKey {
uint16_t devid;
} AMDVIIOTLBKey;
+typedef struct AMDVIIrteGA {
+ uint64_t ga_lo;
+ uint64_t ga_hi;
+} AMDVIIrteGA;
+
/* XT IOMMU General Interrupt Control Register layout */
FIELD(AMDVI_XT_GEN_INTR, DEST_MODE, 2, 1)
FIELD(AMDVI_XT_GEN_INTR, DEST_LO, 8, 24)
@@ -96,6 +101,37 @@ FIELD(AMDVI_XT_GEN_INTR, VECTOR, 32, 8)
FIELD(AMDVI_XT_GEN_INTR, DELIVERY_MODE, 40, 1)
FIELD(AMDVI_XT_GEN_INTR, DEST_HI, 56, 8)
+/* Interrupt Remapping Table Fields Formats */
+
+/* Basic 32-bit IRTE layout (GAEn=0) */
+FIELD(AMDVI_IRTE, VALID, 0, 1)
+FIELD(AMDVI_IRTE, SUP_IOPF, 1, 1)
+FIELD(AMDVI_IRTE, INT_TYPE, 2, 3)
+FIELD(AMDVI_IRTE, RQ_EOI, 5, 1)
+FIELD(AMDVI_IRTE, DM, 6, 1)
+FIELD(AMDVI_IRTE, GUEST_MODE, 7, 1)
+FIELD(AMDVI_IRTE, DESTINATION, 8, 8)
+FIELD(AMDVI_IRTE, VECTOR, 16, 8)
+
+/* 128-bit IRTE layout (GAEn=1) */
+FIELD(AMDVI_IRTE_GA_LO, VALID, 0, 1)
+FIELD(AMDVI_IRTE_GA_LO, SUP_IOPF, 1, 1)
+FIELD(AMDVI_IRTE_GA_LO, INT_TYPE, 2, 3)
+FIELD(AMDVI_IRTE_GA_LO, RQ_EOI, 5, 1)
+FIELD(AMDVI_IRTE_GA_LO, DM, 6, 1)
+FIELD(AMDVI_IRTE_GA_LO, GUEST_MODE, 7, 1)
+/*
+ * In the 128-bit IRTE format, XT mode uses IRTE_GA_LOW.Destination[23:0]
+ * together with IRTE_GA_HI.DestinationHi[7:0] to construct a 32-bit x2APIC
+ * destination.
+ * Without XTEn (i.e. when x2APIC support is not enabled), only
+ * IRTE_GA_LOW.Destination[7:0] is used.
+ */
+FIELD(AMDVI_IRTE_GA_LO, DESTINATION, 8, 24)
+
+FIELD(AMDVI_IRTE_GA_HI, VECTOR, 0, 8)
+FIELD(AMDVI_IRTE_GA_HI, DESTINATION_HI, 56, 8)
+
uint64_t amdvi_extended_feature_register(AMDVIState *s)
{
uint64_t feature = AMDVI_DEFAULT_EXT_FEATURES;
@@ -1983,7 +2019,7 @@ static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion
*iommu, hwaddr addr,
}
static int amdvi_get_irte(AMDVIState *s, MSIMessage *origin, uint64_t *dte,
- union irte *irte, uint16_t devid)
+ uint32_t *irte, uint16_t devid)
{
uint64_t irte_root, offset;
@@ -1998,7 +2034,8 @@ static int amdvi_get_irte(AMDVIState *s, MSIMessage
*origin, uint64_t *dte,
return -AMDVI_IR_GET_IRTE;
}
- trace_amdvi_ir_irte_val(irte->val);
+ *irte = le32_to_cpu(*irte);
+ trace_amdvi_ir_irte_val(*irte);
return 0;
}
@@ -2010,8 +2047,9 @@ static int amdvi_int_remap_legacy(AMDVIState *iommu,
X86IOMMUIrq *irq,
uint16_t sid)
{
+ uint8_t int_type;
+ uint32_t irte;
int ret;
- union irte irte;
/* get interrupt remapping table */
ret = amdvi_get_irte(iommu, origin, dte, &irte, sid);
@@ -2019,32 +2057,33 @@ static int amdvi_int_remap_legacy(AMDVIState *iommu,
return ret;
}
- if (!irte.fields.valid) {
+ if (!FIELD_EX32(irte, AMDVI_IRTE, VALID)) {
trace_amdvi_ir_target_abort("RemapEn is disabled");
return -AMDVI_IR_TARGET_ABORT;
}
- if (irte.fields.guest_mode) {
+ if (FIELD_EX32(irte, AMDVI_IRTE, GUEST_MODE)) {
error_report_once("guest mode is not zero");
return -AMDVI_IR_ERR;
}
- if (irte.fields.int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
+ int_type = FIELD_EX32(irte, AMDVI_IRTE, INT_TYPE);
+ if (int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
error_report_once("reserved int_type");
return -AMDVI_IR_ERR;
}
- irq->delivery_mode = irte.fields.int_type;
- irq->vector = irte.fields.vector;
- irq->dest_mode = irte.fields.dm;
- irq->redir_hint = irte.fields.rq_eoi;
- irq->dest = irte.fields.destination;
+ irq->delivery_mode = int_type;
+ irq->vector = FIELD_EX32(irte, AMDVI_IRTE, VECTOR);
+ irq->dest_mode = FIELD_EX32(irte, AMDVI_IRTE, DM);
+ irq->redir_hint = FIELD_EX32(irte, AMDVI_IRTE, RQ_EOI);
+ irq->dest = FIELD_EX32(irte, AMDVI_IRTE, DESTINATION);
return 0;
}
static int amdvi_get_irte_ga(AMDVIState *s, MSIMessage *origin, uint64_t *dte,
- struct irte_ga *irte, uint16_t devid)
+ AMDVIIrteGA *irte, uint16_t devid)
{
uint64_t irte_root, offset;
@@ -2058,7 +2097,9 @@ static int amdvi_get_irte_ga(AMDVIState *s, MSIMessage
*origin, uint64_t *dte,
return -AMDVI_IR_GET_IRTE;
}
- trace_amdvi_ir_irte_ga_val(irte->hi.val, irte->lo.val);
+ irte->ga_lo = le64_to_cpu(irte->ga_lo);
+ irte->ga_hi = le64_to_cpu(irte->ga_hi);
+ trace_amdvi_ir_irte_ga_val(irte->ga_hi, irte->ga_lo);
return 0;
}
@@ -2069,8 +2110,9 @@ static int amdvi_int_remap_ga(AMDVIState *iommu,
X86IOMMUIrq *irq,
uint16_t sid)
{
+ AMDVIIrteGA irte;
+ uint8_t int_type;
int ret;
- struct irte_ga irte;
/* get interrupt remapping table */
ret = amdvi_get_irte_ga(iommu, origin, dte, &irte, sid);
@@ -2078,30 +2120,33 @@ static int amdvi_int_remap_ga(AMDVIState *iommu,
return ret;
}
- if (!irte.lo.fields_remap.valid) {
+ if (!FIELD_EX64(irte.ga_lo, AMDVI_IRTE_GA_LO, VALID)) {
trace_amdvi_ir_target_abort("RemapEn is disabled");
return -AMDVI_IR_TARGET_ABORT;
}
- if (irte.lo.fields_remap.guest_mode) {
+ if (FIELD_EX64(irte.ga_lo, AMDVI_IRTE_GA_LO, GUEST_MODE)) {
error_report_once("guest mode is not zero");
return -AMDVI_IR_ERR;
}
- if (irte.lo.fields_remap.int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
+ int_type = FIELD_EX64(irte.ga_lo, AMDVI_IRTE_GA_LO, INT_TYPE);
+ if (int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
error_report_once("reserved int_type is set");
return -AMDVI_IR_ERR;
}
- irq->delivery_mode = irte.lo.fields_remap.int_type;
- irq->vector = irte.hi.fields.vector;
- irq->dest_mode = irte.lo.fields_remap.dm;
- irq->redir_hint = irte.lo.fields_remap.rq_eoi;
+ irq->delivery_mode = int_type;
+ irq->vector = FIELD_EX64(irte.ga_hi, AMDVI_IRTE_GA_HI, VECTOR);
+ irq->dest_mode = FIELD_EX64(irte.ga_lo, AMDVI_IRTE_GA_LO, DM);
+ irq->redir_hint = FIELD_EX64(irte.ga_lo, AMDVI_IRTE_GA_LO, RQ_EOI);
if (iommu->xten) {
- irq->dest = irte.lo.fields_remap.destination |
- (irte.hi.fields.destination_hi << 24);
+ irq->dest = FIELD_EX64(irte.ga_lo, AMDVI_IRTE_GA_LO, DESTINATION) |
+ (FIELD_EX64(irte.ga_hi, AMDVI_IRTE_GA_HI, DESTINATION_HI)
+ << 24);
} else {
- irq->dest = irte.lo.fields_remap.destination & 0xff;
+ irq->dest = FIELD_EX64(irte.ga_lo, AMDVI_IRTE_GA_LO, DESTINATION) &
+ 0xff;
}
return 0;
--
MST
