Date: Mon, 4 Dec 2017 11:33:08 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Warner Losh <imp@bsdimp.com> Cc: Bruce Evans <brde@optusnet.com.au>, Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r326486 - in head/stand: ofw/libofw powerpc/boot1.chrp Message-ID: <20171204104127.B961@besplex.bde.org> In-Reply-To: <CANCZdfoq_HO-CUm9mC=ZPKxOn2ENd4N2-G330VXC9efft6fDJw@mail.gmail.com> References: <201712030454.vB34ssem056112@repo.freebsd.org> <20171203185027.T872@besplex.bde.org> <CANCZdfoq_HO-CUm9mC=ZPKxOn2ENd4N2-G330VXC9efft6fDJw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 3 Dec 2017, Warner Losh wrote: > On Sun, Dec 3, 2017 at 2:35 AM, Bruce Evans <brde@optusnet.com.au> wrote: > >> On Sun, 3 Dec 2017, Warner Losh wrote: >> >> Log: >>> Include machine/md_var to pick up __syncicache prototype. >> >> This is nonsense. machine/md_var.h is kernel-only, but on powerpc >> it declares __syncicache() which also exists in userland. This include >> is like including the kernel-only header sys/systm.h to declare printf() >> in userland. > > Yea.... This is the sort of thing that should be defined in cpufunc.h or > similar. I didn't feel like moving it, so I made one more bad place in the > tree instead... So I agree with this feedback. Not cpufunc.h. I created it too (rgrimes committed it in 386BSD days). It is for turning CPU instructions into C functions (usually using inline asm function wrappers of single instructions, where the function name should be the same as the instrcution name to make it easier to rememember and inhibit use in MI code) so that even MD code doesn't need to use any inline asm. It was intened to be kernel-only, but even I abused it in userland for the inb() family of functions and later for rdtsc(). The inb() family later became a larger problem. inb() is a special case of bus_space_read_1() and shouldn't be used even to implement bus-space since that gives namespace pollution. Instead of fixing this, the readb() family of functions was added to cpufunc.h. readb() is essentially bus_space_read_1() misimplemented for dev/fb. It is now misused in a couple of other places. cpufunc.h is much more broken on non-x86 arches. On arm, it has lots of extern variables and only 2 inline asms. 1 of these is called by many C inline non-asms that don't belong in cpufunc.h because they can be built out of the main inline asm here and C definitions. Putting everything here gives namespace pollution. Other arches have more inline asms and not many extern variables, but have the pollution. Some have macros instead of inline functions. This requires less pollution but is more fragile. x86 has bogus non-asm wrappers only for a few functions like intr_disable(). There is a problem making the inline functions visible if they are in a different file. The correct way of including machine/cpu.h is to not explicitly include it, but depend on getting it as standard pollution in sys/systm.h. If the C parts were moved to different files, then sys/systm.h would still have to include all of the pollution for compatibility. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171204104127.B961>