tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

[PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

Posted by Calvin Owens 3 months, 4 weeks ago

Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
turbostat can display temperatures when running on kernels compiled
without multiuser support.

Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
 tools/power/x86/turbostat/turbostat.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 925556b90770..f7d665913a52 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
 	int ret = 0;
 
 	caps = cap_get_proc();
-	if (caps == NULL)
+	if (caps == NULL) {
+		/* Support CONFIG_MULTIUSER=n */
+		if (errno == ENOSYS)
+			return 0;
+
 		return 1;
+	}
 
 	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE, &cap_flag_value)) {
 		ret = 1;
-- 
2.47.2

Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

Posted by Zhang, Rui 3 months, 3 weeks ago

On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> turbostat can display temperatures when running on kernels compiled
> without multiuser support.
> 
> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> ---
>  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/power/x86/turbostat/turbostat.c
> b/tools/power/x86/turbostat/turbostat.c
> index 925556b90770..f7d665913a52 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
>  	int ret = 0;
>  
>  	caps = cap_get_proc();
> -	if (caps == NULL)
> +	if (caps == NULL) {
> +		/* Support CONFIG_MULTIUSER=n */
> +		if (errno == ENOSYS)

Can you point me where this knowledge comes from?

I downloaded the libcap source and didn't see how ENOSYS is set.

thanks,
rui
> +			return 0;
> +
>  		return 1;
> +	}
>  
>  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> &cap_flag_value)) {
>  		ret = 1;

Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

Posted by Calvin Owens 3 months, 3 weeks ago

On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > turbostat can display temperatures when running on kernels compiled
> > without multiuser support.
> > 
> > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > ---
> >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/power/x86/turbostat/turbostat.c
> > b/tools/power/x86/turbostat/turbostat.c
> > index 925556b90770..f7d665913a52 100644
> > --- a/tools/power/x86/turbostat/turbostat.c
> > +++ b/tools/power/x86/turbostat/turbostat.c
> > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> >  	int ret = 0;
> >  
> >  	caps = cap_get_proc();
> > -	if (caps == NULL)
> > +	if (caps == NULL) {
> > +		/* Support CONFIG_MULTIUSER=n */
> > +		if (errno == ENOSYS)
> 
> Can you point me where this knowledge comes from?
> 
> I downloaded the libcap source and didn't see how ENOSYS is set.

Hi Rui,

When the kernel is built without multiuser support, the capget() et al.
syscalls are #ifdef'd out:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32

...so userspace will get -ENOSYS when it tries to call them, and that
ends up being propagated to errno in userspace.

Admittedly it is sort of implicit. Maybe a better way to "fix" this
would be to warn the user if the capability check fails, but still
attempt to access the MSR devices? I can do that if you prefer.

That is my only problem here: when check_for_cap_sys_rawio() fails, the
current code doesn't attempt to access the MSR devices at all, even
though in my case it would actually work.

I realize this is very weird: it came up when I was recently including
turbostat as part of an extremely tiny bootable utility image.

Thanks,
Calvin

> thanks,
> rui
> > +			return 0;
> > +
> >  		return 1;
> > +	}
> >  
> >  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > &cap_flag_value)) {
> >  		ret = 1;
>

Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

Posted by Zhang, Rui 3 months, 3 weeks ago

On Sun, 2025-06-15 at 21:38 -0700, Calvin Owens wrote:
> On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> > On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > > turbostat can display temperatures when running on kernels compiled
> > > without multiuser support.
> > > 
> > > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > > ---
> > >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/power/x86/turbostat/turbostat.c
> > > b/tools/power/x86/turbostat/turbostat.c
> > > index 925556b90770..f7d665913a52 100644
> > > --- a/tools/power/x86/turbostat/turbostat.c
> > > +++ b/tools/power/x86/turbostat/turbostat.c
> > > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> > >  	int ret = 0;
> > >  
> > >  	caps = cap_get_proc();
> > > -	if (caps == NULL)
> > > +	if (caps == NULL) {
> > > +		/* Support CONFIG_MULTIUSER=n */
> > > +		if (errno == ENOSYS)
> > 
> > Can you point me where this knowledge comes from?
> > 
> > I downloaded the libcap source and didn't see how ENOSYS is set.
> 
> Hi Rui,
> 
> When the kernel is built without multiuser support, the capget() et al.
> syscalls are #ifdef'd out:
> 
>    
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32
> 
> ...so userspace will get -ENOSYS when it tries to call them, and that
> ends up being propagated to errno in userspace.
> 
> Admittedly it is sort of implicit. Maybe a better way to "fix" this
> would be to warn the user if the capability check fails, but still
> attempt to access the MSR devices? I can do that if you prefer.

if cap_get_proc returns -ENOSYS only when CONFIG_MULTIUSER=n, then I
think the patch is good, maybe we just need some more detailed comment.

and I don't think we should continue when the capability check fails in
general.

> 
> That is my only problem here: when check_for_cap_sys_rawio() fails, the
> current code doesn't attempt to access the MSR devices at all, even
> though in my case it would actually work.

Or maybe we can cover this with the "--force" parameter?
say, does it make sense to address the problem by running "turbostat --
force"?
I'll leave this question to Len.

thanks,
rui
> 
> I realize this is very weird: it came up when I was recently including
> turbostat as part of an extremely tiny bootable utility image.
> 
> Thanks,
> Calvin
> 
> > thanks,
> > rui
> > > +			return 0;
> > > +
> > >  		return 1;
> > > +	}
> > >  
> > >  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > > &cap_flag_value)) {
> > >  		ret = 1;
> >

Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

Posted by Len Brown 3 months, 3 weeks ago

Thanks for the patch, Calvin, I think it is correct.

Rui,
I'll tweak the comment to explain that the system call doesn't always exist.

I think it is correct to continue on -- the rawio check is basically a
graceful way
to prevent an error in accessing MSRs later.  If we don't have that
graceful mechanism,
then we will or will not hit the access error later, and that's okay.

Interesting configuration you have there Calvin.

thanks,
-Len


On Tue, Jun 17, 2025 at 11:28 PM Zhang, Rui <rui.zhang@intel.com> wrote:
>
> On Sun, 2025-06-15 at 21:38 -0700, Calvin Owens wrote:
> > On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> > > On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > > > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > > > turbostat can display temperatures when running on kernels compiled
> > > > without multiuser support.
> > > >
> > > > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > > > ---
> > > >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/power/x86/turbostat/turbostat.c
> > > > b/tools/power/x86/turbostat/turbostat.c
> > > > index 925556b90770..f7d665913a52 100644
> > > > --- a/tools/power/x86/turbostat/turbostat.c
> > > > +++ b/tools/power/x86/turbostat/turbostat.c
> > > > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> > > >   int ret = 0;
> > > >
> > > >   caps = cap_get_proc();
> > > > - if (caps == NULL)
> > > > + if (caps == NULL) {
> > > > +         /* Support CONFIG_MULTIUSER=n */
> > > > +         if (errno == ENOSYS)
> > >
> > > Can you point me where this knowledge comes from?
> > >
> > > I downloaded the libcap source and didn't see how ENOSYS is set.
> >
> > Hi Rui,
> >
> > When the kernel is built without multiuser support, the capget() et al.
> > syscalls are #ifdef'd out:
> >
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32
> >
> > ...so userspace will get -ENOSYS when it tries to call them, and that
> > ends up being propagated to errno in userspace.
> >
> > Admittedly it is sort of implicit. Maybe a better way to "fix" this
> > would be to warn the user if the capability check fails, but still
> > attempt to access the MSR devices? I can do that if you prefer.
>
> if cap_get_proc returns -ENOSYS only when CONFIG_MULTIUSER=n, then I
> think the patch is good, maybe we just need some more detailed comment.
>
> and I don't think we should continue when the capability check fails in
> general.
>
> >
> > That is my only problem here: when check_for_cap_sys_rawio() fails, the
> > current code doesn't attempt to access the MSR devices at all, even
> > though in my case it would actually work.
>
> Or maybe we can cover this with the "--force" parameter?
> say, does it make sense to address the problem by running "turbostat --
> force"?
> I'll leave this question to Len.
>
> thanks,
> rui
> >
> > I realize this is very weird: it came up when I was recently including
> > turbostat as part of an extremely tiny bootable utility image.
> >
> > Thanks,
> > Calvin
> >
> > > thanks,
> > > rui
> > > > +                 return 0;
> > > > +
> > > >           return 1;
> > > > + }
> > > >
> > > >   if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > > > &cap_flag_value)) {
> > > >           ret = 1;
> > >
>


-- 
Len Brown, Intel Open Source Technology Center

Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

Posted by Calvin Owens 3 months, 3 weeks ago

On Sunday 06/15 at 21:38 -0700, Calvin Owens wrote:
> On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> > On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > > turbostat can display temperatures when running on kernels compiled
> > > without multiuser support.
> > > 
> > > Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> > > ---
> > >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/power/x86/turbostat/turbostat.c
> > > b/tools/power/x86/turbostat/turbostat.c
> > > index 925556b90770..f7d665913a52 100644
> > > --- a/tools/power/x86/turbostat/turbostat.c
> > > +++ b/tools/power/x86/turbostat/turbostat.c
> > > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> > >  	int ret = 0;
> > >  
> > >  	caps = cap_get_proc();
> > > -	if (caps == NULL)
> > > +	if (caps == NULL) {
> > > +		/* Support CONFIG_MULTIUSER=n */
> > > +		if (errno == ENOSYS)
> > 
> > Can you point me where this knowledge comes from?
> > 
> > I downloaded the libcap source and didn't see how ENOSYS is set.
> 
> Hi Rui,
> 
> When the kernel is built without multiuser support, the capget() et al.
> syscalls are #ifdef'd out:
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32
> 
> ...so userspace will get -ENOSYS when it tries to call them, and that
> ends up being propagated to errno in userspace.

Sorry, what I meant was: the kernel syscall returns -ENOSYS to
userspace, and the syscall() wrapper (in userspace) assigns the positive
error value to errno, where it remains after cap_get_proc() returns.

> Admittedly it is sort of implicit. Maybe a better way to "fix" this
> would be to warn the user if the capability check fails, but still
> attempt to access the MSR devices? I can do that if you prefer.
> 
> That is my only problem here: when check_for_cap_sys_rawio() fails, the
> current code doesn't attempt to access the MSR devices at all, even
> though in my case it would actually work.
> 
> I realize this is very weird: it came up when I was recently including
> turbostat as part of an extremely tiny bootable utility image.
> 
> Thanks,
> Calvin
> 
> > thanks,
> > rui
> > > +			return 0;
> > > +
> > >  		return 1;
> > > +	}
> > >  
> > >  	if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > > &cap_flag_value)) {
> > >  		ret = 1;
> >