Received-SPF: pass (google.com: domain of linux-kernel+bounces-106735-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240226190344.787149-1-pbonzini@redhat.com> <20240226190344.787149-11-pbonzini@redhat.com>
 <20240314024952.w6n6ol5hjzqayn2g@amd.com>
In-Reply-To: <20240314024952.w6n6ol5hjzqayn2g@amd.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 18 Mar 2024 23:01:09 +0100
Message-ID: <CABgObfYpGUBYj94PSNkYsa_NDGVE=NOJnjHSZoE5d7sSGmVfQA@mail.gmail.com>
Subject: Re: [PATCH v3 10/15] KVM: x86: add fields to struct kvm_arch for CoCo features
To: Michael Roth <michael.roth@amd.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, seanjc@google.com, 
	aik@amd.com, pankaj.gupta@amd.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 14, 2024 at 3:50=E2=80=AFAM Michael Roth <michael.roth@amd.com>=
 wrote:
> I've been trying to get SNP running on top of these patches and hit and
> issue with these due to fpstate_set_confidential() being done during
> svm_vcpu_create(), so when QEMU tries to sync FPU state prior to calling
> SNP_LAUNCH_FINISH it errors out. I think the same would happen with
> SEV-ES as well.
>
> Maybe fpstate_set_confidential() should be relocated to SEV_LAUNCH_FINISH
> site as part of these patches?

To SEV_LAUNCH_UPDATE_VMSA, I think, since that's where the last
opportunity lies to sync the contents of struct kvm_vcpu.

> Also, do you happen to have a pointer to the WIP QEMU patches? Happy to
> help with posting/testing those since we'll need similar for
> SEV_INIT2-based SNP patches.

Pushed to https://gitlab.com/bonzini/qemu, branch sevinit2. There is a
hackish commit "runstate: skip initial CPU reset if reset is not
actually possible" that needs some auditing, because I'd like to
replace

-    cpu_synchronize_all_post_reset();
+    if (cpus_are_resettable()) {
+        cpu_synchronize_all_post_reset();
+    } else {
+        /* Assume that cpu_synchronize_all_post_init() was enough. */
+        assert(runstate_check(RUN_STATE_PRELAUNCH));
+    }

with

-    cpu_synchronize_all_post_reset();
+    /*
+     * cpu_synchronize_all_post_init() has already happened if the VM hasn=
't
+     * launched.
+     */
+    if (!runstate_check(RUN_STATE_PRELAUNCH)) {
+        cpu_synchronize_all_post_reset();
+    }

Paolo