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: <3354e829-ecca-37dc-a530-aaa07e839200@amd.com>
Date:   Tue, 1 Mar 2022 16:45:57 +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 03/13] KVM: SVM: Detect X2APIC virtualization (x2AVIC)
 support
Content-Language: en-US
To:     Maxim Levitsky <mlevitsk@redhat.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org
Cc:     pbonzini@redhat.com, seanjc@google.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-4-suravee.suthikulpanit@amd.com>
 <720d6a8d6cc3013f2f55750982439eac7ed950b0.camel@redhat.com>
From:   Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
In-Reply-To: <720d6a8d6cc3013f2f55750982439eac7ed950b0.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?ZEMxS3NkT3lzbjlaQnh1VEVFeXIwZ2p2alBxZGNVY0hkekN2TGRXaHVqdUhP?=
 =?utf-8?B?QzhqcHp5SEg3R0poNjRGSTlsWEVEckp3d1ZEbzQyT0tDV28wZUxqTkVMaExq?=
 =?utf-8?B?Y0VHNGpWS1hTT3h5SlFVMGV6dGVuL2x5TFBQUFdzK1ZwbDhCc09IQWlhbFdw?=
 =?utf-8?B?N24xZ0RoUTJjL1A2NmFyVGF2bFBEVUxyM293ZTE5cWh6akR1UXlVbzMvMld2?=
 =?utf-8?B?Y0Z1RSsySENvcmRUR1JDNVhoOHg4bEdqMTNxUGdRNmN1QThvMlBhUk4wMnpX?=
 =?utf-8?B?RUZlQ3cyb1Y2UjhZOXV4UXR2Y3ZkdENQeDAyRDNGTk1pd3UvSWRrSHVZNk9a?=
 =?utf-8?B?OXpSWGdJOStkRjBUWG1nWDVvRHdrMU5yano2MmljOGRJTkVzY1dYbFNTaGto?=
 =?utf-8?B?SGF0QjlqRWIxUTVZWWZDNXo0UUpsYjMra3lURTZtN1djM2g3QUVVOVdEUGx6?=
 =?utf-8?B?WEFjak1PRGl0SXFTQjVjVExRT0Z0LzNpeS8wRy9pdUZjT2ppOGtzclJ1Nysy?=
 =?utf-8?B?N0NjM2l0RlVwZ1RmWmJIS2xKT0VwUmVSeUF6eGFSOExaaFdKWlVJWGZKZFFK?=
 =?utf-8?B?K3lhVkpqcDA4eU5NZnlOL3FDQ09pVjZiTDVVNzUxcWpGTFJUK2N0cXFVZkpE?=
 =?utf-8?B?bDNkRnNxOTN5cVZCMng3eDczZ09NYUNSVVFJOXpuUzF4ZGpudDNyUmlsSXpK?=
 =?utf-8?B?OWoyRkVwdnNmeHVVSy9WbFJrbTBVN0lVRkNpdnZLWW45cTdOblFMOUVkTmRH?=
 =?utf-8?B?VWNxRWMzaEFDR0Y1bTJnTS9xY3VuZUZtb1pqMVUwbmZIalFOdkowMlEwcGx4?=
 =?utf-8?B?UGpoVXZVOG84Yk1qMEZheUYySTJyclZFT21DZk9zZ082VXBaU3FJUms1ZVcy?=
 =?utf-8?B?TzFBQ1V6d0dLVDI3d2RYWEVzL1JBTTlFTDhSODNBUU8xWDJLNFhJTk9rcVdu?=
 =?utf-8?B?V2hGQkR2ckQvaHhGUXZoeGwzY05PdWJEM0VVMkxLYmExaHdySU16TWYxVHYv?=
 =?utf-8?B?UGRnMEs2ZXAxL3BVbmlneXRFbCsxbUJwRlB0VTZlaXhxb0FKTjZ5c090MWY5?=
 =?utf-8?B?MzIzWENYbEpxM0RPVm42bnhvTDBKREllWHg0ZHIxcUVpUGVtOHRRejNDWVRM?=
 =?utf-8?B?YlNJRHBOblIwYm9jVGxIb0tESG9ZMkptSUhraXBLbUFRZ0NmSjhzTXJsUWR2?=
 =?utf-8?B?OThlSXUxSlJwcTdhSENlbTlFblJ1M2RZMk15UEZldFlsdURVRGNmSGZSUVY0?=
 =?utf-8?B?cGc2UjdhN0lWQkV0NGpnQTAzZDR6RTJKWXVsOSsxMXl5VW5CMjhhYy95cWg5?=
 =?utf-8?B?YVY0Zll5Y3hEOEFCUEsxcXo0b3diRlpKbElieFhNZTF5eUdYa2F0QW1rWElC?=
 =?utf-8?B?MXo5UE40U1Q3bzdqYVp0Z2hLVG44RVFmS1AybUUxeGpjRVNFOEJ0WHorWmNi?=
 =?utf-8?B?aGFGcEpQK3lGWVZFRC9kSG5vdUNITXVYd3FZRlducjdINGVkQWV6alROdUdP?=
 =?utf-8?B?dE9KQUZ3Zi9QOExEYzNKdVRLdTBJYW1Qcmg0dkRXSHV5RDk4V0JPcTJDNVBh?=
 =?utf-8?B?Y0FnVmlDY29VYlF6ZXZBNFNnd0plcVcyU25MVVcvWElYL090bWV2TVkzcDJC?=
 =?utf-8?B?cEJocnN2MG5ocnZvVEJScXhCNW13aEtZaWFjdlNvUTNiVnp5a2tEQWFZbytH?=
 =?utf-8?B?SHZkL1lSMTJTQUxrY0dqeW1DLzRWbHh1VHZlclR6OW1mWnNsVXN4cC9jTFJB?=
 =?utf-8?B?YTRqWUxaNXJpbS9IKytmRUVJb3lUU0Z1NWoyMFRkQVVUVGFHMEVrRUVSdHZ5?=
 =?utf-8?B?WHNQUFVBUEk3cGZ3bFhDN1pab1ZXQWhwWG5PaWpJK2NwZmVOdTlWRWNiU3F2?=
 =?utf-8?B?Y0o3ZS9BSkZSdWZSZDFsV3pER3hFbTM3RWx3RkVocmtUNzdjRHFxcU9ZUGU1?=
 =?utf-8?B?WStaUjVRbmV2aHBWY2NvMldsVWhTUExXRzdMY29DbWxvTysyVE1uenh4UlYv?=
 =?utf-8?B?dm1GVm9za3BPdmIxQ3lQbUZmK25TbW1qSUZBaGdNVG5wT0hIZHJzN2pMakxX?=
 =?utf-8?B?RkxXNHMwN0dVNmxPZ2lSU0pLUVFRUkhEZFBMRGNscTgwZUJyWGhINFROSDZ4?=
 =?utf-8?B?ZmNCZi9YSVhaL1NmSGJ4SUlIYUNIVjQrdFc4NXBEWk1VcmF2Z2MvMG04eENi?=
 =?utf-8?Q?ASBVki5WlYhgx2ku4UZagck=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b8e9baa7-ab81-45d0-dd93-08d9fb684f73
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:46:07.9250
 (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: L45Y/bpAGLn4WXEVBRq9BkGN1OWrUJSuF9wbngC8coVb3pZFPi4jTv6f7AHCUKUHAotePjwXCdoFvFN9KUGsWg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB5733
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:52 PM, Maxim Levitsky wrote:
> On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote:
>> ....
>> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
>> index 472445aaaf42..abde08ca23ab 100644
>> --- a/arch/x86/kvm/svm/avic.c
>> +++ b/arch/x86/kvm/svm/avic.c
>> @@ -40,6 +40,15 @@
>>   #define AVIC_GATAG_TO_VMID(x)		((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK)
>>   #define AVIC_GATAG_TO_VCPUID(x)		(x & AVIC_VCPU_ID_MASK)
>>   
>> +#define IS_AVIC_MODE_X1(x)		(avic_get_vcpu_apic_mode(x) == AVIC_MODE_X1)
>> +#define IS_AVIC_MODE_X2(x)		(avic_get_vcpu_apic_mode(x) == AVIC_MODE_X2)
>> +
>> +enum avic_modes {
>> +	AVIC_MODE_NONE = 0,
>> +	AVIC_MODE_X1,
>> +	AVIC_MODE_X2,
>> +};
>> +
>>   /* Note:
>>    * This hash table is used to map VM_ID to a struct kvm_svm,
>>    * when handling AMD IOMMU GALOG notification to schedule in
>> @@ -50,6 +59,7 @@ static DEFINE_HASHTABLE(svm_vm_data_hash, SVM_VM_DATA_HASH_BITS);
>>   static u32 next_vm_id = 0;
>>   static bool next_vm_id_wrapped = 0;
>>   static DEFINE_SPINLOCK(svm_vm_data_hash_lock);
>> +static enum avic_modes avic_mode;
>>   
>>   /*
>>    * This is a wrapper of struct amd_iommu_ir_data.
>> @@ -59,6 +69,15 @@ struct amd_svm_iommu_ir {
>>   	void *data;		/* Storing pointer to struct amd_ir_data */
>>   };
>>   
>> +static inline enum avic_modes avic_get_vcpu_apic_mode(struct vcpu_svm *svm)
>> +{
>> +	if (svm->vmcb->control.int_ctl & X2APIC_MODE_MASK)
>> +		return AVIC_MODE_X2;
>> +	else if (svm->vmcb->control.int_ctl & AVIC_ENABLE_MASK)
>> +		return AVIC_MODE_X1;
>> +	else
>> +		return AVIC_MODE_NONE;
>> +}
> I a bit don't like it.
> 
> By definition if a vCPU has x2apic, it will use x2avic and if it is in
> xapic mode it will use plain avic, unless avic is inhibited,
> which will also be the case when vCPU is in x2apic mode but hardware
> doesn't support x2avic.
> 
> But I might have beeing mistaken here - anyway this function should
> be added when it is used so it will be clear how and why it is needed.

I will remove this part.

>>   
>>   /* Note:
>>    * This function is called from IOMMU driver to notify
>> @@ -1016,3 +1035,28 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>>   
>>   	put_cpu();
>>   }
>> +
>> +/*
>> + * Note:
>> + * - The module param avic enable both xAPIC and x2APIC mode.
>> + * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
>> + * - The mode can be switched at run-time.
>> + */
>> +bool avic_hardware_setup(struct kvm_x86_ops *x86_ops)
>> +{
>> +	if (!npt_enabled)
>> +		return false;
>> +
>> +	if (boot_cpu_has(X86_FEATURE_AVIC)) {
>> +		avic_mode = AVIC_MODE_X1;
>> +		pr_info("AVIC enabled\n");
>> +	}
>> +
>> +	if (boot_cpu_has(X86_FEATURE_X2AVIC)) {
>> +		avic_mode = AVIC_MODE_X2;
>> +		pr_info("x2AVIC enabled\n");
>> +	}
>> +
>> +	amd_iommu_register_ga_log_notifier(&avic_ga_log_notifier);
> If AVIC is not enabled, I guess no need to register GA log notifier?

GA log is only used when AVIC is enabled. I'll restore the AVIC-enabled check.

Regards,
Suravee