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 <rakeshjb...@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