Re: [Qemu-devel] [PATCH v5 11/14] virtio-crypto: emulate virtio crypto as a legacy device by default

2016-10-09 Thread Gonglei (Arei)
Hi Michael,

Happy to listen to your voice :)


> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Monday, October 10, 2016 7:48 AM
> Subject: Re: [PATCH v5 11/14] virtio-crypto: emulate virtio crypto as a legacy
> device by default
> 
> On Thu, Oct 06, 2016 at 07:36:44PM +0800, Gonglei wrote:
> > the scenario of virtio crypto device is mostly NFV, which require
> > the existing Guest can't need to do any changes to support virtio
> > crypto, so that they can easily migrate the existing network units
> > to VM. That's also a basic requirement came from our customers
> > in production environment.
> >
> > For virtio crypto driver, we can both support virtio-1.0 or earlier. But
> > Virtio pci driver module can't discovery the virtio-1.0 devices in most
> > existing Guests. If we want do this, we have to require the customers
> > change the virtio pci module for existing guests influence all virtio
> > devices, which is impossible.
> >
> > So, let's emulate the virtio crypto device as a legacy virtio
> > device by default. Using 0x1014 as virtio crypto pci device ID
> > because virtio crypto ID is 20 (0x14).
> >
> > Signed-off-by: Gonglei 
> 
> Sorry, I don't think this makes any sense.
> 
> You certainly can have two drivers: one for legacy and one for modern
> devices. 

Oh, This is indeed a solution.

> It only gets difficult if you try to support transitional
> devices.
> 
> Pls drop this patch.
> 
OK, then I have to temporarily drop the patch 12/14 as well because libqtest
doesn't support virtio-1.0 device yet. 


Regards,
-Gonglei

