public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup
@ 2025-05-07 18:22 Roman Kisel
  2025-05-07 18:22 ` [PATCH hyperv-next 1/2] x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap() Roman Kisel
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Roman Kisel @ 2025-05-07 18:22 UTC (permalink / raw)
  To: ardb, bp, brgerst, dave.hansen, decui, dimitri.sivanich,
	gautham.shenoy, haiyangz, hpa, imran.f.khan, jacob.jun.pan,
	jpoimboe, justin.ernst, kprateek.nayak, kyle.meyer, kys, lenb,
	mhklinux, mingo, nikunj, papaluri, patryk.wlazlyn, peterz, rafael,
	romank, russ.anderson, sohil.mehta, steve.wahl, tglx,
	thomas.lendacky, tiala, wei.liu, yuehaibing, linux-acpi,
	linux-hyperv, linux-kernel, x86
  Cc: apais, benhill, bperkins, sunilmut

This patchset combines two patches that depend on each other and were not applying
cleanly:
  1. Fix APIC ID and VP index confusion in hv_snp_boot_ap():
    https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/
  2. Provide the CPU number in the wakeup AP callback:
    https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/

I rebased the patches on top of the latest hyperv-next tree and updated the second patch
that broke the linux-next build. That fix that, I made one non-functional change:
updated the signature of numachip_wakeup_secondary() to match the parameter list of
wakeup_secondary_cpu().

Roman Kisel (2):
  x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap()
  arch/x86: Provide the CPU number in the wakeup AP callback

 arch/x86/coco/sev/core.c             | 13 ++-----
 arch/x86/hyperv/hv_init.c            | 33 +++++++++++++++++
 arch/x86/hyperv/hv_vtl.c             | 54 ++++------------------------
 arch/x86/hyperv/ivm.c                | 11 ++++--
 arch/x86/include/asm/apic.h          |  8 ++---
 arch/x86/include/asm/mshyperv.h      |  7 ++--
 arch/x86/kernel/acpi/madt_wakeup.c   |  2 +-
 arch/x86/kernel/apic/apic_noop.c     |  8 ++++-
 arch/x86/kernel/apic/apic_numachip.c |  2 +-
 arch/x86/kernel/apic/x2apic_uv_x.c   |  2 +-
 arch/x86/kernel/smpboot.c            | 10 +++---
 include/hyperv/hvgdk_mini.h          |  2 +-
 12 files changed, 76 insertions(+), 76 deletions(-)


base-commit: 9b0844d87b1407681b78130429f798beb366f43f
-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH hyperv-next 1/2] x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap()
  2025-05-07 18:22 [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Roman Kisel
@ 2025-05-07 18:22 ` Roman Kisel
  2025-05-07 18:22 ` [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback Roman Kisel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Roman Kisel @ 2025-05-07 18:22 UTC (permalink / raw)
  To: ardb, bp, brgerst, dave.hansen, decui, dimitri.sivanich,
	gautham.shenoy, haiyangz, hpa, imran.f.khan, jacob.jun.pan,
	jpoimboe, justin.ernst, kprateek.nayak, kyle.meyer, kys, lenb,
	mhklinux, mingo, nikunj, papaluri, patryk.wlazlyn, peterz, rafael,
	romank, russ.anderson, sohil.mehta, steve.wahl, tglx,
	thomas.lendacky, tiala, wei.liu, yuehaibing, linux-acpi,
	linux-hyperv, linux-kernel, x86
  Cc: apais, benhill, bperkins, sunilmut

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>
Reviewed-by: Michael Kelley <mhklinux@outlook.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(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 3b569291dfed..9a8fc144e195 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -672,3 +672,36 @@ bool hv_is_hyperv_initialized(void)
 	return hypercall_msr.enable;
 }
 EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
+
+int hv_apicid_to_vp_index(u32 apic_id)
+{
+	u64 control;
+	u64 status;
+	unsigned long irq_flags;
+	struct hv_get_vp_from_apic_id_in *input;
+	u32 *output, ret;
+
+	local_irq_save(irq_flags);
+
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	memset(input, 0, sizeof(*input));
+	input->partition_id = HV_PARTITION_ID_SELF;
+	input->apic_ids[0] = apic_id;
+
+	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+
+	control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_INDEX_FROM_APIC_ID;
+	status = hv_do_hypercall(control, input, output);
+	ret = output[0];
+
+	local_irq_restore(irq_flags);
+
+	if (!hv_result_success(status)) {
+		pr_err("failed to get vp index from apic id %d, status %#llx\n",
+		       apic_id, status);
+		return -EINVAL;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(hv_apicid_to_vp_index);
diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index ac3d27a766d5..2f32ac1ae40e 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -211,41 +211,9 @@ static int hv_vtl_bringup_vcpu(u32 target_vp_index, int cpu, u64 eip_ignored)
 	return ret;
 }
 
-static int hv_vtl_apicid_to_vp_id(u32 apic_id)
-{
-	u64 control;
-	u64 status;
-	unsigned long irq_flags;
-	struct hv_get_vp_from_apic_id_in *input;
-	u32 *output, ret;
-
-	local_irq_save(irq_flags);
-
-	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
-	memset(input, 0, sizeof(*input));
-	input->partition_id = HV_PARTITION_ID_SELF;
-	input->apic_ids[0] = apic_id;
-
-	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
-
-	control = HV_HYPERCALL_REP_COMP_1 | HVCALL_GET_VP_ID_FROM_APIC_ID;
-	status = hv_do_hypercall(control, input, output);
-	ret = output[0];
-
-	local_irq_restore(irq_flags);
-
-	if (!hv_result_success(status)) {
-		pr_err("failed to get vp id from apic id %d, status %#llx\n",
-		       apic_id, status);
-		return -EINVAL;
-	}
-
-	return ret;
-}
-
 static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip)
 {
-	int vp_id, cpu;
+	int vp_index, cpu;
 
 	/* Find the logical CPU for the APIC ID */
 	for_each_present_cpu(cpu) {
@@ -256,18 +224,18 @@ static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip)
 		return -EINVAL;
 
 	pr_debug("Bringing up CPU with APIC ID %d in VTL2...\n", apicid);
-	vp_id = hv_vtl_apicid_to_vp_id(apicid);
+	vp_index = hv_apicid_to_vp_index(apicid);
 
-	if (vp_id < 0) {
+	if (vp_index < 0) {
 		pr_err("Couldn't find CPU with APIC ID %d\n", apicid);
 		return -EINVAL;
 	}
-	if (vp_id > ms_hyperv.max_vp_index) {
-		pr_err("Invalid CPU id %d for APIC ID %d\n", vp_id, apicid);
+	if (vp_index > ms_hyperv.max_vp_index) {
+		pr_err("Invalid CPU id %d for APIC ID %d\n", vp_index, apicid);
 		return -EINVAL;
 	}
 
-	return hv_vtl_bringup_vcpu(vp_id, cpu, start_eip);
+	return hv_vtl_bringup_vcpu(vp_index, cpu, start_eip);
 }
 
 int __init hv_vtl_early_init(void)
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 77bf05f06b9e..0cc239cdb4da 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -9,6 +9,7 @@
 #include <linux/bitfield.h>
 #include <linux/types.h>
 #include <linux/slab.h>
+#include <linux/cpu.h>
 #include <asm/svm.h>
 #include <asm/sev.h>
 #include <asm/io.h>
@@ -288,7 +289,7 @@ static void snp_cleanup_vmsa(struct sev_es_save_area *vmsa)
 		free_page((unsigned long)vmsa);
 }
 
-int hv_snp_boot_ap(u32 cpu, unsigned long start_ip)
+int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
 {
 	struct sev_es_save_area *vmsa = (struct sev_es_save_area *)
 		__get_free_page(GFP_KERNEL | __GFP_ZERO);
@@ -297,10 +298,27 @@ int hv_snp_boot_ap(u32 cpu, unsigned long start_ip)
 	u64 ret, retry = 5;
 	struct hv_enable_vp_vtl *start_vp_input;
 	unsigned long flags;
+	int cpu, vp_index;
 
 	if (!vmsa)
 		return -ENOMEM;
 
+	/* Find the Hyper-V VP index which might be not the same as APIC ID */
+	vp_index = hv_apicid_to_vp_index(apic_id);
+	if (vp_index < 0 || vp_index > ms_hyperv.max_vp_index)
+		return -EINVAL;
+
+	/*
+	 * Find the Linux CPU number for addressing the per-CPU data, and it
+	 * might not be the same as APIC ID.
+	 */
+	for_each_present_cpu(cpu) {
+		if (arch_match_cpu_phys_id(cpu, apic_id))
+			break;
+	}
+	if (cpu >= nr_cpu_ids)
+		return -EINVAL;
+
 	native_store_gdt(&gdtr);
 
 	vmsa->gdtr.base = gdtr.address;
@@ -348,7 +366,7 @@ int hv_snp_boot_ap(u32 cpu, unsigned long start_ip)
 	start_vp_input = (struct hv_enable_vp_vtl *)ap_start_input_arg;
 	memset(start_vp_input, 0, sizeof(*start_vp_input));
 	start_vp_input->partition_id = -1;
-	start_vp_input->vp_index = cpu;
+	start_vp_input->vp_index = vp_index;
 	start_vp_input->target_vtl.target_vtl = ms_hyperv.vtl;
 	*(u64 *)&start_vp_input->vp_context = __pa(vmsa) | 1;
 
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index bab5ccfc60a7..0b9a3a307d06 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -268,11 +268,11 @@ int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry);
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 bool hv_ghcb_negotiate_protocol(void);
 void __noreturn hv_ghcb_terminate(unsigned int set, unsigned int reason);
-int hv_snp_boot_ap(u32 cpu, unsigned long start_ip);
+int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip);
 #else
 static inline bool hv_ghcb_negotiate_protocol(void) { return false; }
 static inline void hv_ghcb_terminate(unsigned int set, unsigned int reason) {}
-static inline int hv_snp_boot_ap(u32 cpu, unsigned long start_ip) { return 0; }
+static inline int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip) { return 0; }
 #endif
 
 #if defined(CONFIG_AMD_MEM_ENCRYPT) || defined(CONFIG_INTEL_TDX_GUEST)
@@ -306,6 +306,7 @@ static __always_inline u64 hv_raw_get_msr(unsigned int reg)
 {
 	return __rdmsr(reg);
 }
+int hv_apicid_to_vp_index(u32 apic_id);
 
 #else /* CONFIG_HYPERV */
 static inline void hyperv_init(void) {}
@@ -327,6 +328,7 @@ static inline void hv_set_msr(unsigned int reg, u64 value) { }
 static inline u64 hv_get_msr(unsigned int reg) { return 0; }
 static inline void hv_set_non_nested_msr(unsigned int reg, u64 value) { }
 static inline u64 hv_get_non_nested_msr(unsigned int reg) { return 0; }
+static inline int hv_apicid_to_vp_index(u32 apic_id) { return -EINVAL; }
 #endif /* CONFIG_HYPERV */
 
 
diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index cf0923dc727d..2d431b53f587 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -475,7 +475,7 @@ union hv_vp_assist_msr_contents {	 /* HV_REGISTER_VP_ASSIST_PAGE */
 #define HVCALL_CREATE_PORT				0x0095
 #define HVCALL_CONNECT_PORT				0x0096
 #define HVCALL_START_VP					0x0099
-#define HVCALL_GET_VP_ID_FROM_APIC_ID			0x009a
+#define HVCALL_GET_VP_INDEX_FROM_APIC_ID			0x009a
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE	0x00af
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST	0x00b0
 #define HVCALL_SIGNAL_EVENT_DIRECT			0x00c0
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback
  2025-05-07 18:22 [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Roman Kisel
  2025-05-07 18:22 ` [PATCH hyperv-next 1/2] x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap() Roman Kisel
@ 2025-05-07 18:22 ` Roman Kisel
  2025-05-07 20:09   ` Rafael J. Wysocki
  2025-05-08  4:22 ` [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Michael Kelley
  2025-05-08 18:39 ` Wei Liu
  3 siblings, 1 reply; 9+ messages in thread
From: Roman Kisel @ 2025-05-07 18:22 UTC (permalink / raw)
  To: ardb, bp, brgerst, dave.hansen, decui, dimitri.sivanich,
	gautham.shenoy, haiyangz, hpa, imran.f.khan, jacob.jun.pan,
	jpoimboe, justin.ernst, kprateek.nayak, kyle.meyer, kys, lenb,
	mhklinux, mingo, nikunj, papaluri, patryk.wlazlyn, peterz, rafael,
	romank, russ.anderson, sohil.mehta, steve.wahl, tglx,
	thomas.lendacky, tiala, wei.liu, yuehaibing, linux-acpi,
	linux-hyperv, linux-kernel, x86
  Cc: apais, benhill, bperkins, sunilmut

When starting APs, confidential guests and paravisor guests
need to know the CPU number, and the pattern of using the linear
search has emerged in several places. With N processors that leads
to the O(N^2) time complexity.

Provide the CPU number in the AP wake up callback so that one can
get the CPU number in constant time.

Suggested-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
 arch/x86/coco/sev/core.c             | 13 ++-----------
 arch/x86/hyperv/hv_vtl.c             | 12 ++----------
 arch/x86/hyperv/ivm.c                | 15 ++-------------
 arch/x86/include/asm/apic.h          |  8 ++++----
 arch/x86/include/asm/mshyperv.h      |  5 +++--
 arch/x86/kernel/acpi/madt_wakeup.c   |  2 +-
 arch/x86/kernel/apic/apic_noop.c     |  8 +++++++-
 arch/x86/kernel/apic/apic_numachip.c |  2 +-
 arch/x86/kernel/apic/x2apic_uv_x.c   |  2 +-
 arch/x86/kernel/smpboot.c            | 10 +++++-----
 10 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index b0c1a7a57497..7780d55d1833 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -1177,7 +1177,7 @@ static void snp_cleanup_vmsa(struct sev_es_save_area *vmsa, int apic_id)
 		free_page((unsigned long)vmsa);
 }
 
-static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
+static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip, unsigned int cpu)
 {
 	struct sev_es_save_area *cur_vmsa, *vmsa;
 	struct ghcb_state state;
@@ -1185,7 +1185,7 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
 	unsigned long flags;
 	struct ghcb *ghcb;
 	u8 sipi_vector;
-	int cpu, ret;
+	int ret;
 	u64 cr4;
 
 	/*
@@ -1206,15 +1206,6 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
 
 	/* Override start_ip with known protected guest start IP */
 	start_ip = real_mode_header->sev_es_trampoline_start;
-
-	/* Find the logical CPU for the APIC ID */
-	for_each_present_cpu(cpu) {
-		if (arch_match_cpu_phys_id(cpu, apic_id))
-			break;
-	}
-	if (cpu >= nr_cpu_ids)
-		return -EINVAL;
-
 	cur_vmsa = per_cpu(sev_vmsa, cpu);
 
 	/*
diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 2f32ac1ae40e..3d149a2ca4c8 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -211,17 +211,9 @@ static int hv_vtl_bringup_vcpu(u32 target_vp_index, int cpu, u64 eip_ignored)
 	return ret;
 }
 
-static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip)
+static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip, unsigned int cpu)
 {
-	int vp_index, cpu;
-
-	/* Find the logical CPU for the APIC ID */
-	for_each_present_cpu(cpu) {
-		if (arch_match_cpu_phys_id(cpu, apicid))
-			break;
-	}
-	if (cpu >= nr_cpu_ids)
-		return -EINVAL;
+	int vp_index;
 
 	pr_debug("Bringing up CPU with APIC ID %d in VTL2...\n", apicid);
 	vp_index = hv_apicid_to_vp_index(apicid);
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 0cc239cdb4da..e21557b24d19 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -289,7 +289,7 @@ static void snp_cleanup_vmsa(struct sev_es_save_area *vmsa)
 		free_page((unsigned long)vmsa);
 }
 
-int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
+int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip, unsigned int cpu)
 {
 	struct sev_es_save_area *vmsa = (struct sev_es_save_area *)
 		__get_free_page(GFP_KERNEL | __GFP_ZERO);
@@ -298,7 +298,7 @@ int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
 	u64 ret, retry = 5;
 	struct hv_enable_vp_vtl *start_vp_input;
 	unsigned long flags;
-	int cpu, vp_index;
+	int vp_index;
 
 	if (!vmsa)
 		return -ENOMEM;
@@ -308,17 +308,6 @@ int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
 	if (vp_index < 0 || vp_index > ms_hyperv.max_vp_index)
 		return -EINVAL;
 
-	/*
-	 * Find the Linux CPU number for addressing the per-CPU data, and it
-	 * might not be the same as APIC ID.
-	 */
-	for_each_present_cpu(cpu) {
-		if (arch_match_cpu_phys_id(cpu, apic_id))
-			break;
-	}
-	if (cpu >= nr_cpu_ids)
-		return -EINVAL;
-
 	native_store_gdt(&gdtr);
 
 	vmsa->gdtr.base = gdtr.address;
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index c903d358405d..eaf43d446203 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -313,9 +313,9 @@ struct apic {
 	u32	(*get_apic_id)(u32 id);
 
 	/* wakeup_secondary_cpu */
-	int	(*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip);
+	int	(*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip, unsigned int cpu);
 	/* wakeup secondary CPU using 64-bit wakeup point */
-	int	(*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip);
+	int	(*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip, unsigned int cpu);
 
 	char	*name;
 };
@@ -333,8 +333,8 @@ struct apic_override {
 	void	(*send_IPI_self)(int vector);
 	u64	(*icr_read)(void);
 	void	(*icr_write)(u32 low, u32 high);
-	int	(*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip);
-	int	(*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip);
+	int	(*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip, unsigned int cpu);
+	int	(*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip, unsigned int cpu);
 };
 
 /*
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 0b9a3a307d06..5ec92e3e2e37 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -268,11 +268,12 @@ int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry);
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 bool hv_ghcb_negotiate_protocol(void);
 void __noreturn hv_ghcb_terminate(unsigned int set, unsigned int reason);
-int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip);
+int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip, unsigned int cpu);
 #else
 static inline bool hv_ghcb_negotiate_protocol(void) { return false; }
 static inline void hv_ghcb_terminate(unsigned int set, unsigned int reason) {}
-static inline int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip) { return 0; }
+static inline int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip,
+		unsigned int cpu) { return 0; }
 #endif
 
 #if defined(CONFIG_AMD_MEM_ENCRYPT) || defined(CONFIG_INTEL_TDX_GUEST)
diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
index f36f28405dcc..6d7603511f52 100644
--- a/arch/x86/kernel/acpi/madt_wakeup.c
+++ b/arch/x86/kernel/acpi/madt_wakeup.c
@@ -126,7 +126,7 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
 	return 0;
 }
 
-static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
+static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip, unsigned int cpu)
 {
 	if (!acpi_mp_wake_mailbox_paddr) {
 		pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs. Booting with kexec?\n");
diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
index b5bb7a2e8340..58abb941c45b 100644
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -27,7 +27,13 @@ static void noop_send_IPI_allbutself(int vector) { }
 static void noop_send_IPI_all(int vector) { }
 static void noop_send_IPI_self(int vector) { }
 static void noop_apic_icr_write(u32 low, u32 id) { }
-static int noop_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip) { return -1; }
+
+static int noop_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip,
+	unsigned int cpu)
+{
+	return -1;
+}
+
 static u64 noop_apic_icr_read(void) { return 0; }
 static u32 noop_get_apic_id(u32 apicid) { return 0; }
 static void noop_apic_eoi(void) { }
diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
index 16410f087b7a..333536b89bde 100644
--- a/arch/x86/kernel/apic/apic_numachip.c
+++ b/arch/x86/kernel/apic/apic_numachip.c
@@ -56,7 +56,7 @@ static void numachip2_apic_icr_write(int apicid, unsigned int val)
 	numachip2_write32_lcsr(NUMACHIP2_APIC_ICR, (apicid << 12) | val);
 }
 
-static int numachip_wakeup_secondary(u32 phys_apicid, unsigned long start_rip)
+static int numachip_wakeup_secondary(u32 phys_apicid, unsigned long start_rip, unsigned int cpu)
 {
 	numachip_apic_icr_write(phys_apicid, APIC_DM_INIT);
 	numachip_apic_icr_write(phys_apicid, APIC_DM_STARTUP |
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 7fef504ca508..15209f220e1f 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -667,7 +667,7 @@ static __init void build_uv_gr_table(void)
 	}
 }
 
-static int uv_wakeup_secondary(u32 phys_apicid, unsigned long start_rip)
+static int uv_wakeup_secondary(u32 phys_apicid, unsigned long start_rip, unsigned int cpu)
 {
 	unsigned long val;
 	int pnode;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index d6cf1e23c2a3..d52e9238e9fd 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -695,7 +695,7 @@ static void send_init_sequence(u32 phys_apicid)
 /*
  * Wake up AP by INIT, INIT, STARTUP sequence.
  */
-static int wakeup_secondary_cpu_via_init(u32 phys_apicid, unsigned long start_eip)
+static int wakeup_secondary_cpu_via_init(u32 phys_apicid, unsigned long start_eip, unsigned int cpu)
 {
 	unsigned long send_status = 0, accept_status = 0;
 	int num_starts, j, maxlvt;
@@ -842,7 +842,7 @@ int common_cpu_up(unsigned int cpu, struct task_struct *idle)
  * Returns zero if startup was successfully sent, else error code from
  * ->wakeup_secondary_cpu.
  */
-static int do_boot_cpu(u32 apicid, int cpu, struct task_struct *idle)
+static int do_boot_cpu(u32 apicid, unsigned int cpu, struct task_struct *idle)
 {
 	unsigned long start_ip = real_mode_header->trampoline_start;
 	int ret;
@@ -896,11 +896,11 @@ static int do_boot_cpu(u32 apicid, int cpu, struct task_struct *idle)
 	 * - Use an INIT boot APIC message
 	 */
 	if (apic->wakeup_secondary_cpu_64)
-		ret = apic->wakeup_secondary_cpu_64(apicid, start_ip);
+		ret = apic->wakeup_secondary_cpu_64(apicid, start_ip, cpu);
 	else if (apic->wakeup_secondary_cpu)
-		ret = apic->wakeup_secondary_cpu(apicid, start_ip);
+		ret = apic->wakeup_secondary_cpu(apicid, start_ip, cpu);
 	else
-		ret = wakeup_secondary_cpu_via_init(apicid, start_ip);
+		ret = wakeup_secondary_cpu_via_init(apicid, start_ip, cpu);
 
 	/* If the wakeup mechanism failed, cleanup the warm reset vector */
 	if (ret)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback
  2025-05-07 18:22 ` [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback Roman Kisel
@ 2025-05-07 20:09   ` Rafael J. Wysocki
  2025-05-08  1:10     ` Wei Liu
  0 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2025-05-07 20:09 UTC (permalink / raw)
  To: Roman Kisel
  Cc: ardb, bp, brgerst, dave.hansen, decui, dimitri.sivanich,
	gautham.shenoy, haiyangz, hpa, imran.f.khan, jacob.jun.pan,
	jpoimboe, justin.ernst, kprateek.nayak, kyle.meyer, kys, lenb,
	mhklinux, mingo, nikunj, papaluri, patryk.wlazlyn, peterz, rafael,
	russ.anderson, sohil.mehta, steve.wahl, tglx, thomas.lendacky,
	tiala, wei.liu, yuehaibing, linux-acpi, linux-hyperv,
	linux-kernel, x86, apais, benhill, bperkins, sunilmut

On Wed, May 7, 2025 at 8:22 PM Roman Kisel <romank@linux.microsoft.com> wrote:
>
> When starting APs, confidential guests and paravisor guests
> need to know the CPU number, and the pattern of using the linear
> search has emerged in several places. With N processors that leads
> to the O(N^2) time complexity.
>
> Provide the CPU number in the AP wake up callback so that one can
> get the CPU number in constant time.
>
> Suggested-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>

For the ACPI bits

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

and I'm assuming that the x86 folks will take care of this.

Thanks!

> ---
>  arch/x86/coco/sev/core.c             | 13 ++-----------
>  arch/x86/hyperv/hv_vtl.c             | 12 ++----------
>  arch/x86/hyperv/ivm.c                | 15 ++-------------
>  arch/x86/include/asm/apic.h          |  8 ++++----
>  arch/x86/include/asm/mshyperv.h      |  5 +++--
>  arch/x86/kernel/acpi/madt_wakeup.c   |  2 +-
>  arch/x86/kernel/apic/apic_noop.c     |  8 +++++++-
>  arch/x86/kernel/apic/apic_numachip.c |  2 +-
>  arch/x86/kernel/apic/x2apic_uv_x.c   |  2 +-
>  arch/x86/kernel/smpboot.c            | 10 +++++-----
>  10 files changed, 28 insertions(+), 49 deletions(-)
>
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index b0c1a7a57497..7780d55d1833 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -1177,7 +1177,7 @@ static void snp_cleanup_vmsa(struct sev_es_save_area *vmsa, int apic_id)
>                 free_page((unsigned long)vmsa);
>  }
>
> -static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
> +static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip, unsigned int cpu)
>  {
>         struct sev_es_save_area *cur_vmsa, *vmsa;
>         struct ghcb_state state;
> @@ -1185,7 +1185,7 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
>         unsigned long flags;
>         struct ghcb *ghcb;
>         u8 sipi_vector;
> -       int cpu, ret;
> +       int ret;
>         u64 cr4;
>
>         /*
> @@ -1206,15 +1206,6 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
>
>         /* Override start_ip with known protected guest start IP */
>         start_ip = real_mode_header->sev_es_trampoline_start;
> -
> -       /* Find the logical CPU for the APIC ID */
> -       for_each_present_cpu(cpu) {
> -               if (arch_match_cpu_phys_id(cpu, apic_id))
> -                       break;
> -       }
> -       if (cpu >= nr_cpu_ids)
> -               return -EINVAL;
> -
>         cur_vmsa = per_cpu(sev_vmsa, cpu);
>
>         /*
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 2f32ac1ae40e..3d149a2ca4c8 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -211,17 +211,9 @@ static int hv_vtl_bringup_vcpu(u32 target_vp_index, int cpu, u64 eip_ignored)
>         return ret;
>  }
>
> -static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip)
> +static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip, unsigned int cpu)
>  {
> -       int vp_index, cpu;
> -
> -       /* Find the logical CPU for the APIC ID */
> -       for_each_present_cpu(cpu) {
> -               if (arch_match_cpu_phys_id(cpu, apicid))
> -                       break;
> -       }
> -       if (cpu >= nr_cpu_ids)
> -               return -EINVAL;
> +       int vp_index;
>
>         pr_debug("Bringing up CPU with APIC ID %d in VTL2...\n", apicid);
>         vp_index = hv_apicid_to_vp_index(apicid);
> diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
> index 0cc239cdb4da..e21557b24d19 100644
> --- a/arch/x86/hyperv/ivm.c
> +++ b/arch/x86/hyperv/ivm.c
> @@ -289,7 +289,7 @@ static void snp_cleanup_vmsa(struct sev_es_save_area *vmsa)
>                 free_page((unsigned long)vmsa);
>  }
>
> -int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
> +int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip, unsigned int cpu)
>  {
>         struct sev_es_save_area *vmsa = (struct sev_es_save_area *)
>                 __get_free_page(GFP_KERNEL | __GFP_ZERO);
> @@ -298,7 +298,7 @@ int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
>         u64 ret, retry = 5;
>         struct hv_enable_vp_vtl *start_vp_input;
>         unsigned long flags;
> -       int cpu, vp_index;
> +       int vp_index;
>
>         if (!vmsa)
>                 return -ENOMEM;
> @@ -308,17 +308,6 @@ int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
>         if (vp_index < 0 || vp_index > ms_hyperv.max_vp_index)
>                 return -EINVAL;
>
> -       /*
> -        * Find the Linux CPU number for addressing the per-CPU data, and it
> -        * might not be the same as APIC ID.
> -        */
> -       for_each_present_cpu(cpu) {
> -               if (arch_match_cpu_phys_id(cpu, apic_id))
> -                       break;
> -       }
> -       if (cpu >= nr_cpu_ids)
> -               return -EINVAL;
> -
>         native_store_gdt(&gdtr);
>
>         vmsa->gdtr.base = gdtr.address;
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index c903d358405d..eaf43d446203 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -313,9 +313,9 @@ struct apic {
>         u32     (*get_apic_id)(u32 id);
>
>         /* wakeup_secondary_cpu */
> -       int     (*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip);
> +       int     (*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip, unsigned int cpu);
>         /* wakeup secondary CPU using 64-bit wakeup point */
> -       int     (*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip);
> +       int     (*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip, unsigned int cpu);
>
>         char    *name;
>  };
> @@ -333,8 +333,8 @@ struct apic_override {
>         void    (*send_IPI_self)(int vector);
>         u64     (*icr_read)(void);
>         void    (*icr_write)(u32 low, u32 high);
> -       int     (*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip);
> -       int     (*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip);
> +       int     (*wakeup_secondary_cpu)(u32 apicid, unsigned long start_eip, unsigned int cpu);
> +       int     (*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip, unsigned int cpu);
>  };
>
>  /*
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 0b9a3a307d06..5ec92e3e2e37 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -268,11 +268,12 @@ int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry);
>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>  bool hv_ghcb_negotiate_protocol(void);
>  void __noreturn hv_ghcb_terminate(unsigned int set, unsigned int reason);
> -int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip);
> +int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip, unsigned int cpu);
>  #else
>  static inline bool hv_ghcb_negotiate_protocol(void) { return false; }
>  static inline void hv_ghcb_terminate(unsigned int set, unsigned int reason) {}
> -static inline int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip) { return 0; }
> +static inline int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip,
> +               unsigned int cpu) { return 0; }
>  #endif
>
>  #if defined(CONFIG_AMD_MEM_ENCRYPT) || defined(CONFIG_INTEL_TDX_GUEST)
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
> index f36f28405dcc..6d7603511f52 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -126,7 +126,7 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
>         return 0;
>  }
>
> -static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> +static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip, unsigned int cpu)
>  {
>         if (!acpi_mp_wake_mailbox_paddr) {
>                 pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs. Booting with kexec?\n");
> diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
> index b5bb7a2e8340..58abb941c45b 100644
> --- a/arch/x86/kernel/apic/apic_noop.c
> +++ b/arch/x86/kernel/apic/apic_noop.c
> @@ -27,7 +27,13 @@ static void noop_send_IPI_allbutself(int vector) { }
>  static void noop_send_IPI_all(int vector) { }
>  static void noop_send_IPI_self(int vector) { }
>  static void noop_apic_icr_write(u32 low, u32 id) { }
> -static int noop_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip) { return -1; }
> +
> +static int noop_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip,
> +       unsigned int cpu)
> +{
> +       return -1;
> +}
> +
>  static u64 noop_apic_icr_read(void) { return 0; }
>  static u32 noop_get_apic_id(u32 apicid) { return 0; }
>  static void noop_apic_eoi(void) { }
> diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
> index 16410f087b7a..333536b89bde 100644
> --- a/arch/x86/kernel/apic/apic_numachip.c
> +++ b/arch/x86/kernel/apic/apic_numachip.c
> @@ -56,7 +56,7 @@ static void numachip2_apic_icr_write(int apicid, unsigned int val)
>         numachip2_write32_lcsr(NUMACHIP2_APIC_ICR, (apicid << 12) | val);
>  }
>
> -static int numachip_wakeup_secondary(u32 phys_apicid, unsigned long start_rip)
> +static int numachip_wakeup_secondary(u32 phys_apicid, unsigned long start_rip, unsigned int cpu)
>  {
>         numachip_apic_icr_write(phys_apicid, APIC_DM_INIT);
>         numachip_apic_icr_write(phys_apicid, APIC_DM_STARTUP |
> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
> index 7fef504ca508..15209f220e1f 100644
> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -667,7 +667,7 @@ static __init void build_uv_gr_table(void)
>         }
>  }
>
> -static int uv_wakeup_secondary(u32 phys_apicid, unsigned long start_rip)
> +static int uv_wakeup_secondary(u32 phys_apicid, unsigned long start_rip, unsigned int cpu)
>  {
>         unsigned long val;
>         int pnode;
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index d6cf1e23c2a3..d52e9238e9fd 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -695,7 +695,7 @@ static void send_init_sequence(u32 phys_apicid)
>  /*
>   * Wake up AP by INIT, INIT, STARTUP sequence.
>   */
> -static int wakeup_secondary_cpu_via_init(u32 phys_apicid, unsigned long start_eip)
> +static int wakeup_secondary_cpu_via_init(u32 phys_apicid, unsigned long start_eip, unsigned int cpu)
>  {
>         unsigned long send_status = 0, accept_status = 0;
>         int num_starts, j, maxlvt;
> @@ -842,7 +842,7 @@ int common_cpu_up(unsigned int cpu, struct task_struct *idle)
>   * Returns zero if startup was successfully sent, else error code from
>   * ->wakeup_secondary_cpu.
>   */
> -static int do_boot_cpu(u32 apicid, int cpu, struct task_struct *idle)
> +static int do_boot_cpu(u32 apicid, unsigned int cpu, struct task_struct *idle)
>  {
>         unsigned long start_ip = real_mode_header->trampoline_start;
>         int ret;
> @@ -896,11 +896,11 @@ static int do_boot_cpu(u32 apicid, int cpu, struct task_struct *idle)
>          * - Use an INIT boot APIC message
>          */
>         if (apic->wakeup_secondary_cpu_64)
> -               ret = apic->wakeup_secondary_cpu_64(apicid, start_ip);
> +               ret = apic->wakeup_secondary_cpu_64(apicid, start_ip, cpu);
>         else if (apic->wakeup_secondary_cpu)
> -               ret = apic->wakeup_secondary_cpu(apicid, start_ip);
> +               ret = apic->wakeup_secondary_cpu(apicid, start_ip, cpu);
>         else
> -               ret = wakeup_secondary_cpu_via_init(apicid, start_ip);
> +               ret = wakeup_secondary_cpu_via_init(apicid, start_ip, cpu);
>
>         /* If the wakeup mechanism failed, cleanup the warm reset vector */
>         if (ret)
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback
  2025-05-07 20:09   ` Rafael J. Wysocki
@ 2025-05-08  1:10     ` Wei Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Wei Liu @ 2025-05-08  1:10 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Roman Kisel, ardb, bp, brgerst, dave.hansen, decui,
	dimitri.sivanich, gautham.shenoy, haiyangz, hpa, imran.f.khan,
	jacob.jun.pan, jpoimboe, justin.ernst, kprateek.nayak, kyle.meyer,
	kys, lenb, mhklinux, mingo, nikunj, papaluri, patryk.wlazlyn,
	peterz, russ.anderson, sohil.mehta, steve.wahl, tglx,
	thomas.lendacky, tiala, wei.liu, yuehaibing, linux-acpi,
	linux-hyperv, linux-kernel, x86, apais, benhill, bperkins,
	sunilmut

On Wed, May 07, 2025 at 10:09:46PM +0200, Rafael J. Wysocki wrote:
> On Wed, May 7, 2025 at 8:22 PM Roman Kisel <romank@linux.microsoft.com> wrote:
> >
> > When starting APs, confidential guests and paravisor guests
> > need to know the CPU number, and the pattern of using the linear
> > search has emerged in several places. With N processors that leads
> > to the O(N^2) time complexity.
> >
> > Provide the CPU number in the AP wake up callback so that one can
> > get the CPU number in constant time.
> >
> > Suggested-by: Michael Kelley <mhklinux@outlook.com>
> > Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> 
> For the ACPI bits
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> and I'm assuming that the x86 folks will take care of this.
> 

Thank you Rafael. I will take this patch via the hyperv-next tree.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup
  2025-05-07 18:22 [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Roman Kisel
  2025-05-07 18:22 ` [PATCH hyperv-next 1/2] x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap() Roman Kisel
  2025-05-07 18:22 ` [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback Roman Kisel
@ 2025-05-08  4:22 ` Michael Kelley
  2025-05-08 15:09   ` Roman Kisel
  2025-05-08 18:39 ` Wei Liu
  3 siblings, 1 reply; 9+ messages in thread
From: Michael Kelley @ 2025-05-08  4:22 UTC (permalink / raw)
  To: Roman Kisel, ardb@kernel.org, bp@alien8.de, brgerst@gmail.com,
	dave.hansen@linux.intel.com, decui@microsoft.com,
	dimitri.sivanich@hpe.com, gautham.shenoy@amd.com,
	haiyangz@microsoft.com, hpa@zytor.com, imran.f.khan@oracle.com,
	jacob.jun.pan@linux.intel.com, jpoimboe@kernel.org,
	justin.ernst@hpe.com, kprateek.nayak@amd.com, kyle.meyer@hpe.com,
	kys@microsoft.com, lenb@kernel.org, mingo@redhat.com,
	nikunj@amd.com, papaluri@amd.com, patryk.wlazlyn@linux.intel.com,
	peterz@infradead.org, rafael@kernel.org, russ.anderson@hpe.com,
	sohil.mehta@intel.com, steve.wahl@hpe.com, tglx@linutronix.de,
	thomas.lendacky@amd.com, tiala@microsoft.com, wei.liu@kernel.org,
	yuehaibing@huawei.com, linux-acpi@vger.kernel.org,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	x86@kernel.org
  Cc: apais@microsoft.com, benhill@microsoft.com,
	bperkins@microsoft.com, sunilmut@microsoft.com

From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, May 7, 2025 11:22 AM
> 
> This patchset combines two patches that depend on each other and were not applying
> cleanly:
>   1. Fix APIC ID and VP index confusion in hv_snp_boot_ap():
>     https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/ 
>   2. Provide the CPU number in the wakeup AP callback:
>     https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/ 
> 
> I rebased the patches on top of the latest hyperv-next tree and updated the second patch
> that broke the linux-next build. That fix that, I made one non-functional change:
> updated the signature of numachip_wakeup_secondary() to match the parameter list of
> wakeup_secondary_cpu().
> 
> Roman Kisel (2):
>   x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap()
>   arch/x86: Provide the CPU number in the wakeup AP callback

I think this works. It's unfortunate that Patch 1 adds 11 lines of code/comments that
Patch 2 then deletes, which seems like undesirable churn. I was expecting adding the
"cpu" parameter to come first, which then makes fixing the hv_snp_boot_ap() problem
more straightforward. But looking more closely, hv_snp_boot_ap() already has a
parameter erroneously named "cpu", so adding the correct "cpu" parameter isn't
transparent. Hence the order you've chosen is probably the best resolution for a
messy situation. :-)

Michael

> 
>  arch/x86/coco/sev/core.c             | 13 ++-----
>  arch/x86/hyperv/hv_init.c            | 33 +++++++++++++++++
>  arch/x86/hyperv/hv_vtl.c             | 54 ++++------------------------
>  arch/x86/hyperv/ivm.c                | 11 ++++--
>  arch/x86/include/asm/apic.h          |  8 ++---
>  arch/x86/include/asm/mshyperv.h      |  7 ++--
>  arch/x86/kernel/acpi/madt_wakeup.c   |  2 +-
>  arch/x86/kernel/apic/apic_noop.c     |  8 ++++-
>  arch/x86/kernel/apic/apic_numachip.c |  2 +-
>  arch/x86/kernel/apic/x2apic_uv_x.c   |  2 +-
>  arch/x86/kernel/smpboot.c            | 10 +++---
>  include/hyperv/hvgdk_mini.h          |  2 +-
>  12 files changed, 76 insertions(+), 76 deletions(-)
> 
> 
> base-commit: 9b0844d87b1407681b78130429f798beb366f43f
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup
  2025-05-08  4:22 ` [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Michael Kelley
@ 2025-05-08 15:09   ` Roman Kisel
  0 siblings, 0 replies; 9+ messages in thread
From: Roman Kisel @ 2025-05-08 15:09 UTC (permalink / raw)
  To: Michael Kelley
  Cc: apais@microsoft.com, benhill@microsoft.com,
	bperkins@microsoft.com, sunilmut@microsoft.com, ardb@kernel.org,
	bp@alien8.de, brgerst@gmail.com, dave.hansen@linux.intel.com,
	decui@microsoft.com, dimitri.sivanich@hpe.com,
	gautham.shenoy@amd.com, haiyangz@microsoft.com, hpa@zytor.com,
	imran.f.khan@oracle.com, jacob.jun.pan@linux.intel.com,
	jpoimboe@kernel.org, justin.ernst@hpe.com, kprateek.nayak@amd.com,
	kyle.meyer@hpe.com, kys@microsoft.com, lenb@kernel.org,
	mingo@redhat.com, nikunj@amd.com, papaluri@amd.com,
	patryk.wlazlyn@linux.intel.com, peterz@infradead.org,
	rafael@kernel.org, russ.anderson@hpe.com, sohil.mehta@intel.com,
	steve.wahl@hpe.com, tglx@linutronix.de, thomas.lendacky@amd.com,
	tiala@microsoft.com, wei.liu@kernel.org, yuehaibing@huawei.com,
	linux-acpi@vger.kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, x86@kernel.org



On 5/7/2025 9:22 PM, Michael Kelley wrote:
> From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, May 7, 2025 11:22 AM
[...]>
> I think this works. It's unfortunate that Patch 1 adds 11 lines of code/comments that
> Patch 2 then deletes, which seems like undesirable churn. I was expecting adding the
> "cpu" parameter to come first, which then makes fixing the hv_snp_boot_ap() problem
> more straightforward. But looking more closely, hv_snp_boot_ap() already has a
> parameter erroneously named "cpu", so adding the correct "cpu" parameter isn't
> transparent. Hence the order you've chosen is probably the best resolution for a
> messy situation. :-)

Thanks, Michael :) Looked as a good trade-off, went ahead with it. Will
be happy to address any concerns of that folks might have!

> 
> Michael
> 
>>
>>   arch/x86/coco/sev/core.c             | 13 ++-----
>>   arch/x86/hyperv/hv_init.c            | 33 +++++++++++++++++
>>   arch/x86/hyperv/hv_vtl.c             | 54 ++++------------------------
>>   arch/x86/hyperv/ivm.c                | 11 ++++--
>>   arch/x86/include/asm/apic.h          |  8 ++---
>>   arch/x86/include/asm/mshyperv.h      |  7 ++--
>>   arch/x86/kernel/acpi/madt_wakeup.c   |  2 +-
>>   arch/x86/kernel/apic/apic_noop.c     |  8 ++++-
>>   arch/x86/kernel/apic/apic_numachip.c |  2 +-
>>   arch/x86/kernel/apic/x2apic_uv_x.c   |  2 +-
>>   arch/x86/kernel/smpboot.c            | 10 +++---
>>   include/hyperv/hvgdk_mini.h          |  2 +-
>>   12 files changed, 76 insertions(+), 76 deletions(-)
>>
>>
>> base-commit: 9b0844d87b1407681b78130429f798beb366f43f
>> --
>> 2.43.0
> 

-- 
Thank you,
Roman


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup
  2025-05-07 18:22 [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Roman Kisel
                   ` (2 preceding siblings ...)
  2025-05-08  4:22 ` [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Michael Kelley
@ 2025-05-08 18:39 ` Wei Liu
  2025-05-08 19:43   ` Roman Kisel
  3 siblings, 1 reply; 9+ messages in thread
From: Wei Liu @ 2025-05-08 18:39 UTC (permalink / raw)
  To: Roman Kisel
  Cc: ardb, bp, brgerst, dave.hansen, decui, dimitri.sivanich,
	gautham.shenoy, haiyangz, hpa, imran.f.khan, jacob.jun.pan,
	jpoimboe, justin.ernst, kprateek.nayak, kyle.meyer, kys, lenb,
	mhklinux, mingo, nikunj, papaluri, patryk.wlazlyn, peterz, rafael,
	russ.anderson, sohil.mehta, steve.wahl, tglx, thomas.lendacky,
	tiala, wei.liu, yuehaibing, linux-acpi, linux-hyperv,
	linux-kernel, x86, apais, benhill, bperkins, sunilmut

On Wed, May 07, 2025 at 11:22:24AM -0700, Roman Kisel wrote:
> This patchset combines two patches that depend on each other and were not applying
> cleanly:
>   1. Fix APIC ID and VP index confusion in hv_snp_boot_ap():
>     https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/
>   2. Provide the CPU number in the wakeup AP callback:
>     https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/
> 
> I rebased the patches on top of the latest hyperv-next tree and updated the second patch
> that broke the linux-next build. That fix that, I made one non-functional change:
> updated the signature of numachip_wakeup_secondary() to match the parameter list of
> wakeup_secondary_cpu().
> 
> Roman Kisel (2):
>   x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap()
>   arch/x86: Provide the CPU number in the wakeup AP callback

I queue these up.

Just so you know I'm experimenting a new setup. These have been applied
to hyperv-next-staging. It will take some time for them to propagate to
hyperv-next.

Thanks,
Wei.

> 
>  arch/x86/coco/sev/core.c             | 13 ++-----
>  arch/x86/hyperv/hv_init.c            | 33 +++++++++++++++++
>  arch/x86/hyperv/hv_vtl.c             | 54 ++++------------------------
>  arch/x86/hyperv/ivm.c                | 11 ++++--
>  arch/x86/include/asm/apic.h          |  8 ++---
>  arch/x86/include/asm/mshyperv.h      |  7 ++--
>  arch/x86/kernel/acpi/madt_wakeup.c   |  2 +-
>  arch/x86/kernel/apic/apic_noop.c     |  8 ++++-
>  arch/x86/kernel/apic/apic_numachip.c |  2 +-
>  arch/x86/kernel/apic/x2apic_uv_x.c   |  2 +-
>  arch/x86/kernel/smpboot.c            | 10 +++---
>  include/hyperv/hvgdk_mini.h          |  2 +-
>  12 files changed, 76 insertions(+), 76 deletions(-)
> 
> 
> base-commit: 9b0844d87b1407681b78130429f798beb366f43f
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup
  2025-05-08 18:39 ` Wei Liu
@ 2025-05-08 19:43   ` Roman Kisel
  0 siblings, 0 replies; 9+ messages in thread
From: Roman Kisel @ 2025-05-08 19:43 UTC (permalink / raw)
  To: Wei Liu
  Cc: ardb, bp, brgerst, dave.hansen, decui, dimitri.sivanich,
	gautham.shenoy, haiyangz, hpa, imran.f.khan, jacob.jun.pan,
	jpoimboe, justin.ernst, kprateek.nayak, kyle.meyer, kys, lenb,
	mhklinux, mingo, nikunj, papaluri, patryk.wlazlyn, peterz, rafael,
	russ.anderson, sohil.mehta, steve.wahl, tglx, thomas.lendacky,
	tiala, yuehaibing, linux-acpi, linux-hyperv, linux-kernel, x86,
	apais, benhill, bperkins, sunilmut



On 5/8/2025 11:39 AM, Wei Liu wrote:
> On Wed, May 07, 2025 at 11:22:24AM -0700, Roman Kisel wrote:
>> This patchset combines two patches that depend on each other and were not applying
>> cleanly:
>>    1. Fix APIC ID and VP index confusion in hv_snp_boot_ap():
>>      https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/
>>    2. Provide the CPU number in the wakeup AP callback:
>>      https://lore.kernel.org/linux-hyperv/20250430204720.108962-1-romank@linux.microsoft.com/
>>
>> I rebased the patches on top of the latest hyperv-next tree and updated the second patch
>> that broke the linux-next build. That fix that, I made one non-functional change:
>> updated the signature of numachip_wakeup_secondary() to match the parameter list of
>> wakeup_secondary_cpu().
>>
>> Roman Kisel (2):
>>    x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap()
>>    arch/x86: Provide the CPU number in the wakeup AP callback
> 
> I queue these up.
> 
> Just so you know I'm experimenting a new setup. These have been applied
> to hyperv-next-staging. It will take some time for them to propagate to
> hyperv-next.

Thank you very much! That's exciting news about the new staging setup!!

> 
> Thanks,
> Wei.
> 
>>
>>   arch/x86/coco/sev/core.c             | 13 ++-----
>>   arch/x86/hyperv/hv_init.c            | 33 +++++++++++++++++
>>   arch/x86/hyperv/hv_vtl.c             | 54 ++++------------------------
>>   arch/x86/hyperv/ivm.c                | 11 ++++--
>>   arch/x86/include/asm/apic.h          |  8 ++---
>>   arch/x86/include/asm/mshyperv.h      |  7 ++--
>>   arch/x86/kernel/acpi/madt_wakeup.c   |  2 +-
>>   arch/x86/kernel/apic/apic_noop.c     |  8 ++++-
>>   arch/x86/kernel/apic/apic_numachip.c |  2 +-
>>   arch/x86/kernel/apic/x2apic_uv_x.c   |  2 +-
>>   arch/x86/kernel/smpboot.c            | 10 +++---
>>   include/hyperv/hvgdk_mini.h          |  2 +-
>>   12 files changed, 76 insertions(+), 76 deletions(-)
>>
>>
>> base-commit: 9b0844d87b1407681b78130429f798beb366f43f
>> -- 
>> 2.43.0
>>

-- 
Thank you,
Roman


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-05-08 19:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-07 18:22 [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Roman Kisel
2025-05-07 18:22 ` [PATCH hyperv-next 1/2] x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap() Roman Kisel
2025-05-07 18:22 ` [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback Roman Kisel
2025-05-07 20:09   ` Rafael J. Wysocki
2025-05-08  1:10     ` Wei Liu
2025-05-08  4:22 ` [PATCH hyperv-next 0/2] arch/x86, x86/hyperv: Few fixes for the AP startup Michael Kelley
2025-05-08 15:09   ` Roman Kisel
2025-05-08 18:39 ` Wei Liu
2025-05-08 19:43   ` Roman Kisel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox