[PATCH 0/2] virtio: fix complaint by sparse

[PATCH 0/2] virtio: fix complaint by sparse

[Gonglei]

I found some warnings reported by sparse in the virtio code
when I checked virtio-crypto's driver stuff. Let's fix them.

Gonglei (2):
  virtio_pci_modern: fix complaint by sparse
  virtio_ring: fix complaint by sparse

 drivers/virtio/virtio_pci_modern.c | 8 ++++----
 drivers/virtio/virtio_ring.c       | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[PATCH 0/2] virtio: fix complaint by sparse

[Gonglei]

I found some warnings reported by sparse in the virtio code
when I checked virtio-crypto's driver stuff. Let's fix them.

Gonglei (2):
  virtio_pci_modern: fix complaint by sparse
  virtio_ring: fix complaint by sparse

 drivers/virtio/virtio_pci_modern.c | 8 ++++----
 drivers/virtio/virtio_ring.c       | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] virtio_pci_modern: fix complaint by sparse

[Gonglei]

drivers/virtio/virtio_pci_modern.c:66:40: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:66:40:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:66:40:    got restricted __le32 [noderef] [usertype] <asn:2>*lo
drivers/virtio/virtio_pci_modern.c:67:33: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:67:33:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:67:33:    got restricted __le32 [noderef] [usertype] <asn:2>*hi
drivers/virtio/virtio_pci_modern.c:150:32: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:150:32:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:150:32:    got restricted __le32 [noderef] <asn:2>*<noident>
drivers/virtio/virtio_pci_modern.c:151:39: warning: incorrect type in argument 1 (different base types)
drivers/virtio/virtio_pci_modern.c:151:39:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:151:39:    got restricted __le32 [noderef] <asn:2>*<noident>
drivers/virtio/virtio_pci_modern.c:152:32: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:152:32:    expected unsigned int [noderef] [usertype] <asn:2>*addr

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 drivers/virtio/virtio_pci_modern.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index e76bd91..4bf7ab3 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -33,12 +33,12 @@ static inline u8 vp_ioread8(u8 __iomem *addr)
 {
 	return ioread8(addr);
 }
-static inline u16 vp_ioread16 (u16 __iomem *addr)
+static inline u16 vp_ioread16 (__le16 __iomem *addr)
 {
 	return ioread16(addr);
 }
 
-static inline u32 vp_ioread32(u32 __iomem *addr)
+static inline u32 vp_ioread32(__le32 __iomem *addr)
 {
 	return ioread32(addr);
 }
@@ -48,12 +48,12 @@ static inline void vp_iowrite8(u8 value, u8 __iomem *addr)
 	iowrite8(value, addr);
 }
 
-static inline void vp_iowrite16(u16 value, u16 __iomem *addr)
+static inline void vp_iowrite16(u16 value, __le16 __iomem *addr)
 {
 	iowrite16(value, addr);
 }
 
-static inline void vp_iowrite32(u32 value, u32 __iomem *addr)
+static inline void vp_iowrite32(u32 value, __le32 __iomem *addr)
 {
 	iowrite32(value, addr);
 }
-- 
1.8.3.1

[PATCH 1/2] virtio_pci_modern: fix complaint by sparse

[Gonglei]

drivers/virtio/virtio_pci_modern.c:66:40: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:66:40:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:66:40:    got restricted __le32 [noderef] [usertype] <asn:2>*lo
drivers/virtio/virtio_pci_modern.c:67:33: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:67:33:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:67:33:    got restricted __le32 [noderef] [usertype] <asn:2>*hi
drivers/virtio/virtio_pci_modern.c:150:32: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:150:32:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:150:32:    got restricted __le32 [noderef] <asn:2>*<noident>
drivers/virtio/virtio_pci_modern.c:151:39: warning: incorrect type in argument 1 (different base types)
drivers/virtio/virtio_pci_modern.c:151:39:    expected unsigned int [noderef] [usertype] <asn:2>*addr
drivers/virtio/virtio_pci_modern.c:151:39:    got restricted __le32 [noderef] <asn:2>*<noident>
drivers/virtio/virtio_pci_modern.c:152:32: warning: incorrect type in argument 2 (different base types)
drivers/virtio/virtio_pci_modern.c:152:32:    expected unsigned int [noderef] [usertype] <asn:2>*addr

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 drivers/virtio/virtio_pci_modern.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index e76bd91..4bf7ab3 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -33,12 +33,12 @@ static inline u8 vp_ioread8(u8 __iomem *addr)
 {
 	return ioread8(addr);
 }
-static inline u16 vp_ioread16 (u16 __iomem *addr)
+static inline u16 vp_ioread16 (__le16 __iomem *addr)
 {
 	return ioread16(addr);
 }
 
-static inline u32 vp_ioread32(u32 __iomem *addr)
+static inline u32 vp_ioread32(__le32 __iomem *addr)
 {
 	return ioread32(addr);
 }
@@ -48,12 +48,12 @@ static inline void vp_iowrite8(u8 value, u8 __iomem *addr)
 	iowrite8(value, addr);
 }
 
-static inline void vp_iowrite16(u16 value, u16 __iomem *addr)
+static inline void vp_iowrite16(u16 value, __le16 __iomem *addr)
 {
 	iowrite16(value, addr);
 }
 
-static inline void vp_iowrite32(u32 value, u32 __iomem *addr)
+static inline void vp_iowrite32(u32 value, __le32 __iomem *addr)
 {
 	iowrite32(value, addr);
 }
