[PATCH v2] arm64: add the printing of tpidr_elx in __show_regs()

[Leizhen (ThunderTown)]

On 2022/4/28 19:07, Leizhen (ThunderTown) wrote:
> 
> 
> On 2022/4/28 18:21, Will Deacon wrote:
>> On Wed, Mar 16, 2022 at 02:24:08PM +0800, Zhen Lei wrote:
>>> Commit 7158627686f0 ("arm64: percpu: implement optimised pcpu access
>>> using tpidr_el1") and commit 6d99b68933fb ("arm64: alternatives: use
>>> tpidr_el2 on VHE hosts") use tpidr_elx to cache my_cpu_offset to optimize
>>> pcpu access. However, when performing reverse execution based on the
>>> registers and the memory contents in kdump, this information is sometimes
>>> required if there is a pcpu access.
>>>
>>> Signed-off-by: Zhen Lei <thunder.leizhen at huawei.com>
>>> ---
>>>  arch/arm64/kernel/process.c | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> v1 --> v2:
>>> Directly print the tpidr_elx register of the current exception level.
>>> Avoid coupling with the implementation of 'my_cpu_offset'.
>>>
>>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>>> index 5369e649fa79ff8..738932e6fa4e947 100644
>>> --- a/arch/arm64/kernel/process.c
>>> +++ b/arch/arm64/kernel/process.c
>>> @@ -216,6 +216,17 @@ void __show_regs(struct pt_regs *regs)
>>>  	show_regs_print_info(KERN_DEFAULT);
>>>  	print_pstate(regs);
>>>  
>>> +	switch (read_sysreg(CurrentEL)) {
>>
>> This should use is_kernel_in_hyp_mode() to detect if we're running at El2.

static inline bool is_kernel_in_hyp_mode(void)
{
        return read_sysreg(CurrentEL) == CurrentEL_EL2;
}

I think it's more intuitive to use "switch (read_sysreg(CurrentEL))".

>>
>>> +	case CurrentEL_EL1:
>>> +		printk("tpidr_el1 : %016llx\n", read_sysreg(TPIDR_EL1));
>>> +		break;
>>> +	case CurrentEL_EL2:
>>> +		printk("tpidr_el2 : %016llx\n", read_sysreg(TPIDR_EL2));
>>> +		break;
>>> +	default:
>>> +		break;
>>> +	}
>>
>> I think this path can be triggered directly from usermode, so we really
>> shouldn't be printing raw kernel virtual addresses here.
> 
> I run echo c > /proc/sysrq-trigger and didn't trigger this path, but maybe
> there's another way. Analysis from the other side, except for the instruction
> address, all generic registers r0-r31 is output as raw. There's also an
> opportunity to contain the instruction address.

On second thought, there seemed to be nothing wrong with it. The user need
to have capable() first. Then the address of the perpcu memory is not static,
the memory is dynamically allocated, exposing it is no different than exposing sp.

> 
> So how about:
> +       if (oops_in_progress)
> +               printk("tpidr : %016lx\n", __my_cpu_offset);
> 
>>
>> Will
>> .
>>
> 

-- 
Regards,
  Zhen Lei