Linux virtualization list
 help / color / mirror / Atom feed
* Problem with multiple hvc consoles via virtio-console
From: Amit Shah @ 2010-03-24 15:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Virtualization List

Hello,

When multiple hvc console ports are initialised and used via the
virtio_console driver, I can interact with only the first console, the
2nd one doesn't respond.

If I call hvc_kick() even if hvc_poll() returns 0, the 2nd console
becomes responsive.

I've checked that hvc_poll() receives data via get_chars() and feeds it
to tty_insert_flip_char().

Any idea what could be going wrong? This is reproducible on Linus' git
kernel as the guest kernel with the qemu.git repo.

Any pointers?

Thanks,
		Amit

^ permalink raw reply

* Re: [RFC] vhost-blk implementation
From: Badari Pulavarty @ 2010-03-24 17:58 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, virtualization
In-Reply-To: <20100324112336.GA8180@redhat.com>

Michael S. Tsirkin wrote:
> On Tue, Mar 23, 2010 at 12:55:07PM -0700, Badari Pulavarty wrote:
>   
>> Michael S. Tsirkin wrote:
>>     
>>> On Tue, Mar 23, 2010 at 10:57:33AM -0700, Badari Pulavarty wrote:
>>>   
>>>       
>>>> Michael S. Tsirkin wrote:
>>>>     
>>>>         
>>>>> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>>>>>         
>>>>>           
>>>>>> Write Results:
>>>>>> ==============
>>>>>>
>>>>>> I see degraded IO performance when doing sequential IO write
>>>>>> tests with vhost-blk compared to virtio-blk.
>>>>>>
>>>>>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>>>>>
>>>>>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>>>>>> vhost-blk. Wondering why ?
>>>>>>             
>>>>>>             
>>>>> Try to look and number of interrupts and/or number of exits.
>>>>>         
>>>>>           
>>>> I checked interrupts and IO exits - there is no major noticeable   
>>>> difference between
>>>> vhost-blk and virtio-blk scenerios.
>>>>     
>>>>         
>>>>> It could also be that you are overrunning some queue.
>>>>>
>>>>> I don't see any exit mitigation strategy in your patch:
>>>>> when there are already lots of requests in a queue, it's usually
>>>>> a good idea to disable notifications and poll the
>>>>> queue as requests complete. That could help performance.
>>>>>         
>>>>>           
>>>> Do you mean poll eventfd for new requests instead of waiting for new  
>>>> notifications ?
>>>> Where do you do that in vhost-net code ?
>>>>     
>>>>         
>>> vhost_disable_notify does this.
>>>
>>>   
>>>       
>>>> Unlike network socket, since we are dealing with a file, there is no  
>>>> ->poll support for it.
>>>> So I can't poll for the data. And also, Issue I am having is on the   
>>>> write() side.
>>>>     
>>>>         
>>> Not sure I understand.
>>>
>>>   
>>>       
>>>> I looked at it some more - I see 512K write requests on the
>>>> virtio-queue  in both vhost-blk and virtio-blk cases. Both qemu or
>>>> vhost is doing synchronous  writes to page cache (there is no write
>>>> batching in qemu that is affecting this  case).  I still puzzled on
>>>> why virtio-blk outperforms vhost-blk.
>>>>
>>>> Thanks,
>>>> Badari
>>>>     
>>>>         
>>> If you say the number of requests is the same, we are left with:
>>> - requests are smaller for some reason?
>>> - something is causing retries?
>>>   
>>>       
>> No. IO requests sizes are exactly same (512K) in both cases. There are  
>> no retries or
>> errors in both cases. One thing I am not clear is - for some reason  
>> guest kernel
>> could push more data into virtio-ring in case of virtio-blk vs  
>> vhost-blk. Is this possible ?
>> Does guest gets to run much sooner in virtio-blk case than vhost-blk ?  
>> Sorry, if its dumb question -
>> I don't understand  all the vhost details :(
>>
>> Thanks,
>> Badari
>>     
>
> BTW, did you put the backend in non-blocking mode?
> As I said, vhost net passes non-blocking flag to
> socket backend, but vfs_write/read that you use does
> not have an option to do this.
>
> So we'll need to extend the backend to fix that,
> but just for demo purposes, you could set non-blocking
> mode on the file from userspace.
>
>   
Michael,

Atleast I understand why the performance difference now.. My debug
code is changed the behaviour of virtio-blk which confused me.

1) virtio-blk is able to batch up writes from various requests.
2) virtio-blk offloads the writes to different thread

Where as vhost-blk, I do each request syncrhonously. Hence
the difference. You are right - i have to make backend async.
I will working on handing off work to in-kernel threads.
I am not sure about IO completion handling and calling
vhost_add_used_and_signal() out of context. But let
me take a stab at it and ask your help if I run into
issues.

Thanks,
Badari

^ permalink raw reply

* Re: [RFC] vhost-blk implementation
From: Michael S. Tsirkin @ 2010-03-24 18:28 UTC (permalink / raw)
  To: Badari Pulavarty; +Cc: qemu-devel, virtualization
In-Reply-To: <4BAA52DA.2020204@us.ibm.com>

