Date: Thu, 29 Feb 2024 09:17:56 -0800 From: John Baldwin <jhb@FreeBSD.org> To: Mark Johnston <markj@FreeBSD.org>, src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: b9c0003f0fa3 - main - arm64: Initialize x18 for APs earlier during boot Message-ID: <d0bc8ecd-0fe6-465d-b2c8-8a9ed8215fb7@FreeBSD.org> In-Reply-To: <202311131551.3ADFpHhg004260@gitrepo.freebsd.org> References: <202311131551.3ADFpHhg004260@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11/13/23 7:51 AM, Mark Johnston wrote: > The branch main has been updated by markj: > > URL: https://cgit.FreeBSD.org/src/commit/?id=b9c0003f0fa39ead4bb3953b9118ae6f08e560f8 > > commit b9c0003f0fa39ead4bb3953b9118ae6f08e560f8 > Author: Mark Johnston <markj@FreeBSD.org> > AuthorDate: 2023-11-13 15:44:45 +0000 > Commit: Mark Johnston <markj@FreeBSD.org> > CommitDate: 2023-11-13 15:44:45 +0000 > > arm64: Initialize x18 for APs earlier during boot > > When KMSAN is configured, the instrumentation inserts calls to > __msan_get_context_state() into all function prologues. The > implementation dereferences curthread and thus assumes that x18 points > to the PCPU area. This applies in particular to init_secondary(), which > currently is responsible for initializing x18 for APs. > > Move initialization into locore to avoid this problem. No functional > change intended. > > Reviewed by: kib, andrew > MFC after: 2 weeks > Sponsored by: Juniper Networks, Inc. > Sponsored by: Klara, Inc. > Differential Revision: https://reviews.freebsd.org/D42533 (Sorry for just now replying, I'm looking at this in more detail while merging to CheriBSD) I think we can remove a bunch of now-dead code from init_secondary after this, and probably don't even need to pass the cpu number to init_secondary either? That is: void init_secondary(uint64_t cpu) { struct pcpu *pcpup; pmap_t pmap0; uint64_t mpidr; ptrauth_mp_start(cpu); /* * Verify that the value passed in 'cpu' argument (aka context_id) is * valid. Some older U-Boot based PSCI implementations are buggy, * they can pass random value in it. */ mpidr = READ_SPECIALREG(mpidr_el1) & CPU_AFF_MASK; if (cpu >= MAXCPU || cpuid_to_pcpu[cpu] == NULL || PCPU_GET_MPIDR(cpuid_to_pcpu[cpu]) != mpidr) { for (cpu = 0; cpu < mp_maxid; cpu++) if (cpuid_to_pcpu[cpu] != NULL && PCPU_GET_MPIDR(cpuid_to_pcpu[cpu]) == mpidr) break; if ( cpu >= MAXCPU) panic("MPIDR for this CPU is not in pcpu table"); } /* * Identify current CPU. This is necessary to setup * affinity registers and to provide support for * runtime chip identification. * * We need this before signalling the CPU is ready to * let the boot CPU use the results. */ pcpup = cpuid_to_pcpu[cpu]; pcpup->pc_midr = get_midr(); identify_cpu(cpu); Can I think instead become something like: void init_secondary(void) { struct pcpu *pcpup; pmap_t pmap0; uint64_t cpu; cpu = PCPU_GET(cpuid); ptrauth_mp_start(cpu); /* * Identify current CPU. This is necessary to setup * affinity registers and to provide support for * runtime chip identification. * * We need this before signalling the CPU is ready to * let the boot CPU use the results. */ pcpup = get_pcpu(); pcpup->pc_midr = get_midr(); identify_cpu(cpu); Maybe we want to keep an explicit panic if PCPU_GET_MPIDR(pcpup) doesn't match the mpidr from the register? -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?d0bc8ecd-0fe6-465d-b2c8-8a9ed8215fb7>