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

[Julien Grall <julien@xen.org>]

(+ 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.

Also, public/arch-arm.h will need to be updated as we now support the 
PHYSDEVOP hypercall.

> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> ---
>   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.

> +#endif
> +    default:
> +        gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
> +        ret = -ENOSYS;
> +    }
> +
> +    return ret;
>   }
>   
>   /*
> 

Cheers,

-- 
Julien Grall