On Thu, Mar 25, 2010 at 10:34 AM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> On Thu, Mar 25, 2010 at 10:30:42AM -0600, Cam Macdonell wrote:
>> On Thu, Mar 25, 2010 at 3:05 AM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
>> > On Thu, Mar 25, 2010 at 12:09:36AM -0600, Cam Macdonell wrote:
>> >> This patch adds a driver for my shared memory PCI device using the uio_pci
>> >> interface. The driver has three memory regions. The first memory region is for
>> >> device registers for sending interrupts. The second BAR is for receiving MSI-X
>> >> interrupts and the third memory region maps the shared memory. The device only
>> >> exports the first and third memory regions to userspace.
>> >>
>> >> This driver supports MSI-X and regular pin interrupts. Currently, the number of
>> >> MSI vectors is set to 4 which could be increased, but the driver will work with
>> >> fewer vectors. If MSI is not available, then regular interrupts will be used.
>> >> ---
>> >> drivers/uio/Kconfig | 8 ++
>> >> drivers/uio/Makefile | 1 +
>> >> drivers/uio/uio_ivshmem.c | 235 +++++++++++++++++++++++++++++++++++++++++++++
>> >> 3 files changed, 244 insertions(+), 0 deletions(-)
>> >> create mode 100644 drivers/uio/uio_ivshmem.c
>> >>
>> >> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
>> >> index 1da73ec..b92cded 100644
>> >> --- a/drivers/uio/Kconfig
>> >> +++ b/drivers/uio/Kconfig
>> >> @@ -74,6 +74,14 @@ config UIO_SERCOS3
>> >>
>> >> If you compile this as a module, it will be called uio_sercos3.
>> >>
>> >> +config UIO_IVSHMEM
>> >> + tristate "KVM shared memory PCI driver"
>> >> + default n
>> >> + help
>> >> + Userspace I/O interface for the KVM shared memory device. This
>> >> + driver will make available two memory regions, the first is
>> >> + registers and the second is a region for sharing between VMs.
>> >> +
>> >> config UIO_PCI_GENERIC
>> >> tristate "Generic driver for PCI 2.3 and PCI Express cards"
>> >> depends on PCI
>> >> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
>> >> index 18fd818..25c1ca5 100644
>> >> --- a/drivers/uio/Makefile
>> >> +++ b/drivers/uio/Makefile
>> >> @@ -6,3 +6,4 @@ obj-$(CONFIG_UIO_AEC) += uio_aec.o
>> >> obj-$(CONFIG_UIO_SERCOS3) += uio_sercos3.o
>> >> obj-$(CONFIG_UIO_PCI_GENERIC) += uio_pci_generic.o
>> >> obj-$(CONFIG_UIO_NETX) += uio_netx.o
>> >> +obj-$(CONFIG_UIO_IVSHMEM) += uio_ivshmem.o
>> >> diff --git a/drivers/uio/uio_ivshmem.c b/drivers/uio/uio_ivshmem.c
>> >> new file mode 100644
>> >> index 0000000..607435b
>> >> --- /dev/null
>> >> +++ b/drivers/uio/uio_ivshmem.c
>> >> @@ -0,0 +1,235 @@
>> >> +/*
>> >> + * UIO IVShmem Driver
>> >> + *
>> >> + * (C) 2009 Cam Macdonell
>> >> + * based on Hilscher CIF card driver (C) 2007 Hans J. Koch <hjk@xxxxxxxxxxxxx>
>> >> + *
>> >> + * Licensed under GPL version 2 only.
>> >> + *
>> >> + */
>> >> +
>> >> +#include <linux/device.h>
>> >> +#include <linux/module.h>
>> >> +#include <linux/pci.h>
>> >> +#include <linux/uio_driver.h>
>> >> +
>> >> +#include <asm/io.h>
>> >> +
>> >> +#define IntrStatus 0x04
>> >> +#define IntrMask 0x00
>> >> +
>> >> +struct ivshmem_info {
>> >> + struct uio_info *uio;
>> >> + struct pci_dev *dev;
>> >> + char (*msix_names)[256];
>> >> + struct msix_entry *msix_entries;
>> >> + int nvectors;
>> >> +};
>> >> +
>> >> +static irqreturn_t ivshmem_handler(int irq, struct uio_info *dev_info)
>> >> +{
>> >> +
>> >> + void __iomem *plx_intscr = dev_info->mem[0].internal_addr
>> >> + + IntrStatus;
>> >> + u32 val;
>> >> +
>> >> + val = readl(plx_intscr);
>> >> + if (val == 0)
>> >> + return IRQ_NONE;
>> >> +
>> >> + printk(KERN_INFO "Regular interrupt (val = %d)\n", val);
>> >> + return IRQ_HANDLED;
>> >> +}
>> >> +
>> >> +static irqreturn_t ivshmem_msix_handler(int irq, void *opaque)
>> >> +{
>> >> +
>> >> + struct uio_info * dev_info = (struct uio_info *) opaque;
>> >> +
>> >> + /* we have to do this explicitly when using MSI-X */
>> >> + uio_event_notify(dev_info);
>> >
>> > How does userspace know which vector was triggered?
>>
>> Right now, it doesn't. If a user had a particular need they would
>> need to write their own uio driver. I guess this leads to a
>> discussion of MSI support in UIO and how they would work with the
>> userspace.
>
> So why request more than one vector then?
No strong reason, to some extent an artifact of my playing with uio
and MSI together. But, since interrupts on vector 0 are raised on
guest joins/exits, a user could modify the driver to ignore those if
they wanted. Also, this way the device doesn't need to know if the
guest is using 1 or 2 vectors.
>
>> >
>> >> + printk(KERN_INFO "MSI-X interrupt (%d)\n", irq);
>> >> + return IRQ_HANDLED;
>> >> +
>> >
>> > extra empty line
>> >
>> >> +}
>> >> +
>> >> +static int request_msix_vectors(struct ivshmem_info *ivs_info, int nvectors)
>> >> +{
>> >> + int i, err;
>> >> + const char *name = "ivshmem";
>> >> +
>> >> + printk(KERN_INFO "devname is %s\n", name);
>> >
>> > These KERN_INFO messages need to be cleaned up, they would be
>> > look confusing in the log.
>>
>> Agreed. I will clean most of these out.
>>
>> >
>> >> + ivs_info->nvectors = nvectors;
>> >> +
>> >> +
>> >> + ivs_info->msix_entries = kmalloc(nvectors * sizeof *ivs_info->msix_entries,
>> >> + GFP_KERNEL);
>> >> + ivs_info->msix_names = kmalloc(nvectors * sizeof *ivs_info->msix_names,
>> >> + GFP_KERNEL);
>> >> +
>> >
>> > need to handle errors
>> >
>> >> + for (i = 0; i < nvectors; ++i)
>> >> + ivs_info->msix_entries[i].entry = i;
>> >> +
>> >> + err = pci_enable_msix(ivs_info->dev, ivs_info->msix_entries,
>> >> + ivs_info->nvectors);
>> >> + if (err > 0) {
>> >> + ivs_info->nvectors = err; /* msi-x positive error code
>> >> + returns the number available*/
>> >> + err = pci_enable_msix(ivs_info->dev, ivs_info->msix_entries,
>> >> + ivs_info->nvectors);
>> >> + if (err > 0) {
>> >> + printk(KERN_INFO "no MSI (%d). Back to INTx.\n", err);
>> >> + return -ENOSPC;
>> >> + }
>> >> + }
>> >
>> > we can also get err < 0.
>> >
>> >> +
>> >> + printk(KERN_INFO "err is %d\n", err);
>> >> + if (err) return err;
>> >
>> > coding style rule violation
>> >
>> >> +
>> >> + for (i = 0; i < ivs_info->nvectors; i++) {
>> >> +
>> >> + snprintf(ivs_info->msix_names[i], sizeof *ivs_info->msix_names,
>> >> + "%s-config", name);
>> >> +
>> >> + ivs_info->msix_entries[i].entry = i;
>> >> + err = request_irq(ivs_info->msix_entries[i].vector,
>> >> + ivshmem_msix_handler, 0,
>> >> + ivs_info->msix_names[i], ivs_info->uio);
>> >> +
>> >> + if (err) {
>> >> + return -ENOSPC;
>> >
>> > coding style rule violation
>> > no undo on error handling
>> > why override error code with -ENOSPC?
>>
>> Ah, I think I've confused linux and qemu coding styles perhaps. I'll
>> fix these and others above.
>>
>> >
>> >> + }
>> >> + }
>> >> +
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static int __devinit ivshmem_pci_probe(struct pci_dev *dev,
>> >> + const struct pci_device_id *id)
>> >> +{
>> >> + struct uio_info *info;
>> >> + struct ivshmem_info * ivshmem_info;
>> >> + int nvectors = 4;
>> >> +
>> >> + info = kzalloc(sizeof(struct uio_info), GFP_KERNEL);
>> >> + if (!info)
>> >> + return -ENOMEM;
>> >> +
>> >> + ivshmem_info = kzalloc(sizeof(struct ivshmem_info), GFP_KERNEL);
>> >> + if (!ivshmem_info) {
>> >> + kfree(info);
>> >> + return -ENOMEM;
>> >> + }
>> >> +
>> >> + if (pci_enable_device(dev))
>> >> + goto out_free;
>> >> +
>> >> + if (pci_request_regions(dev, "ivshmem"))
>> >> + goto out_disable;
>> >> +
>> >> + info->mem[0].addr = pci_resource_start(dev, 0);
>> >> + if (!info->mem[0].addr)
>> >> + goto out_release;
>> >> +
>> >> + info->mem[0].size = pci_resource_len(dev, 0);
>> >> + info->mem[0].internal_addr = pci_ioremap_bar(dev, 0);
>> >> + if (!info->mem[0].internal_addr) {
>> >> + printk(KERN_INFO "got a null");
>> >> + goto out_release;
>> >> + }
>> >> +
>> >> + info->mem[0].memtype = UIO_MEM_PHYS;
>> >> +
>> >> + info->mem[1].addr = pci_resource_start(dev, 2);
>> >> + if (!info->mem[1].addr)
>> >> + goto out_unmap;
>> >> + info->mem[1].internal_addr = pci_ioremap_bar(dev, 2);
>> >> + if (!info->mem[1].internal_addr)
>> >> + goto out_unmap;
>> >> +
>> >> + info->mem[1].size = pci_resource_len(dev, 2);
>> >> + info->mem[1].memtype = UIO_MEM_PHYS;
>> >> +
>> >> + ivshmem_info->uio = info;
>> >> + ivshmem_info->dev = dev;
>> >> +
>> >> + if (request_msix_vectors(ivshmem_info, nvectors) != 0) {
>> >> + printk(KERN_INFO "regular IRQs\n");
>> >> + info->irq = dev->irq;
>> >> + info->irq_flags = IRQF_SHARED;
>> >> + info->handler = ivshmem_handler;
>> >> + writel(0xffffffff, info->mem[0].internal_addr + IntrMask);
>> >> + } else {
>> >> + printk(KERN_INFO "MSI-X enabled\n");
>> >> + info->irq = -1;
>> >> + }
>> >> +
>> >> + info->name = "ivshmem";
>> >> + info->version = "0.0.1";
>> >> +
>> >> + if (uio_register_device(&dev->dev, info))
>> >> + goto out_unmap2;
>> >> +
>> >> + pci_set_drvdata(dev, info);
>> >> +
>> >> +
>> >> + return 0;
>> >> +out_unmap2:
>> >> + iounmap(info->mem[2].internal_addr);
>> >> +out_unmap:
>> >> + iounmap(info->mem[0].internal_addr);
>> >> +out_release:
>> >> + pci_release_regions(dev);
>> >> +out_disable:
>> >> + pci_disable_device(dev);
>> >> +out_free:
>> >> + kfree (info);
>> >> + return -ENODEV;
>> >> +}
>> >> +
>> >> +static void ivshmem_pci_remove(struct pci_dev *dev)
>> >> +{
>> >> + struct uio_info *info = pci_get_drvdata(dev);
>> >> +
>> >> + uio_unregister_device(info);
>> >> + pci_release_regions(dev);
>> >> + pci_disable_device(dev);
>> >> + pci_set_drvdata(dev, NULL);
>> >> + iounmap(info->mem[0].internal_addr);
>> >> +
>> >> + kfree (info);
>> >> +}
>> >> +
>> >> +static struct pci_device_id ivshmem_pci_ids[] __devinitdata = {
>> >> + {
>> >> + .vendor = 0x1af4,
>> >> + .device = 0x1110,
>> >
>> > vendor ids must be registered with PCI SIG.
>> > this one does not seem to be registered.
>> >
>> >> + .subvendor = PCI_ANY_ID,
>> >> + .subdevice = PCI_ANY_ID,
>> >> + },
>> >> + { 0, }
>> >> +};
>> >> +
>> >> +static struct pci_driver ivshmem_pci_driver = {
>> >> + .name = "uio_ivshmem",
>> >> + .id_table = ivshmem_pci_ids,
>> >> + .probe = ivshmem_pci_probe,
>> >> + .remove = ivshmem_pci_remove,
>> >> +};
>> >> +
>> >> +static int __init ivshmem_init_module(void)
>> >> +{
>> >> + return pci_register_driver(&ivshmem_pci_driver);
>> >> +}
>> >> +
>> >> +static void __exit ivshmem_exit_module(void)
>> >> +{
>> >> + pci_unregister_driver(&ivshmem_pci_driver);
>> >> +}
>> >> +
>> >> +module_init(ivshmem_init_module);
>> >> +module_exit(ivshmem_exit_module);
>> >> +
>> >> +MODULE_DEVICE_TABLE(pci, ivshmem_pci_ids);
>> >> +MODULE_LICENSE("GPL v2");
>> >> +MODULE_AUTHOR("Cam Macdonell");
>> >> --
>> >> 1.6.6.1
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >
>> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