On Wed, Mar 24, 2010 at 10:58:50AM -0700, Badari Pulavarty wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Mar 23, 2010 at 12:55:07PM -0700, Badari Pulavarty wrote:
>>   
>>> Michael S. Tsirkin wrote:
>>>     
>>>> On Tue, Mar 23, 2010 at 10:57:33AM -0700, Badari Pulavarty wrote:
>>>>         
>>>>> Michael S. Tsirkin wrote:
>>>>>             
>>>>>> On Mon, Mar 22, 2010 at 05:34:04PM -0700, Badari Pulavarty wrote:
>>>>>>                   
>>>>>>> Write Results:
>>>>>>> ==============
>>>>>>>
>>>>>>> I see degraded IO performance when doing sequential IO write
>>>>>>> tests with vhost-blk compared to virtio-blk.
>>>>>>>
>>>>>>> # time dd of=/dev/vda if=/dev/zero bs=2M oflag=direct
>>>>>>>
>>>>>>> I get ~110MB/sec with virtio-blk, but I get only ~60MB/sec with
>>>>>>> vhost-blk. Wondering why ?
>>>>>>>                         
>>>>>> Try to look and number of interrupts and/or number of exits.
>>>>>>                   
>>>>> I checked interrupts and IO exits - there is no major noticeable  
>>>>>  difference between
>>>>> vhost-blk and virtio-blk scenerios.
>>>>>             
>>>>>> It could also be that you are overrunning some queue.
>>>>>>
>>>>>> I don't see any exit mitigation strategy in your patch:
>>>>>> when there are already lots of requests in a queue, it's usually
>>>>>> a good idea to disable notifications and poll the
>>>>>> queue as requests complete. That could help performance.
>>>>>>                   
>>>>> Do you mean poll eventfd for new requests instead of waiting for 
>>>>> new  notifications ?
>>>>> Where do you do that in vhost-net code ?
>>>>>             
>>>> vhost_disable_notify does this.
>>>>
>>>>         
>>>>> Unlike network socket, since we are dealing with a file, there is 
>>>>> no  ->poll support for it.
>>>>> So I can't poll for the data. And also, Issue I am having is on 
>>>>> the   write() side.
>>>>>             
>>>> Not sure I understand.
>>>>
>>>>         
>>>>> I looked at it some more - I see 512K write requests on the
>>>>> virtio-queue  in both vhost-blk and virtio-blk cases. Both qemu or
>>>>> vhost is doing synchronous  writes to page cache (there is no write
>>>>> batching in qemu that is affecting this  case).  I still puzzled on
>>>>> why virtio-blk outperforms vhost-blk.
>>>>>
>>>>> Thanks,
>>>>> Badari
>>>>>             
>>>> If you say the number of requests is the same, we are left with:
>>>> - requests are smaller for some reason?
>>>> - something is causing retries?
>>>>         
>>> No. IO requests sizes are exactly same (512K) in both cases. There 
>>> are  no retries or
>>> errors in both cases. One thing I am not clear is - for some reason   
>>> guest kernel
>>> could push more data into virtio-ring in case of virtio-blk vs   
>>> vhost-blk. Is this possible ?
>>> Does guest gets to run much sooner in virtio-blk case than vhost-blk 
>>> ?  Sorry, if its dumb question -
>>> I don't understand  all the vhost details :(
>>>
>>> Thanks,
>>> Badari
>>>     
>>
>> BTW, did you put the backend in non-blocking mode?
>> As I said, vhost net passes non-blocking flag to
>> socket backend, but vfs_write/read that you use does
>> not have an option to do this.
>>
>> So we'll need to extend the backend to fix that,
>> but just for demo purposes, you could set non-blocking
>> mode on the file from userspace.
>>
>>   
> Michael,
>
> Atleast I understand why the performance difference now.. My debug
> code is changed the behaviour of virtio-blk which confused me.
>
> 1) virtio-blk is able to batch up writes from various requests.
> 2) virtio-blk offloads the writes to different thread
>
> Where as vhost-blk, I do each request syncrhonously. Hence
> the difference. You are right - i have to make backend async.
> I will working on handing off work to in-kernel threads.
> I am not sure about IO completion handling and calling
> vhost_add_used_and_signal() out of context. But let
> me take a stab at it and ask your help if I run into
> issues.
>
> Thanks,
> Badari
>


The way I did it for vhost net, requests are synchronous
but non-blocking. So if it can't be done directly,
I delay it.

-- 
MST

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Michael S. Tsirkin @ 2010-03-25  9:15 UTC (permalink / raw)
  To: Cam Macdonell; +Cc: qemu-devel, kvm, virtualization
In-Reply-To: <1269497376-21903-1-git-send-email-cam@cs.ualberta.ca>

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.

Some high level questions, sorry if they have been raised in the past:
- Can this device use virtio framework?
  This gives us some standards to work off, with feature negotiation,
  ability to detect config changes, support for non-PCI
  platforms, decent documentation that is easy to extend,
  legal id range to use.
  You would thus have your driver in uio/uio_virtio_shmem.c

- Why are you using 32 bit long memory accesses for interrupts?
  16 bit IO eould be enough and it's faster. This what virtio-pci does.

- How was the driver tested?

> ---
>  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@linutronix.de>
> + *
> + * 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);
> +    printk(KERN_INFO "MSI-X interrupt (%d)\n", irq);
> +    return IRQ_HANDLED;
> +
> +}
> +
> +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);
> +    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);
> +
> +    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;
> +        }
> +    }
> +
> +    printk(KERN_INFO "err is %d\n", err);
> +    if (err) return err;
> +
> +    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;
> +        }
> +    }
> +
> +    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,
> +        .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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Avi Kivity @ 2010-03-25  9:40 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, kvm, virtualization
In-Reply-To: <20100325091552.GB11153@redhat.com>

On 03/25/2010 11:15 AM, Michael S. Tsirkin wrote:
>
> - Why are you using 32 bit long memory accesses for interrupts?
>    16 bit IO eould be enough and it's faster. This what virtio-pci does.
>
>    

Why is 16 bit io faster?

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Michael S. Tsirkin @ 2010-03-25  9:44 UTC (permalink / raw)
  To: Avi Kivity; +Cc: qemu-devel, kvm, virtualization
In-Reply-To: <4BAB2F79.7030706@redhat.com>

On Thu, Mar 25, 2010 at 11:40:09AM +0200, Avi Kivity wrote:
> On 03/25/2010 11:15 AM, Michael S. Tsirkin wrote:
>>
>> - Why are you using 32 bit long memory accesses for interrupts?
>>    16 bit IO eould be enough and it's faster. This what virtio-pci does.
>>
>>    
>
> Why is 16 bit io faster?

Something to do with need for emulation to get address/data
for pci memory accesses?

> -- 
> error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Avi Kivity @ 2010-03-25  9:58 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, kvm, virtualization
In-Reply-To: <20100325094448.GA15306@redhat.com>

On 03/25/2010 11:44 AM, Michael S. Tsirkin wrote:
> On Thu, Mar 25, 2010 at 11:40:09AM +0200, Avi Kivity wrote:
>    
>> On 03/25/2010 11:15 AM, Michael S. Tsirkin wrote:
>>      
>>> - Why are you using 32 bit long memory accesses for interrupts?
>>>     16 bit IO eould be enough and it's faster. This what virtio-pci does.
>>>
>>>
>>>        
>> Why is 16 bit io faster?
>>      
> Something to do with need for emulation to get address/data
> for pci memory accesses?
>    

pio is definitely faster than mmio (all that is needed is to set one bit 
on the BAR).  But 32 vs. 16 makes no difference.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Michael S. Tsirkin @ 2010-03-25 10:07 UTC (permalink / raw)
  To: Avi Kivity; +Cc: qemu-devel, kvm, virtualization
In-Reply-To: <4BAB33C6.3060504@redhat.com>

On Thu, Mar 25, 2010 at 11:58:30AM +0200, Avi Kivity wrote:
> On 03/25/2010 11:44 AM, Michael S. Tsirkin wrote:
>> On Thu, Mar 25, 2010 at 11:40:09AM +0200, Avi Kivity wrote:
>>    
>>> On 03/25/2010 11:15 AM, Michael S. Tsirkin wrote:
>>>      
>>>> - Why are you using 32 bit long memory accesses for interrupts?
>>>>     16 bit IO eould be enough and it's faster. This what virtio-pci does.
>>>>
>>>>
>>>>        
>>> Why is 16 bit io faster?
>>>      
>> Something to do with need for emulation to get address/data
>> for pci memory accesses?
>>    
>
> pio is definitely faster than mmio (all that is needed is to set one bit  
> on the BAR).  But 32 vs. 16 makes no difference.

