Re: git: f05795e3f65f - main - arm64: write PID in CONTEXTIDR_EL1 on ctx switch

On 10 Sep 2024, at 19:04, Jessica Clarke <jrtc27@FreeBSD.org> wrote:
> 
> On 9 Sep 2024, at 17:05, Andrew Turner <andrew@FreeBSD.org> wrote:
>> 
>> The branch main has been updated by andrew:
>> 
>> URL: https://cgit.FreeBSD.org/src/commit/?id=f05795e3f65f305cb770ae91d8e9c8f05d267e0d
>> 
>> commit f05795e3f65f305cb770ae91d8e9c8f05d267e0d
>> Author:     Zachary Leaf <zachary.leaf@arm.com>
>> AuthorDate: 2024-07-31 08:23:24 +0000
>> Commit:     Andrew Turner <andrew@FreeBSD.org>
>> CommitDate: 2024-09-09 16:03:19 +0000
>> 
>>   arm64: write PID in CONTEXTIDR_EL1 on ctx switch
>> 
>>   Debug and trace features such as Statistical Profiling Extension (SPE)
>>   use the CONTEXTIDR_EL1 register to get the PID of the current process.
>> 
>>   Add a sysctl switch to toggle writing the current PID into this register
>>   in the thread switcher.
>> 
>>   To make use of the feature, the following sysctl switch must be set:
>> 
>>       sysctl machdep.pid_in_contextidr=1
>> 
>>   Kernel code can also toggle the sysctl by writing directly to the global
>>   var behind the sysctl arm64_pid_in_contextidr:
>> 
>>       extern bool arm64_pid_in_contextidr;
>> 
>>   Reviewed by:    andrew
>>   Sponsored by:   Arm Ltd
>>   Differential Revision:  https://reviews.freebsd.org/D46192
>> ---
>> sys/arm64/arm64/genassym.c    |  2 ++
>> sys/arm64/arm64/swtch.S       | 17 +++++++++++++++++
>> sys/arm64/arm64/sys_machdep.c |  6 ++++++
>> 3 files changed, 25 insertions(+)
>> 
>> diff --git a/sys/arm64/arm64/genassym.c b/sys/arm64/arm64/genassym.c
>> index a4db825e976c..5a20169d51c3 100644
>> --- a/sys/arm64/arm64/genassym.c
>> +++ b/sys/arm64/arm64/genassym.c
>> @@ -58,6 +58,8 @@ ASSYM(PCB_TPIDRRO, offsetof(struct pcb, pcb_tpidrro_el0));
>> ASSYM(PCB_ONFAULT, offsetof(struct pcb, pcb_onfault));
>> ASSYM(PCB_FLAGS, offsetof(struct pcb, pcb_flags));
>> 
>> +ASSYM(PR_PID, offsetof(struct proc, p_pid));
>> +
>> ASSYM(SF_UC, offsetof(struct sigframe, sf_uc));
>> 
>> ASSYM(TD_PROC, offsetof(struct thread, td_proc));
>> diff --git a/sys/arm64/arm64/swtch.S b/sys/arm64/arm64/swtch.S
>> index 6af70ca839a0..3a2bf2cb5a7f 100644
>> --- a/sys/arm64/arm64/swtch.S
>> +++ b/sys/arm64/arm64/swtch.S
>> @@ -55,6 +55,15 @@
>> 999:
>> .endm
>> 
>> +.macro pid_in_context_idr label
>> + adrp x9, arm64_pid_in_contextidr
>> + ldr x10, [x9, :lo12:arm64_pid_in_contextidr]
> 
> This loads a 64-bit value, not an 8-bit bool as the variable is
> defined. Depending on the alignment of arm64_pid_in_contextidr this can
> even fail to link, and this shows up in GitHub Actions with the
> clang-12 and clang-13 jobs failing.

Similarly the load of p_pid that follows is reading an int32_t with a
64-bit load.

Jess