Re: [PATCH V9 21/21] csky: support dword access for get_user_size()

On Thu, Oct 18, 2018 at 9:57 AM Guo Ren [off-list ref] wrote:

On Thu, Oct 18, 2018 at 10:34:00AM +0200, Arnd Bergmann wrote:

quoted

On Thu, Oct 18, 2018 at 5:41 AM Guo Ren [off-list ref] wrote:

quoted

On Wed, Oct 17, 2018 at 05:44:17PM +0200, Arnd Bergmann wrote:

quoted

On Tue, Oct 16, 2018 at 5:33 AM Guo Ren [off-list ref] wrote:

quoted

Support dword access for get_user_size and redesign put_user_size with
the same style of get_user_size. It's Ok to use xxx_user_asm_common for
all size of variable with ldb, ldh, ldw, ld.d

ld.d rx, (addr, 0) could "rx <= addr" "and r(x+1) <= addr+4" and this also
follow abiv2 gcc ABI for dword access.

Are you sure this is correct for this?

static inline u32 get_64_to_32(__u64 __user *p)
{
      u32 ret;
      get_user(ret, p);
      return ret;
}

If I read __get_user_asm_common() right, the ld.d would overwrite
two registers, but the caller only expects one, so it clobbers one
that might be in use.

Ah... BUG! I only consider the get_user(u64, u64 *) :P

Change to:
case 8:                                                 \
__get_user_asm_dword((x), ptr, "ld.d", retval);         \
break;

#define __get_user_asm_dword(x, ptr, err)               \
do {                                                    \
        u64 tmp;                                        \
        __get_user_asm_common(tmp, ptr, "ld.d", err);   \
        x = typeof(x) tmp;                              \
} while(0)

#define __put_user_asm_dword(x, ptr, err)               \
do {                                                    \
        u64 tmp = (u64) x;                              \
        __put_user_asm_common(tmp, ptr, "st.d", err);   \
} while(0)

I think this will cause warnings for code that passes a pointer.

The 64-bit __get_user() is really hard, and most 32-bit architectures don't
implement it at all. If you really want to add it, have a look at what
x86 and arm do. IIRC they both use __builtin_choose_expr(),

Thx for the tips and I'll drop the patch first for the upstream.

I want to implement it because of make allmodconfig and
drivers/android/binder.c need it. I'll learn __builtin_choose_expr()
and prepare patch next.

I think we should fix binder.c instead. Many other architectures
have the same problem.

          Arnd