All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ugur Usug <ugurus@amazon.com>
To: <qemu-devel@nongnu.org>
Cc: <mst@redhat.com>, <marcel.apfelbaum@gmail.com>,
	<pbonzini@redhat.com>, <richard.henderson@linaro.org>,
	<eduardo@habkost.net>, Ugur Usug <ugurus@amazon.com>
Subject: [PATCH] hw/i386/amd_iommu: Do not depend on any pci cap ordering
Date: Mon, 29 Jul 2024 11:10:18 +0000	[thread overview]
Message-ID: <20240729111018.87536-1-ugurus@amazon.com> (raw)

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



                 reply	other threads:[~2024-07-29 12:51 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240729111018.87536-1-ugurus@amazon.com \
    --to=ugurus@amazon.com \
    --cc=eduardo@habkost.net \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.