qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: qemu-devel@nongnu.org, Cornelia Huck <cornelia.huck@de.ibm.com>,
	Jens Freimann <jfrei@linux.vnet.ibm.com>,
	Alexander Graf <agraf@suse.de>,
	Jesse Larrew <jlarrew@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] hw/virtio-net.c: set config size using host features
Date: Thu, 07 Mar 2013 17:27:51 +0100	[thread overview]
Message-ID: <5138C007.6080305@de.ibm.com> (raw)
In-Reply-To: <87621319kc.fsf@codemonkey.ws>

> You're misreading how this works.
> 
> Host features are set based on command line arguments.  This is
> advertised to the guest.  The vdev->get_config() call then sanitizes
> features.  For instance, look at:
> 
> static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
> {
>     VirtIONet *n = to_virtio_net(vdev);
>     NetClientState *nc = qemu_get_queue(n->nic);
> 
>     features |= (1 << VIRTIO_NET_F_MAC);
> 
>     if (!peer_has_vnet_hdr(n)) {
>         features &= ~(0x1 << VIRTIO_NET_F_CSUM);
> <snip>
> 
> This removes the VIRTIO_NET_F_CSUM feature if the peer doesn't support
> it.  It's presupposing that the feature bit is set.
> 
> It's a bug in both virtio-ccw that features=0 when get_features is
> called.  You can also tell this with:
> 
> [10:02 AM] anthony@titi:~/git/qemu/hw/s390x$ grep DEFINE_VIRTIO_NET_FEATURES *
> virtio-ccw.c:    DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]),
> 
> So virtio-s390 is doing it wrong, but virtio-ccw looks like its doing it
> right.

There is a  parse error in your statement (error in both virtio-ccw).
Is virtio-ccw ok or not?


At least, this patch seems to work. (That also implies, that a transport
must not hide virtio feature bits).


From: Christian Borntraeger <borntraeger@de.ibm.com>
Date: Thu, 7 Mar 2013 17:21:41 +0100
Subject: [PATCH] Allow virtio-net features for legacy s390 virtio bus

Enable all virtio-net features for the legacy s390 virtio bus.
This also fixes
kernel BUG at /usr/src/packages/BUILD/kernel-default-3.0.58/linux-3.0/drivers/s390/kvm/kvm_virtio.c:121!

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/s390-virtio-bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 1200691..a8a8e19 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -399,6 +399,7 @@ static const VirtIOBindings virtio_s390_bindings = {
 
 static Property s390_virtio_net_properties[] = {
     DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic),
+    DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
     DEFINE_PROP_UINT32("x-txtimer", VirtIOS390Device,
                        net.txtimer, TX_TIMER_INTERVAL),
     DEFINE_PROP_INT32("x-txburst", VirtIOS390Device,
-- 
1.8.0.1

  reply	other threads:[~2013-03-07 16:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1360108037-9211-1-git-send-email-jlarrew@linux.vnet.ibm.com>
     [not found] ` <1360108037-9211-3-git-send-email-jlarrew@linux.vnet.ibm.com>
2013-03-05 16:41   ` [Qemu-devel] [PATCH 2/3] hw/virtio-net.c: set config size using host features Alexander Graf
2013-03-05 16:48   ` Alexander Graf
2013-03-05 17:03     ` Christian Borntraeger
2013-03-07 12:28       ` Christian Borntraeger
2013-03-07 16:03       ` Anthony Liguori
2013-03-07 16:27         ` Christian Borntraeger [this message]
2013-03-07 16:38           ` Andreas Färber
2013-03-07 16:44             ` Anthony Liguori
2013-03-07 16:49               ` Michael S. Tsirkin
2013-03-07 17:23                 ` Anthony Liguori
2013-03-07 17:22               ` Andreas Färber
2013-03-07 17:41                 ` Anthony Liguori
2013-03-07 16:42           ` Alexander Graf
2013-03-07 16:43           ` Anthony Liguori

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=5138C007.6080305@de.ibm.com \
    --to=borntraeger@de.ibm.com \
    --cc=agraf@suse.de \
    --cc=aliguori@us.ibm.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=jfrei@linux.vnet.ibm.com \
    --cc=jlarrew@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.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;
as well as URLs for NNTP newsgroup(s).