Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Paolo Bonzini <pbonzini@redhat.com>,
        "Liu, Jing2" <jing2.liu@intel.com>,
        LKML <linux-kernel@vger.kernel.org>
Cc:     "x86@kernel.org" <x86@kernel.org>,
        "Bae, Chang Seok" <chang.seok.bae@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Arjan van de Ven <arjan@linux.intel.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "Nakajima, Jun" <jun.nakajima@intel.com>,
        Jing Liu <jing2.liu@linux.intel.com>,
        "seanjc@google.com" <seanjc@google.com>,
        "Cooper, Andrew" <andrew.cooper3@citrix.com>
Subject: Re: [patch 13/31] x86/fpu: Move KVMs FPU swapping to FPU core
In-Reply-To: <87lf2v5shb.ffs@tglx>
References: <871r4p9fyh.ffs@tglx>
 <ec9c761d-4b5c-71e2-c1fc-d256b6b78c04@redhat.com>
 <BL0PR11MB3252511FC48E43484DE79A3CA9B89@BL0PR11MB3252.namprd11.prod.outlook.com>
 <6bbc5184-a675-1937-eb98-639906a9cf15@redhat.com> <87wnmf66m5.ffs@tglx>
 <3997787e-402d-4b2b-0f90-4a672c77703f@redhat.com> <87lf2v5shb.ffs@tglx>
Date:   Fri, 15 Oct 2021 11:36:25 +0200
Message-ID: <87a6ja6352.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Paolo,

On Thu, Oct 14 2021 at 21:14, Thomas Gleixner wrote:
> On Thu, Oct 14 2021 at 17:01, Paolo Bonzini wrote:
>>> vcpu_create()
>>> 
>>>    fpu_init_fpstate_user(guest_fpu, supported_xcr0)
>>> 
>>> That will (it does not today) do:
>>> 
>>>       guest_fpu::__state_perm = supported_xcr0 & xstate_get_group_perm();
>>> 
>>> The you have the information you need right in the guest FPU.
>>
>> Good, I wasn't aware of the APIs that will be there.
>
> Me neither, but that's a pretty obvious consequence of the work I'm
> doing for AMX. So I made it up for you. :)

let me make some more up for you!

If you carefully look at part 2 of the rework, then you might notice
that there is a fundamental change which allows to do a real
simplification for KVM FPU handling:

   current->thread.fpu.fpstate

is now a pointer. So you can spare one FPU allocation because we can now
do:

fpu_attach_guest_fpu(supported_xcr0)
{
        guest_fpstate = alloc_fpstate(supported_xcr0);
        fpu_init_fpstate_user(guest_fpstate, supported_xcr0);
        current->thread.fpu.guest_fpstate = guest_fpstate;
}

fpu_swap_kvm_fpu() becomes in the first step:

fpu_swap_kvm_fpu(bool enter_guest)
{
        safe_fpregs_to_fpstate(current->thread.fpu.fpstate);

        swap(current->thread.fpu.fpstate, current->thread.fpu.guest_fpstate);

        restore_fpregs_from_fpstate(current->thread.fpu.fpstate);
}

@enter guest will allow to do some sanity checks

In a second step:

fpu_swap_kvm_fpu(bool enter_guest, u64 guest_needs_features)
{
        possibly_reallocate(enter_guest, guest_needs_features);
        safe_fpregs_to_fpstate(current->thread.fpu.fpstate);

        swap(current->thread.fpu.fpstate, current->thread.fpu.guest_fpstate);

        restore_fpregs_from_fpstate(current->thread.fpu.fpstate);
        possibly_reallocate(enter_guest, guest_needs_features);
}

@guest_needs_features is the information which you gather via guest XCR0
and guest XFD.

So fpu_swap_kvm_fpu() is going to be the place where reallocation happens
and that's good enough for both cases:

vcpu_run()

     fpu_swap_kvm_fpu(); <- 1

     while (...)
           vmenter();

     fpu_swap_kvm_fpu(); <- 2

#1 QEMU user space used feature and has already large fpstate

#2 Guest requires feature but has not used it yet (XCR0/XFD trapping)

See?

It's not only correct, it's also simple and truly beautiful.

Thanks,

        tglx