From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751002AbcFASSa (ORCPT ); Wed, 1 Jun 2016 14:18:30 -0400 Received: from mail-bn1on0069.outbound.protection.outlook.com ([157.56.110.69]:31168 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750850AbcFASSW (ORCPT ); Wed, 1 Jun 2016 14:18:22 -0400 X-Greylist: delayed 51356 seconds by postgrey-1.27 at vger.kernel.org; Wed, 01 Jun 2016 14:18:22 EDT Authentication-Results: amd.com; dkim=none (message not signed) header.d=none;amd.com; dmarc=none action=none header.from=amd.com; From: Suravee Suthikulpanit Subject: Re: [PART1 V5 13/13] svm: Manage vcpu load/unload when enable AVIC To: Paolo Bonzini , , , , , References: <1462388992-25242-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1462388992-25242-14-git-send-email-Suravee.Suthikulpanit@amd.com> <573201AB.2050006@redhat.com> <573B0A08.7040604@redhat.com> CC: , , , Message-ID: <574F26E2.8020803@amd.com> Date: Wed, 1 Jun 2016 13:18:10 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <573B0A08.7040604@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: BY2PR1001CA0030.namprd10.prod.outlook.com (10.164.163.168) To SN1PR12MB0447.namprd12.prod.outlook.com (10.162.105.140) X-MS-Office365-Filtering-Correlation-Id: 6a9812a9-a557-46a2-fac0-08d38a491b30 X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0447;2:RXHAGTC3cXXHgaRgoovXgR0qkiA4r+KL4vJTJYrJMcLuo/lg24JZ3bGajOEfIzmCNZWZwS9C3E3tTAV3iPUPDBWmfE66KIGgPiX+eusIWEZJayocv2wKIYf8Ozz7tTGjbTy15CD7a5Vv4o/WdlWvrAr2vQouN//Y7wCMBsRSdSjb1RVv4M706tnRFZxL3yYR;3:AiKk/AbZmtC1cMmJrKZoNf0DwStKTnve+owmLizS2n8/5ZM174ci0dLU5xV4shuuWMkw8IKUJ6w9h+DvpMGlOvZzYKqTIszwlRyJ7zCmG6ZX++5OojwXof2VnPM+Uc2C;25:4v2k5ps1OW1WyukgpkzBitgCflsit1QFmAezCuI15eo1iN0R/VtinVYjN4McModGuO0mXo/9HUiqmXnKheZzW9I6oeDIXNwi/l5T5a4079dtr7w1JXuVGq3ygUO2JUg5Sh5S/K5i4LB26J114kwWmG/vZ1oTkINK85VvYWxd3EYe6F2FnGP14fqlnXT4B0uUMUEznTMk9Y7PkjlsEhEqzxAKI4XMGNkBlnyngJ9hHFxFeKY/R4qB+lEoFceCOqEfWrOGGDYrInfcSbhqmUpWNjM/Y9Ji+Pd6J+Ti2T442FFFvKFRYnBZg/1hfLBnuno4eZXnkoGzZjPw6M2B+2Zp41CkUyY1H5HjR7SfinivjhXtL0sweJ3LraT1uWylJC/je6cI2aZL4y/tO+jx36ST6es6ACitMxvimBIlcelcRLQ= X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0447; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0447;20:ULClG4BcZycwd5v80HA+uYHrH/RV4Ct9FgoOkwG7EG2NEYYPbY1Y54WCcypokar/OBFkkzYrrKsNJDcrV5G9H3t3jC5e7O29weHzr4l9uxj5KdYiQtzeYaTFll1aq3o6Q7moPueXWxuY9Mhh7elC76iMfrjIB8TqRk7WhiuaIujI97FBZBwMD4NI7sVBNhHPzD3l8f9nWGOuJon3hQhwaIn7k82GsjyInrWx1NHD/iXWeGtLSdAq67dIo4XFPALI1T5SXOjA4aJ1T4Ts6TbeZBgGMZGRDum+Cu8DGYMEeyP1jaRljKmSYoTQFyn+Nv5wUk9xfmeOXKCjU22087dGF0gheHPfQUMzdfA7fwrQt6ewqkPl76uZHATaaFKppXkTkUXH4ic1r6JEBqxHV2wNVFePmyGhTe+QUnS8vi6OqrEq2r9fT1XZa795vpk04MwwcAWfn4I6fpO53xcuVM0F4vkfvCb0Hdtf6HVVuDYhgjjPmLbnLsutdiQFag9IROZ0;4:OrcNfUaqEO1T13pc3e9TP4Bd8GUeARdPHcqCpwfECLLYvvGBwCVKV7HORRS3BQo/Dahf5D+Rw6od8fRhBC/5BiNJd6ypgGLitVWDOGbJGGenWI/hwzWHJ4xn0NyGEI8uSCmxdXDY0wrs8p29dXxxS8jrqg7Bb6w70cXDgHgCJsc1OM9Pq39gNtJfQdqD5mzCFE/Wm/EtHC1yf+swNCtmVNgX6dd5LR005umeUBQEFwBkaYtxAjGEqRU8J1cyX033PdcpwX7cpUoVrT0BC/EECAOowmCfB+chUGo6pjAqm76kmwpp9f57Ewn+qlYIscpGaUM9ONWk3002s4/NGMJ/88DX4RQ6qKA2OLzFN5G72S9DAqLy0EfitZaHI6NbMfpY4oDmRvComLrI0IG8yF3oxdQZRliajXCEh5anqHFqwxQ= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026);SRVR:SN1PR12MB0447;BCL:0;PCL:0;RULEID:;SRVR:SN1PR12MB0447; X-Forefront-PRVS: 096029FF66 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6049001)(6009001)(24454002)(93886004)(42186005)(4326007)(2870700001)(23676002)(54356999)(87266999)(77096005)(3846002)(6116002)(586003)(86362001)(2950100001)(65816999)(76176999)(50986999)(65806001)(189998001)(65956001)(66066001)(2906002)(8676002)(4001350100001)(47776003)(2201001)(5008740100001)(19580405001)(81166006)(36756003)(33656002)(83506001)(5004730100002)(59896002)(345774005)(50466002)(92566002)(5001770100001)(19580395003)(80316001);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR12MB0447;H:[10.236.69.175];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtTTjFQUjEyTUIwNDQ3OzIzOkVuZll5TWFhSEM4U2Y1bm5SYjFGS0JNZ0t1?= =?utf-8?B?Q0lNZm1OVk5YUk1yNXk3ZCtWQXltTGIvZkp6cGR0NlRqaEJhWTFHcS9HQ1lZ?= =?utf-8?B?SnUweWk4U0Q3RWlhTmlFaTNLV29oNm9vMklhSmtzd3czR2pneVBhWnNrYkhw?= =?utf-8?B?cHFPYUtFZTFHZ0ViTmRIcmNGZUg1c091VFIxdk9lZi81eXhCcEFtWGgwMWEr?= =?utf-8?B?dkdZSDdMYVAwcm1WcDN5MCt5Z3kyQnFCd1ZiVGNoMytjaGtpeGx0N3hDL1JI?= =?utf-8?B?SGdTZ1ljN3JhdkczTytLclRuRmI2R2NRYXl1ellWc2gvWjcyU2N0ejUvNThj?= =?utf-8?B?UG51N1M4N0pwQTlvMkQ5RTZpb1ZxM2JUOHFNUU04d3JsWFp5cy9vT0wxbnZx?= =?utf-8?B?S1NPZkM3ZjdIYksrODQ0cy9DRjdiU0REU3hmZXpPb055cTI5YTNZdkU5YkZt?= =?utf-8?B?Z04vZTRqYUFPREZRM0QwYUxVVTJBU1dUcW9qQzhuNERCNFIrZytFZjVLZzI3?= =?utf-8?B?MytTbEZrMXhMODJXUVdUQm8vWm1nMlRZMTQ0cjhnZ0hucVN5YVBDamVFSVdS?= =?utf-8?B?N21ja0VXWXlsTVlpWFBPV09UU2I1aEJNY25uNFZnS2NNUVNvT0c2RzZ3K1U5?= =?utf-8?B?Y3UrT3FtN2JVNDVQa2treU52eitYbnlWVW8xVFNldk5tZDJnc3g1MEVNajlw?= =?utf-8?B?NTZkbi84Mm4yRG90aGhoOVZLZHVVcXE3eDJhZ3p3cDdEUGZCdnhDNEFjNk5u?= =?utf-8?B?K3NpMkJhSVdxeUVmNDZRNk1hdnh0ZVY0OGRmbkw2dWVyMzBZNUpXeTZlVTFv?= =?utf-8?B?akVKWGRhTktkbjV2cGRaMHNGWGVaSmYweGx2WGV3VGYwQnlDbEwvU1kyMHZW?= =?utf-8?B?STJOaVlEcitqLzVzYmxqanRTRU1lUTZWQi8wSExFM2wxUVd4V0l1Um5oNXRi?= =?utf-8?B?cVZhNDhNZGxRZGFNR1dOWUpmeXFkK1ZZZEV3Z2c2QnRCRmRDZGVyMjkzdm1t?= =?utf-8?B?dEpyRjFVU1RDUGg0aWJ1NCtoQkNab2thNWdmTUFRUEZncmhwUW91MGxkOTZB?= =?utf-8?B?MVpHQ2wzZm05Mm1XN2grL1B4YVZ5dDF5TnlhZmlwSzdXek1nb3poMkMvcTBi?= =?utf-8?B?clFuZ0YyNElqVTdhY3Y1MzlsWnN4dzk3cm5wVnhDK0dGWHNoVU9pUlprS2hD?= =?utf-8?B?blB5MzlrdnVXdytXcmNGVlJNR1VIbHFrVUlHUWJ6dWdScUZIZVJLS0pHSHht?= =?utf-8?B?MWEzcmR5M3BMZXRnSmtmYmJaOFJwUFE1NGNQekFTcHdvMkxtNzhTSHBWcDNH?= =?utf-8?B?cG1xTk9IaGlKOUc0NnVnNm1JblQ3cG00VTg4bmxvQ1lSM3c0cExyU1kzRDVQ?= =?utf-8?B?ejJiYnRwNHMrOThCVC9KSEJXd0NtQXJkck5nRVZvdWV0K3pUdll4UXZxRVVr?= =?utf-8?B?OVNaR2o0eTlzYW9BcG9pckJtcHRLN1FQVmQ4N2pUbWpPbEUrSGR4WnowUEM2?= =?utf-8?Q?Na1Li55Qfz2nWFQ5C6MCWEPf5R91CTqLhyaEYmAW9rPBkb?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0447;5:ETqXoMaC2K/5RugE9xPJqAn5dEC0bzkSgGVqyRf79ZDFOyOpZViIgtvpeRTJt6lzRIhDaSu+3P03Nbdnf2e/HGL7WNbjeHSVkWzmFtjaQsXc8RNuA+Dc3VbO7D9yrxQ82BE87hKhB65nTj7FkUV9aQ==;24:Bj++4RxhtrURsAbnDlZOfgtmS1Addi/5GRCSYbi84DQ9jUgxdrrNjtpR3wUXjT4Oz4mCNagE01/rBPBUu8dJmElqtTjlSrK1OUd86vt8HeA=;7:fjT2QzBrnApTt3WO6THY3Asz3oF27CKxRYT/xV8OEUNXg5+6chzXdTJ0x0rZz/l0lpDs8YIIuWcPWt25qzcfCz1ElmsBRKmOiAM1UQR1Dcw+emjQSMvHfUSR7xP28BTacftZiCYDQpvOq71NR1sJq/y/N0tPT4l0pEGSJA4q1PA=;20:st2TFBO3sFCQaLl8nPPchUQmjBGLjpRpGu0Pes3xBRFW5GVnG/0ySnGKu92UR7WCccEeFlhi9Vs1Oc7595s8EdbEqMcNIsfSZjeUJ0iyWguX024Cq20NWoYGkPU9vwhxvvIZ+dBUTHSE9Sdwd08hB7vHGiy45yz/lwvN0e7KsDNhjPLVMSew0snzoPdNsSppgvlifCYVv0MvKY/OLQ8pEJM2iT3Z0QGnsJBuCTeB/NYRYnA8ZsPSWsyo3XjtatVG X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Jun 2016 18:18:17.2814 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB0447 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Paolo/Radim, I was a bit behind on catching up with the AVIC stuff the past couple weeks. Thank you for the help cleaning up on this patch and pulling this in to your tree. I can see now that it has been pulled into the 4.7-rc1. I really appreciate your help. Please see a minor comment below. On 5/17/16 07:09, Paolo Bonzini wrote: > > > On 10/05/2016 17:43, Paolo Bonzini wrote: >> >> >> On 04/05/2016 21:09, Suravee Suthikulpanit wrote: >>> From: Suravee Suthikulpanit >>> >>> When a vcpu is loaded/unloaded to a physical core, we need to update >>> host physical APIC ID information in the Physical APIC-ID table >>> accordingly. >>> >>> Also, when vCPU is blocking/un-blocking (due to halt instruction), >>> we need to make sure that the is-running bit in set accordingly in the >>> physical APIC-ID table. >>> >>> Signed-off-by: Suravee Suthikulpanit >>> Reviewed-by: Radim Krčmář >>> --- >> >> I think this is the only patch that needs a little more work, because >> there are a bunch of unused return values that really should be >> WARN_ON. In addition the load and put cases are different enough that >> they should be separate functions. >> >> Can you please test this? >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index f3dbf1d33a61..3168d6c8d24f 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -184,7 +184,7 @@ struct vcpu_svm { >> u32 ldr_reg; >> struct page *avic_backing_page; >> u64 *avic_physical_id_cache; >> - bool avic_is_blocking; >> + bool avic_is_running; >> }; >> >> #define AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK (0xFF) >> @@ -1321,18 +1321,20 @@ free_avic: >> /** >> * This function is called during VCPU halt/unhalt. >> */ >> -static int avic_set_running(struct kvm_vcpu *vcpu, bool is_run) >> +static void avic_set_running(struct kvm_vcpu *vcpu, bool is_run) >> { >> u64 entry; >> int h_physical_id = __default_cpu_present_to_apicid(vcpu->cpu); >> struct vcpu_svm *svm = to_svm(vcpu); >> >> if (!kvm_vcpu_apicv_active(vcpu)) >> - return 0; >> + return; >> + >> + svm->avic_is_running = is_run; Shouldn't we do this below ---> >> >> /* ID = 0xff (broadcast), ID > 0xff (reserved) */ >> - if (h_physical_id >= AVIC_MAX_PHYSICAL_ID_COUNT) >> - return -EINVAL; >> + if (WARN_ON(h_physical_id >= AVIC_MAX_PHYSICAL_ID_COUNT)) >> + return; <--- HERE >> >> entry = READ_ONCE(*(svm->avic_physical_id_cache)); >> WARN_ON(is_run == !!(entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK)); >> @@ -1341,36 +1343,45 @@ static int avic_set_running(struct kvm_vcpu *vcpu, bool is_run) >> if (is_run) >> entry |= AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK; >> WRITE_ONCE(*(svm->avic_physical_id_cache), entry); >> - >> - return 0; >> } >> >> [....] >> >> The two functions now have the same signature as their callers, >> svm_vcpu_load and svm_vcpu_put. > > Radim, does this look sane? I plan to include it in my pull request > (I'm running AMD autotest now and it passed the first few tests). > > Thanks, > > Paolo > I have also tested the changes in the 4.7-rc1 on the CZ hardware w/ AVIC and everything looks good. As for the nested virtualization. Currently, this is not working with AVIC enabled on the host, but it works fine w/ AVIC disabled. I'll look into enabling nested virtualization w/ AVIC enable next. Thank you for all your help, Suravee