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

Jan Beulich posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/ca1812c2-dadf-422a-a195-9c285ce08077@suse.com
[PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Jan Beulich 1 month, 1 week ago
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@suse.com>
---
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.

acpi_set_pdc_bits() having moved to the cpufreq driver looks to have been
a mistake. It covers not only P-state related bits, but also C-state and
T-state ones. (This is only a latent issue as long as
https://lists.xen.org/archives/html/xen-devel/2026-02/msg00875.html
wouldn't land.)

--- a/xen/arch/x86/acpi/lib.c
+++ b/xen/arch/x86/acpi/lib.c
@@ -124,6 +124,9 @@ int arch_acpi_set_pdc_bits(u32 acpi_id,
 	if (cpu_has(c, X86_FEATURE_EIST))
 		pdc[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP & mask;
 
+	if (hwp_active())
+		pdc[2] |= ACPI_PDC_CPPC_NATIVE_INTR & mask;
+
 	if (cpu_has(c, X86_FEATURE_ACPI))
 		pdc[2] |= ACPI_PDC_T_FFH & mask;
 
@@ -142,8 +145,5 @@ int arch_acpi_set_pdc_bits(u32 acpi_id,
 	    !(ecx & CPUID5_ECX_INTERRUPT_BREAK))
 		pdc[2] &= ~(ACPI_PDC_C_C1_FFH | ACPI_PDC_C_C2C3_FFH);
 
-	if (hwp_active())
-		pdc[2] |= ACPI_PDC_CPPC_NATIVE_INTR;
-
 	return 0;
 }
--- 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) )
             mask |= ACPI_PDC_P_MASK | ACPI_PDC_SMP_C1PT;
+
         if ( xen_processor_pmbits & XEN_PROCESSOR_PM_TX )
             mask |= ACPI_PDC_T_MASK | ACPI_PDC_SMP_C1PT;
+
         bits[2] &= (ACPI_PDC_C_MASK | ACPI_PDC_P_MASK | ACPI_PDC_T_MASK |
                     ACPI_PDC_SMP_C1PT) & ~mask;
+
         ret = arch_acpi_set_pdc_bits(acpi_id, bits, mask);
     }
     if ( !ret && __copy_to_guest_offset(pdc, 2, bits + 2, 1) )
--- a/xen/include/acpi/pdc_intel.h
+++ b/xen/include/acpi/pdc_intel.h
@@ -43,7 +43,8 @@
 
 #define ACPI_PDC_P_MASK			(ACPI_PDC_P_FFH | \
 					 ACPI_PDC_SMP_P_SWCOORD | \
-					 ACPI_PDC_SMP_P_HWCOORD)
+					 ACPI_PDC_SMP_P_HWCOORD | \
+					 ACPI_PDC_CPPC_NATIVE_INTR)
 
 #define ACPI_PDC_T_MASK			(ACPI_PDC_T_FFH | \
 					 ACPI_PDC_SMP_T_SWCOORD)

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Roger Pau Monné 1 month, 1 week ago
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@suse.com>
> ---
> 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?

