[PATCH] partial 64-bit bus space generic implementation

Brooks Davis brooks at freebsd.org
Wed Apr 17 16:02:12 UTC 2013


On Wed, Apr 17, 2013 at 09:42:32AM -0600, Warner Losh wrote:
> 
> On Apr 16, 2013, at 7:08 PM, Brooks Davis wrote:
> 
> > I've implemented generic_bs_*_8() for a subset of MIPS platforms
> > (!sibyte and !o32 ABI) based on the mips3_ld and mips3_sd code in
> > cpufunc.h.  I've verified that simple reads and writes work for our
> > MIPS64 ISA CPU.  The patch passes make universe, but I've not tested it
> > on any mips32 systems.  Any reviews or objections to this patch?
> > 
> > -- Brooks
> > 
> > http://people.freebsd.org/~brooks/patches/mips-bs8.diff
> > 
> > MFP4 223084:
> > 
> > Partially implement generic_bs_*_8() for MIPS platforms.
> > 
> > This is known to work with TARGET_ARCH=mips64 with FreeBSD/BERI.
> > Assuming that other definitions in cpufunc.h are correct it will work on
> > non-o64 ABI systems except sibyte. On sibyte and o32 systems
> > generic_bs_*_8() will remain panic() implementations.
> 
> Why not include sibyte? Doesn't it have LD/SD instructions in 64-bit mode?

Because the sibyte implementation uses sb_big_endian_read## macros and
there is no 64-bit macro available.

