* [Qemu-devel] [PATCH 0/2] virtio-net: fix config size for older guests @ 2013-02-02 23:06 Anthony Liguori 2013-02-02 23:06 ` [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init Anthony Liguori 2013-02-02 23:06 ` [Qemu-devel] [PATCH 2/2] virtio-net: do not expose a larger config size if multiqueue isn't enabled Anthony Liguori 0 siblings, 2 replies; 4+ messages in thread From: Anthony Liguori @ 2013-02-02 23:06 UTC (permalink / raw) To: qemu-devel; +Cc: Michael Tokarev virtio-net multiqueue added a new field to the config layout. The config layout is rounded up to the nearest power of 2 and this field caused field causes the size to go from 8 bytes to 16 bytes. For reasons unknown, the increase in config size causes the Windows virtio-net driver to fail. This is probably a bug in the driver but it exposes a larger bug in QEMU. While we disable features for compatibility machines, we always use the latest config size. This presents a slightly different machine model than what we should be exposing. This series fixes this specific problem such that -M pc-1.3 should work again with the Windows drivers. -M pc will still fail. We need to find a better way to do this but in absence of a nicer approach this seems likes a good compromise for 1.4. I think we need to choose something like the MAX() config size required by all of the enabled host features. Besides -M pc-1.3, another work around would be: -global virtio-net-pci.mq=off This should work after this patch series is applied. We can put this work around in the release notes as suggested for users of the virtio-win drivers until they are fixed. Reported-by: Michael Tokarev <mjt@tls.msk.ru> FYI: only lightly tested on my end but so far, it seems to work. ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init 2013-02-02 23:06 [Qemu-devel] [PATCH 0/2] virtio-net: fix config size for older guests Anthony Liguori @ 2013-02-02 23:06 ` Anthony Liguori 2013-02-03 0:12 ` Andreas Färber 2013-02-02 23:06 ` [Qemu-devel] [PATCH 2/2] virtio-net: do not expose a larger config size if multiqueue isn't enabled Anthony Liguori 1 sibling, 1 reply; 4+ messages in thread From: Anthony Liguori @ 2013-02-02 23:06 UTC (permalink / raw) To: qemu-devel; +Cc: Anthony Liguori, Michael Tokarev Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- hw/s390x/s390-virtio-bus.c | 3 ++- hw/s390x/virtio-ccw.c | 3 ++- hw/virtio-pci.c | 3 ++- hw/virtio.h | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index d467781..089ed92 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -153,7 +153,8 @@ static int s390_virtio_net_init(VirtIOS390Device *dev) { VirtIODevice *vdev; - vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net); + vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net, + dev->host_features); if (!vdev) { return -1; } diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 231f81e..d92e427 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -555,7 +555,8 @@ static int virtio_ccw_net_init(VirtioCcwDevice *dev) { VirtIODevice *vdev; - vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net); + vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net, + dev->host_features[0]); if (!vdev) { return -1; } diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 9abbcdf..a869f53 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -997,7 +997,8 @@ static int virtio_net_init_pci(PCIDevice *pci_dev) VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); VirtIODevice *vdev; - vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, &proxy->net); + vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, &proxy->net, + proxy->host_features); vdev->nvectors = proxy->nvectors; virtio_init_pci(proxy, vdev); diff --git a/hw/virtio.h b/hw/virtio.h index a29a54d..1e206b8 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -243,7 +243,8 @@ typedef struct VirtIOBlkConf VirtIOBlkConf; VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk); struct virtio_net_conf; VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, - struct virtio_net_conf *net); + struct virtio_net_conf *net, + uint32_t host_features); typedef struct virtio_serial_conf virtio_serial_conf; VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *serial); VirtIODevice *virtio_balloon_init(DeviceState *dev); -- 1.8.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init 2013-02-02 23:06 ` [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init Anthony Liguori @ 2013-02-03 0:12 ` Andreas Färber 0 siblings, 0 replies; 4+ messages in thread From: Andreas Färber @ 2013-02-03 0:12 UTC (permalink / raw) To: Anthony Liguori; +Cc: Michael Tokarev, qemu-devel Am 03.02.2013 00:06, schrieb Anthony Liguori: > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > hw/s390x/s390-virtio-bus.c | 3 ++- > hw/s390x/virtio-ccw.c | 3 ++- > hw/virtio-pci.c | 3 ++- > hw/virtio.h | 3 ++- > 4 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c > index d467781..089ed92 100644 > --- a/hw/s390x/s390-virtio-bus.c > +++ b/hw/s390x/s390-virtio-bus.c > @@ -153,7 +153,8 @@ static int s390_virtio_net_init(VirtIOS390Device *dev) > { > VirtIODevice *vdev; > > - vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net); > + vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net, > + dev->host_features); > if (!vdev) { > return -1; > } > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index 231f81e..d92e427 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -555,7 +555,8 @@ static int virtio_ccw_net_init(VirtioCcwDevice *dev) > { > VirtIODevice *vdev; > > - vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net); > + vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net, > + dev->host_features[0]); > if (!vdev) { > return -1; > } > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 9abbcdf..a869f53 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -997,7 +997,8 @@ static int virtio_net_init_pci(PCIDevice *pci_dev) > VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > VirtIODevice *vdev; > > - vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, &proxy->net); > + vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, &proxy->net, > + proxy->host_features); > > vdev->nvectors = proxy->nvectors; > virtio_init_pci(proxy, vdev); > diff --git a/hw/virtio.h b/hw/virtio.h > index a29a54d..1e206b8 100644 > --- a/hw/virtio.h > +++ b/hw/virtio.h > @@ -243,7 +243,8 @@ typedef struct VirtIOBlkConf VirtIOBlkConf; > VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk); > struct virtio_net_conf; > VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, > - struct virtio_net_conf *net); > + struct virtio_net_conf *net, > + uint32_t host_features); > typedef struct virtio_serial_conf virtio_serial_conf; > VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *serial); > VirtIODevice *virtio_balloon_init(DeviceState *dev); The virtio_net_init() implementation went into 2/2 so this breaks. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH 2/2] virtio-net: do not expose a larger config size if multiqueue isn't enabled 2013-02-02 23:06 [Qemu-devel] [PATCH 0/2] virtio-net: fix config size for older guests Anthony Liguori 2013-02-02 23:06 ` [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init Anthony Liguori @ 2013-02-02 23:06 ` Anthony Liguori 1 sibling, 0 replies; 4+ messages in thread From: Anthony Liguori @ 2013-02-02 23:06 UTC (permalink / raw) To: qemu-devel; +Cc: Anthony Liguori, Michael Tokarev The windows PV drivers seem to get very upset if the config size changes. Even when we disable features, we don't change the config size today which is a bug. This patch is a pretty rough way to solve this problem and it only handles the multiqueue case specifically. Cc: Michael Tokarev <mjt@tls.msk.ru> Reported-by: Michael Tokarev <mjt@tls.msk.ru> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- hw/virtio-net.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index e37358a..8d15813 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -73,6 +73,7 @@ typedef struct VirtIONet int multiqueue; uint16_t max_queues; uint16_t curr_queues; + int config_size; } VirtIONet; static VirtIONetQueue *virtio_net_get_subqueue(NetClientState *nc) @@ -99,20 +100,20 @@ static VirtIONet *to_virtio_net(VirtIODevice *vdev) static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) { VirtIONet *n = to_virtio_net(vdev); - struct virtio_net_config netcfg; + struct virtio_net_config netcfg = {}; stw_p(&netcfg.status, n->status); stw_p(&netcfg.max_virtqueue_pairs, n->max_queues); memcpy(netcfg.mac, n->mac, ETH_ALEN); - memcpy(config, &netcfg, sizeof(netcfg)); + memcpy(config, &netcfg, n->config_size); } static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) { VirtIONet *n = to_virtio_net(vdev); - struct virtio_net_config netcfg; + struct virtio_net_config netcfg = {}; - memcpy(&netcfg, config, sizeof(netcfg)); + memcpy(&netcfg, config, n->config_size); if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) && memcmp(netcfg.mac, n->mac, ETH_ALEN)) { @@ -1279,14 +1280,21 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, } VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, - virtio_net_conf *net) + virtio_net_conf *net, uint32_t host_features) { VirtIONet *n; int i; + int config_size; + if (host_features & VIRTIO_NET_F_MQ) { + config_size = sizeof(struct virtio_net_config); + } else { + config_size = 8; + } n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET, - sizeof(struct virtio_net_config), + config_size, sizeof(VirtIONet)); + n->config_size = config_size; n->vdev.get_config = virtio_net_get_config; n->vdev.set_config = virtio_net_set_config; -- 1.8.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-02-03 0:12 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-02-02 23:06 [Qemu-devel] [PATCH 0/2] virtio-net: fix config size for older guests Anthony Liguori 2013-02-02 23:06 ` [Qemu-devel] [PATCH 1/2] virtio-net: pass host features to virtio_net_init Anthony Liguori 2013-02-03 0:12 ` Andreas Färber 2013-02-02 23:06 ` [Qemu-devel] [PATCH 2/2] virtio-net: do not expose a larger config size if multiqueue isn't enabled Anthony Liguori
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).