Date: Wed, 17 Apr 2013 11:02:13 -0500 From: Brooks Davis <brooks@freebsd.org> To: Warner Losh <imp@bsdimp.com> Cc: freebsd-mips@freebsd.org Subject: Re: [PATCH] partial 64-bit bus space generic implementation Message-ID: <20130417160213.GB17916@lor.one-eyed-alien.net> In-Reply-To: <E68E3545-5202-4A13-B7D1-889F8FFD840B@bsdimp.com> References: <20130417010835.GA17779@lor.one-eyed-alien.net> <E68E3545-5202-4A13-B7D1-889F8FFD840B@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--QKdGvSO+nmPlgiQ/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 17, 2013 at 09:42:32AM -0600, Warner Losh wrote: >=20 > On Apr 16, 2013, at 7:08 PM, Brooks Davis wrote: >=20 > > 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? > >=20 > > -- Brooks > >=20 > > http://people.freebsd.org/~brooks/patches/mips-bs8.diff > >=20 > > MFP4 223084: > >=20 > > Partially implement generic_bs_*_8() for MIPS platforms. > >=20 > > This is known to work with TARGET_ARCH=3Dmips64 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. >=20 > 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 > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > --- 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 =3D=3D _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 > >=20 > > /* generic bus_space tag */ > > bus_space_tag_t mips_bus_space_generic =3D &generic_space; > > @@ -297,7 +305,11 @@ > > generic_bs_r_8(void *t, bus_space_handle_t handle, bus_size_t offset) > > { > >=20 > > +#ifdef rd64 > > + return(rd64(handle + offset)); > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > 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 =3D bsh + offset; > >=20 > > + while (count--) > > + *addr++ =3D rd64(baddr); > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > /* > > @@ -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 =3D bsh + offset; > >=20 > > + while (count--) { > > + *addr++ =3D rd64(baddr); > > + baddr +=3D 8; > > + } > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > /* > > @@ -419,7 +445,11 @@ > > uint64_t value) > > { > >=20 > > +#ifdef wr64 > > + wr64(bsh + offset, value); > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > /* > > @@ -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 =3D bsh + offset; > >=20 > > + while (count--) > > + wr64(baddr, *addr++); > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > /* > > @@ -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 =3D bsh + offset; > >=20 > > + while (count--) { > > + wr64(baddr, *addr++); > > + baddr +=3D 8; > > + } > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > /* > > @@ -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 =3D bsh + offset; > >=20 > > + while (count--) > > + wr64(addr, value); > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > /* > > @@ -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 =3D bsh + offset; > >=20 > > + for (; count !=3D 0; count--, addr +=3D 8) > > + wr64(addr, value); > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > /* > > @@ -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 =3D bsh1 + off1; > > + bus_addr_t addr2 =3D bsh2 + off2; > >=20 > > + if (addr1 >=3D addr2) { > > + /* src after dest: copy forward */ > > + for (; count !=3D 0; count--, addr1 +=3D 8, addr2 +=3D 8) > > + wr64(addr2, rd64(addr1)); > > + } else { > > + /* dest after src: copy backwards */ > > + for (addr1 +=3D 8 * (count - 1), addr2 +=3D 8 * (count - 1); > > + count !=3D 0; count--, addr1 -=3D 8, addr2 -=3D 8) > > + wr64(addr2, rd64(addr1)); > > + } > > +#else > > panic("%s: not implemented", __func__); > > +#endif > > } > >=20 > > void > > Index: sys/mips/include/cpufunc.h > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > --- 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 >=20 > 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) =3D (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 --QKdGvSO+nmPlgiQ/ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iD8DBQFRbseEXY6L6fI4GtQRAvQ8AKCLLR8TWeJ2BUemYn2Bc5xgtQCcyACfQpMc yDJizQADIHOQQhreTONvI+c= =iElB -----END PGP SIGNATURE----- --QKdGvSO+nmPlgiQ/--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130417160213.GB17916>