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 <[email protected]>
>> ---
>> 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.

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

Jan

Reply via email to