Re: [PATCHv3 14/14] x86/acpi: Add support for CPU offlining for ACPI MADT wakeup method

[Huang, Kai]

> 
> > > +
> > >  int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
> > >  			      const unsigned long end)
> > >  {
> > >  	struct acpi_madt_multiproc_wakeup *mp_wake;
> > >  
> > >  	mp_wake = (struct acpi_madt_multiproc_wakeup *)header;
> > > -	if (BAD_MADT_ENTRY(mp_wake, end))
> > > +	if (!mp_wake)
> > > +		return -EINVAL;
> > 
> > I think you can keep the BAD_MADT_ENTRY() check as a standard check, and ...
> 
> No. BAD_MADT_ENTRY() will fail if the struct version is V0 because the
> size will be smaller than sizeof(struct acpi_madt_multiproc_wakeup).

Ah OK.  Maybe worth a comment like below?


	/*
	 * Cannot use the standard BAD_MADT_ENTRY() to sanity check the
@mp_wake
	 * entry.  'sizeof (struct acpi_madt_multiproc_wakeup)' can be larger
	 * than the actual size of the MP wakeup entry in ACPI table because
the
	 * 'reset_vector' is only available in the V2 MP wakeup structure.
	 */


[...]

> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -878,10 +878,14 @@ static int crash_nmi_callback(unsigned int val, struct pt_regs *regs)
>  	cpu_emergency_disable_virtualization();
>  
>  	atomic_dec(&waiting_for_crash_ipi);
> -	/* Assume hlt works */
> -	halt();
> -	for (;;)
> -		cpu_relax();
> +
> +	if (smp_ops.crash_play_dead) {
> +	    smp_ops.crash_play_dead();
> +	} else {
> +		halt();
> +		for (;;)
> +			cpu_relax();
> +	}
>  

Agree this is better than explicitly checking TDX guest. :-)