Re: [PATCH RFC] KVM: nSVM: Fix L1 state corruption upon return from SMM

From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	Maxim Levitsky <mlevitsk@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>,
	Cathy Avery <cavery@redhat.com>,
	Emanuele Giuseppe Esposito <eesposit@redhat.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH RFC] KVM: nSVM: Fix L1 state corruption upon return from SMM
Date: Thu, 24 Jun 2021 09:41:57 +0200	[thread overview]
Message-ID: <87lf6z4sl6.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <82327cd1-92ca-9f6b-3af0-8215e9d21eae@redhat.com>

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 23/06/21 18:21, Sean Christopherson wrote:
>> On Wed, Jun 23, 2021, Sean Christopherson wrote:
>>> And I believe this hackery is necessary only because nested_svm_vmexit() isn't
>>> following the architcture in the first place.  I.e. using vmcb01 to restore
>>> host state is flat out wrong.
>> 
>> Ah, that's not true, using vmcb01 is allowed by "may store some or all host state
>> in hidden on-chip memory".
>
> And also, "Different implementations may choose to save the hidden parts 
> of the host’s segment registers as well as the selectors".
>
>>  From a performance perspective, I do like the SMI/RSM shenanigans.  I'm not
>> totally opposed to the trickery since I think it will break a guest if and only
>> if the L1 guest is also violating the APM.  And we're not fudging the spec thaat
>> much :-)
>
> Yeah, that was my reasoning as well.  Any reference to "hidden on-chip 
> memory", plus the forbidding modifications of the host save area, sort 
> of implies that the processor can actually flush that hidden on-chip 
> memory for whatever reason (such as on some sleep states?!?).

Ok, so it seems nobody feel strongly against the idea I've implemented
in the RFC. We could've avoided saving L1 host state upon L2 enter in
vmcb01 altogether, true, but we would still need some sort of a cache
emulating "hidden on-chip memory" for performance reasons. Resurrecting
'hsave'/ allocating 'special' vmcb01_smm/... and modifying nested state
seems to be unneeded, the L2->SMM case should be rare indeed.

I'll add a testcase to smm selftest and submit v1 then, thanks!

-- 
Vitaly