diff mbox series

[1/2] hw/pci-host/gt64120: Fix PCI I/O config register endianness

Message ID 20230118094023.49517-2-philmd@linaro.org
State Superseded
Headers show
Series hw/pci-host/gt64120: Fix regression on big-endian targets | expand

Commit Message

Philippe Mathieu-Daudé Jan. 18, 2023, 9:40 a.m. UTC 
The MByteSwap bit only affects the data register endianness,
not the config register. Map the config register once in the
gt64120_realize() handler, and only remap the data register
when the mapping is updated.

Fixes: 145e2198d7 ("gt64xxx: Endian-swap using PCI_HOST_BRIDGE MemoryRegionOps")
Reported-by: Klaus Jensen <its@irrelevant.dk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/gt64120.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

Comments

Klaus Jensen Jan. 18, 2023, 9:52 a.m. UTC | #1 
On Jan 18 10:40, Philippe Mathieu-Daudé wrote:
> The MByteSwap bit only affects the data register endianness,
> not the config register. Map the config register once in the
> gt64120_realize() handler, and only remap the data register
> when the mapping is updated.
> 
> Fixes: 145e2198d7 ("gt64xxx: Endian-swap using PCI_HOST_BRIDGE MemoryRegionOps")
> Reported-by: Klaus Jensen <its@irrelevant.dk>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Can confirm that this fixes the regression. Thanks Philippe!

Tested-by: Klaus Jensen <k.jensen@samsung.com>
diff mbox series

Patch

diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c
index f226d03420..a88b59f576 100644
--- a/hw/pci-host/gt64120.c
+++ b/hw/pci-host/gt64120.c
@@ -320,13 +320,6 @@  static void gt64120_isd_mapping(GT64120State *s)
 
 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_conf_ops[] = {
-        &pci_host_conf_be_ops, &pci_host_conf_le_ops
-    };
-    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();
@@ -339,22 +332,13 @@  static void gt64120_update_pci_cfgdata_mapping(GT64120State *s)
      * - Table 16: 32-bit PCI Transaction Endianess
      * - Table 158: PCI_0 Command, Offset: 0xc00
      */
-    if (memory_region_is_mapped(&phb->conf_mem)) {
-        memory_region_del_subregion(&s->ISD_mem, &phb->conf_mem);
-        object_unparent(OBJECT(&phb->conf_mem));
-    }
-    memory_region_init_io(&phb->conf_mem, OBJECT(phb),
-                          pci_host_conf_ops[s->regs[GT_PCI0_CMD] & 1],
-                          s, "pci-conf-idx", 4);
-    memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGADDR << 2,
-                                        &phb->conf_mem, 1);
-
     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->regs[GT_PCI0_CMD] & 1 ? &pci_host_data_le_ops
+                                                   : &pci_host_data_be_ops,
                           s, "pci-conf-data", 4);
     memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGDATA << 2,
                                         &phb->data_mem, 1);
@@ -1207,6 +1191,11 @@  static void gt64120_realize(DeviceState *dev, Error **errp)
                                 get_system_io(),
                                 PCI_DEVFN(18, 0), TYPE_PCI_BUS);
 
+    memory_region_init_io(&phb->conf_mem, OBJECT(phb), &pci_host_conf_le_ops,
+                          s, "pci-conf-idx", 4);
+    memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGADDR << 2,
+                                        &phb->conf_mem, 1);
+
     pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
 
     /*