Re: [PATCH v6 2/3] xen/arm: Enable the existing x86 virtual PCI support for ARM.

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jan,

> On 15 Oct 2021, at 11:41, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 15.10.2021 12:33, Bertrand Marquis wrote:
>>> On 15 Oct 2021, at 11:24, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> On 15.10.2021 11:52, Bertrand Marquis wrote:
>>>>> On 15 Oct 2021, at 09:32, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>>>>> On Thu, Oct 14, 2021 at 03:49:50PM +0100, Bertrand Marquis wrote:
>>>>>> @@ -752,6 +752,19 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>>>>>> 
>>>>>>   check_pdev(pdev);
>>>>>> 
>>>>>> +#ifdef CONFIG_ARM
>>>>>> +    /*
>>>>>> +     * On ARM PCI devices discovery will be done by Dom0. Add vpci 
>>>>>> handler when
>>>>>> +     * Dom0 inform XEN to add the PCI devices in XEN.
>>>>>> +     */
>>>>>> +    ret = vpci_add_handlers(pdev);
>>>>>> +    if ( ret )
>>>>>> +    {
>>>>>> +        printk(XENLOG_ERR "Setup of vPCI failed: %d\n", ret);
>>>>>> +        goto out;
>>>>>> +    }
>>>>>> +#endif
>>>>> 
>>>>> I think vpci_add_handlers should be called after checking that
>>>>> pdev->domain is != NULL, so I would move this chunk a bit below.
>>>> 
>>>> On arm this would prevent the dom0less use case or to have the PCI
>>>> bus enumerated from an other domain.
>>>> @oleksandr: can you comment on this one, you might have a better
>>>> answer than me on this ?
>>> 
>>> Well, without Xen doing the enumeration, some other entity would need
>>> to do so, including the reporting to Xen. Obviously without a Dom0 it
>>> would be ambiguous which domain to assign the device to; perhaps it
>>> should be the caller in this case? That would make that caller domain
>>> a pseudo-hwdom though, as far as PCI is concerned, which may not be
>>> desirable according to my (limited) understanding of dom0less.
>> 
>> This is not really related to this patch but the plan is the following:
>> - enumeration would have to be done by the firmware or boot loader before
>> - xen will have some code to detect PCI devices
>> - dom0less can be used to assign PCI devices to guest
>> 
>> Anyway does not change the fact that this must be called when domain is
>> not NULL and I will fix that.
> 
> Since we now all seem to agree that the NULL would have been a problem,
> may I ask in how far any of this has actually been tested?

With the whole serie currently on gitlab we have extensively tested passing
through PCI devices on Arm in several configuration (number of device, MSI,
MSI-X) and check that PCI was still functional on x86.

With the patches pushed to Xen right now it was checked that:
- xen compiles properly on arm32, arm64 and x86
- xen compiles properly with VPCI activated (using a patch) on arm32 and arm64
- xen on x86 is functionnal (using basic test on QEMU)
- xen on arm64 is functionnal (with some extensive tests on different targets)

We are only lacking some actual testing on arm32 internally.

Regards
Bertrand

> 
> Jan