[PATCH] hw/i386/amd_iommu: Do not depend on any pci cap ordering

Ugur Usug posted 1 patch 3 months, 4 weeks ago
hw/i386/amd_iommu.c |  3 ++-
hw/i386/amd_iommu.h | 22 +++++++++++++---------
2 files changed, 15 insertions(+), 10 deletions(-)
[PATCH] hw/i386/amd_iommu: Do not depend on any pci cap ordering
Posted by Ugur Usug 3 months, 4 weeks ago
Currently, pci capabilities are inserted in the reverse order
capabilities are added (see 'pci_add_capability'). amd_iommu hardware
pci realization depends on this implicitly. It assumes that the first
capability added will be the last one (at the tail) and its respective
'PCI_CAP_LIST_NEXT' is zero. This is implicit in 'AMDVI_CAPAB_FEATURES'
in which the next pointer is assumed as zero. However, this assumption
can be both fragile for any update in the future and not easy to spot or
understand.

Given that the PCI spec. has no specific rule for ordering of
capabilities, with this commit we don't assume anything related with the
ordering in pci subsystem and treat feature flag (upper 16-bits portion)
of the capability header separately.

Signed-off-by: Ugur Usug <ugurus@amazon.com>
---
 hw/i386/amd_iommu.c |  3 ++-
 hw/i386/amd_iommu.h | 22 +++++++++++++---------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 6d4fde72f9b..791c4edc704 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1558,7 +1558,8 @@ static void amdvi_pci_realize(PCIDevice *pdev, Error **errp)
     pci_config_set_prog_interface(pdev->config, 0);
 
     /* reset AMDVI specific capabilities, all r/o */
-    pci_set_long(pdev->config + s->capab_offset, AMDVI_CAPAB_FEATURES);
+    pci_set_word(pdev->config + s->capab_offset + AMDVI_CAPAB_FEAT_REG,
+                 AMDVI_CAPAB_FEATURES);
     pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_BAR_LOW,
                  AMDVI_BASE_ADDR & ~(0xffff0000));
     pci_set_long(pdev->config + s->capab_offset + AMDVI_CAPAB_BAR_HIGH,
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 73619fe9eaa..513c8f7a9cf 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -26,6 +26,7 @@
 #include "qom/object.h"
 
 /* Capability registers */
+#define AMDVI_CAPAB_FEAT_REG          0x02
 #define AMDVI_CAPAB_BAR_LOW           0x04
 #define AMDVI_CAPAB_BAR_HIGH          0x08
 #define AMDVI_CAPAB_RANGE             0x0C
@@ -34,14 +35,17 @@
 #define AMDVI_CAPAB_SIZE              0x18
 #define AMDVI_CAPAB_REG_SIZE          0x04
 
-/* Capability header data */
+/*
+ * Capability header data which covers the capability id and the feature
+ * flags reside at upper 16-bits portion of the header.
+ */
 #define AMDVI_CAPAB_ID_SEC            0xf
-#define AMDVI_CAPAB_FLAT_EXT          (1 << 28)
-#define AMDVI_CAPAB_EFR_SUP           (1 << 27)
-#define AMDVI_CAPAB_FLAG_NPCACHE      (1 << 26)
-#define AMDVI_CAPAB_FLAG_HTTUNNEL     (1 << 25)
-#define AMDVI_CAPAB_FLAG_IOTLBSUP     (1 << 24)
-#define AMDVI_CAPAB_INIT_TYPE         (3 << 16)
+#define AMDVI_CAPAB_FLAT_EXT          (1 << 12)
+#define AMDVI_CAPAB_EFR_SUP           (1 << 11)
+#define AMDVI_CAPAB_FLAG_NPCACHE      (1 << 10)
+#define AMDVI_CAPAB_FLAG_HTTUNNEL     (1 << 9)
+#define AMDVI_CAPAB_FLAG_IOTLBSUP     (1 << 8)
+#define AMDVI_CAPAB_INIT_TYPE         (3 << 0)
 
 /* No. of used MMIO registers */
 #define AMDVI_MMIO_REGS_HIGH  7
@@ -183,8 +187,8 @@
 /* capabilities header */
 #define AMDVI_CAPAB_FEATURES (AMDVI_CAPAB_FLAT_EXT | \
         AMDVI_CAPAB_FLAG_NPCACHE | AMDVI_CAPAB_FLAG_IOTLBSUP \
-        | AMDVI_CAPAB_ID_SEC | AMDVI_CAPAB_INIT_TYPE | \
-        AMDVI_CAPAB_FLAG_HTTUNNEL |  AMDVI_CAPAB_EFR_SUP)
+        | AMDVI_CAPAB_INIT_TYPE | AMDVI_CAPAB_FLAG_HTTUNNEL \
+        | AMDVI_CAPAB_EFR_SUP)
 
 /* AMDVI default address */
 #define AMDVI_BASE_ADDR 0xfed80000
-- 
2.40.1