* [PATCH v3 1/2] hw/pci-host/gt64120: Fix endianness handling
2025-03-30 21:01 [PATCH v3 0/2] GT64120 PCI endianness fixes and cleanup Rakesh Jeyasingh
@ 2025-03-30 21:01 ` Rakesh Jeyasingh
2025-03-30 21:01 ` [PATCH v3 2/2] hw/pci-host: Remove unused pci_host_data_be_ops Rakesh Jeyasingh
1 sibling, 0 replies; 4+ messages in thread
From: Rakesh Jeyasingh @ 2025-03-30 21:01 UTC (permalink / raw)
To: qemu-devel, philmd, thuth
Cc: pbonzini, balaton, marcandre.lureau, rakeshjb010
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),
+ >64120_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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v3 2/2] hw/pci-host: Remove unused pci_host_data_be_ops
2025-03-30 21:01 [PATCH v3 0/2] GT64120 PCI endianness fixes and cleanup Rakesh Jeyasingh
2025-03-30 21:01 ` [PATCH v3 1/2] hw/pci-host/gt64120: Fix endianness handling Rakesh Jeyasingh
@ 2025-03-30 21:01 ` Rakesh Jeyasingh
2025-03-31 11:35 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 4+ messages in thread
From: Rakesh Jeyasingh @ 2025-03-30 21:01 UTC (permalink / raw)
To: qemu-devel, philmd, thuth
Cc: pbonzini, balaton, marcandre.lureau, rakeshjb010
pci_host_data_be_ops became unused after endianness fixes
Signed-off-by: Rakesh Jeyasingh <rakeshjb010@gmail.com>
---
hw/pci/pci_host.c | 6 ------
include/hw/pci-host/dino.h | 4 ----
include/hw/pci/pci_host.h | 1 -
3 files changed, 11 deletions(-)
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 80f91f409f..56f7f28a1a 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -217,12 +217,6 @@ const MemoryRegionOps pci_host_data_le_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-const MemoryRegionOps pci_host_data_be_ops = {
- .read = pci_host_data_read,
- .write = pci_host_data_write,
- .endianness = DEVICE_BIG_ENDIAN,
-};
-
static bool pci_host_needed(void *opaque)
{
PCIHostState *s = opaque;
diff --git a/include/hw/pci-host/dino.h b/include/hw/pci-host/dino.h
index fd7975c798..5dc8cdf610 100644
--- a/include/hw/pci-host/dino.h
+++ b/include/hw/pci-host/dino.h
@@ -109,10 +109,6 @@ static const uint32_t reg800_keep_bits[DINO800_REGS] = {
struct DinoState {
PCIHostState parent_obj;
- /*
- * PCI_CONFIG_ADDR is parent_obj.config_reg, via pci_host_conf_be_ops,
- * so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops.
- */
uint32_t config_reg_dino; /* keep original copy, including 2 lowest bits */
uint32_t iar0;
diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h
index e52d8ec2cd..954dd446fa 100644
--- a/include/hw/pci/pci_host.h
+++ b/include/hw/pci/pci_host.h
@@ -68,6 +68,5 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, unsigned len);
extern const MemoryRegionOps pci_host_conf_le_ops;
extern const MemoryRegionOps pci_host_conf_be_ops;
extern const MemoryRegionOps pci_host_data_le_ops;
-extern const MemoryRegionOps pci_host_data_be_ops;
#endif /* PCI_HOST_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread