Re: [PATCH 2/2] virtio_ring: fix complaint by sparse

2016-11-22 Thread Cornelia Huck 
On Tue, 22 Nov 2016 13:51:50 +0800
Gonglei  wrote:

>  # make C=2 CF="-D__CHECK_ENDIAN__" ./drivers/virtio/
> 
> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
> (different base types)
> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
> [assigned] i
> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
> next
> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
> (different base types)
> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
> [assigned] i
> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
> next
> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
> (different base types)
> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
> [assigned] i
> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
> next
> drivers/virtio/virtio_ring.c:604:39: warning: incorrect type in initializer 
> (different base types)
> drivers/virtio/virtio_ring.c:604:39:expected unsigned short [unsigned] 
> [usertype] nextflag
> drivers/virtio/virtio_ring.c:604:39:got restricted __virtio16
> drivers/virtio/virtio_ring.c:612:33: warning: restricted __virtio16 degrades 
> to integer
> 
> Signed-off-by: Gonglei 
> ---
>  drivers/virtio/virtio_ring.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/2] virtio_ring: fix complaint by sparse

2016-11-22 Thread Andy Lutomirski 
On Tue, Nov 22, 2016 at 7:16 AM, Thomas Huth  wrote:
> On 22.11.2016 16:04, Michael S. Tsirkin wrote:
>> On Tue, Nov 22, 2016 at 01:51:50PM +0800, Gonglei wrote:
>>>  # make C=2 CF="-D__CHECK_ENDIAN__" ./drivers/virtio/
>>>
>>> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
>>> (different base types)
>>> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
>>> [assigned] i
>>> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 
>>> [usertype] next
>>> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
>>> (different base types)
>>> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
>>> [assigned] i
>>> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 
>>> [usertype] next
>>> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
>>> (different base types)
>>> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
>>> [assigned] i
>>> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 
>>> [usertype] next
>>> drivers/virtio/virtio_ring.c:604:39: warning: incorrect type in initializer 
>>> (different base types)
>>> drivers/virtio/virtio_ring.c:604:39:expected unsigned short [unsigned] 
>>> [usertype] nextflag
>>> drivers/virtio/virtio_ring.c:604:39:got restricted __virtio16
>>> drivers/virtio/virtio_ring.c:612:33: warning: restricted __virtio16 
>>> degrades to integer
>>>
>>> Signed-off-by: Gonglei 
>>> ---
>>>  drivers/virtio/virtio_ring.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index 489bfc6..d2863c3 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -420,7 +420,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
>>>  if (i == err_idx)
>>>  break;
>>>  vring_unmap_one(vq, [i]);
>>> -i = vq->vring.desc[i].next;
>>> +i = virtio16_to_cpu(_vq->vdev, vq->vring.desc[i].next);
>>>  }
>>>
>>>  vq->vq.num_free += total_sg;
> [...]
>> Wow are you saying endian-ness is all wrong for the next field?
>> How do things ever work then?
>
> The above code is only in the error cleanup path (after the
> "unmap_release" label, introduced by commit 780bc7903), so it likely has
> never been exercised in the field.
> I think Gonlei's patch is right, there should be a virtio16_to_cpu() here.

Agreed.

>
>  Thomas
>
>



-- 
Andy Lutomirski
AMA Capital Management, LLC
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/2] virtio_ring: fix complaint by sparse

2016-11-22 Thread Thomas Huth 
On 22.11.2016 16:04, Michael S. Tsirkin wrote:
> On Tue, Nov 22, 2016 at 01:51:50PM +0800, Gonglei wrote:
>>  # make C=2 CF="-D__CHECK_ENDIAN__" ./drivers/virtio/
>>
>> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
>> (different base types)
>> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
>> [assigned] i
>> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
>> next
>> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
>> (different base types)
>> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
>> [assigned] i
>> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
>> next
>> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
>> (different base types)
>> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
>> [assigned] i
>> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
>> next
>> drivers/virtio/virtio_ring.c:604:39: warning: incorrect type in initializer 
>> (different base types)
>> drivers/virtio/virtio_ring.c:604:39:expected unsigned short [unsigned] 
>> [usertype] nextflag
>> drivers/virtio/virtio_ring.c:604:39:got restricted __virtio16
>> drivers/virtio/virtio_ring.c:612:33: warning: restricted __virtio16 degrades 
>> to integer
>>
>> Signed-off-by: Gonglei 
>> ---
>>  drivers/virtio/virtio_ring.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> index 489bfc6..d2863c3 100644
>> --- a/drivers/virtio/virtio_ring.c
>> +++ b/drivers/virtio/virtio_ring.c
>> @@ -420,7 +420,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
>>  if (i == err_idx)
>>  break;
>>  vring_unmap_one(vq, [i]);
>> -i = vq->vring.desc[i].next;
>> +i = virtio16_to_cpu(_vq->vdev, vq->vring.desc[i].next);
>>  }
>>  
>>  vq->vq.num_free += total_sg;
[...]
> Wow are you saying endian-ness is all wrong for the next field?
> How do things ever work then?

