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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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:
> >> The treatment of ACPI_PDC_CPPC_NATIVE_INTR should follow that of other P-
> >> state related bits. Add the bit to ACPI_PDC_P_MASK and apply "mask" in
> >> arch_acpi_set_pdc_bits() when setting that bit. Move this next to the
> >> other P-state related logic.
> >>
> >> Further apply ACPI_PDC_P_MASK also when the amd-cppc driver is in use.
> >>
> >> Also leave a comment regarding the clearing of bits and add a couple of
> >> blank lines.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >> ---
> >> Including XEN_PROCESSOR_PM_CPPC may need accompanying with some change to
> >> arch_acpi_set_pdc_bits(), but it's entirely unclear to me what to do
> >> there. I'm unaware of an AMD counterpart of Intel's "Intel® Processor
> >> Vendor-Specific ACPI". Plus even when the powernow driver is in use, we
> >> never set any bits, as EIST is an Intel-only feature.
> > 
> > We possibly never need to set any bits there for AMD, as those _PDC
> > Processor bits are Intel specific?
> 
> Indeed, that's a possibility.
> 
> >> --- 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?

> > 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.  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.

Thanks, Roger.