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: <9f3ffe16-2516-d4ec-528e-6347ef884ad5@amd.com>
Date:   Tue, 28 Jun 2022 09:35:28 +0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH v6 15/17] KVM: SVM: Use target APIC ID to complete x2AVIC
 IRQs when possible
Content-Language: en-US
To:     Maxim Levitsky <mlevitsk@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc:     seanjc@google.com, joro@8bytes.org, jon.grimm@amd.com,
        wei.huang2@amd.com, terry.bowman@amd.com
References: <20220519102709.24125-1-suravee.suthikulpanit@amd.com>
 <20220519102709.24125-16-suravee.suthikulpanit@amd.com>
 <b8610296-6fb7-e110-900f-4616e1e39bb4@redhat.com>
 <d761ef283bc91002322f3cd66c124d329c25f04f.camel@redhat.com>
From:   "Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>
In-Reply-To: <d761ef283bc91002322f3cd66c124d329c25f04f.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?UndzQVJZOEZ1NDFiM0Mxai9ackxleHZhWWNYNkphU1VDUE5idUsxUUdFeVBm?=
 =?utf-8?B?UnRUVlpnREpXbGRFYnVVWE1uTldLQ25yQmhBangxdlQzWm5nTEMycm00emsr?=
 =?utf-8?B?MWxzQURIMlZoSjZydERFVDhESXNsNlM4OERPRFowZ0FqSStSSDNnSC9BaTBm?=
 =?utf-8?B?ZUpCeHNmTG9sUXJaZEZFQmkvNEYwa0NobkhtcmovQkFiZFhzaTMwSG5SZUpr?=
 =?utf-8?B?SkRqR2RiVWk3NGJOU3N6cXJ2dFVHOElTUDdUaFhHcG1KVUtWWU1mWjdCMDdX?=
 =?utf-8?B?YlBMN3p0V0U1ejcrTEp5OEZwMWl2Um1LV1lVZDZIbHM1NEt0SXYvN3RzdnZj?=
 =?utf-8?B?UHBKK2JQNnIybW1LNThtRFpVUmRyOHhPYUlmL0VKOHJOMTQyZGtSVWR2TElL?=
 =?utf-8?B?aHl1U2hFb2RnTGVIZ0xsSjdTVEtkaEZxTDZqNFhXc3FZTG5XNCtWSDBGTGtX?=
 =?utf-8?B?RlBLL2JhR3Z6N2ZscWZwZ2tXeEtrajNJTGhLQ2d4VDlIWk54Ulk1NzVpWVFE?=
 =?utf-8?B?eEFRUTNQVmtBWXhIOFJUZDRDYTlXdXAvWndRQWdBeThLN3RnRkZKUEV0YW0w?=
 =?utf-8?B?aDJEbHplV01rWFl5SUUrKzMyMS9DajAxMUZyaVdKMkkrZDBCZlFZbjBCOC8r?=
 =?utf-8?B?VlFVMzk3Nk1UN0VGVGltWHhTdEl3YWQzZ1M2ZHMwRkVETXYzSEx1OGZIdXlG?=
 =?utf-8?B?aGhWRW8zK1A2N0VDMzlCYjZJeUlnOUxWaS9iUkkvdVg1K1psckRoT29lYkpy?=
 =?utf-8?B?SHhYTERIMTcwcXpHeEhuSTFqbnZ1dFJiVVJrS2NZZitGWGVuNzREZCtpOW5S?=
 =?utf-8?B?bTJyOXdrbHV1c0ZVZ0c1RUpGRFc4SGk1cCt3QWRoYndkK1BqQ1lJamlqWTZ5?=
 =?utf-8?B?Y2pUTjlFYlJGZk53Um0zMUZJUW8yVzR5U0xCSFdzSkFtOWxvL0J5MnhQd1FF?=
 =?utf-8?B?TVJ2L21NcTh1R1B2cjkzbnNmNFcxVjZ3OHhrS1BKbFJleHp0S0RRRjM3OGwy?=
 =?utf-8?B?WXQ3Uktua1o0ckdXQXpZaWhiMnBMTXMzQ3FrYlBJOERKUFFJT1o1emNGbTdS?=
 =?utf-8?B?bzdmdjU1SzE2TFR2dkp1bE84Rlg3eVlsVi9ObjU5NFpjbVZGNmd3UHRaTFhj?=
 =?utf-8?B?V2VBUWpZWmhxc3pFQjVuTDZBVjM3Tkd5ZXJSZTY4dlErMHo0L0J3aUhva01r?=
 =?utf-8?B?WEwrcURkanBwT1diTVNQZFczK1dlQ2FTbk55b2NZSzY2M2hrSWtxUTRIMlJm?=
 =?utf-8?B?OHNCTjFJaDNWNGJoYVpjbitEeU9Gc0NzSVp1U0t3dHpnbTVuMWlSVUxjZ20v?=
 =?utf-8?B?N2FBdHRDM2g1VVpTRXEydkZneEFlQ1dMbm5BMXZzYUpEUDBwRW9KYk9CcDVa?=
 =?utf-8?B?Qkd3b0FUQVkzUlRDUnRISlFJOUhHZFZQK01PR3RrdDc5akk3UGdmSzBEQUFK?=
 =?utf-8?B?WlB5R2NaUXVzd3hGU2xiYzBIRzRhSUhLakVKUFhnWTZpR3BVaTlqeGh5dU1R?=
 =?utf-8?B?MURiRU9GeGU3UUhsRWFoK1hEaWtBMEl0QTY0ZjZEcGRiYzFZTHNDbHV0cGxJ?=
 =?utf-8?B?R0dRSG1LbEJMZ1NTRkd6ZFIzYzJuSi9XSVJYSmVhUzd2RlhMak4vYWpoY1BJ?=
 =?utf-8?B?dXZjWStoanFNZXJGOFNkcVZ1NFU3V05nd1UvSFpVTmlsWG9RQ2lTSWl4ZlRP?=
 =?utf-8?B?aURlekpuUERSK2xTRVRlbFYzNWE0YmRlQzVOM2VycjdJcGhpSlM5NFBoUEJL?=
 =?utf-8?B?ajNBTVF0Ylp6V2FqNm0xeVJ4RHhKeXJKUWFzSUNVNmpVdUg5MDN3bTBsT2lu?=
 =?utf-8?B?dUNSTndQMVZpZFd1WEpUSnJTeG1MM3liak1PSStlVWNwSE9aRE5rWkRxcHN2?=
 =?utf-8?B?SDRwaXdvNTZDbjR1eFdGbCtzdEdXakFmUlFQNHNrLzR6UFNJV3dhNnZqUUlq?=
 =?utf-8?B?YS9jUHRoOUFGT2xLQm90QUo5S3lnZVYwcUNkSTEwRDQ2MjljU3pTbU9IK2hD?=
 =?utf-8?B?cDZYMXRvQ1k3SVU1eUdIa1JTamlvT21lRU8wK2NkQjRvK2ZFSU5KSk80NUFT?=
 =?utf-8?B?Ty9MeGR6TjBNMXJRNXdGUFRodnBybzRVbUs0cmUrMytaR3p3MlJPcVR3S0dF?=
 =?utf-8?Q?qPuR+pzRf2Se9g9u1de3YYSfD?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3e83b3a7-a1db-4115-3ebe-08da58aee259
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5445.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jun 2022 02:35:37.4680
 (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: 8l5Qspjcp4nITbR4v9VTNnIQ8dNAz5HeJwLZlR38myEIPcyuKWT+k3Dsd/gU8iDFT9C/bAYUstZ10MwcRrjdRQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR12MB1703
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


On 6/28/2022 5:55 AM, Maxim Levitsky wrote:
> On Fri, 2022-06-24 at 18:41 +0200, Paolo Bonzini wrote:
>> On 5/19/22 12:27, Suravee Suthikulpanit wrote:
>>> +			 * If the x2APIC logical ID sub-field (i.e. icrh[15:0]) contains zero
>>> +			 * or more than 1 bits, we cannot match just one vcpu to kick for
>>> +			 * fast path.
>>> +			 */
>>> +			if (!first || (first != last))
>>> +				return -EINVAL;
>>> +
>>> +			apic = first - 1;
>>> +			if ((apic < 0) || (apic > 15) || (cluster >= 0xfffff))
>>> +				return -EINVAL;
>>
>> Neither of these is possible: first == 0 has been cheked above, and
>> ffs(icrh & 0xffff) cannot exceed 15.  Likewise, cluster is actually
>> limited to 16 bits, not 20.
>>
>> Plus, C is not Pascal so no parentheses. :)
>>
>> Putting everything together, it can be simplified to this:
>>
>> +                       int cluster = (icrh & 0xffff0000) >> 16;
>> +                       int apic = ffs(icrh & 0xffff) - 1;
>> +
>> +                       /*
>> +                        * If the x2APIC logical ID sub-field (i.e. icrh[15:0])
>> +                        * contains anything but a single bit, we cannot use the
>> +                        * fast path, because it is limited to a single vCPU.
>> +                        */
>> +                       if (apic < 0 || icrh != (1 << apic))
>> +                               return -EINVAL;
>> +
>> +                       l1_physical_id = (cluster << 4) + apic;
>>
>>
>>> +			apic_id = (cluster << 4) + apic;
> 
> Hi Paolo and Suravee Suthikulpanit!
> 
> Note that this patch is not needed anymore, I fixed the avic_kick_target_vcpus_fast function,
> and added the support for x2apic because it was very easy to do
> (I already needed to parse logical id for flat and cluser modes)
> 
> Best regards,
> 	Maxim Levitsky
> 

Understood. I was about to send v7 to remove this patch from the series, but too late. I'll test the current queue branch and provide update.

Best Regards,
Suravee