Hi, Roger,
On Mon, Nov 21, 2022 at 10:04 AM Roger Pau Monné roger.pau@citrix.com wrote:
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);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.
Are you working on this patch anymore? My Xen HWP patches need a Linux patch like this one to set bit 12 in the PDC. I had an affected user test with this patch and it worked, serving as an equivalent of Linux commit a21211672c9a ("ACPI / processor: Request native thermal interrupt handling via _OSC").
Another idea is to use Linux's arch_acpi_set_pdc_bits() to make the hypercall to Xen. It occurs earlier: acpi_processor_set_pdc() acpi_processor_alloc_pdc() acpi_set_pdc_bits() arch_acpi_set_pdc_bits() acpi_processor_eval_pdc()
So the IDLE_NOMWAIT masking in acpi_processor_eval_pdc() would still apply. arch_acpi_set_pdc_bits() is provided the buffer, so it's a little cleaner in that respect.
Thanks, Jason