Linux Virtualization - Nov 2014 - [PATCH v4 04/42] virtio: disable virtio 1.0 in transports

disable virtio 1.0 in transports that don't
support it yet.
We will gradually re-enable as support is added.

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
 drivers/lguest/lguest_device.c     | 3 ++-
 drivers/misc/mic/card/mic_virtio.c | 2 ++
 drivers/s390/kvm/virtio_ccw.c      | 3 ++-
 drivers/virtio/virtio_mmio.c       | 2 ++
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 4d29bcd..4deaf88 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -106,7 +106,8 @@ static u64 lg_get_features(struct virtio_device *vdev)
 		if (in_features[i / 8] & (1 << (i % 8)))
 			features |= (1ULL << i);
 
-	return features;
+	/* lguest is not in virtio 1.0 */
+	return features & ~BIT_ULL(VIRTIO_F_VERSION_1);
 }
 
 /*
diff --git a/drivers/misc/mic/card/mic_virtio.c
b/drivers/misc/mic/card/mic_virtio.c
index 6d94f04..edc77f1 100644
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -82,6 +82,8 @@ static u64 mic_get_features(struct virtio_device *vdev)
 		if (ioread8(&in_features[i / 8]) & (BIT_ULL(i % 8)))
 			features |= BIT_ULL(i);
 
+	/* MIC is not in virtio 1.0, disable it for now. */
+	features &= ~BIT_ULL(VIRTIO_F_VERSION_1);
 	return features;
 }
 
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index abba04d..08536f0 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -704,7 +704,8 @@ static u64 virtio_ccw_get_features(struct virtio_device
*vdev)
 out_free:
 	kfree(features);
 	kfree(ccw);
-	return rc;
+	/* TODO: enable virtio 1.0 */
+	return rc & ~BIT_ULL(VIRTIO_F_VERSION_1);;
 }
 
 static void virtio_ccw_finalize_features(struct virtio_device *vdev)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index fd01c6d..e1d38a9 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -151,6 +151,8 @@ static u64 vm_get_features(struct virtio_device *vdev)
 	features = readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES);
 	writel(1, vm_dev->base + VIRTIO_MMIO_HOST_FEATURES_SEL);
 	features |= ((u64)readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES) <<
32);
+	/* TODO: enable virtio 1.0 support */
+	features &= ~BIT_ULL(VIRTIO_F_VERSION_1);
 	return features;
 }
 
-- 
MST

On Tue, 25 Nov 2014 18:41:35 +0200
"Michael S. Tsirkin" <mst at redhat.com> wrote:
> disable virtio 1.0 in transports that don't
> support it yet.
I'd prefer if you disabled it for _every_ transport in this patch,
until the needed infrastructure is in place. Else this is a bit
confusing.
> We will gradually re-enable as support is added.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  drivers/lguest/lguest_device.c     | 3 ++-
>  drivers/misc/mic/card/mic_virtio.c | 2 ++
>  drivers/s390/kvm/virtio_ccw.c      | 3 ++-
>  drivers/virtio/virtio_mmio.c       | 2 ++
>  4 files changed, 8 insertions(+), 2 deletions(-)
Why do you disable ccw but not pci? (Doesn't pci need any changes
transport-side?) And you missed the old s390 virtio transport in
drivers/s390/kvm/kvm_virtio.c :)
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> index abba04d..08536f0 100644
> --- a/drivers/s390/kvm/virtio_ccw.c
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -704,7 +704,8 @@ static u64 virtio_ccw_get_features(struct virtio_device
*vdev)
>  out_free:
>  	kfree(features);
>  	kfree(ccw);
> -	return rc;
> +	/* TODO: enable virtio 1.0 */
> +	return rc & ~BIT_ULL(VIRTIO_F_VERSION_1);;
double ';'

FWIW, as negotiating a revision >= 1 is a pre-req for virtio 1.0
support on ccw, virtio 1.0 is already implicitly disabled.
>  }
> 
>  static void virtio_ccw_finalize_features(struct virtio_device *vdev)

On Tue, Nov 25, 2014 at 06:29:42PM +0100, Cornelia Huck
wrote:
> On Tue, 25 Nov 2014 18:41:35 +0200
> "Michael S. Tsirkin" <mst at redhat.com> wrote:
> 
> > disable virtio 1.0 in transports that don't
> > support it yet.
> 
> I'd prefer if you disabled it for _every_ transport in this patch,
> until the needed infrastructure is in place. Else this is a bit
> confusing.
Well the only transports left are pci and rpoc, and these only
read the low 32 bit of the features from the device -
so there's nothing to clear.

E.g. the following would be even more confusing, would it not:

	u32 features;
	....
	features &= ~BIT_ULL(VIRTIO_F_VERSION_1);

Agree?
> > We will gradually re-enable as support is added.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> > ---
> >  drivers/lguest/lguest_device.c     | 3 ++-
> >  drivers/misc/mic/card/mic_virtio.c | 2 ++
> >  drivers/s390/kvm/virtio_ccw.c      | 3 ++-
> >  drivers/virtio/virtio_mmio.c       | 2 ++
> >  4 files changed, 8 insertions(+), 2 deletions(-)
> 
> Why do you disable ccw but not pci? (Doesn't pci need any changes
> transport-side?)
No because the register is 32 bit wide there.
> And you missed the old s390 virtio transport in
> drivers/s390/kvm/kvm_virtio.c :)
Good catch. I can fix that for v5, but see below.
> > diff --git a/drivers/s390/kvm/virtio_ccw.c
b/drivers/s390/kvm/virtio_ccw.c
> > index abba04d..08536f0 100644
> > --- a/drivers/s390/kvm/virtio_ccw.c
> > +++ b/drivers/s390/kvm/virtio_ccw.c
> > @@ -704,7 +704,8 @@ static u64 virtio_ccw_get_features(struct
virtio_device *vdev)
> >  out_free:
> >  	kfree(features);
> >  	kfree(ccw);
> > -	return rc;
> > +	/* TODO: enable virtio 1.0 */
> > +	return rc & ~BIT_ULL(VIRTIO_F_VERSION_1);;
> 
> double ';'
> 
> FWIW, as negotiating a revision >= 1 is a pre-req for virtio 1.0
> support on ccw, virtio 1.0 is already implicitly disabled.
Ah, you mean device guarantees that VIRTIO_F_VERSION_1 isn't set
if guest sets revision to 0?
In that case it's probably best to drop this from both ccw
devices.

It is here temporarily anyway, only in order to avoid
using transitional devices incorrectly when bisecting.

> >  }
> > 
> >  static void virtio_ccw_finalize_features(struct virtio_device *vdev)