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

Hey Mark,

On 6/7/26 23:16, Mark Cave-Ayland wrote:
> On 06/07/2026 21:29, Philippe Mathieu-Daudé wrote:
> 
>> 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).
> 
> Thanks for taking a look, Phil. IIRC the reason I added 
> slot_reserved_mask was so that it was possible to populate pciA 
> programatically in sun4uv_init(), and then enable the mask so that 
> specifying an additional PCI device using -device on the command line 
> without an explict PCI bus would cause the automatic allocation to place 
> the device on pciB.

The slot_reserved_mask calls make sense, but AFAICT they don't work as
expected, since CLI devices are added on pciA. If you want to have a
look on top of this snippet, be my guest! Otherwise I'll continue
debugging that tomorrow, long day and late here :\

Regards,

Phil.