|
From: | Thomas Huth |
Subject: | Re: [PATCH] hw/mips/gt64xxx_pci: Don't endian-swap GT_PCI0_CFGADDR |
Date: | Thu, 30 Mar 2023 12:09:06 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 |
On 29/03/2023 18.07, Philippe Mathieu-Daudé wrote:
On 29/3/23 18:09, Rob Landley wrote:On 3/28/23 12:02, Philippe Mathieu-Daudé wrote:On 20/3/23 17:58, Nathan Chancellor wrote:On Wed, Mar 08, 2023 at 12:33:38AM +0100, Philippe Mathieu-Daudé wrote:On 23/2/23 17:19, Jiaxun Yang wrote:145e2198d749 ("hw/mips/gt64xxx_pci: Endian-swap using PCI_HOST_BRIDGEMemoryRegionOps") converted CFGADDR/CFGDATA registers to use PCI_HOST_BRIDGE's accessor facility and enabled byte swap for both CFGADDR/CFGDATA register.However CFGADDR as a ISD internal register is not controled by MByteSwap bit, it follows endian of all other ISD register, which means it ties to little endian.Move mapping of CFGADDR out of gt64120_update_pci_cfgdata_mapping to disableendian-swapping. This should fix some recent reports about poweroff hang.Fixes: 145e2198d749 ("hw/mips/gt64xxx_pci: Endian-swap using PCI_HOST_BRIDGE MemoryRegionOps")Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- hw/pci-host/gt64120.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)So this works on little-endian hosts, but fails on big-endian ones :( I.e. on Linux we have early_console_write() -> prom_putchar() looping: IN: prom_putchar 0x8010fab8: lbu v0,0(v1) 0x8010fabc: andi v0,v0,0x20 0x8010fac0: beqz v0,0x8010fab8 0x8010fac4: andi v0,a0,0xff gt64120: Illegal register read reg:0x3fc size:4 value:0x00000000 gt64120: Illegal register read reg:0x3fc size:4 value:0x00000000 gt64120: Illegal register read reg:0x3fc size:4 value:0x00000000 gt64120: Illegal register read reg:0x3fc size:4 value:0x00000000 gt64120: Illegal register read reg:0x3fc size:4 value:0x00000000 ...Is there going to be a new version of this patch or a different solution to the poweroff hang then? I am still seeing that with tip of tree QEMU and I see 8.0.0-rc0 has been tagged; I would hate for this to end up in a release version.I couldn't work a fix, however I ran our (new) tests on merge commit 3db29dcac2 which is before the offending commit 145e2198d749, and they fail. So I suppose Malta on big-endian host is badly broken since quite some time. Thus clearly nobody tests/runs Malta there.I test/run malta with the mips and mipsel binaries at https://landley.net/toybox/downloads/binaries/mkroot/0.8.9/ but I'm still locally applying the first patch I saw to fix this (attached) until upstream sorts itself out.Works fine for me. Somebody said it was the wrong fix but I don't remember why...This is a correct /partial/ fix. With this patch, Malta works on little endian hosts. No luck with big-endian hosts, but this was broken previous to 3db29dcac2 rework, so apparently not a big deal ¯\_(ツ)_/¯
I've bisected now on a big endian s390x machine, and it looks like it has been broken here:
0c8427baf0f66bdaecae41891304f6e15242e682 is the first bad commit commit 0c8427baf0f66bdaecae41891304f6e15242e682 Author: Jiaxun Yang <jiaxun.yang@flygoat.com> Date: Wed Oct 26 21:18:21 2022 +0200 hw/mips/malta: Use bootloader helper to set BAR registers... we should maybe really run a selected list of avocado jobs on a big endian host (either .travis-ci.yml or the gitlab-CI custom runner?) to avoid such regressions?
Thomas
[Prev in Thread] | Current Thread | [Next in Thread] |