Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/2] cpupower: fix amd cpu (family < 0x17) active state issue | From | xufuhai <> | Date | Mon, 29 Mar 2021 11:51:18 +0800 |
| |
yeah Shuah~ thanks for your reply
For this issue, not meaning "current CPU frequency" but "boost state support--->Active" during "cpupower frequency-info" command as below:
boost state support: Supported: yes Active: yes/no
I think the state returned from the command for amd cpu (family < 0x17) should be like as below:
as non-root Active state: Active: Error while evaluating Boost Capabilities on CPU xx -- are you root?
as root Active state: Active: yes (if supported) no (if not supprted)
I don't wanna see the state returned like below:
as non-root Active state: Active: yes
as root Active state: Active: yes (if supported) no (if not supprted)
I will paste the related code by detailed for showing the issue:
if amd cpu family < 0x17 , will run amd_pci_get_num_boost_states function: ----------------------------------------------------- /linux/tools/power/cpupower/utils/helper/misc.c if (cpupower_cpu_info.caps & CPUPOWER_CAP_AMD_CPB) { *support = 1;
/* AMD Family 0x17 does not utilize PCI D18F4 like prior * families and has no fixed discrete boost states but * has Hardware determined variable increments instead. */
if (cpupower_cpu_info.caps & CPUPOWER_CAP_AMD_CPB_MSR) { if (!read_msr(cpu, MSR_AMD_HWCR, &val)) { if (!(val & CPUPOWER_AMD_CPBDIS)) *active = 1; } } else { ret = amd_pci_get_num_boost_states(active, states); if (ret) return ret; } ---------------------------------------------------
/linux/tools/power/cpupower/utils/helper/amd.c amd_pci_get_num_boost_states: val = pci_read_byte(device, 0x15c);
if (val & 3) *active = 1; else ----------------------------------------------------
pci_read_byte will memset val to 0xff if caller has no permission to access to read from pci_dev but for amd_pci_get_num_boost_states, active state will set 1 meaning "yes". I believe that active state should return negative value to caller function meaning "have no permission" will showing " Error while evaluating Boost Capabilities on CPU xx -- are you root?"
---------------------------------------------------- static inline void pci_read_data(struct pci_dev *d, void *buf, int pos, int len) { if (pos & (len-1)) d->access->error("Unaligned read: pos=%02x, len=%d", pos, len); if (pos + len <= d->cache_len) memcpy(buf, d->cache + pos, len); else if (!d->methods->read(d, pos, buf, len)) memset(buf, 0xff, len); }
byte pci_read_byte(struct pci_dev *d, int pos) { byte buf; pci_read_data(d, &buf, pos, 1); return buf; } ----------------------------------------------------
在 2021/3/24 下午6:27, xufuhai 写道: > From: xufuhai <xufuhai@kuaishou.com> > > For the old AMD processor (family < 0x17), cpupower will call the > amd_pci_get_num_boost_states function, but for the non-root user > pci_read_byte function (implementation comes from the psutil library), > val will be set to 0xff, indicating that there is no read function > callback. At this time, the original logic will set the cpupower turbo > active state to yes. This is an obvious issue~ > > Reproduce procedure: > cpupower frequency-info > > Signed-off-by: xufuhai <xufuhai@kuaishou.com> > Signed-off-by: chenguanqiao <chenguanqiao@kuaishou.com> > Signed-off-by: lishujin <lishujin@kuaishou.com> > --- > tools/power/cpupower/utils/helpers/amd.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/tools/power/cpupower/utils/helpers/amd.c b/tools/power/cpupower/utils/helpers/amd.c > index 97f2c857048e..6f9504906afa 100644 > --- a/tools/power/cpupower/utils/helpers/amd.c > +++ b/tools/power/cpupower/utils/helpers/amd.c > @@ -137,6 +137,13 @@ int amd_pci_get_num_boost_states(int *active, int *states) > return -ENODEV; > > val = pci_read_byte(device, 0x15c); > + > + /* If val is 0xff, meaning has no permisson to > + * get the boost states, return -1 > + */ > + if (val == 0xff) > + return -1; > + > if (val & 3) > *active = 1; > else >
| |