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: <214b5d5e-f936-f50c-b26c-334ecbbdface@amd.com>
Date:   Tue, 1 Mar 2022 16:59:23 +0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.6.1
Subject: Re: [RFC PATCH 04/13] KVM: SVM: Only call vcpu_(un)blocking when AVIC
 is enabled.
Content-Language: en-US
To:     Maxim Levitsky <mlevitsk@redhat.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org, seanjc@google.com
Cc:     pbonzini@redhat.com, joro@8bytes.org, jon.grimm@amd.com,
        wei.huang2@amd.com, terry.bowman@amd.com
References: <20220221021922.733373-1-suravee.suthikulpanit@amd.com>
 <20220221021922.733373-5-suravee.suthikulpanit@amd.com>
 <dc820a37ef302ed7c11315c01c6f434d5506c543.camel@redhat.com>
From:   Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
In-Reply-To: <dc820a37ef302ed7c11315c01c6f434d5506c543.camel@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Y1dGL08wZEVEWnMySlIzM3QyVUdISzJ1d0xuQlB3aitUSnpSZnpCVE85ZDJN?=
 =?utf-8?B?eGM5OGxzVDJjMnM4S1h3aUV1Q3BFUVdVMkxsM3RWYk5UN3gwZFh4TzFMbmRM?=
 =?utf-8?B?TG11VEFWSWtMMVhna0pKcVJOcGkzQmFjTy9lQ05GU0psZXl4R2hndFd2UDhs?=
 =?utf-8?B?K01NNnArU2RnN2NEZnV3Z0RkSnZSZXVIbFJsNDN1VHlOZXNBN2FlUFpWUHBa?=
 =?utf-8?B?QVJDQlUwclE2R0tCbjRzZWp4Uk9ySHNubWdrMkUrZjFxdWRyeXJTMGRkK0RC?=
 =?utf-8?B?MFFzWVo3OTdoMmw0U1VZTkhoNXJrM1VZc1NsZXRqS2ZBL0h6WXQ2VWJjWnVz?=
 =?utf-8?B?b3l0UFdaa1lDYzFmSVhWZzhUTmZzTzlSVHRIeFZsRGxva1c3YVluZDhaYVU0?=
 =?utf-8?B?MEFDTGl6RDl4bnJZblVTUnhHWStUTFpQUGNBK1lKR1BYNE85aDRwRUo0WkRD?=
 =?utf-8?B?bjhoV0M2K1VDc2FNeXZGeHYyS0dPd3hqRk9KUlFBenJSMHoxeTFpc3ZxOHBh?=
 =?utf-8?B?TXVzZmJCWjE0LzcwdjNOZFdJSk9ORytocjdiTDllRW9CdWYvN1JvNytOSWxx?=
 =?utf-8?B?cGF1Z0x1a2EzUU5wRE0rVjlweVg0N1R1T1NUK3ZJMkx6RXJSdFMyZmZWQWVr?=
 =?utf-8?B?ekFUZlc3QkRXUXVhbytyay8xZ2x2YW9IbHlUaTlXV3dUWEMyK3htY0dEOXhv?=
 =?utf-8?B?Wmw5TDhHYzlvTzBzdmRldkJsZ09NS0hidGZMWGg5eGRyT2pRRTltVnJEU3Zk?=
 =?utf-8?B?STVyUDc5MDd2cmkwaThYMDFyWlZFaGVrMVlZNmdOZEZYM1BROFFMMHo4N0d4?=
 =?utf-8?B?QWp2K3gzcFNFcXlJV3cwWm03c0dJY29DRlNwOEFKTmNUd2FGMlNabVFXcUNn?=
 =?utf-8?B?TlY1NkNYT2F5QUxTODB6SExSbjJtbit2U29lREZZZHZIRjV5Z1FZei8rRjh0?=
 =?utf-8?B?ZGw3UTRVVTlNdklHaUhmcDJ1OWN1SXQ0dERWeGlRMmtQRDlXQnFDQVRFMUsz?=
 =?utf-8?B?Y25CM0p0d3p2aGM5ejI3b2J4Y1c2K3lQWmRzOUxpbnZjVmtNUXJ2WmRJV3kv?=
 =?utf-8?B?bFAvWFU2RDB2My96d0F0ZUpWT1h0dW1jaXRHSFpzc0VxVVVYbGhzdkMycWpH?=
 =?utf-8?B?cXhFMDFKTVY0RXM3YzdaaTNEN3NzdjZRY1F3OWVubUVjSUxuYWpHNlI0eE1E?=
 =?utf-8?B?TnJYcEtRNmIwWElkVHIwcFo2U0IrUEt3ZUw1WGlIWGo4ZUJuY0o1cThJaTIz?=
 =?utf-8?B?QUJMSzhGN3htZytsYzRMRGVDdFZTWGJOYnFTcUJEMFFybFdRZHR5N1hLVjhO?=
 =?utf-8?B?cVRCKzI5U2NSRXRUbkNhZ2Q3dG5EQ01rKzY4TmxmVVZYMlVhSGRzM1lMeDNM?=
 =?utf-8?B?OEpWZHBsUUhkaklIaXhlcjFmY2JWWC96MlBWcTBML3lxMXZCR3NUYWlKWWJL?=
 =?utf-8?B?SVhLUFBqRTJCcStGUFY4bi81SHhCczdaaUVEUTl1NTdRNFQ1Ty9GMStGd1FO?=
 =?utf-8?B?akV3UnpSaUxob3gvU0NiYU0rTkRncUJTUWNIYjYzNnh4TGdTc21pMjRxNGlk?=
 =?utf-8?B?Q1JteFR5MHBjaE92cDVTQk1WVHcrU3JENVNtRDFKdVZRTW9GdGFxZUxpTXhD?=
 =?utf-8?B?YW91K1lWK25YOFppWVhsa2FtZEhFM2kvK2hxWHBYU3ZFQlJOZUhZSE9rc1hy?=
 =?utf-8?B?M3hGL3pXUUM1YkNpckh3RDE1Wm13VzVSa1JMM1BBNnJKbnpNTnlvN2xZNmFY?=
 =?utf-8?B?V21xSnp1WFlPdUJiLzdVNFYzbDZUUUlMS1dOZktaSHluL21PK0EzNDN3T0Yv?=
 =?utf-8?B?Q3U4N3pPWXpRZndMeGxhZTJycGpBWk5aYVl0cWltQk9TMDVUaFFRTHlTNUFr?=
 =?utf-8?B?QWpzb3ROK0gxNmdVdG5FaSt4RlJ1a2NRYUpmZUVKeGdaNnBEaUIrc0ZKL3J6?=
 =?utf-8?B?L0MvczZNeFZCOE12UXl2eUxXU2RYSE5YUjVsWHlqYXVlWDdmRUxMbHh4NGNE?=
 =?utf-8?B?SlZSSHV6L0RENEQvUEg4bEJWcnc1ZlNtTmkycWU0NUZmRUtyQ2cvcXlhaFMr?=
 =?utf-8?B?K283dDhsWWFqQ0JzdDZLdVZNOHdBTmR2b2lKb1pPMzdORGlLTUZ5Ynh2eUdk?=
 =?utf-8?B?ZHh5NzZ2djBpZFE2cjMxVldpUThWZEdSM1ZkWlJldWROcTR5T2JRb3B1ZnVD?=
 =?utf-8?Q?il/JG7JAeIxr+8s9vgf3GZk=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 905f5749-3654-48e6-c93d-08d9fb6a303d
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5445.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Mar 2022 09:59:34.5033
 (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: WQb9pg0XE/RYBrvNHoLJ8rL910joX4kQhWke2INuhctfxEjL0HK1brVLm2fjUXPOHLDQmEsP5AGDY6i5gnyQmw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB3719
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,T_SCC_BODY_TEXT_LINE
        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

Hi Maxim,

On 2/24/22 11:54 PM, Maxim Levitsky wrote:
> On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote:
>> The kvm_x86_ops.vcpu_(un)blocking are needed by AVIC only.
>> Therefore, set the ops only when AVIC is enabled.
>>
>> Suggested-by: Sean Christopherson<seanjc@google.com>
>> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
>> ---
>>   arch/x86/kvm/svm/avic.c | 12 ++++++++++--
>>   arch/x86/kvm/svm/svm.c  |  7 -------
>>   arch/x86/kvm/svm/svm.h  |  2 --
>>   3 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
>> index abde08ca23ab..0040824e4376 100644
>> --- a/arch/x86/kvm/svm/avic.c
>> +++ b/arch/x86/kvm/svm/avic.c
>> @@ -996,7 +996,7 @@ void avic_vcpu_put(struct kvm_vcpu *vcpu)
>>   	WRITE_ONCE(*(svm->avic_physical_id_cache), entry);
>>   }
>>   
>> -void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
>> +static void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
>>   {
>>   	if (!kvm_vcpu_apicv_active(vcpu))
>>   		return;
>> @@ -1021,7 +1021,7 @@ void avic_vcpu_blocking(struct kvm_vcpu *vcpu)
>>   	preempt_enable();
>>   }
>>   
>> -void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>> +static void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>>   {
>>   	int cpu;
>>   
>> @@ -1057,6 +1057,14 @@ bool avic_hardware_setup(struct kvm_x86_ops *x86_ops)
>>   		pr_info("x2AVIC enabled\n");
>>   	}
>>   
>> +	if (avic_mode) {
>> +		x86_ops->vcpu_blocking = avic_vcpu_blocking;
>> +		x86_ops->vcpu_unblocking = avic_vcpu_unblocking;
>> +	} else {
>> +		x86_ops->vcpu_blocking = NULL;
>> +		x86_ops->vcpu_unblocking = NULL;
>> +	}
>> +
>>   	amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier);
>>   	return !!avic_mode;
>>   }
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 3048f4b758d6..3687026f2859 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -4531,8 +4531,6 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
>>   	.prepare_guest_switch = svm_prepare_guest_switch,
>>   	.vcpu_load = svm_vcpu_load,
>>   	.vcpu_put = svm_vcpu_put,
>> -	.vcpu_blocking = avic_vcpu_blocking,
>> -	.vcpu_unblocking = avic_vcpu_unblocking,
>>   
>>   	.update_exception_bitmap = svm_update_exception_bitmap,
>>   	.get_msr_feature = svm_get_msr_feature,
>> @@ -4819,11 +4817,6 @@ static __init int svm_hardware_setup(void)
>>   
>>   	enable_apicv = avic = avic && avic_hardware_setup(&svm_x86_ops);
>>   
>> -	if (!enable_apicv) {
>> -		svm_x86_ops.vcpu_blocking = NULL;
>> -		svm_x86_ops.vcpu_unblocking = NULL;
>> -	}
> Isn't this code already zeros these callbacks when avic is not enabled?

Ah, right. I'll remove the setting to NULL.

> I am not sure why this patch is needed to be honest.

It's not related to x2AVIC. It was recommended by Sean earlier
in another patch series:

   https://lore.kernel.org/lkml/Yc3r1U6WFVDtJCZn@google.com/

Since this series introduces the helper function avic_hardware_setup(),
and re-factor the AVIC setup code into the function. So, I am including
his recommendation in this series instead..

Regards,
Suravee