Re: [PATCH v2 10/11] vpci: Add initial support for virtual PCI bus topology

[Michal Orzel <michal.orzel@xxxxxxx>]

On 28.09.2021 09:59, Jan Beulich wrote:
> On 28.09.2021 09:48, Michal Orzel wrote:
>> On 23.09.2021 14:55, Oleksandr Andrushchenko wrote:
>>> --- a/xen/drivers/passthrough/pci.c
>>> +++ b/xen/drivers/passthrough/pci.c
>>> @@ -833,6 +833,63 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
>>>      return ret;
>>>  }
>>>  
>>> +static struct vpci_dev *pci_find_virtual_device(const struct domain *d,
>>> +                                                const struct pci_dev *pdev)
>>> +{
>>> +    struct vpci_dev *vdev;
>>> +
>>> +    list_for_each_entry ( vdev, &d->vdev_list, list )
>>> +        if ( vdev->pdev == pdev )
>>> +            return vdev;
>>> +    return NULL;
>>> +}
>>> +
>>> +int pci_add_virtual_device(struct domain *d, const struct pci_dev *pdev)
>>> +{
>>> +    struct vpci_dev *vdev;
>>> +
>>> +    ASSERT(!pci_find_virtual_device(d, pdev));
>>> +
>>> +    /* Each PCI bus supports 32 devices/slots at max. */
>>> +    if ( d->vpci_dev_next > 31 )
>>> +        return -ENOSPC;
>>> +
>>> +    vdev = xzalloc(struct vpci_dev);
>>> +    if ( !vdev )
>>> +        return -ENOMEM;
>>> +
>>> +    /* We emulate a single host bridge for the guest, so segment is always 
>>> 0. */
>>> +    *(u16*) &vdev->seg = 0;
>> Empty line hear would improve readability due to the asterisks being so 
>> close to each other.
>> Apart from that:
>> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
>>> +    /*
>>> +     * The bus number is set to 0, so virtual devices are seen
>>> +     * as embedded endpoints behind the root complex.
>>> +     */
>>> +    *((u8*) &vdev->bus) = 0;
>>> +    *((u8*) &vdev->devfn) = PCI_DEVFN(d->vpci_dev_next++, 0);
> 
> All of these casts are (a) malformed and (b) unnecessary in the first
> place, afaics at least.
> 
Agree.
*((u8*) &vdev->bus) = 0;
is the same as:
vdev->bus = 0;
> Jan
>