Re: [PATCH v1 3/3] xen/pci: add discovered PCI device at boot

[Mykyta Poturai <Mykyta_Poturai@xxxxxxxx>]

On 04.08.25 11:35, Jan Beulich wrote:
> On 01.08.2025 11:22, Mykyta Poturai wrote:
>> Create add_discovered_pci_devices function that calls pci_device_add
>> on every PCI device discovered.
>> The devices will be added to dom_io so that they can be assigned
>> later to other domains.
> 
> And why's the intermediate step necessary? IOW can't they be assigned to their
> target domains right away, and only whatever's left would go to DOM_IO?

For Dom0less case, guest domains are not yet created at this point.

>> --- a/xen/drivers/passthrough/pci.c
>> +++ b/xen/drivers/passthrough/pci.c
>> @@ -1180,6 +1180,34 @@ int __init scan_pci_devices(void)
>>       return ret;
>>   }
>>   
>> +static int __init _add_discovered_pci_devices(struct pci_seg *pseg, void 
>> *arg)
>> +{
>> +    struct pci_dev *pdev;
>> +    int ret = 0;
>> +
>> +    list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
>> +    {
>> +        ret = pci_add_device(dom_io, pdev->seg, pdev->bus, pdev->devfn, 
>> NULL,
>> +                             NUMA_NO_NODE);
>> +        if ( ret < 0 )
>> +        {
>> +            printk(XENLOG_ERR
>> +                   "%pp: Failure adding the discovered pci device (Error 
>> %d)\n",
>> +                   &pdev->sbdf, ret);
>> +            break;
>> +        }
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +void __init add_discovered_pci_devices(void)
>> +{
>> +    pcidevs_lock();
>> +    pci_segments_iterate(_add_discovered_pci_devices, NULL);
>> +    pcidevs_unlock();
>> +}
> 
> This looks to merely be a specialized form of what ...
> 
>>   struct setup_hwdom {
>>       struct domain *d;
>>       int (*handler)(uint8_t devfn, struct pci_dev *pdev);
> 
> ... follows below here. By generalizing what we have (perhaps from the top, 
> i.e.
> iommu_hwdom_init()), you'd also avoid violating Misra rule 2.1 on x86, as you 
> add
> unreachable code there.
> 
> Jan

Can you please elaborate a little further on how you see this 
generalization? With routines you mentioned being specifically for 
hwdom, which may not exist, and the different approaches to PCI 
initialization on Arm/x86 (as far as I understand on x86 all of the PCI 
related stuff is initialized from iommu helpers, while Arm has a 
dedicated init step) I am afraid I can't find a nice point of contact to 
generalize this, but I can be missing somthing due to unfamiliarity with 
x86 code.

Another way of addressing possible MISRA violation I can see is moving 
the add_discovered_pci_devices to xen/arch/arm/pci/pci.c so it will only 
be compiled when used.

-- 
Mykyta