On Thu, Feb 23, 2023 at 04:19:58PM +0000, 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.y...@flygoat.com>
Thanks for the fix!
Tested-by: Nathan Chancellor <nat...@kernel.org>
> ---
> hw/pci-host/gt64120.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c
> index f226d0342039..82c15edb4698 100644
> --- a/hw/pci-host/gt64120.c
> +++ b/hw/pci-host/gt64120.c
> @@ -321,9 +321,6 @@ static void gt64120_isd_mapping(GT64120State *s)
> 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_conf_ops[] = {
> - &pci_host_conf_be_ops, &pci_host_conf_le_ops
> - };
> static const MemoryRegionOps *pci_host_data_ops[] = {
> &pci_host_data_be_ops, &pci_host_data_le_ops
> };
> @@ -339,15 +336,6 @@ static void
> gt64120_update_pci_cfgdata_mapping(GT64120State *s)
> * - Table 16: 32-bit PCI Transaction Endianess
> * - Table 158: PCI_0 Command, Offset: 0xc00
> */
> - if (memory_region_is_mapped(&phb->conf_mem)) {
> - memory_region_del_subregion(&s->ISD_mem, &phb->conf_mem);
> - object_unparent(OBJECT(&phb->conf_mem));
> - }
> - memory_region_init_io(&phb->conf_mem, OBJECT(phb),
> - pci_host_conf_ops[s->regs[GT_PCI0_CMD] & 1],
> - s, "pci-conf-idx", 4);
> - memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGADDR << 2,
> - &phb->conf_mem, 1);
>
> if (memory_region_is_mapped(&phb->data_mem)) {
> memory_region_del_subregion(&s->ISD_mem, &phb->data_mem);
> @@ -1208,6 +1196,12 @@ static void gt64120_realize(DeviceState *dev, Error
> **errp)
> PCI_DEVFN(18, 0), TYPE_PCI_BUS);
>
> pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "gt64120_pci");
> + memory_region_init_io(&phb->conf_mem, OBJECT(phb),
> + &pci_host_conf_le_ops,
> + s, "pci-conf-idx", 4);
> + memory_region_add_subregion_overlap(&s->ISD_mem, GT_PCI0_CFGADDR << 2,
> + &phb->conf_mem, 1);
> +
>
> /*
> * The whole address space decoded by the GT-64120A doesn't generate
> --
> 2.37.1 (Apple Git-137.1)
>
>
