On Thu, Jan 28, 2021 at 4:43 PM Babu Moger <[email protected]> wrote:
> This support also fixes an issue where a guest may sometimes see an
> inconsistent value for the SPEC_CTRL MSR on processors that support
> this feature. With the current SPEC_CTRL support, the first write to
> SPEC_CTRL is intercepted and the virtualized version of the SPEC_CTRL
> MSR is not updated. When the guest reads back the SPEC_CTRL MSR, it
> will be 0x0, instead of the actual expected value. There isn’t a
> security concern here, because the host SPEC_CTRL value is or’ed with
> the Guest SPEC_CTRL value to generate the effective SPEC_CTRL value.
> KVM writes with the guest's virtualized SPEC_CTRL value to SPEC_CTRL
> MSR just before the VMRUN, so it will always have the actual value
> even though it doesn’t appear that way in the guest. The guest will
> only see the proper value for the SPEC_CTRL register if the guest was
> to write to the SPEC_CTRL register again. With Virtual SPEC_CTRL
> support, the save area spec_ctrl is properly saved and restored.
> So, the guest will always see the proper value when it is read back.
Note that there are actually two significant problems with the way the
new feature interacts with the KVM code before this patch:
1) All bits set by the first non-zero write become sticky until the
vCPU is reset (because svm->spec_ctrl is never modified after the
first non-zero write).
2) The current guest IA32_SPEC_CTRL value isn't actually known to the
hypervisor. It thinks that there are no writes to the MSR after the
first non-zero write, so that sticky value will be returned to
KVM_GET_MSRS. This breaks live migration.
Basically, an always-on V_SPEC_CTRL breaks existing hypervisors. It
must, therefore, default to off. However, I see that our Rome and
Milan CPUs already report the existence of this feature.
On 6/3/22 22:11, Jim Mattson wrote:
> On Thu, Jan 28, 2021 at 4:43 PM Babu Moger <[email protected]> wrote:
>
>> This support also fixes an issue where a guest may sometimes see an
>> inconsistent value for the SPEC_CTRL MSR on processors that support
>> this feature. With the current SPEC_CTRL support, the first write to
>> SPEC_CTRL is intercepted and the virtualized version of the SPEC_CTRL
>> MSR is not updated. When the guest reads back the SPEC_CTRL MSR, it
>> will be 0x0, instead of the actual expected value. There isn’t a
>> security concern here, because the host SPEC_CTRL value is or’ed with
>> the Guest SPEC_CTRL value to generate the effective SPEC_CTRL value.
>> KVM writes with the guest's virtualized SPEC_CTRL value to SPEC_CTRL
>> MSR just before the VMRUN, so it will always have the actual value
>> even though it doesn’t appear that way in the guest. The guest will
>> only see the proper value for the SPEC_CTRL register if the guest was
>> to write to the SPEC_CTRL register again. With Virtual SPEC_CTRL
>> support, the save area spec_ctrl is properly saved and restored.
>> So, the guest will always see the proper value when it is read back.
>
> Note that there are actually two significant problems with the way the
> new feature interacts with the KVM code before this patch:
> 1) All bits set by the first non-zero write become sticky until the
> vCPU is reset (because svm->spec_ctrl is never modified after the
> first non-zero write).
When X86_FEATURE_V_SPEC_CTRL is set, then svm->spec_ctrl isn't used.
> 2) The current guest IA32_SPEC_CTRL value isn't actually known to the
> hypervisor.
The hypervisor can read the value as long as it is not an SEV-ES or
SEV-SNP guest.
> It thinks that there are no writes to the MSR after the
> first non-zero write, so that sticky value will be returned to
> KVM_GET_MSRS. This breaks live migration.
KVM_GET_MSRS should go through the normal read MSR path, which will read
the guest MSR value from either svm->vmcb->save.spec_ctrl if
X86_FEATURE_V_SPEC_CTRL is set or from svm->spec_ctrl, otherwise. And the
write MSR path will do similar.
I'm probably missing something here, because I'm not good with the whole
live migration thing as it relates to host and guest features.
Thanks,
Tom
>
> Basically, an always-on V_SPEC_CTRL breaks existing hypervisors. It
> must, therefore, default to off. However, I see that our Rome and
> Milan CPUs already report the existence of this feature.
On Mon, Jun 13, 2022 at 8:10 AM Tom Lendacky <[email protected]> wrote:
>
> On 6/3/22 22:11, Jim Mattson wrote:
> > On Thu, Jan 28, 2021 at 4:43 PM Babu Moger <[email protected]> wrote:
> >
> >> This support also fixes an issue where a guest may sometimes see an
> >> inconsistent value for the SPEC_CTRL MSR on processors that support
> >> this feature. With the current SPEC_CTRL support, the first write to
> >> SPEC_CTRL is intercepted and the virtualized version of the SPEC_CTRL
> >> MSR is not updated. When the guest reads back the SPEC_CTRL MSR, it
> >> will be 0x0, instead of the actual expected value. There isn’t a
> >> security concern here, because the host SPEC_CTRL value is or’ed with
> >> the Guest SPEC_CTRL value to generate the effective SPEC_CTRL value.
> >> KVM writes with the guest's virtualized SPEC_CTRL value to SPEC_CTRL
> >> MSR just before the VMRUN, so it will always have the actual value
> >> even though it doesn’t appear that way in the guest. The guest will
> >> only see the proper value for the SPEC_CTRL register if the guest was
> >> to write to the SPEC_CTRL register again. With Virtual SPEC_CTRL
> >> support, the save area spec_ctrl is properly saved and restored.
> >> So, the guest will always see the proper value when it is read back.
> >
> > Note that there are actually two significant problems with the way the
> > new feature interacts with the KVM code before this patch:
> > 1) All bits set by the first non-zero write become sticky until the
> > vCPU is reset (because svm->spec_ctrl is never modified after the
> > first non-zero write).
>
> When X86_FEATURE_V_SPEC_CTRL is set, then svm->spec_ctrl isn't used.
Post-patch, yes. I'm talking about how this new hardware feature broke
versions of KVM *before* this patch was submitted.
> > 2) The current guest IA32_SPEC_CTRL value isn't actually known to the
> > hypervisor.
>
> The hypervisor can read the value as long as it is not an SEV-ES or
> SEV-SNP guest.
Yes, it can, but KVM prior to this patch did not. Again, I'm talking
about how this new hardware feature broke *existing* versions of KVM.
> > It thinks that there are no writes to the MSR after the
> > first non-zero write, so that sticky value will be returned to
> > KVM_GET_MSRS. This breaks live migration.
>
> KVM_GET_MSRS should go through the normal read MSR path, which will read
> the guest MSR value from either svm->vmcb->save.spec_ctrl if
> X86_FEATURE_V_SPEC_CTRL is set or from svm->spec_ctrl, otherwise. And the
> write MSR path will do similar.
You really are gaslighting me, aren't you?
> I'm probably missing something here, because I'm not good with the whole
> live migration thing as it relates to host and guest features.
AMD added a new VMCB field that existing hypervisors knew nothing
about. This VMCB field contains the current value of the guest's
IA32_SPEC_CTRL. Since the hypervisor doesn't know that this field even
exists, it cannot migrate it.
On 6/13/22 14:23, Jim Mattson wrote:
> On Mon, Jun 13, 2022 at 8:10 AM Tom Lendacky <[email protected]> wrote:
>>
>> On 6/3/22 22:11, Jim Mattson wrote:
>>> On Thu, Jan 28, 2021 at 4:43 PM Babu Moger <[email protected]> wrote:
>>>
>>>> This support also fixes an issue where a guest may sometimes see an
>>>> inconsistent value for the SPEC_CTRL MSR on processors that support
>>>> this feature. With the current SPEC_CTRL support, the first write to
>>>> SPEC_CTRL is intercepted and the virtualized version of the SPEC_CTRL
>>>> MSR is not updated. When the guest reads back the SPEC_CTRL MSR, it
>>>> will be 0x0, instead of the actual expected value. There isn’t a
>>>> security concern here, because the host SPEC_CTRL value is or’ed with
>>>> the Guest SPEC_CTRL value to generate the effective SPEC_CTRL value.
>>>> KVM writes with the guest's virtualized SPEC_CTRL value to SPEC_CTRL
>>>> MSR just before the VMRUN, so it will always have the actual value
>>>> even though it doesn’t appear that way in the guest. The guest will
>>>> only see the proper value for the SPEC_CTRL register if the guest was
>>>> to write to the SPEC_CTRL register again. With Virtual SPEC_CTRL
>>>> support, the save area spec_ctrl is properly saved and restored.
>>>> So, the guest will always see the proper value when it is read back.
>>>
>>> Note that there are actually two significant problems with the way the
>>> new feature interacts with the KVM code before this patch:
>>> 1) All bits set by the first non-zero write become sticky until the
>>> vCPU is reset (because svm->spec_ctrl is never modified after the
>>> first non-zero write).
>>
>> When X86_FEATURE_V_SPEC_CTRL is set, then svm->spec_ctrl isn't used.
>
> Post-patch, yes. I'm talking about how this new hardware feature broke
> versions of KVM *before* this patch was submitted.
Ah, yes, I get it now. I wasn't picking up on the aspect of running older
KVM versions on the newer hardware, sorry.
I understand what you're driving at, now. We do tell the hardware teams
that add this type of feature that we need a VMCB enable bit, e.g. make it
an opt in feature. I'll be sure to communicate that to them again so that
this type of issue can be avoided in the future.
Thanks,
Tom
>