svn commit: r313037 - in head/sys: amd64/include kern mips/include net powerpc/include sparc64/include

[Jason Harmening]

I suspect this broke rmlocks for mips because the rmlock implementation
takes the address of the per-CPU pc_rm_queue when building tracker lists.
That address may be later accessed from another CPU and will then translate
to the wrong physical region if the address was taken relative to the
globally-constant pcpup VA used on mips.

Regardless, for mips get_pcpup() should be implemented as pcpu_find(curcpu)
since returning an address that may mean something different depending on
the CPU seems like a big POLA violation if nothing else.

I'm more concerned about the report of powerpc breakage.  For powerpc we
simply take each pcpu pointer from the pc_allcpu list (which is the same
value stored in the cpuid_to_pcpu array) and pass it through the ap_pcpu
global to each AP's startup code, which then stores it in sprg0.  It should
be globally unique and won't have the variable-translation issues seen on
mips.   Andreas, are you certain this change was responsible the breakage
you saw, and was it the same sort of hang observed on mips?

On Sat, Feb 4, 2017 at 1:25 AM, Andreas Tobler <andreast at freebsd.org> wrote:

> On 04.02.17 07:27, Jason Harmening wrote:
>
>> It's hard to argue with that:)  I've backed it out until we can figure
>> out what's going on.
>> Sorry for the breakage.
>>
>
>
> For the record, powerpc64 was also affected.
>
> Andreas
>
>>
>> On Fri, Feb 3, 2017 at 9:54 PM, Kurt Lidl <lidl at pix.net
>> <mailto:lidl at pix.net>> wrote:
>>
>>     Having just spent a couple of hours bisecting what broke the kernel on
>>     my mips64 machine, I can definitively state it was this commit.
>>
>>     With this commit in place, the kernel hangs early in the
>>     autoconfiguration:
>>
>>     gcc version 4.2.1 20070831 patched [FreeBSD]
>>     Preloaded elf kernel "kernel" at 0xffffffff80aa96a0.
>>     real memory  = 523239424 (510976K bytes)
>>     Physical memory chunk(s):
>>     0x00bf3000 - 0x080d5fff, 122564608 bytes (29923 pages)
>>     0x08101000 - 0x0ff00fff, 132120576 bytes (32256 pages)
>>     0x410000000 - 0x41f196fff, 253325312 bytes (61847 pages)
>>     avail memory = 504360960 (480MB)
>>     Create COP2 context zone
>>     AP #1 started!
>>     FreeBSD/SMP: Multiprocessor System Detected: 2 CPUs
>>     ---- hangs here ----
>>
>>     -Kurt
>>
>>     On 2/4/17 12:29 AM, Jason Harmening wrote:
>>
>>         Hi,
>>
>>         I'm a bit confused as to how this change breaks MIPS.  The new
>>         function,
>>         get_pcpu() is intended to be used only to access the per-cpu data
>>         pointer locally.  It returns pcpup, which is the per-cpu pointer
>>         wired
>>         into the local TLB to translate to the local CPU's physical data
>>         region,
>>         correct?
>>
>>         This is the same value used by the per-CPU accessors such as
>>         PCPU_ADD
>>         and PCPU_GET.  The MI portions of this change only use get_pcpu()
>> to
>>         access  the local CPU's data, e.g. under a critical section in the
>>         rmlock.  It is not intended to be used for iterating all CPUs.
>>
>>         If I've missed something and MIPS is truly broken by this, then
>> I'll
>>         gladly revert, but (maybe because it's late) I'm not seeing
>>         where this
>>         goes wrong on MIPS.
>>
>>         Thanks,
>>         Jason
>>
>>         On Fri, Feb 3, 2017 at 8:12 PM, Alexander Kabaev
>>         <kabaev at gmail.com <mailto:kabaev at gmail.com>
>>         <mailto:kabaev at gmail.com <mailto:kabaev at gmail.com>>> wrote:
>>
>>             On Wed, 1 Feb 2017 03:32:49 +0000 (UTC)
>>             "Jason A. Harmening" <jah at FreeBSD.org> wrote:
>>
>>             > Author: jah
>>             > Date: Wed Feb  1 03:32:49 2017
>>             > New Revision: 313037
>>             > URL: https://svnweb.freebsd.org/changeset/base/313037
>>         <https://svnweb.freebsd.org/changeset/base/313037>
>>             <https://svnweb.freebsd.org/changeset/base/313037
>>         <https://svnweb.freebsd.org/changeset/base/313037>>
>>             >
>>             > Log:
>>             >   Implement get_pcpu() for the remaining architectures and
>>         use it to
>>             >   replace pcpu_find(curcpu) in MI code.
>>             >
>>             > Modified:
>>             >   head/sys/amd64/include/pcpu.h
>>             >   head/sys/kern/kern_rmlock.c
>>             >   head/sys/mips/include/pcpu.h
>>             >   head/sys/net/netisr.c
>>             >   head/sys/powerpc/include/cpufunc.h
>>             >   head/sys/powerpc/include/pcpu.h
>>             >   head/sys/sparc64/include/pcpu.h
>>             >
>>
>>             Hi,
>>
>>             this change was not reviewed nor testing was thought for all
>>             architectures it touches. The change happens to break MIPS
>> quite
>>             thoroughly, since MIPS is using different pointers when
>>         accessing PCPU
>>             area locally and when doing iterations using cpu_to_cpuid
>>         array. I
>>             therefore officially am requesting this change to be
>>         reverted until
>>             reasonable solution is found to unbreak architectures that
>>         use wired
>>             TLBs to access local per-CPU data.
>>
>>             --
>>             Alexander Kabaev
>>
>>
>>
>>
>>
>