Re: [PATCH 2/2] gpio: tz1090-pdc: Use resource_size to fix off-by-one resource size calculation

[James Hogan]

On 28/12/14 06:01, Axel Lin wrote:
> Signed-off-by: Axel Lin 

Thanks, both patches
Acked-by: James Hogan 

Note that the previous off-by-one behaviour should be harmless since the
SoC IO memory region is unmapped on Meta (i.e. the ioremap becomes a
no-op as the whole of IO memory is accessible with normal memory
accesses regardless of MMU).

Cheers
James

> ---
>  drivers/gpio/gpio-tz1090-pdc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-tz1090-pdc.c b/drivers/gpio/gpio-tz1090-pdc.c
> index d753622..ede7e40 100644
> --- a/drivers/gpio/gpio-tz1090-pdc.c
> +++ b/drivers/gpio/gpio-tz1090-pdc.c
> @@ -190,7 +190,7 @@ static int tz1090_pdc_gpio_probe(struct platform_device 
> *pdev)
>  
>   /* Ioremap the registers */
>   priv->reg = devm_ioremap(&pdev->dev, res_regs->start,
> -  res_regs->end - res_regs->start);
> +  resource_size(res_regs));
>   if (!priv->reg) {
>   dev_err(&pdev->dev, "unable to ioremap registers\n");
>   return -ENOMEM;
> 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH repost 09/16] metag/uaccess: fix sparse errors

[James Hogan]

Hi,

On 25/12/14 09:29, Michael S. Tsirkin wrote:
> virtio wants to read bitwise types from userspace using get_user.  At the
> moment this triggers sparse errors, since the value is passed through an
> integer.
> 
> Fix that up using __force.

I still see these sparse warnings with metag even with your patch:

  CHECK   drivers/vhost/vhost.c
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1004:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1022:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1373:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1377:21: warning: cast from restricted __virtio32
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16
drivers/vhost/vhost.c:1425:13: warning: cast from restricted __virtio16

Which something like the following hunk fixes in a similar way to yours:

diff --git a/arch/metag/include/asm/uaccess.h b/arch/metag/include/asm/uaccess.h
index 0748b0a97986..594497053748 100644
--- a/arch/metag/include/asm/uaccess.h
+++ b/arch/metag/include/asm/uaccess.h
@@ -112,13 +112,17 @@ do {  
  \
retval = 0; \
switch (size) { \
case 1: \
-   retval = __put_user_asm_b((unsigned int)x, ptr); break; \
+   retval = __put_user_asm_b((__force unsigned int)x, ptr); \
+   break;  \
case 2: \
-   retval = __put_user_asm_w((unsigned int)x, ptr); break; \
+   retval = __put_user_asm_w((__force unsigned int)x, ptr); \
+   break;  \
case 4: \
-   retval = __put_user_asm_d((unsigned int)x, ptr); break; \
+   retval = __put_user_asm_d((__force unsigned int)x, ptr); \
+   break;  \
case 8: \
-   retval = __put_user_asm_l((unsigned long long)x, ptr); break; \
+   retval = __put_user_asm_l((__force unsigned long long)x, ptr); \
+   break;  \
default:\
__put_user_bad();   \
}   

As far as I understand it, using __force on the value (as opposed to the
pointer) is safe here, in the sense of not masking any genuine defects.
Do you agree? Do you want to apply that hunk with your patch too?

Note, this change also suppresses warnings for writing a pointer to
userland due to the casts to unsigned int / unsigned long long, such as
the following (each 4 times due to 4 cases above):

kernel/signal.c:2740:25: warning: cast removes address space of expression
kernel/signal.c:2747:24: warning: cast removes address space of expression
kernel/signal.c:2760:24: warning: cast removes address space of expression
kernel/signal.c:2761:24: warning: cast removes address space of expression
kernel/signal.c:2775:24: warning: cast removes address space of expression
kernel/signal.c:2779:24: warning: cast removes address space of expression
kernel/signal.c:3202:25: warning: cast removes address space of expression
kernel/signal.c:3225:17: warning: cast removes address space of expression
kernel/futex.c:2769:16: warning: cast removes address space of expression

> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  arch/metag/include/asm/uaccess.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/metag/include/asm/uaccess.h 
> b/arch/metag/include/asm/uaccess.h
> index 0748b0a..c314b45 100644
> --- a/arc