Re: [PATCH hyperv-next v3] x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap()

[Roman Kisel <romank@linux.microsoft.com>]

On 4/29/2025 8:42 PM, Michael Kelley wrote:
> From: Roman Kisel <romank@linux.microsoft.com> Sent: Monday, April 28, 2025 11:27 AM
>>
>> To start an application processor in SNP-isolated guest, a hypercall
>> is used that takes a virtual processor index. The hv_snp_boot_ap()
>> function uses that START_VP hypercall but passes as VP index to it
>> what it receives as a wakeup_secondary_cpu_64 callback: the APIC ID.
>>
>> As those two aren't generally interchangeable, that may lead to hung
>> APs if the VP index and the APIC ID don't match up.
>>
>> Update the parameter names to avoid confusion as to what the parameter
>> is. Use the APIC ID to the VP index conversion to provide the correct
>> input to the hypercall.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 44676bb9d566 ("x86/hyperv: Add smp support for SEV-SNP guest")
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>> ---
>> [V3]
>> 	- Removed the misleading comment about the APIC ID and VP indices.
>> 	- Removed the not sufficiently founded if statement that was added
>> 	  to the previous version of the patch to avoid the O(n) time complexity.
>> 	  I'll follow up with a separate patch to address that as that pattern
>> 	  has crept into other places in the code in the AP wakeup path.
>> 	- Fixed the logging message to use the "VP index" terminology
>> 	  consistently.
>>      ** Thank you, Michael! **
>>
>> [V2]
>> 	https://lore.kernel.org/linux-hyperv/20250425213512.1837061-1-romank@linux.microsoft.com/
>>      - Fixed the terminology in the patch and other code to use
>>        the term "VP index" consistently
>>      ** Thank you, Michael! **
>>
>>      - Missed not enabling the SNP-SEV options in the local testing,
>>        and sent a patch that breaks the build.
>>      ** Thank you, Saurabh! **
>>
>>      - Added comments and getting the Linux kernel CPU number from
>>        the available data.
>>
>> [V1]
>>      https://lore.kernel.org/linux-hyperv/20250424215746.467281-1-romank@linux.microsoft.com/
>> ---
>>   arch/x86/hyperv/hv_init.c       | 33 +++++++++++++++++++++++++
>>   arch/x86/hyperv/hv_vtl.c        | 44 +++++----------------------------
>>   arch/x86/hyperv/ivm.c           | 22 +++++++++++++++--
>>   arch/x86/include/asm/mshyperv.h |  6 +++--
>>   include/hyperv/hvgdk_mini.h     |  2 +-
>>   5 files changed, 64 insertions(+), 43 deletions(-)
> 
> Thanks for fixing the terminology, in addition to fixing the bug that is your original
> purpose for the patch.

Thanks for your support and feedback :)

> 
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>

-- 
Thank you,
Roman