Message-ID: <6f7fec84-1915-4499-104d-d29bf78270b5@amd.com>
Date:   Fri, 10 Feb 2023 17:32:55 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v2 10/11] KVM: SVM: implement support for vNMI
To:     Sean Christopherson <seanjc@google.com>,
        Maxim Levitsky <mlevitsk@redhat.com>
Cc:     kvm@vger.kernel.org, Sandipan Das <sandipan.das@amd.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Borislav Petkov <bp@alien8.de>,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Daniel Sneddon <daniel.sneddon@linux.intel.com>,
        Jiaxi Chen <jiaxi.chen@linux.intel.com>,
        Babu Moger <babu.moger@amd.com>, linux-kernel@vger.kernel.org,
        Jing Liu <jing2.liu@intel.com>,
        Wyes Karny <wyes.karny@amd.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>,
        Santosh Shukla <santosh.shukla@amd.com>
References: <20221129193717.513824-1-mlevitsk@redhat.com>
 <20221129193717.513824-11-mlevitsk@redhat.com> <Y9R1+hPaTWcEZMOX@google.com>
Content-Language: en-US
From:   Santosh Shukla <santosh.shukla@amd.com>
In-Reply-To: <Y9R1+hPaTWcEZMOX@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?d1RwNHdMWmh3WEhYdWtINzFlYzRJRCtOMGVaRTZ3YWk4dUxRVmU4TnJlYUwz?=
 =?utf-8?B?bnFoVEx2em5LcHFiTTBKZ1dEUENnU2lBdE1QaU5sU1hobnVsMkRVR3JZNWM3?=
 =?utf-8?B?eDFjeHZ6Umg2L2xOSlJ1QWhPT2pjNmtoaEliTXdhRWo5akEyMS9Qb284WU1T?=
 =?utf-8?B?THdsNzJ4Zld3UkhRblMwTUptZWF0S3U2akNMSnlFSGtnRVM0bmdBRFBRUEZR?=
 =?utf-8?B?enZqUTduMnZQRlBRMmpWU0h6SzVvN1pZZFdRVHF1N3NERzFKelRSNk9HMzI0?=
 =?utf-8?B?dDZ4eGtWWlc1ZG8wN09LeGQxSnlLcWx2V1NkcU83TkNPSkY5bGl1bkZlWjY3?=
 =?utf-8?B?T0x4aTZITjNCai9aaWpZMnpEMldjR3A4N1ZBV2QxOCtSN1ZxOEd5TzNCc09U?=
 =?utf-8?B?Z1pEVC91ZzFqbVlqRXZPcmVudG5QWXJmMFJQMU9BamxsZldzTlBGUXVzdmJ2?=
 =?utf-8?B?WkRzemNneWhlbUxWTmZ1Szd0UjI0SEsvNjFidng5ajJQWnlSVmFLNzJRWHZW?=
 =?utf-8?B?eVJabjV3Qnc4MmpGZDdQM3k2ZW5FcUovSm85TEpPS00wajdyRjR3d3Q4aGRo?=
 =?utf-8?B?Ymd3ek8xL0MxWWduc251ZkRMclhTMTNxL1VrSURYanFBOUJ2aDNJUEk4c1Jn?=
 =?utf-8?B?VitJK3NzZ0JzYlVLelFFMXJ0ekRkU1ZoWjZtdHFZdmJZQ3Y1RjJHb1U0SGdM?=
 =?utf-8?B?SlF2LzV6NjZHSlNaa3BvamFWS2UwS3dpMzI4czlHaEJZb3BYQ2dyS2o4am5X?=
 =?utf-8?B?MUFVUG5hN1lpamVOTGJPSFByOS92dWdhSWxDVnR3STVWMVptaU5VL0NrckY4?=
 =?utf-8?B?MkxvT3RGNVB4cTV6Q1FzUEJFMTVEYlNIS3pMZUZ4Slo5S05vUHExYldyYWMx?=
 =?utf-8?B?R0Fkd0Z4NkwxR1E0NVM0VEZVMU9YdXI5TkRjckJ6TjNDUUtXR2U2VCtZenhh?=
 =?utf-8?B?UjJFQTBxV2kvK2R4UDRjQWNoN3Y4TXFWTkRNS1VPc24rb0dJRnh2NVk3OHVX?=
 =?utf-8?B?cml1VU04ZE9TK0o4Y1pKNjNTdlhKT1QxTWszZHBnb0c4Z1VYY2NVYTlQZUx5?=
 =?utf-8?B?eG1tS0dSc1pvaktKWFEyOXZublhSKzExSlF3REtGcm1Ra0YwNXllcXVBR1BV?=
 =?utf-8?B?UUM1Vkh6THg5OTc1L20yRUVaTUllbWZ1MVhxbnMrV1hHaXI5NVpvbno3ZytG?=
 =?utf-8?B?ZDJicTdUZHBqU1BLemphMEVvOERIWlBlWUF1ZkRMT1d3NnBiZVdVa0xLcmgx?=
 =?utf-8?B?bjhPNnl4em9UcE9lWFBNcjJSZkhQeGg3NWhMTDBhUmR6dGpxOHdjS2ExY2pn?=
 =?utf-8?B?RXZQSit2NXFrQitaUjI2Y05HM3hneFQ2QXQ5dGFqZ1pQVi9KR0ZYQXBqNU1k?=
 =?utf-8?B?NlBSQzNjaHoyOHZqWXlRbFRrUlp2U0wrdlhqZVQrUTRwUkk5MktDbDBIc1BI?=
 =?utf-8?B?bm9LYVdyZ3g4czMzQlVhY1A5akJlNWM0THlpdzF3QkpzR2VLbzVaNlZIL0h5?=
 =?utf-8?B?THU4Q3pBWnh2WTE5THBybGRoblNvSWNZT3pHRXM0TU92R0ltM1YxYkkrNmZE?=
 =?utf-8?B?MGdxUmpaZ21DMTdWcFpXNjV3UWdQTEVOUnVCN1NmOE9QelBYQnkzcG4yS01h?=
 =?utf-8?B?UUhHbkx5Um4zejkzOStqMUpkbUo2citvSCt6S0NLeXZvYTRjNFkrTE0zVXg0?=
 =?utf-8?B?VjcxQXY2TTJGejR5TzJUajVqOTZmamFIZmFjZUJRajVzMFdkamZBVVQ3dUFk?=
 =?utf-8?B?SDVCVlRVNlFQZ2h3SmkzWGxTeU9lbzdWQzlnQmt1a1M3NnRYbGc2aW1UZmlz?=
 =?utf-8?B?WG5BdUU5NzB5dXZGdE5KSmxYYXhHNzRucVNvUEVSRll6YUJ4Vm5TZ3BobEpL?=
 =?utf-8?B?TmZxUm9Fc2dlcHNwY2pkcWdWODNkQTRSMjlOY0ZxN0xrK3kxQi83MTVVZURZ?=
 =?utf-8?B?dXVWWU9JblB2VHJRSVg4cUtxSHc0V09ZU3Z4NlV3TGV5a2w0TXg2Y3NuSWYv?=
 =?utf-8?B?SjVBUU5YekdmZFltTzFPV0lJcEZmYThsS3VSbmxrU1o5bnorS3pobDYzTWFk?=
 =?utf-8?B?RUh3TGxtd0tsSDFiOEJyT0ZqZXpOa005L3F4Q2ErL2ZhdVByUVJoRXRxZ25T?=
 =?utf-8?Q?7+h0GsYrGl1n4WG4K7sh+0itu?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0abbf29a-9ffb-46af-97c3-08db0b5ec74b
