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

[Thomas Gleixner <tglx@linutronix.de>]

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