> > Index: sys/mips/mips/bus_space_generic.c
> > ===================================================================
> > --- sys/mips/mips/bus_space_generic.c	(revision 249555)
> > +++ sys/mips/mips/bus_space_generic.c	(working copy)
> > @@ -202,9 +202,11 @@
> > #define rd8(a) cvmx_read64_uint8(a)
> > #define rd16(a) cvmx_read64_uint16(a)
> > #define rd32(a) cvmx_read64_uint32(a)
> > +#define rd64(a) cvmx_read64_uint64(a)
> > #define wr8(a, v) cvmx_write64_uint8(a, v)
> > #define wr16(a, v) cvmx_write64_uint16(a, v)
> > #define wr32(a, v) cvmx_write64_uint32(a, v)
> > +#define wr64(a, v) cvmx_write64_uint64(a, v)
> > #elif defined(CPU_SB1) && _BYTE_ORDER == _BIG_ENDIAN
> > #include <mips/sibyte/sb_bus_space.h>
> > #define rd8(a) sb_big_endian_read8(a)
> > @@ -217,10 +219,16 @@
> > #define rd8(a) readb(a)
> > #define rd16(a) readw(a)
> > #define rd32(a) readl(a)
> > +#ifdef readd
> > +#define rd64(a)	readd((a))
> > +#endif
> > #define wr8(a, v) writeb(a, v)
> > #define wr16(a, v) writew(a, v)
> > #define wr32(a, v) writel(a, v)
> > +#ifdef writed
> > +#define wr64(a, v) writed((uint64_t *)(a), v)
> > #endif
> > +#endif
> > 
> > /* generic bus_space tag */
> > bus_space_tag_t mips_bus_space_generic = &generic_space;
> > @@ -297,7 +305,11 @@
> > generic_bs_r_8(void *t, bus_space_handle_t handle, bus_size_t offset)
> > {
> > 
> > +#ifdef rd64
> > +	return(rd64(handle + offset));
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > void
> > @@ -333,8 +345,14 @@
> > generic_bs_rm_8(void *t, bus_space_handle_t bsh, bus_size_t offset,
> >     uint64_t *addr, size_t count)
> > {
> > +#ifdef rd64
> > +	bus_addr_t baddr = bsh + offset;
> > 
> > +	while (count--)
> > +		*addr++ = rd64(baddr);
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > /*
> > @@ -382,8 +400,16 @@
> > generic_bs_rr_8(void *t, bus_space_handle_t bsh, bus_size_t offset,
> >     uint64_t *addr, size_t count)
> > {
> > +#ifdef rd64
> > +	bus_addr_t baddr = bsh + offset;
> > 
> > +	while (count--) {
> > +		*addr++ = rd64(baddr);
> > +		baddr += 8;
> > +	}
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > /*
> > @@ -419,7 +445,11 @@
> >     uint64_t value)
> > {
> > 
> > +#ifdef wr64
> > +	wr64(bsh + offset, value);
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > /*
> > @@ -460,8 +490,14 @@
> > generic_bs_wm_8(void *t, bus_space_handle_t bsh, bus_size_t offset,
> >     const uint64_t *addr, size_t count)
> > {
> > +#ifdef wr64
> > +	bus_addr_t baddr = bsh + offset;
> > 
> > +	while (count--)
> > +		wr64(baddr, *addr++);
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > /*
> > @@ -508,8 +544,16 @@
> > generic_bs_wr_8(void *t, bus_space_handle_t bsh, bus_size_t offset,
> >     const uint64_t *addr, size_t count)
> > {
> > +#ifdef wr64
> > +	bus_addr_t baddr = bsh + offset;
> > 
> > +	while (count--) {
> > +		wr64(baddr, *addr++);
> > +		baddr += 8;
> > +	}
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > /*
> > @@ -550,8 +594,14 @@
> > generic_bs_sm_8(void *t, bus_space_handle_t bsh, bus_size_t offset,
> >     uint64_t value, size_t count)
> > {
> > +#ifdef wr64
> > +	bus_addr_t addr = bsh + offset;
> > 
> > +	while (count--)
> > +		wr64(addr, value);
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > /*
> > @@ -592,8 +642,14 @@
> > generic_bs_sr_8(void *t, bus_space_handle_t bsh, bus_size_t offset,
> >     uint64_t value, size_t count)
> > {
> > +#ifdef wr64
> > +	bus_addr_t addr = bsh + offset;
> > 
> > +	for (; count != 0; count--, addr += 8)
> > +		wr64(addr, value);
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > /*
> > @@ -664,8 +720,23 @@
> > generic_bs_c_8(void *t, bus_space_handle_t bsh1, bus_size_t off1,
> >     bus_space_handle_t bsh2, bus_size_t off2, size_t count)
> > {
> > +#if defined(rd64) && defined(wr64)
> > +	bus_addr_t addr1 = bsh1 + off1;
> > +	bus_addr_t addr2 = bsh2 + off2;
> > 
> > +	if (addr1 >= addr2) {
> > +		/* src after dest: copy forward */
> > +		for (; count != 0; count--, addr1 += 8, addr2 += 8)
> > +			wr64(addr2, rd64(addr1));
> > +	} else {
> > +		/* dest after src: copy backwards */
> > +		for (addr1 += 8 * (count - 1), addr2 += 8 * (count - 1);
> > +		    count != 0; count--, addr1 -= 8, addr2 -= 8)
> > +			wr64(addr2, rd64(addr1));
> > +	}
> > +#else
> > 	panic("%s: not implemented", __func__);
> > +#endif
> > }
> > 
> > void
> > Index: sys/mips/include/cpufunc.h
> > ===================================================================
> > --- sys/mips/include/cpufunc.h	(revision 249555)
> > +++ sys/mips/include/cpufunc.h	(working copy)
> > @@ -354,9 +354,15 @@
> > #define	readb(va)	(*(volatile uint8_t *) (va))
> > #define	readw(va)	(*(volatile uint16_t *) (va))
> > #define	readl(va)	(*(volatile uint32_t *) (va))
> > +#if defined(__GNUC__) && !defined(__mips_o32)
> > +#define	readd(a)	(*(volatile uint64_t *)(a))
> > +#endif
> 
> Why __GNU_C__ here?

Because that's the ifdef that was used a few lines up around this:

#if defined(__GNUC__) && !defined(__mips_o32)
#define mips3_ld(a)     (*(const volatile uint64_t *)(a))
#define mips3_sd(a, v)  (*(volatile uint64_t *)(a) = (v))
#else
uint64_t mips3_ld(volatile uint64_t *va);
void mips3_sd(volatile uint64_t *, uint64_t);
#endif  /* __GNUC__ */

It's not at all clear to me that's the right ifdef, but someone once
thought it was so I went with it.  I'm happy to go with a more appropriate
ifdef.  For my purposes something that pinned it to mips64 would be fine
since supporting mips32 on our platform is a non-goal.

Thanks,
Brooks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-mips/attachments/20130417/7057ddd2/attachment.sig>


More information about the freebsd-mips mailing list