linux-kernel - Re: [PATCH hyperv-next v3] x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <50dd72d6-6e12-4b0e-954d-5b7e57fd02fb@linux.microsoft.com>
Date: Wed, 30 Apr 2025 08:15:17 -0700
From: Roman Kisel <romank@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.com>
Cc: "apais@...rosoft.com" <apais@...rosoft.com>,
 "benhill@...rosoft.com" <benhill@...rosoft.com>,
 "bperkins@...rosoft.com" <bperkins@...rosoft.com>,
 "sunilmut@...rosoft.com" <sunilmut@...rosoft.com>,
 "bp@...en8.de" <bp@...en8.de>,
 "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
 "decui@...rosoft.com" <decui@...rosoft.com>,
 "haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
 "hpa@...or.com" <hpa@...or.com>, "kys@...rosoft.com" <kys@...rosoft.com>,
 "mikelley@...rosoft.com" <mikelley@...rosoft.com>,
 "mingo@...hat.com" <mingo@...hat.com>,
 "tglx@...utronix.de" <tglx@...utronix.de>,
 "tiala@...rosoft.com" <tiala@...rosoft.com>,
 "wei.liu@...nel.org" <wei.liu@...nel.org>,
 "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH hyperv-next v3] x86/hyperv: Fix APIC ID and VP index
 confusion in hv_snp_boot_ap()



On 4/29/2025 8:42 PM, Michael Kelley wrote:
> From: Roman Kisel <romank@...ux.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@...r.kernel.org
>> Fixes: 44676bb9d566 ("x86/hyperv: Add smp support for SEV-SNP guest")
>> Signed-off-by: Roman Kisel <romank@...ux.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@...look.com>

-- 
Thank you,
Roman