| Message ID | 20250329004941.372000-1-rakeshjb010@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] hw/pci-host/gt64120.c: Fix PCI host bridge endianness handling | expand |
Hi Rakesh, On 29/3/25 01:49, 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 patch: > - Adds custom read/write handlers to preserve native big-endian for the host > bridge (phb->config_reg & 0x00fff800 == 0). Here you check for bus == 0 && device == 0, is that what you want? (I'm confused because you only describe "for the host bridge"). If so I'd rather add a self-describing method: static bool is_phb_dev0(const PCIHostState *phb) { return extract32(phb->config_reg, 11, 5 /* dev */ + 8 /* bus) == 0; } > - Swaps non-bridge devices when MByteSwap = 0, using size-appropriate swaps > (bswap16 for 2-byte, bswap32 for 4-byte) to convert PCI's little-endian data > to match the MIPS guest's big-endian expectation; no swap occurs for the host > bridge or when MByteSwap = 1 (little-endian mode). > - Removes gt64120_update_pci_cfgdata_mapping(), moving data_mem initialization > to gt64120_realize() > - Removes unused pci_host_data_be_ops and a misleading comment in dino.h. > > 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 <rakeshjb010@gmail.com> > --- > hw/pci-host/gt64120.c | 83 ++++++++++++++++++++++---------------- > hw/pci/pci_host.c | 6 --- > include/hw/pci-host/dino.h | 5 +-- > include/hw/pci/pci_host.h | 1 - > 4 files changed, 50 insertions(+), 45 deletions(-) > > diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c > index d5c13a89b6..4e45d0aa53 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,49 @@ static const MemoryRegionOps isd_mem_ops = { > }, > }; > > +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); You check the Enable bit in the write path but not here, any reason? > + > + /* Only swap for non-bridge devices in big-endian mode */ > + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { Please use instead of (s->regs[GT_PCI0_CMD] & 1): bswap = FIELD_EX32(s->regs[GT_PCI0_CMD]m GT_PCI0_CMD, MByteSwap); > + 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); > + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { > + 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, > + }, > +}; Per GT64120 rev 1.4, chapter "6.2.2 PCI Master CPU Byte Swapping": When the GT–64120 is configured for 64-bit PCI mode, byte swapping occurs across all 8 byte lanes when the ByteSwap bit is set for PCI_0. > static void gt64120_reset(DeviceState *dev) > { > GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev); > @@ -1178,7 +1188,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 +1211,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 Please split the changes below in a distinct cleanup patch. > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > index 80f91f409f..56f7f28a1a 100644 > --- a/hw/pci/pci_host.c > +++ b/hw/pci/pci_host.c > @@ -217,12 +217,6 @@ const MemoryRegionOps pci_host_data_le_ops = { > .endianness = DEVICE_LITTLE_ENDIAN, > }; > > -const MemoryRegionOps pci_host_data_be_ops = { > - .read = pci_host_data_read, > - .write = pci_host_data_write, > - .endianness = DEVICE_BIG_ENDIAN, > -}; > - > static bool pci_host_needed(void *opaque) > { > PCIHostState *s = opaque; > diff --git a/include/hw/pci-host/dino.h b/include/hw/pci-host/dino.h > index fd7975c798..df509dbc18 100644 > --- a/include/hw/pci-host/dino.h > +++ b/include/hw/pci-host/dino.h > @@ -109,10 +109,7 @@ static const uint32_t reg800_keep_bits[DINO800_REGS] = { > struct DinoState { > PCIHostState parent_obj; > > - /* > - * PCI_CONFIG_ADDR is parent_obj.config_reg, via pci_host_conf_be_ops, > - * so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. > - */ > + > uint32_t config_reg_dino; /* keep original copy, including 2 lowest bits */ > > uint32_t iar0; > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > index e52d8ec2cd..954dd446fa 100644 > --- a/include/hw/pci/pci_host.h > +++ b/include/hw/pci/pci_host.h > @@ -68,6 +68,5 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, unsigned len); > extern const MemoryRegionOps pci_host_conf_le_ops; > extern const MemoryRegionOps pci_host_conf_be_ops; > extern const MemoryRegionOps pci_host_data_le_ops; > -extern const MemoryRegionOps pci_host_data_be_ops; > > #endif /* PCI_HOST_H */
On Sat, 29 Mar 2025, Philippe Mathieu-Daudé wrote: > Hi Rakesh, > > On 29/3/25 01:49, 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 patch: >> - Adds custom read/write handlers to preserve native big-endian for the >> host >> bridge (phb->config_reg & 0x00fff800 == 0). > > Here you check for bus == 0 && device == 0, is that what you want? (I'm > confused because you only describe "for the host bridge"). > > If so I'd rather add a self-describing method: > > static bool is_phb_dev0(const PCIHostState *phb) > { > return extract32(phb->config_reg, 11, 5 /* dev */ + 8 /* bus) == 0; > } There are already macros such as PCI_BUS_NUM PCI_FUNC. Are they any use instead of another function? Regards, BALATON Zoltan >> - Swaps non-bridge devices when MByteSwap = 0, using size-appropriate swaps >> (bswap16 for 2-byte, bswap32 for 4-byte) to convert PCI's little-endian >> data >> to match the MIPS guest's big-endian expectation; no swap occurs for the >> host >> bridge or when MByteSwap = 1 (little-endian mode). >> - Removes gt64120_update_pci_cfgdata_mapping(), moving data_mem >> initialization >> to gt64120_realize() >> - Removes unused pci_host_data_be_ops and a misleading comment in dino.h. >> >> 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 <rakeshjb010@gmail.com> >> --- >> hw/pci-host/gt64120.c | 83 ++++++++++++++++++++++---------------- >> hw/pci/pci_host.c | 6 --- >> include/hw/pci-host/dino.h | 5 +-- >> include/hw/pci/pci_host.h | 1 - >> 4 files changed, 50 insertions(+), 45 deletions(-) >> >> diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c >> index d5c13a89b6..4e45d0aa53 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,49 @@ static const MemoryRegionOps isd_mem_ops = { >> }, >> }; >> +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); > > You check the Enable bit in the write path but not here, any reason? >> + >> + /* Only swap for non-bridge devices in big-endian mode */ >> + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { > > Please use instead of (s->regs[GT_PCI0_CMD] & 1): > > bswap = FIELD_EX32(s->regs[GT_PCI0_CMD]m GT_PCI0_CMD, MByteSwap); > >> + 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); >> + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { >> + 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, >> + }, >> +}; > > Per GT64120 rev 1.4, chapter "6.2.2 PCI Master CPU Byte Swapping": > > When the GT–64120 is configured for 64-bit PCI mode, byte > swapping occurs across all 8 byte lanes when the ByteSwap > bit is set for PCI_0. > >> static void gt64120_reset(DeviceState *dev) >> { >> GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev); >> @@ -1178,7 +1188,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 +1211,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 > > Please split the changes below in a distinct cleanup patch. > >> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c >> index 80f91f409f..56f7f28a1a 100644 >> --- a/hw/pci/pci_host.c >> +++ b/hw/pci/pci_host.c >> @@ -217,12 +217,6 @@ const MemoryRegionOps pci_host_data_le_ops = { >> .endianness = DEVICE_LITTLE_ENDIAN, >> }; >> -const MemoryRegionOps pci_host_data_be_ops = { >> - .read = pci_host_data_read, >> - .write = pci_host_data_write, >> - .endianness = DEVICE_BIG_ENDIAN, >> -}; >> - >> static bool pci_host_needed(void *opaque) >> { >> PCIHostState *s = opaque; >> diff --git a/include/hw/pci-host/dino.h b/include/hw/pci-host/dino.h >> index fd7975c798..df509dbc18 100644 >> --- a/include/hw/pci-host/dino.h >> +++ b/include/hw/pci-host/dino.h >> @@ -109,10 +109,7 @@ static const uint32_t reg800_keep_bits[DINO800_REGS] = >> { >> struct DinoState { >> PCIHostState parent_obj; >> - /* >> - * PCI_CONFIG_ADDR is parent_obj.config_reg, via pci_host_conf_be_ops, >> - * so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. >> - */ >> + >> uint32_t config_reg_dino; /* keep original copy, including 2 lowest >> bits */ >> uint32_t iar0; >> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h >> index e52d8ec2cd..954dd446fa 100644 >> --- a/include/hw/pci/pci_host.h >> +++ b/include/hw/pci/pci_host.h >> @@ -68,6 +68,5 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, unsigned >> len); >> extern const MemoryRegionOps pci_host_conf_le_ops; >> extern const MemoryRegionOps pci_host_conf_be_ops; >> extern const MemoryRegionOps pci_host_data_le_ops; >> -extern const MemoryRegionOps pci_host_data_be_ops; >> #endif /* PCI_HOST_H */ > >
Thanks, BALATON I looked into PCI_BUS_NUM and PCI_SLOT from include/hw/pci/pci.h (L15-24): - PCI_BUS_NUM(x) (((x) >> 8) & 0xff)) --> bits 15-8. - PCI_SLOT(devfn) (((devfn) >> 3) & 0x1f)), which don’t align properly with the 32-bit phb->config_reg layout used in your GT-64120 . Since these macros are intended for a 16-bit BDF (Bus-Device-Function) format rather than the full 32-bit PCI config address, For phb->config_reg (32-bit: bus 23-16, device 15-11) these extract the wrong bits On Sat, Mar 29, 2025 at 6:48 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Sat, 29 Mar 2025, Philippe Mathieu-Daudé wrote: > > Hi Rakesh, > > > > On 29/3/25 01:49, 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 patch: > >> - Adds custom read/write handlers to preserve native big-endian for the > >> host > >> bridge (phb->config_reg & 0x00fff800 == 0). > > > > Here you check for bus == 0 && device == 0, is that what you want? (I'm > > confused because you only describe "for the host bridge"). > > > > If so I'd rather add a self-describing method: > > > > static bool is_phb_dev0(const PCIHostState *phb) > > { > > return extract32(phb->config_reg, 11, 5 /* dev */ + 8 /* bus) == 0; > > } > > There are already macros such as PCI_BUS_NUM PCI_FUNC. Are they any use > instead of another function? > > Regards, > BALATON Zoltan > > >> - Swaps non-bridge devices when MByteSwap = 0, using size-appropriate > swaps > >> (bswap16 for 2-byte, bswap32 for 4-byte) to convert PCI's > little-endian > >> data > >> to match the MIPS guest's big-endian expectation; no swap occurs for > the > >> host > >> bridge or when MByteSwap = 1 (little-endian mode). > >> - Removes gt64120_update_pci_cfgdata_mapping(), moving data_mem > >> initialization > >> to gt64120_realize() > >> - Removes unused pci_host_data_be_ops and a misleading comment in > dino.h. > >> > >> 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 <rakeshjb010@gmail.com> > >> --- > >> hw/pci-host/gt64120.c | 83 ++++++++++++++++++++++---------------- > >> hw/pci/pci_host.c | 6 --- > >> include/hw/pci-host/dino.h | 5 +-- > >> include/hw/pci/pci_host.h | 1 - > >> 4 files changed, 50 insertions(+), 45 deletions(-) > >> > >> diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c > >> index d5c13a89b6..4e45d0aa53 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,49 @@ static const MemoryRegionOps isd_mem_ops = { > >> }, > >> }; > >> +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); > > > > You check the Enable bit in the write path but not here, any reason? > >> + > >> + /* Only swap for non-bridge devices in big-endian mode */ > >> + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) > { > > > > Please use instead of (s->regs[GT_PCI0_CMD] & 1): > > > > bswap = FIELD_EX32(s->regs[GT_PCI0_CMD]m GT_PCI0_CMD, MByteSwap); > > > >> + 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); > >> + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) > { > >> + 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, > >> + }, > >> +}; > > > > Per GT64120 rev 1.4, chapter "6.2.2 PCI Master CPU Byte Swapping": > > > > When the GT–64120 is configured for 64-bit PCI mode, byte > > swapping occurs across all 8 byte lanes when the ByteSwap > > bit is set for PCI_0. > > > >> static void gt64120_reset(DeviceState *dev) > >> { > >> GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev); > >> @@ -1178,7 +1188,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 +1211,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 > > > > Please split the changes below in a distinct cleanup patch. > > > >> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > >> index 80f91f409f..56f7f28a1a 100644 > >> --- a/hw/pci/pci_host.c > >> +++ b/hw/pci/pci_host.c > >> @@ -217,12 +217,6 @@ const MemoryRegionOps pci_host_data_le_ops = { > >> .endianness = DEVICE_LITTLE_ENDIAN, > >> }; > >> -const MemoryRegionOps pci_host_data_be_ops = { > >> - .read = pci_host_data_read, > >> - .write = pci_host_data_write, > >> - .endianness = DEVICE_BIG_ENDIAN, > >> -}; > >> - > >> static bool pci_host_needed(void *opaque) > >> { > >> PCIHostState *s = opaque; > >> diff --git a/include/hw/pci-host/dino.h b/include/hw/pci-host/dino.h > >> index fd7975c798..df509dbc18 100644 > >> --- a/include/hw/pci-host/dino.h > >> +++ b/include/hw/pci-host/dino.h > >> @@ -109,10 +109,7 @@ static const uint32_t > reg800_keep_bits[DINO800_REGS] = > >> { > >> struct DinoState { > >> PCIHostState parent_obj; > >> - /* > >> - * PCI_CONFIG_ADDR is parent_obj.config_reg, via > pci_host_conf_be_ops, > >> - * so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. > >> - */ > >> + > >> uint32_t config_reg_dino; /* keep original copy, including 2 > lowest > >> bits */ > >> uint32_t iar0; > >> diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > >> index e52d8ec2cd..954dd446fa 100644 > >> --- a/include/hw/pci/pci_host.h > >> +++ b/include/hw/pci/pci_host.h > >> @@ -68,6 +68,5 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, > unsigned > >> len); > >> extern const MemoryRegionOps pci_host_conf_le_ops; > >> extern const MemoryRegionOps pci_host_conf_be_ops; > >> extern const MemoryRegionOps pci_host_data_le_ops; > >> -extern const MemoryRegionOps pci_host_data_be_ops; > >> #endif /* PCI_HOST_H */ > > > >
On Sat, Mar 29, 2025 at 5:18 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > Hi Rakesh, > > On 29/3/25 01:49, 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 patch: > > - Adds custom read/write handlers to preserve native big-endian for the > host > > bridge (phb->config_reg & 0x00fff800 == 0). > > Here you check for bus == 0 && device == 0, is that what you want? (I'm > confused because you only describe "for the host bridge"). yes, I meant bus 0, device 0 > If so I'd rather add a self-describing method: > > static bool is_phb_dev0(const PCIHostState *phb) > { > return extract32(phb->config_reg, 11, 5 /* dev */ + 8 /* bus) == 0; > } > > > - Swaps non-bridge devices when MByteSwap = 0, using size-appropriate > swaps > > (bswap16 for 2-byte, bswap32 for 4-byte) to convert PCI's > little-endian data > > to match the MIPS guest's big-endian expectation; no swap occurs for > the host > > bridge or when MByteSwap = 1 (little-endian mode). > > - Removes gt64120_update_pci_cfgdata_mapping(), moving data_mem > initialization > > to gt64120_realize() > > - Removes unused pci_host_data_be_ops and a misleading comment in dino.h. > > > > 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 <rakeshjb010@gmail.com> > > --- > > hw/pci-host/gt64120.c | 83 ++++++++++++++++++++++---------------- > > hw/pci/pci_host.c | 6 --- > > include/hw/pci-host/dino.h | 5 +-- > > include/hw/pci/pci_host.h | 1 - > > 4 files changed, 50 insertions(+), 45 deletions(-) > > > > diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c > > index d5c13a89b6..4e45d0aa53 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,49 @@ static const MemoryRegionOps isd_mem_ops = { > > }, > > }; > > > > +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); > > You check the Enable bit in the write path but not here, any reason? > thanks for pointing it out, I missed to notice it > > + > > + /* Only swap for non-bridge devices in big-endian mode */ > > + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { > > Please use instead of (s->regs[GT_PCI0_CMD] & 1): > > bswap = FIELD_EX32(s->regs[GT_PCI0_CMD]m GT_PCI0_CMD, MByteSwap); > > > + 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); > > + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { > > + 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, > > + }, > > +}; > > Per GT64120 rev 1.4, chapter "6.2.2 PCI Master CPU Byte Swapping": > > When the GT–64120 is configured for 64-bit PCI mode, byte > swapping occurs across all 8 byte lanes when the ByteSwap > bit is set for PCI_0. > > > static void gt64120_reset(DeviceState *dev) > > { > > GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev); > > @@ -1178,7 +1188,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 +1211,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 > > Please split the changes below in a distinct cleanup patch. > > > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c > > index 80f91f409f..56f7f28a1a 100644 > > --- a/hw/pci/pci_host.c > > +++ b/hw/pci/pci_host.c > > @@ -217,12 +217,6 @@ const MemoryRegionOps pci_host_data_le_ops = { > > .endianness = DEVICE_LITTLE_ENDIAN, > > }; > > > > -const MemoryRegionOps pci_host_data_be_ops = { > > - .read = pci_host_data_read, > > - .write = pci_host_data_write, > > - .endianness = DEVICE_BIG_ENDIAN, > > -}; > > - > > static bool pci_host_needed(void *opaque) > > { > > PCIHostState *s = opaque; > > diff --git a/include/hw/pci-host/dino.h b/include/hw/pci-host/dino.h > > index fd7975c798..df509dbc18 100644 > > --- a/include/hw/pci-host/dino.h > > +++ b/include/hw/pci-host/dino.h > > @@ -109,10 +109,7 @@ static const uint32_t > reg800_keep_bits[DINO800_REGS] = { > > struct DinoState { > > PCIHostState parent_obj; > > > > - /* > > - * PCI_CONFIG_ADDR is parent_obj.config_reg, via > pci_host_conf_be_ops, > > - * so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. > > - */ > > + > > uint32_t config_reg_dino; /* keep original copy, including 2 > lowest bits */ > > > > uint32_t iar0; > > diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h > > index e52d8ec2cd..954dd446fa 100644 > > --- a/include/hw/pci/pci_host.h > > +++ b/include/hw/pci/pci_host.h > > @@ -68,6 +68,5 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, > unsigned len); > > extern const MemoryRegionOps pci_host_conf_le_ops; > > extern const MemoryRegionOps pci_host_conf_be_ops; > > extern const MemoryRegionOps pci_host_data_le_ops; > > -extern const MemoryRegionOps pci_host_data_be_ops; > > > > #endif /* PCI_HOST_H */ > Proposed v3 Changes: 1. Use extract32 in is_phb_dev0() for bus 0, device 0 check (Philippe). 2. FIELD_EX32 instead of (s->regs[GT_PCI0_CMD] & 1) (Philippe). 3. Size-specific swaps: bswap16 for 2-byte, bswap32 for 4-byte (Paolo). 4. check the Enable bit in the read path (Philippe). 5. Split cleanup into [PATCH v3 2/2].
diff --git a/hw/pci-host/gt64120.c b/hw/pci-host/gt64120.c index d5c13a89b6..4e45d0aa53 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,49 @@ static const MemoryRegionOps isd_mem_ops = { }, }; +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)) { + 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); + if (!(s->regs[GT_PCI0_CMD] & 1) && (phb->config_reg & 0x00fff800)) { + 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 +1188,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 +1211,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 diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c index 80f91f409f..56f7f28a1a 100644 --- a/hw/pci/pci_host.c +++ b/hw/pci/pci_host.c @@ -217,12 +217,6 @@ const MemoryRegionOps pci_host_data_le_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; -const MemoryRegionOps pci_host_data_be_ops = { - .read = pci_host_data_read, - .write = pci_host_data_write, - .endianness = DEVICE_BIG_ENDIAN, -}; - static bool pci_host_needed(void *opaque) { PCIHostState *s = opaque; diff --git a/include/hw/pci-host/dino.h b/include/hw/pci-host/dino.h index fd7975c798..df509dbc18 100644 --- a/include/hw/pci-host/dino.h +++ b/include/hw/pci-host/dino.h @@ -109,10 +109,7 @@ static const uint32_t reg800_keep_bits[DINO800_REGS] = { struct DinoState { PCIHostState parent_obj; - /* - * PCI_CONFIG_ADDR is parent_obj.config_reg, via pci_host_conf_be_ops, - * so that we can map PCI_CONFIG_DATA to pci_host_data_be_ops. - */ + uint32_t config_reg_dino; /* keep original copy, including 2 lowest bits */ uint32_t iar0; diff --git a/include/hw/pci/pci_host.h b/include/hw/pci/pci_host.h index e52d8ec2cd..954dd446fa 100644 --- a/include/hw/pci/pci_host.h +++ b/include/hw/pci/pci_host.h @@ -68,6 +68,5 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, unsigned len); extern const MemoryRegionOps pci_host_conf_le_ops; extern const MemoryRegionOps pci_host_conf_be_ops; extern const MemoryRegionOps pci_host_data_le_ops; -extern const MemoryRegionOps pci_host_data_be_ops; #endif /* PCI_HOST_H */
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 patch: - Adds custom read/write handlers to preserve native big-endian for the host bridge (phb->config_reg & 0x00fff800 == 0). - Swaps non-bridge devices when MByteSwap = 0, using size-appropriate swaps (bswap16 for 2-byte, bswap32 for 4-byte) to convert PCI's little-endian data to match the MIPS guest's big-endian expectation; no swap occurs for the host bridge or when MByteSwap = 1 (little-endian mode). - Removes gt64120_update_pci_cfgdata_mapping(), moving data_mem initialization to gt64120_realize() - Removes unused pci_host_data_be_ops and a misleading comment in dino.h. 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 <rakeshjb010@gmail.com> --- hw/pci-host/gt64120.c | 83 ++++++++++++++++++++++---------------- hw/pci/pci_host.c | 6 --- include/hw/pci-host/dino.h | 5 +-- include/hw/pci/pci_host.h | 1 - 4 files changed, 50 insertions(+), 45 deletions(-)