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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <SN6PR02MB4157F5E0375016711820D2FBD4832@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Wed, 30 Apr 2025 03:42:20 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Roman Kisel <romank@...ux.microsoft.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>
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>
Subject: RE: [PATCH hyperv-next v3] x86/hyperv: Fix APIC ID and VP index
 confusion in hv_snp_boot_ap()

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.

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