Received-SPF: pass (google.com: domain of linux-kernel+bounces-193973-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <f0de86a8-81c9-4656-862f-e229845d47cd@amd.com>
Date: Wed, 29 May 2024 16:14:10 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 1/3] KVM: SEV-ES: Prevent MSR access post VMSA
 encryption
To: Paolo Bonzini <pbonzini@redhat.com>, seanjc@google.com,
 nikunj.dadhania@amd.com
Cc: thomas.lendacky@amd.com, tglx@linutronix.de, mingo@redhat.com,
 bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
 michael.roth@amd.com, pankaj.gupta@amd.com, kvm@vger.kernel.org,
 linux-kernel@vger.kernel.org, santosh.shukla@amd.com, ravi.bangoria@amd.com
References: <20240523121828.808-1-ravi.bangoria@amd.com>
 <20240523121828.808-2-ravi.bangoria@amd.com>
 <3eca1e7e-9ddc-47a2-b214-d8788a069222@redhat.com>
Content-Language: en-US
From: Ravi Bangoria <ravi.bangoria@amd.com>
In-Reply-To: <3eca1e7e-9ddc-47a2-b214-d8788a069222@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?T0MwTGV4enhZZE5Qdkl0dy9NdXl1OVFjVFB0SG0wUnR2Skdqdnh4MlFJa0dQ?=
 =?utf-8?B?ZWI0dHlDMVUzb2NQT0tEbDBsSDBXMnZlMDE5Y0lzdjhBZHVGUTNIanJZMElX?=
 =?utf-8?B?STlzZTA0N1dFaFRmT0s5T2hERmdDUHAwUm96SjBOTjRkdnRJOHc3Y0lacEw4?=
 =?utf-8?B?QVkwNGx5RW9BZDBoS1BDUFNZRFJXYjYyYU93dWt5YW9Ja0lkT3VoVkZkUDJD?=
 =?utf-8?B?ZFJYV3hWMnBibkhhK1Fsd1ZJYW1CZzk2c3RHazU4eHBGOTJhY3Q4MVVUempr?=
 =?utf-8?B?bTdqSTBKVkw0NWp5dEczajBlV2YwbFBVK1NjcXVNdlB1bFlaUDBXTjg5RSsz?=
 =?utf-8?B?eUVHQ0NENE54czl0Y0RlaTY0cjdPSTBCcTc2LzN4eEhNS0w4NWxCRksrRHBt?=
 =?utf-8?B?aENidTJoNkREd2tMbzd1TVZRRGsvbGkzK01pOTB6RzlJNS9WbnZ6UVNDSlQy?=
 =?utf-8?B?cVFaYUhVTkxUZmpZT0tCcEF0ZjdESU4xUjZiOHN4UWdrOVFhSit4SXh2YTBm?=
 =?utf-8?B?a29Edy9pTVlwdU1iVEV1RUo2bHhjU25yd3dRRWZlTXIyOTRIMlA5S2tkMU43?=
 =?utf-8?B?K3QrMmV5QXJHVmJZOWVjV085Qlc2TzJHYTd1cGh3dXhuR0lFcFdnQnRrbWYr?=
 =?utf-8?B?dWRPVzNQamdNVXdnQWhYa3QvelN0Uk4wVkNJZnFaa21ZOG9CeXVSM2x5cnk1?=
 =?utf-8?B?WXpHOEhtUFBPaFhzbWlDb0Y3amZlVitENVFZWmd0ckp5ZHlqcVp5eDgrSHhH?=
 =?utf-8?B?VTdOQ1NsUFgvTXN4NXNXeFJmZUhhR0h0bHZZRDZiUHA1RXZGUllrWlVCWHJI?=
 =?utf-8?B?UUNyVFpJcDRJZ2xaSCt6c3U3QnFWQ2djYWFqYnJoa1BZVTBPYTV4UE9CZFhB?=
 =?utf-8?B?QWltRmRrV0FTRGhqSGpudFV4TVhQaVlGZmZPQkNnVHBVZzdVYkhIbEJob3lB?=
 =?utf-8?B?TGEwNXBkN3NsdzhVOGFlOGxqVGdYayt6eHVWRUtic05hcVVoYXp3eHZrb0Z4?=
 =?utf-8?B?Q0RNQjlFRVRTck5saUprazB5Sm5EdnE1YXNzdzNLNkRtTllhVDFJcFZlS1N2?=
 =?utf-8?B?KzNLamNjY1duZjZVRmtxcHBPdklKTzB3WmNOQ09jR2p0bTJ0WlFsdWFkditK?=
 =?utf-8?B?STJIbWpTZXU5M2xoa2h4UXplMWt6NTFzQlYrMFAwQW9IYTBob044L2orZ0Vh?=
 =?utf-8?B?aGNnT3AwcGVMMVlqbjVKdlp1OXd3M0xrRWhaK2hrMVNmbURhVFVQNDJMNmp4?=
 =?utf-8?B?RVp4cTEyQTNWQlQ1L29DNXZHWHJNZFJJbUtsY3owRXRCMFlUTzFqblhhWFRa?=
 =?utf-8?B?MU1FeXM3dEFqajZuTk9mT0Ixd0kxMTBId1Z2enlNK0xmTHJhZnpMSDhIbElY?=
 =?utf-8?B?WG1aWlprSUZkc3l6NGtEeWRkekxDL0VVdGZqRkhuQkk1T0FwWWV6enV0em1a?=
 =?utf-8?B?RFd0cmxBdVBQSVNnZDVqVlQxVmJLdndqWWhyV1pmZmR5VGpqOUJtME1kQjEy?=
 =?utf-8?B?MFEydE96WmwzVm9iY3hqWGU5TFVqOXZzUHgzMERTeDRyS0NmYzhWZnRFZmRH?=
 =?utf-8?B?aWVjZ0w2eDdER3orblVGMVA5NG0zd2Q1VXB2cXRZNGF0aDdQQVhDMGhZQTZF?=
 =?utf-8?B?Zldhd1kvRlZuM296R0NxMUxvOWVDdmZBS3B3a0szbGVjRzdpYlpUa3o3S2FV?=
 =?utf-8?B?eFo1VG1GeCtsYThiQ05Lam5kTGtXZ252MCtXOGZHSFFwcjl5WHNPTTNQWjRG?=
 =?utf-8?B?ZWZEWDFZTWV6YkVwTkNyOXJzMFVmaUNZRlpBajlNa00wbHpuZEY5SGJHOHVZ?=
 =?utf-8?B?c1p0Q2I2eFgzQkY4cTljblFnRzg3SnVDak8vSFk4MDJkWnBCcXVwaDhrMGdG?=
 =?utf-8?B?V2hlMlZxNGxhSDlFUHBEeW9TcHk1bmpONVAwSHRCN2ZPTEM5c3BLNWpEQjlO?=
 =?utf-8?B?akdrcTVZRThza29IR0tEbzZOQ1ZpYUIyRlhvcHBzdG9LM01zQ1duVElEWFZF?=
 =?utf-8?B?Z3ZFRWtTbHk1WWFQZk5KZ0JMbkZrTVowQ09yTFJIUGVwVDVDcldKUzM1Z040?=
 =?utf-8?B?YjJhYVBwZ25HUG9zaEpsRk02a2NheWJLY29SVFJoSFlPTWtSN2Q4RVNjV1V0?=
 =?utf-8?Q?+071HC4lRFU3KVA0ViO122V4N?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 27e5c911-63df-430f-b354-08dc7fcc4c68
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB6588.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 May 2024 10:44:21.5046
 (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: y2sf2YfFI77W0BZo4i0xt9lffwhMzcNpMTsoTMNad/T3bBqmjEt/4HizbT/Y6isHHuZ94d2TsOPziP1XOQiVLg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB7855

On 5/28/2024 10:01 PM, Paolo Bonzini wrote:
> On 5/23/24 14:18, Ravi Bangoria wrote:
>> From: Nikunj A Dadhania <nikunj@amd.com>
>>
>> KVM currently allows userspace to read/write MSRs even after the VMSA is
>> encrypted. This can cause unintentional issues if MSR access has side-
>> effects. For ex, while migrating a guest, userspace could attempt to
>> migrate MSR_IA32_DEBUGCTLMSR and end up unintentionally disabling LBRV on
>> the target. Fix this by preventing access to those MSRs which are context
>> switched via the VMSA, once the VMSA is encrypted.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
>> ---
>>   arch/x86/kvm/svm/svm.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 3d0549ca246f..489b0183f37d 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -2834,10 +2834,24 @@ static int svm_get_msr_feature(struct kvm_msr_entry *msr)
>>       return 0;
>>   }
>>   +static bool
>> +sev_es_prevent_msr_access(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>> +{
>> +    return sev_es_guest(vcpu->kvm) &&
>> +           vcpu->arch.guest_state_protected &&
>> +           svm_msrpm_offset(msr_info->index) != MSR_INVALID &&
>> +           !msr_write_intercepted(vcpu, msr_info->index);
>> +}
>> +
>>   static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   {
>>       struct vcpu_svm *svm = to_svm(vcpu);
>>   +    if (sev_es_prevent_msr_access(vcpu, msr_info)) {
>> +        msr_info->data = 0;
>> +        return 0;
> 
> This should return -EINVAL, not 0.  Likewise below in svm_set_msr().

Sure.

Thanks,
Ravi