From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH v4 3/3] Inter-VM shared memory PCI device
Date: Thu, 15 Apr 2010 11:33:43 +0300
Message-ID: <4BC6CF67.5060608@redhat.com>
References: <1270680720-8457-1-git-send-email-cam@cs.ualberta.ca>	 <1270680720-8457-2-git-send-email-cam@cs.ualberta.ca>	 <1270680720-8457-3-git-send-email-cam@cs.ualberta.ca>	 <1270680720-8457-4-git-send-email-cam@cs.ualberta.ca>	 <4BC388EF.8000706@redhat.com> <m2s8286e4ee1004141630u9fcce2i395e4af96cb53e2e@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org
To: Cam Macdonell <cam@cs.ualberta.ca>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:60744 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755745Ab0DOIdv (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 15 Apr 2010 04:33:51 -0400
In-Reply-To: <m2s8286e4ee1004141630u9fcce2i395e4af96cb53e2e@mail.gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 04/15/2010 02:30 AM, Cam Macdonell wrote:
>>
>>> Sample programs, init scripts and the shared memory server are available
>>> in a
>>> git repo here:
>>>
>>>      www.gitorious.org/nahanni
>>>
>>>        
>> Please consider qemu.git/contrib.
>>      
> Should the compilation be tied into Qemu's regular build with a switch
> (e.g. --enable-ivshmem-server)? Or should it be its own separate
> build?
>    

It can have its own makefile.
>>> ---
>>>   Makefile.target |    3 +
>>>   hw/ivshmem.c    |  700
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   qemu-char.c     |    6 +
>>>   qemu-char.h     |    3 +
>>>
>>>        
>> qemu-doc.texi | 45 +++++++++++++
>>      
> Seems to be light on qdev devices.  I notice there is a section named
> "Data Type Index" that "could be used for qdev device names and
> options", but is currently empty.  Should I place documentation there
> of device there or just add it to "3.3 Invocation"?
>    

I think those are in qemu-options.hx.  Just put it somewhere where it 
seems appropriate.

>    
>>      
>>>   4 files changed, 712 insertions(+), 0 deletions(-)
>>>   create mode 100644 hw/ivshmem.c
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 1ffd802..bc9a681 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -199,6 +199,9 @@ obj-$(CONFIG_USB_OHCI) += usb-ohci.o
>>>   obj-y += rtl8139.o
>>>   obj-y += e1000.o
>>>
>>> +# Inter-VM PCI shared memory
>>> +obj-y += ivshmem.o
>>> +
>>>
>>>        
>> depends on CONFIG_PCI
>>      
> as in
>
> obj-($CONFIG_PCI) += ivshmem.o
>
>
> the variable CONFIG_PCI doesn't seem to be set during configuration.
> I don't see any other PCI devices that depend on it.

My mistake, keep as is.

> Do we also want
> to depend on CONFIG_KVM?
>    

No real need.

>>> +static void create_shared_memory_BAR(IVShmemState *s, int fd) {
>>> +
>>> +    s->shm_fd = fd;
>>> +
>>> +    s->ivshmem_offset = qemu_ram_mmap(s->shm_fd, s->ivshmem_size,
>>> +             MAP_SHARED, 0);
>>>
>>>        
>> Where did the offset go?
>>      
> 0 is the offset.  I include the offset parameter in qemu_ram_mmap() to
> make it flexible for other uses.

Makes sense.

> Are you suggesting to take an
> optional offset as an argument to -device?
>    

No need.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.