Re: [PATCH] hw/mips/gt64xxx_pci: Don't endian-swap GT_PCI0_CFGADDR

[Nathan Chancellor <nathan@kernel.org>]

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_BRIDGE
> > MemoryRegionOps") 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 disable
> > endian-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.

Cheers,
Nathan