2023-11-28 18:55:18

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH] x86/cpu: Update power flags

As described on page 99 of
"Processing Programming Reference (PPR) for AMD Family 19h Model 61h, Revision B1 Processor".
(AMD Documentation Hub Document 56713)

Tested on an "AMD Ryzen 7 7840U w/ Radeon 780M Graphics".

Signed-off-by: Thomas Weißschuh <[email protected]>
---
arch/x86/kernel/cpu/powerflags.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/cpu/powerflags.c b/arch/x86/kernel/cpu/powerflags.c
index fd6ec2aa0303..0c98405aeae2 100644
--- a/arch/x86/kernel/cpu/powerflags.c
+++ b/arch/x86/kernel/cpu/powerflags.c
@@ -21,4 +21,7 @@ const char *const x86_power_flags[32] = {
"eff_freq_ro", /* Readonly aperf/mperf */
"proc_feedback", /* processor feedback interface */
"acc_power", /* accumulated power mechanism */
+ "connected_standby", /* connected standby */
+ "rapl", /* running average power limit */
+ "fast_cppc", /* fast collaborative processor performance control */
};

---
base-commit: df60cee26a2e3d937a319229e335cb3f9c1f16d2
change-id: 20231128-powerflags-8ce66a96f6f3

Best regards,
--
Thomas Weißschuh <[email protected]>


2023-11-28 19:13:05

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86/cpu: Update power flags

On Tue, Nov 28, 2023 at 07:54:54PM +0100, Thomas Weißschuh wrote:
> As described on page 99 of
> "Processing Programming Reference (PPR) for AMD Family 19h Model 61h, Revision B1 Processor".
> (AMD Documentation Hub Document 56713)
>
> Tested on an "AMD Ryzen 7 7840U w/ Radeon 780M Graphics".
>
> Signed-off-by: Thomas Weißschuh <[email protected]>
> ---
> arch/x86/kernel/cpu/powerflags.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/powerflags.c b/arch/x86/kernel/cpu/powerflags.c
> index fd6ec2aa0303..0c98405aeae2 100644
> --- a/arch/x86/kernel/cpu/powerflags.c
> +++ b/arch/x86/kernel/cpu/powerflags.c
> @@ -21,4 +21,7 @@ const char *const x86_power_flags[32] = {
> "eff_freq_ro", /* Readonly aperf/mperf */
> "proc_feedback", /* processor feedback interface */
> "acc_power", /* accumulated power mechanism */
> + "connected_standby", /* connected standby */
> + "rapl", /* running average power limit */
> + "fast_cppc", /* fast collaborative processor performance control */

No need.

The beginning of Documentation/arch/x86/cpuinfo.rst tries to explain
why.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-11-28 19:26:24

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH] x86/cpu: Update power flags

On 2023-11-28 20:12:17+0100, Borislav Petkov wrote:
> On Tue, Nov 28, 2023 at 07:54:54PM +0100, Thomas Weißschuh wrote:
> > As described on page 99 of
> > "Processing Programming Reference (PPR) for AMD Family 19h Model 61h, Revision B1 Processor".
> > (AMD Documentation Hub Document 56713)
> >
> > Tested on an "AMD Ryzen 7 7840U w/ Radeon 780M Graphics".
> >
> > Signed-off-by: Thomas Weißschuh <[email protected]>
> > ---
> > arch/x86/kernel/cpu/powerflags.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/powerflags.c b/arch/x86/kernel/cpu/powerflags.c
> > index fd6ec2aa0303..0c98405aeae2 100644
> > --- a/arch/x86/kernel/cpu/powerflags.c
> > +++ b/arch/x86/kernel/cpu/powerflags.c
> > @@ -21,4 +21,7 @@ const char *const x86_power_flags[32] = {
> > "eff_freq_ro", /* Readonly aperf/mperf */
> > "proc_feedback", /* processor feedback interface */
> > "acc_power", /* accumulated power mechanism */
> > + "connected_standby", /* connected standby */
> > + "rapl", /* running average power limit */
> > + "fast_cppc", /* fast collaborative processor performance control */
>
> No need.
>
> The beginning of Documentation/arch/x86/cpuinfo.rst tries to explain
> why.

Isn't this introduction more about the cpuinfo "flags" fields?
These power management flags have their own field "power management".

Without the patch it looks like this on my machine in cpuinfo:

power management: ts ttp tm hwpstate cpb eff_freq_ro [13] [14] [15]

So they are already reported, but only their numeric value.

Anyways, if it's not correct to have them then let's drop the patch.


Thomas

2023-11-28 19:37:25

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86/cpu: Update power flags

On Tue, Nov 28, 2023 at 08:25:40PM +0100, Thomas Weißschuh wrote:
> > The beginning of Documentation/arch/x86/cpuinfo.rst tries to explain
> > why.
>
> Isn't this introduction more about the cpuinfo "flags" fields?

That's why I said "tries to explain". See below - I've been meaning to
write this for a long time now.

> power management: ts ttp tm hwpstate cpb eff_freq_ro [13] [14] [15]

Yeah, nothing cares about those - see below.

Thx.

---
diff --git a/Documentation/arch/x86/cpuinfo.rst b/Documentation/arch/x86/cpuinfo.rst
index 08246e8ac835..95c7ebcc13a9 100644
--- a/Documentation/arch/x86/cpuinfo.rst
+++ b/Documentation/arch/x86/cpuinfo.rst
@@ -13,6 +13,29 @@ or KVM want to expose the feature to a KVM guest, it can and should have
an X86_FEATURE_* defined. These flags represent hardware features as
well as software features.

+The /proc/cpuinfo list is not exhaustive and represents an ill-fated
+attempt from long time ago to put feature flags in an easy to find place
+for userspace. However,
+
+* the amount of feature flags is growing by the CPU generation, leading
+to unparseable and unwieldy /proc/cpuinfo
+
+* what is more, those feature flags even need to be in that file because
+userspace doesn't even care about them - glibc et al already use CPUID
+to find out what the target machine supports and what not. And even if
+it doesn't do that and the hw supports CPUID faulting, userspace can
+simply probe for the feature and figure out if it is supported or not
+
+* furthermore, those flag strings become an ABI the moment they appear
+there and maintaining them forever when nothing even uses them is a lot
+of wasted effort
+
+So, the current use of /proc/cpuinfo is to show features which the
+kernel has *enabled* and supports. As in: the CPUID feature flag is
+there, there's an additional setup which the kernel has done while
+booting and the functionality is there and ready to use. A perfect
+example for that is "user_shstk".
+
If users want to know if a feature is available on a given system, they
try to find the flag in /proc/cpuinfo. If a given flag is present, it
means that the kernel supports it and is currently making it available.


--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette