public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: kvm-devel <kvm@vger.kernel.org>
Subject: Re: [PATCH][RFC] Prepare virtio for upstream QEMU merging
Date: Tue, 14 Oct 2008 19:30:50 +0200	[thread overview]
Message-ID: <48F4D74A.5090602@redhat.com> (raw)
In-Reply-To: <48F4C6FF.4010801@codemonkey.ws>

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.


  reply	other threads:[~2008-10-14 17:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-13 18:28 [PATCH][RFC] Prepare virtio for upstream QEMU merging Anthony Liguori
2008-10-14 16:12 ` Avi Kivity
2008-10-14 16:21   ` Anthony Liguori
2008-10-14 17:30     ` Avi Kivity [this message]
2008-10-14 18:08       ` Anthony Liguori
2008-10-15 13:57         ` Avi Kivity
2008-10-15 14:11           ` Anthony Liguori
2008-10-16  9:55             ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48F4D74A.5090602@redhat.com \
    --to=avi@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=kvm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox