Re: [PATCH] kvm guest balloon driver

[Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>]

Marcelo Tosatti wrote:
> On Tue, Jan 08, 2008 at 09:42:13AM -0600, Anthony Liguori wrote:
>   
>> Marcelo Tosatti wrote:
>>     
>>> Following patch introduces a KVM guest balloon driver. Communication
>>> to/from the host is performed via virtio.
>>>
>>>       
>
> I'll address the other comments.
>
>   
>>> +	virtballoon.dev = vdev;
>>> +	init_waitqueue_head(&virtballoon.balloon_wait);
>>> +	init_waitqueue_head(&virtballoon.rmmod_wait);
>>> +	atomic_set(&virtballoon.inflight_bufs, 0);
>>> +
>>> +	err = request_irq(pvdev->pci_dev->irq, balloon_irq, IRQF_SHARED,
>>> +			  pvdev->vdev.dev.bus_id, &virtballoon);
>>> +	if (err)
>>> +		goto out_free_vq;
>>>  
>>>       
>> Why is it taking over the irq?  This is very, very wrong.  A virtio 
>> device cannot be dependent on being used on top of the virtio-pci backend.
>>     
>
> A notification is necessary whenever the host changes the target value
> in the config space. So right now this notification is sharing the
> same IRQ as the main PCI device.
>   

A message should be sent over a virtqueue to indicate that the other 
need needs to reread a config value.

You really need two virtqueues.  The virtqueue you have now for the 
guest to send messages to the host, and another virtqueue that the guest 
adds buffers too that the host can use to send messages to the guest.  A 
good example of using two queues for bidirectional communication would 
be the virtio_console driver.

BTW, I don't think the target should be a config value.  You don't gain 
anything from it being in the config space and it's somewhat unnatural 
for a virtio device.  It makes more sense as a message to the guest.

The PCI config space is not automatically saved/restored during migration.

> Do you have any suggestion on how to retrieve the IRQ of the virtio
> device, or some other notification mechanism?
>   

A virtio device does not necessarily have an interrupt or it may have 
multiple interrupts.

Regards,

Anthony Liguori


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace