From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH][RFC] Prepare virtio for upstream QEMU merging
Date: Tue, 14 Oct 2008 19:30:50 +0200
Message-ID: <48F4D74A.5090602@redhat.com>
References: <48F3934E.9040809@codemonkey.ws> <48F4C4FF.8020207@redhat.com> <48F4C6FF.4010801@codemonkey.ws>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: kvm-devel <kvm@vger.kernel.org>
To: Anthony Liguori <anthony@codemonkey.ws>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:33247 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752511AbYJNRbB (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 14 Oct 2008 13:31:01 -0400
In-Reply-To: <48F4C6FF.4010801@codemonkey.ws>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Anthony Liguori wrote:
>>
>> Why not merge these bits prior to merging virtio?  They aren't kvm
>> specific and would be good in mainline qemu.
>>   
>
> I'd rather have a consumer of an interface before merging the actual
> infrastructure.
>

So merge them all into qemu at the same time (as separate patches, if
you like).

>>>  
>>> +#ifdef USE_KVM
>>>  /* dhclient uses AF_PACKET but doesn't pass auxdata to the kernel so
>>>   * it never finds out that the packets don't have valid checksums. 
>>> This
>>>   * causes dhclient to get upset.  Fedora's carried a patch for ages to
>>> @@ -181,6 +190,7 @@ static void work_around_broken_dhclient(struct
>>> virtio_net_hdr *hdr,
>>>          hdr->flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;
>>>      }
>>>  }
>>> +#endif
>>>       
>>
>> Is this kvm specific?
>>   
>
> It is because the GSO stuff has not gone into upstream QEMU yet.  I'd
> rather merge a crippled virtio implementation, and then merge each of
> the optimizations than vice-versa.  Primarily because I'm concerned
> some of these optimizations will require refactoring.
>

Ok.

>
>>> +#ifdef USE_KVM
>>> +    VRingDesc *desc;
>>> +    VRingAvail *avail;
>>> +    VRingUsed *used;
>>> +#else
>>> +    target_phys_addr_t desc;
>>> +    target_phys_addr_t avail;
>>> +    target_phys_addr_t used;
>>> +#endif
>>> +} VRing;
>>>       
>>
>> Stumped.  Why?
>>   
>
> In KVM, desc points directly to guest memory.  The non-KVM accessors
> use stl/ldl et al so they only need the physical address base.
>
> If you agree with this approach, I'll clean up the patch and send again.

The amount of code duplication is frightening.

I suggest a target_phys_ptr_t which is a struct containing either a
pointer or a target_phys_ptr_t, depending on guest/host endianness. 
Accessors through this type will either deref the pointer or call the
qemu variant.

Oh, and we need to set the dirty bit so live migration works.  Or do we
have a hack in place to force copying of the ring at the last stage?


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.