All of lore.kernel.org
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: "Wang, Wei W" <wei.w.wang@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Jing Liu <jing2.liu@linux.intel.com>,
	"Zhong, Yang" <yang.zhong@intel.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Sean Christoperson <seanjc@google.com>,
	"Nakajima, Jun" <jun.nakajima@intel.com>,
	"Tian, Kevin" <kevin.tian@intel.com>
Subject: Re: [patch 5/6] x86/fpu: Provide fpu_update_guest_xcr0/xfd()
Date: Tue, 14 Dec 2021 22:35:43 +0100	[thread overview]
Message-ID: <878rwm7tu8.fsf@secure.mitica> (raw)
In-Reply-To: <87k0g7rkwj.ffs@tglx> (Thomas Gleixner's message of "Tue, 14 Dec 2021 21:28:28 +0100")

Thomas Gleixner <tglx@linutronix.de> wrote:

Hi Thomas

> On Tue, Dec 14 2021 at 20:07, Juan Quintela wrote:
>> Thomas Gleixner <tglx@linutronix.de> wrote:
>>> On Tue, Dec 14 2021 at 16:11, Wei W. Wang wrote:
>>>> We need to check with the QEMU migration maintainer (Dave and Juan CC-ed)
>>>> if changing that ordering would be OK.
>>>> (In general, I think there are no hard rules documented for this ordering)
>>>
>>> There haven't been ordering requirements so far, but with dynamic
>>> feature enablement there are.
>>>
>>> I really want to avoid going to the point to deduce it from the
>>> xstate:xfeatures bitmap, which is just backwards and Qemu has all the
>>> required information already.
>>
>> First of all, I claim ZERO knowledge about low level x86_64.
>
> Lucky you.

Well, that is true until I have to debug some bug, at that time I miss
the knowledge O:-)

>> Once told that, this don't matter for qemu migration, code is at
>
> Once, that was at the time where rubber boots were still made of wood,
> right? :)

I forgot to add: "famous last words".

>> target/i386/kvm/kvm.c:kvm_arch_put_registers()
>>
>>
>>     ret = kvm_put_xsave(x86_cpu);
>>     if (ret < 0) {
>>         return ret;
>>     }
>>     ret = kvm_put_xcrs(x86_cpu);
>>     if (ret < 0) {
>>         return ret;
>>     }
>>     /* must be before kvm_put_msrs */
>>     ret = kvm_inject_mce_oldstyle(x86_cpu);
>
> So this has already ordering requirements.
>
>>     if (ret < 0) {
>>         return ret;
>>     }
>>     ret = kvm_put_msrs(x86_cpu, level);
>>     if (ret < 0) {
>>         return ret;
>>     }
>>
>> If it needs to be done in any other order, it is completely independent
>> of whatever is inside the migration stream.
>
> From the migration data perspective that's correct, but I have the
> nagging feeling that this in not that simple.

Oh, I was not meaning that it was simple at all.

We have backward compatibility baggage on x86_64 that is grotesque at
this point.  We don't send a single msr (by that name) on the main
migration stream.  And then we send them based on "conditions".  So the
trick as somithing like:

- qemu reads the msrs from kvm at some order
- it stores them in a list of MSR's
- On migration pre_save we "mangle" that msr's and other cpu state to
  on the main (decades old) state
- then we send the main state
- then we send conditionally the variable state

on reception side:

- we receive everything that the sending side have sent
- we "demangle" it on pre_load
- then we create the list of MSR's that need to be transferred
- at that point we send them to kvm in another random order

So yes, I fully agree that it is not _that_ simple O:-)


>> I guess that Paolo will put some light here.
>
> I fear shining light on that will unearth quite a few skeletons :)

It is quite probable.

When a bugzilla start with: We found a bug while we were trying to
migrate during (BIOS) boot, I just ran for the hills O:-)

Later, Juan.


  reply	other threads:[~2021-12-14 21:35 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-14  2:50 [patch 0/6] x86/fpu: Preparatory changes for guest AMX support Thomas Gleixner
2021-12-14  2:50 ` [patch 1/6] x86/fpu: Extend fpu_xstate_prctl() with guest permissions Thomas Gleixner
2021-12-14  5:13   ` Tian, Kevin
2021-12-14 10:37     ` Paolo Bonzini
2021-12-14  2:50 ` [patch 2/6] x86/fpu: Prepare guest FPU for dynamically enabled FPU features Thomas Gleixner
2021-12-14  2:50 ` [patch 3/6] x86/fpu: Make XFD initialization in __fpstate_reset() a function argument Thomas Gleixner
2021-12-14  2:50 ` [patch 4/6] x86/fpu: Add guest support to xfd_enable_feature() Thomas Gleixner
2021-12-14  6:05   ` Tian, Kevin
2021-12-14 10:21     ` Paolo Bonzini
2021-12-14 13:15       ` Thomas Gleixner
2021-12-15  5:46         ` Tian, Kevin
2021-12-15  9:53           ` Thomas Gleixner
2021-12-15 10:02             ` Tian, Kevin
2021-12-14  2:50 ` [patch 5/6] x86/fpu: Provide fpu_update_guest_xcr0/xfd() Thomas Gleixner
2021-12-14  6:25   ` Tian, Kevin
2021-12-14 15:09   ` Wang, Wei W
2021-12-14 15:40     ` Thomas Gleixner
2021-12-14 16:11       ` Wang, Wei W
2021-12-14 18:04         ` Thomas Gleixner
2021-12-14 19:07           ` Juan Quintela
2021-12-14 20:28             ` Thomas Gleixner
2021-12-14 21:35               ` Juan Quintela [this message]
2021-12-15  2:17                 ` Wang, Wei W
2021-12-15 10:09                   ` Thomas Gleixner
2021-12-15 10:27                     ` Paolo Bonzini
2021-12-15 10:41                       ` Paolo Bonzini
2021-12-16  1:00                         ` Tian, Kevin
2021-12-16  5:36                         ` Tian, Kevin
2021-12-16 21:07                           ` Paolo Bonzini
2021-12-16 10:21                         ` Tian, Kevin
2021-12-16 10:24                           ` Paolo Bonzini
2021-12-16 10:26                           ` Paolo Bonzini
2021-12-16 13:00                         ` Tian, Kevin
2021-12-16  1:04                       ` Tian, Kevin
2021-12-16  9:34                         ` Thomas Gleixner
2021-12-16  9:59                           ` Tian, Kevin
2021-12-16 14:12                             ` Thomas Gleixner
2021-12-17 15:33                               ` Tian, Kevin
2021-12-15  6:14   ` Tian, Kevin
2021-12-14  2:50 ` [patch 6/6] x86/fpu: Provide kvm_sync_guest_vmexit_xfd_state() Thomas Gleixner
2021-12-15  6:35   ` Liu, Jing2
2021-12-15  9:49     ` Thomas Gleixner
2021-12-14  6:50 ` [patch 0/6] x86/fpu: Preparatory changes for guest AMX support Tian, Kevin
2021-12-14  6:52 ` Liu, Jing2
2021-12-14  7:54   ` Tian, Kevin
2021-12-14 10:42 ` Paolo Bonzini
2021-12-14 13:24   ` 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=878rwm7tu8.fsf@secure.mitica \
    --to=quintela@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=jing2.liu@linux.intel.com \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=wei.w.wang@intel.com \
    --cc=x86@kernel.org \
    --cc=yang.zhong@intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.