On Tue, Feb 15, 2022 at 09:55:52PM +0530, Naveen N. Rao wrote:

quoted

quoted

quoted

quoted

quoted

quoted

I think this is wrong. We need to differentiate
between ftrace_caller() and ftrace_regs_caller()
here, and only return pt_regs if coming in through
ftrace_regs_caller() (i.e., FL_SAVE_REGS is set).

Not sure I follow you.

This is based on 5740a7c71ab6 ("s390/ftrace: add
HAVE_DYNAMIC_FTRACE_WITH_ARGS support")

It's all the point of HAVE_DYNAMIC_FTRACE_WITH_ARGS,
have the regs also with ftrace_caller().

Sure you only have the params, but that's the same on
s390, so what did I miss ?

Steven has explained the rationale for this in his other response:
https://lore.kernel.org/all/20220215093849.556d5444@gandalf.local.home/ (local)

Thanks for this pointer, this clarifies a couple of things!

quoted

quoted

quoted

It looks like s390 is special since it apparently saves all
registers even for ftrace_caller:
https://lore.kernel.org/all/YbipdU5X4HNDWIni@osiris/ (local)

It is not what I understand from their code, see https://elixir.bootlin.com/linux/v5.17-rc3/source/arch/s390/kernel/mcount.S#L37


They have a common macro called with argument 'allregs' which is set
to 0 for ftrace_caller() and 1 for ftrace_regs_caller().
When allregs == 1, the macro seems to save more.

But ok, I can do like x86, but I need a trick to know whether
FL_SAVE_REGS is set or not, like they do with fregs->regs.cs
Any idea what the condition can be for powerpc ?

We'll need to explicitly zero-out something in pt_regs in ftrace_caller().
We can probably use regs->msr since we don't expect it to be zero when saved
from ftrace_regs_caller().

quoted

Finally, it looks like this change is done  via commit 894979689d3a
("s390/ftrace: provide separate ftrace_caller/ftrace_regs_caller
implementations") four hours the same day after the implementation of
arch_ftrace_get_regs()

They may have forgotten to change arch_ftrace_get_regs() which was added
in commit 5740a7c71ab6 ("s390/ftrace: add HAVE_DYNAMIC_FTRACE_WITH_ARGS
support") with the assumption that ftrace_caller and ftrace_regs_caller
where identical.

Indeed, good find!

Thank you for bringing this up!

So, the in both variants s390 provides nearly identical data. The only
difference is that for FL_SAVE_REGS the program status word mask is
missing; therefore it is not possible to figure out the condition code
or if interrupts were enabled/disabled.

Vasily, Sven, I think we have two options here:

- don't provide sane psw mask contents at all and say (again) that
  ptregs contents are identical

- provide (finally) a full psw mask contents using epsw, and indicate
  validity with a flags bit in pt_regs

I would vote for the second option, even though epsw is slow. But this
is about the third or fourth time this came up in different
contexts. So I'd guess we should go for the slow but complete
solution. Opinions?