qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Rakesh Jeyasingh <rakeshjb010@gmail.com>
To: qemu-devel@nongnu.org, philmd@linaro.org, thuth@redhat.com
Cc: pbonzini@redhat.com, balaton@eik.bme.hu,
	marcandre.lureau@redhat.com, rakeshjb010@gmail.com
Subject: [PATCH v3 1/2] hw/pci-host/gt64120: Fix endianness handling
Date: Mon, 31 Mar 2025 02:31:54 +0530	[thread overview]
Message-ID: <20250330210155.74295-2-rakeshjb010@gmail.com> (raw)
In-Reply-To: <20250330210155.74295-1-rakeshjb010@gmail.com>

The GT-64120 PCI controller requires special handling where:
1. Host bridge(bus 0 ,device 0) must use native endianness
2. Other devices follow MByteSwap bit in GT_PCI0_CMD

Previous implementation accidentally swapped all accesses, breaking
host bridge detection (lspci -d 11ab:4620).

This patch:
- Removes gt64120_update_pci_cfgdata_mapping(), moving data_mem initialization
  to gt64120_realize()
- Adds custom read/write handlers
- Replace raw bit check with FIELD_EX32 for MByteSwap .
- Use extract32 for bus/device check (bus 0, device 0).
- Implement size-specific swaps (bswap16 for 2-byte, bswap32 for 4-byte)
  per MemoryRegionOps requirements.

Fixes: 145e2198 ("hw/mips/gt64xxx_pci: Endian-swap using PCI_HOST_BRIDGE MemoryRegionOps")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2826

Signed-off-by: Rakesh Jeyasingh <rakeshjb010@gmail.com>
---
 hw/pci-host/gt64120.c | 99 ++++++++++++++++++++++++++++---------------
 1 file changed, 65 insertions(+), 34 deletions(-)

diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c
index d5c13a89b6..b6abfb1512 100644
--- a/hw/pci-host/gt64120.c
+++ b/hw/pci-host/gt64120.c
@@ -320,38 +320,6 @@ static void gt64120_isd_mapping(GT64120State *s)
     memory_region_transaction_commit();
 }
 
-static void gt64120_update_pci_cfgdata_mapping(GT64120State *s)
-{
-    /* Indexed on MByteSwap bit, see Table 158: PCI_0 Command, Offset: 0xc00 */
-    static const MemoryRegionOps *pci_host_data_ops[] = {
-        &pci_host_data_be_ops, &pci_host_data_le_ops
-    };
-    PCIHostState *phb = PCI_HOST_BRIDGE(s);
-
-    memory_region_transaction_begin();
-
-    /*
-     * The setting of the MByteSwap bit and MWordSwap bit in the PCI Internal
-     * Command Register determines how data transactions from the CPU to/from
-     * PCI are handled along with the setting of the Endianness bit in the CPU
-     * Configuration Register. See:
-     * - Table 16: 32-bit PCI Transaction Endianness
-     * - Table 158: PCI_0 Command, Offset: 0xc00
-     */
-
-    if (memory_region_is_mapped(&phb->data_mem)) {
-        memory_region_del_subregion(&s->ISD_mem, &phb->data_mem);
-        object_unparent(OBJECT(&phb->data_mem));
-    }
-    memory_region_init_io(&phb->data_mem, OBJECT(phb),
-                          pci_host_data_ops[s->regs[GT_PCI0_CMD] & 1],
-                          s, "pci-conf-data", 4);
-    memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGDATA << 2,
-                                        &phb->data_mem, 1);
-
-    memory_region_transaction_commit();
-}
-
 static void gt64120_pci_mapping(GT64120State *s)
 {
     memory_region_transaction_begin();
@@ -645,7 +613,6 @@ static void gt64120_writel(void *opaque, hwaddr addr,
     case GT_PCI0_CMD:
     case GT_PCI1_CMD:
         s->regs[saddr] = val & 0x0401fc0f;
-        gt64120_update_pci_cfgdata_mapping(s);
         break;
     case GT_PCI0_TOR:
     case GT_PCI0_BS_SCS10:
@@ -1024,6 +991,65 @@ static const MemoryRegionOps isd_mem_ops = {
     },
 };
 
+static bool is_phb_dev0(const PCIHostState *phb)
+{
+    /*Checks if the current PCI configuration access targets the host bridge(bus 0, device 0)*/
+    return extract32(phb->config_reg, 11, 5/*dev*/ + 8/*bus*/) == 0;
+}
+
+static uint64_t gt64120_pci_data_read(void *opaque, hwaddr addr, unsigned size)
+{
+    GT64120State *s = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
+    uint32_t val;
+    bool le_mode = FIELD_EX32(s->regs[GT_PCI0_CMD], GT_PCI0_CMD, MByteSwap);
+
+    if (!(phb->config_reg & (1 << 31))) {
+        val = 0xffffffff;
+    } else {
+        val = pci_data_read(phb->bus, phb->config_reg | (addr & 3), size);
+    }
+
+    /* Only swap for non-bridge devices in big-endian mode */
+    if (!le_mode && !is_phb_dev0(phb)) {
+        if (size == 2) {
+            val = bswap16(val);
+        } else if (size == 4) {
+            val = bswap32(val); 
+        }
+    }
+    return val;
+}
+
+static void gt64120_pci_data_write(void *opaque, hwaddr addr, 
+    uint64_t val, unsigned size)
+{
+    GT64120State *s = opaque;
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
+    bool le_mode = FIELD_EX32(s->regs[GT_PCI0_CMD], GT_PCI0_CMD, MByteSwap);
+
+    if (!le_mode && !is_phb_dev0(phb)) {
+        if (size == 2) {
+            val = bswap16(val);
+        } else if (size == 4) {
+            val = bswap32(val);
+        }
+    }
+    if (phb->config_reg & (1u << 31)){
+        pci_data_write(phb->bus, phb->config_reg | (addr & 3), val, size);
+    }
+}
+
+static const MemoryRegionOps gt64120_pci_data_ops = {
+    .read = gt64120_pci_data_read,
+    .write = gt64120_pci_data_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
 static void gt64120_reset(DeviceState *dev)
 {
     GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev);
@@ -1178,7 +1204,6 @@ static void gt64120_reset(DeviceState *dev)
 
     gt64120_isd_mapping(s);
     gt64120_pci_mapping(s);
-    gt64120_update_pci_cfgdata_mapping(s);
 }
 
 static void gt64120_realize(DeviceState *dev, Error **errp)
@@ -1202,6 +1227,12 @@ static void gt64120_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGADDR << 2,
                                         &phb->conf_mem, 1);
 
+    memory_region_init_io(&phb->data_mem, OBJECT(phb),
+                          &gt64120_pci_data_ops,
+                          s, "pci-conf-data", 4);
+    memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGDATA << 2,
+                                        &phb->data_mem, 1);
+
 
     /*
      * The whole address space decoded by the GT-64120A doesn't generate
-- 
2.43.0



  reply	other threads:[~2025-03-30 21:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-30 21:01 [PATCH v3 0/2] GT64120 PCI endianness fixes and cleanup Rakesh Jeyasingh
2025-03-30 21:01 ` Rakesh Jeyasingh [this message]
2025-03-30 21:01 ` [PATCH v3 2/2] hw/pci-host: Remove unused pci_host_data_be_ops Rakesh Jeyasingh
2025-03-31 11:35   ` Philippe Mathieu-Daudé

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=20250330210155.74295-2-rakeshjb010@gmail.com \
    --to=rakeshjb010@gmail.com \
    --cc=balaton@eik.bme.hu \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /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 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).