| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190319121416.27495651@x1.home>
Date: Tue, 19 Mar 2019 12:14:16 -0600
From: Alex Williamson <alex.williamson@...hat.com>
To: "Liu, Yi L" <yi.l.liu@...el.com>
Cc: kwankhede@...dia.com, kevin.tian@...el.com,
baolu.lu@...ux.intel.com, yi.y.sun@...el.com, joro@...tes.org,
jean-philippe.brucker@....com, peterx@...hat.com,
linux-kernel@...r.kernel.org
Subject: Re: [RFC v2 1/2] vfio/pci: export common symbols in vfio-pci
On Tue, 12 Mar 2019 16:18:22 +0800
"Liu, Yi L" <yi.l.liu@...el.com> wrote:
> This patch exports the following symbols from vfio-pci driver
> for vfio-pci alike driver. e.g. vfio-pci-mdev driver
>
> *) vfio_pci_set_vga_decode();
> *) vfio_pci_release();
> *) vfio_pci_open();
> *) vfio_pci_register_dev_region();
> *) vfio_pci_ioctl();
> *) vfio_pci_rw();
> *) vfio_pci_mmap();
> *) vfio_pci_request();
> *) vfio_pci_probe_misc();
> *) vfio_pci_remove_misc();
> *) vfio_err_handlers;
> *) vfio_pci_reflck_attach();
> *) vfio_pci_reflck_put();
Exporting all these symbols scares me a bit. They're GPL and we don't
guarantee a kernel ABI, but I don't really want to support arbitrary
use cases either. What if we re-factored the shared bits into a common
file and just linked them together? Thanks,
Alex
> Cc: Kevin Tian <kevin.tian@...el.com>
> Cc: Lu Baolu <baolu.lu@...ux.intel.com>
> Signed-off-by: Liu, Yi L <yi.l.liu@...el.com>
> ---
> drivers/vfio/pci/vfio_pci.c | 101 ++++++++++++++++++++++--------------
> drivers/vfio/pci/vfio_pci_private.h | 17 ++++++
> 2 files changed, 78 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index ff60bd1..e36b922 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -73,7 +73,7 @@ static inline bool vfio_vga_disabled(void)
> * has no way to get to it and routing can be disabled externally at the
> * bridge.
> */
> -static unsigned int vfio_pci_set_vga_decode(void *opaque, bool single_vga)
> +unsigned int vfio_pci_set_vga_decode(void *opaque, bool single_vga)
> {
> struct vfio_pci_device *vdev = opaque;
> struct pci_dev *tmp = NULL, *pdev = vdev->pdev;
> @@ -103,6 +103,7 @@ static unsigned int vfio_pci_set_vga_decode(void *opaque, bool single_vga)
>
> return decodes;
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_set_vga_decode);
>
> static inline bool vfio_pci_is_vga(struct pci_dev *pdev)
> {
> @@ -410,7 +411,7 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
> pci_set_power_state(pdev, PCI_D3hot);
> }
>
> -static void vfio_pci_release(void *device_data)
> +void vfio_pci_release(void *device_data)
> {
> struct vfio_pci_device *vdev = device_data;
>
> @@ -425,8 +426,9 @@ static void vfio_pci_release(void *device_data)
>
> module_put(THIS_MODULE);
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_release);
>
> -static int vfio_pci_open(void *device_data)
> +int vfio_pci_open(void *device_data)
> {
> struct vfio_pci_device *vdev = device_data;
> int ret = 0;
> @@ -450,6 +452,7 @@ static int vfio_pci_open(void *device_data)
> module_put(THIS_MODULE);
> return ret;
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_open);
>
> static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
> {
> @@ -634,9 +637,10 @@ int vfio_pci_register_dev_region(struct vfio_pci_device *vdev,
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_register_dev_region);
>
> -static long vfio_pci_ioctl(void *device_data,
> - unsigned int cmd, unsigned long arg)
> +long vfio_pci_ioctl(void *device_data,
> + unsigned int cmd, unsigned long arg)
> {
> struct vfio_pci_device *vdev = device_data;
> unsigned long minsz;
> @@ -1079,8 +1083,9 @@ static long vfio_pci_ioctl(void *device_data,
>
> return -ENOTTY;
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_ioctl);
>
> -static ssize_t vfio_pci_rw(void *device_data, char __user *buf,
> +ssize_t vfio_pci_rw(void *device_data, char __user *buf,
> size_t count, loff_t *ppos, bool iswrite)
> {
> unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> @@ -1111,6 +1116,7 @@ static ssize_t vfio_pci_rw(void *device_data, char __user *buf,
>
> return -EINVAL;
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_rw);
>
> static ssize_t vfio_pci_read(void *device_data, char __user *buf,
> size_t count, loff_t *ppos)
> @@ -1130,7 +1136,7 @@ static ssize_t vfio_pci_write(void *device_data, const char __user *buf,
> return vfio_pci_rw(device_data, (char __user *)buf, count, ppos, true);
> }
>
> -static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma)
> +int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma)
> {
> struct vfio_pci_device *vdev = device_data;
> struct pci_dev *pdev = vdev->pdev;
> @@ -1173,7 +1179,7 @@ static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma)
> */
> if (!vdev->barmap[index]) {
> ret = pci_request_selected_regions(pdev,
> - 1 << index, "vfio-pci");
> + 1 << index, vdev->name);
> if (ret)
> return ret;
>
> @@ -1191,8 +1197,9 @@ static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma)
> return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> req_len, vma->vm_page_prot);
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_mmap);
>
> -static void vfio_pci_request(void *device_data, unsigned int count)
> +void vfio_pci_request(void *device_data, unsigned int count)
> {
> struct vfio_pci_device *vdev = device_data;
>
> @@ -1211,6 +1218,7 @@ static void vfio_pci_request(void *device_data, unsigned int count)
>
> mutex_unlock(&vdev->igate);
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_request);
>
> static const struct vfio_device_ops vfio_pci_ops = {
> .name = "vfio-pci",
> @@ -1223,8 +1231,31 @@ static const struct vfio_device_ops vfio_pci_ops = {
> .request = vfio_pci_request,
> };
>
> -static int vfio_pci_reflck_attach(struct vfio_pci_device *vdev);
> -static void vfio_pci_reflck_put(struct vfio_pci_reflck *reflck);
> +int vfio_pci_probe_misc(struct pci_dev *pdev, struct vfio_pci_device *vdev)
> +{
> + if (vfio_pci_is_vga(pdev)) {
> + vga_client_register(pdev, vdev, NULL, vfio_pci_set_vga_decode);
> + vga_set_legacy_decoding(pdev,
> + vfio_pci_set_vga_decode(vdev, false));
> + }
> +
> + if (!disable_idle_d3) {
> + /*
> + * pci-core sets the device power state to an unknown value at
> + * bootup and after being removed from a driver. The only
> + * transition it allows from this unknown state is to D0, which
> + * typically happens when a driver calls pci_enable_device().
> + * We're not ready to enable the device yet, but we do want to
> + * be able to get to D3. Therefore first do a D0 transition
> + * before going to D3.
> + */
> + pci_set_power_state(pdev, PCI_D0);
> + pci_set_power_state(pdev, PCI_D3hot);
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(vfio_pci_probe_misc);
>
> static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> @@ -1259,6 +1290,7 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> }
>
> vdev->pdev = pdev;
> + vdev->name = "vfio-pci";
> vdev->irq_type = VFIO_PCI_NUM_IRQS;
> mutex_init(&vdev->igate);
> spin_lock_init(&vdev->irqlock);
> @@ -1280,28 +1312,23 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> return ret;
> }
>
> + ret = vfio_pci_probe_misc(pdev, vdev);
> + return ret;
> +}
> +
> +void vfio_pci_remove_misc(struct pci_dev *pdev)
> +{
> if (vfio_pci_is_vga(pdev)) {
> - vga_client_register(pdev, vdev, NULL, vfio_pci_set_vga_decode);
> + vga_client_register(pdev, NULL, NULL, NULL);
> vga_set_legacy_decoding(pdev,
> - vfio_pci_set_vga_decode(vdev, false));
> + VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM |
> + VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM);
> }
>
> - if (!disable_idle_d3) {
> - /*
> - * pci-core sets the device power state to an unknown value at
> - * bootup and after being removed from a driver. The only
> - * transition it allows from this unknown state is to D0, which
> - * typically happens when a driver calls pci_enable_device().
> - * We're not ready to enable the device yet, but we do want to
> - * be able to get to D3. Therefore first do a D0 transition
> - * before going to D3.
> - */
> + if (!disable_idle_d3)
> pci_set_power_state(pdev, PCI_D0);
> - pci_set_power_state(pdev, PCI_D3hot);
> - }
> -
> - return ret;
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_remove_misc);
>
> static void vfio_pci_remove(struct pci_dev *pdev)
> {
> @@ -1317,16 +1344,7 @@ static void vfio_pci_remove(struct pci_dev *pdev)
> kfree(vdev->region);
> mutex_destroy(&vdev->ioeventfds_lock);
> kfree(vdev);
> -
> - if (vfio_pci_is_vga(pdev)) {
> - vga_client_register(pdev, NULL, NULL, NULL);
> - vga_set_legacy_decoding(pdev,
> - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM |
> - VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM);
> - }
> -
> - if (!disable_idle_d3)
> - pci_set_power_state(pdev, PCI_D0);
> + vfio_pci_remove_misc(pdev);
> }
>
> static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
> @@ -1357,9 +1375,10 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
> return PCI_ERS_RESULT_CAN_RECOVER;
> }
>
> -static const struct pci_error_handlers vfio_err_handlers = {
> +const struct pci_error_handlers vfio_err_handlers = {
> .error_detected = vfio_pci_aer_err_detected,
> };
> +EXPORT_SYMBOL_GPL(vfio_err_handlers);
>
> static struct pci_driver vfio_pci_driver = {
> .name = "vfio-pci",
> @@ -1418,7 +1437,7 @@ static int vfio_pci_reflck_find(struct pci_dev *pdev, void *data)
> return 0;
> }
>
> -static int vfio_pci_reflck_attach(struct vfio_pci_device *vdev)
> +int vfio_pci_reflck_attach(struct vfio_pci_device *vdev)
> {
> bool slot = !pci_probe_reset_slot(vdev->pdev->slot);
>
> @@ -1433,6 +1452,7 @@ static int vfio_pci_reflck_attach(struct vfio_pci_device *vdev)
>
> return PTR_ERR_OR_ZERO(vdev->reflck);
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_reflck_attach);
>
> static void vfio_pci_reflck_release(struct kref *kref)
> {
> @@ -1444,10 +1464,11 @@ static void vfio_pci_reflck_release(struct kref *kref)
> mutex_unlock(&reflck_lock);
> }
>
> -static void vfio_pci_reflck_put(struct vfio_pci_reflck *reflck)
> +void vfio_pci_reflck_put(struct vfio_pci_reflck *reflck)
> {
> kref_put_mutex(&reflck->kref, vfio_pci_reflck_release, &reflck_lock);
> }
> +EXPORT_SYMBOL_GPL(vfio_pci_reflck_put);
>
> struct vfio_devices {
> struct vfio_device **devices;
> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> index 8c0009f..da9afe5 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -90,6 +90,7 @@ struct vfio_pci_reflck {
> struct vfio_pci_device {
> struct pci_dev *pdev;
> void __iomem *barmap[PCI_STD_RESOURCE_END + 1];
> + char *name;
> bool bar_mmap_supported[PCI_STD_RESOURCE_END + 1];
> u8 *pci_config_map;
> u8 *vconfig;
> @@ -125,6 +126,8 @@ struct vfio_pci_device {
> struct list_head ioeventfds_list;
> };
>
> +extern const struct pci_error_handlers vfio_err_handlers;
> +
> #define is_intx(vdev) (vdev->irq_type == VFIO_PCI_INTX_IRQ_INDEX)
> #define is_msi(vdev) (vdev->irq_type == VFIO_PCI_MSI_IRQ_INDEX)
> #define is_msix(vdev) (vdev->irq_type == VFIO_PCI_MSIX_IRQ_INDEX)
> @@ -154,6 +157,20 @@ extern long vfio_pci_ioeventfd(struct vfio_pci_device *vdev, loff_t offset,
> extern int vfio_pci_init_perm_bits(void);
> extern void vfio_pci_uninit_perm_bits(void);
>
> +int vfio_pci_open(void *device_data);
> +void vfio_pci_release(void *device_data);
> +long vfio_pci_ioctl(void *device_data,
> + unsigned int cmd, unsigned long arg);
> +ssize_t vfio_pci_rw(void *device_data, char __user *buf,
> + size_t count, loff_t *ppos, bool iswrite);
> +int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma);
> +void vfio_pci_request(void *device_data, unsigned int count);
> +int vfio_pci_reflck_attach(struct vfio_pci_device *vdev);
> +void vfio_pci_reflck_put(struct vfio_pci_reflck *reflck);
> +unsigned int vfio_pci_set_vga_decode(void *opaque, bool single_vga);
> +int vfio_pci_probe_misc(struct pci_dev *pdev, struct vfio_pci_device *vdev);
> +void vfio_pci_remove_misc(struct pci_dev *pdev);
> +
> extern int vfio_config_init(struct vfio_pci_device *vdev);
> extern void vfio_config_free(struct vfio_pci_device *vdev);
>
Powered by blists - more mailing lists