The above code is only in the error cleanup path (after the
"unmap_release" label, introduced by commit 780bc7903), so it likely has
never been exercised in the field.
I think Gonlei's patch is right, there should be a virtio16_to_cpu() here.

 Thomas


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/2] virtio_ring: fix complaint by sparse

2016-11-22 Thread Michael S. Tsirkin 
On Tue, Nov 22, 2016 at 01:51:50PM +0800, Gonglei wrote:
>  # make C=2 CF="-D__CHECK_ENDIAN__" ./drivers/virtio/
> 
> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
> (different base types)
> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
> [assigned] i
> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
> next
> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
> (different base types)
> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
> [assigned] i
> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
> next
> drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
> (different base types)
> drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
> [assigned] i
> drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
> next
> drivers/virtio/virtio_ring.c:604:39: warning: incorrect type in initializer 
> (different base types)
> drivers/virtio/virtio_ring.c:604:39:expected unsigned short [unsigned] 
> [usertype] nextflag
> drivers/virtio/virtio_ring.c:604:39:got restricted __virtio16
> drivers/virtio/virtio_ring.c:612:33: warning: restricted __virtio16 degrades 
> to integer
> 
> Signed-off-by: Gonglei 
> ---
>  drivers/virtio/virtio_ring.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 489bfc6..d2863c3 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -420,7 +420,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
>   if (i == err_idx)
>   break;
>   vring_unmap_one(vq, [i]);
> - i = vq->vring.desc[i].next;
> + i = virtio16_to_cpu(_vq->vdev, vq->vring.desc[i].next);
>   }
>  
>   vq->vq.num_free += total_sg;
> @@ -601,7 +601,7 @@ bool virtqueue_kick(struct virtqueue *vq)
>  static void detach_buf(struct vring_virtqueue *vq, unsigned int head)
>  {
>   unsigned int i, j;
> - u16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
> + __virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
>  
>   /* Clear data ptr. */
>   vq->desc_state[head].data = NULL;
> -- 
> 1.8.3.1
> 

Wow are you saying endian-ness is all wrong for the next field?
How do things ever work then?

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v2 0/2] virtio-crypto: add Linux driver

2016-11-22 Thread Gonglei 
The virtio crypto device is a virtual cryptography device
as well as a kind of virtual hardware accelerator for
virtual machines. The encryption anddecryption requests
are placed in the data queue and are ultimately handled by
thebackend crypto accelerators. The second queue is the
control queue used to create or destroy sessions for
symmetric algorithms and will control some advanced features
in the future. The virtio crypto device provides the following
cryptoservices: CIPHER, MAC, HASH, and AEAD.

For more information about virtio-crypto device, please see:
  http://qemu-project.org/Features/VirtioCrypto

For better reviewing:

Patch 1 introduces the little edian functions for VIRTIO_1 
devices.

Patch 2 mainly includes five files:
 1) virtio_crypto.h is the header file for virtio-crypto device,
which is based on the virtio-crypto specification. 
 2) virtio_crypto.c is the entry of the driver module,
which is similar with other virtio devices, such as virtio-net,
virtio-input etc. 
 3) virtio_crypto_mgr.c is used to manage the virtio
crypto devices in the system. We support up to 32 virtio-crypto
devices currently. I use a global list to store the virtio crypto
devices which refer to Intel QAT driver. Meanwhile, the file
includs the functions of add/del/search/start/stop for virtio
crypto devices.
 4) virtio_crypto_common.h is a private header file for virtio
crypto driver, includes structure definations, and function declarations.
 5) virtio_crypto_algs.c is the realization of algs based on Linux Crypto 
Framwork,
which can register different crypto algorithms. Currently it's only support 
AES-CBC.
The Crypto guys can mainly focus to this file. 

Actually I have no idea the virtio-crypto driver should be gone in whose
tree, Michael's or Herbert's?

Would you give me a feedback? Thanks a lot!


v2:
 - stop doing DMA from the stack, CONFIG_VMAP_STACK=y [Salvatore]
 - convert __virtio32/64 to __le32/64 in virtio_crypto.h
 - remove VIRTIO_CRYPTO_S_STARTED based on the lastest virtio crypto spec.
 - introduces the little edian functions for VIRTIO_1 devices in patch 1.

Gonglei (2):
  virtio: introduce little edian functions for virtio_cread/write#
family
  crypto: add virtio-crypto driver

 MAINTAINERS  |   8 +
 drivers/crypto/Kconfig   |   2 +
 drivers/crypto/Makefile  |   1 +
 drivers/crypto/virtio/Kconfig|  10 +
 drivers/crypto/virtio/Makefile   |   5 +
 drivers/crypto/virtio/virtio_crypto.c| 444 +++
 drivers/crypto/virtio/virtio_crypto_algs.c   | 524 +++
 drivers/crypto/virtio/virtio_crypto_common.h | 124 +++
 drivers/crypto/virtio/virtio_crypto_mgr.c| 258 +
 include/linux/virtio_config.h|  45 +++
 include/uapi/linux/Kbuild|   1 +
 include/uapi/linux/virtio_crypto.h   | 435 ++
 include/uapi/linux/virtio_ids.h  |   1 +
 13 files changed, 1858 insertions(+)
 create mode 100644 drivers/crypto/virtio/Kconfig
 create mode 100644 drivers/crypto/virtio/Makefile
 create mode 100644 drivers/crypto/virtio/virtio_crypto.c
 create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c
 create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h
 create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c
 create mode 100644 include/uapi/linux/virtio_crypto.h

-- 
1.8.3.1


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v2 2/2] crypto: add virtio-crypto driver

2016-11-22 Thread Gonglei 
This patch introduces virtio-crypto driver for Linux Kernel.

The virtio crypto device is a virtual cryptography device
as well as a kind of virtual hardware accelerator for
virtual machines. The encryption anddecryption requests
are placed in the data queue and are ultimately handled by
thebackend crypto accelerators. The second queue is the
control queue used to create or destroy sessions for
symmetric algorithms and will control some advanced features
in the future. The virtio crypto device provides the following
cryptoservices: CIPHER, MAC, HASH, and AEAD.

For more information about virtio-crypto device, please see:
  http://qemu-project.org/Features/VirtioCrypto

CC: Michael S. Tsirkin 
CC: Cornelia Huck 
CC: Stefan Hajnoczi 
CC: Herbert Xu 
CC: Halil Pasic 
CC: David S. Miller 
CC: Zeng Xin 
Signed-off-by: Gonglei 
---
 MAINTAINERS  |   8 +
 drivers/crypto/Kconfig   |   2 +
 drivers/crypto/Makefile  |   1 +
 drivers/crypto/virtio/Kconfig|  10 +
 drivers/crypto/virtio/Makefile   |   5 +
 drivers/crypto/virtio/virtio_crypto.c| 444 +++
 drivers/crypto/virtio/virtio_crypto_algs.c   | 524 +++
 drivers/crypto/virtio/virtio_crypto_common.h | 124 +++
 drivers/crypto/virtio/virtio_crypto_mgr.c| 258 +
 include/uapi/linux/Kbuild|   1 +
 include/uapi/linux/virtio_crypto.h   | 435 ++
 include/uapi/linux/virtio_ids.h  |   1 +
 12 files changed, 1813 insertions(+)
 create mode 100644 drivers/crypto/virtio/Kconfig
 create mode 100644 drivers/crypto/virtio/Makefile
 create mode 100644 drivers/crypto/virtio/virtio_crypto.c
 create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c
 create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h
 create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c
 create mode 100644 include/uapi/linux/virtio_crypto.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 411e3b8..d6b18bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12844,6 +12844,14 @@ S: Maintained
 F: drivers/virtio/virtio_input.c
 F: include/uapi/linux/virtio_input.h
 
