* [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core
@ 2015-07-22 9:36 Michael S. Tsirkin
2015-07-23 8:14 ` Jason Wang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2015-07-22 9:36 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Jason Wang, qemu-block,
Stefan Hajnoczi
Virtio 1 requires this, and all devices are clean by now,
so let's do it!
Exceptions:
- virtio-blk
- compat machine types
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Untested - consider this pseudo-code - it just seems easier to write it
in C than try to explain it.
include/hw/compat.h | 22 +++++++++++++++++++++-
include/hw/virtio/virtio.h | 4 +++-
hw/block/virtio-blk.c | 1 +
hw/net/virtio-net.c | 2 --
hw/scsi/virtio-scsi.c | 2 --
5 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 4a43466..94c8097 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,27 @@
#define HW_COMPAT_H
#define HW_COMPAT_2_3 \
- /* empty */
+ {\
+ .driver = "virtio-blk-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },{\
+ .driver = "virtio-balloon-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },{\
+ .driver = "virtio-serial-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },{\
+ .driver = "virtio-9p-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },{\
+ .driver = "virtio-rng-pci",\
+ .property = "any_layout",\
+ .value = "off",\
+ },
#define HW_COMPAT_2_2 \
/* empty */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 473fb75..fbb3c06 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -214,7 +214,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
DEFINE_PROP_BIT64("event_idx", _state, _field, \
VIRTIO_RING_F_EVENT_IDX, true), \
DEFINE_PROP_BIT64("notify_on_empty", _state, _field, \
- VIRTIO_F_NOTIFY_ON_EMPTY, true)
+ VIRTIO_F_NOTIFY_ON_EMPTY, true) \
+ DEFINE_PROP_BIT64("any_layout", _state, _field, \
+ VIRTIO_F_ANY_LAYOUT, true)
hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6aefda4..015b9b5 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -731,6 +731,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features)
virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
+ virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT);
if (s->conf.config_wce) {
virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 304d3dd..e203058 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1777,8 +1777,6 @@ static void virtio_net_instance_init(Object *obj)
}
static Property virtio_net_properties[] = {
- DEFINE_PROP_BIT("any_layout", VirtIONet, host_features,
- VIRTIO_F_ANY_LAYOUT, true),
DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
VIRTIO_NET_F_GUEST_CSUM, true),
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index f7d3c7c..d17698d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -953,8 +953,6 @@ static Property virtio_scsi_properties[] = {
0xFFFF),
DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
128),
- DEFINE_PROP_BIT("any_layout", VirtIOSCSI, host_features,
- VIRTIO_F_ANY_LAYOUT, true),
DEFINE_PROP_BIT("hotplug", VirtIOSCSI, host_features,
VIRTIO_SCSI_F_HOTPLUG, true),
DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,
--
MST
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core
2015-07-22 9:36 [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core Michael S. Tsirkin
@ 2015-07-23 8:14 ` Jason Wang
2015-07-23 14:39 ` Michael S. Tsirkin
2015-07-23 10:15 ` Christian Borntraeger
2015-07-23 10:43 ` Cornelia Huck
2 siblings, 1 reply; 6+ messages in thread
From: Jason Wang @ 2015-07-23 8:14 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, qemu-block, Stefan Hajnoczi
On 07/22/2015 05:36 PM, Michael S. Tsirkin wrote:
> Virtio 1 requires this,
I think you mean transitional not virtio 1?
> and all devices are clean by now,
> so let's do it!
>
> Exceptions:
> - virtio-blk
> - compat machine types
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Untested - consider this pseudo-code - it just seems easier to write it
> in C than try to explain it.
>
> include/hw/compat.h | 22 +++++++++++++++++++++-
> include/hw/virtio/virtio.h | 4 +++-
> hw/block/virtio-blk.c | 1 +
> hw/net/virtio-net.c | 2 --
> hw/scsi/virtio-scsi.c | 2 --
> 5 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 4a43466..94c8097 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,27 @@
> #define HW_COMPAT_H
>
> #define HW_COMPAT_2_3 \
> - /* empty */
> + {\
> + .driver = "virtio-blk-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-balloon-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-serial-pci",\
> + .property = "any_layout",\
> + .value = "off",\
In send_control_msg() it has
...
if (!virtqueue_pop(vq, &elem)) {
return 0;
}
memcpy(elem.in_sg[0].iov_base, buf, len);
...
So looks like serial is not ready for any layout.
> + },{\
> + .driver = "virtio-9p-pci",\
> + .property = "any_layout",\
> + .value = "off",\
In handle_9p_output() it has
...
BUG_ON(pdu->elem.out_sg[0].iov_len < 7);
...
So looks like 9p does not support any layout at least.
> + },{\
> + .driver = "virtio-rng-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },
>
> #define HW_COMPAT_2_2 \
> /* empty */
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 473fb75..fbb3c06 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -214,7 +214,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> DEFINE_PROP_BIT64("event_idx", _state, _field, \
> VIRTIO_RING_F_EVENT_IDX, true), \
> DEFINE_PROP_BIT64("notify_on_empty", _state, _field, \
> - VIRTIO_F_NOTIFY_ON_EMPTY, true)
> + VIRTIO_F_NOTIFY_ON_EMPTY, true) \
> + DEFINE_PROP_BIT64("any_layout", _state, _field, \
> + VIRTIO_F_ANY_LAYOUT, true)
>
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 6aefda4..015b9b5 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -731,6 +731,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features)
> virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
> virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
> virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
> + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT);
>
> if (s->conf.config_wce) {
> virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 304d3dd..e203058 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1777,8 +1777,6 @@ static void virtio_net_instance_init(Object *obj)
> }
>
> static Property virtio_net_properties[] = {
> - DEFINE_PROP_BIT("any_layout", VirtIONet, host_features,
> - VIRTIO_F_ANY_LAYOUT, true),
> DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
> DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
> VIRTIO_NET_F_GUEST_CSUM, true),
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index f7d3c7c..d17698d 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -953,8 +953,6 @@ static Property virtio_scsi_properties[] = {
> 0xFFFF),
> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
> 128),
> - DEFINE_PROP_BIT("any_layout", VirtIOSCSI, host_features,
> - VIRTIO_F_ANY_LAYOUT, true),
> DEFINE_PROP_BIT("hotplug", VirtIOSCSI, host_features,
> VIRTIO_SCSI_F_HOTPLUG, true),
> DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core
2015-07-22 9:36 [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core Michael S. Tsirkin
2015-07-23 8:14 ` Jason Wang
@ 2015-07-23 10:15 ` Christian Borntraeger
2015-07-23 14:37 ` Michael S. Tsirkin
2015-07-23 10:43 ` Cornelia Huck
2 siblings, 1 reply; 6+ messages in thread
From: Christian Borntraeger @ 2015-07-23 10:15 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Jason Wang, Stefan Hajnoczi,
qemu-block
Am 22.07.2015 um 11:36 schrieb Michael S. Tsirkin:
> Virtio 1 requires this, and all devices are clean by now,
> so let's do it!
>
> Exceptions:
> - virtio-blk
> - compat machine types
Is this targetted for 2.4 or 2.5? Pushing all the 1.0 stuff in 2.4 while
in hard freeze seems a bit dangerous as long as there are discussions going on.
(I just started looking into the scsi feature mail thread - no opinion yet)
Christian
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Untested - consider this pseudo-code - it just seems easier to write it
> in C than try to explain it.
>
> include/hw/compat.h | 22 +++++++++++++++++++++-
> include/hw/virtio/virtio.h | 4 +++-
> hw/block/virtio-blk.c | 1 +
> hw/net/virtio-net.c | 2 --
> hw/scsi/virtio-scsi.c | 2 --
> 5 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 4a43466..94c8097 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,27 @@
> #define HW_COMPAT_H
>
> #define HW_COMPAT_2_3 \
> - /* empty */
> + {\
> + .driver = "virtio-blk-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-balloon-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-serial-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-9p-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-rng-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },
>
> #define HW_COMPAT_2_2 \
> /* empty */
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 473fb75..fbb3c06 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -214,7 +214,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> DEFINE_PROP_BIT64("event_idx", _state, _field, \
> VIRTIO_RING_F_EVENT_IDX, true), \
> DEFINE_PROP_BIT64("notify_on_empty", _state, _field, \
> - VIRTIO_F_NOTIFY_ON_EMPTY, true)
> + VIRTIO_F_NOTIFY_ON_EMPTY, true) \
> + DEFINE_PROP_BIT64("any_layout", _state, _field, \
> + VIRTIO_F_ANY_LAYOUT, true)
>
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 6aefda4..015b9b5 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -731,6 +731,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features)
> virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
> virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
> virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
> + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT);
>
> if (s->conf.config_wce) {
> virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 304d3dd..e203058 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1777,8 +1777,6 @@ static void virtio_net_instance_init(Object *obj)
> }
>
> static Property virtio_net_properties[] = {
> - DEFINE_PROP_BIT("any_layout", VirtIONet, host_features,
> - VIRTIO_F_ANY_LAYOUT, true),
> DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
> DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
> VIRTIO_NET_F_GUEST_CSUM, true),
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index f7d3c7c..d17698d 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -953,8 +953,6 @@ static Property virtio_scsi_properties[] = {
> 0xFFFF),
> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
> 128),
> - DEFINE_PROP_BIT("any_layout", VirtIOSCSI, host_features,
> - VIRTIO_F_ANY_LAYOUT, true),
> DEFINE_PROP_BIT("hotplug", VirtIOSCSI, host_features,
> VIRTIO_SCSI_F_HOTPLUG, true),
> DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core
2015-07-22 9:36 [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core Michael S. Tsirkin
2015-07-23 8:14 ` Jason Wang
2015-07-23 10:15 ` Christian Borntraeger
@ 2015-07-23 10:43 ` Cornelia Huck
2 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2015-07-23 10:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, qemu-block, Jason Wang, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini
On Wed, 22 Jul 2015 12:36:22 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Virtio 1 requires this, and all devices are clean by now,
> so let's do it!
>
> Exceptions:
> - virtio-blk
> - compat machine types
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Untested - consider this pseudo-code - it just seems easier to write it
> in C than try to explain it.
>
> include/hw/compat.h | 22 +++++++++++++++++++++-
> include/hw/virtio/virtio.h | 4 +++-
> hw/block/virtio-blk.c | 1 +
> hw/net/virtio-net.c | 2 --
> hw/scsi/virtio-scsi.c | 2 --
> 5 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 4a43466..94c8097 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,27 @@
> #define HW_COMPAT_H
>
> #define HW_COMPAT_2_3 \
> - /* empty */
> + {\
> + .driver = "virtio-blk-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-balloon-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-serial-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-9p-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },{\
> + .driver = "virtio-rng-pci",\
> + .property = "any_layout",\
> + .value = "off",\
> + },
Isn't that a bit late for 2.4? Especially as not all devices are clean
yet, as Jason pointed out.
If we do this for 2.5, we'll also need compat handlers for ccw.
>
> #define HW_COMPAT_2_2 \
> /* empty */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core
2015-07-23 10:15 ` Christian Borntraeger
@ 2015-07-23 14:37 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2015-07-23 14:37 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Kevin Wolf, qemu-block, Jason Wang, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini
On Thu, Jul 23, 2015 at 12:15:22PM +0200, Christian Borntraeger wrote:
> Am 22.07.2015 um 11:36 schrieb Michael S. Tsirkin:
> > Virtio 1 requires this, and all devices are clean by now,
> > so let's do it!
> >
> > Exceptions:
> > - virtio-blk
> > - compat machine types
>
> Is this targetted for 2.4 or 2.5? Pushing all the 1.0 stuff in 2.4 while
> in hard freeze seems a bit dangerous as long as there are discussions going on.
> (I just started looking into the scsi feature mail thread - no opinion yet)
>
> Christian
As long as it's a bugfix, and it's not dangerous, we can merge it for
2.4. Whether the specific patch is dangerous needs to be argued
on a case by case basis.
I think merging 1.0 patches late in release cycle might be
ok simply because 1.0 is disabled by default.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > Untested - consider this pseudo-code - it just seems easier to write it
> > in C than try to explain it.
> >
> > include/hw/compat.h | 22 +++++++++++++++++++++-
> > include/hw/virtio/virtio.h | 4 +++-
> > hw/block/virtio-blk.c | 1 +
> > hw/net/virtio-net.c | 2 --
> > hw/scsi/virtio-scsi.c | 2 --
> > 5 files changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 4a43466..94c8097 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -2,7 +2,27 @@
> > #define HW_COMPAT_H
> >
> > #define HW_COMPAT_2_3 \
> > - /* empty */
> > + {\
> > + .driver = "virtio-blk-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-balloon-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-serial-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-9p-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-rng-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },
> >
> > #define HW_COMPAT_2_2 \
> > /* empty */
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index 473fb75..fbb3c06 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -214,7 +214,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> > DEFINE_PROP_BIT64("event_idx", _state, _field, \
> > VIRTIO_RING_F_EVENT_IDX, true), \
> > DEFINE_PROP_BIT64("notify_on_empty", _state, _field, \
> > - VIRTIO_F_NOTIFY_ON_EMPTY, true)
> > + VIRTIO_F_NOTIFY_ON_EMPTY, true) \
> > + DEFINE_PROP_BIT64("any_layout", _state, _field, \
> > + VIRTIO_F_ANY_LAYOUT, true)
> >
> > hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> > hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 6aefda4..015b9b5 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -731,6 +731,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features)
> > virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
> > virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
> > virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
> > + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT);
> >
> > if (s->conf.config_wce) {
> > virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 304d3dd..e203058 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -1777,8 +1777,6 @@ static void virtio_net_instance_init(Object *obj)
> > }
> >
> > static Property virtio_net_properties[] = {
> > - DEFINE_PROP_BIT("any_layout", VirtIONet, host_features,
> > - VIRTIO_F_ANY_LAYOUT, true),
> > DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
> > DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
> > VIRTIO_NET_F_GUEST_CSUM, true),
> > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> > index f7d3c7c..d17698d 100644
> > --- a/hw/scsi/virtio-scsi.c
> > +++ b/hw/scsi/virtio-scsi.c
> > @@ -953,8 +953,6 @@ static Property virtio_scsi_properties[] = {
> > 0xFFFF),
> > DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
> > 128),
> > - DEFINE_PROP_BIT("any_layout", VirtIOSCSI, host_features,
> > - VIRTIO_F_ANY_LAYOUT, true),
> > DEFINE_PROP_BIT("hotplug", VirtIOSCSI, host_features,
> > VIRTIO_SCSI_F_HOTPLUG, true),
> > DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,
> >
I just noticed that scsi uses
DEFINE_PROP_BIT for feature bits. That's also a bug
(should be DEFINE_PROP_BIT64).
My patch fixes any_layout but we need to fix the rest.
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core
2015-07-23 8:14 ` Jason Wang
@ 2015-07-23 14:39 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2015-07-23 14:39 UTC (permalink / raw)
To: Jason Wang
Cc: Kevin Wolf, Paolo Bonzini, qemu-block, qemu-devel,
Stefan Hajnoczi
On Thu, Jul 23, 2015 at 04:14:36PM +0800, Jason Wang wrote:
>
>
> On 07/22/2015 05:36 PM, Michael S. Tsirkin wrote:
> > Virtio 1 requires this,
>
> I think you mean transitional not virtio 1?
>
> > and all devices are clean by now,
> > so let's do it!
> >
> > Exceptions:
> > - virtio-blk
> > - compat machine types
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > Untested - consider this pseudo-code - it just seems easier to write it
> > in C than try to explain it.
> >
> > include/hw/compat.h | 22 +++++++++++++++++++++-
> > include/hw/virtio/virtio.h | 4 +++-
> > hw/block/virtio-blk.c | 1 +
> > hw/net/virtio-net.c | 2 --
> > hw/scsi/virtio-scsi.c | 2 --
> > 5 files changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 4a43466..94c8097 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -2,7 +2,27 @@
> > #define HW_COMPAT_H
> >
> > #define HW_COMPAT_2_3 \
> > - /* empty */
> > + {\
> > + .driver = "virtio-blk-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-balloon-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },{\
> > + .driver = "virtio-serial-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
>
> In send_control_msg() it has
>
> ...
> if (!virtqueue_pop(vq, &elem)) {
> return 0;
> }
>
> memcpy(elem.in_sg[0].iov_base, buf, len);
> ...
>
> So looks like serial is not ready for any layout.
>
> > + },{\
> > + .driver = "virtio-9p-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
>
> In handle_9p_output() it has
>
> ...
> BUG_ON(pdu->elem.out_sg[0].iov_len < 7);
> ...
>
> So looks like 9p does not support any layout at least.
I guess we could add code to disable virtio 1 for these two.
But it seems easier to fix them.
> > + },{\
> > + .driver = "virtio-rng-pci",\
> > + .property = "any_layout",\
> > + .value = "off",\
> > + },
> >
> > #define HW_COMPAT_2_2 \
> > /* empty */
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index 473fb75..fbb3c06 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -214,7 +214,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
> > DEFINE_PROP_BIT64("event_idx", _state, _field, \
> > VIRTIO_RING_F_EVENT_IDX, true), \
> > DEFINE_PROP_BIT64("notify_on_empty", _state, _field, \
> > - VIRTIO_F_NOTIFY_ON_EMPTY, true)
> > + VIRTIO_F_NOTIFY_ON_EMPTY, true) \
> > + DEFINE_PROP_BIT64("any_layout", _state, _field, \
> > + VIRTIO_F_ANY_LAYOUT, true)
> >
> > hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> > hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 6aefda4..015b9b5 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -731,6 +731,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features)
> > virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
> > virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
> > virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
> > + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT);
> >
> > if (s->conf.config_wce) {
> > virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 304d3dd..e203058 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -1777,8 +1777,6 @@ static void virtio_net_instance_init(Object *obj)
> > }
> >
> > static Property virtio_net_properties[] = {
> > - DEFINE_PROP_BIT("any_layout", VirtIONet, host_features,
> > - VIRTIO_F_ANY_LAYOUT, true),
> > DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true),
> > DEFINE_PROP_BIT("guest_csum", VirtIONet, host_features,
> > VIRTIO_NET_F_GUEST_CSUM, true),
> > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> > index f7d3c7c..d17698d 100644
> > --- a/hw/scsi/virtio-scsi.c
> > +++ b/hw/scsi/virtio-scsi.c
> > @@ -953,8 +953,6 @@ static Property virtio_scsi_properties[] = {
> > 0xFFFF),
> > DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI, parent_obj.conf.cmd_per_lun,
> > 128),
> > - DEFINE_PROP_BIT("any_layout", VirtIOSCSI, host_features,
> > - VIRTIO_F_ANY_LAYOUT, true),
> > DEFINE_PROP_BIT("hotplug", VirtIOSCSI, host_features,
> > VIRTIO_SCSI_F_HOTPLUG, true),
> > DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-23 14:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-22 9:36 [Qemu-devel] [PATCH RFC] virtio: set any_layout in virtio core Michael S. Tsirkin
2015-07-23 8:14 ` Jason Wang
2015-07-23 14:39 ` Michael S. Tsirkin
2015-07-23 10:15 ` Christian Borntraeger
2015-07-23 14:37 ` Michael S. Tsirkin
2015-07-23 10:43 ` Cornelia Huck
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).