Re: [PATCH 2/3] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0

[Roger Pau Monné]

On Mon, Nov 21, 2022 at 03:10:36PM +0100, Jan Beulich wrote:
> On 21.11.2022 11:21, Roger Pau Monne wrote:
> > --- a/drivers/acpi/processor_pdc.c
> > +++ b/drivers/acpi/processor_pdc.c
> > @@ -137,6 +137,14 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
> >  		buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
> >  
> >  	}
> > +	if (xen_initial_domain())
> > +		/*
> > +		 * When Linux is running as Xen dom0 it's the hypervisor the
> > +		 * entity in charge of the processor power management, and so
> > +		 * Xen needs to check the OS capabilities reported in the _PDC
> > +		 * buffer matches what the hypervisor driver supports.
> > +		 */
> > +		xen_sanitize_pdc((uint32_t *)pdc_in->pointer->buffer.pointer);
> >  	status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
> 
> Again looking at our old XenoLinux forward port we had this inside the
> earlier if(), as an _alternative_ to the &= (I don't think it's valid
> to apply both the kernel's and Xen's adjustments). That would also let
> you use "buffer" rather than re-calculating it via yet another (risky
> from an abstract pov) cast.

Hm, I've wondered this and decided it wasn't worth to short-circuit
the boot_option_idle_override conditional because ACPI_PDC_C_C2C3_FFH
and ACPI_PDC_C_C1_FFH will be set anyway by Xen in
arch_acpi_set_pdc_bits() as part of ACPI_PDC_C_CAPABILITY_SMP.

I could re-use some of the code in there, but didn't want to make it
more difficult to read just for the benefit of reusing buffer.

> It was the very nature of requiring Xen-specific conditionals which I
> understand was the reason why so far no attempt was made to get this
> (incl the corresponding logic for patch 1) into any upstream kernel.

Yes, well, it's all kind of ugly.  Hence my suggestion to simply avoid
doing any ACPI Processor object handling in Linux with the native code
and handle it all in a Xen specific driver.  That requires the Xen
driver being able to fetch more data itself form the ACPI Processor
methods, but also unties it from the dependency on the data being
filled by the generic code, and the 'tricks' is plays into fooling
generic code to think certain processors are online.

Thanks, Roger.