[v2] ppc440_pcix: Do not expose a bridge device on PCI bus

[PATCH v2] ppc440_pcix: Do not expose a bridge device on PCI bus

Posted by BALATON Zoltan 2 weeks, 4 days ago

Real 460EX SoC apparently does not expose a bridge device and having
it appear on PCI bus confuses an AmigaOS file system driver that uses
this to detect which machine it is running on.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
Here's another version that keeps the values and only drops the device
so it's even less likely it could break anything, in case this can be
accepted for 9.0.

 hw/pci-host/ppc440_pcix.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/pci-host/ppc440_pcix.c b/hw/pci-host/ppc440_pcix.c
index 1926ae2a27..ef212d99aa 100644
--- a/hw/pci-host/ppc440_pcix.c
+++ b/hw/pci-host/ppc440_pcix.c
@@ -52,7 +52,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST)
 struct PPC440PCIXState {
     PCIHostState parent_obj;
 
-    PCIDevice *dev;
+    uint8_t config[PCI_CONFIG_SPACE_SIZE];
     struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
     struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
     uint32_t sts;
@@ -171,7 +171,7 @@ static void ppc440_pcix_reg_write4(void *opaque, hwaddr addr,
     trace_ppc440_pcix_reg_write(addr, val, size);
     switch (addr) {
     case PCI_VENDOR_ID ... PCI_MAX_LAT:
-        stl_le_p(s->dev->config + addr, val);
+        stl_le_p(s->config + addr, val);
         break;
 
     case PCIX0_POM0LAL:
@@ -302,7 +302,7 @@ static uint64_t ppc440_pcix_reg_read4(void *opaque, hwaddr addr,
 
     switch (addr) {
     case PCI_VENDOR_ID ... PCI_MAX_LAT:
-        val = ldl_le_p(s->dev->config + addr);
+        val = ldl_le_p(s->config + addr);
         break;
 
     case PCIX0_POM0LAL:
@@ -498,10 +498,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
     memory_region_init(&s->iomem, OBJECT(dev), "pci-io", 64 * KiB);
     h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
                          ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
-                         PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
-
-    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
-                               TYPE_PPC4xx_HOST_BRIDGE);
+                         PCI_DEVFN(1, 0), 1, TYPE_PCI_BUS);
 
     memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
     memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
-- 
2.30.9

Re: [PATCH v2] ppc440_pcix: Do not expose a bridge device on PCI bus

Posted by Philippe Mathieu-Daudé 2 weeks, 2 days ago

On 11/4/24 21:24, BALATON Zoltan wrote:
> Real 460EX SoC apparently does not expose a bridge device and having
> it appear on PCI bus confuses an AmigaOS file system driver that uses
> this to detect which machine it is running on.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> Here's another version that keeps the values and only drops the device
> so it's even less likely it could break anything, in case this can be
> accepted for 9.0.
> 
>   hw/pci-host/ppc440_pcix.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v2] ppc440_pcix: Do not expose a bridge device on PCI bus

Posted by BALATON Zoltan 2 weeks, 1 day ago

On Sat, 13 Apr 2024, Philippe Mathieu-Daudé wrote:
> On 11/4/24 21:24, BALATON Zoltan wrote:
>> Real 460EX SoC apparently does not expose a bridge device and having
>> it appear on PCI bus confuses an AmigaOS file system driver that uses
>> this to detect which machine it is running on.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> Here's another version that keeps the values and only drops the device
>> so it's even less likely it could break anything, in case this can be
>> accepted for 9.0.
>>
>>   hw/pci-host/ppc440_pcix.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks. Nick, could you ack this please so it could be merged if you 
won't send more pull requests? (I'm the maintainer of this file as it's 
only used by sam460ex so maybe an ack is not needed but it could help to 
show you have no problem with it.)

Regards,
BALATON Zoltan

Re: [PATCH v2] ppc440_pcix: Do not expose a bridge device on PCI bus

Posted by Philippe Mathieu-Daudé 2 weeks ago

On 14/4/24 15:00, BALATON Zoltan wrote:
> On Sat, 13 Apr 2024, Philippe Mathieu-Daudé wrote:
>> On 11/4/24 21:24, BALATON Zoltan wrote:
>>> Real 460EX SoC apparently does not expose a bridge device and having
>>> it appear on PCI bus confuses an AmigaOS file system driver that uses
>>> this to detect which machine it is running on.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> Here's another version that keeps the values and only drops the device
>>> so it's even less likely it could break anything, in case this can be
>>> accepted for 9.0.
>>>
>>>   hw/pci-host/ppc440_pcix.c | 11 ++++-------
>>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Thanks. Nick, could you ack this please so it could be merged if you 
> won't send more pull requests? (I'm the maintainer of this file as it's 
> only used by sam460ex so maybe an ack is not needed but it could help to 
> show you have no problem with it.)

No need, queued, thanks.

Re: [PATCH v2] ppc440_pcix: Do not expose a bridge device on PCI bus

Posted by BALATON Zoltan 2 weeks, 3 days ago

On Thu, 11 Apr 2024, BALATON Zoltan wrote:
> Real 460EX SoC apparently does not expose a bridge device and having
> it appear on PCI bus confuses an AmigaOS file system driver that uses
> this to detect which machine it is running on.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> Here's another version that keeps the values and only drops the device
> so it's even less likely it could break anything, in case this can be
> accepted for 9.0.

Looks like there will be an rc4 so can this one and
https://patchew.org/QEMU/20240410222543.0EA534E6005@zero.eik.bme.hu/
be also merged for 9.0 please?

Regards,
BALATON Zoltan

> hw/pci-host/ppc440_pcix.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/hw/pci-host/ppc440_pcix.c b/hw/pci-host/ppc440_pcix.c
> index 1926ae2a27..ef212d99aa 100644
> --- a/hw/pci-host/ppc440_pcix.c
> +++ b/hw/pci-host/ppc440_pcix.c
> @@ -52,7 +52,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST)
> struct PPC440PCIXState {
>     PCIHostState parent_obj;
>
> -    PCIDevice *dev;
> +    uint8_t config[PCI_CONFIG_SPACE_SIZE];
>     struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
>     struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
>     uint32_t sts;
> @@ -171,7 +171,7 @@ static void ppc440_pcix_reg_write4(void *opaque, hwaddr addr,
>     trace_ppc440_pcix_reg_write(addr, val, size);
>     switch (addr) {
>     case PCI_VENDOR_ID ... PCI_MAX_LAT:
> -        stl_le_p(s->dev->config + addr, val);
> +        stl_le_p(s->config + addr, val);
>         break;
>
>     case PCIX0_POM0LAL:
> @@ -302,7 +302,7 @@ static uint64_t ppc440_pcix_reg_read4(void *opaque, hwaddr addr,
>
>     switch (addr) {
>     case PCI_VENDOR_ID ... PCI_MAX_LAT:
> -        val = ldl_le_p(s->dev->config + addr);
> +        val = ldl_le_p(s->config + addr);
>         break;
>
>     case PCIX0_POM0LAL:
> @@ -498,10 +498,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
>     memory_region_init(&s->iomem, OBJECT(dev), "pci-io", 64 * KiB);
>     h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
>                          ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
> -                         PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
> -
> -    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
> -                               TYPE_PPC4xx_HOST_BRIDGE);
> +                         PCI_DEVFN(1, 0), 1, TYPE_PCI_BUS);
>
>     memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
>     memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
>