Re: [PATCH v3 2/8] keyhandler: drop regs parameter from handle_keyregs()

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 12/02/2024 09:13, Jan Beulich wrote:
> On 08.02.2024 23:09, Julien Grall wrote:
> > On 05/02/2024 13:28, Jan Beulich wrote:
> > > In preparation for further removal of regs parameters, drop it here. In
> > > the two places where it's actually needed, retrieve IRQ context if
> > > available, or else guest context.
> > >
> > > Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > ---
> > > As an alternative to the new boolean parameter, I wonder if we couldn't
> > > special-case the idle vCPU case: It's only there where we would not have
> > > proper context retrievable via guest_cpu_user_regs().
> > I am trying to understand the implication. Looking at the code, it seems
> > in the case where we pass NULL, we would expect to call
> > run_in_exception_handler().
> Right, when NULL was passed so far, and when true is passed now, that's
> to indicate to invoke run_in_exception_handler().
>
> > If I am not mistaken, at least for Arm, regs would not be the same as
> > guest_cpu_user_regs(). So I think your current approach is more correct.
> >
> > Did I miss anything?
> Whether regs are the same isn't overly relevant here. The thing that's
> relevant is whether what would be logged actually makes sense. And
> invoking guest_cpu_user_regs() in idle vCPU context makes no sense.
> Whereas in other contexts its result is good enough to show the present
> state of the CPU; there's no real need in such a case to go through
> run_in_exception_handler().
>
> The present approach therefore isn't necessarily "more correct", but it
> is closer to prior behavior.
>
> The corner case that makes me prefer the presently chosen approach is
> when a CPU is in the middle of scheduling, especially considering x86's
> lazy switching (when current != this_cpu(curr_vcpu). The main reason to
> mention the alternative is because iirc Andrew had suggested to move
> more in that direction.
Thanks for the clarification. I don't have any strong preference either 
way. So I would be happy to go with your solution.
Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall