Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <161188083424.28787.9510741752032213167.stgit@bmoger-ubuntu>
 <161188100955.28787.11816849358413330720.stgit@bmoger-ubuntu>
 <CALMp9eTU5h4juDyGePnuDN39FudYUqyAnnQdALZM8KfiMo93YA@mail.gmail.com> <5d380b11-079f-e941-25cf-747f66310695@amd.com>
In-Reply-To: <5d380b11-079f-e941-25cf-747f66310695@amd.com>
From:   Jim Mattson <jmattson@google.com>
Date:   Mon, 13 Jun 2022 12:23:40 -0700
Message-ID: <CALMp9eRnC1RgRwj64TJcXdhhL6g835N_-E8FbeHVre6aX=18-A@mail.gmail.com>
Subject: Re: [PATCH v4 2/2] KVM: SVM: Add support for Virtual SPEC_CTRL
To:     Tom Lendacky <thomas.lendacky@amd.com>
Cc:     Babu Moger <babu.moger@amd.com>, pbonzini@redhat.com,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        fenghua.yu@intel.com, tony.luck@intel.com, wanpengli@tencent.com,
        kvm@vger.kernel.org, peterz@infradead.org, seanjc@google.com,
        joro@8bytes.org, x86@kernel.org, kyung.min.park@intel.com,
        linux-kernel@vger.kernel.org, krish.sadhukhan@oracle.com,
        hpa@zytor.com, mgross@linux.intel.com, vkuznets@redhat.com,
        kim.phillips@amd.com, wei.huang2@amd.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jun 13, 2022 at 8:10 AM Tom Lendacky <thomas.lendacky@amd.com> wrot=
e:
>
> On 6/3/22 22:11, Jim Mattson wrote:
> > On Thu, Jan 28, 2021 at 4:43 PM Babu Moger <babu.moger@amd.com> 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=E2=80=99t=
 a
> >> security concern here, because the host SPEC_CTRL value is or=E2=80=99=
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=E2=80=99t 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.