[PATCH 4/4] arm64/fpsimd: Document the use of TIF_FOREIGN_FPSTATE by KVM
Marc Zyngier
maz at kernel.org
Wed Oct 27 04:26:33 PDT 2021
On Thu, 21 Oct 2021 16:57:15 +0100,
Mark Brown <broonie at kernel.org> wrote:
>
> [1 <text/plain; us-ascii (quoted-printable)>]
> On Thu, Oct 21, 2021 at 04:11:24PM +0100, Marc Zyngier wrote:
> > The bit of documentation that talks about TIF_FOREIGN_FPSTATE
> > does not mention the ungodly tricks that KVM plays with this flag.
> >
> > Try and document this for the posterity.
>
> Yes, more documentation here would definitely be helpful - it's pretty
> hard to follow what KVM is doing here.
>
> > * CPU currently contain the most recent userland FPSIMD state of the current
> > - * task.
> > + * task *or* the state of the corresponding KVM vcpu if userspace is behaving
> > + * as a VMM and that the vcpu has used FP during its last run. In the latter
> > + * case, KVM will set TIF_FOREIGN_FPSTATE on kvm_vcpu_put(). For all intents
> > + * and purposes, the vcpu FP state is treated identically to userspace's.
>
> I'm not able to find a kvm_vcpu_put() function in upstream, just
> kvm_cpu_put_sysregs_vhe(). There's kvm_arch_vcpu_put() which is called
> from the vcpu_put() function in generic KVM code but they don't show up
> until you start mangling the name in that comment.
You, vcpu_put() is the one I had in mind.
> It'd be good to
> mention what vcpu_put() is actually doing and a bit more about the
> general model, KVM is behaving differently here AFAICT in that it flags
> the current state as invalid when it saves the context to memory rather
> than when an event happens that requires that the context be reloaded.
> There's no problem there but it's a bit surprising due the difference
> and worth highlighting.
There is a bit more to it: KVM flags the userspace state as invalid,
but also ties the guest state to the current task via
fpsimd_bind_state_to_cpu() so that the state can be saved on
vcpu_put() via fpsimd_save_and_flush_cpu_state(), or if we end-up
running kernel_neon_begin() because of some softirq handling.
> I think I'd also be inclined to restructure this to foreground the fact
> that it's the state of the current task but that task may be a VMM. So
> something more like
>
> ...contain the most recent FPSIMD state of the current userspace
> task. If the task is behaving as a VMM then this will be
> managed by KVM which will...
>
> making it a bit easier to follow (assuming my understanding of what's
> going on is correct, if not then I guess something else needs
> clarifying!).
I'll have a go at rewriting this.
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list