-- 
1.8.3.1

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

[Gonglei]

 # 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 <arei.gonglei@huawei.com>
---
 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, &desc[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

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

[Gonglei]

 # 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 <arei.gonglei@huawei.com>
---
 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, &desc[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

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

[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 <arei.gonglei@huawei.com>
> ---
>  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, &desc[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

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

[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 <arei.gonglei@huawei.com>
>> ---
>>  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, &desc[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

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

[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 <arei.gonglei@huawei.com>
>> ---
>>  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, &desc[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

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

[Andy Lutomirski]

On Tue, Nov 22, 2016 at 7:16 AM, Thomas Huth <thuth@redhat.com> 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 <arei.gonglei@huawei.com>
>>> ---
>>>  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, &desc[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

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

[Andy Lutomirski]

On Tue, Nov 22, 2016 at 7:16 AM, Thomas Huth <thuth@redhat.com> 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 <arei.gonglei@huawei.com>
>>> ---
>>>  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, &desc[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

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

[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 <arei.gonglei@huawei.com>
> ---
>  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, &desc[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

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

[Cornelia Huck]

On Tue, 22 Nov 2016 13:51:50 +0800
Gonglei <arei.gonglei@huawei.com> 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 <arei.gonglei@huawei.com>
> ---
>  drivers/virtio/virtio_ring.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

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

[Cornelia Huck]

On Tue, 22 Nov 2016 13:51:50 +0800
Gonglei <arei.gonglei@huawei.com> 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 <arei.gonglei@huawei.com>
> ---
>  drivers/virtio/virtio_ring.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

RE: [PATCH 0/2] virtio: fix complaint by sparse

[Gonglei (Arei)]

Ping...?

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Tuesday, November 22, 2016 1:52 PM
> To: virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org
> Cc: mst@redhat.com; jasowang@redhat.com; Gonglei (Arei)
> Subject: [PATCH 0/2] virtio: fix complaint by sparse
> 
> I found some warnings reported by sparse in the virtio code
> when I checked virtio-crypto's driver stuff. Let's fix them.
> 
> Gonglei (2):
>   virtio_pci_modern: fix complaint by sparse
>   virtio_ring: fix complaint by sparse
> 
>  drivers/virtio/virtio_pci_modern.c | 8 ++++----
>  drivers/virtio/virtio_ring.c       | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> --
> 1.8.3.1
> 

RE: [PATCH 0/2] virtio: fix complaint by sparse

[Gonglei (Arei)]

Ping...?

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Tuesday, November 22, 2016 1:52 PM
> To: virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org
> Cc: mst@redhat.com; jasowang@redhat.com; Gonglei (Arei)
> Subject: [PATCH 0/2] virtio: fix complaint by sparse
> 
> I found some warnings reported by sparse in the virtio code
> when I checked virtio-crypto's driver stuff. Let's fix them.
> 
> Gonglei (2):
>   virtio_pci_modern: fix complaint by sparse
>   virtio_ring: fix complaint by sparse
> 
>  drivers/virtio/virtio_pci_modern.c | 8 ++++----
>  drivers/virtio/virtio_ring.c       | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> --
> 1.8.3.1
> 

Re: [PATCH 0/2] virtio: fix complaint by sparse

[Michael S. Tsirkin]

this is in my tree, will be in the next pull request.

On Sat, Nov 26, 2016 at 09:36:50AM +0000, Gonglei (Arei) wrote:
> Ping...?
> 
> > -----Original Message-----
> > From: Gonglei (Arei)
> > Sent: Tuesday, November 22, 2016 1:52 PM
> > To: virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org
> > Cc: mst@redhat.com; jasowang@redhat.com; Gonglei (Arei)
> > Subject: [PATCH 0/2] virtio: fix complaint by sparse
> > 
> > I found some warnings reported by sparse in the virtio code
> > when I checked virtio-crypto's driver stuff. Let's fix them.
> > 
> > Gonglei (2):
> >   virtio_pci_modern: fix complaint by sparse
> >   virtio_ring: fix complaint by sparse
> > 
> >  drivers/virtio/virtio_pci_modern.c | 8 ++++----
> >  drivers/virtio/virtio_ring.c       | 4 ++--
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > --
> > 1.8.3.1
> > 

Re: [PATCH 0/2] virtio: fix complaint by sparse

[Michael S. Tsirkin]

this is in my tree, will be in the next pull request.

On Sat, Nov 26, 2016 at 09:36:50AM +0000, Gonglei (Arei) wrote:
> Ping...?
> 
> > -----Original Message-----
> > From: Gonglei (Arei)
> > Sent: Tuesday, November 22, 2016 1:52 PM
> > To: virtualization@lists.linux-foundation.org; linux-kernel@vger.kernel.org
> > Cc: mst@redhat.com; jasowang@redhat.com; Gonglei (Arei)
> > Subject: [PATCH 0/2] virtio: fix complaint by sparse
> > 
> > I found some warnings reported by sparse in the virtio code
> > when I checked virtio-crypto's driver stuff. Let's fix them.
> > 
> > Gonglei (2):
> >   virtio_pci_modern: fix complaint by sparse
> >   virtio_ring: fix complaint by sparse
> > 
> >  drivers/virtio/virtio_pci_modern.c | 8 ++++----
> >  drivers/virtio/virtio_ring.c       | 4 ++--
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > --
> > 1.8.3.1
> >