Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.03.2026 09:50, Roger Pau Monné wrote:
> On Thu, Mar 05, 2026 at 09:17:23AM +0100, Jan Beulich wrote:
>> On 04.03.2026 17:36, Roger Pau Monné wrote:
>>> On Wed, Mar 04, 2026 at 03:37:25PM +0100, Jan Beulich wrote:
>>>> --- a/xen/drivers/cpufreq/cpufreq.c
>>>> +++ b/xen/drivers/cpufreq/cpufreq.c
>>>> @@ -694,14 +694,23 @@ int acpi_set_pdc_bits(unsigned int acpi_
>>>>      {
>>>>          uint32_t mask = 0;
>>>>  
>>>> +        /*
>>>> +         * Accumulate all the bits under Xen's control, to mask them off, 
>>>> for
>>>> +         * arch_acpi_set_pdc_bits() to then set those we want set.
>>>> +         */
>>>>          if ( xen_processor_pmbits & XEN_PROCESSOR_PM_CX )
>>>>              mask |= ACPI_PDC_C_MASK | ACPI_PDC_SMP_C1PT;
>>>> -        if ( xen_processor_pmbits & XEN_PROCESSOR_PM_PX )
>>>> +
>>>> +        if ( xen_processor_pmbits &
>>>> +             (XEN_PROCESSOR_PM_PX | XEN_PROCESSOR_PM_CPPC) )
>>>
>>> Currently the CPPC driver is AMD only, and hence when using it we
>>> don't care about filtering the _PDC bits, because the ones Xen knows
>>> about are Intel-only?
>>>
>>> As you say, we likely need some clarification about whether there's
>>> _PDC bits AMD care about?
>>>
>>> Linux seems to unconditionally set bits in _PDC, so some of those
>>> might actually be parsed by AMD.
>>
>> Or it setting whatever it wants is meaningless on AMD systems. Where I
>> have extracted ACPI tables readily to hand, there's no _PDC there.
> 
> Oh, interesting, so there's no method to start with.  Is there an _OSC
> method however for processor objects?  _PDC is deprecated, and maybe
> AMD systems only expose the equivalent non-deprecated _OSC?

There is, yes.

>>> I think we might want to split the setting of XEN_PROCESSOR_PM_CPPC
>>> here from the addition of ACPI_PDC_CPPC_NATIVE_INTR into
>>> ACPI_PDC_P_MASK.  The latter we can possibly untie from the questions
>>> we have about AMD usage of _PDC.
>>
>> Hmm, yes, I can certainly split the patch. I'm looking at it a little
>> differently, though: Us leaving any P-state related bits in place when
>> cpufreq handling is done in Xen has been a mistake anyway.
> 
> Yes, TBH I even wondered whether we might just wipe whatever the OS
> sets in the _PDC bits and completely fill it from Xen (unless for the
> weird/broken case where dom0 is driving cpufreq?).
> 
> This is kind of what Xen already does now.

Indeed (except for the T-state ones, as support for that was never added
to Xen).

>  However see below.
> 
>> What's
>> unclear is solely whether because of us driving things some bits need
>> setting (likely none if AMD systems indeed don't surface _PDC in the
>> first place).
> 
> Since we have the parsing of the ACPI related data done from dom0 it's
> not only Xen that needs to support the feature, but dom0 also needs to
> know how to parse it.  Or we just assume the driver in dom0 must
> strictly know how to parse data from the features enabled by Xen.
> 
> Maybe Xen supported bits should be & with the dom0 ones?  So dom0
> would set what it can parse, and Xen would AND that with what the
> cpufreq drivers support?  However that would be an ABI change.

What cpufreq drivers are you talking about here? When Xen handles P-
state transitions, the drivers in Dom0 would preferably not even be
loaded. That's what the forward-port did. Upstream they may be loaded,
but they then can't actually do anything (and they may exit early).
Coordination is necessary only with the ACPI driver(s), and that's what
this function is about.

Jan