Date: Mon, 10 Dec 2018 14:59:14 -0700 From: Warner Losh <imp@bsdimp.com> To: Hans Petter Selasky <hselasky@freebsd.org> Cc: "freebsd-mips@freebsd.org" <freebsd-mips@freebsd.org> Subject: Re: svn commit: r341787 - in head/sys: arm/include mips/include powerpc/include Message-ID: <CANCZdfovLnMg9sQ%2BkDsvzdYqpMDkLv3WPwDusUSz8JL=PMdRMQ@mail.gmail.com> In-Reply-To: <CANCZdfqiFrH6WZ8zqfxg_Dmk_pUGbewTVNh%2Bs5rV9wP=80ZuVQ@mail.gmail.com> References: <201812101338.wBADcE5A026483@repo.freebsd.org> <CANCZdfqiFrH6WZ8zqfxg_Dmk_pUGbewTVNh%2Bs5rV9wP=80ZuVQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Dec 10, 2018 at 11:09 AM Warner Losh <imp@bsdimp.com> wrote: > [[ redirected to mips mailing list ]] > > This doesn't quite do all platforms, see below, but 32-bit mips is omitted. > > On Mon, Dec 10, 2018 at 6:38 AM Hans Petter Selasky <hselasky@freebsd.org> > wrote: > >> Author: hselasky >> Date: Mon Dec 10 13:38:13 2018 >> New Revision: 341787 >> URL: https://svnweb.freebsd.org/changeset/base/341787 >> >> Log: >> Implement atomic_swap_xxx() for all platforms. >> >> +#if defined(__mips_n64) || defined(__mips_n32) >> +static __inline uint64_t >> +atomic_swap_64(volatile uint64_t *ptr, const uint64_t value) >> > > 32-bit MIPS still lacks this. It will need to be implemented in assembler. > Since we don't support SMP on mips-32 (or could easily move to not > supporting it if we have kernels that purport to support it), it would be > implemented as save status0; clear interrupts; do the swap; restore > status0. If we actually do have SMP 32-mips, then it's much harder. > I'll take a stab at fixing this. However, the issue does expose some weaknesses in how modules are selected for a given kernel. There's 4 SMP 32-bit mips kernels, 3 of which aren't very relevant. SWARM_SMP and XLP both have SMP in them. However, these are 64-bit processors, and should be booting 64-bit kernels, especially since compat32 exists. We can safely ignore both of these, ad if we de-orbited them, nobody would care (speak up if you care now please). We've already done this with Octeon, so there's some precedent. There's GXEMUL32, which has this option. gxemul is not used so much anymore now that MALTA works great with QEMU, so that can be ignored (maybe even deoribitted soon, but not today). None of the AP seem to have multiple cores, which is the bulk of the config files, so that's good. These can support in-kernel 64-bit atomic_swap_64 by disabling interrupts. No interrupt -> atomic operation. However, this may break if people using this function aren't pedantically correct in other 64-bit ops. However, this class of machines doesn't care about mpr/mps controllers and those modules aren't compiled by default for those kernel, so it's likely academic there. The swap_64 function is easy enough to implement though, and it's possible someone would hook them up and/or there be other uses. There's some 64-bit ones, but that's fine. Those can easily support 64-bit ops. No issues here. Then we have one 32-bit multi-core product. The JZ4780 supports a dual-core XBURST processor, so the SMP there is not a mistake, and is actually needed to be able to flush the cache properly. However, I think we're OK even here. For the mpr/mps drivers, it's not an issue: those modules will never be used. In fact, no modules are built for the JZ4780 kernel, and we don't need the linux KPI. So we should be OK not providing one there. We likely need to start tagging platforms that don't support the 64-bits ops somehow so we have a quick framework for new uses of this that pop up. Grepping through the tree, I found several instances where we have a list of architectures (not always the same list) that don't support 64-bits. So we should audit the tree here. Might make sense to have some drivers generally tagged as 64-bit only to work around this issue as well, and leave the edge cases of 32-bit platforms that could theoretically support them to do some kind of testing + opt-in. I'll look at knocking that together as well. > Warner > > >> +{ >> + uint64_t retval; >> + >> + retval = *ptr; >> + >> + while (!atomic_fcmpset_64(ptr, &retval, value)) >> + ; >> + return (retval); >> +} >> +#endif >> + >> +static __inline unsigned long >> +atomic_swap_long(volatile unsigned long *ptr, const unsigned long value) >> +{ >> + unsigned long retval; >> + >> + retval = *ptr; >> + >> + while (!atomic_fcmpset_32((volatile uint32_t *)ptr, >> + (uint32_t *)&retval, value)) >> + ; >> + return (retval); >> +} >> + >> +static __inline uintptr_t >> +atomic_swap_ptr(volatile uintptr_t *ptr, const uintptr_t value) >> +{ >> + uintptr_t retval; >> + >> + retval = *ptr; >> + >> + while (!atomic_fcmpset_32((volatile uint32_t *)ptr, >> + (uint32_t *)&retval, value)) >> + ; >> + return (retval); >> +} >> + >> #endif /* ! _MACHINE_ATOMIC_H_ */ >> >> Modified: head/sys/powerpc/include/atomic.h >> >> ============================================================================== >> --- head/sys/powerpc/include/atomic.h Mon Dec 10 09:45:57 2018 >> (r341786) >> +++ head/sys/powerpc/include/atomic.h Mon Dec 10 13:38:13 2018 >> (r341787) >> @@ -852,6 +852,9 @@ atomic_swap_64(volatile u_long *p, u_long v) >> #define atomic_fetchadd_64 atomic_fetchadd_long >> #define atomic_swap_long atomic_swap_64 >> #define atomic_swap_ptr atomic_swap_64 >> +#else >> +#define atomic_swap_long(p,v) atomic_swap_32((volatile u_int >> *)(p), v) >> +#define atomic_swap_ptr(p,v) atomic_swap_32((volatile u_int >> *)(p), v) >> #endif >> >> #undef __ATOMIC_REL >> >>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfovLnMg9sQ%2BkDsvzdYqpMDkLv3WPwDusUSz8JL=PMdRMQ>