> acpi_set_pdc_bits() having moved to the cpufreq driver looks to have been
> a mistake. It covers not only P-state related bits, but also C-state and
> T-state ones. (This is only a latent issue as long as
> https://lists.xen.org/archives/html/xen-devel/2026-02/msg00875.html
> wouldn't land.)
> 
> --- a/xen/arch/x86/acpi/lib.c
> +++ b/xen/arch/x86/acpi/lib.c
> @@ -124,6 +124,9 @@ int arch_acpi_set_pdc_bits(u32 acpi_id,
>  	if (cpu_has(c, X86_FEATURE_EIST))
>  		pdc[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP & mask;
>  
> +	if (hwp_active())
> +		pdc[2] |= ACPI_PDC_CPPC_NATIVE_INTR & mask;
> +
>  	if (cpu_has(c, X86_FEATURE_ACPI))
>  		pdc[2] |= ACPI_PDC_T_FFH & mask;
>  
> @@ -142,8 +145,5 @@ int arch_acpi_set_pdc_bits(u32 acpi_id,
>  	    !(ecx & CPUID5_ECX_INTERRUPT_BREAK))
>  		pdc[2] &= ~(ACPI_PDC_C_C1_FFH | ACPI_PDC_C_C2C3_FFH);
>  
> -	if (hwp_active())
> -		pdc[2] |= ACPI_PDC_CPPC_NATIVE_INTR;
> -
>  	return 0;
>  }
> --- 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.

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.

Thanks, Roger.

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Jan Beulich 1 month, 1 week ago
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@suse.com>
>> ---
>> 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

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Roger Pau Monné 1 month, 1 week ago
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@suse.com>
> >> ---
> >> 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.

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Jan Beulich 1 month, 1 week ago
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

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Roger Pau Monné 1 month, 1 week ago
On Thu, Mar 05, 2026 at 10:20:02AM +0100, Jan Beulich wrote:
> 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.

Hm, then an OS would use the Xen _PDC adjusted buffer with the _OSC
Processor object method I think, and we are back to the position of
not having an AMD specification for those bits.

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

I was talking about the Xen cpufreq driver, sorry the context was
confusing.

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

Well, yes, on FreeBSD I simply overtake the native ACPI Processor
driver with a Xen specific one that has higher priority.  So the
native ACPI Processor driver doesn't even attach.  I think Linux is
slightly different in that it allows the native driver to do the
fetching of the information, and then the Xen driver only does the
uploading.

> Coordination is necessary only with the ACPI driver(s), and that's what
> this function is about.

I think Xen also needs coordination with the driver in dom0 that
fetches the information from ACPI?  It's not only Xen that needs to
report what the cpufreq driver support, but also dom0 would need to
expose what it can correctly parse.

Thanks, Roger.

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Jan Beulich 1 month, 1 week ago
On 05.03.2026 11:17, Roger Pau Monné wrote:
> On Thu, Mar 05, 2026 at 10:20:02AM +0100, Jan Beulich wrote:
>> On 05.03.2026 09:50, Roger Pau Monné wrote:
>>> 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?
> 
> I was talking about the Xen cpufreq driver, sorry the context was
> confusing.
> 
>> 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).
> 
> Well, yes, on FreeBSD I simply overtake the native ACPI Processor
> driver with a Xen specific one that has higher priority.  So the
> native ACPI Processor driver doesn't even attach.  I think Linux is
> slightly different in that it allows the native driver to do the
> fetching of the information, and then the Xen driver only does the
> uploading.
> 
>> Coordination is necessary only with the ACPI driver(s), and that's what
>> this function is about.
> 
> I think Xen also needs coordination with the driver in dom0 that
> fetches the information from ACPI?

That's what I meant with "ACPI driver(s)".

>  It's not only Xen that needs to
> report what the cpufreq driver support, but also dom0 would need to
> expose what it can correctly parse.

Hmm, yes, strictly speaking we should tie setting of respective bits to
Dom0 having uploaded corresponding data. The order of these operations
may, however, be at best undefined (and possibly be well defined in the
unhelpful - for us - order). I don't think I see anything we can do
about this.

Jan

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Roger Pau Monné 1 month, 1 week ago
On Thu, Mar 05, 2026 at 12:39:51PM +0100, Jan Beulich wrote:
> On 05.03.2026 11:17, Roger Pau Monné wrote:
> > On Thu, Mar 05, 2026 at 10:20:02AM +0100, Jan Beulich wrote:
> >> On 05.03.2026 09:50, Roger Pau Monné wrote:
> >>> 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?
> > 
> > I was talking about the Xen cpufreq driver, sorry the context was
> > confusing.
> > 
> >> 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).
> > 
> > Well, yes, on FreeBSD I simply overtake the native ACPI Processor
> > driver with a Xen specific one that has higher priority.  So the
> > native ACPI Processor driver doesn't even attach.  I think Linux is
> > slightly different in that it allows the native driver to do the
> > fetching of the information, and then the Xen driver only does the
> > uploading.
> > 
> >> Coordination is necessary only with the ACPI driver(s), and that's what
> >> this function is about.
> > 
> > I think Xen also needs coordination with the driver in dom0 that
> > fetches the information from ACPI?
> 
> That's what I meant with "ACPI driver(s)".
> 
> >  It's not only Xen that needs to
> > report what the cpufreq driver support, but also dom0 would need to
> > expose what it can correctly parse.
> 
> Hmm, yes, strictly speaking we should tie setting of respective bits to
> Dom0 having uploaded corresponding data. The order of these operations
> may, however, be at best undefined (and possibly be well defined in the
> unhelpful - for us - order). I don't think I see anything we can do
> about this.

I'm afraid it's the other way around, you need to first call _PDC, and
then fetch the data.  As I've learned the hard way while doing the
FreeBSD driver: you must call _PDC before attempting to fetch the
data, as ACPI will modulate what gets returned/is present on the
Processor objects based on what support the OSPM has specified in the
_PDC bits.

Anyway, not sure there's much we can do now about any of this, it's
too late to change the interface, and what we have seems to kind of
work on for the purpose.

Thanks, Roger.

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Jan Beulich 1 month, 1 week ago
On 05.03.2026 13:30, Roger Pau Monné wrote:
> On Thu, Mar 05, 2026 at 12:39:51PM +0100, Jan Beulich wrote:
>> On 05.03.2026 11:17, Roger Pau Monné wrote:
>>> On Thu, Mar 05, 2026 at 10:20:02AM +0100, Jan Beulich wrote:
>>>> On 05.03.2026 09:50, Roger Pau Monné wrote:
>>>>> 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?
>>>
>>> I was talking about the Xen cpufreq driver, sorry the context was
>>> confusing.
>>>
>>>> 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).
>>>
>>> Well, yes, on FreeBSD I simply overtake the native ACPI Processor
>>> driver with a Xen specific one that has higher priority.  So the
>>> native ACPI Processor driver doesn't even attach.  I think Linux is
>>> slightly different in that it allows the native driver to do the
>>> fetching of the information, and then the Xen driver only does the
>>> uploading.
>>>
>>>> Coordination is necessary only with the ACPI driver(s), and that's what
>>>> this function is about.
>>>
>>> I think Xen also needs coordination with the driver in dom0 that
>>> fetches the information from ACPI?
>>
>> That's what I meant with "ACPI driver(s)".
>>
>>>  It's not only Xen that needs to
>>> report what the cpufreq driver support, but also dom0 would need to
>>> expose what it can correctly parse.
>>
>> Hmm, yes, strictly speaking we should tie setting of respective bits to
>> Dom0 having uploaded corresponding data. The order of these operations
>> may, however, be at best undefined (and possibly be well defined in the
>> unhelpful - for us - order). I don't think I see anything we can do
>> about this.
> 
> I'm afraid it's the other way around, you need to first call _PDC, and
> then fetch the data.  As I've learned the hard way while doing the
> FreeBSD driver: you must call _PDC before attempting to fetch the
> data, as ACPI will modulate what gets returned/is present on the
> Processor objects based on what support the OSPM has specified in the
> _PDC bits.

In which case at least for Linux we're okay, as what we need it has always
been capable of parsing.

> Anyway, not sure there's much we can do now about any of this, it's
> too late to change the interface, and what we have seems to kind of
> work on for the purpose.

Which reads almost(?) like an ack-in-disguise to me ...

Jan

Re: [PATCH] x86/ACPI: _PDC bits vs HWP/CPPC
Posted by Roger Pau Monné 1 month, 1 week ago
On Thu, Mar 05, 2026 at 01:40:23PM +0100, Jan Beulich wrote:
> On 05.03.2026 13:30, Roger Pau Monné wrote:
> > On Thu, Mar 05, 2026 at 12:39:51PM +0100, Jan Beulich wrote:
> >> On 05.03.2026 11:17, Roger Pau Monné wrote:
> >>> On Thu, Mar 05, 2026 at 10:20:02AM +0100, Jan Beulich wrote:
> >>>> On 05.03.2026 09:50, Roger Pau Monné wrote:
> >>>>> 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?
> >>>
> >>> I was talking about the Xen cpufreq driver, sorry the context was
> >>> confusing.
> >>>
> >>>> 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).
> >>>
> >>> Well, yes, on FreeBSD I simply overtake the native ACPI Processor
> >>> driver with a Xen specific one that has higher priority.  So the
> >>> native ACPI Processor driver doesn't even attach.  I think Linux is
> >>> slightly different in that it allows the native driver to do the
> >>> fetching of the information, and then the Xen driver only does the
> >>> uploading.
> >>>
> >>>> Coordination is necessary only with the ACPI driver(s), and that's what
> >>>> this function is about.
> >>>
> >>> I think Xen also needs coordination with the driver in dom0 that
> >>> fetches the information from ACPI?
> >>
> >> That's what I meant with "ACPI driver(s)".
> >>
> >>>  It's not only Xen that needs to
> >>> report what the cpufreq driver support, but also dom0 would need to
> >>> expose what it can correctly parse.
> >>
> >> Hmm, yes, strictly speaking we should tie setting of respective bits to
> >> Dom0 having uploaded corresponding data. The order of these operations
> >> may, however, be at best undefined (and possibly be well defined in the
> >> unhelpful - for us - order). I don't think I see anything we can do
> >> about this.
> > 
> > I'm afraid it's the other way around, you need to first call _PDC, and
> > then fetch the data.  As I've learned the hard way while doing the
> > FreeBSD driver: you must call _PDC before attempting to fetch the
> > data, as ACPI will modulate what gets returned/is present on the
> > Processor objects based on what support the OSPM has specified in the
> > _PDC bits.
> 
> In which case at least for Linux we're okay, as what we need it has always
> been capable of parsing.
> 
> > Anyway, not sure there's much we can do now about any of this, it's
> > too late to change the interface, and what we have seems to kind of
> > work on for the purpose.
> 
> Which reads almost(?) like an ack-in-disguise to me ...

Possibly, I'm just a bit hesitant because I'm unsure about what CPPC
requires to be set in the _PDC.  Right now we would allow any
P-related bits set by dom0 to be propagated into _PDC when using CPPC
driver.  With this change we would mask any P-related bits from _PDC
when using CPPC.

I think this is likely fine, but would like to get confirmation from
AMD.  Maybe I'm overthinking it.

Penny, Jason, Alejandro, Stefano: do you know which/if there's any
_PDC/_OSC bits defined on AMD Processor Objects, or is it just
supposed to be always empty?  It would be good to get some
clarification there about which bits if any are supposed to be
meaningful when using CPPC.

Also, on a related tangent, \_SB._OSC seems to also define some
vendor-agnostic bits related to CPPC [0].  Should the CPPC cpufreq
driver force any of those to be set?  "CPPC {2} Support" bits seem
like they should be set when Xen is using the CPPC cpufreq driver?

Thanks, Roger.

[0] https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/Device_Configuration.html#osc-operating-system-capabilities