Re: [PATCH 08/17] Introduce libio to common code for io read/write

[Christoffer Dall <christoffer.dall@linaro.org>]

On Mon, Feb 03, 2014 at 02:51:15PM +0100, Andrew Jones wrote:
> On Sat, Feb 01, 2014 at 06:24:51PM -0800, Christoffer Dall wrote:
> > On Tue, Jan 21, 2014 at 05:21:54PM +0100, Andrew Jones wrote:
> > > v3:
> > >  - get rid of CONFIG_64BIT, replace with asserts
> > >  - get rid of {read,write}_len()
> > >  - fix bad *64_to_cpu macros
> > 
> > note that putting the changelog after the "---" prevents it from
> > getting committed, but I'm not sure if there are different
> > conventions for the unit-tests framework.
> 
> I'll drop the '---' in the v4 patches
> 

no, I meant that the v3 changelog should be below the '---' not above it
- we don't want to commit a changelog to git.

> > 
> > > 
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > > ---
> > >  lib/libio.h | 173 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 173 insertions(+)
> > >  create mode 100644 lib/libio.h
> > > 
> > > diff --git a/lib/libio.h b/lib/libio.h
> > > new file mode 100644
> > > index 0000000000000..6adc6a7bbee61
> > > --- /dev/null
> > > +++ b/lib/libio.h
> > > @@ -0,0 +1,173 @@
> > > +#ifndef _LIBIO_H_
> > > +#define _LIBIO_H_
> > > +/*
> > > + * Adapted from the Linux kernel's include/asm-generic/io.h and
> > > + * arch/arm/include/asm/io.h
> > > + */
> > > +#include "libcflat.h"
> > > +
> > > +#ifndef NDEBUG
> > > +#define LIBIO_ASSERT(expr) do { \
> > > +	if (!(expr)) printf("%s:%d: assert failed!\n", __FILE__, __LINE__);\
> > > +} while (0)
> > > +#else
> > > +#define LIBIO_ASSERT(expr) do { } while (0)
> > > +#endif
> > > +
> > > +typedef u32 compat_ptr_t;
> > > +
> > > +/*
> > > + * Use compat_ptr() on 64-bit arches when casting 32-bit
> > 
> > arches? architectures? archs?
> 
> kernel code has 88 archs and 55 arches, I guess we could go with
> archs by majority rules, but I prefer arches.
> 

fair enough, I associated a national park.  Bike shedding on my part
anyway.

> > 
> > > + * addresses to pointers in order to avoid compiler warnings.
> > > + */
> > > +static inline void *compat_ptr(compat_ptr_t ptr)
> > > +{
> > > +	return (void *)(unsigned long)ptr;
> > > +}
> > > +
> > > +static inline compat_ptr_t ptr_to_compat(void *ptr)
> > > +{
> > > +	return (u32)(unsigned long)ptr;
> > > +}
> > > +
> > > +#ifndef __raw_readb
> > > +static inline u8 __raw_readb(const volatile void *addr)
> > > +{
> > > +	return *(const volatile u8 *)addr;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __raw_readw
> > > +static inline u16 __raw_readw(const volatile void *addr)
> > > +{
> > > +	return *(const volatile u16 *)addr;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __raw_readl
> > > +static inline u32 __raw_readl(const volatile void *addr)
> > > +{
> > > +	return *(const volatile u32 *)addr;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __raw_readq
> > > +static inline u64 __raw_readq(const volatile void *addr)
> > > +{
> > > +	LIBIO_ASSERT(sizeof(unsigned long) == sizeof(u64));
> > > +	return *(const volatile u64 *)addr;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __raw_writeb
> > > +static inline void __raw_writeb(u8 b, volatile void *addr)
> > > +{
> > > +	*(volatile u8 *)addr = b;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __raw_writew
> > > +static inline void __raw_writew(u16 b, volatile void *addr)
> > > +{
> > > +	*(volatile u16 *)addr = b;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __raw_writel
> > > +static inline void __raw_writel(u32 b, volatile void *addr)
> > > +{
> > > +	*(volatile u32 *)addr = b;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __raw_writeq
> > > +static inline void __raw_writeq(u64 b, volatile void *addr)
> > > +{
> > > +	LIBIO_ASSERT(sizeof(unsigned long) == sizeof(u64));
> > > +	*(volatile u64 *)addr = b;
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __bswap16
> > > +static inline u16 __bswap16(u16 x)
> > > +{
> > > +	return ((x >> 8) & 0xff) | ((x & 0xff) << 8);
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __bswap32
> > > +static inline u32 __bswap32(u32 x)
> > > +{
> > > +	return ((x & 0xff000000) >> 24) | ((x & 0x00ff0000) >>  8) |
> > > +	       ((x & 0x0000ff00) <<  8) | ((x & 0x000000ff) << 24);
> > > +}
> > > +#endif
> > > +
> > > +#ifndef __bswap64
> > > +static inline u64 __bswap64(u64 x)
> > > +{
> > > +	return ((x & 0x00000000000000ffULL) << 56) |
> > > +	       ((x & 0x000000000000ff00ULL) << 40) |
> > > +	       ((x & 0x0000000000ff0000ULL) << 24) |
> > > +	       ((x & 0x00000000ff000000ULL) <<  8) |
> > > +	       ((x & 0x000000ff00000000ULL) >>  8) |
> > > +	       ((x & 0x0000ff0000000000ULL) >> 24) |
> > > +	       ((x & 0x00ff000000000000ULL) >> 40) |
> > > +	       ((x & 0xff00000000000000ULL) >> 56);
> > > +}
> > > +#endif
> > > +
> > > +#ifndef cpu_is_be
> > > +#define cpu_is_be 0
> > > +#endif
> > > +
> > > +#define le16_to_cpu(x) \
> > > +	({ u16 __r = cpu_is_be ? __bswap16(x) : (x); __r; })
> > > +#define cpu_to_le16 le16_to_cpu
> > > +
> > > +#define le32_to_cpu(x) \
> > > +	({ u32 __r = cpu_is_be ? __bswap32(x) : (x); __r; })
> > > +#define cpu_to_le32 le32_to_cpu
> > > +
> > > +#define le64_to_cpu(x) \
> > > +	({ u64 __r = cpu_is_be ? __bswap64(x) : (x); __r; })
> > > +#define cpu_to_le64 le64_to_cpu
> > > +
> > > +#define be16_to_cpu(x) \
> > > +	({ u16 __r = !cpu_is_be ? __bswap16(x) : (x); __r; })
> > > +#define cpu_to_be16 be16_to_cpu
> > > +
> > > +#define be32_to_cpu(x) \
> > > +	({ u32 __r = !cpu_is_be ? __bswap32(x) : (x); __r; })
> > > +#define cpu_to_be32 be32_to_cpu
> > > +
> > > +#define be64_to_cpu(x) \
> > > +	({ u64 __r = !cpu_is_be ? __bswap64(x) : (x); __r; })
> > > +#define cpu_to_be64 be64_to_cpu
> > > +
> > > +#ifndef rmb
> > > +#define rmb() do { } while (0)
> > > +#endif
> > > +#ifndef wmb
> > > +#define wmb() do { } while (0)
> > > +#endif
> > > +
> > > +#define readb(addr) \
> > > +	({ u8 __r = __raw_readb(addr); rmb(); __r; })
> > > +#define readw(addr) \
> > > +	({ u16 __r = le16_to_cpu(__raw_readw(addr)); rmb(); __r; })
> > > +#define readl(addr) \
> > > +	({ u32 __r = le32_to_cpu(__raw_readl(addr)); rmb(); __r; })
> > > +#define readq(addr) \
> > > +	({ u64 __r = le64_to_cpu(__raw_readq(addr)); rmb(); __r; })
> > > +
> > > +#define writeb(b, addr) \
> > > +	({ wmb(); __raw_writeb(b, addr); })
> > > +#define writew(b, addr) \
> > > +	({ wmb(); __raw_writew(cpu_to_le16(b), addr); })
> > > +#define writel(b, addr) \
> > > +	({ wmb(); __raw_writel(cpu_to_le32(b), addr); })
> > > +#define writeq(b, addr) \
> > > +	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
> > > +
> > > +#endif
> > 
> > so these definitions always assume that your device is LE, which is
> > probably true for ARM and x86 for now, but if we ever use this on PPC or
> > want to test ARM BE with a BE QEMU, will it still work?
> 
> I don't know if it'll be an issue or not. Let's cross that bridge when we
> get to it. We can alway move/duplicate these accessors into each arch
> specific io.h file, when necessary.
> 

ok, I can live with that.

> > 
> > (There's some trickery with virtio and endianness that I don't remember
> > 100%, so if we assume this ever only reads and writes to virtio space,
> > we may be in the clear.)
> 
> The virtio spec is switching to everything LE, I believe, so the above
> should be good for virtio devices anyway.
> 
> 
yep.