Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1527241 > unrolled thread

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

Started byGonglei <arei.gonglei@huawei.com>
First post2016-11-22 07:00 +0100
Last post2016-11-22 19:00 +0100
Articles 7 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] virtio: fix complaint by sparse Gonglei <arei.gonglei@huawei.com> - 2016-11-22 07:00 +0100
    [PATCH 1/2] virtio_pci_modern: fix complaint by sparse Gonglei <arei.gonglei@huawei.com> - 2016-11-22 07:00 +0100
    [PATCH 2/2] virtio_ring: fix complaint by sparse Gonglei <arei.gonglei@huawei.com> - 2016-11-22 07:00 +0100
      Re: [PATCH 2/2] virtio_ring: fix complaint by sparse "Michael S. Tsirkin" <mst@redhat.com> - 2016-11-22 16:10 +0100
        Re: [PATCH 2/2] virtio_ring: fix complaint by sparse Thomas Huth <thuth@redhat.com> - 2016-11-22 16:20 +0100
          Re: [PATCH 2/2] virtio_ring: fix complaint by sparse Andy Lutomirski <luto@amacapital.net> - 2016-11-22 18:30 +0100
      Re: [PATCH 2/2] virtio_ring: fix complaint by sparse Cornelia Huck <cornelia.huck@de.ibm.com> - 2016-11-22 19:00 +0100

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

FromGonglei <arei.gonglei@huawei.com>
Date2016-11-22 07:00 +0100
Subject[PATCH 0/2] virtio: fix complaint by sparse
Message-ID<sGchz-34U-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

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

FromGonglei <arei.gonglei@huawei.com>
Date2016-11-22 07:00 +0100
Subject[PATCH 1/2] virtio_pci_modern: fix complaint by sparse
Message-ID<sGchz-34U-5@gated-at.bofh.it>
In reply to#1527241
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

[toc] | [prev] | [next] | [standalone]

#1527243 — [PATCH 2/2] virtio_ring: fix complaint by sparse

FromGonglei <arei.gonglei@huawei.com>
Date2016-11-22 07:00 +0100
Subject[PATCH 2/2] virtio_ring: fix complaint by sparse
Message-ID<sGchz-34U-7@gated-at.bofh.it>
In reply to#1527241
 # 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

[toc] | [prev] | [next] | [standalone]

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

From"Michael S. Tsirkin" <mst@redhat.com>
Date2016-11-22 16:10 +0100
SubjectRe: [PATCH 2/2] virtio_ring: fix complaint by sparse
Message-ID<sGkRQ-lv-3@gated-at.bofh.it>
In reply to#1527243
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

[toc] | [prev] | [next] | [standalone]

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

FromThomas Huth <thuth@redhat.com>
Date2016-11-22 16:20 +0100
SubjectRe: [PATCH 2/2] virtio_ring: fix complaint by sparse
Message-ID<sGl1v-oK-11@gated-at.bofh.it>
In reply to#1527582
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

[toc] | [prev] | [next] | [standalone]

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

FromAndy Lutomirski <luto@amacapital.net>
Date2016-11-22 18:30 +0100
SubjectRe: [PATCH 2/2] virtio_ring: fix complaint by sparse
Message-ID<sGn3j-1LJ-29@gated-at.bofh.it>
In reply to#1527591
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

[toc] | [prev] | [next] | [standalone]

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

FromCornelia Huck <cornelia.huck@de.ibm.com>
Date2016-11-22 19:00 +0100
SubjectRe: [PATCH 2/2] virtio_ring: fix complaint by sparse
Message-ID<sGnwl-1Wv-9@gated-at.bofh.it>
In reply to#1527243
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>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web