> > ---
> >  docs/specs/pci-ids.txt| 2 ++
> >  hw/virtio/virtio-crypto-pci.c | 4 +++-
> >  include/hw/pci/pci.h  | 2 ++
> >  3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> > index fd27c67..662d4f8 100644
> > --- a/docs/specs/pci-ids.txt
> > +++ b/docs/specs/pci-ids.txt
> > @@ -22,6 +22,7 @@ maintained as part of the virtio specification.
> >  1af4:1004  SCSI host bus adapter device (legacy)
> >  1af4:1005  entropy generator device (legacy)
> >  1af4:1009  9p filesystem device (legacy)
> > +1af4:1014  crypto device (legacy)
> >
> >  1af4:1041  network device (modern)
> >  1af4:1042  block device (modern)
> > @@ -32,6 +33,7 @@ maintained as part of the virtio specification.
> >  1af4:1049  9p filesystem device (modern)
> >  1af4:1050  virtio gpu device (modern)
> >  1af4:1052  virtio input device (modern)
> > +1af4:1054  crypto device (modern)
> >
> >  1af4:10f0  Available for experimental usage without registration.  Must
> get
> > to  official ID when the code leaves the test lab (i.e. when seeking
> > diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c
> > index 21d9984..30c10f0 100644
> > --- a/hw/virtio/virtio-crypto-pci.c
> > +++ b/hw/virtio/virtio-crypto-pci.c
> > @@ -32,7 +32,6 @@ static void virtio_crypto_pci_realize(VirtIOPCIProxy
> *vpci_dev, Error **errp)
> >  DeviceState *vdev = DEVICE(&vcrypto->vdev);
> >
> >  qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> > -virtio_pci_force_virtio_1(vpci_dev);
> >  object_property_set_bool(OBJECT(vdev), true, "realized", errp);
> >  object_property_set_link(OBJECT(vcrypto),
> >   OBJECT(vcrypto->vdev.conf.cryptodev), "cryptodev",
> > @@ -49,6 +48,9 @@ static void virtio_crypto_pci_class_init(ObjectClass
> *klass, void *data)
> >  set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> >  dc->props = virtio_crypto_pci_properties;
> >
> > +pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> > +pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_CRYPTO;
> > +pcidev_k->revision = 0;
> >  pcidev_k->class_id = PCI_CLASS_OTHERS;
> >  }
> >
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 772692f..5881101 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -83,6 +83,8 @@
> >  #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
> >  #define PCI_DEVICE_ID_VIRTIO_9P  0x1009
> >  #define PCI_DEVICE_ID_VIRTIO_VSOCK   0x1012
> > +#define PCI_DEVICE_ID_VIRTIO_CRYPTO  0x1014
> > +
> >
> >  #define PCI_VENDOR_ID_REDHAT 0x1b36
> >  #define PCI_DEVICE_ID_REDHAT_BRIDGE  0x0001
> > --
> > 1.7.12.4
> >



Re: [Qemu-devel] [PATCH v5 11/14] virtio-crypto: emulate virtio crypto as a legacy device by default

2016-10-09 Thread Michael S. Tsirkin
On Thu, Oct 06, 2016 at 07:36:44PM +0800, Gonglei wrote:
> the scenario of virtio crypto device is mostly NFV, which require
> the existing Guest can't need to do any changes to support virtio
> crypto, so that they can easily migrate the existing network units
> to VM. That's also a basic requirement came from our customers
> in production environment.
> 
> For virtio crypto driver, we can both support virtio-1.0 or earlier. But
> Virtio pci driver module can't discovery the virtio-1.0 devices in most
> existing Guests. If we want do this, we have to require the customers
> change the virtio pci module for existing guests influence all virtio
> devices, which is impossible.
>
> So, let's emulate the virtio crypto device as a legacy virtio
> device by default. Using 0x1014 as virtio crypto pci device ID
> because virtio crypto ID is 20 (0x14).
> 
> Signed-off-by: Gonglei 

Sorry, I don't think this makes any sense.

You certainly can have two drivers: one for legacy and one for modern
devices.  It only gets difficult if you try to support transitional
devices.

Pls drop this patch.

> ---
>  docs/specs/pci-ids.txt| 2 ++
>  hw/virtio/virtio-crypto-pci.c | 4 +++-
>  include/hw/pci/pci.h  | 2 ++
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
> index fd27c67..662d4f8 100644
> --- a/docs/specs/pci-ids.txt
> +++ b/docs/specs/pci-ids.txt
> @@ -22,6 +22,7 @@ maintained as part of the virtio specification.
>  1af4:1004  SCSI host bus adapter device (legacy)
>  1af4:1005  entropy generator device (legacy)
>  1af4:1009  9p filesystem device (legacy)
> +1af4:1014  crypto device (legacy)
>  
>  1af4:1041  network device (modern)
>  1af4:1042  block device (modern)
> @@ -32,6 +33,7 @@ maintained as part of the virtio specification.
>  1af4:1049  9p filesystem device (modern)
>  1af4:1050  virtio gpu device (modern)
>  1af4:1052  virtio input device (modern)
> +1af4:1054  crypto device (modern)
>  
>  1af4:10f0  Available for experimental usage without registration.  Must get
> to  official ID when the code leaves the test lab (i.e. when seeking
> diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c
> index 21d9984..30c10f0 100644
> --- a/hw/virtio/virtio-crypto-pci.c
> +++ b/hw/virtio/virtio-crypto-pci.c
> @@ -32,7 +32,6 @@ static void virtio_crypto_pci_realize(VirtIOPCIProxy 
> *vpci_dev, Error **errp)
>  DeviceState *vdev = DEVICE(&vcrypto->vdev);
>  
>  qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> -virtio_pci_force_virtio_1(vpci_dev);
>  object_property_set_bool(OBJECT(vdev), true, "realized", errp);
>  object_property_set_link(OBJECT(vcrypto),
>   OBJECT(vcrypto->vdev.conf.cryptodev), "cryptodev",
> @@ -49,6 +48,9 @@ static void virtio_crypto_pci_class_init(ObjectClass 
> *klass, void *data)
>  set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>  dc->props = virtio_crypto_pci_properties;
>  
> +pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> +pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_CRYPTO;
> +pcidev_k->revision = 0;
>  pcidev_k->class_id = PCI_CLASS_OTHERS;
>  }
>  
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 772692f..5881101 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -83,6 +83,8 @@
>  #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
>  #define PCI_DEVICE_ID_VIRTIO_9P  0x1009
>  #define PCI_DEVICE_ID_VIRTIO_VSOCK   0x1012
> +#define PCI_DEVICE_ID_VIRTIO_CRYPTO  0x1014
> +
>  
>  #define PCI_VENDOR_ID_REDHAT 0x1b36
>  #define PCI_DEVICE_ID_REDHAT_BRIDGE  0x0001
> -- 
> 1.7.12.4
> 



[Qemu-devel] [PATCH v5 11/14] virtio-crypto: emulate virtio crypto as a legacy device by default

2016-10-06 Thread Gonglei
the scenario of virtio crypto device is mostly NFV, which require
the existing Guest can't need to do any changes to support virtio
crypto, so that they can easily migrate the existing network units
to VM. That's also a basic requirement came from our customers
in production environment.

For virtio crypto driver, we can both support virtio-1.0 or earlier. But
Virtio pci driver module can't discovery the virtio-1.0 devices in most
existing Guests. If we want do this, we have to require the customers
change the virtio pci module for existing guests influence all virtio
devices, which is impossible.

So, let's emulate the virtio crypto device as a legacy virtio
device by default. Using 0x1014 as virtio crypto pci device ID
because virtio crypto ID is 20 (0x14).

Signed-off-by: Gonglei 
---
 docs/specs/pci-ids.txt| 2 ++
 hw/virtio/virtio-crypto-pci.c | 4 +++-
 include/hw/pci/pci.h  | 2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
index fd27c67..662d4f8 100644
--- a/docs/specs/pci-ids.txt
+++ b/docs/specs/pci-ids.txt
@@ -22,6 +22,7 @@ maintained as part of the virtio specification.
 1af4:1004  SCSI host bus adapter device (legacy)
 1af4:1005  entropy generator device (legacy)
 1af4:1009  9p filesystem device (legacy)
+1af4:1014  crypto device (legacy)
 
 1af4:1041  network device (modern)
 1af4:1042  block device (modern)
@@ -32,6 +33,7 @@ maintained as part of the virtio specification.
 1af4:1049  9p filesystem device (modern)
 1af4:1050  virtio gpu device (modern)
 1af4:1052  virtio input device (modern)
+1af4:1054  crypto device (modern)
 
 1af4:10f0  Available for experimental usage without registration.  Must get
to  official ID when the code leaves the test lab (i.e. when seeking
diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c
index 21d9984..30c10f0 100644
--- a/hw/virtio/virtio-crypto-pci.c
+++ b/hw/virtio/virtio-crypto-pci.c
@@ -32,7 +32,6 @@ static void virtio_crypto_pci_realize(VirtIOPCIProxy 
*vpci_dev, Error **errp)
 DeviceState *vdev = DEVICE(&vcrypto->vdev);
 
 qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
-virtio_pci_force_virtio_1(vpci_dev);
 object_property_set_bool(OBJECT(vdev), true, "realized", errp);
 object_property_set_link(OBJECT(vcrypto),
  OBJECT(vcrypto->vdev.conf.cryptodev), "cryptodev",
@@ -49,6 +48,9 @@ static void virtio_crypto_pci_class_init(ObjectClass *klass, 
void *data)
 set_bit(DEVICE_CATEGORY_MISC, dc->categories);
 dc->props = virtio_crypto_pci_properties;
 
+pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_CRYPTO;
+pcidev_k->revision = 0;
 pcidev_k->class_id = PCI_CLASS_OTHERS;
 }
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 772692f..5881101 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -83,6 +83,8 @@
 #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
 #define PCI_DEVICE_ID_VIRTIO_9P  0x1009
 #define PCI_DEVICE_ID_VIRTIO_VSOCK   0x1012
+#define PCI_DEVICE_ID_VIRTIO_CRYPTO  0x1014
+
 
 #define PCI_VENDOR_ID_REDHAT 0x1b36
 #define PCI_DEVICE_ID_REDHAT_BRIDGE  0x0001
-- 
1.7.12.4