* Re: [PATCH 1/3] KVM: x86: Load SMRAM in a single shot when leaving SMM
[not found] ` <20190402150311.29481-2-sean.j.christopherson@intel.com>
@ 2019-04-10 9:19 ` Paolo Bonzini
2019-04-10 14:17 ` Sean Christopherson
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2019-04-10 9:19 UTC (permalink / raw)
To: Sean Christopherson, Radim Krčmář, Joerg Roedel
Cc: kvm, Jon Doron, Jim Mattson, Liran Alon, Vitaly Kuznetsov
On 02/04/19 17:03, Sean Christopherson wrote:
> Ostensibly, the only motivation for having HF_SMM_MASK set throughout
> the loading of state from the SMRAM save state area is so that the
> memory accesses from GET_SMSTATE() are tagged with role.smm (though
> arguably even that is unnecessary).
Why is that unnecessary? If they do not have role.smm they would access
video RAM or TSEG, not the state save area.
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 0/3] KVM: x86: clear HF_SMM_MASK before loading state
[not found] <20190402150311.29481-1-sean.j.christopherson@intel.com>
[not found] ` <20190402150311.29481-2-sean.j.christopherson@intel.com>
@ 2019-04-10 9:25 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2019-04-10 9:25 UTC (permalink / raw)
To: Sean Christopherson, Radim Krčmář, Joerg Roedel
Cc: kvm, Jon Doron, Jim Mattson, Liran Alon, Vitaly Kuznetsov
On 02/04/19 17:03, Sean Christopherson wrote:
> RSM emulation is currently broken on VMX when the interrupted guest has
> CR4.VMXE=1. Similar breakage has also occurred in the past due to
> HF_SMM_MASK being cleared only after RSM completes, i.e. loading non-SMM
> state while is_smm() returns true is unsurprisingly problematic.
>
> Rather than dance around the issue of HF_SMM_MASK being set when loading
> SMSTATE into architectural state, rework RSM emulation itself to clear
> HF_SMM_MASK prior to loading architectural state. AFAICT, the only
> motivation for having HF_SMM_MASK set throughout is so that the memory
> access from GET_SMSTATE() are tagged with role.smm (though arguably even
> that is unnecessary). Sidestep that particular issue by taking the
> enter_smm() approach of reading all of SMSTATE into a buffer and then
> loading state from the buffer.
>
> The actual fix is the same concept as an earlier RFC, but without first
> moving em_rsm() to x86.c, i.e. doesn't add a big pile of dependent patches
> before fixing the bug. I'm still planning on sending a series to move
> the bulk of em_rsm() to x86.c, but it'll be a true cleanup.
>
> [1] https://patchwork.kernel.org/cover/10875623/
>
> Sean Christopherson (3):
> KVM: x86: Load SMRAM in a single shot when leaving SMM
> KVM: x86: Open code kvm_set_hflags
> KVM: x86: clear SMM flags before loading state while leaving SMM
>
> arch/x86/include/asm/kvm_emulate.h | 4 +-
> arch/x86/include/asm/kvm_host.h | 5 +-
> arch/x86/kvm/emulate.c | 160 +++++++++++++++--------------
> arch/x86/kvm/svm.c | 30 ++----
> arch/x86/kvm/vmx/vmx.c | 4 +-
> arch/x86/kvm/x86.c | 38 ++++---
> 6 files changed, 118 insertions(+), 123 deletions(-)
>
Queued, thanks. I only changed the name of the emulator callback from
smm_changed to post_leave_smm, since it is invoked only on RSM.
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/3] KVM: x86: Load SMRAM in a single shot when leaving SMM
2019-04-10 9:19 ` [PATCH 1/3] KVM: x86: Load SMRAM in a single shot when leaving SMM Paolo Bonzini
@ 2019-04-10 14:17 ` Sean Christopherson
0 siblings, 0 replies; 3+ messages in thread
From: Sean Christopherson @ 2019-04-10 14:17 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Radim Krčmář,
Joerg Roedel, kvm, Jon Doron, Jim Mattson, Liran Alon,
Vitaly Kuznetsov
On Wed, Apr 10, 2019 at 11:19:19AM +0200, Paolo Bonzini wrote:
> On 02/04/19 17:03, Sean Christopherson wrote:
> > Ostensibly, the only motivation for having HF_SMM_MASK set throughout
> > the loading of state from the SMRAM save state area is so that the
> > memory accesses from GET_SMSTATE() are tagged with role.smm (though
> > arguably even that is unnecessary).
>
> Why is that unnecessary? If they do not have role.smm they would access
> video RAM or TSEG, not the state save area.
You're right, feel free to strike that line from the record. For some
reason I had it in my mind that enter_smm() was writing SMRAM before
setting HF_SMM_MASK.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-10 14:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190402150311.29481-1-sean.j.christopherson@intel.com>
[not found] ` <20190402150311.29481-2-sean.j.christopherson@intel.com>
2019-04-10 9:19 ` [PATCH 1/3] KVM: x86: Load SMRAM in a single shot when leaving SMM Paolo Bonzini
2019-04-10 14:17 ` Sean Christopherson
2019-04-10 9:25 ` [PATCH 0/3] KVM: x86: clear HF_SMM_MASK before loading state Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).