Received-SPF: pass (google.com: domain of linux-kernel+bounces-201742-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <07bb7d6a-f06d-7787-f76a-56a78cfb54e9@amd.com>
Date: Wed, 5 Jun 2024 09:42:25 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.0.3
Subject: Re: [PATCH] KVM: SEV-ES: Fix svm_get_msr()/svm_set_msr() for
 KVM_SEV_ES_INIT guests
To: Michael Roth <michael.roth@amd.com>, kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
 Sean Christopherson <seanjc@google.com>,
 Ravi Bangoria <ravi.bangoria@amd.com>, Srikanth Aithal <sraithal@amd.com>
References: <20240604233510.764949-1-michael.roth@amd.com>
From: "Nikunj A. Dadhania" <nikunj@amd.com>
In-Reply-To: <20240604233510.764949-1-michael.roth@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?VFJ0TjF4UTB2ZlVlN2NBMWdKN2s2VUM2cDllbEd5T3F1cEhRTFZuOGZFeUV3?=
 =?utf-8?B?d3k4WVdBeTJ0enFmbmlJWUhQVHRSVy9vczIzaHdOaVVwTVUzUTRUSlBMR2tW?=
 =?utf-8?B?WHJMS2FyVzJ1M3ZjUHl3VXBFOHkyTWZJcjV5endiSmM2LzZwR0JkOFJiNFVG?=
 =?utf-8?B?T3FybnhuMVF6RldTdmlYYzlCUVlzYzUvNEI0ZVg2aDFiT2JZR1VGYzkxWW0x?=
 =?utf-8?B?ZG5pSUYxbmc3U3JWOXA2NnF5Z041VHNMTThHd0UrTzYzWXFTeENMZmN6bUIx?=
 =?utf-8?B?NmExTnJiQVQ1WUV4K0dUbkFaRTUxMkMwbG9ZRFpYSTdnQmdFdkl4aXBVeW5G?=
 =?utf-8?B?cUV3ODN0TDFpck5MOTAzdEhwN3VhWWpCVnJmUVNIZjVYZCtQZVhWeHR4My92?=
 =?utf-8?B?WmdzNFhQZHRzQ0VIdWx2TDhrSGlqNkJSaDJSa0M0aFlxMFNRVFg5N0kvL1g1?=
 =?utf-8?B?eVNoZ2xiUDZBVmp2eGVicFkxTW9EUDNMT3JuMDJTQ3FQOEZ1OVVtTENOc1NK?=
 =?utf-8?B?akJuOTg4czFZdVZGbmNNNnVhSllGUStaWGJ1eWN5djZuNVp3WURBWFNwNkVw?=
 =?utf-8?B?ekh5UjR6N0NzcVNkKzU5ZnpRZVR3cW9oRzIyRS9tT3BMZk5LWlJFVDQwTy9G?=
 =?utf-8?B?bDVEWjk5VGdqSkhkenFaL1pHckxGUEoyUFhsdVducWlRRXVQZGRWZFdpWk0w?=
 =?utf-8?B?SzFMamZZeTkrNXREZGx4a2FVUnkyUmlQdUhHaFlIQmd3WVNnN1k0bWQvWGUx?=
 =?utf-8?B?L0ltcTVxT09IcGpEOTR0LzF2amtua01QTzZ2bXBjZHFFaisxRkdPRVhFU3Fy?=
 =?utf-8?B?Nnh6UVlPRGJJMG1LbnVsaVJ2eldLRWF0eDFMamRua1hLODdJdFl6YmczeXY1?=
 =?utf-8?B?VUd2UGtMMEdoUnR3REdCNDhJZXJic0dYNDFJcWQyNkJ5UUpyVXhGbittRjcw?=
 =?utf-8?B?bnBQQVJIa2YyU3NZRnNTcG9uaU9HZ0c5c1BNVUtNVlVDSHd0QnhTUkxIVjN6?=
 =?utf-8?B?Q3o0VE4zUGI5TXIxU1dNWlFpQW9ydHVuTXJCSitTTlJITXpXZDVjZi9QSlJm?=
 =?utf-8?B?SHVIOHpHeEc3dURva2x2dE1jMDRkd3dLQnozVXprUk1jN0VuMTV4L3R6a3lY?=
 =?utf-8?B?Wmg3YUJYQkJhYXNSWlBReFhEcVV2Y2NkUjJDYXc5WVFweHFsN1dpN2NGZXYz?=
 =?utf-8?B?OVhwNzFwNXpoeXYrLzlXaUFabEw3QnkxUnJOaXhyUCsxZGg4anIzVnhlU0Yv?=
 =?utf-8?B?WEx0bGdJalVSTXBwenNmQmQ0SXZQa3pqL3NTTGhWSGl1aFRzd3g1UnQ4b3dM?=
 =?utf-8?B?Q3F0V3oxQ2dFendyQXNTdXJtTDZuYnJINTVmamNaMHFmWHRCVndiaDRyb2RT?=
 =?utf-8?B?Q2cwdWNMeTJhNjliU2NKRmdkWnBZTzlycVRqWFRVcDJaczZJK0FiZDdHQzFE?=
 =?utf-8?B?d0o3SmY5VVBsRzJIb3ZZdlFWOGZpUkdNY1RhaXBVam1RVVNkNUd0SjVlK1da?=
 =?utf-8?B?d0xCamJ4cWV3S2FpeXJZdWs5OTlWamFIbWg3NjZETVhuQThOdGtqa2RYZWs1?=
 =?utf-8?B?LzhuTWwyd3BlR0JOeUpJOGJqclZNYnRFenRGMkh2T3VPNHU5N0FBbkhDRHBK?=
 =?utf-8?B?NGZwUlh4M05BVjdGaUFaZ3pHOU1ieDdEeVRnaGpVMzMvSzY1Q1NCQU1rbHgw?=
 =?utf-8?B?NnE5ejhEckFyMFlGOTZPbEpabGxySzZBU0d1cDFKbDhDamxZeitEUndhOGtn?=
 =?utf-8?B?dkZNMXgyV0hXUzR4Tm8remNlSjM2d1hvL2F1ekEvbFRqckJzQzd0b05xQnZQ?=
 =?utf-8?B?YVQ4RUJTc2hQcGZENjZRMVlQcEZSckFsS1hPZCtlMXFTVzgwaFo5c09Rd1Qx?=
 =?utf-8?B?eElZdjFJVGlFRzFYSDRmYy9hc3M4WUxPdkZrSkFudTNyRGhzdkhvd2NHWXNv?=
 =?utf-8?B?T0JQeWxiSVRPcUk0M1JoUnExU0t4WnJzOUc0eVZjd2ZsRkEwbGFCTEhER3ZP?=
 =?utf-8?B?WXFLTVFvbnpnWUNoVmRtWE5ucnpSOTdRVFhvUmNyTU9JNGIrOFdMVXRadTA0?=
 =?utf-8?B?ZC8rTVEyS25mMjF3Um43ZDl0R2JuWkhuMlUwZXZNeHFkRVpqNWxjTmVJekpN?=
 =?utf-8?Q?7LUAolVX6DF1E1ORzNZRzSIYd?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 326b3c0a-adf1-4528-935a-08dc8515ba13
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB6309.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jun 2024 04:12:34.6021
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bY3lJT7rBoOERIvz0x9AftEMc3TCfKrE6uR5+/t+gMf7kfji9EQfWVXjp6aUsjvA1wY2dzJcIvpwdiHQ+rHtUQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR12MB4395


On 6/5/2024 5:05 AM, Michael Roth wrote:
> With commit 27bd5fdc24c0 ("KVM: SEV-ES: Prevent MSR access post VMSA
> encryption"), older VMMs like QEMU 9.0 and older will fail when booting
> SEV-ES guests with something like the following error:
> 
>   qemu-system-x86_64: error: failed to get MSR 0x174
>   qemu-system-x86_64: ../qemu.git/target/i386/kvm/kvm.c:3950: kvm_get_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
> 
> This is because older VMMs that might still call
> svm_get_msr()/svm_set_msr() for SEV-ES guests after guest boot even if
> those interfaces were essentially just noops because of the vCPU state
> being encrypted and stored separately in the VMSA. Now those VMMs will
> get an -EINVAL and generally crash.
> 
> Newer VMMs that are aware of KVM_SEV_INIT2 however are already aware of
> the stricter limitations of what vCPU state can be sync'd during
> guest run-time, so newer QEMU for instance will work both for legacy
> KVM_SEV_ES_INIT interface as well as KVM_SEV_INIT2.
> 
> So when using KVM_SEV_INIT2 it's okay to assume userspace can deal with
> -EINVAL, whereas for legacy KVM_SEV_ES_INIT the kernel might be dealing
> with either an older VMM and so it needs to assume that returning
> -EINVAL might break the VMM.
> 
> Address this by only returning -EINVAL if the guest was started with
> KVM_SEV_INIT2. Otherwise, just silently return.
> 
> Cc: Ravi Bangoria <ravi.bangoria@amd.com>
> Cc: Nikunj A Dadhania <nikunj@amd.com>
> Reported-by: Srikanth Aithal <sraithal@amd.com>
> Closes: https://lore.kernel.org/lkml/37usuu4yu4ok7be2hqexhmcyopluuiqj3k266z4gajc2rcj4yo@eujb23qc3zcm/
> Fixes: 27bd5fdc24c0 ("KVM: SEV-ES: Prevent MSR access post VMSA encryption")
> Signed-off-by: Michael Roth <michael.roth@amd.com>

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>

> ---
>  arch/x86/kvm/svm/svm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index b252a2732b6f..c58da281f14f 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2855,7 +2855,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  
>  	if (sev_es_prevent_msr_access(vcpu, msr_info)) {
>  		msr_info->data = 0;
> -		return -EINVAL;
> +		return vcpu->kvm->arch.has_protected_state ? -EINVAL : 0;
>  	}
>  
>  	switch (msr_info->index) {
> @@ -3010,7 +3010,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  	u64 data = msr->data;
>  
>  	if (sev_es_prevent_msr_access(vcpu, msr))
> -		return -EINVAL;
> +		return vcpu->kvm->arch.has_protected_state ? -EINVAL : 0;
>  
>  	switch (ecx) {
>  	case MSR_AMD64_TSC_RATIO:
>