Received-SPF: pass (google.com: domain of linux-crypto+bounces-3241-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C
Date: Mon, 1 Apr 2024 18:17:31 -0500
From: Michael Roth <michael.roth@amd.com>
To: Paolo Bonzini <pbonzini@redhat.com>
CC: <kvm@vger.kernel.org>, <linux-coco@lists.linux.dev>, <linux-mm@kvack.org>,
	<linux-crypto@vger.kernel.org>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>, <tglx@linutronix.de>, <mingo@redhat.com>,
	<jroedel@suse.de>, <thomas.lendacky@amd.com>, <hpa@zytor.com>,
	<ardb@kernel.org>, <seanjc@google.com>, <vkuznets@redhat.com>,
	<jmattson@google.com>, <luto@kernel.org>, <dave.hansen@linux.intel.com>,
	<slp@redhat.com>, <pgonda@google.com>, <peterz@infradead.org>,
	<srinivas.pandruvada@linux.intel.com>, <rientjes@google.com>,
	<dovmurik@linux.ibm.com>, <tobin@ibm.com>, <bp@alien8.de>, <vbabka@suse.cz>,
	<kirill@shutemov.name>, <ak@linux.intel.com>, <tony.luck@intel.com>,
	<sathyanarayanan.kuppuswamy@linux.intel.com>, <alpergun@google.com>,
	<jarkko@kernel.org>, <ashish.kalra@amd.com>, <nikunj.dadhania@amd.com>,
	<pankaj.gupta@amd.com>, <liam.merwick@oracle.com>, Brijesh Singh
	<brijesh.singh@amd.com>, Harald Hoyer <harald@profian.com>
Subject: Re: [PATCH v12 12/29] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_FINISH command
Message-ID: <20240401231731.kjvse7m7oqni7uyg@amd.com>
References: <20240329225835.400662-1-michael.roth@amd.com>
 <20240329225835.400662-13-michael.roth@amd.com>
 <40382494-7253-442b-91a8-e80c38fb4f2c@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <40382494-7253-442b-91a8-e80c38fb4f2c@redhat.com>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Apr 2024 23:17:48.0658
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: b09a6e0b-d136-43aa-2f94-08dc52a1f1e7
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com]
X-MS-Exchange-CrossTenant-AuthSource:
	DS3PEPF000099D9.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB6740

On Sat, Mar 30, 2024 at 09:41:30PM +0100, Paolo Bonzini wrote:
> On 3/29/24 23:58, Michael Roth wrote:
> > 
> > +		/* Handle boot vCPU first to ensure consistent measurement of initial state. */
> > +		if (!boot_vcpu_handled && vcpu->vcpu_id != 0)
> > +			continue;
> > +
> > +		if (boot_vcpu_handled && vcpu->vcpu_id == 0)
> > +			continue;
> 
> Why was this not necessary for KVM_SEV_LAUNCH_UPDATE_VMSA?  Do we need it
> now?

I tried to find the original discussion for more context, but can't seem to
locate it. But AIUI, there are cases where a VMM may create AP vCPUs earlier
than it does the BSP, in which case kvm_for_each_vcpu() might return an AP
as it's first entry and cause that VMSA to get measured before, leading
to a different measurement depending on the creation ordering.

Measuring the BSP first ensures consistent measurement, since the
initial AP contents are all identical so their ordering doesn't matter.

For SNP, it makes sense to take the more consistent approach right off
the bat. But for SEV-ES, it's possible that there are VMMs/userspaces
out there that have already accounted for this in their measurement
calculations, so it could cause issues if we should the behavior for all
SEV-ES. We could however limit the change to KVM_X86_SEV_ES_VM and
document that as part of KVM_SEV_INIT2, since there is similarly chance
for measurement changes their WRT to the new FPU/XSAVE sync'ing that was
added.


> 
> > +See SEV-SNP specification [snp-fw-abi]_ for SNP_LAUNCH_FINISH further details
> > +on launch finish input parameters.
> 
> See SNP_LAUNCH_FINISH in the SEV-SNP specification [snp-fw-abi]_ for further
> details on the input parameters in ``struct kvm_sev_snp_launch_finish``.

Will make similar changes for the others as well. Thanks!

-Mike

> 
> Paolo
> 
>