Right. That's what I meant.

> -- 
> error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Cam Macdonell @ 2010-03-25 16:18 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, kvm, virtualization
In-Reply-To: <20100325091552.GB11153@redhat.com>

On Thu, Mar 25, 2010 at 3:15 AM, Michael S. Tsirkin <mst@redhat.com> 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.
>
> Some high level questions, sorry if they have been raised in the past:
> - Can this device use virtio framework?
>  This gives us some standards to work off, with feature negotiation,
>  ability to detect config changes, support for non-PCI
>  platforms, decent documentation that is easy to extend,
>  legal id range to use.
>  You would thus have your driver in uio/uio_virtio_shmem.c

There has been previous discussion of virtio, however while virtio is
good for exporting guest memory, it's not ideal for importing memory
into a guest.

>
> - Why are you using 32 bit long memory accesses for interrupts?
>  16 bit IO eould be enough and it's faster. This what virtio-pci does.
>
> - How was the driver tested?

I have some test programs in the git repo I linked to.  I've been
using some simple producer/consumer tests to test the interrupt
framework.

>
>> ---
>>  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@linutronix.de>
>> + *
>> + * 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);
>> +    printk(KERN_INFO "MSI-X interrupt (%d)\n", irq);
>> +    return IRQ_HANDLED;
>> +
>> +}
>> +
>> +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);
>> +    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);
>> +
>> +    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;
>> +        }
>> +    }
>> +
>> +    printk(KERN_INFO "err is %d\n", err);
>> +    if (err) return err;
>> +
>> +    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;
>> +        }
>> +    }
>> +
>> +    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,
>> +        .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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Anthony Liguori @ 2010-03-25 16:23 UTC (permalink / raw)
  To: Cam Macdonell; +Cc: virtualization, qemu-devel, kvm, Michael S. Tsirkin
In-Reply-To: <8286e4ee1003250918x659e87ai9e5ca7c68c23a7c3@mail.gmail.com>

On 03/25/2010 11:18 AM, Cam Macdonell wrote:
> On Thu, Mar 25, 2010 at 3:15 AM, Michael S. Tsirkin<mst@redhat.com>  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.
>>>        
>> Some high level questions, sorry if they have been raised in the past:
>> - Can this device use virtio framework?
>>   This gives us some standards to work off, with feature negotiation,
>>   ability to detect config changes, support for non-PCI
>>   platforms, decent documentation that is easy to extend,
>>   legal id range to use.
>>   You would thus have your driver in uio/uio_virtio_shmem.c
>>      
> There has been previous discussion of virtio, however while virtio is
> good for exporting guest memory, it's not ideal for importing memory
> into a guest.
>    

virtio is a DMA-based API which means that it doesn't assume cache 
coherent shared memory.  The PCI transport takes advantage of cache 
coherent shared memory but it's not strictly required.

Memory sharing in virtio would be a layering violation because it forces 
cache coherent shared memory for all virtio transports.

Regards,

Anthony Liguori

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Avi Kivity @ 2010-03-25 16:32 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: virtualization, qemu-devel, kvm, Michael S. Tsirkin
In-Reply-To: <4BAB8DE9.9080603@codemonkey.ws>

On 03/25/2010 06:23 PM, Anthony Liguori wrote:
>> There has been previous discussion of virtio, however while virtio is
>> good for exporting guest memory, it's not ideal for importing memory
>> into a guest.
>
> virtio is a DMA-based API which means that it doesn't assume cache 
> coherent shared memory.  The PCI transport takes advantage of cache 
> coherent shared memory but it's not strictly required.

Aren't we violating this by not using dma_alloc_coherent() for the queues?

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Michael S. Tsirkin @ 2010-03-25 16:33 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel, kvm, virtualization
In-Reply-To: <4BAB8DE9.9080603@codemonkey.ws>

On Thu, Mar 25, 2010 at 11:23:05AM -0500, Anthony Liguori wrote:
> On 03/25/2010 11:18 AM, Cam Macdonell wrote:
>> On Thu, Mar 25, 2010 at 3:15 AM, Michael S. Tsirkin<mst@redhat.com>  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.
>>>>        
>>> Some high level questions, sorry if they have been raised in the past:
>>> - Can this device use virtio framework?
>>>   This gives us some standards to work off, with feature negotiation,
>>>   ability to detect config changes, support for non-PCI
>>>   platforms, decent documentation that is easy to extend,
>>>   legal id range to use.
>>>   You would thus have your driver in uio/uio_virtio_shmem.c
>>>      
>> There has been previous discussion of virtio, however while virtio is
>> good for exporting guest memory, it's not ideal for importing memory
>> into a guest.
>>    
>
> virtio is a DMA-based API which means that it doesn't assume cache  
> coherent shared memory.  The PCI transport takes advantage of cache  
> coherent shared memory but it's not strictly required.
>
> Memory sharing in virtio would be a layering violation because it forces  
> cache coherent shared memory for all virtio transports.
>
> Regards,
>
> Anthony Liguori

I am not sure I understand the argument.  We can still reuse feature
negotiation, notifications etc from virtio.  If some transports can not
support cache coherent shared memory, the device won't work there.
But it won't work there anyway, even without using virtio.
We are not forcing all devices to assume cache coherency.

In other words, let's not fall into midlayer mistake, let's
treat virtio as a library.

-- 
MST

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Michael S. Tsirkin @ 2010-03-25 16:40 UTC (permalink / raw)
  To: Avi Kivity; +Cc: virtualization, kvm, Anthony Liguori, qemu-devel
In-Reply-To: <4BAB900F.5030104@redhat.com>

On Thu, Mar 25, 2010 at 06:32:15PM +0200, Avi Kivity wrote:
> On 03/25/2010 06:23 PM, Anthony Liguori wrote:
>>> There has been previous discussion of virtio, however while virtio is
>>> good for exporting guest memory, it's not ideal for importing memory
>>> into a guest.
>>
>> virtio is a DMA-based API which means that it doesn't assume cache  
>> coherent shared memory.  The PCI transport takes advantage of cache  
>> coherent shared memory but it's not strictly required.
>
> Aren't we violating this by not using dma_alloc_coherent() for the queues?

I don't see what changing this would buys us though, unless
a non-cache coherent architecture implements kvm.
TCG runs everything on a single processor so there are
no cache coherency issues.

> -- 
> error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH v3 1/1] Shared memory uio_pci driver
From: Avi Kivity @ 2010-03-25 16:50 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtualization, kvm, Anthony Liguori, qemu-devel
In-Reply-To: <20100325164052.GB16506@redhat.com>

