Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <20c2142a-ec88-02cf-01f2-cf7f8dfcef77@amd.com>
Date:   Fri, 29 Jul 2022 11:21:58 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCHv2 4/7] KVM: SVM: Report NMI not allowed when Guest busy
 handling VNMI
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Maxim Levitsky <mlevitsk@redhat.com>,
        Tom Lendacky <thomas.lendacky@amd.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220709134230.2397-1-santosh.shukla@amd.com>
 <20220709134230.2397-5-santosh.shukla@amd.com> <Yth5hl+RlTaa5ybj@google.com>
From:   "Shukla, Santosh" <santosh.shukla@amd.com>
In-Reply-To: <Yth5hl+RlTaa5ybj@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?YTJCbmcvMTg4ZHMwMVp0MkJoUnhzTkVsd3NGS0l1L1kxcHRObW5TRHV3bkRk?=
 =?utf-8?B?MWFyM2ZENlYwaEs5ZjU5a0xpNThMb2Q3MG54Z2QwWmx3T1Vka0JucmR0bEF1?=
 =?utf-8?B?c01nU09CWU01d0VjOWp6QVRhVDFRZTkwSitHdUpQSEFHWFZ4TVY5Z0cvUlJU?=
 =?utf-8?B?ajNJSkxNcnhCNFF0OHk0QkFmeEU0dzloWHhndWlBeFBRajlOS20xTFlXMTZI?=
 =?utf-8?B?cEt3K3NQbm56TGdkRVkxenBBbmNBMXBMVDJWSGxMMlBXWDV0SE1TTGh1NWZq?=
 =?utf-8?B?bFNBMUVFbjZnOGdUTG1oOHVTZU8wVDRoZXRBWHJLTDJrNFVhSWIvK0FYQnNW?=
 =?utf-8?B?SGxCZGI1cmovT3JRMW1HWks0cWgzZjF2Y2h2NUhuYml4anJCMG15VkhJcDQ2?=
 =?utf-8?B?NHpNbnpvRUZyc2tSVFlieHRhdkJRU21SUDdUaUJEMm5tUmFGLzJnSm02M0Vk?=
 =?utf-8?B?bnQ0RGtMbHkzZnNFbXRqV3FmSjg2RTM4b3cvbmpLVUttY3lReGJod0t6Wm9m?=
 =?utf-8?B?cnZ4RDdtY3BEc3dhMStoN0FIUGdRQzFYenFsbEhGQ2Q5d2VJREdFWDJaWWQr?=
 =?utf-8?B?NWI4MlduQnZzeVhLelpwZVJYWDROQnNwWkRLS2RsOW15MkJtWFpRd0xXR01R?=
 =?utf-8?B?TkpxQWhlR2U0dHc4MUtlWngwM0FMSTAvS2RLT0dpMDJlRGo0d1pYN0w4bTRu?=
 =?utf-8?B?d09ndmZnQUNBVm5lb25VcTVqZzZtTDhtNU5qZXpVK1hWUzE3eUZ0V1RkZzJy?=
 =?utf-8?B?bzZlT0JHQTB4OVZ0cFhmdDh5MmxtZHdnTWN5K01EWllETnhFcDJrQmx3UGJV?=
 =?utf-8?B?ZTRlZVlCNXNWRjZPRXBnOHRLZUw0MC9MbjJWTDV4N3FYZmkxR29wSGRraUFq?=
 =?utf-8?B?dFYrNXJPdU1YczhxZG1uaVNUN2hITC93ajIzdTVuaWZGY1kwMHdjWUx4U1Jq?=
 =?utf-8?B?MlIxZUo5a09hZElGbnBTYlVLb3ZEUjV3cDRKQWpkSTlyT3gyUVM2QldmbVpB?=
 =?utf-8?B?a2hmczVWa2JaajRwNVoyV0lnL2JRQ3RGSlhWSEU0akRPMWJMTnBRV1V0WThr?=
 =?utf-8?B?eDNXTjFkQytEWkFtWm9SdjZDaGl4enpweXcrWGk2MlphdGxGVyt2VzhidzIx?=
 =?utf-8?B?Q1piN1daaU5PYWlhd0l1aWJud0M4dFVOeW5Bc0sza2xVVTV3bTE4cU54K0tv?=
 =?utf-8?B?Q09EN0F4Vnhja01EbGlDQ0JOVThpbHA3aXlSbGZwL3JVVEZHSHg0Ti9YZ1hT?=
 =?utf-8?B?Q284RkF1V3MvakZvTFg4VlI0bURZRVQzNHVwaXpQVFJjcUphalM3eWFnTUVh?=
 =?utf-8?B?Z0ZaeXNzdmZWWW1kdFV1SnZEZ1kwTTVoOUlYWGZQQXVjaDFmVkU0enFYR0pk?=
 =?utf-8?B?L3YwTnB3STZPTWdCaDVxWUh6bE14Y2pNU0FEMU8xcDRRcTN1aUNDU3RHeWZO?=
 =?utf-8?B?djVJdGlxemJ1alczemw4WnRNMXU4M1pYaDdad0VuR29mSHMyVGhTcW9DSDZx?=
 =?utf-8?B?MjlNd0xXOWI4QjNnbGR2U3FkMXNLckcrdFQycExMZkQxanJMTTl1SDNKcFFC?=
 =?utf-8?B?Y2hnWDFTa01Wb2toWFhZdFR0Ym8vOFltWTZncWNQdTgrVDVPSS9vVHhBOS9C?=
 =?utf-8?B?bGl5REhRM3czRG94d0kweUk0bjB4bkdQYjhlM0lkSFZqT1N3c001aWV5YWh3?=
 =?utf-8?B?aDB3b1ZlcG15YUp1R1ZyK201SHFJUGJ3dnFUWXVtL0xmMEt2UEI4NFpLbkZN?=
 =?utf-8?B?d2UyUzlBNllHUjU5Qk1oeHE5dDlFektyeFhjUDdXOGhSZUYrQThMUU1aYTNq?=
 =?utf-8?B?QjB1WlpZdGEzRlF4Z0NKM1haM01uZzQzK2l1SmFOVW5aVGJBa0RLbnVpc2Ri?=
 =?utf-8?B?REdMSHkxei80ZkgrYTEwMmNzV09UNEdGOHVvK3FZeTB3Yy9uNWoxZ1pyTi8v?=
 =?utf-8?B?bFcwZTRmbjl3M1A3WVNyTDBnbVhaSEs1aVhId1NWY1V4eUFvYWtlK0pWTGNZ?=
 =?utf-8?B?aXYybm1Zb0NSOW9NeWpYcmkzUHJBZUh5aVAyc0Zkbmt2c2dmRHBPalJPUGhn?=
 =?utf-8?B?TVdYK1gycjNDTWFIRW84RVdPWmNKYm4rNTRMS0l3cFkwY1dUMkszb0Ztck5j?=
 =?utf-8?Q?5O/xPOu5GIw8yFT0Y1zCGFIpD?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c98fa337-c0da-44f1-8525-08da71267a2b
X-MS-Exchange-CrossTenant-AuthSource: CY5PR12MB6323.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2022 05:52:10.1995
 (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: rtfVTDMVuMjZ/oY/HVU1MaYEr40nVVsBD5FyqEqdC+m2VdqVLqmaCPHiEl707Ttq9Cf1WWFRKsMH/cFJPnCPIg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5091
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hello Sean,

On 7/21/2022 3:24 AM, Sean Christopherson wrote:
> On Sat, Jul 09, 2022, Santosh Shukla wrote:
>> In the VNMI case, Report NMI is not allowed when the processor set the
>> V_NMI_MASK to 1 which means the Guest is busy handling VNMI.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
>> ---
>> v2:
>> - Moved vnmi check after is_guest_mode() in func _nmi_blocked().
>> - Removed is_vnmi_mask_set check from _enable_nmi_window().
>> as it was a redundent check.
>>
>>  arch/x86/kvm/svm/svm.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 3574e804d757..44c1f2317b45 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -3480,6 +3480,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu)
>>  	if (is_guest_mode(vcpu) && nested_exit_on_nmi(svm))
>>  		return false;
>>  
>> +	if (is_vnmi_enabled(svm) && is_vnmi_mask_set(svm))
>> +		return true;
>> +
>>  	ret = (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
>>  	      (vcpu->arch.hflags & HF_NMI_MASK);
>>  
>> @@ -3609,6 +3612,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu)
>>  {
>>  	struct vcpu_svm *svm = to_svm(vcpu);
>>  
>> +	if (is_vnmi_enabled(svm))
>> +		return;
> 
> Ugh, is there really no way to trigger an exit when NMIs become unmasked?  Because
> if there isn't, this is broken for KVM.
> 

Yes. there is.

NMI_INTERCEPT will trigger VMEXIT when second NMI arrives while guest is busy handling first NMI.
And in that scenario, Guest will exit with V_NMI_MASK set to 1, KVM can inject pending(Second)
NMI(V_NMI=1). Guest will resume handling the first NMI, then HW will
clear the V_NMI_MASK and later HW will take the pending V_NMI in side the guest. 

I'll handle above case in v3.

Thanks,
Santosh

> On bare metal, if two NMIs arrive "simultaneously", so long as NMIs aren't blocked,
> the first NMI will be delivered and the second will be pended, i.e. software will
> see both NMIs.  And if that doesn't hold true, the window for a true collision is
> really, really tiny.
> 
> But in KVM, because a vCPU may not be run a long duration, that window becomes
> very large.  To not drop NMIs and more faithfully emulate hardware, KVM allows two
> NMIs to be _pending_.  And when that happens, KVM needs to trigger an exit when
> NMIs become unmasked _after_ the first NMI is injected.
> 
>> +
>>  	if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK)
>>  		return; /* IRET will cause a vm exit */
>>  
>> -- 
>> 2.25.1
>>