Re: [PATCH 2/8] arm64: remove __exception annotations

[James Morse <james.morse@arm.com>]

Hi guys,

On 04/10/2019 15:10, Masami Hiramatsu wrote:
> On Fri, 4 Oct 2019 11:17:17 +0100
> Mark Rutland <mark.rutland@arm.com> wrote:
>> On Thu, Oct 03, 2019 at 06:16:36PM +0100, James Morse wrote:
>>> Since commit 732674980139 ("arm64: unwind: reference pt_regs via embedded
>>> stack frame") arm64 has has not used the __exception annotation to dump
>>> the pt_regs during stack tracing. in_exception_text() has no callers.
>>>
>>> This annotation is only used to blacklist kprobes, it means the same as
>>> __kprobes.
>>>
>>> Section annotations like this require the functions to be grouped
>>> together between the start/end markers, and placed according to
>>> the linker script. For kprobes we also have NOKPROBE_SYMBOL() which
>>> logs the symbol address in a section that kprobes parses and
>>> blacklists at boot.
>>>
>>> Using NOKPROBE_SYMBOL() instead lets kprobes publish the list of
>>> blacklisted symbols, and saves us from having an arm64 specific
>>> spelling of __kprobes.

>>> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
>>> index a17393ff6677..b0b3ba56e919 100644
>>> --- a/arch/arm64/include/asm/exception.h
>>> +++ b/arch/arm64/include/asm/exception.h
>>
>> [...]
>>
>>> -asmlinkage void __exception do_debug_exception(unsigned long addr_if_watchpoint,
>>> -					       unsigned int esr,
>>> -					       struct pt_regs *regs)
>>> +asmlinkage void do_debug_exception(unsigned long addr_if_watchpoint,
>>> +				   unsigned int esr, struct pt_regs *regs)
>>>  {
>>>  	const struct fault_info *inf = esr_to_debug_fault_info(esr);
>>>  	unsigned long pc = instruction_pointer(regs);
>>
>> I assume you meant to add NOKPROBE_SYMBOL(do_debug_exception) here.
>>
>> Assuming so, and with that fixed up:
>>
>> Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> Good catch, if so, this looks good to me too.

I should have noted it in the commit message, but the NOKPROBE_SYMBOL(do_debug_exception)
is already there! Added by commit 2dd0e8d2d2a15 ("arm64: Kprobes with single stepping
support").

(kprobing the debug handler is so bad, we blacklist it twice!)

I'll fix the commit message.


> with that fixed up:
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

I assume you're both happy for me to apply these tags.


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel