Re: [tip:x86/mm] x86, mm: Use a bitfield to mask nuisanceget_user() warnings

[Russell King - ARM Linux]

On Tue, Feb 12, 2013 at 09:32:54AM -0800, Linus Torvalds wrote:
> On Tue, Feb 12, 2013 at 9:14 AM, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
> >
> > No, I think what he is talking about it this bit:
> 
> Ok, I agree that the bitfield code actually looks cleaner.
> 
> That said, maybe gcc has an easier time using a few odd builtins and
> magic typeof's. But at least the bitfield trick looks half-way
> portable..

I've just been trying hpa's solution on ARM, and I can't get it to work,
because the compiler refuses to put the struct { unsigned long long ... }
into the register(s) we need for the out of line assembly:

#define get_user(x,p)							\
	({								\
		register const typeof(*(p)) __user *__p asm("r0") = (p);\
		register int __e asm("r0");				\
		register struct {					\
			unsigned long long __r2:8 * sizeof(*(__p));	\
		} __v asm("r2");					\
		switch (sizeof(*(__p))) {				\
		case 1: 						\
			__get_user_x(__v.__r2, __p, __e, 1, "lr");	\
			break;						\
		case 2: 						\
			__get_user_x(__v.__r2, __p, __e, 2, "r3", "lr");\
			break;						\
		case 4: 						\
			__get_user_x(__v.__r2, __p, __e, 4, "lr");	\
			break;						\
		case 8: 						\
			__get_user_x(__v.__r2, __p, __e, 8, "lr");	\
			break;						\
		default: __e = __get_user_bad(); break; 		\
		}							\
		x = (typeof(*(__p))) __v.__r2;				\
		__e;							\
	})

This ends up with __v.__r2 ending up in r1/(r2) not r2/(r3).

However, I do have a working solution for 32-bit ARM which seems to work
fine with my test cases here, though as I mentioned to hpa, it may not
be portable to other 32-bit architectures:

#ifdef BIG_ENDIAN
#define __get_user_xb(__r2,__p,__e,__s,__i...)				\
	__get_user_x(__r2,(unsigned)__p+4,__e,__s,__i)
#else
#define __get_user_xb __get_user_x
#endif

#define get_user(x,p)							\
	({								\
		register const typeof(*(p)) __user *__p asm("r0") = (p);\
		register int __e asm("r0");				\
		register typeof(x) __r2 asm("r2");			\
		switch (sizeof(*(__p))) {				\
		case 1: 						\
			__get_user_x(__r2, __p, __e, 1, "lr");		\
			break;						\
		case 2: 						\
			__get_user_x(__r2, __p, __e, 2, "r3", "lr");	\
			break;						\
		case 4: 						\
			__get_user_x(__r2, __p, __e, 4, "lr");		\
			break;						\
		case 8: 						\
			{						\
			if (sizeof((x)) < 8)				\
				__get_user_xb(__r2, __p, __e, 4, "lr"); \
			else						\
				__get_user_x(__r2, __p, __e, 8, "lr");	\
			}						\
			break;						\
		default: __e = __get_user_bad(); break; 		\
		}							\
		x = (typeof(*(__p))) __r2;				\
		__e;							\
	})

It's risky because it relies upon a "register" being allocated as 32-bits
even if typeof(x) is 8-bit or 16-bit.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/