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 <[email protected]>
AuthorDate: 2023-11-13 15:44:45 +0000
Commit: Mark Johnston <[email protected]>
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