+VIRTIO CRYPTO DRIVER
+M:  Gonglei 
+L:  virtualization@lists.linux-foundation.org
+L:  linux-cry...@vger.kernel.org
+S:  Maintained
+F:  drivers/crypto/virtio/
+F:  include/uapi/linux/virtio_crypto.h
+
 VIA RHINE NETWORK DRIVER
 S: Orphan
 F: drivers/net/ethernet/via/via-rhine.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f..7956478 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -555,4 +555,6 @@ config CRYPTO_DEV_ROCKCHIP
 
 source "drivers/crypto/chelsio/Kconfig"
 
+source "drivers/crypto/virtio/Kconfig"
+
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index ad7250f..bc53cb8 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
 obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
 obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
 obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
+obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/
diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig
new file mode 100644
index 000..ceae88c
--- /dev/null
+++ b/drivers/crypto/virtio/Kconfig
@@ -0,0 +1,10 @@
+config CRYPTO_DEV_VIRTIO
+   tristate "VirtIO crypto driver"
+   depends on VIRTIO
+select CRYPTO_AEAD
+select CRYPTO_AUTHENC
+select CRYPTO_BLKCIPHER
+   default m
+   help
+ This driver provides support for virtio crypto device. If you
+ choose 'M' here, this module will be called virtio-crypto.
diff --git a/drivers/crypto/virtio/Makefile b/drivers/crypto/virtio/Makefile
new file mode 100644
index 000..a316e92
--- /dev/null
+++ b/drivers/crypto/virtio/Makefile
@@ -0,0 +1,5 @@
+obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio-crypto.o
+virtio-crypto-objs := \
+   virtio_crypto_algs.o \
+   virtio_crypto_mgr.o \
+   virtio_crypto.o
diff --git a/drivers/crypto/virtio/virtio_crypto.c 
b/drivers/crypto/virtio/virtio_crypto.c
new file mode 100644
index 000..56fdfed
--- /dev/null
+++ b/drivers/crypto/virtio/virtio_crypto.c
@@ -0,0 +1,444 @@
+ /* Driver for Virtio crypto device.
+  *
+  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
+  *
+  * This program is free software; you can redistribute it and/or modify
+  * it under the terms of the GNU General Public License as published by
+  * the Free Software Foundation; either version 2 of the License, or
+  * (at your option) any later version.
+  *
+  * This program is distributed in the hope that it will be useful,
+  * but WITHOUT ANY WARRANTY;

[PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family

2016-11-22 Thread Gonglei 
Virtio modern devices are always little edian, let's introduce
the LE functions for read/write configuration space for
virtio modern devices, which avoid complaint by Sparse when
we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.

Signed-off-by: Gonglei 
---
 include/linux/virtio_config.h | 45 +++
 1 file changed, 45 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 26c155b..de05707 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct virtio_device 
*vdev,
_r; \
})
 
+static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
+unsigned int offset)
+{
+   __le16 ret;
+
+   vdev->config->get(vdev, offset, , sizeof(ret));
+   return ret;
+}
+
+static inline void virtio_cwrite16_le(struct virtio_device *vdev,
+  unsigned int offset, __le16 val)
+{
+   vdev->config->set(vdev, offset, , sizeof(val));
+}
+
+static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
+unsigned int offset)
+{
+   __le32 ret;
+
+   vdev->config->get(vdev, offset, , sizeof(ret));
+   return ret;
+}
+
+static inline void virtio_cwrite32_le(struct virtio_device *vdev,
+  unsigned int offset, __le32 val)
+{
+   vdev->config->set(vdev, offset, , sizeof(val));
+}
+
+static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
+unsigned int offset)
+{
+   __le64 ret;
+
+   __virtio_cread_many(vdev, offset, , 1, sizeof(ret));
+   return ret;
+}
+
+static inline void virtio_cwrite64_le(struct virtio_device *vdev,
+  unsigned int offset, __le64 val)
+{
+   vdev->config->set(vdev, offset, , sizeof(val));
+}
+
 #endif /* _LINUX_VIRTIO_CONFIG_H */
-- 
1.8.3.1


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization