* [PATCH v4 09/27] x86/acpi: Adapt assembly for PIE support
From: Thomas Garnier @ 2018-05-29 22:15 UTC (permalink / raw)
To: kernel-hardening
Cc: Thomas Garnier, Rafael J. Wysocki, Len Brown, Pavel Machek,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-pm,
linux-kernel
In-Reply-To: <20180529221625.33541-1-thgarnie@google.com>
Change the assembly code to use only relative references of symbols for the
kernel to be PIE compatible.
Position Independent Executable (PIE) support will allow to extend the
KASLR randomization range 0xffffffff80000000.
Signed-off-by: Thomas Garnier <thgarnie@google.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
arch/x86/kernel/acpi/wakeup_64.S | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kernel/acpi/wakeup_64.S b/arch/x86/kernel/acpi/wakeup_64.S
index 50b8ed0317a3..472659c0f811 100644
--- a/arch/x86/kernel/acpi/wakeup_64.S
+++ b/arch/x86/kernel/acpi/wakeup_64.S
@@ -14,7 +14,7 @@
* Hooray, we are in Long 64-bit mode (but still running in low memory)
*/
ENTRY(wakeup_long64)
- movq saved_magic, %rax
+ movq saved_magic(%rip), %rax
movq $0x123456789abcdef0, %rdx
cmpq %rdx, %rax
jne bogus_64_magic
@@ -25,14 +25,14 @@ ENTRY(wakeup_long64)
movw %ax, %es
movw %ax, %fs
movw %ax, %gs
- movq saved_rsp, %rsp
+ movq saved_rsp(%rip), %rsp
- movq saved_rbx, %rbx
- movq saved_rdi, %rdi
- movq saved_rsi, %rsi
- movq saved_rbp, %rbp
+ movq saved_rbx(%rip), %rbx
+ movq saved_rdi(%rip), %rdi
+ movq saved_rsi(%rip), %rsi
+ movq saved_rbp(%rip), %rbp
- movq saved_rip, %rax
+ movq saved_rip(%rip), %rax
jmp *%rax
ENDPROC(wakeup_long64)
@@ -45,7 +45,7 @@ ENTRY(do_suspend_lowlevel)
xorl %eax, %eax
call save_processor_state
- movq $saved_context, %rax
+ leaq saved_context(%rip), %rax
movq %rsp, pt_regs_sp(%rax)
movq %rbp, pt_regs_bp(%rax)
movq %rsi, pt_regs_si(%rax)
@@ -64,13 +64,14 @@ ENTRY(do_suspend_lowlevel)
pushfq
popq pt_regs_flags(%rax)
- movq $.Lresume_point, saved_rip(%rip)
+ leaq .Lresume_point(%rip), %rax
+ movq %rax, saved_rip(%rip)
- movq %rsp, saved_rsp
- movq %rbp, saved_rbp
- movq %rbx, saved_rbx
- movq %rdi, saved_rdi
- movq %rsi, saved_rsi
+ movq %rsp, saved_rsp(%rip)
+ movq %rbp, saved_rbp(%rip)
+ movq %rbx, saved_rbx(%rip)
+ movq %rdi, saved_rdi(%rip)
+ movq %rsi, saved_rsi(%rip)
addq $8, %rsp
movl $3, %edi
@@ -82,7 +83,7 @@ ENTRY(do_suspend_lowlevel)
.align 4
.Lresume_point:
/* We don't restore %rax, it must be 0 anyway */
- movq $saved_context, %rax
+ leaq saved_context(%rip), %rax
movq saved_context_cr4(%rax), %rbx
movq %rbx, %cr4
movq saved_context_cr3(%rax), %rbx
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply related
* [PATCH v4 00/27] x86: PIE support and option to extend KASLR randomization
From: Thomas Garnier via Virtualization @ 2018-05-29 22:15 UTC (permalink / raw)
To: kernel-hardening
Cc: Nicolas Pitre, Sergey Senozhatsky, Jan Kiszka, Paolo Bonzini,
Pavel Machek, Christoph Lameter, linux-arch, linux-sparse,
Matthias Kaehlcke, xen-devel, Petr Mladek, linux-pm,
Nicholas Piggin, Cao jin, Andy Lutomirski, Thomas Gleixner,
nixiaoming, Skip Jiri Kosina, Randy Dunlap, Rafael J. Wysocki,
linux-kernel, Jia Zhang, Luis R. Rodriguez, linux-crypto,
Greg Kroah-Hartman <gre>
Changes:
- patch v4:
- Simplify early boot by removing global variables.
- Modify the mcount location script for __mcount_loc intead of the address
read in the ftrace implementation.
- Edit commit description to explain better where the kernel can be located.
- Streamlined the testing done on each patch proposal. Always testing
hibernation, suspend, ftrace and kprobe to ensure no regressions.
- patch v3:
- Update on message to describe longer term PIE goal.
- Minor change on ftrace if condition.
- Changed code using xchgq.
- patch v2:
- Adapt patch to work post KPTI and compiler changes
- Redo all performance testing with latest configs and compilers
- Simplify mov macro on PIE (MOVABS now)
- Reduce GOT footprint
- patch v1:
- Simplify ftrace implementation.
- Use gcc mstack-protector-guard-reg=%gs with PIE when possible.
- rfc v3:
- Use --emit-relocs instead of -pie to reduce dynamic relocation space on
mapped memory. It also simplifies the relocation process.
- Move the start the module section next to the kernel. Remove the need for
-mcmodel=large on modules. Extends module space from 1 to 2G maximum.
- Support for XEN PVH as 32-bit relocations can be ignored with
--emit-relocs.
- Support for GOT relocations previously done automatically with -pie.
- Remove need for dynamic PLT in modules.
- Support dymamic GOT for modules.
- rfc v2:
- Add support for global stack cookie while compiler default to fs without
mcmodel=kernel
- Change patch 7 to correctly jump out of the identity mapping on kexec load
preserve.
These patches make the changes necessary to build the kernel as Position
Independent Executable (PIE) on x86_64. A PIE kernel can be relocated below
the top 2G of the virtual address space. It allows to optionally extend the
KASLR randomization range from 1G to 3G. The chosen range is the one currently
available, future changes will allow the kernel module to have a wider
randomization range.
Thanks a lot to Ard Biesheuvel & Kees Cook on their feedback on compiler
changes, PIE support and KASLR in general. Thanks to Roland McGrath on his
feedback for using -pie versus --emit-relocs and details on compiler code
generation.
The patches:
- 1-3, 5-13, 18-19: Change in assembly code to be PIE compliant.
- 4: Add a new _ASM_MOVABS macro to fetch a symbol address generically.
- 14: Adapt percpu design to work correctly when PIE is enabled.
- 15: Provide an option to default visibility to hidden except for key symbols.
It removes errors between compilation units.
- 16: Add PROVIDE_HIDDEN replacement on the linker script for weak symbols to
reduce GOT footprint.
- 17: Adapt relocation tool to handle PIE binary correctly.
- 20: Add support for global cookie.
- 21: Support ftrace with PIE (used on Ubuntu config).
- 22: Add option to move the module section just after the kernel.
- 23: Adapt module loading to support PIE with dynamic GOT.
- 24: Make the GOT read-only.
- 25: Add the CONFIG_X86_PIE option (off by default).
- 26: Adapt relocation tool to generate a 64-bit relocation table.
- 27: Add the CONFIG_RANDOMIZE_BASE_LARGE option to increase relocation range
from 1G to 3G (off by default).
Performance/Size impact:
Size of vmlinux (Default configuration):
File size:
- PIE disabled: +0.18%
- PIE enabled: -1.977% (less relocations)
.text section:
- PIE disabled: same
- PIE enabled: same
Size of vmlinux (Ubuntu configuration):
File size:
- PIE disabled: +0.21%
- PIE enabled: +10%
.text section:
- PIE disabled: same
- PIE enabled: +0.001%
The size increase is mainly due to not having access to the 32-bit signed
relocation that can be used with mcmodel=kernel. A small part is due to reduced
optimization for PIE code. This bug [1] was opened with gcc to provide a better
code generation for kernel PIE.
Hackbench (50% and 1600% on thread/process for pipe/sockets):
- PIE disabled: no significant change (avg -/+ 0.5% on latest test).
- PIE enabled: between -1% to +1% in average (default and Ubuntu config).
Kernbench (average of 10 Half and Optimal runs):
Elapsed Time:
- PIE disabled: no significant change (avg -0.5%)
- PIE enabled: average -0.5% to +0.5%
System Time:
- PIE disabled: no significant change (avg -0.1%)
- PIE enabled: average -0.4% to +0.4%.
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82303
diffstat:
Documentation/x86/x86_64/mm.txt | 3
arch/x86/Kconfig | 45 ++++++
arch/x86/Makefile | 58 ++++++++
arch/x86/boot/boot.h | 2
arch/x86/boot/compressed/Makefile | 5
arch/x86/boot/compressed/misc.c | 10 +
arch/x86/crypto/aes-x86_64-asm_64.S | 45 ++++--
arch/x86/crypto/aesni-intel_asm.S | 8 -
arch/x86/crypto/aesni-intel_avx-x86_64.S | 6
arch/x86/crypto/camellia-aesni-avx-asm_64.S | 42 +++---
arch/x86/crypto/camellia-aesni-avx2-asm_64.S | 44 +++---
arch/x86/crypto/camellia-x86_64-asm_64.S | 8 -
arch/x86/crypto/cast5-avx-x86_64-asm_64.S | 50 ++++---
arch/x86/crypto/cast6-avx-x86_64-asm_64.S | 44 +++---
arch/x86/crypto/des3_ede-asm_64.S | 96 +++++++++-----
arch/x86/crypto/ghash-clmulni-intel_asm.S | 4
arch/x86/crypto/glue_helper-asm-avx.S | 4
arch/x86/crypto/glue_helper-asm-avx2.S | 6
arch/x86/crypto/sha256-avx2-asm.S | 23 ++-
arch/x86/entry/calling.h | 2
arch/x86/entry/entry_32.S | 3
arch/x86/entry/entry_64.S | 25 ++-
arch/x86/include/asm/asm.h | 1
arch/x86/include/asm/bug.h | 2
arch/x86/include/asm/ftrace.h | 4
arch/x86/include/asm/jump_label.h | 8 -
arch/x86/include/asm/kvm_host.h | 8 -
arch/x86/include/asm/module.h | 11 +
arch/x86/include/asm/page_64_types.h | 9 +
arch/x86/include/asm/paravirt_types.h | 12 +
arch/x86/include/asm/percpu.h | 25 ++-
arch/x86/include/asm/pgtable_64_types.h | 6
arch/x86/include/asm/pm-trace.h | 2
arch/x86/include/asm/processor.h | 16 +-
arch/x86/include/asm/sections.h | 8 +
arch/x86/include/asm/setup.h | 2
arch/x86/include/asm/stackprotector.h | 19 ++
arch/x86/kernel/Makefile | 6
arch/x86/kernel/acpi/wakeup_64.S | 31 ++--
arch/x86/kernel/asm-offsets.c | 3
arch/x86/kernel/asm-offsets_32.c | 3
arch/x86/kernel/asm-offsets_64.c | 3
arch/x86/kernel/cpu/common.c | 3
arch/x86/kernel/cpu/microcode/core.c | 4
arch/x86/kernel/ftrace.c | 42 +++++-
arch/x86/kernel/head64.c | 23 ++-
arch/x86/kernel/head_32.S | 3
arch/x86/kernel/head_64.S | 31 +++-
arch/x86/kernel/kvm.c | 6
arch/x86/kernel/module.c | 181 ++++++++++++++++++++++++++-
arch/x86/kernel/module.lds | 3
arch/x86/kernel/process.c | 5
arch/x86/kernel/relocate_kernel_64.S | 16 +-
arch/x86/kernel/setup_percpu.c | 5
arch/x86/kernel/vmlinux.lds.S | 13 +
arch/x86/kvm/svm.c | 4
arch/x86/lib/cmpxchg16b_emu.S | 8 -
arch/x86/mm/dump_pagetables.c | 3
arch/x86/power/hibernate_asm_64.S | 4
arch/x86/tools/relocs.c | 169 +++++++++++++++++++++++--
arch/x86/tools/relocs.h | 4
arch/x86/tools/relocs_common.c | 15 +-
arch/x86/xen/xen-asm.S | 12 -
arch/x86/xen/xen-head.S | 11 -
arch/x86/xen/xen-pvh.S | 13 +
drivers/base/firmware_loader/main.c | 4
include/asm-generic/sections.h | 6
include/asm-generic/vmlinux.lds.h | 12 +
include/linux/compiler.h | 7 +
init/Kconfig | 16 ++
kernel/kallsyms.c | 16 +-
kernel/trace/trace.h | 4
lib/dynamic_debug.c | 4
scripts/link-vmlinux.sh | 14 ++
scripts/recordmcount.c | 79 +++++++----
75 files changed, 1109 insertions(+), 343 deletions(-)
^ permalink raw reply
* Re: [PATCH 09/11] misc: throttler: Add core support for non-thermal throttling
From: Matthias Kaehlcke @ 2018-05-29 21:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Arnd Bergmann,
Rob Herring, Mark Rutland, linux-pm, devicetree, linux-kernel,
Brian Norris, Douglas Anderson
In-Reply-To: <20180528080826.GA17162@kroah.com>
On Mon, May 28, 2018 at 10:08:26AM +0200, Greg Kroah-Hartman wrote:
> On Fri, May 25, 2018 at 01:30:41PM -0700, Matthias Kaehlcke wrote:
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > index 20be70c3f118..01a1714dd2ad 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -57,3 +57,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
> > obj-$(CONFIG_OCXL) += ocxl/
> > obj-$(CONFIG_MISC_RTSX) += cardreader/
> > +obj-y += throttler/
>
> Shouldn't you depend on a Kconfig option to traverse into this
> directory?
Ack
> > diff --git a/drivers/misc/throttler/Kconfig b/drivers/misc/throttler/Kconfig
> > new file mode 100644
> > index 000000000000..ef8388f6bc0a
> > --- /dev/null
> > +++ b/drivers/misc/throttler/Kconfig
> > @@ -0,0 +1,13 @@
> > +menuconfig THROTTLER
> > + bool "Throttler support"
> > + default n
>
> the default is always 'n' no need to say it again here :)
Will remove
> > + depends on OF
> > + select CPU_FREQ
> > + select PM_DEVFREQ
> > + help
> > + This option enables core support for non-thermal throttling of CPUs
> > + and devfreq devices.
> > +
> > + Note that you also need a event monitor module usually called
> > + *_throttler.
> > +
> > diff --git a/drivers/misc/throttler/Makefile b/drivers/misc/throttler/Makefile
> > new file mode 100644
> > index 000000000000..c8d920cee315
> > --- /dev/null
> > +++ b/drivers/misc/throttler/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_THROTTLER) += core.o
> > diff --git a/drivers/misc/throttler/core.c b/drivers/misc/throttler/core.c
> > new file mode 100644
> > index 000000000000..c058d03212b8
> > --- /dev/null
> > +++ b/drivers/misc/throttler/core.c
> > @@ -0,0 +1,373 @@
> > +/*
> > + * Core code for non-thermal throttling
> > + *
> > + * Copyright (C) 2018 Google, Inc.
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
>
> Please just use a single SPDX line, like checkpatch.pl would have warned
> you about. No need for the full verbous license boiler-plate text here
> at all.
Ok
^ permalink raw reply
* Re: [PATCH 09/11] misc: throttler: Add core support for non-thermal throttling
From: Matthias Kaehlcke @ 2018-05-29 20:57 UTC (permalink / raw)
To: Chanwoo Choi
Cc: MyungJoo Ham, Kyungmin Park, Arnd Bergmann, Greg Kroah-Hartman,
Rob Herring, Mark Rutland, linux-pm, devicetree, linux-kernel,
Brian Norris, Douglas Anderson
In-Reply-To: <5B0BB095.4040106@samsung.com>
On Mon, May 28, 2018 at 04:32:37PM +0900, Chanwoo Choi wrote:
> IMHO, you better to split out the devfreq patches from
> 'throttler' patch set. Because I'm not sure throttler is either
> necessary or not.
>
> After finishing the review of 'throttler' patches without devfreq handling,
> it would be better for you to send devfreq patches separately.
I could certainly try to get 'throttler' with only cpufreq support
merged, but that would kind of defeat the purpose.
I first sent a RFC patch for the devfreq policy notifiers
(https://patchwork.kernel.org/patch/10401999/) to get an idea if this
is a reasonable path to pursue. In response you asked about "real code
and patches" and here it is :)
For my use case throttler is not really useful without devfreq
support. In this sense I prefer to know 'early' if there are any
blocking issues, rather then making the effort to get a limited
version of the driver merged, and then learn that I wasted my own and
the reviewers time because it is a dead end.
> On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote:
> > The purpose of the throttler is to provide support for non-thermal
> > throttling. Throttling is triggered by external event, e.g. the
> > detection of a high battery discharge current, close to the OCP limit
> > of the battery. The throttler is only in charge of the throttling, not
> > the monitoring, which is done by another (possibly platform specific)
> > driver.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > drivers/misc/Kconfig | 1 +
> > drivers/misc/Makefile | 1 +
> > drivers/misc/throttler/Kconfig | 13 ++
> > drivers/misc/throttler/Makefile | 1 +
> > drivers/misc/throttler/core.c | 373 ++++++++++++++++++++++++++++++++
> > include/linux/throttler.h | 10 +
> > 6 files changed, 399 insertions(+)
> > create mode 100644 drivers/misc/throttler/Kconfig
> > create mode 100644 drivers/misc/throttler/Makefile
> > create mode 100644 drivers/misc/throttler/core.c
> > create mode 100644 include/linux/throttler.h
> >
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index 5d713008749b..691d9625d83c 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -513,4 +513,5 @@ source "drivers/misc/echo/Kconfig"
> > source "drivers/misc/cxl/Kconfig"
> > source "drivers/misc/ocxl/Kconfig"
> > source "drivers/misc/cardreader/Kconfig"
> > +source "drivers/misc/throttler/Kconfig"
> > endmenu
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > index 20be70c3f118..01a1714dd2ad 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -57,3 +57,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
> > obj-$(CONFIG_OCXL) += ocxl/
> > obj-$(CONFIG_MISC_RTSX) += cardreader/
> > +obj-y += throttler/
> > diff --git a/drivers/misc/throttler/Kconfig b/drivers/misc/throttler/Kconfig
> > new file mode 100644
> > index 000000000000..ef8388f6bc0a
> > --- /dev/null
> > +++ b/drivers/misc/throttler/Kconfig
> > @@ -0,0 +1,13 @@
> > +menuconfig THROTTLER
> > + bool "Throttler support"
> > + default n
> > + depends on OF
> > + select CPU_FREQ
> > + select PM_DEVFREQ
> > + help
> > + This option enables core support for non-thermal throttling of CPUs
> > + and devfreq devices.
> > +
> > + Note that you also need a event monitor module usually called
> > + *_throttler.
> > +
> > diff --git a/drivers/misc/throttler/Makefile b/drivers/misc/throttler/Makefile
> > new file mode 100644
> > index 000000000000..c8d920cee315
> > --- /dev/null
> > +++ b/drivers/misc/throttler/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_THROTTLER) += core.o
> > diff --git a/drivers/misc/throttler/core.c b/drivers/misc/throttler/core.c
> > new file mode 100644
> > index 000000000000..c058d03212b8
> > --- /dev/null
> > +++ b/drivers/misc/throttler/core.c
> > @@ -0,0 +1,373 @@
> > +/*
> > + * Core code for non-thermal throttling
> > + *
> > + * Copyright (C) 2018 Google, Inc.
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#include <linux/cpufreq.h>
> > +#include <linux/devfreq.h>
> > +#include <linux/kernel.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +
> > +/*
> > + * Non-thermal throttling: throttling of system components in response to
> > + * external events (e.g. high battery discharge current).
> > + *
> > + * The throttler supports throttling through cpufreq and devfreq. Multiple
> > + * levels of throttling can be configured. At level 0 no throttling is
> > + * active on behalf of the throttler, for values > 0 throttling is typically
> > + * configured to be increasingly aggressive with each level.
> > + * The number of throttling levels is not limited by the throttler (though
> > + * it is likely limited by the throttling devices). It is not necessary to
> > + * configure the same number of levels for all throttling devices. If the
> > + * requested throttling level for a device is higher than the maximum level
> > + * of the device the throttler will sleect the maximum throttling level of
> > + * the device.
> > + *
> > + * Non-thermal throttling is split in two parts:
> > + *
> > + * - throttler core
> > + * - parses the thermal policy
> > + * - applies throttling settings for a requested level of throttling
> > + *
> > + * - event monitor driver
> > + * - monitors the events that trigger throttling
> > + * - determines the throttling level (often limited to on/off)
> > + * - requests throttler core to apply throttling settings
> > + *
> > + * It is possible for a system to have more than one throttler and the
> > + * throttlers may make use of the same throttling devices, in case of
> > + * conflicting settings for a device the more aggressive values will be
> > + * applied.
> > + *
> > + */
> > +
> > +struct thrcfg {
> > + uint32_t *freqs;
> > + int num_levels;
> > +};
> > +
> > +struct cpufreq_thrdev {
> > + uint32_t cpu;
> > + struct thrcfg cfg;
> > +};
> > +
> > +struct devfreq_thrdev {
> > + struct devfreq *devfreq;
> > + struct thrcfg cfg;
> > + struct throttler *thr;
> > + struct notifier_block nb;
> > +};
> > +
> > +struct __thr_cpufreq {
> > + struct cpufreq_thrdev *devs;
> > + int ndevs;
> > + struct notifier_block nb;
> > +};
> > +
> > +struct __thr_devfreq {
> > + struct devfreq_thrdev *devs;
> > + int ndevs;
> > +};
> > +
> > +struct throttler {
> > + struct device *dev;
> > + int level;
> > + struct __thr_cpufreq cpufreq;
> > + struct __thr_devfreq devfreq;
> > +};
> > +
> > +static unsigned long thr_get_throttling_freq(struct thrcfg *cfg, int level)
> > +{
> > + if (level == 0 ) {
> > + WARN(true, "level == 0");
> > + return 0;
> > + }
> > +
> > + if (level <= cfg->num_levels)
> > + return cfg->freqs[level - 1];
> > + else
> > + return cfg->freqs[cfg->num_levels - 1];
> > +}
> > +
> > +static int thr_cpufreq_event(struct notifier_block *nb,
> > + unsigned long event, void *data)
> > +{
> > + struct throttler *thr =
> > + container_of(nb, struct throttler, cpufreq.nb);
> > + struct cpufreq_policy *policy = data;
> > + struct cpufreq_thrdev *ctd;
> > + int i;
> > +
> > + if ((event != CPUFREQ_ADJUST) || (thr->level == 0))
> > + return NOTIFY_DONE;
> > +
> > + for (i = 0; i < thr->cpufreq.ndevs; i++) {
> > + ctd = &thr->cpufreq.devs[i];
> > +
> > + if (ctd->cpu == policy->cpu) {
> > + unsigned long clamp_freq =
> > + thr_get_throttling_freq(&ctd->cfg, thr->level);
> > + if (clamp_freq < policy->max) {
> > + cpufreq_verify_within_limits(policy, 0, clamp_freq);
> > + }
> > + }
> > + }
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static int thr_devfreq_event(struct notifier_block *nb,
> > + unsigned long event, void *data)
> > +{
> > + struct devfreq_thrdev *dtd =
> > + container_of(nb, struct devfreq_thrdev, nb);
> > + struct throttler *thr = dtd->thr;
> > + struct devfreq_policy *policy = data;
> > + unsigned long clamp_freq;
> > +
> > + if ((event != DEVFREQ_ADJUST) || (thr->level == 0))
> > + return NOTIFY_DONE;
> > +
> > + clamp_freq = thr_get_throttling_freq(&dtd->cfg, thr->level);
> > + if (clamp_freq < policy->max)
> > + devfreq_verify_within_limits(policy, 0, clamp_freq);
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static void thr_cpufreq_update_policy(struct throttler *thr)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < thr->cpufreq.ndevs; i++) {
> > + struct cpufreq_thrdev *ctd = &thr->cpufreq.devs[i];
> > + struct cpufreq_policy *policy = cpufreq_cpu_get(ctd->cpu);
> > +
> > + if (!policy) {
> > + dev_warn(thr->dev, "CPU%d does have no cpufreq policy!\n", ctd->cpu);
> > + continue;
> > + }
> > +
> > + cpufreq_update_policy(ctd->cpu);
> > + cpufreq_cpu_put(policy);
> > + }
> > +}
> > +
> > +static int thr_parse_thrcfg(struct throttler *thr,
> > + struct device_node *np, struct thrcfg *cfg) {
> > + int err;
> > +
> > + cfg->num_levels =
> > + of_property_count_u32_elems(np, "throttling-frequencies");
> > + if (cfg->num_levels < 0) {
> > + pr_err("%s: failed to determine number of throttling frequencies\n",
> > + np->full_name);
> > + return cfg->num_levels;
> > + }
> > +
> > + cfg->freqs = devm_kzalloc(thr->dev,
> > + cfg->num_levels * sizeof(u32), GFP_KERNEL);
> > + if (!cfg->freqs)
> > + return -ENOMEM;
> > +
> > + err = of_property_read_u32_array(np, "throttling-frequencies",
> > + cfg->freqs, cfg->num_levels);
> > + if (err) {
> > + pr_err("%s: failed to read throttling frequencies\n", np->full_name);
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct devfreq *thr_find_devfreq_dev(struct throttler *thr,
> > + struct device_node *np_df) {
> > + struct device_node *node;
> > + struct platform_device *pdev;
> > +
> > + node = of_parse_phandle(np_df, "device", 0);
> > + if (!node) {
> > + pr_err("%s: failed to get devfreq parent device\n",
> > + np_df->full_name);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + pdev = of_find_device_by_node(node);
> > + if (!pdev) {
> > + pr_err("%s: could not find devfreq parent device\n",
> > + node->full_name);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + return dev_to_devfreq(&pdev->dev);
> > +}
> > +
> > +static int thr_parse_dt(struct throttler *thr, struct device_node *np)
> > +{
> > + struct device_node *node, *child;
> > + int err, i;
> > +
> > + node = of_get_child_by_name(np, "cpufreq");
> > + if (node) {
> > + thr->cpufreq.ndevs = of_get_child_count(node);
> > + thr->cpufreq.devs = devm_kzalloc(thr->dev,
> > + sizeof(*thr->cpufreq.devs) * thr->cpufreq.ndevs,
> > + GFP_KERNEL);
> > +
> > + i = 0;
> > + for_each_child_of_node(node, child) {
> > + struct cpufreq_thrdev *ctd = &thr->cpufreq.devs[i];
> > +
> > + err = of_property_read_u32(child, "cpu", &ctd->cpu);
> > + if (err) {
> > + pr_err("%s: failed to read CPU id\n", child->full_name);
> > + return err;
> > + }
> > +
> > + err = thr_parse_thrcfg(thr, child, &ctd->cfg);
> > + if (err)
> > + return err;
> > +
> > + i++;
> > + }
> > + }
> > +
> > + node = of_get_child_by_name(np, "devfreq");
> > + if (node) {
> > + thr->devfreq.ndevs = of_get_child_count(node);
> > + thr->devfreq.devs = devm_kzalloc(thr->dev,
> > + sizeof(*thr->devfreq.devs) * thr->devfreq.ndevs,
> > + GFP_KERNEL);
> > +
> > + i = 0;
> > + for_each_child_of_node(node, child) {
> > + struct devfreq_thrdev *dtd = &thr->devfreq.devs[i];
> > +
> > + dtd->thr = thr;
> > +
> > + dtd->devfreq = thr_find_devfreq_dev(thr, child);
> > + if (IS_ERR(dtd->devfreq))
> > + return PTR_ERR(dtd->devfreq);
> > +
> > + err = thr_parse_thrcfg(thr, child, &dtd->cfg);
> > + if (err)
> > + return err;
> > +
> > + i++;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void thr_update_devfreq(struct devfreq *devfreq)
> > +{
> > + mutex_lock(&devfreq->lock);
> > + update_devfreq(devfreq);
> > + mutex_unlock(&devfreq->lock);
> > +}
> > +
> > +void throttler_set_level(struct throttler *thr, int level)
> > +{
> > + int i;
> > +
> > + if (level == thr->level)
> > + return;
> > +
> > + dev_dbg(thr->dev, "throttling level: %d\n", level);
> > + thr->level = level;
> > +
> > + if (thr->cpufreq.ndevs > 0)
> > + thr_cpufreq_update_policy(thr);
> > +
> > + if (thr->devfreq.ndevs > 0)
> > + for (i = 0; i < thr->devfreq.ndevs; i++)
> > + thr_update_devfreq(thr->devfreq.devs[i].devfreq);
> > +}
> > +EXPORT_SYMBOL_GPL(throttler_set_level);
> > +
> > +struct throttler *throttler_setup(struct device *dev)
> > +{
> > + struct throttler *thr;
> > + struct device_node *np = dev->of_node;
> > + int err, i;
> > +
> > + if (!np)
> > + /* should never happen */
> > + return ERR_PTR(-EINVAL);
> > +
> > + thr = devm_kzalloc(dev, sizeof(*thr), GFP_KERNEL);
> > + if (!thr)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + thr->dev = dev;
> > +
> > + err = thr_parse_dt(thr, np);
> > + if (err)
> > + return ERR_PTR(err);
> > +
> > + if (thr->cpufreq.ndevs > 0) {
> > + thr->cpufreq.nb.notifier_call = thr_cpufreq_event;
> > + err = cpufreq_register_notifier(&thr->cpufreq.nb,
> > + CPUFREQ_POLICY_NOTIFIER);
> > + if (err < 0) {
> > + dev_err(dev, "failed to register cpufreq notifier\n");
> > + return ERR_PTR(err);
> > + }
> > + }
> > +
> > + for (i = 0; i < thr->devfreq.ndevs; i++) {
> > + struct devfreq_thrdev *dtd = &thr->devfreq.devs[i];
> > +
> > + dtd->nb.notifier_call = thr_devfreq_event;
> > + err = devm_devfreq_register_notifier(dev, dtd->devfreq,
> > + &dtd->nb, DEVFREQ_POLICY_NOTIFIER);
> > + if (err < 0) {
> > + dev_err(dev, "failed to register devfreq notifier\n");
> > + goto err_cpufreq_unregister;
> > + }
> > + }
> > +
> > + return thr;
> > +
> > +err_cpufreq_unregister:
> > + if (thr->cpufreq.ndevs > 0)
> > + cpufreq_unregister_notifier(&thr->cpufreq.nb,
> > + CPUFREQ_POLICY_NOTIFIER);
> > +
> > + return ERR_PTR(err);
> > +}
> > +EXPORT_SYMBOL_GPL(throttler_setup);
> > +
> > +void throttler_teardown(struct throttler *thr)
> > +{
> > + int i;
> > +
> > + thr->level = 0;
> > +
> > + if (thr->cpufreq.ndevs > 0) {
> > + thr_cpufreq_update_policy(thr);
> > +
> > + cpufreq_unregister_notifier(&thr->cpufreq.nb,
> > + CPUFREQ_POLICY_NOTIFIER);
> > + }
> > +
> > + if (thr->devfreq.ndevs > 0)
> > + for (i = 0; i < thr->devfreq.ndevs; i++)
> > + thr_update_devfreq(thr->devfreq.devs[i].devfreq);
> > +}
> > +EXPORT_SYMBOL_GPL(throttler_teardown);
> > diff --git a/include/linux/throttler.h b/include/linux/throttler.h
> > new file mode 100644
> > index 000000000000..cab8c466da4b
> > --- /dev/null
> > +++ b/include/linux/throttler.h
> > @@ -0,0 +1,10 @@
> > +#ifndef __LINUX_THROTTLER_H__
> > +#define __LINUX_THROTTLER_H__
> > +
> > +struct throttler;
> > +
> > +extern struct throttler *throttler_setup(struct device *dev);
> > +extern void throttler_teardown(struct throttler *thr);
> > +extern void throttler_set_level(struct throttler *thr, int level);
> > +
> > +#endif /* __LINUX_THROTTLER_H__ */
> >
^ permalink raw reply
* Re: [PATCH 07/11] PM / devfreg: Add support policy notifiers
From: Matthias Kaehlcke @ 2018-05-29 20:02 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Kyungmin Park, Chanwoo Choi, Arnd Bergmann, Greg Kroah-Hartman,
Rob Herring, Mark Rutland, linux-pm@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Brian Norris, Douglas Anderson
In-Reply-To: <20180528051949epcms1p3bedd6ac3e2f20d0bcd8cb082349d97fa@epcms1p3>
On Mon, May 28, 2018 at 02:19:49PM +0900, MyungJoo Ham wrote:
> >Policy notifiers are called before a frequency change and may narrow
> >the min/max frequency range in devfreq_policy, which is used to adjust
> >the target frequency if it is beyond this range.
> >
> >Also add a few helpers:
> > - devfreq_verify_within_[dev_]limits()
> > - should be used by the notifiers for policy adjustments.
> > - dev_to_devfreq()
> > - lookup a devfreq strict from a device pointer
> >
> >Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >---
> > drivers/devfreq/devfreq.c | 47 +++++++++++++++++++++-------
> > include/linux/devfreq.h | 66 +++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 102 insertions(+), 11 deletions(-)
>
> Hello Matthias,
>
>
> Why should we have yet another notifier from an instance of devfreq?
> Wouldn't it better to let the current notifier (transition notifier)
> handle new events as well by adding possible event states to it?
Honestly the main reason is that I sought inspiration from cpufreq,
which uses a dedicated policy notifier. Unfortunately this change
predates the git history so I don't know what was the exact rationale
to do it this way.
Some minor advantages that I see are:
- transition notifiers aren't bothered about adjustments and viceversa
- different data types are passed for transitions and adjustments,
which makes code of notifiers that handle both a bit more messy.
> Anyway, is this the reason why you've separated some data of devfreq
> into "policy" struct? (I was wondering why while reading commit 6/11).
The DEVFREQ_ADJUST is the reason for the "policy struct". With this
change we are dealing with 3 types of frequency pairs: user
(df->min/max_freq), devinfo (df->scaling_min/max_freq) and the
policy/adjustable ones. I think it is cleaner to group them in a
struct (and sub-structs), rather than having 6 individual
<type>_min/max_freq variables. Also it allows to only pass the policy
object to the notifiers, instead of the entire devfreq device.
I opted to do the introduction of the struct policy in a separate NOP
patch, because I think it is easier to review the 'reorg' churn
and the functional change separately.
Please let me know if you'd prefer to have certain things done
differently.
Thanks
Matthias
^ permalink raw reply
* Re: [PATCH v4] vfio: platform: Fix using devices in PM Domains
From: Alex Williamson @ 2018-05-29 19:42 UTC (permalink / raw)
To: Auger Eric
Cc: Geert Uytterhoeven, kvm, linux-renesas-soc, linux-pm,
linux-kernel
In-Reply-To: <8bcadbb5-064c-0ad2-30a2-4aeec6253dd1@redhat.com>
On Tue, 29 May 2018 21:06:08 +0200
Auger Eric <eric.auger@redhat.com> wrote:
> Hi,
> On 05/29/2018 07:15 PM, Geert Uytterhoeven wrote:
> > If a device is part of a PM Domain (e.g. power and/or clock domain), its
> > power state is managed using Runtime PM. Without Runtime PM, the device
> > may not be powered up or clocked, causing subtle failures, crashes, or
> > system lock-ups when the device is accessed by the guest.
> >
> > Fix this by adding Runtime PM support, powering the device when the VFIO
> > device is opened by the guest.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> Acked-by: Eric Auger <eric.auger@redhat.com>
Looks ok to me too, applied with Eric's ack to vfio next branch for
v4.18. Thanks,
Alex
> > ---
> > Against linux-vfio/next
> >
> > v4:
> > - Keep device powered while issuing reset,
> >
> > v3:
> > - Drop controversial note about unsafeness of exporting fine-grained
> > power management from host to guest,
> >
> > v2:
> > - Improve wording,
> > - Add Reviewed-by.
> > ---
> > drivers/vfio/platform/vfio_platform_common.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> > index b60bb5326668498c..c0cd824be2b767be 100644
> > --- a/drivers/vfio/platform/vfio_platform_common.c
> > +++ b/drivers/vfio/platform/vfio_platform_common.c
> > @@ -17,6 +17,7 @@
> > #include <linux/iommu.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/slab.h>
> > #include <linux/types.h>
> > #include <linux/uaccess.h>
> > @@ -239,6 +240,7 @@ static void vfio_platform_release(void *device_data)
> > ret, extra_dbg ? extra_dbg : "");
> > WARN_ON(1);
> > }
> > + pm_runtime_put(vdev->device);
> > vfio_platform_regions_cleanup(vdev);
> > vfio_platform_irq_cleanup(vdev);
> > }
> > @@ -269,6 +271,10 @@ static int vfio_platform_open(void *device_data)
> > if (ret)
> > goto err_irq;
> >
> > + ret = pm_runtime_get_sync(vdev->device);
> > + if (ret < 0)
> > + goto err_pm;
> > +
> > ret = vfio_platform_call_reset(vdev, &extra_dbg);
> > if (ret && vdev->reset_required) {
> > dev_warn(vdev->device, "reset driver is required and reset call failed in open (%d) %s\n",
> > @@ -283,6 +289,8 @@ static int vfio_platform_open(void *device_data)
> > return 0;
> >
> > err_rst:
> > + pm_runtime_put(vdev->device);
> > +err_pm:
> > vfio_platform_irq_cleanup(vdev);
> > err_irq:
> > vfio_platform_regions_cleanup(vdev);
> > @@ -690,6 +698,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
> >
> > mutex_init(&vdev->igate);
> >
> > + pm_runtime_enable(vdev->device);
> > return 0;
> >
> > put_iommu:
> > @@ -707,6 +716,7 @@ struct vfio_platform_device *vfio_platform_remove_common(struct device *dev)
> > vdev = vfio_del_group_dev(dev);
> >
> > if (vdev) {
> > + pm_runtime_disable(vdev->device);
> > vfio_platform_put_reset(vdev);
> > vfio_iommu_group_put(dev->iommu_group, dev);
> > }
> >
^ permalink raw reply
* [PATCH 2/2] PCI: imx: Initial imx7d pm support
From: Leonard Crestez @ 2018-05-29 19:39 UTC (permalink / raw)
To: Andrey Smirnov, Philipp Zabel, Lucas Stach, Richard Zhu,
linux-pci, linux-pm, linux-arm-kernel, linux-kernel
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Anson Huang, Jingoo Han,
Joao Pinto, Rafael J. Wysocki, Abel Vesa
In-Reply-To: <cover.1527621510.git.leonard.crestez@nxp.com>
On imx7d the phy is turned off in suspend and must be reset on resume.
Right now lspci -v fails after a suspend/resume cycle, fix this by
adding minimal suspend/resume code from the nxp vendor tree.
This is currently only enabled for imx7 but the same sequence can be
applied to other imx pcie variants.
Tested on imx7d-sabresd with an Intel 5622ANHMW wireless pcie adapter.
The original author is mostly Richard Zhu <hongxing.zhu@nxp.com>, this
patch adjusts the code to the upstream imx7d implemention using reset
controls and power domains.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/pci/dwc/pci-imx6.c | 94 ++++++++++++++++++++++++++++++++++++--
1 file changed, 89 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
index 4818ef875f8a..ff6077eeb387 100644
--- a/drivers/pci/dwc/pci-imx6.c
+++ b/drivers/pci/dwc/pci-imx6.c
@@ -540,10 +540,27 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
dev_err(dev, "Speed change timeout\n");
return -EINVAL;
}
+static void imx6_pcie_ltssm_enable(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+ switch (imx6_pcie->variant) {
+ case IMX6Q:
+ case IMX6SX:
+ case IMX6QP:
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6Q_GPR12_PCIE_CTL_2,
+ IMX6Q_GPR12_PCIE_CTL_2);
+ break;
+ case IMX7D:
+ reset_control_deassert(imx6_pcie->apps_reset);
+ }
+}
+
static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
{
struct dw_pcie *pci = imx6_pcie->pci;
struct device *dev = pci->dev;
u32 tmp;
@@ -558,15 +575,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);
/* Start LTSSM. */
- if (imx6_pcie->variant == IMX7D)
- reset_control_deassert(imx6_pcie->apps_reset);
- else
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+ imx6_pcie_ltssm_enable(dev);
ret = imx6_pcie_wait_for_link(imx6_pcie);
if (ret)
goto err_reset_phy;
@@ -681,10 +694,80 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
static const struct dw_pcie_ops dw_pcie_ops = {
.link_up = imx6_pcie_link_up,
};
+#ifdef CONFIG_PM_SLEEP
+static int imx6_pcie_suspend_noirq(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+ if (imx6_pcie->variant == IMX7D) {
+ /* Disable clks */
+ clk_disable_unprepare(imx6_pcie->pcie);
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
+ clk_disable_unprepare(imx6_pcie->pcie_bus);
+ /* turn off external osc input */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
+ IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+ }
+
+ return 0;
+}
+
+static void imx6_pcie_ltssm_disable(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+ switch (imx6_pcie->variant) {
+ case IMX6Q:
+ case IMX6SX:
+ case IMX6QP:
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6Q_GPR12_PCIE_CTL_2, 0);
+ break;
+ case IMX7D:
+ reset_control_assert(imx6_pcie->apps_reset);
+ break;
+ }
+}
+
+static int imx6_pcie_resume_noirq(struct device *dev)
+{
+ int ret = 0;
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+ struct pcie_port *pp = &imx6_pcie->pci->pp;
+
+ if (imx6_pcie->variant == IMX7D) {
+ imx6_pcie_ltssm_disable(dev);
+
+ imx6_pcie_assert_core_reset(imx6_pcie);
+ imx6_pcie_init_phy(imx6_pcie);
+ imx6_pcie_deassert_core_reset(imx6_pcie);
+
+ /*
+ * controller maybe turn off, re-configure again
+ */
+ dw_pcie_setup_rc(pp);
+
+ imx6_pcie_ltssm_enable(dev);
+
+ ret = imx6_pcie_wait_for_link(imx6_pcie);
+ if (ret < 0)
+ pr_info("pcie link is down after resume.\n");
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops imx6_pcie_pm_ops = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
+ imx6_pcie_resume_noirq)
+};
+#endif
+
static int imx6_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct dw_pcie *pci;
struct imx6_pcie *imx6_pcie;
@@ -847,10 +930,11 @@ static const struct of_device_id imx6_pcie_of_match[] = {
static struct platform_driver imx6_pcie_driver = {
.driver = {
.name = "imx6q-pcie",
.of_match_table = imx6_pcie_of_match,
.suppress_bind_attrs = true,
+ .pm = &imx6_pcie_pm_ops,
},
.probe = imx6_pcie_probe,
.shutdown = imx6_pcie_shutdown,
};
--
2.17.0
^ permalink raw reply related
* [PATCH 1/2] reset: imx7: Fix always writing bits as 0
From: Leonard Crestez @ 2018-05-29 19:39 UTC (permalink / raw)
To: Andrey Smirnov, Philipp Zabel, Lucas Stach, Richard Zhu,
linux-pci, linux-pm, linux-arm-kernel, linux-kernel
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Anson Huang, Jingoo Han,
Joao Pinto, Rafael J. Wysocki, Abel Vesa
In-Reply-To: <cover.1527621510.git.leonard.crestez@nxp.com>
Right now the only user of reset-imx7 is pci-imx6 and the
reset_control_assert and deassert calls on pciephy_reset don't toggle
the PCIEPHY_BTN and PCIEPHY_G_RST bits as expected. Fix this by writing
1 or 0 respectively.
The reference manual is not very clear regarding SRC_PCIEPHY_RCR but for
other registers like MIPIPHY and HSICPHY the bits are explicitly
documented as "1 means assert, 0 means deassert".
The values are still reversed for IMX7_RESET_PCIE_CTRL_APPS_EN.
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
drivers/reset/reset-imx7.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
index 4db177bc89bc..fdeac1946429 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -78,11 +78,11 @@ static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev)
static int imx7_reset_set(struct reset_controller_dev *rcdev,
unsigned long id, bool assert)
{
struct imx7_src *imx7src = to_imx7_src(rcdev);
const struct imx7_src_signal *signal = &imx7_src_signals[id];
- unsigned int value = 0;
+ unsigned int value = assert ? signal->bit : 0;
switch (id) {
case IMX7_RESET_PCIEPHY:
/*
* wait for more than 10us to release phy g_rst and
--
2.17.0
^ permalink raw reply related
* [PATCH 0/2] PCI: Initial imx7d pm support
From: Leonard Crestez @ 2018-05-29 19:39 UTC (permalink / raw)
To: Andrey Smirnov, Philipp Zabel, Lucas Stach, Richard Zhu,
linux-pci, linux-pm, linux-arm-kernel, linux-kernel
Cc: Joao Pinto, Abel Vesa, Anson Huang, Jingoo Han, Rafael J. Wysocki,
Lorenzo Pieralisi, Bjorn Helgaas
This series adds initial pm support on imx7d so that after
suspend/resume lspci works again. This mostly copies the resume sequence
from the imx tree.
More can be done later to reduce power in suspend as well as adding
support for other socs.
This is motivated mostly by a desire to bring imx PM code closer to
upstream. It is possible that I am missing some things about how PM
should be done for pci host drivers, it would be great if you could
point me the right way.
It also relies on this bugfix for PGC offsets:
https://lkml.org/lkml/2018/5/29/138
Without that patch resume hangs on first PCI read from PCI-PM core. It
is not strictly related to PCI but pci-imx6 is the only user of gpcv2
power domains.
Patch 1 in this series is also technically an unrelated bugfix, however
pci-imx6 is the only user.
Leonard Crestez (2):
reset: imx7: Fix always writing bits as 0
PCI: imx: Initial imx7d pm support
drivers/pci/dwc/pci-imx6.c | 94 ++++++++++++++++++++++++++++++++++++--
drivers/reset/reset-imx7.c | 2 +-
2 files changed, 90 insertions(+), 6 deletions(-)
--
2.17.0
^ permalink raw reply
* Re: [PATCH 05/11] PM / devfreq: governors: Return device frequency limits instead of user limits
From: Matthias Kaehlcke @ 2018-05-29 19:32 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Kyungmin Park, Chanwoo Choi, Arnd Bergmann, Greg Kroah-Hartman,
Rob Herring, Mark Rutland, linux-pm@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Brian Norris, Douglas Anderson
In-Reply-To: <20180528050444epcms1p8e6552d55231a099ec89304dfb1f819b9@epcms1p8>
On Mon, May 28, 2018 at 02:04:44PM +0900, MyungJoo Ham wrote:
> >The performance, powersave and simpleondemand governors can return
> >df->min/max_freq, which are the user defined frequency limits.
> >update_devfreq() already takes care of adjusting the target frequency
> >with the user limits if necessary, therefore we can return
> >df->scaling_min/max_freq instead, which is the min/max frequency
> >supported by the device at a given time (depending on the
> >enabled/disabled OPPs)
> >
> >Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >---
> > drivers/devfreq/governor_performance.c | 2 +-
> > drivers/devfreq/governor_powersave.c | 2 +-
> > drivers/devfreq/governor_simpleondemand.c | 6 +++---
> > 3 files changed, 5 insertions(+), 5 deletions(-)
> >
>
> Actually, even scaling_max_freq and scaling_min_freq are
> covered centerally at devfreq.c:update_devfreq();
>
> Wouldn't it be sufficient to return UINT_MAX for performance
> and return UINT_MIN (0) for powersave, if the purpose is to
> remove redundancy?
>
> In the same sense, we may return UINT_MAX for freq-increasing
> case for simpleondemand as well, because they are filtered
> centrally anyway.
>
> (This commit might be better merged to 4/11 in that case as well.)
I did this in the first variant of the patch (before sending it in a
series), but Chanwoo Choi objected:
https://patchwork.kernel.org/patch/10404893/
I also still think that returning a constant would be the cleanest
solution if we can agree on this. What do you think about
DEVFREQ_MIN/MAX_FREQ (0/UINT_MAX) to make things slightly clearer?
^ permalink raw reply
* Re: [PATCH v4] vfio: platform: Fix using devices in PM Domains
From: Auger Eric @ 2018-05-29 19:06 UTC (permalink / raw)
To: Geert Uytterhoeven, Alex Williamson
Cc: kvm, linux-renesas-soc, linux-pm, linux-kernel
In-Reply-To: <1527614128-30196-1-git-send-email-geert+renesas@glider.be>
Hi,
On 05/29/2018 07:15 PM, Geert Uytterhoeven wrote:
> If a device is part of a PM Domain (e.g. power and/or clock domain), its
> power state is managed using Runtime PM. Without Runtime PM, the device
> may not be powered up or clocked, causing subtle failures, crashes, or
> system lock-ups when the device is accessed by the guest.
>
> Fix this by adding Runtime PM support, powering the device when the VFIO
> device is opened by the guest.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> Against linux-vfio/next
>
> v4:
> - Keep device powered while issuing reset,
>
> v3:
> - Drop controversial note about unsafeness of exporting fine-grained
> power management from host to guest,
>
> v2:
> - Improve wording,
> - Add Reviewed-by.
> ---
> drivers/vfio/platform/vfio_platform_common.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index b60bb5326668498c..c0cd824be2b767be 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -17,6 +17,7 @@
> #include <linux/iommu.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/uaccess.h>
> @@ -239,6 +240,7 @@ static void vfio_platform_release(void *device_data)
> ret, extra_dbg ? extra_dbg : "");
> WARN_ON(1);
> }
> + pm_runtime_put(vdev->device);
> vfio_platform_regions_cleanup(vdev);
> vfio_platform_irq_cleanup(vdev);
> }
> @@ -269,6 +271,10 @@ static int vfio_platform_open(void *device_data)
> if (ret)
> goto err_irq;
>
> + ret = pm_runtime_get_sync(vdev->device);
> + if (ret < 0)
> + goto err_pm;
> +
> ret = vfio_platform_call_reset(vdev, &extra_dbg);
> if (ret && vdev->reset_required) {
> dev_warn(vdev->device, "reset driver is required and reset call failed in open (%d) %s\n",
> @@ -283,6 +289,8 @@ static int vfio_platform_open(void *device_data)
> return 0;
>
> err_rst:
> + pm_runtime_put(vdev->device);
> +err_pm:
> vfio_platform_irq_cleanup(vdev);
> err_irq:
> vfio_platform_regions_cleanup(vdev);
> @@ -690,6 +698,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>
> mutex_init(&vdev->igate);
>
> + pm_runtime_enable(vdev->device);
> return 0;
>
> put_iommu:
> @@ -707,6 +716,7 @@ struct vfio_platform_device *vfio_platform_remove_common(struct device *dev)
> vdev = vfio_del_group_dev(dev);
>
> if (vdev) {
> + pm_runtime_disable(vdev->device);
> vfio_platform_put_reset(vdev);
> vfio_iommu_group_put(dev->iommu_group, dev);
> }
>
^ permalink raw reply
* Re: [PATCH 02/11] PM / devfreq: Fix handling of min/max_freq == 0
From: Matthias Kaehlcke @ 2018-05-29 18:57 UTC (permalink / raw)
To: Chanwoo Choi
Cc: MyungJoo Ham, Kyungmin Park, Arnd Bergmann, Greg Kroah-Hartman,
Rob Herring, Mark Rutland, linux-pm, devicetree, linux-kernel,
Brian Norris, Douglas Anderson, Ørjan Eide,
John Einar Reitan
In-Reply-To: <5B0BA3BB.8060505@samsung.com>
On Mon, May 28, 2018 at 03:37:47PM +0900, Chanwoo Choi wrote:
> Hi,
>
> On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote:
> > Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the
> > devfreq device") initializes df->min/max_freq with the min/max OPP when
> > the device is added. Later commit f1d981eaecf8 ("PM / devfreq: Use the
> > available min/max frequency") adds df->scaling_min/max_freq and the
> > following to the frequency adjustment code:
> >
> > max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq);
> >
> > With the current handling of min/max_freq this is incorrect:
> >
> > Even though df->max_freq is now initialized to a value != 0 user space
> > can still set it to 0, in this case max_freq would be 0 instead of
> > df->scaling_max_freq as intended. In consequence the frequency adjustment
> > is not performed:
> >
> > if (max_freq && freq > max_freq) {
> > freq = max_freq;
> >
> > To fix this set df->min/max freq to the min/max OPP in max/max_freq_store,
> > when the user passes a value of 0. This also prevents df->max_freq from
> > being set below the min OPP when df->min_freq is 0, and similar for
> > min_freq. Since it is now guaranteed that df->min/max_freq can't be 0 the
> > checks for this case can be removed.
> >
> > Fixes: f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency")
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > drivers/devfreq/devfreq.c | 30 ++++++++++++++++++------------
> > 1 file changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index 0057ef5b0a98..67da4e7b486b 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -283,11 +283,11 @@ int update_devfreq(struct devfreq *devfreq)
> > max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq);
> > min_freq = MAX(devfreq->scaling_min_freq, devfreq->min_freq);
> >
> > - if (min_freq && freq < min_freq) {
> > + if (freq < min_freq) {
> > freq = min_freq;
> > flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> > }
> > - if (max_freq && freq > max_freq) {
> > + if (freq > max_freq) {
> > freq = max_freq;
> > flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */
> > }
> > @@ -1123,17 +1123,20 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
> > struct devfreq *df = to_devfreq(dev);
> > unsigned long value;
> > int ret;
> > - unsigned long max;
> >
> > ret = sscanf(buf, "%lu", &value);
> > if (ret != 1)
> > return -EINVAL;
> >
> > mutex_lock(&df->lock);
> > - max = df->max_freq;
> > - if (value && max && value > max) {
> > - ret = -EINVAL;
> > - goto unlock;
> > +
> > + if (value) {
> > + if (value > df->max_freq) {
> > + ret = -EINVAL;
> > + goto unlock;
> > + }
> > + } else {
> > + value = df->profile->freq_table[df->profile->max_state - 1];
> > }
>
> If you want to prevent that df->min_freq is zero,
> you should reinitialize 'value' as following.
> Because freq_table must be in ascending order.
> value = df->profile->freq_table[0];
Thanks for pointing this out!
The devfreq device I tested with (a Mali GPU) uses descending order
for some reason, and I assumed that's the usual order.
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_devfreq.c#208
It seems the ordering doesn't have any impact beyond this patch. If
the order isn't mandatory for drivers that set up their own freq_table
we should probably support both cases to be safe.
> > @@ -1158,17 +1161,20 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
> > struct devfreq *df = to_devfreq(dev);
> > unsigned long value;
> > int ret;
> > - unsigned long min;
> >
> > ret = sscanf(buf, "%lu", &value);
> > if (ret != 1)
> > return -EINVAL;
> >
> > mutex_lock(&df->lock);
> > - min = df->min_freq;
> > - if (value && min && value < min) {
> > - ret = -EINVAL;
> > - goto unlock;
> > +
> > + if (!value) {
> > + value = df->profile->freq_table[0];
>
> ditto.
> value = df->profile->freq_table[df->profile->max_state - 1];
>
> > + } else {
> > + if (value < df->min_freq) {
> > + ret = -EINVAL;
> > + goto unlock;
> > + }
> > }
> >
> > df->max_freq = value;
> >
>
> Actually, min_freq_store() and max_freq_store() are very similar.
> But, this patch changed the order of conditional statement as following:
> If there is no special reason, you better to keep the same format
> for the readability.
>
>
> min_freq_store()
> if (value) {
> ...
> } else {
> value = df->profile->freq_table[df->profile->max_state - 1];
> }
>
>
> max_freq_store()
> if (!value) {
> value = df->profile->freq_table[0];
> } else {
> ...
>
Agreed, better use the same format, I'll update it in the next revision.
^ permalink raw reply
* Re: [PATCH v3 21/27] x86/ftrace: Adapt function tracing for PIE support
From: Thomas Garnier via Virtualization @ 2018-05-29 18:37 UTC (permalink / raw)
To: Steven Rostedt
Cc: Kate Stewart, Nicolas Pitre, the arch/x86 maintainers,
Sergey Senozhatsky, Len Brown, Tom Lendacky, Peter Zijlstra,
Yonghong Song, Christopher Li, Dave Hansen, Dominik Brodowski,
LKML, Masahiro Yamada, Jan Beulich, Pavel Machek, H . Peter Anvin,
Kernel Hardening, Christoph Lameter, Alok Kataria,
Linux Doc Mailing List, linux-arch, Jonathan Corbet <corbet@
In-Reply-To: <CAJcbSZGQmQM7PuER0kEqt3aG9O-8vh-g1EA2jnkVsJPx-Htvrw@mail.gmail.com>
On Thu, May 24, 2018 at 1:41 PM Thomas Garnier <thgarnie@google.com> wrote:
> On Thu, May 24, 2018 at 1:16 PM Steven Rostedt <rostedt@goodmis.org>
wrote:
> > On Thu, 24 May 2018 13:40:24 +0200
> > Petr Mladek <pmladek@suse.com> wrote:
> > > On Wed 2018-05-23 12:54:15, Thomas Garnier wrote:
> > > > When using -fPIE/PIC with function tracing, the compiler generates a
> > > > call through the GOT (call *__fentry__@GOTPCREL). This instruction
> > > > takes 6 bytes instead of 5 on the usual relative call.
> > > >
> > > > If PIE is enabled, replace the 6th byte of the GOT call by a 1-byte
> nop
> > > > so ftrace can handle the previous 5-bytes as before.
> > > >
> > > > Position Independent Executable (PIE) support will allow to extended
> the
> > > > KASLR randomization range below the -2G memory limit.
> > > >
> > > > Signed-off-by: Thomas Garnier <thgarnie@google.com>
> > > > ---
> > > > arch/x86/include/asm/ftrace.h | 6 +++--
> > > > arch/x86/include/asm/sections.h | 4 ++++
> > > > arch/x86/kernel/ftrace.c | 42
> +++++++++++++++++++++++++++++++--
> > > > 3 files changed, 48 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/arch/x86/include/asm/ftrace.h
> b/arch/x86/include/asm/ftrace.h
> > > > index c18ed65287d5..8f2decce38d8 100644
> > > > --- a/arch/x86/include/asm/ftrace.h
> > > > +++ b/arch/x86/include/asm/ftrace.h
> > > > @@ -25,9 +25,11 @@ extern void __fentry__(void);
> > > > static inline unsigned long ftrace_call_adjust(unsigned long addr)
> > > > {
> > > > /*
> > > > - * addr is the address of the mcount call instruction.
> > > > - * recordmcount does the necessary offset calculation.
> > > > + * addr is the address of the mcount call instruction. PIE has
> always a
> > > > + * byte added to the start of the function.
> > > > */
> > > > + if (IS_ENABLED(CONFIG_X86_PIE))
> > > > + addr -= 1;
> > >
> > > This seems to modify the address even for modules that are _not_
> compiled with
> > > PIE, see below.
> > Can one load a module not compiled for PIE in a kernel with PIE?
> > >
> > > > return addr;
> > > > }
> > > >
> > > > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > > > index 01ebcb6f263e..73b3c30cb7a3 100644
> > > > --- a/arch/x86/kernel/ftrace.c
> > > > +++ b/arch/x86/kernel/ftrace.c
> > > > @@ -135,6 +135,44 @@ ftrace_modify_code_direct(unsigned long ip,
> unsigned const char *old_code,
> > > > return 0;
> > > > }
> > > >
> > > > +/* Bytes before call GOT offset */
> > > > +const unsigned char got_call_preinsn[] = { 0xff, 0x15 };
> > > > +
> > > > +static int
> > > > +ftrace_modify_initial_code(unsigned long ip, unsigned const char
> *old_code,
> > > > + unsigned const char *new_code)
> > > > +{
> > > > + unsigned char replaced[MCOUNT_INSN_SIZE + 1];
> > > > +
> > > > + ftrace_expected = old_code;
> > > > +
> > > > + /*
> > > > + * If PIE is not enabled or no GOT call was found, default to
the
> > > > + * original approach to code modification.
> > > > + */
> > > > + if (!IS_ENABLED(CONFIG_X86_PIE) ||
> > > > + probe_kernel_read(replaced, (void *)ip, sizeof(replaced)) ||
> > > > + memcmp(replaced, got_call_preinsn,
sizeof(got_call_preinsn)))
> > > > + return ftrace_modify_code_direct(ip, old_code,
new_code);
> > >
> > > And this looks like an attempt to handle modules compiled without
> > > PIE. Does it works with the right ip in that case?
> > I'm guessing the || is for the "or no GOT call was found", but it
> > doesn't explain why no GOT would be found.
> Yes, maybe I could have made it work by using text_ip_addr earlier.
> > >
> > > I wonder if a better solution would be to update
> > > scripts/recordmcount.c to store the incremented location into the
> module.
> I will look into it.
Found a way to properly change the __mcount_loc using the preprocessing
(removing the need for -1 on the addr). It will be part of the next version.
Thanks for the feedback.
> > If recordmcount.c can handle this, then I think that's the preferred
> > approach. Thanks!
> > -- Steve
> > >
> > > IMPORTANT: I have only vague picture about how this all works. It is
> > > possible that I am completely wrong. The code might be correct,
> > > especially if you tested this situation.
> > >
> > > Best Regards,
> > > Petr
> --
> Thomas
--
Thomas
^ permalink raw reply
* Re: [PATCH 01/11] PM / devfreq: Init user limits from OPP limits, not viceversa
From: Matthias Kaehlcke @ 2018-05-29 18:06 UTC (permalink / raw)
To: Chanwoo Choi
Cc: MyungJoo Ham, Kyungmin Park, Arnd Bergmann, Greg Kroah-Hartman,
Rob Herring, Mark Rutland, linux-pm, devicetree, linux-kernel,
Brian Norris, Douglas Anderson
In-Reply-To: <5B0B9319.9060709@samsung.com>
On Mon, May 28, 2018 at 02:26:49PM +0900, Chanwoo Choi wrote:
> Hi,
>
> On 2018년 05월 26일 05:30, Matthias Kaehlcke wrote:
> > Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding
> > the devfreq device") introduced the initialization of the user
> > limits min/max_freq from the lowest/highest available OPPs. Later
> > commit f1d981eaecf8 ("PM / devfreq: Use the available min/max
> > frequency") added scaling_min/max_freq, which actually represent
> > the frequencies of the lowest/highest available OPP. scaling_min/
> > max_freq are initialized with the values from min/max_freq, which
> > is totally correct in the context, but a bit awkward to read.
> >
> > Swap the initialization and assign scaling_min/max_freq with the
> > OPP freqs and then the user limts min/max_freq with scaling_min/
> > max_freq.
> >
> > Needless to say that this change is a NOP, intended to improve
> > readability.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > drivers/devfreq/devfreq.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index fe2af6aa88fc..0057ef5b0a98 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -604,21 +604,21 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > mutex_lock(&devfreq->lock);
> > }
> >
> > - devfreq->min_freq = find_available_min_freq(devfreq);
> > - if (!devfreq->min_freq) {
> > + devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> > + if (!devfreq->scaling_min_freq) {
> > mutex_unlock(&devfreq->lock);
> > err = -EINVAL;
> > goto err_dev;
> > }
> > - devfreq->scaling_min_freq = devfreq->min_freq;
> > + devfreq->min_freq = devfreq->scaling_min_freq;
> >
> > - devfreq->max_freq = find_available_max_freq(devfreq);
> > - if (!devfreq->max_freq) {
> > + devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> > + if (!devfreq->scaling_max_freq) {
> > mutex_unlock(&devfreq->lock);
> > err = -EINVAL;
> > goto err_dev;
> > }
> > - devfreq->scaling_max_freq = devfreq->max_freq;
> > + devfreq->max_freq = devfreq->scaling_max_freq;
> >
> > dev_set_name(&devfreq->dev, "devfreq%d",
> > atomic_inc_return(&devfreq_no));
> >
>
> I already replied with my Reviewed-by tag. You are missing my tag.
>
> Again,
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Sorry, forgot to add the tag after creating the series. I now added it
to my local tree.
^ permalink raw reply
* Re: [PATCH 1/2] kernel/SRCU: provide a static initializer
From: Paul E. McKenney @ 2018-05-29 17:26 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sebastian Andrzej Siewior, Linux PM, Linux Kernel Mailing List,
Rafael J. Wysocki, Viresh Kumar, Thomas Gleixner
In-Reply-To: <CAJZ5v0j_O=nL6Eym5GqGGDC8LPB5vu-YAp9DFfZSJsHd-TMEmg@mail.gmail.com>
On Tue, May 29, 2018 at 07:14:49PM +0200, Rafael J. Wysocki wrote:
> On Tue, May 29, 2018 at 2:04 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Tue, May 29, 2018 at 10:09:51AM +0200, Rafael J. Wysocki wrote:
> >> On Fri, May 25, 2018 at 12:19 PM, Sebastian Andrzej Siewior
> >> <bigeasy@linutronix.de> wrote:
> >> > There are macros for static initializer for the three out of four
> >> > possible notifier types, that are:
> >> > ATOMIC_NOTIFIER_HEAD()
> >> > BLOCKING_NOTIFIER_HEAD()
> >> > RAW_NOTIFIER_HEAD()
> >> >
> >> > This patch provides a static initilizer for the forth type to make it
> >> > complete.
> >> >
> >> > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> >>
> >> I cannot apply this without an ACK from Paul.
> >
> > I have both queued, but if you would prefer to take them, then for the
> > SRCU piece:
> >
> > Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> Thanks!
>
> I'll route them along with the other cpufreq material then.
Please let me know when you have them queued so that I can remove
them from -rcu.
Thanx, Paul
^ permalink raw reply
* [PATCH v4] vfio: platform: Fix using devices in PM Domains
From: Geert Uytterhoeven @ 2018-05-29 17:15 UTC (permalink / raw)
To: Eric Auger, Alex Williamson
Cc: kvm, linux-renesas-soc, linux-pm, linux-kernel,
Geert Uytterhoeven
If a device is part of a PM Domain (e.g. power and/or clock domain), its
power state is managed using Runtime PM. Without Runtime PM, the device
may not be powered up or clocked, causing subtle failures, crashes, or
system lock-ups when the device is accessed by the guest.
Fix this by adding Runtime PM support, powering the device when the VFIO
device is opened by the guest.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
Against linux-vfio/next
v4:
- Keep device powered while issuing reset,
v3:
- Drop controversial note about unsafeness of exporting fine-grained
power management from host to guest,
v2:
- Improve wording,
- Add Reviewed-by.
---
drivers/vfio/platform/vfio_platform_common.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index b60bb5326668498c..c0cd824be2b767be 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -17,6 +17,7 @@
#include <linux/iommu.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/uaccess.h>
@@ -239,6 +240,7 @@ static void vfio_platform_release(void *device_data)
ret, extra_dbg ? extra_dbg : "");
WARN_ON(1);
}
+ pm_runtime_put(vdev->device);
vfio_platform_regions_cleanup(vdev);
vfio_platform_irq_cleanup(vdev);
}
@@ -269,6 +271,10 @@ static int vfio_platform_open(void *device_data)
if (ret)
goto err_irq;
+ ret = pm_runtime_get_sync(vdev->device);
+ if (ret < 0)
+ goto err_pm;
+
ret = vfio_platform_call_reset(vdev, &extra_dbg);
if (ret && vdev->reset_required) {
dev_warn(vdev->device, "reset driver is required and reset call failed in open (%d) %s\n",
@@ -283,6 +289,8 @@ static int vfio_platform_open(void *device_data)
return 0;
err_rst:
+ pm_runtime_put(vdev->device);
+err_pm:
vfio_platform_irq_cleanup(vdev);
err_irq:
vfio_platform_regions_cleanup(vdev);
@@ -690,6 +698,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
mutex_init(&vdev->igate);
+ pm_runtime_enable(vdev->device);
return 0;
put_iommu:
@@ -707,6 +716,7 @@ struct vfio_platform_device *vfio_platform_remove_common(struct device *dev)
vdev = vfio_del_group_dev(dev);
if (vdev) {
+ pm_runtime_disable(vdev->device);
vfio_platform_put_reset(vdev);
vfio_iommu_group_put(dev->iommu_group, dev);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/2] kernel/SRCU: provide a static initializer
From: Rafael J. Wysocki @ 2018-05-29 17:14 UTC (permalink / raw)
To: Paul McKenney
Cc: Rafael J. Wysocki, Sebastian Andrzej Siewior, Linux PM,
Linux Kernel Mailing List, Rafael J. Wysocki, Viresh Kumar,
Thomas Gleixner
In-Reply-To: <20180529120443.GE3803@linux.vnet.ibm.com>
On Tue, May 29, 2018 at 2:04 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Tue, May 29, 2018 at 10:09:51AM +0200, Rafael J. Wysocki wrote:
>> On Fri, May 25, 2018 at 12:19 PM, Sebastian Andrzej Siewior
>> <bigeasy@linutronix.de> wrote:
>> > There are macros for static initializer for the three out of four
>> > possible notifier types, that are:
>> > ATOMIC_NOTIFIER_HEAD()
>> > BLOCKING_NOTIFIER_HEAD()
>> > RAW_NOTIFIER_HEAD()
>> >
>> > This patch provides a static initilizer for the forth type to make it
>> > complete.
>> >
>> > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>
>> I cannot apply this without an ACK from Paul.
>
> I have both queued, but if you would prefer to take them, then for the
> SRCU piece:
>
> Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Thanks!
I'll route them along with the other cpufreq material then.
Cheers,
Rafael
^ permalink raw reply
* Re: [PATCH] cpupower : Fix header name to read idle state name
From: Shuah Khan @ 2018-05-29 16:55 UTC (permalink / raw)
To: Abhishek Goel, trenn, linux-pm, linux-kernel; +Cc: s1seetee, Shuah Khan
In-Reply-To: <20180528110303.12860-1-huntbag@linux.vnet.ibm.com>
On 05/28/2018 05:03 AM, Abhishek Goel wrote:
> The names of the idle states in the output of cpupower monitor command are
> truncated to 4 characters. On POWER9, this creates ambiguity as the states
> are named "stop0", "stop1", etc.
>
> root:~# cpupower monitor
> |Idle_Stats
> PKG |CORE|CPU | snoo | stop | stop | stop | stop | stop | stop
> 0| 0| 0| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 1.90
> 0| 0| 1| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00
> 0| 0| 2| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00
> 0| 0| 3| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00
>
> This patch modifies the output to print the state name that results in a
> legible output. The names will be printed with atmost 1 padding in left.
>
> root:~# cpupower monitor
> | Idle_Stats
> PKG|CORE| CPU|snooze|stop0L| stop0|stop1L| stop1|stop2L| stop2
> 0| 0| 0| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 0.72
> 0| 0| 1| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00
> 0| 0| 2| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00
> 0| 0| 3| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00| 0.00
>
> This patch does not affect the output for intel.
> Output for intel before applying the patch:
>
> root:~# cpupower monitor
> |Idle_Stats
> CPU | POLL | C1-S | C1E- | C3-S | C6-S | C7s- | C8-S | C9-S | C10-
> 0| 0.00| 0.14| 0.39| 0.35| 7.41| 0.00| 17.67| 1.01| 70.03
> 2| 0.00| 0.19| 0.47| 0.10| 6.50| 0.00| 29.66| 2.17| 58.07
> 1| 0.00| 0.11| 0.50| 1.50| 9.11| 0.18| 18.19| 0.40| 66.63
> 3| 0.00| 0.67| 0.42| 0.03| 5.84| 0.00| 12.58| 0.77| 77.14
>
> Output for intel after applying the patch:
>
> root:~# cpupower monitor
> | Idle_Stats
> CPU| POLL | C1-S | C1E- | C3-S | C6-S | C7s- | C8-S | C9-S | C10-
> 0| 0.03| 0.33| 1.01| 0.27| 3.03| 0.00| 19.18| 0.00| 71.24
> 2| 0.00| 1.58| 0.58| 0.42| 8.55| 0.09| 21.11| 0.99| 63.32
> 1| 0.00| 1.26| 0.88| 0.43| 9.00| 0.02| 7.78| 4.65| 71.91
> 3| 0.00| 0.30| 0.42| 0.06| 13.62| 0.21| 30.29| 0.00| 52.45
>
> Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
> ---
>
> Previous attempt was made by Seeteena Thoufeek. This is the patch list
> link: https://patchwork.kernel.org/project/linux-pm/list/?submitter=
> 172041&state=*&archive=both
>
> .../utils/idle_monitor/cpuidle_sysfs.c | 15 ++++++++
> .../utils/idle_monitor/cpupower-monitor.c | 35 +++++++++++--------
> .../utils/idle_monitor/cpupower-monitor.h | 9 +++++
> 3 files changed, 44 insertions(+), 15 deletions(-)
>
> diff --git a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
> index 5b3205f16217..5b8c4956ff9a 100644
> --- a/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
> +++ b/tools/power/cpupower/utils/idle_monitor/cpuidle_sysfs.c
> @@ -126,6 +126,20 @@ void fix_up_intel_idle_driver_name(char *tmp, int num)
> }
> }
>
> +#ifdef __powerpc__
> +void map_power_idle_state_name(char *tmp)
> +{
> + if (!strncmp(tmp, "stop0_lite", CSTATE_NAME_LEN))
> + strcpy(tmp, "stop0L");
> + else if (!strncmp(tmp, "stop1_lite", CSTATE_NAME_LEN))
> + strcpy(tmp, "stop1L");
> + else if (!strncmp(tmp, "stop2_lite", CSTATE_NAME_LEN))
> + strcpy(tmp, "stop2L");
> +}
> +#else
> +void map_power_idle_state_name(char *tmp) { }
> +#endif
> +
> static struct cpuidle_monitor *cpuidle_register(void)
> {
> int num;
> @@ -145,6 +159,7 @@ static struct cpuidle_monitor *cpuidle_register(void)
> if (tmp == NULL)
> continue;
>
> + map_power_idle_state_name(tmp);
> fix_up_intel_idle_driver_name(tmp, num);
> strncpy(cpuidle_cstates[num].name, tmp, CSTATE_NAME_LEN - 1);
> free(tmp);
> diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> index 05f953f0f0a0..051da0a7c454 100644
> --- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> +++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.c
> @@ -70,36 +70,43 @@ void print_n_spaces(int n)
> printf(" ");
> }
>
> -/* size of s must be at least n + 1 */
> +/*s is filled with left and right spaces
> + *to make its length atleast n+1
> + */
> int fill_string_with_spaces(char *s, int n)
> {
> + char *temp;
> int len = strlen(s);
> - if (len > n)
> +
> + if (len >= n)
> return -1;
> +
> + temp = malloc(sizeof(char) * (n+1));
> for (; len < n; len++)
> s[len] = ' ';
> s[len] = '\0';
> + snprintf(temp, n+1, " %s", s);
> + strcpy(s, temp);
> + free(temp);
> return 0;
> }
>
> +#define MAX_COL_WIDTH 6
> void print_header(int topology_depth)
> {
> int unsigned mon;
> int state, need_len;
> cstate_t s;
> char buf[128] = "";
> - int percent_width = 4;
>
> fill_string_with_spaces(buf, topology_depth * 5 - 1);
> printf("%s|", buf);
>
> for (mon = 0; mon < avail_monitors; mon++) {
> - need_len = monitors[mon]->hw_states_num * (percent_width + 3)
> + need_len = monitors[mon]->hw_states_num * (MAX_COL_WIDTH + 1)
> - 1;
> - if (mon != 0) {
> - printf("|| ");
> - need_len--;
> - }
> + if (mon != 0)
> + printf("||");
> sprintf(buf, "%s", monitors[mon]->name);
> fill_string_with_spaces(buf, need_len);
> printf("%s", buf);
> @@ -107,23 +114,21 @@ void print_header(int topology_depth)
> printf("\n");
>
> if (topology_depth > 2)
> - printf("PKG |");
> + printf(" PKG|");
> if (topology_depth > 1)
> printf("CORE|");
> if (topology_depth > 0)
> - printf("CPU |");
> + printf(" CPU|");
>
> for (mon = 0; mon < avail_monitors; mon++) {
> if (mon != 0)
> - printf("|| ");
> - else
> - printf(" ");
> + printf("||");
> for (state = 0; state < monitors[mon]->hw_states_num; state++) {
> if (state != 0)
> - printf(" | ");
> + printf("|");
> s = monitors[mon]->hw_states[state];
> sprintf(buf, "%s", s.name);
> - fill_string_with_spaces(buf, percent_width);
> + fill_string_with_spaces(buf, MAX_COL_WIDTH);
> printf("%s", buf);
> }
> printf(" ");
> diff --git a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
> index 9e43f3371fbc..2ae50b499e0a 100644
> --- a/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
> +++ b/tools/power/cpupower/utils/idle_monitor/cpupower-monitor.h
> @@ -15,7 +15,16 @@
>
> #define MONITORS_MAX 20
> #define MONITOR_NAME_LEN 20
> +
> +/* CSTATE_NAME_LEN is limited by header field width defined
> + * in cpupower-monitor.c. Header field width is defined to be
> + * sum of percent width and two spaces for padding.
> + */
> +#ifdef __powerpc__
> +#define CSTATE_NAME_LEN 7
> +#else
> #define CSTATE_NAME_LEN 5
> +#endif
> #define CSTATE_DESC_LEN 60
>
> int cpu_count;
>
Thanks for the patch. Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux.git cpupower
for 4.18-rc1
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v3 09/27] x86/acpi: Adapt assembly for PIE support
From: Thomas Garnier @ 2018-05-29 15:55 UTC (permalink / raw)
To: Pavel Machek
Cc: Herbert Xu, David S . Miller, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf, Greg KH,
Philippe Ombredanne, Kate Stewart, Arnaldo Carvalho de Melo,
Yonghong Song, Andrey Ryabinin, Kees Cook, Tom Lendacky,
Kirill A. Shutemov, Andy Lutomirski, Dominik Brodowski,
Borislav Petkov, Borislav Petkov, Rafael J. Wysocki, Len Brown,
Juergen Gross <jgros>
In-Reply-To: <20180529123114.GB21399@amd>
On Tue, May 29, 2018 at 5:31 AM Pavel Machek <pavel@ucw.cz> wrote:
> On Fri 2018-05-25 10:00:04, Thomas Garnier wrote:
> > On Fri, May 25, 2018 at 2:14 AM Pavel Machek <pavel@ucw.cz> wrote:
> >
> > > On Thu 2018-05-24 09:35:42, Thomas Garnier wrote:
> > > > On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:
> > > >
> > > > > On Wed 2018-05-23 12:54:03, Thomas Garnier wrote:
> > > > > > Change the assembly code to use only relative references of
symbols
> > for
> > > > the
> > > > > > kernel to be PIE compatible.
> > > > > >
> > > > > > Position Independent Executable (PIE) support will allow to
> > extended the
> > > > > > KASLR randomization range below the -2G memory limit.
> > > >
> > > > > What testing did this get?
> > > >
> > > > Tested boot, hibernation and performance on qemu and dedicated
machine.
> >
> > > Well, this is suspend, not hibernation code.
> >
> > > So "sudo pm-suspend" or "echo mem > /sys/power/state" would be good
> > > way to test this.
> >
> > Thanks, it worked. I added this to the testsuite I use for KASLR.
> Thanks!
> You can add my Acked-by:.
Will do. Thanks for the review.
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
Thomas
^ permalink raw reply
* Re: [PATCH] cpufreq / CPPC: Add cpuinfo_cur_freq support for CPPC
From: Prakash, Prashanth @ 2018-05-29 15:44 UTC (permalink / raw)
To: George Cherian, George Cherian, linux-kernel, linux-pm; +Cc: rjw, viresh.kumar
In-Reply-To: <0959a657-5a98-1d4c-ed9d-ed057e3bd9ad@caviumnetworks.com>
On 5/28/2018 1:09 AM, George Cherian wrote:
> Hi Prashanth,
>
> On 05/26/2018 02:30 AM, Prakash, Prashanth wrote:
>>
>> On 5/25/2018 12:27 AM, George Cherian wrote:
>>> Hi Prashanth,
>>>
>>> On 05/25/2018 12:55 AM, Prakash, Prashanth wrote:
>>>> Hi George,
>>>>
>>>> On 5/22/2018 5:42 AM, George Cherian wrote:
>>>>> Per Section 8.4.7.1.3 of ACPI 6.2, The platform provides performance
>>>>> feedback via set of performance counters. To determine the actual
>>>>> performance level delivered over time, OSPM may read a set of
>>>>> performance counters from the Reference Performance Counter Register
>>>>> and the Delivered Performance Counter Register.
>>>>>
>>>>> OSPM calculates the delivered performance over a given time period by
>>>>> taking a beginning and ending snapshot of both the reference and
>>>>> delivered performance counters, and calculating:
>>>>>
>>>>> delivered_perf = reference_perf X (delta of delivered_perf counter / delta of reference_perf counter).
>>>>>
>>>>> Implement the above and hook this to the cpufreq->get method.
>>>>>
>>>>> Signed-off-by: George Cherian <george.cherian@cavium.com>
>>>>> ---
>>>>> drivers/cpufreq/cppc_cpufreq.c | 44 ++++++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 44 insertions(+)
>>>>>
>>>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>>>> index b15115a..a046915 100644
>>>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>>>> @@ -240,10 +240,54 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>>> return ret;
>>>>> }
>>>>> +static int cppc_get_rate_from_fbctrs(struct cppc_perf_fb_ctrs fb_ctrs_t0,
>>>>> + struct cppc_perf_fb_ctrs fb_ctrs_t1)
>>>>> +{
>>>>> + u64 delta_reference, delta_delivered;
>>>>> + u64 reference_perf, ratio;
>>>>> +
>>>>> + reference_perf = fb_ctrs_t0.reference_perf;
>>>>> + if (fb_ctrs_t1.reference > fb_ctrs_t0.reference)
>>>>> + delta_reference = fb_ctrs_t1.reference - fb_ctrs_t0.reference;
>>>>> + else /* Counters would have wrapped-around */
>>>>> + delta_reference = ((u64)(~((u64)0)) - fb_ctrs_t0.reference) +
>>>>> + fb_ctrs_t1.reference;
>>>>> +
>>>>> + if (fb_ctrs_t1.delivered > fb_ctrs_t0.delivered)
>>>>> + delta_delivered = fb_ctrs_t1.delivered - fb_ctrs_t0.delivered;
>>>>> + else /* Counters would have wrapped-around */
>>>>> + delta_delivered = ((u64)(~((u64)0)) - fb_ctrs_t0.delivered) +
>>>>> + fb_ctrs_t1.delivered;
>>>> We need to check that the wraparound time is long enough to make sure that
>>>> the counters cannot wrap around more than once. We can register a get() api
>>>> only after checking that wraparound time value is reasonably high.
>>>>
>>>> I am not aware of any platforms where wraparound time is soo short, but
>>>> wouldn't hurt to check once during init.
>>> By design the wraparound time is a 64 bit counter, for that matter even
>>> all the feedback counters too are 64 bit counters. I don't see any
>>> chance in which the counters can wraparound twice in back to back reads.
>>> The only situation is in which system itself is running at a really high
>>> frequency. Even in that case today's spec is not sufficient to support the same.
>>
>> The spec doesn't say these have to be 64bit registers. The wraparound
>> counter register is in spec to communicate the worst case(shortest)
>> counter rollover time.
>
> Spec says these are 32 or 64 bit registers. Spec also defines counter
> wraparound time in seconds. The minimum value possible is 1 as zero means the counters are never assumed to wrap around. Even in platforms with value set as 1 (1 sec) I dont really see a situation in which
> the counter can wraparound twice if we are putting a delay of 2usec
> between sampling.
ok.
>
>>
>> As as mentioned before this is just a defensive check to make sure that
>> the platform has not set it to some very low number (which is allowed
>> by the spec).
> It might be unnecessary to have a check like this.
>>
>>>
>>>>> +
>>>>> + if (delta_reference) /* Check to avoid divide-by zero */
>>>>> + ratio = (delta_delivered * 1000) / delta_reference;
>>>> Why not just return the computed value here instead of *1000 and later /1000?
>>>> return (ref_per * delta_del) / delta_ref;
>>> Yes.
>>>>> + else
>>>>> + return -EINVAL;
>>>> Instead of EINVAL, i think we should return current frequency.
>>>>
>>> Sorry, I didn't get you, How do you calculate the current frequency?
>>> Did you mean reference performance?
>> I mean the performance that OSPM/Linux had requested earlier.
>> i.e the desired_perf
> Okay, I will make necessary changes for this in v2.
>
>>>
>>>> The counters can pause if CPUs are in idle state during our sampling interval, so
>>>> If the counters did not progress, it is reasonable to assume the delivered perf was
>>>> equal to desired perf.
>>> No, that is wrong. Here the check is for reference performance delta.
>>> This counter can never pause. In case of cpuidle only the delivered counters could pause. Delivered counters will pause only if the particular core enters power down mode, Otherwise we would be still clocking the core and we should be getting a delta across 2 sampling periods. In case if the reference counter is paused which by design is not correct then there is no point in returning reference performance numbers. That too is wrong. In case the low level FW is not updating the
>>> counters properly then it should be evident till Linux, instead of returning a bogus frequency.
>>
>> Again you are describing how it works on a specific platform and not
>> how it is described in spec. Section 8.4.7.1.3.1.1 of ACPI 6.2 states
>> "The Reference Performance Counter Register counts at a fixed rate
>> any time the processor is active."
>> > Implies the counters *may* pause in idle state -I can imagine an
>> implementation where you can keep this counter running and
>> account for it via delivered counter, but we cannot make any
>> assumptions outside of what the spec describes.
>>
>>>>
>>>> Even if platform wanted to limit, since the CPUs were asleep(idle) we could not have
>>>> observed lower performance, so we will not throw off any logic that could be driven
>>>> using the returned value.
>>>>> +
>>>>> + return (reference_perf * ratio) / 1000;
>>>> This should be converted to KHz as cpufreq is not aware of CPPC abstract scale
>>> In our platform all performance registers are implemented in KHz. Because of which we never had an issue with conversion. I am not
>>> aware whether ACPI mandates to use any particular unit. How is that
>>> implemented in your platform? Just to avoid any extra conversion don't
>>> you feel it is better to always report in KHz from firmware.
>> Again think of spec not a specific platform :)
>> - The CPPC spec works on abstract scale and cpufreq works in KHz.
>> - The above computed value is in abstract scale
>> - The abstarct scale may be in KHz on your platform, but we cannot assume the
>> same about all the platforms
> For now can I assume it to be in KHz only?
No, it will break platforms where abstract scale is not in KHz.
> I am not sure how to convert the abstract scale to Khz.
> Can you please give me some pointers on the same?
Take a look at cppc_cpufreq_perf_to_khz and cppc_cpufreq_khz_to_perf
in the same file (cppc_cpufreq.c). We use this in almost every function
registered with core cpufreq.
||
> In spec there is currently no interface which tells what is the abstract
> scale!!
CPPC v3 adds some additional hooks for this. On CPPC v2, we try to use
few DMI table entries to get the ratio between abstract scale and KHz.
>>>
>>>>> +}
>>>>> +
>>>>> +static unsigned int cppc_cpufreq_get_rate(unsigned int cpunum)
>>>>> +{
>>>>> + struct cppc_perf_fb_ctrs fb_ctrs_t0 = {0}, fb_ctrs_t1 = {0};
>>>>> + int ret;
>>>>> +
>>>>> + ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t0);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + ret = cppc_get_perf_ctrs(cpunum, &fb_ctrs_t1);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + return cppc_get_rate_from_fbctrs(fb_ctrs_t0, fb_ctrs_t1);
>>>>> +}
>>>> We need to make sure that we get a reasonably sample so make sure the reported
>>>> performance is accurate.
>>>> The counters can capture short transient throttling/limiting, so by sampling a really
>>>> short duration of time we could return quite inaccurate measure of performance.
>>>>
>>> I would say it as a momentary thing only when the frequency is being ramped up/down.
>> This exact behavior would depend on how different limiting functions are implemented.
>> So this would vary from one platform to another.
>>>
>>>> We need to include some reasonable delay between the two calls. What is reasonable
>>>> is debatable - so this can be few(2-10) microseconds defined as default. If the same value
>>>> doesn't work for all the platforms, we might need to add a platform specific value.
>>>>
>>> cppc_get_perf_ctrs itself is a slow call, we have to format the CPC packet and ring a doorbell and then the response to be read from the shared registers. My initial implementation had a delay but in testing,
>>> I found that it was unnecessary to have such a delay. Can you please
>>> let me know whether it works without delay in your platform?
>>>
>>> Or else let me know whether udelay(10) is sufficient in between the
>>> calls.
>> Feedback counters need not be in PCC .
>> 2us should be sufficient.
> Yes I will add this to v2.
>>>>> +
>>>>> static struct cpufreq_driver cppc_cpufreq_driver = {
>>>>> .flags = CPUFREQ_CONST_LOOPS,
>>>>> .verify = cppc_verify_policy,
>>>>> .target = cppc_cpufreq_set_target,
>>>>> + .get = cppc_cpufreq_get_rate,
>>>>> .init = cppc_cpufreq_cpu_init,
>>>>> .stop_cpu = cppc_cpufreq_stop_cpu,
>>>>> .name = "cppc_cpufreq",
>>>>
>>
^ permalink raw reply
* Re: [PATCH v3 09/27] x86/acpi: Adapt assembly for PIE support
From: Pavel Machek @ 2018-05-29 12:31 UTC (permalink / raw)
To: Thomas Garnier
Cc: Kate Stewart, Nicolas Pitre, the arch/x86 maintainers,
Sergey Senozhatsky, Petr Mladek, Len Brown, Peter Zijlstra,
Yonghong Song, Christopher Li, Dave Hansen, Dominik Brodowski,
LKML, Masahiro Yamada, Jan Beulich, H . Peter Anvin,
Kernel Hardening, Christoph Lameter, Alok Kataria,
Linux Doc Mailing List, linux-arch, Jonathan Corbet, Herbert Xu
In-Reply-To: <CAJcbSZH17D01Stk4vRKKzjW6dxvK8x+S9sWL6vUopSP9=-x7Nw@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1182 bytes --]
On Fri 2018-05-25 10:00:04, Thomas Garnier wrote:
> On Fri, May 25, 2018 at 2:14 AM Pavel Machek <pavel@ucw.cz> wrote:
>
> > On Thu 2018-05-24 09:35:42, Thomas Garnier wrote:
> > > On Thu, May 24, 2018 at 4:03 AM Pavel Machek <pavel@ucw.cz> wrote:
> > >
> > > > On Wed 2018-05-23 12:54:03, Thomas Garnier wrote:
> > > > > Change the assembly code to use only relative references of symbols
> for
> > > the
> > > > > kernel to be PIE compatible.
> > > > >
> > > > > Position Independent Executable (PIE) support will allow to
> extended the
> > > > > KASLR randomization range below the -2G memory limit.
> > >
> > > > What testing did this get?
> > >
> > > Tested boot, hibernation and performance on qemu and dedicated machine.
>
> > Well, this is suspend, not hibernation code.
>
> > So "sudo pm-suspend" or "echo mem > /sys/power/state" would be good
> > way to test this.
>
> Thanks, it worked. I added this to the testsuite I use for KASLR.
Thanks!
You can add my Acked-by:.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] cpuidle/powernv : Add Description for cpuidle state
From: Michael Ellerman @ 2018-05-29 12:30 UTC (permalink / raw)
To: Stewart Smith, rjw, daniel.lezcano, benh, paulus, linux-pm,
linuxppc-dev, linux-kernel
Cc: Abhishek Goel
In-Reply-To: <87d0xf6y16.fsf@linux.vnet.ibm.com>
Stewart Smith <stewart@linux.ibm.com> writes:
> Abhishek Goel <huntbag@linux.vnet.ibm.com> writes:
>> @@ -215,7 +216,7 @@ static inline void add_powernv_state(int index, const char *name,
>> u64 psscr_val, u64 psscr_mask)
>> {
>> strlcpy(powernv_states[index].name, name, CPUIDLE_NAME_LEN);
>> - strlcpy(powernv_states[index].desc, name, CPUIDLE_NAME_LEN);
>> + strlcpy(powernv_states[index].desc, desc, CPUIDLE_DESC_LEN);
>
> We should still fall back to using name in the event of desc being null,
> as not all firmware will expose the descriptions.
>
>> @@ -311,6 +313,11 @@ static int powernv_add_idle_states(void)
>> pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
>> goto out;
>> }
>> + if (of_property_read_string_array(power_mgt,
>> + "ibm,cpu-idle-state-descs", descs, dt_idle_states) < 0) {
>> + pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-descs in DT\n");
>> + goto out;
>> + }
>
> I don't think pr_warn is appropriate here, as for all current released
> firmware we don't have that property. I think perhaps just silently
> continuing on is okay, as we have to keep compatibility with that firmware.
+1
Anyone who's wondering why they're not seeing the names can just look at
the device tree on the machine to see if the property is there or not,
they don't need a printk in dmesg to tell them.
cheers
^ permalink raw reply
* Re: [PATCH 1/2] kernel/SRCU: provide a static initializer
From: Paul E. McKenney @ 2018-05-29 12:04 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Sebastian Andrzej Siewior, Linux PM, Linux Kernel Mailing List,
Rafael J. Wysocki, Viresh Kumar, Thomas Gleixner
In-Reply-To: <CAJZ5v0jEUR7YxzujvZn8WVK+dMxmOxoHNiLKcmgCq9awHR=vYg@mail.gmail.com>
On Tue, May 29, 2018 at 10:09:51AM +0200, Rafael J. Wysocki wrote:
> On Fri, May 25, 2018 at 12:19 PM, Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> > There are macros for static initializer for the three out of four
> > possible notifier types, that are:
> > ATOMIC_NOTIFIER_HEAD()
> > BLOCKING_NOTIFIER_HEAD()
> > RAW_NOTIFIER_HEAD()
> >
> > This patch provides a static initilizer for the forth type to make it
> > complete.
> >
> > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> I cannot apply this without an ACK from Paul.
I have both queued, but if you would prefer to take them, then for the
SRCU piece:
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Thanx, Paul
> > ---
> > include/linux/notifier.h | 34 +++++++++++++++++++++++++++++-----
> > include/linux/srcutiny.h | 6 +++---
> > include/linux/srcutree.h | 6 +++---
> > 3 files changed, 35 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/linux/notifier.h b/include/linux/notifier.h
> > index 6d731110e0db..f35c7bf76143 100644
> > --- a/include/linux/notifier.h
> > +++ b/include/linux/notifier.h
> > @@ -43,9 +43,7 @@
> > * in srcu_notifier_call_chain(): no cache bounces and no memory barriers.
> > * As compensation, srcu_notifier_chain_unregister() is rather expensive.
> > * SRCU notifier chains should be used when the chain will be called very
> > - * often but notifier_blocks will seldom be removed. Also, SRCU notifier
> > - * chains are slightly more difficult to use because they require special
> > - * runtime initialization.
> > + * often but notifier_blocks will seldom be removed.
> > */
> >
> > struct notifier_block;
> > @@ -91,7 +89,7 @@ struct srcu_notifier_head {
> > (name)->head = NULL; \
> > } while (0)
> >
> > -/* srcu_notifier_heads must be initialized and cleaned up dynamically */
> > +/* srcu_notifier_heads must be cleaned up dynamically */
> > extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
> > #define srcu_cleanup_notifier_head(name) \
> > cleanup_srcu_struct(&(name)->srcu);
> > @@ -104,7 +102,13 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
> > .head = NULL }
> > #define RAW_NOTIFIER_INIT(name) { \
> > .head = NULL }
> > -/* srcu_notifier_heads cannot be initialized statically */
> > +
> > +#define SRCU_NOTIFIER_INIT(name, pcpu) \
> > + { \
> > + .mutex = __MUTEX_INITIALIZER(name.mutex), \
> > + .head = NULL, \
> > + .srcu = __SRCU_STRUCT_INIT(name.srcu, pcpu), \
> > + }
> >
> > #define ATOMIC_NOTIFIER_HEAD(name) \
> > struct atomic_notifier_head name = \
> > @@ -116,6 +120,26 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
> > struct raw_notifier_head name = \
> > RAW_NOTIFIER_INIT(name)
> >
> > +#ifdef CONFIG_TREE_SRCU
> > +#define _SRCU_NOTIFIER_HEAD(name, mod) \
> > + static DEFINE_PER_CPU(struct srcu_data, \
> > + name##_head_srcu_data); \
> > + mod struct srcu_notifier_head name = \
> > + SRCU_NOTIFIER_INIT(name, name##_head_srcu_data)
> > +
> > +#else
> > +#define _SRCU_NOTIFIER_HEAD(name, mod) \
> > + mod struct srcu_notifier_head name = \
> > + SRCU_NOTIFIER_INIT(name, name)
> > +
> > +#endif
> > +
> > +#define SRCU_NOTIFIER_HEAD(name) \
> > + _SRCU_NOTIFIER_HEAD(name, /* not static */)
> > +
> > +#define SRCU_NOTIFIER_HEAD_STATIC(name) \
> > + _SRCU_NOTIFIER_HEAD(name, static)
> > +
> > #ifdef __KERNEL__
> >
> > extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
> > diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> > index 261471f407a5..f41d2fb09f87 100644
> > --- a/include/linux/srcutiny.h
> > +++ b/include/linux/srcutiny.h
> > @@ -43,7 +43,7 @@ struct srcu_struct {
> >
> > void srcu_drive_gp(struct work_struct *wp);
> >
> > -#define __SRCU_STRUCT_INIT(name) \
> > +#define __SRCU_STRUCT_INIT(name, __ignored) \
> > { \
> > .srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
> > .srcu_cb_tail = &name.srcu_cb_head, \
> > @@ -56,9 +56,9 @@ void srcu_drive_gp(struct work_struct *wp);
> > * Tree SRCU, which needs some per-CPU data.
> > */
> > #define DEFINE_SRCU(name) \
> > - struct srcu_struct name = __SRCU_STRUCT_INIT(name)
> > + struct srcu_struct name = __SRCU_STRUCT_INIT(name, name)
> > #define DEFINE_STATIC_SRCU(name) \
> > - static struct srcu_struct name = __SRCU_STRUCT_INIT(name)
> > + static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name)
> >
> > void synchronize_srcu(struct srcu_struct *sp);
> >
> > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > index 4eda108abee0..745d4ca4dd50 100644
> > --- a/include/linux/srcutree.h
> > +++ b/include/linux/srcutree.h
> > @@ -104,9 +104,9 @@ struct srcu_struct {
> > #define SRCU_STATE_SCAN1 1
> > #define SRCU_STATE_SCAN2 2
> >
> > -#define __SRCU_STRUCT_INIT(name) \
> > +#define __SRCU_STRUCT_INIT(name, pcpu_name) \
> > { \
> > - .sda = &name##_srcu_data, \
> > + .sda = &pcpu_name, \
> > .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
> > .srcu_gp_seq_needed = 0 - 1, \
> > __SRCU_DEP_MAP_INIT(name) \
> > @@ -133,7 +133,7 @@ struct srcu_struct {
> > */
> > #define __DEFINE_SRCU(name, is_static) \
> > static DEFINE_PER_CPU(struct srcu_data, name##_srcu_data);\
> > - is_static struct srcu_struct name = __SRCU_STRUCT_INIT(name)
> > + is_static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name##_srcu_data)
> > #define DEFINE_SRCU(name) __DEFINE_SRCU(name, /* not static */)
> > #define DEFINE_STATIC_SRCU(name) __DEFINE_SRCU(name, static)
> >
> > --
> > 2.17.0
> >
>
^ permalink raw reply
* Re: [PATCH v14 0/2] Kryo CPU scaling driver
From: Amit Kucheria @ 2018-05-29 11:11 UTC (permalink / raw)
To: Ilia Lin
Cc: vireshk, nm, sboyd, Rob Herring, Mark Rutland, Rafael J. Wysocki,
Linux PM list, devicetree, LKML
In-Reply-To: <1527250067-1256-1-git-send-email-ilialin@codeaurora.org>
On Fri, May 25, 2018 at 3:07 PM, Ilia Lin <ilialin@codeaurora.org> wrote:
> [v14]
> * Addressed comment from Sudeep about DT compatible
> * Added MAINTAINERS entry
>
> [v13]
> * Addressed comment from Sudeep about DT compatible check on init
>
> [v12]
> * Addressed comments from Sudeep and Viresh about the single init
>
> [v11]
> * Addressed comment from Russel about device_node reference
> * Addressed comment from Sudeep about the late_initcall
> * Transformed init into probe to take care of deferals
>
> [v10]
> * Split the series into domains
> * Addressed comments from Viresh and Sudeep about logical CPU numbering.
>
> The qcom-cpufreq-kryo driver is aimed to support different SOC versions.
> The driver reads eFuse information and chooses the required OPP subset
> by passing the OPP supported-hw parameter.
>
> The series depends on the series from Viresh:
> https://patchwork.kernel.org/patch/10418139/
>
> The previous spin was here:
> https://patchwork.kernel.org/patch/10424949/
>
> Ilia Lin (2):
> cpufreq: Add Kryo CPU scaling driver
> dt-bindings: cpufreq: Document operating-points-v2-kryo-cpu
>
> .../devicetree/bindings/opp/kryo-cpufreq.txt | 680 +++++++++++++++++++++
> MAINTAINERS | 7 +
> drivers/cpufreq/Kconfig.arm | 10 +
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/cpufreq-dt-platdev.c | 3 +
> drivers/cpufreq/qcom-cpufreq-kryo.c | 206 +++++++
> 6 files changed, 907 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
> create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
>
FWIW,
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
Tested-by: Amit Kucheria <amit.kucheria@linaro.org>
^ permalink raw reply
* Re: [PATCH V2] cpufreq: reinitialize new policy min/max when writing scaling_(max|min)_freq
From: Viresh Kumar @ 2018-05-29 10:26 UTC (permalink / raw)
To: Kevin Wangtao; +Cc: rjw, linux-pm, linux-kernel, gengyanping, sunzhaosheng
In-Reply-To: <1527319008-66663-1-git-send-email-kevin.wangtao@hisilicon.com>
On 26-05-18, 15:16, Kevin Wangtao wrote:
> consider such situation, current user_policy.min is 1000000,
> current user_policy.max is 1200000, in cpufreq_set_policy,
> other driver may update policy.min to 1200000, policy.max to
> 1300000. After that, If we input "echo 1300000 > scaling_min_freq",
> then user_policy.min will be 1300000, and user_policy.max is
> still 1200000, because the input value is checked with policy.max
> not user_policy.max. if we get all related cpus offline and
> online again, it will cause cpufreq_init_policy fail because
> user_policy.min is higher than user_policy.max.
>
> The solution is when user space tries to write scaling_(max|min)_freq,
> the min/max of new_policy should be reinitialized with min/max
> of user_policy, like what cpufreq_update_policy does.
>
> Signed-off-by: Kevin Wangtao <kevin.wangtao@hisilicon.com>
> ---
> drivers/cpufreq/cpufreq.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index b79c532..82123a1 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -697,6 +697,8 @@ static ssize_t store_##file_name \
> struct cpufreq_policy new_policy; \
> \
> memcpy(&new_policy, policy, sizeof(*policy)); \
Maybe add a comment here on why this is required ?
> + new_policy.min = policy->user_policy.min; \
> + new_policy.max = policy->user_policy.max; \
> \
> ret = sscanf(buf, "%u", &new_policy.object); \
> if (ret != 1) \
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox