Patchwork virtio-serial: don't set MULTIPORT for 1 port dev

login
register
mail settings
Submitter Michael S. Tsirkin
Date Feb. 12, 2010, 1:42 p.m.
Message ID <20100212134214.GA4214@redhat.com>
Download mbox | patch
Permalink /patch/45182/
State New
Headers show

Comments

Michael S. Tsirkin - Feb. 12, 2010, 1:42 p.m.
Since commit 98b19252cf1bd97c54bc4613f3537c5ec0aae263, all
serial devices declare MULTIPORT feature.
To allow 0.12 compatibility, we should clear this when
max_nr_ports is 1.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio-serial-bus.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Amit Shah - Feb. 12, 2010, 2:23 p.m.
On (Fri) Feb 12 2010 [15:42:14], Michael S. Tsirkin wrote:
> Since commit 98b19252cf1bd97c54bc4613f3537c5ec0aae263, all
> serial devices declare MULTIPORT feature.
> To allow 0.12 compatibility, we should clear this when
> max_nr_ports is 1.

In addition to this, setting max_nr_ports to 1 is needed when -M 0.12 is
selected.

However, is this the only way to do it? Gerd?

> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio-serial-bus.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index ab456ea..d0e0219 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -335,8 +335,10 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
>  
>  static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
>  {
> -    features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT);
> -
> +    VirtIOSerial *vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
> +    if (vser->bus->max_nr_ports > 1) {
> +        features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT);
> +    }
>      return features;
>  }
>  
> -- 
> 1.6.6.144.g5c3af

		Amit
Gerd Hoffmann - Feb. 15, 2010, 9:03 a.m.
On 02/12/10 15:23, Amit Shah wrote:
> On (Fri) Feb 12 2010 [15:42:14], Michael S. Tsirkin wrote:
>> Since commit 98b19252cf1bd97c54bc4613f3537c5ec0aae263, all
>> serial devices declare MULTIPORT feature.
>> To allow 0.12 compatibility, we should clear this when
>> max_nr_ports is 1.
>
> In addition to this, setting max_nr_ports to 1 is needed when -M 0.12 is
> selected.

Indeed.

> However, is this the only way to do it? Gerd?

Is there a qdev property for max_nr_ports?  Then simply adding a compat 
property will do the trick.

cheers,
   Gerd
Amit Shah - Feb. 15, 2010, 1:21 p.m.
On (Fri) Feb 12 2010 [15:42:14], Michael S. Tsirkin wrote:
> Since commit 98b19252cf1bd97c54bc4613f3537c5ec0aae263, all
> serial devices declare MULTIPORT feature.
> To allow 0.12 compatibility, we should clear this when
> max_nr_ports is 1.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Amit Shah <amit.shah@redhat.com>

A couple more patches are needed to introduce a 0.12 machine type and
default to 1 max_nr_ports for machine types < 0.13, which I'll be
sending shortly.

I'll also make this patch 3/3 in that series, just for completenes..

		Amit
Michael S. Tsirkin - Feb. 15, 2010, 2:07 p.m.
On Mon, Feb 15, 2010 at 06:51:31PM +0530, Amit Shah wrote:
> On (Fri) Feb 12 2010 [15:42:14], Michael S. Tsirkin wrote:
> > Since commit 98b19252cf1bd97c54bc4613f3537c5ec0aae263, all
> > serial devices declare MULTIPORT feature.
> > To allow 0.12 compatibility, we should clear this when
> > max_nr_ports is 1.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Acked-by: Amit Shah <amit.shah@redhat.com>
> 
> A couple more patches are needed to introduce a 0.12 machine type and
> default to 1 max_nr_ports for machine types < 0.13, which I'll be
> sending shortly.

Note that we also need to fill in virtio feature bits for the rest of
devices, per machine type.  Care to deal with it?

> I'll also make this patch 3/3 in that series, just for completenes..
> 
> 		Amit
>
Amit Shah - Feb. 15, 2010, 3:36 p.m.
On (Mon) Feb 15 2010 [16:07:28], Michael S. Tsirkin wrote:
> On Mon, Feb 15, 2010 at 06:51:31PM +0530, Amit Shah wrote:
> > On (Fri) Feb 12 2010 [15:42:14], Michael S. Tsirkin wrote:
> > > Since commit 98b19252cf1bd97c54bc4613f3537c5ec0aae263, all
> > > serial devices declare MULTIPORT feature.
> > > To allow 0.12 compatibility, we should clear this when
> > > max_nr_ports is 1.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Acked-by: Amit Shah <amit.shah@redhat.com>
> > 
> > A couple more patches are needed to introduce a 0.12 machine type and
> > default to 1 max_nr_ports for machine types < 0.13, which I'll be
> > sending shortly.
> 
> Note that we also need to fill in virtio feature bits for the rest of
> devices, per machine type.  Care to deal with it?

Yes, that has to be done (and at least for s390 and ppc for
virtio-serial). I've not yet seen how they work, but they should be
similar to pc (if at all they have similar versioning support).

		Amit

Patch

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index ab456ea..d0e0219 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -335,8 +335,10 @@  static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
 
 static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
 {
-    features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT);
-
+    VirtIOSerial *vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+    if (vser->bus->max_nr_ports > 1) {
+        features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT);
+    }
     return features;
 }