X-MS-Exchange-CrossTenant-AuthSource: IA1PR12MB6305.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Feb 2023 12:03:10.5704
 (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: 1Kivtw4Tr43LzlMNcwXIBdWg1PCfrBogkt0PvkTLuBwXr56SPKUgbTcxUiuD3oQtZQrzBxNNxHrLNQTLkHtt2w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4051
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/28/2023 6:40 AM, Sean Christopherson wrote:
> On Tue, Nov 29, 2022, Maxim Levitsky wrote:
>> This patch implements support for injecting pending
>> NMIs via the .kvm_x86_set_hw_nmi_pending using new AMD's vNMI
>> feature.
>>
>> Note that the vNMI can't cause a VM exit, which is needed
>> when a nested guest intercepts NMIs.
>>
>> Therefore to avoid breaking nesting, the vNMI is inhibited while
>> a nested guest is running and instead, the legacy NMI window
>> detection and delivery method is used.
>>
>> While it is possible to passthrough the vNMI if a nested guest
>> doesn't intercept NMIs, such usage is very uncommon, and it's
>> not worth to optimize for.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>> ---
>>  arch/x86/kvm/svm/nested.c |  42 +++++++++++++++
>>  arch/x86/kvm/svm/svm.c    | 111 ++++++++++++++++++++++++++++++--------
>>  arch/x86/kvm/svm/svm.h    |  10 ++++
>>  3 files changed, 140 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
>> index e891318595113e..5bea672bf8b12d 100644
>> --- a/arch/x86/kvm/svm/nested.c
>> +++ b/arch/x86/kvm/svm/nested.c
>> @@ -623,6 +623,42 @@ static bool is_evtinj_nmi(u32 evtinj)
>>  	return type == SVM_EVTINJ_TYPE_NMI;
>>  }
>>  
>> +static void nested_svm_save_vnmi(struct vcpu_svm *svm)
>> +{
>> +	struct vmcb *vmcb01 = svm->vmcb01.ptr;
>> +
>> +	/*
>> +	 * Copy the vNMI state back to software NMI tracking state
>> +	 * for the duration of the nested run
>> +	 */
>> +
> 
> Unecessary newline.
> 
ok.

>> +	svm->nmi_masked = vmcb01->control.int_ctl & V_NMI_MASK;
>> +	svm->vcpu.arch.nmi_pending += vmcb01->control.int_ctl & V_NMI_PENDING;
>> +}
>> +
>> +static void nested_svm_restore_vnmi(struct vcpu_svm *svm)
>> +{
>> +	struct kvm_vcpu *vcpu = &svm->vcpu;
>> +	struct vmcb *vmcb01 = svm->vmcb01.ptr;
>> +
>> +	/*
>> +	 * Restore the vNMI state from the software NMI tracking state
>> +	 * after a nested run
>> +	 */
>> +
> 
> Unnecessary newline.
>

ok.
 
>> +	if (svm->nmi_masked)
>> +		vmcb01->control.int_ctl |= V_NMI_MASK;
>> +	else
>> +		vmcb01->control.int_ctl &= ~V_NMI_MASK;
>> +
>> +	if (vcpu->arch.nmi_pending) {
>> +		vcpu->arch.nmi_pending--;
>> +		vmcb01->control.int_ctl |= V_NMI_PENDING;
>> +	} else
>> +		vmcb01->control.int_ctl &= ~V_NMI_PENDING;
> 
> Needs curly braces.
> 
Yes.

Thanks,
Santosh