[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [KERNEL PATCH v9 3/3] xen/privcmd: Add new syscall to get gsi from dev
On Fri, 20 Sep 2024, Chen, Jiqian wrote: > On 2024/9/19 06:49, Stefano Stabellini wrote: > > On Thu, 12 Sep 2024, Jiqian Chen wrote: > >> On PVH dom0, when passthrough a device to domU, QEMU and xl tools > >> want to use gsi number to do pirq mapping, see QEMU code > >> xen_pt_realize->xc_physdev_map_pirq, and xl code > >> pci_add_dm_done->xc_physdev_map_pirq, but in current codes, the gsi > >> number is got from file /sys/bus/pci/devices/<sbdf>/irq, that is > >> wrong, because irq is not equal with gsi, they are in different > >> spaces, so pirq mapping fails. > >> And in current linux codes, there is no method to get gsi > >> for userspace. > >> > >> For above purpose, record gsi of pcistub devices when init > >> pcistub and add a new syscall into privcmd to let userspace > >> can get gsi when they have a need. > >> > >> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx> > >> Signed-off-by: Huang Rui <ray.huang@xxxxxxx> > >> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx> > >> --- > >> v8->v9 changes: > >> Changed the syscall name from "IOCTL_PRIVCMD_GSI_FROM_DEV" to > >> "IOCTL_PRIVCMD_PCIDEV_GET_GSI". Also changed the other functions name. > >> Changed the macro wrapping "pcistub_get_gsi_from_sbdf" from > >> "CONFIG_XEN_ACPI" to "CONFIG_XEN_PCIDEV_BACKEND" to fix compile errors > >> reported by CI robot. > >> Changed the parameter gsi of struct privcmd_pcidev_get_gsi from int to u32. > >> > >> v7->v8 changes: > >> In function privcmd_ioctl_gsi_from_dev, return -EINVAL when not confige > >> CONFIG_XEN_ACPI. > >> Used PCI_BUS_NUM PCI_SLOT PCI_FUNC instead of open coding. > >> > >> v6->v7 changes: > >> Changed implementation to add a new parameter "gsi" to struct > >> pcistub_device and set gsi when pcistub initialize device. Then when > >> userspace wants to get gsi and pass sbdf, we can return that gsi. > >> > >> v5->v6 changes: > >> Changed implementation to add a new syscall to translate irq to gsi, > >> instead adding a new gsi sysfs node, because the pci Maintainer didn't > >> allow to add that sysfs node. > >> > >> v3->v5 changes: > >> No. > >> > >> v2->v3 changes: > >> Suggested by Roger: Abandoned previous implementations that added new > >> syscall to get gsi from irq and changed to add a new sysfs node for gsi, > >> then userspace can get gsi number from sysfs node. > >> --- > >> | Reported-by: kernel test robot <lkp@xxxxxxxxx> > >> | Closes: > >> https://lore.kernel.org/oe-kbuild-all/202406090826.whl6Cb7R-lkp@xxxxxxxxx/ > >> --- > >> | Reported-by: kernel test robot <lkp@xxxxxxxxx> > >> | Closes: > >> https://lore.kernel.org/oe-kbuild-all/202405171113.T431PC8O-lkp@xxxxxxxxx/ > >> --- > >> drivers/xen/privcmd.c | 30 +++++++++++++++++++++++ > >> drivers/xen/xen-pciback/pci_stub.c | 38 +++++++++++++++++++++++++++--- > >> include/uapi/xen/privcmd.h | 7 ++++++ > >> include/xen/acpi.h | 9 +++++++ > >> 4 files changed, 81 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c > >> index 9563650dfbaf..1ed612d21543 100644 > >> --- a/drivers/xen/privcmd.c > >> +++ b/drivers/xen/privcmd.c > >> @@ -46,6 +46,9 @@ > >> #include <xen/page.h> > >> #include <xen/xen-ops.h> > >> #include <xen/balloon.h> > >> +#ifdef CONFIG_XEN_ACPI > >> +#include <xen/acpi.h> > >> +#endif > >> > >> #include "privcmd.h" > >> > >> @@ -844,6 +847,29 @@ static long privcmd_ioctl_mmap_resource(struct file > >> *file, > >> return rc; > >> } > >> > >> +static long privcmd_ioctl_pcidev_get_gsi(struct file *file, void __user > >> *udata) > >> +{ > >> +#ifdef CONFIG_XEN_ACPI > >> + int rc; > >> + struct privcmd_pcidev_get_gsi kdata; > >> + > >> + if (copy_from_user(&kdata, udata, sizeof(kdata))) > >> + return -EFAULT; > >> + > >> + rc = pcistub_get_gsi_from_sbdf(kdata.sbdf); > >> + if (rc < 0) > >> + return rc; > >> + > >> + kdata.gsi = rc; > >> + if (copy_to_user(udata, &kdata, sizeof(kdata))) > >> + return -EFAULT; > >> + > >> + return 0; > >> +#else > >> + return -EINVAL; > >> +#endif > >> +} > >> + > >> #ifdef CONFIG_XEN_PRIVCMD_EVENTFD > >> /* Irqfd support */ > >> static struct workqueue_struct *irqfd_cleanup_wq; > >> @@ -1543,6 +1569,10 @@ static long privcmd_ioctl(struct file *file, > >> ret = privcmd_ioctl_ioeventfd(file, udata); > >> break; > >> > >> + case IOCTL_PRIVCMD_PCIDEV_GET_GSI: > >> + ret = privcmd_ioctl_pcidev_get_gsi(file, udata); > >> + break; > >> + > >> default: > >> break; > >> } > >> diff --git a/drivers/xen/xen-pciback/pci_stub.c > >> b/drivers/xen/xen-pciback/pci_stub.c > >> index 8ce27333f54b..2ea8e4075adc 100644 > >> --- a/drivers/xen/xen-pciback/pci_stub.c > >> +++ b/drivers/xen/xen-pciback/pci_stub.c > >> @@ -56,6 +56,9 @@ struct pcistub_device { > >> > >> struct pci_dev *dev; > >> struct xen_pcibk_device *pdev;/* non-NULL if struct pci_dev is in use */ > >> +#ifdef CONFIG_XEN_ACPI > >> + int gsi; > >> +#endif > >> }; > >> > >> /* Access to pcistub_devices & seized_devices lists and the > >> initialize_devices > >> @@ -88,6 +91,9 @@ static struct pcistub_device > >> *pcistub_device_alloc(struct pci_dev *dev) > >> > >> kref_init(&psdev->kref); > >> spin_lock_init(&psdev->lock); > >> +#ifdef CONFIG_XEN_ACPI > >> + psdev->gsi = -1; > >> +#endif > >> > >> return psdev; > >> } > >> @@ -220,6 +226,25 @@ static struct pci_dev > >> *pcistub_device_get_pci_dev(struct xen_pcibk_device *pdev, > >> return pci_dev; > >> } > >> > >> +#ifdef CONFIG_XEN_PCIDEV_BACKEND > > > > > > This breaks configurations without CONFIG_ACPI and with > > CONFIG_XEN_PCIDEV_BACKEND. > > > > Also there should be no dependency between PCIDEV_BACKEND and > > pcistub_get_gsi_from_sbdf. > > > > I think we should solve the build issues this way: > > > > - privcmd_ioctl_pcidev_get_gsi should have: > > #if defined(CONFIG_XEN_ACPI) && defined(CONFIG_XEN_PCI_STUB) > > > > - here we should have #ifdef CONFIG_XEN_ACPI as you had before > > > > > > As far as I can tell the above should be able to address all valid > > combinations. > This can't pass a combination that: > CONFIG_XEN_ACPI=y > CONFIG_XEN_PCI_STUB=y > CONFIG_XEN_PCIDEV_BACKEND=m > Reported by robot > https://lore.kernel.org/oe-kbuild-all/202406090826.whl6Cb7R-lkp@xxxxxxxxx/ > > At this combination, privcmd_ioctl_pcidev_get_gsi can call > pcistub_get_gsi_from_sbdf because CONFIG_XEN_ACPI and CONFIG_XEN_PCI_STUB are > both "y". > But when the compiler tries to find the implementation of > pcistub_get_gsi_from_sbdf, it fails (ld: vmlinux.o: in function > `privcmd_ioctl_pcidev_get_gsi': > /home/cjq/code/upstream/kernel_test_robot/linux_xen/build_dir/../drivers/xen/privcmd.c:859: > undefined reference to `pcistub_get_gsi_from_sbdf'), because the value of > CONFIG_XEN_PCIDEV_BACKEND is "m" (file drivers/xen/xen-pciback/Makefile shows > "obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback.o"), so that xen-pciback is > built as a module. The attached .config, with the appended changes on top of your patch work for me. I have CONFIG_XEN_PCIDEV_BACKEND=m. diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 1ed612d21543..49e1c5e0c32a 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -849,7 +849,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file, static long privcmd_ioctl_pcidev_get_gsi(struct file *file, void __user *udata) { -#ifdef CONFIG_XEN_ACPI +#if defined(CONFIG_XEN_ACPI) && defined(CONFIG_XEN_PCI_STUB) int rc; struct privcmd_pcidev_get_gsi kdata; diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index 2ea8e4075adc..d5807b622a59 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -226,7 +226,7 @@ static struct pci_dev *pcistub_device_get_pci_dev(struct xen_pcibk_device *pdev, return pci_dev; } -#ifdef CONFIG_XEN_PCIDEV_BACKEND +#ifdef CONFIG_ACPI int pcistub_get_gsi_from_sbdf(unsigned int sbdf) { struct pcistub_device *psdev; Attachment:
.config
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |