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.
> 
> I wasn't sure whether merging this patch now or wait for your update
> during the freeze so hold my PR for a bit; today I looked a bit at it.
> 
> Some notes:
> 
> - pci_bus_*_slot_reserved_mask() API isn't much used. I supposed it
>    worked for a pair of corner cases but doesn't scale (I expect it
>    to be called in more code paths).
> 
> - Bus capacity is odd. We have a generic Bus-class @max_dev field but
>    do not use it with PCI variants. If set, it limit the instances
>    capacity. The capacity is tracked in the instance state, and when
>    no @max_dev is set the only way to mark a bus full is using the
>    qbus_mark_full() helper. Then if we remove devices, the bus remains
>    full, and there is no helper to restore available slots.
> 
> 
> 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 */

Per the manual "Up to four Advanced PCI Bridge ASICs (APB)s may be used
with the UltraSPARC-IIi".

> +    qbus_mark_full(&phb->bus->qbus);
>   }