public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>
Cc: "Liu, Jing2" <jing2.liu@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Nakajima, Jun" <jun.nakajima@intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Jing Liu <jing2.liu@linux.intel.com>,
	"Cooper, Andrew" <andrew.cooper3@citrix.com>,
	"Bae, Chang Seok" <chang.seok.bae@intel.com>
Subject: Re: Thoughts of AMX KVM support based on latest kernel
Date: Tue, 16 Nov 2021 21:36:03 +0100	[thread overview]
Message-ID: <87zgq34z4c.ffs@tglx> (raw)
In-Reply-To: <04978d6d-8e1a-404d-b30d-402a7569c1f0@redhat.com>

Paolo,

On Tue, Nov 16 2021 at 20:49, Paolo Bonzini wrote:
> On 11/16/21 19:55, Thomas Gleixner wrote:
>> We can do that, but I'm unhappy about this conditional in schedule(). So
>> I was asking for doing a simple KVM only solution first:
>> 
>> vcpu_run()
>>          kvm_load_guest_fpu()
>>              wrmsrl(XFD, guest_fpstate->xfd);
>>              XRSTORS
>>            
>>          do {
>> 
>>             local_irq_disable();
>> 
>>             if (test_thread_flag(TIF_NEED_FPU_LOAD))
>> 		switch_fpu_return()
>>                    wrmsrl(XFD, guest_fpstate->xfd);
>> 
>>             do {
>>                  vmenter();              // Guest modifies XFD
>>             } while (reenter);
>> 
>>             update_xfd_state();          // Restore consistency
>> 
>>             local_irq_enable();
>> 
>> and check how bad that is for KVM in terms of overhead on AMX systems.
>
> I agree, this is how we handle SPEC_CTRL for example and it can be 
> extended to XFD.

SPEC_CTRL is different because it's done right after each VMEXIT.

XFD can be done lazy when breaking out of the exit fastpath loop before
enabling interrupts.

> We should first do that, then switch to the MSR lists. 
>   Hacking into schedule() should really be the last resort.
>
>>            local_irq_enable();     <- Problem starts here
>> 
>>            preempt_enable();	   <- Becomes wider here
>
> It doesn't become that much wider because there's always preempt 
> notifiers.  So if it's okay to save XFD in the XSAVES wrapper and in 
> kvm_arch_vcpu_put(), that might be already remove the need to do it 
> schedule().

Did not think about preemption notifiers. Probably because I hate
notifiers with a passion since I had to deal with the CPU hotplug
notifier trainwreck.

But yes that would work. So the places to do that would be:

1) kvm_sched_out() -> kvm_arch_vcpu_put()
2) kernel_fpu_begin_mask()
3) kvm_put_guest_fpu()

But I really would start with the trivial version I suggested because
that's already in the slow path and not at every VMEXIT.

I'd be really surprised if that RDMSR is truly noticeable within all the
other crud this path is doing.

Thanks,

        tglx

  parent reply	other threads:[~2021-11-16 20:36 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-10 13:01 Thoughts of AMX KVM support based on latest kernel Liu, Jing2
2021-11-16 12:18 ` Thomas Gleixner
2021-11-16 16:05   ` Sean Christopherson
2021-11-16 18:55     ` Thomas Gleixner
2021-11-16 19:49       ` Paolo Bonzini
2021-11-16 20:14         ` Sean Christopherson
2021-11-16 20:36         ` Thomas Gleixner [this message]
2021-11-16 22:11           ` Nakajima, Jun
2021-11-17  7:39           ` Paolo Bonzini
2021-11-16 20:12       ` Sean Christopherson
2021-11-16 23:35         ` Thomas Gleixner
2021-11-16 19:20 ` Thomas Gleixner
2021-11-17  4:52   ` Nakajima, Jun
2021-11-17  7:31     ` Paolo Bonzini
2021-11-17 10:15       ` Tian, Kevin
2021-11-17 12:24         ` Thomas Gleixner
2021-11-17 12:53         ` Paolo Bonzini
2021-11-18 23:17           ` Nakajima, Jun
2021-11-19 10:13             ` Thomas Gleixner
2021-11-19 15:41               ` Nakajima, Jun
2021-12-08  0:50                 ` Rewording of Setting IA32_XFD[18] (Re: Thoughts of AMX KVM support based on latest kernel) Nakajima, Jun
2021-12-08 13:57                   ` Paolo Bonzini
2021-12-10 21:53                   ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zgq34z4c.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=andrew.cooper3@citrix.com \
    --cc=arjan@linux.intel.com \
    --cc=chang.seok.bae@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=jing2.liu@intel.com \
    --cc=jing2.liu@linux.intel.com \
    --cc=jun.nakajima@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox