On 19.08.2021 14:35, Julien Grall wrote:
> 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.
I don't think "latest" matters much here. Considering there was no
physdevop support at all on Arm, enabling whichever set seems like
a good fit would be okay.
Having written this I realize that by "latest" you may mean whether
the used sub-ops have not been obsoleted by newer ones (rather than
the last ones that were added to the physdevops set). While indeed
PHYSDEVOP_pci_device_add hasn't been superseded so far, I have a
vague recollection of there being some missing part. Without me
remembering details I'm afraid using what is there is the best we
can do for for the moment. However, ...
>> --- 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;
>> + }
... I don't think it should be only "add" which gets supported. "remove"
exists not just for the purpose of hot-unplug, but also for Dom0 to
remove (and then re-add) devices after e.g. bus re-numbering. (There are
some gaps there iirc, but still ...)
> 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.
+1 (if sensibly possible)
Jan
