Re: [PATCH] powerpc/64: ftrace don't trace real mode

[Nicholas Piggin <npiggin@gmail.com>]

Naveen N. Rao's on March 23, 2020 2:17 am:
> Nicholas Piggin wrote:
>> Naveen N. Rao's on March 21, 2020 4:39 am:
>>> Hi Nick,
>>> 
>>> Nicholas Piggin wrote:
>>>> This warns and prevents tracing attempted in a real-mode context.
>>> 
>>> Is this something you're seeing often? Last time we looked at this, KVM 
>>> was the biggest offender and we introduced paca->ftrace_enabled as a way 
>>> to disable ftrace while in KVM code.
>> 
>> Not often but it has a tendancy to blow up the least tested code at the
>> worst times :)
>> 
>> Machine check is bad, I'm sure HMI too but I haven't tested that yet.
>> 
>> I've fixed up most of it with annotations, this is obviously an extra
>> safety not something to rely on like ftrace_enabled. Probably even the
>> WARN_ON here is dangerous here, but I don't want to leave these bugs
>> in there.
> 
> Ok, makes sense.
> 
>> 
>> Although the machine check and hmi code touch a fair bit of stuff and
>> annotating is a bit fragile. It might actually be better if the
>> paca->ftrace_enabled could be a nesting counter, then we could use it
>> in machine checks too and avoid a lot of annotations.
> 
> I'm not too familiar with MC/HMI, but I suppose those aren't re-entrant?  
> If those have access to an emergency stack, can we save/restore 
> ftrace_enabled state across the handlers?

Yeah that's true. They're not highly reentrant though, we could just 
make it an 8 bit counter, it's nicer than saving / restoring from stack
(but I guess I could do that too).

> 
> We're primarily disabling ftrace across idle/offline/KVM right now. I'm 
> not sure if nesting is useful there.
> 
>> 
>>> While this is cheap when handling ftrace_regs_caller() as done in this 
>>> patch, for simple function tracing (see below), we will have to grab the 
>>> MSR which will slow things down slightly.
>> 
>> mfmsr is not too bad these days. 
> 
> That's great!

Thanks,
Nick