Re: [PATCH v2] hw/sparc64/sun4u: Mark unusable PCI busses as full to ease device plugging

On 6/7/26 19:29, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 30/6/26 00:20, Mark Cave-Ayland wrote:
>> On 29/06/2026 14:43, Thomas Huth wrote:
>>
>>> On 09/06/2026 08.02, Thomas Huth wrote:
>>>> On 08/06/2026 23.29, Mark Cave-Ayland wrote:
>>>>> On 08/06/2026 09:26, Michael Tokarev wrote:
>>>>>
>>>>>> On 30.04.2026 12:50, Thomas Huth wrote:
>>>>>>> On 09/03/2026 19.14, Thomas Huth wrote:
>>>>>>>> From: Thomas Huth <[email protected]>
>>>>>>>>
>>>>>>>> When trying to plug a PCI device to a Sparc64 machine, you 
>>>>>>>> currently
>>>>>>>> have to specify the right bus ("bus=pciB"), otherwise you get 
>>>>>>>> this error:
>>>>>>>>
>>>>>>>>   $ qemu-system-sparc64 -device virtio-scsi-pci
>>>>>>>>   qemu-system-sparc64: -device virtio-scsi-pci: PCI: no slot/ 
>>>>>>>> function
>>>>>>>>   available for virtio-scsi-pci, all in use or reserved
>>>>>>>>
>>>>>>>> This is quite annoying for the unexperienced users, and it also 
>>>>>>>> breaks
>>>>>>>> e.g. the iotests ("make check-block") when running with qemu- 
>>>>>>>> system- sparc64.
>>>>>>>>
>>>>>>>> Mark the non-usable PCI busses as full now, so that QEMU can 
>>>>>>>> automatically
>>>>>>>> plug new PCI devices to the right "pciB" bus.
>>>>>>>>
>>>>>>>> Signed-off-by: Thomas Huth <[email protected]>
>>>>>>>> ---
>>>>>>>>   v2: Do the change in the machine code, not in the sabre device
>>>>>>>>
>>>>>>>>   hw/sparc64/sun4u.c | 7 +++++++
>>>>>>>>   1 file changed, 7 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>>>>>>>> index b8bda1eb816..37539535c6c 100644
>>>>>>>> --- a/hw/sparc64/sun4u.c
>>>>>>>> +++ b/hw/sparc64/sun4u.c
>>>>>>>> @@ -723,6 +723,13 @@ static void sun4uv_init(MemoryRegion 
>>>>>>>> *address_space_mem,
>>>>>>>>       fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_DEPTH, graphic_depth);
>>>>>>>>       qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>>>>>>>> +
>>>>>>>> +    /*
>>>>>>>> +     * Mark internal PCI busses as full so that the plugging of 
>>>>>>>> additional
>>>>>>>> +     * PCI devices happens on the right bus that still has free 
>>>>>>>> slots:
>>>>>>>> +     */
>>>>>>>> +    qbus_mark_full(&pci_bus->qbus);
>>>>>>>> +    qbus_mark_full(&pci_busA->qbus);
>>>>>>>>   }
>>>>>>>>   enum {
>>>>>>>
>>>>>>> Friendly ping!
>>>>>>
>>>>>> This, being a simple change, somehow has not been reviewed still, so
>>>>>> it's sitting in the queue...
>>>>>
>>>>> Yes, apologies - I've been absolutely flat out recently, and I've 
>>>>> got a note to take a look at this and Peter's patch as soon as I can.
>>>>>
>>>>> Is this fixing an urgent issue with "make check-block" or similar?
>>>>
>>>> It's not urgent since this has been broken forever, but yes, it 
>>>> fixes "make check- block" for the case where sparc64 is the only 
>>>> available target during "configure -- target-list=sparc64-softmmu" 
>>>> and thus the iotests run with qemu-system-sparc64.
>>> Ping!
>>>
>>> While it's not urgent, soft-freeze date is in a week, so it would be 
>>> nice to get this merged before it misses another release ... WDYT?
>>
>> Hi Thomas,
>>
>> I'm still playing catch-up a bit here, but I can't immediately find my 
>> notes on using pciA under QEMU. I think if it fixes the iotests then 
>> go ahead and merge it for now, which gives me time in freeze to 
>> investigate any reported issues.


> Anyway, back to the problem here, the first part is indeed as noticed
> Thomas to use qbus_mark_full() for the root bus:
> 
> -- >8 --
> diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
> index cd2328ad53f..6a8d0b6c6d2 100644
> --- a/hw/pci-host/sabre.c
> +++ b/hw/pci-host/sabre.c
> @@ -400,6 +400,9 @@ static void sabre_realize(DeviceState *dev, Error 
> **errp)
>       s->bridgeA = PCI_BRIDGE(pci_dev);
>       pci_bridge_map_irq(s->bridgeA, "pciA", pci_simbaA_map_irq);
>       pci_realize_and_unref(pci_dev, phb->bus, &error_fatal);
> +
> +    /* Only in-built Simba APBs can exist on the root bus */
> +    qbus_mark_full(&phb->bus->qbus);
>   }
> 
>   static void sabre_init(Object *obj)
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index d69ed9a81ac..b65f3d1a140 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -546,7 +546,7 @@ static void sun4uv_init(MemoryRegion 
> *address_space_mem,
>       unsigned int i;
>       uint64_t initrd_addr, initrd_size, kernel_addr, kernel_size, 
> kernel_entry;
>       SabreState *sabre;
> -    PCIBus *pci_bus, *pci_busA, *pci_busB;
> +    PCIBus *pci_busA, *pci_busB;
>       PCIDevice *ebus, *pci_dev;
>       SysBusDevice *s;
>       DeviceState *iommu, *dev;
> @@ -588,14 +588,13 @@ static void sun4uv_init(MemoryRegion 
> *address_space_mem,
>               qdev_get_gpio_in_named(DEVICE(cpu), "ivec-irq", i));
>       }
> 
> -    pci_bus = PCI_HOST_BRIDGE(sabre)->bus;
>       pci_busA = pci_bridge_get_sec_bus(sabre->bridgeA);
>       pci_busB = pci_bridge_get_sec_bus(sabre->bridgeB);
> 
> -    /* Only in-built Simba APBs can exist on the root bus, slot 0 on 
> busA is
> -       reserved (leaving no slots free after on-board devices) however 
> slots
> -       0-3 are free on busB */
> -    pci_bus_set_slot_reserved_mask(pci_bus, 0xfffffffc);
> +    /*
> +     * Slot 0 on busA is reserved (leaving no slots free after on-board
> +     * devices) however slots 0-3 are free on busB
> +     */
>       pci_bus_set_slot_reserved_mask(pci_busA, 0xfffffff1);
>       pci_bus_set_slot_reserved_mask(pci_busB, 0xfffffff0);
> 
> ---
> 
> I'm still looking at the 2nd part, the reserved slots.

(Because with this patch, the test passes as we can plug devices from
the command line, but they end plugged on pciA instead of pciB -- thus
the 2nd part missing).