On 28/03/2025 15.16, BALATON Zoltan wrote:
On Fri, 28 Mar 2025, Paolo Bonzini wrote:
On 3/27/25 21:50, BALATON Zoltan wrote:
On Thu, 27 Mar 2025, rakeshj wrote:
The GT-64120 PCI controller requires special handling where:
1. Host bridge (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 fix:
- Adds device filtering via (phb->config_reg & 0x00FFF800)
- Preserves native endianness for host bridge
- Maintains swapping for other devices in big-endian mode
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: rakeshj <rakeshjb...@gmail.com>
---
hw/pci-host/gt64120.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c
index d5c13a89b6..098f8e5988 100644
--- a/hw/pci-host/gt64120.c
+++ b/hw/pci-host/gt64120.c
@@ -320,11 +320,46 @@ static void gt64120_isd_mapping(GT64120State *s)
memory_region_transaction_commit();
}
+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 = pci_data_read(phb->bus, phb->config_reg, size);
+
+ /* Only swap for non-bridge devices in big-endian mode */
+ if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) {
+ val = bswap32(val);
I don't know if this is the best way to fix this issue as I don't know
what the issue is in the first place (isn't PCI usually little endian?)
Yes but this particular PCI host bridge is the exception, as it is the
only user of pci_host_data_be_ops.
but I think you can't just use bswap here because it also needs to take
into account the endianness of the host QEMU is running on.
It should be fine. You should take into account:
- the endianness produced by pci_data_read/pci_data_write (always little
endian)
- the endianness expected by the guest (big endian under the conditions in
the patch)
- the endianness expected by memory.c (always little endian, as specified
in gt64120_pci_data_ops)
Because there is either zero or one mismatch, bswap32 is fine.
This may worth a comment but I'm still not convinced this works on big
endian host because I think pci_data_read returns val in host endianness and
if you want big endian then you only need to bswap on LE host not on BE
host. Was this tested on BE host and confirmed it works?
FWIW, I just checked the patch on a big endian host, and it seems to work as
expected, with both qemu-system-mips and qemu-system-mipsel the "lspci -d
11ab:4620" was working correctly there.
Thomas
