On 28.09.21 11:17, Michal Orzel wrote:
>
> 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.
Will add
>>> Apart from that:
>>> Reviewed-by: Michal Orzel <michal.or...@arm.com>
>>>> + /*
>>>> + * 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;
Overengineering at its best ;)
Will fix that
>> Jan
>>
Thank you,
Oleksandr