On 03/25/2010 06:40 PM, Michael S. Tsirkin wrote:
> On Thu, Mar 25, 2010 at 06:32:15PM +0200, Avi Kivity wrote:
>    
>> On 03/25/2010 06:23 PM, Anthony Liguori wrote:
>>      
>>>> There has been previous discussion of virtio, however while virtio is
>>>> good for exporting guest memory, it's not ideal for importing memory
>>>> into a guest.
>>>>          
>>> virtio is a DMA-based API which means that it doesn't assume cache
>>> coherent shared memory.  The PCI transport takes advantage of cache
>>> coherent shared memory but it's not strictly required.
>>>        
>> Aren't we violating this by not using dma_alloc_coherent() for the queues?
>>      
> I don't see what changing this would buys us though, unless
> a non-cache coherent architecture implements kvm.
>    

We're preventing people from implementing virtio devices in hardware, 
not that anyone's doing that.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: Problem with multiple hvc consoles via virtio-console
From: Amit Shah @ 2010-03-26  5:01 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-kernel, Virtualization List
In-Reply-To: <20100324153408.GB18581@amit-x200.redhat.com>

On (Wed) Mar 24 2010 [21:04:08], Amit Shah wrote:
> Hello,
> 
> When multiple hvc console ports are initialised and used via the
> virtio_console driver, I can interact with only the first console, the
> 2nd one doesn't respond.
> 
> If I call hvc_kick() even if hvc_poll() returns 0, the 2nd console
> becomes responsive.

Actually it's not that fancy: if I login to the first console (hvc0) and
then spawn another one (hvc1), I can't login into hvc1. If I spawn hvc1
before logging into hvc0, I can work with both.

		Amit

^ permalink raw reply

* [RFC] GSoC 2010: Memory Compression for Virtualized Environments
From: Nitin Gupta @ 2010-03-27  7:38 UTC (permalink / raw)
  To: virtualization; +Cc: KVM development list

Hi,

I will be applying to GSoC 2010 under The Linux Foundation as mentoring
organization (Virtualization working group). Below is the application for my
planned project: "Memory Compression for Virtualized Environments"
(according to LF template). I would be thankful for any comments/feedback.

* Name

Nitin Gupta

* University / current enrollment

University of Massachusetts Amherst

* Short bio / overview of your background

I'm currently enrolled in MS (computer science) program at UMass Amherst and
have 3+ years of experience fixing memory related issues in some proprietary
kernel. I have also made contributions to the Linux kernel and Xen.

* Subscribe to the mailing list of the appropriate group and introduce yourself

Subscribed to: virtualization at lists dot linux-foundation.org

* Tell us your IRC nick with which you will use the group's IRC channel

