[0/3] xen: ACPI processor related fixes

Message

Roger Pau Monné Nov. 21, 2022, 10:21 a.m. UTC

Hello,

This series aims to fix some shortcomings with the handling of ACPI
Processors objects when running as a Xen dom0.

First two patches fix the execution of the _PDC methods for all CPUs on
the system and not just the ones available to dom0, while also making
sure that the _PDC capabilities reported to ACPI match what the
perfrmance and power drivers in Xen can handle.

Final patch fixes the Xen ACPI Processor driver to also work when used
in a PVH dom0, that has a custom build ACPI MADT table and mismatched
Processor UIDs between the MADT and the Processor objects in the dynamic
AML.

I don't really like the current implementation of the Xen ACPI Processor
driver, it IMO relies too much on data being fetched by generic kernel
code.  For one the generic fetcher functions can take CPUID data into
account in order to sanitize what's found in ACPI, but capabilities
reported to dom0 can be different from the native ones.  Also the Xen
ACPI Processor code relies on cloning the data from CPUs in order to fill
for the pCPUs > vCPUs, but this is wrong when running on heterogeneous
systems.

Last patch introduces some helpers to Xen ACPI Processor that should
allow fetching all the required data, for each ACPI Processor object on
the dynamic tables.  It might be helpful to explore disabling any
Processor object handling done by generic drivers and just fetch all the
data from the Xen Processor driver itself for every Processor object on
the namespace.  Likewise it might be better to just execute _PDC from
that same Xen ACPI Processor driver instead of polluting the generic
ACPI Processor driver.

The series should be taken as a RFC partially, due to my own doubts
about whether the current implementation is indeed the right one moving
forward.

Thanks, Roger.

Roger Pau Monne (3):
  acpi/processor: fix evaluating _PDC method when running as Xen dom0
  acpi/processor: sanitize _PDC buffer bits when running as Xen dom0
  xen/acpi: upload power and performance related data from a PVH dom0

 arch/x86/include/asm/xen/hypervisor.h |  12 ++
 arch/x86/xen/enlighten.c              |  44 +++++
 drivers/acpi/processor_pdc.c          |  19 +++
 drivers/xen/xen-acpi-processor.c      | 225 ++++++++++++++++++++++++--
 4 files changed, 284 insertions(+), 16 deletions(-)

Comments

Jan Beulich Nov. 21, 2022, 2:20 p.m. UTC | #1

On 21.11.2022 11:21, Roger Pau Monne wrote:
> Hello,
> 
> This series aims to fix some shortcomings with the handling of ACPI
> Processors objects when running as a Xen dom0.
> 
> First two patches fix the execution of the _PDC methods for all CPUs on
> the system and not just the ones available to dom0, while also making
> sure that the _PDC capabilities reported to ACPI match what the
> perfrmance and power drivers in Xen can handle.
> 
> Final patch fixes the Xen ACPI Processor driver to also work when used
> in a PVH dom0, that has a custom build ACPI MADT table and mismatched
> Processor UIDs between the MADT and the Processor objects in the dynamic
> AML.
> 
> I don't really like the current implementation of the Xen ACPI Processor
> driver, it IMO relies too much on data being fetched by generic kernel
> code.  For one the generic fetcher functions can take CPUID data into
> account in order to sanitize what's found in ACPI, but capabilities
> reported to dom0 can be different from the native ones.  Also the Xen
> ACPI Processor code relies on cloning the data from CPUs in order to fill
> for the pCPUs > vCPUs, but this is wrong when running on heterogeneous
> systems.

Yes, these are problems (and as per reading the description of the
last patch you even extend this "cloning" of data), but ...

> Last patch introduces some helpers to Xen ACPI Processor that should
> allow fetching all the required data, for each ACPI Processor object on
> the dynamic tables.  It might be helpful to explore disabling any
> Processor object handling done by generic drivers and just fetch all the
> data from the Xen Processor driver itself for every Processor object on
> the namespace.  Likewise it might be better to just execute _PDC from
> that same Xen ACPI Processor driver instead of polluting the generic
> ACPI Processor driver.

... cloning functions living elsewhere also has the genuine problem of
them then needing to be kept in sync without there being any trigger to
know when an original function was changed in some way.

Jan

Roger Pau Monné Nov. 21, 2022, 4:27 p.m. UTC | #2

On Mon, Nov 21, 2022 at 03:20:53PM +0100, Jan Beulich wrote:
> On 21.11.2022 11:21, Roger Pau Monne wrote:
> > Hello,
> > 
> > This series aims to fix some shortcomings with the handling of ACPI
> > Processors objects when running as a Xen dom0.
> > 
> > First two patches fix the execution of the _PDC methods for all CPUs on
> > the system and not just the ones available to dom0, while also making
> > sure that the _PDC capabilities reported to ACPI match what the
> > perfrmance and power drivers in Xen can handle.
> > 
> > Final patch fixes the Xen ACPI Processor driver to also work when used
> > in a PVH dom0, that has a custom build ACPI MADT table and mismatched
> > Processor UIDs between the MADT and the Processor objects in the dynamic
> > AML.
> > 
> > I don't really like the current implementation of the Xen ACPI Processor
> > driver, it IMO relies too much on data being fetched by generic kernel
> > code.  For one the generic fetcher functions can take CPUID data into
> > account in order to sanitize what's found in ACPI, but capabilities
> > reported to dom0 can be different from the native ones.  Also the Xen
> > ACPI Processor code relies on cloning the data from CPUs in order to fill
> > for the pCPUs > vCPUs, but this is wrong when running on heterogeneous
> > systems.
> 
> Yes, these are problems (and as per reading the description of the
> last patch you even extend this "cloning" of data), but ...
> 
> > Last patch introduces some helpers to Xen ACPI Processor that should
> > allow fetching all the required data, for each ACPI Processor object on
> > the dynamic tables.  It might be helpful to explore disabling any
> > Processor object handling done by generic drivers and just fetch all the
> > data from the Xen Processor driver itself for every Processor object on
> > the namespace.  Likewise it might be better to just execute _PDC from
> > that same Xen ACPI Processor driver instead of polluting the generic
> > ACPI Processor driver.
> 
> ... cloning functions living elsewhere also has the genuine problem of
> them then needing to be kept in sync without there being any trigger to
> know when an original function was changed in some way.

Well, yes, but using generic functions also has the risk of them being
modified to take into account CPUID data for example and then the
result would no longer be suitable for Xen's usage without us
noticing.

Also has the downside of parsing the data into Linux structures which
then need to be translated into Xen format.  It might be more straight
forward to just evaluate the required ACPI methods and parse the ACPI
data from the Xen ACPI Processor driver into the format used by Xen
and upload that to the hypervisor.

I realize however this is a big change, and would mean almost a
rewrite from scratch of the Xen ACPI Processor driver.  I wouldn't
want to start that task without having agreement that this is the
correct way forward.

Roger.