Re: [PATCH v1 10/14] xen/arm: Discovering PCI devices and add the PCI devices in XEN.

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Julien,

> On 19 Aug 2021, at 1:35 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> (+ Jan)
> 
> Hi Rahul,
> 
> On 19/08/2021 13:02, Rahul Singh wrote:
>> Hardware domain is in charge of doing the PCI enumeration and will
>> discover the PCI devices and then will communicate to XEN via hyper
>> call PHYSDEVOP_pci_device_add to add the PCI devices in XEN.
> 
> There are other PHYSDEVOP operations to add PCI devices. I think it is fine 
> to only implement the latest (CC Jan for some opinion and confirm this is the 
> latest). However, this ought to be explained in the commit message.

As per Jan comments I will add the PHYSDEVOP_pci_device_remove() in the next 
version.
> 
> Also, public/arch-arm.h will need to be updated as we now support the 
> PHYSDEVOP hypercall.

Ok.
> 
>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>> ---
>>  xen/arch/arm/physdev.c | 39 ++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 36 insertions(+), 3 deletions(-)
>> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
>> index e91355fe22..ccce8f0eba 100644
>> --- a/xen/arch/arm/physdev.c
>> +++ b/xen/arch/arm/physdev.c
>> @@ -9,12 +9,45 @@
>>  #include <xen/errno.h>
>>  #include <xen/sched.h>
>>  #include <asm/hypercall.h>
>> -
>> +#include <xen/guest_access.h>
>> +#include <xsm/xsm.h>
>>    int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>  {
>> -    gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
>> -    return -ENOSYS;
>> +    int ret = 0;
>> +
>> +    switch ( cmd )
>> +    {
>> +#ifdef CONFIG_HAS_PCI
>> +    case PHYSDEVOP_pci_device_add: {
>> +        struct physdev_pci_device_add add;
>> +        struct pci_dev_info pdev_info;
>> +        nodeid_t node = NUMA_NO_NODE;
>> +
>> +        ret = -EFAULT;
>> +        if ( copy_from_guest(&add, arg, 1) != 0 )
>> +            break;
>> +
>> +        pdev_info.is_extfn = !!(add.flags & XEN_PCI_DEV_EXTFN);
>> +        if ( add.flags & XEN_PCI_DEV_VIRTFN )
>> +        {
>> +            pdev_info.is_virtfn = 1;
>> +            pdev_info.physfn.bus = add.physfn.bus;
>> +            pdev_info.physfn.devfn = add.physfn.devfn;
>> +        }
>> +        else
>> +            pdev_info.is_virtfn = 0;
>> +
>> +        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
>> +        break;
>> +    }
> 
> This is pretty much a copy of the x86 version without the NUMA bit. So I 
> think we want to move the implementation in common code.

Ok. Let me move the PHYSDEVOP_pci_device_* to common code.

Regards,
Rahul