IRC nick: ngupta (irc.oftc.net #virt)

* What platform do you use to code? Hardware specifications and OS

Linux kernel development on x86 and x86_x64.

* Did you ever code in C or C++/Perl/python/..., yes/no? what is your
experience?

Excellent C skills - programming in C since 5+ years (as hobby and
professionally).

* If you apply for a project on our ideas list, have you experience in the
areas listed under "Desired knowledge"?

This is not in ideas list but have worked extensively in all areas related to
this project.

* Were you involved in development in the project's group in the past?
What was your contribution?

I have made contributions to the Linux kernel in general:
 - Ported LZO de/compressor to kernel.
 - Developed in-memory compressed swap device (compcache/ramzswap) over a
period of 3 years. This includes a memory allocator called xvmalloc developed
from scratch. It is now included in mainline as a staging driver and is
already part of Ubuntu and (unofficial) builds of Google Android:
http://code.google.com/p/compcache/
 - Fixed d-cache aliasing problem on ARM. Same problem found and fixed on MIPS
and Sparc64.
 - Pointed out of-by-one error in swapon syscall implementation. Fixed by
Hugh Dickins in 2.6.33.
 - Implemented experimental patch for CIFS VFS implementation on kernel 2.6.8
to send multiple read requests in parallel (http://linux-mm.org/NitinGupta)

 * Were you involved in other OpenSource development projects in the past?
which, when and in what role?

 - Ported to kernel and extended TLSF allocator
allocator (http://rtportal.upv.es/rtmalloc/) with support for multiple
memory pools. This has replaced Xen's default xmalloc allocator.
 - Currently developing small IDE especially suited for large C based projects
like the Linux kernel: http://code.google.com/p/kxref/ (low priority)

 * Why have you chosen your development idea and what do you expect from
your implementation?

I have been working on the idea of memory compression for about 3 years (part
time) resulting in development of ramzswap driver which provides in-memory
compressed swap devices. This approch simplified the development however it
has some serious disadvantages:
 - It cannot compress page-cache pages
 - It incurs block I/O layer overhead
 - It requires curious hooks in block layer to function properly:
http://lkml.org/lkml/2010/1/4/534 which were later Nacked by Linus.
 - The approach makes it difficult to implement dynamic cache resizing (though
you can dynamically add/remove ramzswap devices of arbitrary size).

So, this GSoC project aims to provide a new approach for achieving memory
compression that solves all above issues: cleanly hook into reclaim path
directly, providing both swap and pagecache compression, avoiding all block I/O
overhead.

Project motivation, design and implementation details are present in this
document:
www.scribd.com/doc/28713197/Memory-Compression-for-Virtualized-Environments


Your Project

    * What do you want to achieve?

Achieve higher consolidation ratios (more VMs per host) for KVM platform using
memory compression technique at host level. This technique has already been
proven to be useful on desktop and embedded devices. I believe it has great
potential on servers too as virtualization becomes ubiquitous.

    * If you have chosen an idea from our list, why did you choose this
specific idea?

Idea not in list.

    * If you are proposing a project of your own, what is unique about it?

Please see details above.

    * What makes you suited to carry the project?

Several years of experience in Linux kernel development as reflected by
previous and on-going projects.

    * How much time do you plan to invest in the project before, during and
after the Summer of Code (We expect full time 40h/week during GSoC, but better
make this explicit)?

I will be working full-time on this project and plan to continue working till
it gets merged into mainline (and then continue as maintainer)

    * Please provide a schedule of how this time will be spent on subtasks of
the project. While this is only preliminary, you will be required to provide a
detailed plan latest at the beginning of GSoC and during the project you will
issue weekly progress reports against that plan.

Timeline: May to Aug (4 months)

Month 1: Discuss new design with mentor, prototype and finalize on approach
Month 2: Implement swap cache compression
Month 3: Implement page cache compression
Month 4: Dynamic cache resizing


(draft) Project motivation, design and implementation details:
www.scribd.com/doc/28713197/Memory-Compression-for-Virtualized-Environments


Thanks,
Nitin

^ permalink raw reply

* Re: [RFC] GSoC 2010: Memory Compression for Virtualized Environments
From: Alexander Graf @ 2010-03-27  9:01 UTC (permalink / raw)
  To: ngupta; +Cc: KVM development list, virtualization
In-Reply-To: <4BADB612.4010408@vflare.org>


On 27.03.2010, at 08:38, Nitin Gupta wrote:

> Hi,
> 
> I will be applying to GSoC 2010 under The Linux Foundation as mentoring
> organization (Virtualization working group). Below is the application for my
> planned project: "Memory Compression for Virtualized Environments"
> (according to LF template). I would be thankful for any comments/feedback.
> 
> * Name
> 
> Nitin Gupta
> 
> * University / current enrollment
> 
> University of Massachusetts Amherst
> 
> * Short bio / overview of your background
> 
> I'm currently enrolled in MS (computer science) program at UMass Amherst and
> have 3+ years of experience fixing memory related issues in some proprietary
> kernel. I have also made contributions to the Linux kernel and Xen.
> 
> * Subscribe to the mailing list of the appropriate group and introduce yourself
> 
> Subscribed to: virtualization at lists dot linux-foundation.org
> 
> * Tell us your IRC nick with which you will use the group's IRC channel
> 
> IRC nick: ngupta (irc.oftc.net #virt)
> 
> * What platform do you use to code? Hardware specifications and OS
> 
> Linux kernel development on x86 and x86_x64.
> 
> * Did you ever code in C or C++/Perl/python/..., yes/no? what is your
> experience?
> 
> Excellent C skills - programming in C since 5+ years (as hobby and
> professionally).

s/since/for/

> 
> * If you apply for a project on our ideas list, have you experience in the
> areas listed under "Desired knowledge"?
> 
> This is not in ideas list but have worked extensively in all areas related to
> this project.
> 
> * Were you involved in development in the project's group in the past?
> What was your contribution?
> 
> I have made contributions to the Linux kernel in general:
> - Ported LZO de/compressor to kernel.
> - Developed in-memory compressed swap device (compcache/ramzswap) over a
> period of 3 years. This includes a memory allocator called xvmalloc developed
> from scratch. It is now included in mainline as a staging driver and is
> already part of Ubuntu and (unofficial) builds of Google Android:
> http://code.google.com/p/compcache/
> - Fixed d-cache aliasing problem on ARM. Same problem found and fixed on MIPS
> and Sparc64.
> - Pointed out of-by-one error in swapon syscall implementation. Fixed by
> Hugh Dickins in 2.6.33.
> - Implemented experimental patch for CIFS VFS implementation on kernel 2.6.8
> to send multiple read requests in parallel (http://linux-mm.org/NitinGupta)
> 
> * Were you involved in other OpenSource development projects in the past?
> which, when and in what role?
> 
> - Ported to kernel and extended TLSF allocator
> allocator (http://rtportal.upv.es/rtmalloc/) with support for multiple
> memory pools. This has replaced Xen's default xmalloc allocator.
> - Currently developing small IDE especially suited for large C based projects
> like the Linux kernel: http://code.google.com/p/kxref/ (low priority)
> 
> * Why have you chosen your development idea and what do you expect from
> your implementation?
> 
> I have been working on the idea of memory compression for about 3 years (part
> time) resulting in development of ramzswap driver which provides in-memory
> compressed swap devices. This approch simplified the development however it
> has some serious disadvantages:
> - It cannot compress page-cache pages
> - It incurs block I/O layer overhead
> - It requires curious hooks in block layer to function properly:
> http://lkml.org/lkml/2010/1/4/534 which were later Nacked by Linus.
> - The approach makes it difficult to implement dynamic cache resizing (though
> you can dynamically add/remove ramzswap devices of arbitrary size).
> 
> So, this GSoC project aims to provide a new approach for achieving memory
> compression that solves all above issues: cleanly hook into reclaim path
> directly, providing both swap and pagecache compression, avoiding all block I/O
> overhead.
> 
> Project motivation, design and implementation details are present in this
> document:
> www.scribd.com/doc/28713197/Memory-Compression-for-Virtualized-Environments

Very interesting project.

I'm not 100% sure it's a good idea to waste CPU time on page cache compression, but then again I guess with 64-core systems coming up CPU power is a lot cheaper than I/O. You should definitely keep NUMA in mind while doing this though. The target systems for this certainly aren't single node systems ;-).

Another thing that I realized while reading through this is that I'm missing the virtualization link. You do explain it in the introduction, but I certainly fail to see why this should be limited to virtualization. It'd improve swapping penalty in general.

Either way, I'm eager to see this get accepted :-).


Alex

^ permalink raw reply

* Re: [RFC] GSoC 2010: Memory Compression for Virtualized Environments
From: Nitin Gupta @ 2010-03-27  9:47 UTC (permalink / raw)
  To: Alexander Graf; +Cc: KVM development list, virtualization
In-Reply-To: <9583F974-0EA0-40E2-9285-C04ECA395FC1@suse.de>

On 03/27/2010 02:31 PM, Alexander Graf wrote:
> 
> On 27.03.2010, at 08:38, Nitin Gupta wrote:
> 
<snip>

>> So, this GSoC project aims to provide a new approach for achieving memory
>> compression that solves all above issues: cleanly hook into reclaim path
>> directly, providing both swap and pagecache compression, avoiding all block I/O
>> overhead.
>>
>> Project motivation, design and implementation details are present in this
>> document:
>> www.scribd.com/doc/28713197/Memory-Compression-for-Virtualized-Environments
> 
> Very interesting project.
> 
> I'm not 100% sure it's a good idea to waste CPU time on page cache compression, but then again I guess with 64-core systems coming up CPU power is a lot cheaper than I/O. You should definitely keep NUMA in mind while doing this though. The target systems for this certainly aren't single node systems ;-).
> 

The target is certainly large scale systems. For 64-bit etc. crazy machines it might
be worth to even dedicate 1 or 2 cores for de/compression (though not necessary).
Even for desktops, 4 cores are now so common.

Then comes embedded, where other issues also come into picture -- slow random writes
on flash cards, wear-leveling issues, power to de/compress vs writing to flash etc.

I highlighted virtualization since large scale systems (and virtualization workload) are
easily justifiable for this feature :)

> Another thing that I realized while reading through this is that I'm missing the virtualization link. You do explain it in the introduction, but I certainly fail to see why this should be limited to virtualization. It'd improve swapping penalty in general.
> 

It is not limited to virtualization case and it does improve I/O penalty in non-virtualized case too
(as shown by compcache work). I highlighted virtualization only because its looks like an important
use case and benefits of memory compression are yet to be explored in this area.


On a side note: another project proposal I'm going to submit is:
 "Virtual Co-processor: Flexible and Scalable Virtual Machines"
you can see draft of the paper here:
http://www.scribd.com/doc/23978468/Virtual-Co-processors-Flexible-and-Scalable-Virtual-Machines



> Either way, I'm eager to see this get accepted :-).
> 

And I'm eager to start work on this :)


Thanks for your comments.
Nitin

^ permalink raw reply

* Call for Workshops: 2nd International Conference on Cloud Computing (CloudComp2010)
From: Ming Zhao @ 2010-03-28 20:57 UTC (permalink / raw)


[Apologies if you receive multiple copies of this announcement]



Update:
-------

* Workshop proposal submission deadline is extended to March 31





========================================================================
CLOUDCOMP 2010 CALL FOR WORKSHOP PROPOSALS

The 2010 International Conference on Cloud Computing (CloudComp 2010)
will be held in Barcelona, Spain, from Monday 25th to Friday 29th
October 2010. Workshops will be held on Friday 29th October 2010

We invite proposals for workshops and tutorials. Workshops are
intended to provide a platform for presenting novel ideas in a less
formal and possibly more focused way than is possioble in the main
conference. Organisers are free to set the format for workshops in
order to best fit the theme. We will include full- or half-day
workshops in the programme.

Tutorials are intended to be more directed at learning, for graduate
and industrial audiences, of technologies and techniques appropriate
to the conference. We especially welcome tutorial proposals by
practitioners in significant fields, or tutorials that present
cross-disciplinary approaches to cloud computing. We expect tutorials
to be half-day only.

Please submit proposals to the CloudComp 2010 Workshop Chair, Simon
Dobson (cloudcompws2010@icst.org). Proposals should be by email (PDF or 
plain text) with "CloudComp" in the subject header. The deadlines for 
submissions are:

- Workshops: 0700 UTC Wednesday 31st March 2010
- Tutorials: 0700 UTC Wednesday 21st July 2010

Notifications of acceptance will be sent out no more than two weeks
after the submission deadline. Both workshop and tutorial proposals
should include:

  1. The name of the workshop or tutorial.
  2. A statement of goals oals and/or learning outcomes.
  3. The names and affiliations of the organisers.
  4. The names of programme committee members, presenters or keynote 
speakers.
  5. A draft call for participation
  6. The expected number of attendees and the planned length (full- or 
half-day).
  7. A description of the plans for publicity.
  8. An explanation of any plans for proceedings or learning materials.
  9. A description of past versions of the event, including dates,
     organisers, submission and acceptance counts, attendance, sites,
     registration fees and summary budget information, if any.
10. The URL of the workshop or tutorial web site, if available.

Please forward any questions to Simon Dobson at cloudcompws2010@icst.org.






-- 
Ming Zhao, Assistant Professor
School of Computing and Information Sciences
Florida International University
Tel: (305) 348-2034, Fax: (305) 348-3549
Web: http://www.cis.fiu.edu/~zhaom

^ permalink raw reply

* Re: [PATCH 0/7] virtio: console: Fixes, new flexible way of discovering ports
From: Rusty Russell @ 2010-03-30  5:16 UTC (permalink / raw)
  To: Amit Shah; +Cc: mst, virtualization
In-Reply-To: <1269334680-1369-1-git-send-email-amit.shah@redhat.com>

On Tue, 23 Mar 2010 07:27:53 pm Amit Shah wrote:
> Hello all,
> 
> This new patchset switches over to using the control queue for port
> discovery so that we can stay in sync with the host for port
> enumeration and also don't use bitmaps to limit us to config space
> sizes.
> 
> This changes the ABI, so it would be better to merge this for 2.6.34
> so that we don't have to worry about released kernels with the older
> ABI.

This looks horribly like development during the merge window.

Sorry,
Rusty.

^ permalink raw reply

* [PATCH 1/4] MAINTAINERS: Put the virtio-console entry in correct alphabetical order
From: Rusty Russell @ 2010-03-30  5:17 UTC (permalink / raw)
  To: virtualization; +Cc: Amit Shah, Michael S. Tsirkin

From: Amit Shah <amit.shah@redhat.com>

Move around the entry for virtio-console to keep the file sorted.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 MAINTAINERS |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 449d444..e3548dd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2474,12 +2474,6 @@ L:	linuxppc-dev@ozlabs.org
 S:	Odd Fixes
 F:	drivers/char/hvc_*
 
-VIRTIO CONSOLE DRIVER
-M:	Amit Shah <amit.shah@redhat.com>
-L:	virtualization@lists.linux-foundation.org
-S:	Maintained
-F:	drivers/char/virtio_console.c
-
 iSCSI BOOT FIRMWARE TABLE (iBFT) DRIVER
 M:	Peter Jones <pjones@redhat.com>
 M:	Konrad Rzeszutek Wilk <konrad@kernel.org>
@@ -5961,6 +5955,13 @@ S:	Maintained
 F:	Documentation/filesystems/vfat.txt
 F:	fs/fat/
 
+VIRTIO CONSOLE DRIVER
+M:	Amit Shah <amit.shah@redhat.com>
+L:	virtualization@lists.linux-foundation.org
+S:	Maintained
+F:	drivers/char/virtio_console.c
+F:	include/linux/virtio_console.h
+
 VIRTIO HOST (VHOST)
 M:	"Michael S. Tsirkin" <mst@redhat.com>
 L:	kvm@vger.kernel.org

^ permalink raw reply related

* [PATCH 2/4] virtio: console: Fix early_put_chars usage
From: Rusty Russell @ 2010-03-30  5:19 UTC (permalink / raw)
  To: virtualization; +Cc: Amit Shah, Christian Borntraeger, Michael S. Tsirkin

From: Francois Diakhate <fdiakh@gmail.com>

Currently early_put_chars is not used by virtio_console because it can
only be used once a port has been found, at which point it's too late
because it is no longer needed. This patch should fix it.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 drivers/char/virtio_console.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 18b1b06..f33ceaa 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -650,13 +650,13 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 {
 	struct port *port;
 
+	if (unlikely(early_put_chars))
+		return early_put_chars(vtermno, buf, count);
+
 	port = find_port_by_vtermno(vtermno);
 	if (!port)
 		return 0;
 
-	if (unlikely(early_put_chars))
-		return early_put_chars(vtermno, buf, count);
-
 	return send_buf(port, (void *)buf, count);
 }

^ permalink raw reply related

* [PATCH 3/4] virtio: console makes incorrect assumption about virtio API
From: Rusty Russell @ 2010-03-30  5:19 UTC (permalink / raw)
  To: Amit Shah; +Cc: Michael S. Tsirkin, virtualization

The get_buf() API sets the second arg to the number of bytes *written*
by the other side; in this case it should be zero as these are output buffers.

lguest gets this right (obviously kvm's console doesn't), resulting in
continual buildup of console writes.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
To: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -415,20 +415,16 @@ static ssize_t send_buf(struct port *por
 	out_vq->vq_ops->kick(out_vq);
 
 	if (ret < 0) {
-		len = 0;
+		in_count = 0;
 		goto fail;
 	}
 
-	/*
-	 * Wait till the host acknowledges it pushed out the data we
-	 * sent. Also ensure we return to userspace the number of
-	 * bytes that were successfully consumed by the host.
-	 */
+	/* Wait till the host acknowledges it pushed out the data we sent. */
 	while (!out_vq->vq_ops->get_buf(out_vq, &len))
 		cpu_relax();
 fail:
 	/* We're expected to return the amount of data we wrote */
-	return len;
+	return in_count;
 }
 
 /*

^ permalink raw reply

* [PATCH 4/4] virtio: disable multiport console support.
From: Rusty Russell @ 2010-03-30  5:19 UTC (permalink / raw)
  To: Amit Shah; +Cc: Michael S. Tsirkin, virtualization

Unfortunately there proved to be at least one bug which requires an
ABI change, so we're best off not introducing multiport support until
2.6.35.

While I generally left the multiport code paths intact, I really wanted
to remove the ABI defines from the header, which meant some quite deep cuts.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
To: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c  |  274 -----------------------------------------
 include/linux/virtio_console.h |   19 --
 2 files changed, 4 insertions(+), 289 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -252,15 +252,10 @@ static bool is_console_port(struct port 
 	return false;
 }
 
+/* This is incomplete. */
 static inline bool use_multiport(struct ports_device *portdev)
 {
-	/*
-	 * This condition can be true when put_chars is called from
-	 * early_init
-	 */
-	if (!portdev->vdev)
-		return 0;
-	return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
+	return false;
 }
 
 static void free_buf(struct port_buffer *buf)
@@ -373,31 +368,8 @@ out:
 	return ret;
 }
 
-static ssize_t send_control_msg(struct port *port, unsigned int event,
-				unsigned int value)
-{
-	struct scatterlist sg[1];
-	struct virtio_console_control cpkt;
-	struct virtqueue *vq;
-	unsigned int len;
-
-	if (!use_multiport(port->portdev))
-		return 0;
-
-	cpkt.id = port->id;
-	cpkt.event = event;
-	cpkt.value = value;
-
-	vq = port->portdev->c_ovq;
-
-	sg_init_one(sg, &cpkt, sizeof(cpkt));
-	if (vq->vq_ops->add_buf(vq, sg, 1, 0, &cpkt) >= 0) {
-		vq->vq_ops->kick(vq);
-		while (!vq->vq_ops->get_buf(vq, &len))
-			cpu_relax();
-	}
-	return 0;
-}
+/* For when we support control messages. */
+#define send_control_msg(port, event, value) 0
 
 static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
 {
@@ -882,142 +854,6 @@ static int remove_port(struct port *port
 	return 0;
 }
 
-/* Any private messages that the Host and Guest want to share */
-static void handle_control_message(struct ports_device *portdev,
-				   struct port_buffer *buf)
-{
-	struct virtio_console_control *cpkt;
-	struct port *port;
-	size_t name_size;
-	int err;
-
-	cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
-
-	port = find_port_by_id(portdev, cpkt->id);
-	if (!port) {
-		/* No valid header at start of buffer.  Drop it. */
-		dev_dbg(&portdev->vdev->dev,
-			"Invalid index %u in control packet\n", cpkt->id);
-		return;
-	}
-
-	switch (cpkt->event) {
-	case VIRTIO_CONSOLE_CONSOLE_PORT:
-		if (!cpkt->value)
-			break;
-		if (is_console_port(port))
-			break;
-
-		init_port_console(port);
-		/*
-		 * Could remove the port here in case init fails - but
-		 * have to notify the host first.
-		 */
-		break;
-	case VIRTIO_CONSOLE_RESIZE:
-		if (!is_console_port(port))
-			break;
-		port->cons.hvc->irq_requested = 1;
-		resize_console(port);
-		break;
-	case VIRTIO_CONSOLE_PORT_OPEN:
-		port->host_connected = cpkt->value;
-		wake_up_interruptible(&port->waitqueue);
-		break;
-	case VIRTIO_CONSOLE_PORT_NAME:
-		/*
-		 * Skip the size of the header and the cpkt to get the size
-		 * of the name that was sent
-		 */
-		name_size = buf->len - buf->offset - sizeof(*cpkt) + 1;
-
-		port->name = kmalloc(name_size, GFP_KERNEL);
-		if (!port->name) {
-			dev_err(port->dev,
-				"Not enough space to store port name\n");
-			break;
-		}
-		strncpy(port->name, buf->buf + buf->offset + sizeof(*cpkt),
-			name_size - 1);
-		port->name[name_size - 1] = 0;
-
-		/*
-		 * Since we only have one sysfs attribute, 'name',
-		 * create it only if we have a name for the port.
-		 */
-		err = sysfs_create_group(&port->dev->kobj,
-					 &port_attribute_group);
-		if (err) {
-			dev_err(port->dev,
-				"Error %d creating sysfs device attributes\n",
-				err);
-		} else {
-			/*
-			 * Generate a udev event so that appropriate
-			 * symlinks can be created based on udev
-			 * rules.
-			 */
-			kobject_uevent(&port->dev->kobj, KOBJ_CHANGE);
-		}
-		break;
-	case VIRTIO_CONSOLE_PORT_REMOVE:
-		/*
-		 * Hot unplug the port.  We don't decrement nr_ports
-		 * since we don't want to deal with extra complexities
-		 * of using the lowest-available port id: We can just
-		 * pick up the nr_ports number as the id and not have
-		 * userspace send it to us.  This helps us in two
-		 * ways:
-		 *
-		 * - We don't need to have a 'port_id' field in the
-		 *   config space when a port is hot-added.  This is a
-		 *   good thing as we might queue up multiple hotplug
-		 *   requests issued in our workqueue.
-		 *
-		 * - Another way to deal with this would have been to
-		 *   use a bitmap of the active ports and select the
-		 *   lowest non-active port from that map.  That
-		 *   bloats the already tight config space and we
-		 *   would end up artificially limiting the
-		 *   max. number of ports to sizeof(bitmap).  Right
-		 *   now we can support 2^32 ports (as the port id is
-		 *   stored in a u32 type).
-		 *
-		 */
-		remove_port(port);
-		break;
-	}
-}
-
-static void control_work_handler(struct work_struct *work)
-{
-	struct ports_device *portdev;
-	struct virtqueue *vq;
-	struct port_buffer *buf;
-	unsigned int len;
-
-	portdev = container_of(work, struct ports_device, control_work);
-	vq = portdev->c_ivq;
-
-	spin_lock(&portdev->cvq_lock);
-	while ((buf = vq->vq_ops->get_buf(vq, &len))) {
-		spin_unlock(&portdev->cvq_lock);
-
-		buf->len = len;
-		buf->offset = 0;
-
-		handle_control_message(portdev, buf);
-
-		spin_lock(&portdev->cvq_lock);
-		if (add_inbuf(portdev->c_ivq, buf) < 0) {
-			dev_warn(&portdev->vdev->dev,
-				 "Error adding buffer to queue\n");
-			free_buf(buf);
-		}
-	}
-	spin_unlock(&portdev->cvq_lock);
-}
-
 static void in_intr(struct virtqueue *vq)
 {
 	struct port *port;
@@ -1206,62 +1042,6 @@ fail:
 	return err;
 }
 
-/*
- * The workhandler for config-space updates.
- *
- * This is called when ports are hot-added.
- */
-static void config_work_handler(struct work_struct *work)
-{
-	struct virtio_console_config virtconconf;
-	struct ports_device *portdev;
-	struct virtio_device *vdev;
-	int err;
-
-	portdev = container_of(work, struct ports_device, config_work);
-
-	vdev = portdev->vdev;
-	vdev->config->get(vdev,
-			  offsetof(struct virtio_console_config, nr_ports),
-			  &virtconconf.nr_ports,
-			  sizeof(virtconconf.nr_ports));
-
-	if (portdev->config.nr_ports == virtconconf.nr_ports) {
-		/*
-		 * Port 0 got hot-added.  Since we already did all the
-		 * other initialisation for it, just tell the Host
-		 * that the port is ready if we find the port.  In
-		 * case the port was hot-removed earlier, we call
-		 * add_port to add the port.
-		 */
-		struct port *port;
-
-		port = find_port_by_id(portdev, 0);
-		if (!port)
-			add_port(portdev, 0);
-		else
-			send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
-		return;
-	}
-	if (virtconconf.nr_ports > portdev->config.max_nr_ports) {
-		dev_warn(&vdev->dev,
-			 "More ports specified (%u) than allowed (%u)",
-			 portdev->config.nr_ports + 1,
-			 portdev->config.max_nr_ports);
-		return;
-	}
-	if (virtconconf.nr_ports < portdev->config.nr_ports)
-		return;
-
-	/* Hot-add ports */
-	while (virtconconf.nr_ports - portdev->config.nr_ports) {
-		err = add_port(portdev, portdev->config.nr_ports);
-		if (err)
-			break;
-		portdev->config.nr_ports++;
-	}
-}
-
 static int init_vqs(struct ports_device *portdev)
 {
 	vq_callback_t **io_callbacks;
@@ -1385,7 +1165,6 @@ static int __devinit virtcons_probe(stru
 	struct ports_device *portdev;
 	u32 i;
 	int err;
-	bool multiport;
 
 	portdev = kmalloc(sizeof(*portdev), GFP_KERNEL);
 	if (!portdev) {
@@ -1411,32 +1190,8 @@ static int __devinit virtcons_probe(stru
 		goto free;
 	}
 
-	multiport = false;
 	portdev->config.nr_ports = 1;
 	portdev->config.max_nr_ports = 1;
-	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
-		multiport = true;
-		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_MULTIPORT;
-
-		vdev->config->get(vdev, offsetof(struct virtio_console_config,
-						 nr_ports),
-				  &portdev->config.nr_ports,
-				  sizeof(portdev->config.nr_ports));
-		vdev->config->get(vdev, offsetof(struct virtio_console_config,
-						 max_nr_ports),
-				  &portdev->config.max_nr_ports,
-				  sizeof(portdev->config.max_nr_ports));
-		if (portdev->config.nr_ports > portdev->config.max_nr_ports) {
-			dev_warn(&vdev->dev,
-				 "More ports (%u) specified than allowed (%u). Will init %u ports.",
-				 portdev->config.nr_ports,
-				 portdev->config.max_nr_ports,
-				 portdev->config.max_nr_ports);
-
-			portdev->config.nr_ports = portdev->config.max_nr_ports;
-		}
-	}
-
 	/* Let the Host know we support multiple ports.*/
 	vdev->config->finalize_features(vdev);
 
@@ -1449,22 +1204,6 @@ static int __devinit virtcons_probe(stru
 	spin_lock_init(&portdev->ports_lock);
 	INIT_LIST_HEAD(&portdev->ports);
 
-	if (multiport) {
-		unsigned int nr_added_bufs;
-
-		spin_lock_init(&portdev->cvq_lock);
-		INIT_WORK(&portdev->control_work, &control_work_handler);
-		INIT_WORK(&portdev->config_work, &config_work_handler);
-
-		nr_added_bufs = fill_queue(portdev->c_ivq, &portdev->cvq_lock);
-		if (!nr_added_bufs) {
-			dev_err(&vdev->dev,
-				"Error allocating buffers for control queue\n");
-			err = -ENOMEM;
-			goto free_vqs;
-		}
-	}
-
 	for (i = 0; i < portdev->config.nr_ports; i++)
 		add_port(portdev, i);
 
@@ -1472,10 +1211,6 @@ static int __devinit virtcons_probe(stru
 	early_put_chars = NULL;
 	return 0;
 
-free_vqs:
-	vdev->config->del_vqs(vdev);
-	kfree(portdev->in_vqs);
-	kfree(portdev->out_vqs);
 free_chrdev:
 	unregister_chrdev(portdev->chr_major, "virtio-portsdev");
 free:
@@ -1521,7 +1256,6 @@ static struct virtio_device_id id_table[
 
 static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_SIZE,
-	VIRTIO_CONSOLE_F_MULTIPORT,
 };
 
 static struct virtio_driver virtio_console = {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -12,7 +12,6 @@
 
 /* Feature bits */
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
-#define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
 
 struct virtio_console_config {
 	/* colums of the screens */
@@ -25,24 +24,6 @@ struct virtio_console_config {
 	__u32 nr_ports;
 } __attribute__((packed));
 
-/*
- * A message that's passed between the Host and the Guest for a
- * particular port.
- */
-struct virtio_console_control {
-	__u32 id;		/* Port number */
-	__u16 event;		/* The kind of control event (see below) */
-	__u16 value;		/* Extra information for the key */
-};
-
-/* Some events for control messages */
-#define VIRTIO_CONSOLE_PORT_READY	0
-#define VIRTIO_CONSOLE_CONSOLE_PORT	1
-#define VIRTIO_CONSOLE_RESIZE		2
-#define VIRTIO_CONSOLE_PORT_OPEN	3
-#define VIRTIO_CONSOLE_PORT_NAME	4
-#define VIRTIO_CONSOLE_PORT_REMOVE	5
-
 #ifdef __KERNEL__
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
 #endif /* __KERNEL__ */

^ permalink raw reply

* Re: [PATCH 0/7] virtio: console: Fixes, new flexible way of discovering ports
From: Amit Shah @ 2010-03-30  5:20 UTC (permalink / raw)
  To: Rusty Russell; +Cc: mst, virtualization
In-Reply-To: <201003301546.14309.rusty@rustcorp.com.au>

On (Tue) Mar 30 2010 [15:46:14], Rusty Russell wrote:
> On Tue, 23 Mar 2010 07:27:53 pm Amit Shah wrote:
> > Hello all,
> > 
> > This new patchset switches over to using the control queue for port
> > discovery so that we can stay in sync with the host for port
> > enumeration and also don't use bitmaps to limit us to config space
> > sizes.
> > 
> > This changes the ABI, so it would be better to merge this for 2.6.34
> > so that we don't have to worry about released kernels with the older
> > ABI.
> 
> This looks horribly like development during the merge window.

Yep, let's go ahead with disabling multiport for now and enabling it
later for .35.

Thanks,

		Amit

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox