linux-kernel - Re: [PATCH hyperv-next v2] arch/x86: Provide the CPU number in the wakeup AP callback

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87cyctphqo.ffs@tglx>
Date: Wed, 30 Apr 2025 21:33:03 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Roman Kisel <romank@...ux.microsoft.com>, ardb@...nel.org, bp@...en8.de,
 dave.hansen@...ux.intel.com, decui@...rosoft.com,
 dimitri.sivanich@....com, haiyangz@...rosoft.com, hpa@...or.com,
 imran.f.khan@...cle.com, jacob.jun.pan@...ux.intel.com, jgross@...e.com,
 justin.ernst@....com, kprateek.nayak@....com, kyle.meyer@....com,
 kys@...rosoft.com, lenb@...nel.org, mingo@...hat.com, nikunj@....com,
 papaluri@....com, perry.yuan@....com, peterz@...radead.org,
 rafael@...nel.org, romank@...ux.microsoft.com, russ.anderson@....com,
 steve.wahl@....com, thomas.lendacky@....com, tim.c.chen@...ux.intel.com,
 tony.luck@...el.com, wei.liu@...nel.org, xin@...or.com,
 yuehaibing@...wei.com, linux-acpi@...r.kernel.org,
 linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org
Cc: apais@...rosoft.com, benhill@...rosoft.com, bperkins@...rosoft.com,
 sunilmut@...rosoft.com
Subject: Re: [PATCH hyperv-next v2] arch/x86: Provide the CPU number in the
 wakeup AP callback

On Wed, Apr 30 2025 at 09:14, Roman Kisel wrote:
> -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, int cpu)

unsigned int cpu please. There are no negative CPU numbers yet :)

>  {
>  	struct sev_es_save_area *cur_vmsa, *vmsa;
>  	struct ghcb_state state;
> @@ -1187,7 +1187,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;
>  
>  	/*
> @@ -1208,15 +1208,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;
> -

I just looked what arch_match_cpu_phys_id() actually does and I couldn't
help myself to get a fit of laughter. x86 uses the weak default function
in drivers/of/cpu.c:

bool __weak arch_match_cpu_phys_id(int cpu, u64 phys_id)
{
	return (u32)phys_id == cpu;
}

So this loop is the most convoluted way to write:

       cpu = apic_id;

which is valid because the to be started CPU must be present, no?

I'm not opposed against the CPU number argument per se, but the
justification for it is dubious at best.

Thanks,

        tglx