Skip to content

Conversation

@jiayingbao
Copy link

backport update and fix:
Support highest performance change interrupt
Allow model specific EPPs and set balanced EPP for EMR and SPR
other update

Test:
intel_pstate driver works as expected and also cpupower and turbostat to check frequency info

zhaogogyi and others added 25 commits August 29, 2024 11:24
enable pull request checking workflow for 6.6-velinux branch
commit 37b6ddb upstream.

Setting global turbo flag based on CPU 0 P-state limits is problematic
as it limits max P-state request on every CPU on the system just based
on its P-state limits.

There are two cases in which global.turbo_disabled flag is set:
- When the MSR_IA32_MISC_ENABLE_TURBO_DISABLE bit is set to 1
in the MSR MSR_IA32_MISC_ENABLE. This bit can be only changed by
the system BIOS before power up.
- When the max non turbo P-state is same as max turbo P-state for CPU 0.

The second check is not a valid to decide global turbo state based on
the CPU 0. CPU 0 max turbo P-state can be same as max non turbo P-state,
but for other CPUs this may not be true.

There is no guarantee that max P-state limits are same for every CPU. This
is possible that during fusing max P-state for a CPU is constrained. Also
with the Intel Speed Select performance profile, CPU 0 may not be present
in all profiles. In this case the max non turbo and turbo P-state can be
set to the lowest possible P-state by the hardware when switched to
such profile. Since max non turbo and turbo P-state is same,
global.turbo_disabled flag will be set.

Once global.turbo_disabled is set, any scaling max and min frequency
update for any CPU will result in its max P-state constrained to the max
non turbo P-state.

Hence remove the check of max non turbo P-state equal to max turbo P-state
of CPU 0 to set global turbo disabled flag.

Intel-SIG: commit 37b6ddb cpufreq: intel_pstate: Revise global turbo disable check.
Backport intel_pstate driver update for 6.6

Signed-off-by: Srinivas Pandruvada <[email protected]>
[ rjw: Subject edit ]
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
…ce EPP

commit 2719675 upstream.

The platform firmware can provide a balance performance EPP value by
enabling HWP and programming the EPP to the desired value.

However, currently this only takes effect for processors listed in
intel_epp_balance_perf[], so in order to enable a new processor model
to utilize this mechanism, that table needs to be updated.  It arguably
should not be necessary to modify the kernel to work properly with
every new generation of processors, though, and distributions that don't
always ship the most recent kernels should be able to run reasonably well
on new hardware without code changes.

For this reason, move the check to avoid updating the EPP when the balance
performance EPP is unmodified from the power-up default of 0x80 after the
check that allows the firmware-provided balance performance EPP value to
be retrieved.  This will cause the code to always look for the firmware-
provided value before consulting intel_epp_balance_perf[] and the handling
of new hardware will not depend on whether or not that thable has been
updated yet.

Intel-SIG: commit 2719675 cpufreq: intel_pstate: Prioritize firmware-provided balance performance EPP.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Srinivas Pandruvada <[email protected]>
[ rjw: Subject and changelog edits ]
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit e950131 upstream.

Users may disable HWP in firmware, in which case intel_pstate will give up
unless the CPU model is explicitly supported.

See also the following past commits:

 - commit df51f28 ("cpufreq: intel_pstate: Add Sapphire Rapids support
   in no-HWP mode")
 - commit d8de7a4 ("cpufreq: intel_pstate: Add Skylake servers support")
 - commit 706c532 ("cpufreq: intel_pstate: Add Cometlake support in
   no-HWP mode")
 - commit fbdc21e ("cpufreq: intel_pstate: Add Icelake servers support in
   no-HWP mode")
 - commit 71bb5c8 ("cpufreq: intel_pstate: Add Tigerlake support in
   no-HWP mode")

Intel-SIG: commit e950131 cpufreq: intel_pstate: Add Emerald Rapids support in no-HWP
mode.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Zhenguo Yao <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
[ rjw: Changelog edits ]
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 4615ac9 upstream.

Commit 09c448d ("cpufreq: intel_pstate: Use IOWAIT flag in Atom
algorithm") removed the last user of cpudata::prev_cummulative_iowait.
Remove the member too.

Found by https://github.com/jirislaby/clang-struct.

Intel-SIG: commit 4615ac9 cpufreq: intel_pstate: remove cpudata.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Jiri Slaby (SUSE) <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 240a8da upstream.

The current implementation allows model specific EPP override for
balanced_performance. Add feature to allow model specific EPP for all
predefined EPP strings. For example for some CPU models, even changing
performance EPP has benefits

Use a mask of EPPs as driver_data instead of just balanced_performance.

Intel-SIG: commit 240a8da cpufreq: intel_pstate: Allow model specific EPPs.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Srinivas Pandruvada <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
…r_cleanup()

commit f186b2d upstream.

Remove the spinlock locking from intel_pstate_driver_cleanup() as it is
not necessary because no other code accessing all_cpu_data[] can run in
parallel with that function.

Had the locking been necessary, though, it would have been incorrect
because the lock in question is acquired from a hardirq handler and
it cannot be acquired from thread context without disabling interrupts.

Intel-SIG: commit f186b2d cpufreq: intel_pstate: Drop redundant locking from.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 12ebba4 upstream.

Because intel_pstate_enable/disable_hwp_interrupt() are only called from
thread context, they need not save the IRQ flags when using a spinlock
as interrupts are guaranteed to be enabled when they run, so make them
use spin_lock/unlock_irq().

Intel-SIG: commit 12ebba4 cpufreq: intel_pstate: Simplify spinlock locking.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 432acb2 upstream.

Make intel_pstate_disable_hwp_interrupt() wait for canceled delayed work
to complete to avoid leftover work items running when it returns which
may be during driver unregistration and may confuse things going forward.

Intel-SIG: commit 432acb2 cpufreq: intel_pstate: Wait for canceled delayed work to complete.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 0f2828e upstream.

Drop two redundant checks involving READ_ONCE() from notify_hwp_interrupt()
and make it check hwp_active without READ_ONCE() which is not necessary,
because that variable is only set once during the early initialization of
the driver.

In order to make that clear, annotate hwp_active with __ro_after_init.

Intel-SIG: commit 0f2828e cpufreq: intel_pstate: Get rid of unnecessary READ_ONCE() annotations.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit e97a982 upstream.

There are at least 3 variables in intel_pstate that do not get updated
after they have been initialized, so annotate them with __ro_after_init.

Intel-SIG: commit e97a982 cpufreq: intel_pstate: Use __ro_after_init for three variables.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 032c556 upstream.

Fold intel_pstate_max_within_limits() into its only caller.

No functional impact.

Intel-SIG: commit 032c556 cpufreq: intel_pstate: Fold intel_pstate_max_within_limits() into caller.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
…ialization

commit 0940f1a upstream.

The global.turbo_disabled is updated quite often, especially in the
passive mode in which case it is updated every time the scheduler calls
into the driver.  However, this is generally not necessary and it adds
MSR read overhead to scheduler code paths (and that particular MSR is
slow to read).

For this reason, make the driver read MSR_IA32_MISC_ENABLE_TURBO_DISABLE
just once at the cpufreq driver registration time and remove all of the
in-flight updates of global.turbo_disabled.

Intel-SIG: commit 0940f1a cpufreq: intel_pstate: Do not update global.turbo_disabled.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit c626a43 upstream.

Now that global.turbo_disabled can only change at the cpufreq driver
registration time, initialize global.no_turbo at that time too so they
are in sync to start with (if the former is set, the latter cannot be
updated later anyway).

That allows show_no_turbo() to be simlified because it does not need
to check global.turbo_disabled and store_no_turbo() can be rearranged
to avoid doing anything if the new value of global.no_turbo is equal
to the current one and only return an error on attempts to clear
global.no_turbo when global.turbo_disabled.

While at it, eliminate the redundant ret variable from store_no_turbo().

No intentional functional impact.

Intel-SIG: commit c626a43 cpufreq: intel_pstate: Rearrange show_no_turbo() and store_no_turbo().
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 9558fae upstream.

Because global.no_turbo is generally not read under intel_pstate_driver_lock
make store_no_turbo() use WRITE_ONCE() for updating it (this is the only
place at which it is updated except for the initialization) and make the
majority of places reading it use READ_ONCE().

Also remove redundant global.turbo_disabled checks from places that
depend on the 'true' value of global.no_turbo because it can only be
'true' if global.turbo_disabled is also 'true'.

Intel-SIG: commit 9558fae cpufreq: intel_pstate: Read global.no_turbo under READ_ONCE().
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit f32587d upstream.

Replace the global.turbo_disabled in __intel_pstate_update_max_freq() with
a global.no_turbo one to make store_no_turbo() actually update the maximum
CPU frequency on the trubo preference changes, which needs to be consistent
with arch_set_max_freq_ratio() called from there.

For more consistency, replace the global.turbo_disabled checks in
__intel_pstate_cpu_init() and intel_cpufreq_adjust_perf() with
global.no_turbo checks either.

Intel-SIG: commit f32587d cpufreq: intel_pstate: Replace three global.turbo_disabled checks.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit e8217b4 upstream.

There are 3 places at which the maximum CPU frequency may change,
store_no_turbo(), intel_pstate_update_limits() (when called by the
cpufreq core) and intel_pstate_notify_work() (when handling a HWP
change notification).  Currently, cpuinfo.max_freq is only updated by
store_no_turbo() and intel_pstate_notify_work(), although it principle
it may be necessary to update it in intel_pstate_update_limits() either.

Make all of them mutually consistent.

Intel-SIG: commit e8217b4 cpufreq: intel_pstate: Update the maximum CPU frequency consistently.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 8c55654 upstream.

The reference to this variable is hidden in an #ifdef:

drivers/cpufreq/intel_pstate.c:2440:32: error: 'intel_pstate_cpu_oob_ids' defined but not used [-Werror=unused-const-variable=]

Use the same check around the definition.

Intel-SIG: commit 8c55654 cpufreq: intel_pstate: hide unused intel_pstate_cpu_oob_ids[].
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 0a206fe upstream.

make C=1 currently gives the following warning:

drivers/cpufreq/intel_pstate.c:262: warning: Function parameter or struct member 'epp_cached' not described in 'cpudata'

Add the missing ":" to fix the trivial kernel-doc syntax error.

Intel-SIG: commit 0a206fe cpufreq: intel_pstate: fix struct cpudata::epp_cached kernel-doc.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Jeff Johnson <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 1e24c31 upstream.

Fix unchecked MSR access error for processors with no HWP support. On
such processors, maximum frequency can be changed by the system firmware
using ACPI event ACPI_PROCESSOR_NOTIFY_HIGEST_PERF_CHANGED. This results
in accessing HWP MSR 0x771.

Call Trace:
	<TASK>
	generic_exec_single+0x58/0x120
	smp_call_function_single+0xbf/0x110
	rdmsrl_on_cpu+0x46/0x60
	intel_pstate_get_hwp_cap+0x1b/0x70
	intel_pstate_update_limits+0x2a/0x60
	acpi_processor_notify+0xb7/0x140
	acpi_ev_notify_dispatch+0x3b/0x60

HWP MSR 0x771 can be only read on a CPU which supports HWP and enabled.
Hence intel_pstate_get_hwp_cap() can only be called when hwp_active is
true.

Intel-SIG: commit 1e24c31 cpufreq: intel_pstate: Fix unchecked HWP MSR access.
Backport intel_pstate driver update for 6.6 from 6.11

Reported-by: Sebastian Andrzej Siewior <[email protected]>
Closes: https://lore.kernel.org/linux-pm/[email protected]/
Fixes: e8217b4 ("cpufreq: intel_pstate: Update the maximum CPU frequency consistently")
Tested-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Srinivas Pandruvada <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 350cbb5 upstream.

After recent changes in intel_pstate, global.turbo_disabled is only set
at the initialization time and never changed.  However, it turns out
that on some systems the "turbo disabled" bit in MSR_IA32_MISC_ENABLE,
the initial state of which is reflected by global.turbo_disabled, can be
flipped later and there should be a way to take that into account (other
than checking that MSR every time the driver runs which is costly and
useless overhead on the vast majority of systems).

For this purpose, notice that before the changes in question,
store_no_turbo() contained a turbo_is_disabled() check that was used
for updating global.turbo_disabled if the "turbo disabled" bit in
MSR_IA32_MISC_ENABLE had been flipped and that functionality can be
restored.  Then, users will be able to reset global.turbo_disabled
by writing 0 to no_turbo which used to work before on systems with
flipping "turbo disabled" bit.

This guarantees the driver state to remain in sync, but READ_ONCE()
annotations need to be added in two places where global.turbo_disabled
is accessed locklessly, so modify the driver to make that happen.

Intel-SIG: commit 350cbb5 cpufreq: intel_pstate: Check turbo_is_disabled() in store_no_turbo().
Backport intel_pstate driver update for 6.6 from 6.11

Fixes: 0940f1a ("cpufreq: intel_pstate: Do not update global.turbo_disabled after initialization")
Closes: https://lore.kernel.org/linux-pm/[email protected]
Suggested-by: Srinivas Pandruvada <[email protected]>
Reported-by: Xi Ruoyao <[email protected]>
Tested-by: Xi Ruoyao <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit acfc429 upstream.

Replace boot_cpu_has() with cpu_feature_enabled().

Intel-SIG: commit acfc429 cpufreq: intel_pstate: Replace boot_cpu_has().
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Srinivas Pandruvada <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 7ea8193 upstream.

When CPUID[6].EAX[15] is set to 1, this CPU supports notification for
HWP (Hardware P-states) highest performance change.

Add a feature flag to check if the CPU supports HWP highest performance
change.

Intel-SIG: commit 7ea8193 x86/cpufeatures: Add HWP highest perf change feature flag.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Srinivas Pandruvada <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit d845cd9 upstream.

On some systems, the HWP (Hardware P-states) highest performance level
can change from the value set at boot-up. This behavior can lead to two
issues:

- The 'cpuinfo_max_freq' within the 'cpufreq' sysfs will not reflect
the CPU's highest achievable performance.
- Even if the CPU's highest performance level is increased after booting,
the CPU may not reach the full expected performance.

The availability of this feature is indicated by the CPUID instruction:
if CPUID[6].EAX[15] is set to 1, the feature is supported. When supported,
setting bit 2 of the MSR_HWP_INTERRUPT register enables notifications of
the highest performance level changes. Therefore, as part of enabling the
HWP interrupt, bit 2 of the MSR_HWP_INTERRUPT should also be set when this
feature is supported.

Upon a change in the highest performance level, a new HWP interrupt is
generated, with bit 3 of the MSR_HWP_STATUS register set, and the
MSR_HWP_CAPABILITIES register is updated with the new highest performance
limit.

The processing of the interrupt is the same as the guaranteed performance
change. Notify change to cpufreq core and update MSR_HWP_REQUEST with new
performance limits.

The current driver implementation already takes care of the highest
performance change as part of:
commit dfeeedc ("cpufreq: intel_pstate: Update cpuinfo.max_freq
on HWP_CAP changes")

For example:
Before highest performance change interrupt:
cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
3700000
cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq
3700000

After highest performance changes interrupt:
cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq
3900000
cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
3900000

Intel-SIG: commit d845cd9 cpufreq: intel_pstate: Support highest performance change interrupt.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Srinivas Pandruvada <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
commit 64a66f4 upstream.

On Intel Emerald Rapids machines, we ship the Energy Performance Preference
(EPP) default for balance_performance as 128. However, during an internal
investigation together with Intel, we have determined that 32 is a more
suitable value. This leads to significant improvements in both performance
and energy:

POV-Ray: 32% faster | 12% less energy
OpenSSL: 12% faster | energy within 1%
Build Linux Kernel: 29% faster | 18% less energy

Therefore, we should move the default EPP for balance_performance to 32.
This is in line with what has already been done for Sapphire Rapids.

Intel-SIG: commit 64a66f4 cpufreq: intel_pstate: Update Balance performance EPP for Emerald Rapids.
Backport intel_pstate driver update for 6.6 from 6.11

Signed-off-by: Pedro Henrique Kopper <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
Link: https://patch.msgid.link/Zqu6zjVMoiXwROBI@capivara
Signed-off-by: Rafael J. Wysocki <[email protected]>
[ Yingbao Jia: amend commit log ]
Signed-off-by: Yingbao Jia <[email protected]>
@kchuyizhou
Copy link

LGTM.

@guojinhui-liam guojinhui-liam merged commit 7ae82fd into openvelinux:6.6-velinux Dec 16, 2024
2 of 3 checks passed
guojinhui-liam added a commit that referenced this pull request Dec 16, 2024
PvsNarasimha pushed a commit to PvsNarasimha/kernel that referenced this pull request Dec 27, 2024
commit 99d4850 upstream

Found by leak sanitizer:
```
==1632594==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7f2953a7077b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
    openvelinux#1 0x556701d6fbbf in perf_env__read_cpuid util/env.c:369
    openvelinux#2 0x556701d70589 in perf_env__cpuid util/env.c:465
    openvelinux#3 0x55670204bba2 in x86__is_amd_cpu arch/x86/util/env.c:14
    #4 0x5567020487a2 in arch__post_evsel_config arch/x86/util/evsel.c:83
    #5 0x556701d8f78b in evsel__config util/evsel.c:1366
    openvelinux#6 0x556701ef5872 in evlist__config util/record.c:108
    openvelinux#7 0x556701cd6bcd in test__PERF_RECORD tests/perf-record.c:112
    openvelinux#8 0x556701cacd07 in run_test tests/builtin-test.c:236
    openvelinux#9 0x556701cacfac in test_and_print tests/builtin-test.c:265
    openvelinux#10 0x556701cadddb in __cmd_test tests/builtin-test.c:402
    openvelinux#11 0x556701caf2aa in cmd_test tests/builtin-test.c:559
    openvelinux#12 0x556701d3b557 in run_builtin tools/perf/perf.c:323
    openvelinux#13 0x556701d3bac8 in handle_internal_command tools/perf/perf.c:377
    openvelinux#14 0x556701d3be90 in run_argv tools/perf/perf.c:421
    openvelinux#15 0x556701d3c3f8 in main tools/perf/perf.c:537
    openvelinux#16 0x7f2952a46189 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).
```

Fixes: f7b58cb ("perf mem/c2c: Add load store event mappings for AMD")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Ravi Bangoria <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
PvsNarasimha pushed a commit to PvsNarasimha/kernel that referenced this pull request Jan 7, 2025
commit 99d4850 upstream

Found by leak sanitizer:
```
==1632594==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7f2953a7077b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
    openvelinux#1 0x556701d6fbbf in perf_env__read_cpuid util/env.c:369
    openvelinux#2 0x556701d70589 in perf_env__cpuid util/env.c:465
    openvelinux#3 0x55670204bba2 in x86__is_amd_cpu arch/x86/util/env.c:14
    #4 0x5567020487a2 in arch__post_evsel_config arch/x86/util/evsel.c:83
    #5 0x556701d8f78b in evsel__config util/evsel.c:1366
    openvelinux#6 0x556701ef5872 in evlist__config util/record.c:108
    openvelinux#7 0x556701cd6bcd in test__PERF_RECORD tests/perf-record.c:112
    openvelinux#8 0x556701cacd07 in run_test tests/builtin-test.c:236
    openvelinux#9 0x556701cacfac in test_and_print tests/builtin-test.c:265
    openvelinux#10 0x556701cadddb in __cmd_test tests/builtin-test.c:402
    openvelinux#11 0x556701caf2aa in cmd_test tests/builtin-test.c:559
    openvelinux#12 0x556701d3b557 in run_builtin tools/perf/perf.c:323
    openvelinux#13 0x556701d3bac8 in handle_internal_command tools/perf/perf.c:377
    openvelinux#14 0x556701d3be90 in run_argv tools/perf/perf.c:421
    openvelinux#15 0x556701d3c3f8 in main tools/perf/perf.c:537
    openvelinux#16 0x7f2952a46189 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).
```

Fixes: f7b58cb ("perf mem/c2c: Add load store event mappings for AMD")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Ravi Bangoria <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
PvsNarasimha pushed a commit to PvsNarasimha/kernel that referenced this pull request Jan 7, 2025
commit 99d4850 upstream

Found by leak sanitizer:
```
==1632594==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7f2953a7077b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
    openvelinux#1 0x556701d6fbbf in perf_env__read_cpuid util/env.c:369
    openvelinux#2 0x556701d70589 in perf_env__cpuid util/env.c:465
    openvelinux#3 0x55670204bba2 in x86__is_amd_cpu arch/x86/util/env.c:14
    #4 0x5567020487a2 in arch__post_evsel_config arch/x86/util/evsel.c:83
    #5 0x556701d8f78b in evsel__config util/evsel.c:1366
    openvelinux#6 0x556701ef5872 in evlist__config util/record.c:108
    openvelinux#7 0x556701cd6bcd in test__PERF_RECORD tests/perf-record.c:112
    openvelinux#8 0x556701cacd07 in run_test tests/builtin-test.c:236
    openvelinux#9 0x556701cacfac in test_and_print tests/builtin-test.c:265
    openvelinux#10 0x556701cadddb in __cmd_test tests/builtin-test.c:402
    openvelinux#11 0x556701caf2aa in cmd_test tests/builtin-test.c:559
    openvelinux#12 0x556701d3b557 in run_builtin tools/perf/perf.c:323
    openvelinux#13 0x556701d3bac8 in handle_internal_command tools/perf/perf.c:377
    openvelinux#14 0x556701d3be90 in run_argv tools/perf/perf.c:421
    openvelinux#15 0x556701d3c3f8 in main tools/perf/perf.c:537
    openvelinux#16 0x7f2952a46189 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).
```

Fixes: f7b58cb ("perf mem/c2c: Add load store event mappings for AMD")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Ravi Bangoria <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
PvsNarasimha pushed a commit to PvsNarasimha/kernel that referenced this pull request Jan 7, 2025
commit 99d4850 upstream

Found by leak sanitizer:
```
==1632594==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7f2953a7077b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
    openvelinux#1 0x556701d6fbbf in perf_env__read_cpuid util/env.c:369
    openvelinux#2 0x556701d70589 in perf_env__cpuid util/env.c:465
    openvelinux#3 0x55670204bba2 in x86__is_amd_cpu arch/x86/util/env.c:14
    #4 0x5567020487a2 in arch__post_evsel_config arch/x86/util/evsel.c:83
    #5 0x556701d8f78b in evsel__config util/evsel.c:1366
    openvelinux#6 0x556701ef5872 in evlist__config util/record.c:108
    openvelinux#7 0x556701cd6bcd in test__PERF_RECORD tests/perf-record.c:112
    openvelinux#8 0x556701cacd07 in run_test tests/builtin-test.c:236
    openvelinux#9 0x556701cacfac in test_and_print tests/builtin-test.c:265
    openvelinux#10 0x556701cadddb in __cmd_test tests/builtin-test.c:402
    openvelinux#11 0x556701caf2aa in cmd_test tests/builtin-test.c:559
    openvelinux#12 0x556701d3b557 in run_builtin tools/perf/perf.c:323
    openvelinux#13 0x556701d3bac8 in handle_internal_command tools/perf/perf.c:377
    openvelinux#14 0x556701d3be90 in run_argv tools/perf/perf.c:421
    openvelinux#15 0x556701d3c3f8 in main tools/perf/perf.c:537
    openvelinux#16 0x7f2952a46189 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).
```

Fixes: f7b58cb ("perf mem/c2c: Add load store event mappings for AMD")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Ravi Bangoria <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: PvsNarasimha <[email protected]>
PvsNarasimha pushed a commit to PvsNarasimha/kernel that referenced this pull request Jan 30, 2025
commit 99d4850 upstream

Found by leak sanitizer:
```
==1632594==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7f2953a7077b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
    openvelinux#1 0x556701d6fbbf in perf_env__read_cpuid util/env.c:369
    openvelinux#2 0x556701d70589 in perf_env__cpuid util/env.c:465
    openvelinux#3 0x55670204bba2 in x86__is_amd_cpu arch/x86/util/env.c:14
    #4 0x5567020487a2 in arch__post_evsel_config arch/x86/util/evsel.c:83
    #5 0x556701d8f78b in evsel__config util/evsel.c:1366
    openvelinux#6 0x556701ef5872 in evlist__config util/record.c:108
    openvelinux#7 0x556701cd6bcd in test__PERF_RECORD tests/perf-record.c:112
    openvelinux#8 0x556701cacd07 in run_test tests/builtin-test.c:236
    openvelinux#9 0x556701cacfac in test_and_print tests/builtin-test.c:265
    openvelinux#10 0x556701cadddb in __cmd_test tests/builtin-test.c:402
    openvelinux#11 0x556701caf2aa in cmd_test tests/builtin-test.c:559
    openvelinux#12 0x556701d3b557 in run_builtin tools/perf/perf.c:323
    openvelinux#13 0x556701d3bac8 in handle_internal_command tools/perf/perf.c:377
    openvelinux#14 0x556701d3be90 in run_argv tools/perf/perf.c:421
    openvelinux#15 0x556701d3c3f8 in main tools/perf/perf.c:537
    openvelinux#16 0x7f2952a46189 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).
```

Fixes: f7b58cb ("perf mem/c2c: Add load store event mappings for AMD")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Ravi Bangoria <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: PvsNarasimha <[email protected]>
PvsNarasimha pushed a commit to PvsNarasimha/kernel that referenced this pull request Feb 5, 2025
commit 99d4850 upstream.

Found by leak sanitizer:
```
==1632594==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7f2953a7077b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
    openvelinux#1 0x556701d6fbbf in perf_env__read_cpuid util/env.c:369
    openvelinux#2 0x556701d70589 in perf_env__cpuid util/env.c:465
    openvelinux#3 0x55670204bba2 in x86__is_amd_cpu arch/x86/util/env.c:14
    #4 0x5567020487a2 in arch__post_evsel_config arch/x86/util/evsel.c:83
    #5 0x556701d8f78b in evsel__config util/evsel.c:1366
    openvelinux#6 0x556701ef5872 in evlist__config util/record.c:108
    openvelinux#7 0x556701cd6bcd in test__PERF_RECORD tests/perf-record.c:112
    openvelinux#8 0x556701cacd07 in run_test tests/builtin-test.c:236
    openvelinux#9 0x556701cacfac in test_and_print tests/builtin-test.c:265
    openvelinux#10 0x556701cadddb in __cmd_test tests/builtin-test.c:402
    openvelinux#11 0x556701caf2aa in cmd_test tests/builtin-test.c:559
    openvelinux#12 0x556701d3b557 in run_builtin tools/perf/perf.c:323
    openvelinux#13 0x556701d3bac8 in handle_internal_command tools/perf/perf.c:377
    openvelinux#14 0x556701d3be90 in run_argv tools/perf/perf.c:421
    openvelinux#15 0x556701d3c3f8 in main tools/perf/perf.c:537
    openvelinux#16 0x7f2952a46189 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).
```

Fixes: f7b58cb ("perf mem/c2c: Add load store event mappings for AMD")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Ravi Bangoria <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: PvsNarasimha <[email protected]>
guojinhui-liam pushed a commit that referenced this pull request Feb 10, 2025
[ Upstream commit 4a74da0 ]

KASAN reports an out of bounds read:
BUG: KASAN: slab-out-of-bounds in __kuid_val include/linux/uidgid.h:36
BUG: KASAN: slab-out-of-bounds in uid_eq include/linux/uidgid.h:63 [inline]
BUG: KASAN: slab-out-of-bounds in key_task_permission+0x394/0x410
security/keys/permission.c:54
Read of size 4 at addr ffff88813c3ab618 by task stress-ng/4362

CPU: 2 PID: 4362 Comm: stress-ng Not tainted 5.10.0-14930-gafbffd6c3ede #15
Call Trace:
 __dump_stack lib/dump_stack.c:82 [inline]
 dump_stack+0x107/0x167 lib/dump_stack.c:123
 print_address_description.constprop.0+0x19/0x170 mm/kasan/report.c:400
 __kasan_report.cold+0x6c/0x84 mm/kasan/report.c:560
 kasan_report+0x3a/0x50 mm/kasan/report.c:585
 __kuid_val include/linux/uidgid.h:36 [inline]
 uid_eq include/linux/uidgid.h:63 [inline]
 key_task_permission+0x394/0x410 security/keys/permission.c:54
 search_nested_keyrings+0x90e/0xe90 security/keys/keyring.c:793

This issue was also reported by syzbot.

It can be reproduced by following these steps(more details [1]):
1. Obtain more than 32 inputs that have similar hashes, which ends with the
   pattern '0xxxxxxxe6'.
2. Reboot and add the keys obtained in step 1.

The reproducer demonstrates how this issue happened:
1. In the search_nested_keyrings function, when it iterates through the
   slots in a node(below tag ascend_to_node), if the slot pointer is meta
   and node->back_pointer != NULL(it means a root), it will proceed to
   descend_to_node. However, there is an exception. If node is the root,
   and one of the slots points to a shortcut, it will be treated as a
   keyring.
2. Whether the ptr is keyring decided by keyring_ptr_is_keyring function.
   However, KEYRING_PTR_SUBTYPE is 0x2UL, the same as
   ASSOC_ARRAY_PTR_SUBTYPE_MASK.
3. When 32 keys with the similar hashes are added to the tree, the ROOT
   has keys with hashes that are not similar (e.g. slot 0) and it splits
   NODE A without using a shortcut. When NODE A is filled with keys that
   all hashes are xxe6, the keys are similar, NODE A will split with a
   shortcut. Finally, it forms the tree as shown below, where slot 6 points
   to a shortcut.

                      NODE A
              +------>+---+
      ROOT    |       | 0 | xxe6
      +---+   |       +---+
 xxxx | 0 | shortcut  :   : xxe6
      +---+   |       +---+
 xxe6 :   :   |       |   | xxe6
      +---+   |       +---+
      | 6 |---+       :   : xxe6
      +---+           +---+
 xxe6 :   :           | f | xxe6
      +---+           +---+
 xxe6 | f |
      +---+

4. As mentioned above, If a slot(slot 6) of the root points to a shortcut,
   it may be mistakenly transferred to a key*, leading to a read
   out-of-bounds read.

To fix this issue, one should jump to descend_to_node if the ptr is a
shortcut, regardless of whether the node is root or not.

[1] https://lore.kernel.org/linux-kernel/[email protected]/

[jarkko: tweaked the commit message a bit to have an appropriate closes
 tag.]
Fixes: b2a4df2 ("KEYS: Expand the capacity of a keyring")
Reported-by: [email protected]
Closes: https://lore.kernel.org/all/[email protected]/T/
Signed-off-by: Chen Ridong <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Jarkko Sakkinen <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
Signed-off-by: Liangyan <[email protected]>
guojinhui-liam pushed a commit that referenced this pull request Feb 11, 2025
commit 99d4850 upstream.

Found by leak sanitizer:
```
==1632594==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 21 byte(s) in 1 object(s) allocated from:
    #0 0x7f2953a7077b in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:439
    #1 0x556701d6fbbf in perf_env__read_cpuid util/env.c:369
    #2 0x556701d70589 in perf_env__cpuid util/env.c:465
    #3 0x55670204bba2 in x86__is_amd_cpu arch/x86/util/env.c:14
    #4 0x5567020487a2 in arch__post_evsel_config arch/x86/util/evsel.c:83
    #5 0x556701d8f78b in evsel__config util/evsel.c:1366
    #6 0x556701ef5872 in evlist__config util/record.c:108
    #7 0x556701cd6bcd in test__PERF_RECORD tests/perf-record.c:112
    #8 0x556701cacd07 in run_test tests/builtin-test.c:236
    #9 0x556701cacfac in test_and_print tests/builtin-test.c:265
    #10 0x556701cadddb in __cmd_test tests/builtin-test.c:402
    #11 0x556701caf2aa in cmd_test tests/builtin-test.c:559
    #12 0x556701d3b557 in run_builtin tools/perf/perf.c:323
    #13 0x556701d3bac8 in handle_internal_command tools/perf/perf.c:377
    #14 0x556701d3be90 in run_argv tools/perf/perf.c:421
    #15 0x556701d3c3f8 in main tools/perf/perf.c:537
    #16 0x7f2952a46189 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 21 byte(s) leaked in 1 allocation(s).
```

Fixes: f7b58cb ("perf mem/c2c: Add load store event mappings for AMD")
Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Ravi Bangoria <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: PvsNarasimha <[email protected]>
guojinhui-liam pushed a commit that referenced this pull request May 8, 2025
commit a699781 upstream.

A sysfs reader can race with a device reset or removal, attempting to
read device state when the device is not actually present. eg:

     [exception RIP: qed_get_current_link+17]
  #8 [ffffb9e4f2907c48] qede_get_link_ksettings at ffffffffc07a994a [qede]
  #9 [ffffb9e4f2907cd8] __rh_call_get_link_ksettings at ffffffff992b01a3
 #10 [ffffb9e4f2907d38] __ethtool_get_link_ksettings at ffffffff992b04e4
 #11 [ffffb9e4f2907d90] duplex_show at ffffffff99260300
 #12 [ffffb9e4f2907e38] dev_attr_show at ffffffff9905a01c
 #13 [ffffb9e4f2907e50] sysfs_kf_seq_show at ffffffff98e0145b
 #14 [ffffb9e4f2907e68] seq_read at ffffffff98d902e3
 #15 [ffffb9e4f2907ec8] vfs_read at ffffffff98d657d1
 #16 [ffffb9e4f2907f00] ksys_read at ffffffff98d65c3f
 #17 [ffffb9e4f2907f38] do_syscall_64 at ffffffff98a052fb

 crash> struct net_device.state ffff9a9d21336000
    state = 5,

state 5 is __LINK_STATE_START (0b1) and __LINK_STATE_NOCARRIER (0b100).
The device is not present, note lack of __LINK_STATE_PRESENT (0b10).

This is the same sort of panic as observed in commit 4224cfd
("net-sysfs: add check for netdevice being present to speed_show").

There are many other callers of __ethtool_get_link_ksettings() which
don't have a device presence check.

Move this check into ethtool to protect all callers.

Fixes: d519e17 ("net: export device speed and duplex via sysfs")
Fixes: 4224cfd ("net-sysfs: add check for netdevice being present to speed_show")
Signed-off-by: Jamie Bainbridge <[email protected]>
Link: https://patch.msgid.link/8bae218864beaa44ed01628140475b9bf641c5b0.1724393671.git.jamie.bainbridge@gmail.com
Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit a699781)
Signed-off-by: Tao Ma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants