* Re: [PATCH v12 00/11] Appended signatures support for IMA appraisal
From: Thiago Jung Bauermann @ 2019-08-27 1:04 UTC (permalink / raw)
To: Jordan Hand
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Mimi Zohar,
Dmitry Kasatkin, James Morris, Serge E. Hallyn, David Howells,
David Woodhouse, Jessica Yu, Herbert Xu, David S. Miller,
Jonathan Corbet, AKASHI, Takahiro
In-Reply-To: <9682b5d0-1634-2dd0-2cbb-eb1fa8ba7423@linux.microsoft.com>
Hello Jordan,
Jordan Hand <jorhand@linux.microsoft.com> writes:
> On 6/27/19 7:19 PM, Thiago Jung Bauermann wrote:
>> On the OpenPOWER platform, secure boot and trusted boot are being
>> implemented using IMA for taking measurements and verifying signatures.
>> Since the kernel image on Power servers is an ELF binary, kernels are
>> signed using the scripts/sign-file tool and thus use the same signature
>> format as signed kernel modules.
>>
>> This patch series adds support in IMA for verifying those signatures.
>> It adds flexibility to OpenPOWER secure boot, because it allows it to boot
>> kernels with the signature appended to them as well as kernels where the
>> signature is stored in the IMA extended attribute.
>
> I know this is pretty late, but I just wanted to let you know that I
> tested this patch set on x86_64 with QEMU.
>
> That is, I enrolled a key to _ima keyring, signed my kernel and modules
> with appended signatures (with scripts/sign-file), set the IMA policy to
> appraise and measure my kernel and modules. Also tested kexec appraisal.
>
> You can add my tested-by if you'd like.
Thanks for testing!
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* [PATCH v2] doc: kselftest: update for clarity on running kselftests in CI rings
From: Shuah Khan @ 2019-08-27 0:37 UTC (permalink / raw)
To: shuah, corbet
Cc: Shuah Khan, linux-kselftest, linux-doc, linux-kernel, dan.rue,
anders.roxell
Update to add clarity and recommendations on running newer kselftests
on older kernels vs. matching the kernel and kselftest revisions.
The recommendation is "Match kernel revision and kselftest."
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
Changes since v1: Fixed "WARNING: Title underline too short."
Documentation/dev-tools/kselftest.rst | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index 25604904fa6e..308506c5e8fa 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -12,6 +12,31 @@ write new tests using the framework on Kselftest wiki:
https://kselftest.wiki.kernel.org/
+Recommendations on running kselftests in Continuous Integration test rings
+==========================================================================
+
+It is recommended that users run Kselftest from the same release. Running
+newer Kselftest on older kernels isn't recommended for the following
+reasons:
+
+- Kselftest from mainline and linux-next might not be stable enough to run
+ on stable kernels.
+- Kselftests detect feature dependencies at run-time and skip tests if a
+ feature and/or configuration they test aren't enabled. Running newer
+ tests on older kernels could result in a few too many skipped/failed
+ conditions. It becomes difficult to evaluate the results.
+- Newer tests provide better coverage. However, users should make a judgement
+ call on coverage vs. run to run consistency and being able to compare
+ run to run results on older kernels.
+
+Recommendations:
+
+Match kernel revision and kselftest. Especially important for LTS and
+Stable kernel Continuous Integration test rings.
+
+Hot-plug tests
+==============
+
On some systems, hot-plug tests could hang forever waiting for cpu and
memory to be ready to be offlined. A special hot-plug target is created
to run the full range of hot-plug tests. In default mode, hot-plug tests run
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] doc: kselftest: update for clarity on running kselftests in CI rings
From: Shuah Khan @ 2019-08-27 0:14 UTC (permalink / raw)
To: shuah, corbet
Cc: linux-kselftest, linux-doc, linux-kernel, dan.rue, anders.roxell,
Shuah Khan
In-Reply-To: <20190827000753.25081-1-skhan@linuxfoundation.org>
On 8/26/19 6:07 PM, Shuah Khan wrote:
> Update to add clarity and recommendations on running newer kselftests
> on older kernels vs. matching the kernel and kselftest revisions.
>
> The recommendation is "Match kernel revision and kselftest."
>
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> ---
> Documentation/dev-tools/kselftest.rst | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
> index 25604904fa6e..e55d9229fa8c 100644
> --- a/Documentation/dev-tools/kselftest.rst
> +++ b/Documentation/dev-tools/kselftest.rst
> @@ -12,6 +12,31 @@ write new tests using the framework on Kselftest wiki:
>
> https://kselftest.wiki.kernel.org/
>
> +Recommendations on running kselftests in Continuous Integration test rings
> +=========================================================================
Sorry. I missed the "WARNING: Title underline too short."
Will send v2 with this corrected.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH] doc: kselftest: update for clarity on running kselftests in CI rings
From: Shuah Khan @ 2019-08-27 0:07 UTC (permalink / raw)
To: shuah, corbet
Cc: Shuah Khan, linux-kselftest, linux-doc, linux-kernel, dan.rue,
anders.roxell
Update to add clarity and recommendations on running newer kselftests
on older kernels vs. matching the kernel and kselftest revisions.
The recommendation is "Match kernel revision and kselftest."
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
Documentation/dev-tools/kselftest.rst | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index 25604904fa6e..e55d9229fa8c 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -12,6 +12,31 @@ write new tests using the framework on Kselftest wiki:
https://kselftest.wiki.kernel.org/
+Recommendations on running kselftests in Continuous Integration test rings
+=========================================================================
+
+It is recommended that users run Kselftest from the same release. Running
+newer Kselftest on older kernels isn't recommended for the following
+reasons:
+
+- Kselftest from mainline and linux-next might not be stable enough to run
+ on stable kernels.
+- Kselftests detect feature dependencies at run-time and skip tests if a
+ feature and/or configuration they test aren't enabled. Running newer
+ tests on older kernels could result in a few too many skipped/failed
+ conditions. It becomes difficult to evaluate the results.
+- Newer tests provide better coverage. However, users should make a judgement
+ call on coverage vs. run to run consistency and being able to compare
+ run to run results on older kernels.
+
+Recommendations:
+
+Match kernel revision and kselftest. Especially important for LTS and
+Stable kernel Continuous Integration test rings.
+
+Hot-plug tests
+==============
+
On some systems, hot-plug tests could hang forever waiting for cpu and
memory to be ready to be offlined. A special hot-plug target is created
to run the full range of hot-plug tests. In default mode, hot-plug tests run
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v12 00/11] Appended signatures support for IMA appraisal
From: Jordan Hand @ 2019-08-26 22:46 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Mimi Zohar, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <20190628021934.4260-1-bauerman@linux.ibm.com>
On 6/27/19 7:19 PM, Thiago Jung Bauermann wrote:
> On the OpenPOWER platform, secure boot and trusted boot are being
> implemented using IMA for taking measurements and verifying signatures.
> Since the kernel image on Power servers is an ELF binary, kernels are
> signed using the scripts/sign-file tool and thus use the same signature
> format as signed kernel modules.
>
> This patch series adds support in IMA for verifying those signatures.
> It adds flexibility to OpenPOWER secure boot, because it allows it to boot
> kernels with the signature appended to them as well as kernels where the
> signature is stored in the IMA extended attribute.
I know this is pretty late, but I just wanted to let you know that I
tested this patch set on x86_64 with QEMU.
That is, I enrolled a key to _ima keyring, signed my kernel and modules
with appended signatures (with scripts/sign-file), set the IMA policy to
appraise and measure my kernel and modules. Also tested kexec appraisal.
You can add my tested-by if you'd like.
-Jordan
^ permalink raw reply
* Re: [PATCH 01/11] ftrace: move recordmcount tools to scripts/ftrace
From: Steven Rostedt @ 2019-08-26 22:44 UTC (permalink / raw)
To: Changbin Du
Cc: Ingo Molnar, Jonathan Corbet, Jessica Yu, Thomas Gleixner, x86,
linux-doc, linux-kernel, linux-arm-kernel, linux-mips,
linux-parisc, linuxppc-dev, linux-riscv, linux-s390, linux-sh,
sparclinux, linux-arch, linux-kbuild, John F . Reiser,
Matt Helsley
In-Reply-To: <20190825132330.5015-2-changbin.du@gmail.com>
On Sun, 25 Aug 2019 21:23:20 +0800
Changbin Du <changbin.du@gmail.com> wrote:
> Move ftrace tools to its own directory. We will add another tool later.
>
> Cc: John F. Reiser <jreiser@BitWagon.com>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
> scripts/.gitignore | 1 -
> scripts/Makefile | 2 +-
> scripts/Makefile.build | 10 +++++-----
> scripts/ftrace/.gitignore | 4 ++++
> scripts/ftrace/Makefile | 4 ++++
> scripts/{ => ftrace}/recordmcount.c | 0
> scripts/{ => ftrace}/recordmcount.h | 0
> scripts/{ => ftrace}/recordmcount.pl | 0
> 8 files changed, 14 insertions(+), 7 deletions(-)
> create mode 100644 scripts/ftrace/.gitignore
> create mode 100644 scripts/ftrace/Makefile
> rename scripts/{ => ftrace}/recordmcount.c (100%)
> rename scripts/{ => ftrace}/recordmcount.h (100%)
> rename scripts/{ => ftrace}/recordmcount.pl (100%)
> mode change 100755 => 100644
Note, we are in the process of merging recordmcount with objtool. It
would be better to continue from that work.
http://lkml.kernel.org/r/2767f55f4a5fbf30ba0635aed7a9c5ee92ac07dd.1563992889.git.mhelsley@vmware.com
-- Steve
>
> diff --git a/scripts/.gitignore b/scripts/.gitignore
> index 17f8cef88fa8..1b5b5d595d80 100644
> --- a/scripts/.gitignore
> +++ b/scripts/.gitignore
> @@ -6,7 +6,6 @@ conmakehash
> kallsyms
> pnmtologo
> unifdef
> -recordmcount
> sortextable
> asn1_compiler
> extract-cert
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 16bcb8087899..d5992def49a8 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -14,7 +14,6 @@ hostprogs-$(CONFIG_BUILD_BIN2C) += bin2c
> hostprogs-$(CONFIG_KALLSYMS) += kallsyms
> hostprogs-$(CONFIG_LOGO) += pnmtologo
> hostprogs-$(CONFIG_VT) += conmakehash
> -hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
> hostprogs-$(CONFIG_BUILDTIME_EXTABLE_SORT) += sortextable
> hostprogs-$(CONFIG_ASN1) += asn1_compiler
> hostprogs-$(CONFIG_MODULE_SIG) += sign-file
> @@ -34,6 +33,7 @@ hostprogs-y += unifdef
> subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
> subdir-$(CONFIG_MODVERSIONS) += genksyms
> subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> +subdir-$(CONFIG_FTRACE) += ftrace
>
> # Let clean descend into subdirs
> subdir- += basic dtc gdb kconfig mod package
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 2f66ed388d1c..67558983c518 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -188,18 +188,18 @@ endif
> # files, including recordmcount.
> sub_cmd_record_mcount = \
> if [ $(@) != "scripts/mod/empty.o" ]; then \
> - $(objtree)/scripts/recordmcount $(RECORDMCOUNT_FLAGS) "$(@)"; \
> + $(objtree)/scripts/ftrace/recordmcount $(RECORDMCOUNT_FLAGS) "$(@)"; \
> fi;
> -recordmcount_source := $(srctree)/scripts/recordmcount.c \
> - $(srctree)/scripts/recordmcount.h
> +recordmcount_source := $(srctree)/scripts/ftrace/recordmcount.c \
> + $(srctree)/scripts/ftrace/recordmcount.h
> else
> -sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
> +sub_cmd_record_mcount = perl $(srctree)/scripts/ftrace/recordmcount.pl "$(ARCH)" \
> "$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
> "$(if $(CONFIG_64BIT),64,32)" \
> "$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS)" \
> "$(LD) $(KBUILD_LDFLAGS)" "$(NM)" "$(RM)" "$(MV)" \
> "$(if $(part-of-module),1,0)" "$(@)";
> -recordmcount_source := $(srctree)/scripts/recordmcount.pl
> +recordmcount_source := $(srctree)/scripts/ftrace/recordmcount.pl
> endif # BUILD_C_RECORDMCOUNT
> cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)), \
> $(sub_cmd_record_mcount))
> diff --git a/scripts/ftrace/.gitignore b/scripts/ftrace/.gitignore
> new file mode 100644
> index 000000000000..54d582c8faad
> --- /dev/null
> +++ b/scripts/ftrace/.gitignore
> @@ -0,0 +1,4 @@
> +#
> +# Generated files
> +#
> +recordmcount
> diff --git a/scripts/ftrace/Makefile b/scripts/ftrace/Makefile
> new file mode 100644
> index 000000000000..6797e51473e5
> --- /dev/null
> +++ b/scripts/ftrace/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
> +always := $(hostprogs-y)
> diff --git a/scripts/recordmcount.c b/scripts/ftrace/recordmcount.c
> similarity index 100%
> rename from scripts/recordmcount.c
> rename to scripts/ftrace/recordmcount.c
> diff --git a/scripts/recordmcount.h b/scripts/ftrace/recordmcount.h
> similarity index 100%
> rename from scripts/recordmcount.h
> rename to scripts/ftrace/recordmcount.h
> diff --git a/scripts/recordmcount.pl b/scripts/ftrace/recordmcount.pl
> old mode 100755
> new mode 100644
> similarity index 100%
> rename from scripts/recordmcount.pl
> rename to scripts/ftrace/recordmcount.pl
^ permalink raw reply
* Re: [PATCH 05/11] ftrace: create memcache for hash entries
From: Changbin Du @ 2019-08-26 22:35 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Changbin Du, Steven Rostedt, Ingo Molnar, Jonathan Corbet,
Jessica Yu, Thomas Gleixner, x86, linux-doc, linux-kernel,
linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
linux-riscv, linux-s390, linux-sh, sparclinux, linux-arch,
linux-kbuild
In-Reply-To: <20190826074437.GM2369@hirez.programming.kicks-ass.net>
On Mon, Aug 26, 2019 at 09:44:37AM +0200, Peter Zijlstra wrote:
> On Sun, Aug 25, 2019 at 09:23:24PM +0800, Changbin Du wrote:
> > When CONFIG_FTRACE_FUNC_PROTOTYPE is enabled, thousands of
> > ftrace_func_entry instances are created. So create a dedicated
> > memcache to enhance performance.
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> > kernel/trace/ftrace.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index a314f0768b2c..cfcb8dad93ea 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -94,6 +94,8 @@ struct ftrace_ops *function_trace_op __read_mostly = &ftrace_list_end;
> > /* What to set function_trace_op to */
> > static struct ftrace_ops *set_function_trace_op;
> >
> > +struct kmem_cache *hash_entry_cache;
> > +
> > static bool ftrace_pids_enabled(struct ftrace_ops *ops)
> > {
> > struct trace_array *tr;
> > @@ -1169,7 +1171,7 @@ static int add_hash_entry(struct ftrace_hash *hash, unsigned long ip,
> > {
> > struct ftrace_func_entry *entry;
> >
> > - entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> > + entry = kmem_cache_alloc(hash_entry_cache, GFP_KERNEL);
> > if (!entry)
> > return -ENOMEM;
> >
> > @@ -6153,6 +6155,15 @@ void __init ftrace_init(void)
> > if (ret)
> > goto failed;
> >
> > + hash_entry_cache = kmem_cache_create("ftrace-hash",
> > + sizeof(struct ftrace_func_entry),
> > + sizeof(struct ftrace_func_entry),
> > + 0, NULL);
> > + if (!hash_entry_cache) {
> > + pr_err("failed to create ftrace hash entry cache\n");
> > + goto failed;
> > + }
>
> Wait what; you already have then in the binary image, now you're
> allocating extra memory for each of them?
>
No, here we only allocate ftrace hash entries. The prototype data is not copied.
The entry->priv points to prototype data in binary.
> Did you look at what ORC does? Is the binary search really not fast
> enough?
For ftrace, binary search is not enough. Just like the hash tables
(ftrace_graph_notrace_hash, ftrace_graph_hash) we already have which is used to
filter traced functions.
--
Cheers,
Changbin Du
^ permalink raw reply
* Re: [PATCH 03/11] asm-generic: add generic dwarf definition
From: Changbin Du @ 2019-08-26 22:25 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Changbin Du, Steven Rostedt, Ingo Molnar, Jonathan Corbet,
Jessica Yu, Thomas Gleixner, x86, linux-doc, linux-kernel,
linux-arm-kernel, linux-mips, linux-parisc, linuxppc-dev,
linux-riscv, linux-s390, linux-sh, sparclinux, linux-arch,
linux-kbuild
In-Reply-To: <20190826074215.GL2369@hirez.programming.kicks-ass.net>
Hi, Peter,
On Mon, Aug 26, 2019 at 09:42:15AM +0200, Peter Zijlstra wrote:
> On Sun, Aug 25, 2019 at 09:23:22PM +0800, Changbin Du wrote:
> > Add generic DWARF constant definitions. We will use it later.
> >
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> > include/asm-generic/dwarf.h | 199 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 199 insertions(+)
> > create mode 100644 include/asm-generic/dwarf.h
> >
> > diff --git a/include/asm-generic/dwarf.h b/include/asm-generic/dwarf.h
> > new file mode 100644
> > index 000000000000..c705633c2a8f
> > --- /dev/null
> > +++ b/include/asm-generic/dwarf.h
> > @@ -0,0 +1,199 @@
> > +/* SPDX-License-Identifier: GPL-2.0
> > + *
> > + * Architecture independent definitions of DWARF.
> > + *
> > + * Copyright (C) 2019 Changbin Du <changbin.du@gmail.com>
>
> You're claiming copyright on dwarf definitions? ;-)
>
> I'm thinking only Oracle was daft enough to think stuff like that was
> copyrightable.
>
ok, let me remove copyright line. I think SPDX claim is okay, right?
> Also; I think it would be very good to not use/depend on DWARF for this.
>
It only includes the DWARF expersion opcodes, not all of dwarf stuffs.
> You really don't need all of DWARF; I'm thikning you only need a few
> types; for location we already have regs_get_kernel_argument() which
> has all the logic to find the n-th argument.
>
regs_get_kernel_argument() can handle most cases, but if the size of one paramater
exceeds 64bit (it is rare in kernel), we must recalculate the locations. So I think
dwarf location descriptor is the most accurate one.
--
Cheers,
Changbin Du
^ permalink raw reply
* [PATCH v2 3/3] fbdev: fbmem: avoid exporting fb_center_logo
From: Peter Rosin @ 2019-08-26 19:59 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox
In-Reply-To: <20190826195740.29415-1-peda@axentia.se>
The variable is only ever used from fbcon.c which is linked into the
same module. Therefore, the export is not needed.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/video/fbdev/core/fbmem.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index a7d8022db516..17e250a515b0 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -54,7 +54,6 @@ int num_registered_fb __read_mostly;
EXPORT_SYMBOL(num_registered_fb);
bool fb_center_logo __read_mostly;
-EXPORT_SYMBOL(fb_center_logo);
unsigned int fb_logo_count __read_mostly;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 2/3] fbdev: fbmem: allow overriding the number of bootup logos
From: Peter Rosin @ 2019-08-26 19:58 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox
In-Reply-To: <20190826195740.29415-1-peda@axentia.se>
Probably most useful if you only want one logo regardless of how many
CPU cores you have.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/fb/fbcon.rst | 5 +++++
drivers/video/fbdev/core/fbcon.c | 7 +++++++
drivers/video/fbdev/core/fbmem.c | 4 +++-
include/linux/fb.h | 1 +
4 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
index 65ba40255137..9f0b399d8d4e 100644
--- a/Documentation/fb/fbcon.rst
+++ b/Documentation/fb/fbcon.rst
@@ -174,6 +174,11 @@ C. Boot options
displayed due to multiple CPUs, the collected line of logos is moved
as a whole.
+9. fbcon=logo-count:<n>
+
+ The value 'n' overrides the number of bootup logos. Zero gives the
+ default, which is the number of online cpus.
+
C. Attaching, Detaching and Unloading
Before going on to how to attach, detach and unload the framebuffer console, an
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c9235a2f42f8..be4bc5540aad 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -536,6 +536,13 @@ static int __init fb_console_setup(char *this_opt)
fb_center_logo = true;
continue;
}
+
+ if (!strncmp(options, "logo-count:", 11)) {
+ options += 11;
+ if (*options)
+ fb_logo_count = simple_strtoul(options, &options, 0);
+ continue;
+ }
}
return 1;
}
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 64dd732021d8..a7d8022db516 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -56,6 +56,8 @@ EXPORT_SYMBOL(num_registered_fb);
bool fb_center_logo __read_mostly;
EXPORT_SYMBOL(fb_center_logo);
+unsigned int fb_logo_count __read_mostly;
+
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
@@ -689,7 +691,7 @@ int fb_show_logo(struct fb_info *info, int rotate)
int y;
y = fb_show_logo_line(info, rotate, fb_logo.logo, 0,
- num_online_cpus());
+ fb_logo_count ?: num_online_cpus());
y = fb_show_extra_logos(info, y, rotate);
return y;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 303771264644..5f2b05406262 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -630,6 +630,7 @@ extern int fb_new_modelist(struct fb_info *info);
extern struct fb_info *registered_fb[FB_MAX];
extern int num_registered_fb;
extern bool fb_center_logo;
+extern unsigned int fb_logo_count;
extern struct class *fb_class;
#define for_each_registered_fb(i) \
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/3] fbdev: fix numbering of fbcon options
From: Peter Rosin @ 2019-08-26 19:58 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox
In-Reply-To: <20190826195740.29415-1-peda@axentia.se>
Three shall be the number thou shalt count, and the number of the
counting shall be three. Four shalt thou not count...
One! Two! Five!
Fixes: efb985f6b265 ("[PATCH] fbcon: Console Rotation - Add framebuffer console documentation")
Signed-off-by: Peter Rosin <peda@axentia.se>
---
Documentation/fb/fbcon.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst
index ebca41785abe..65ba40255137 100644
--- a/Documentation/fb/fbcon.rst
+++ b/Documentation/fb/fbcon.rst
@@ -127,7 +127,7 @@ C. Boot options
is typically located on the same video card. Thus, the consoles that
are controlled by the VGA console will be garbled.
-4. fbcon=rotate:<n>
+5. fbcon=rotate:<n>
This option changes the orientation angle of the console display. The
value 'n' accepts the following:
@@ -152,21 +152,21 @@ C. Boot options
Actually, the underlying fb driver is totally ignorant of console
rotation.
-5. fbcon=margin:<color>
+6. fbcon=margin:<color>
This option specifies the color of the margins. The margins are the
leftover area at the right and the bottom of the screen that are not
used by text. By default, this area will be black. The 'color' value
is an integer number that depends on the framebuffer driver being used.
-6. fbcon=nodefer
+7. fbcon=nodefer
If the kernel is compiled with deferred fbcon takeover support, normally
the framebuffer contents, left in place by the firmware/bootloader, will
be preserved until there actually is some text is output to the console.
This option causes fbcon to bind immediately to the fbdev device.
-7. fbcon=logo-pos:<location>
+8. fbcon=logo-pos:<location>
The only possible 'location' is 'center' (without quotes), and when
given, the bootup logo is moved from the default top-left corner
--
2.11.0
^ permalink raw reply related
* [PATCH v2 0/3] Add possibility to specify the number of displayed logos
From: Peter Rosin @ 2019-08-26 19:58 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
Cc: Peter Rosin, Bartlomiej Zolnierkiewicz, Jonathan Corbet,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-doc@vger.kernel.org, Matthew Wilcox
Hi!
The first patch fixes the fact that there are two items numbered "4" in
the list of fbcon options. This bug is a teenager...
The second patch extends that list with a new option that allows the
user to display any number of logos (that fits on the screen). I need it
to limit the display to only one logo instead of one for each CPU core.
Changes since v1
- do not needlessly export fb_logo_count [Matthew Wilcox]
- added patch 3/3, which removes the export of fb_center_logo
Cheers,
Peter
Peter Rosin (3):
fbdev: fix numbering of fbcon options
fbdev: fbmem: allow overriding the number of bootup logos
fbdev: fbmem: avoid exporting fb_center_logo
Documentation/fb/fbcon.rst | 13 +++++++++----
drivers/video/fbdev/core/fbcon.c | 7 +++++++
drivers/video/fbdev/core/fbmem.c | 5 +++--
include/linux/fb.h | 1 +
4 files changed, 20 insertions(+), 6 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH 0/6] hwspinlock: allow sharing of hwspinlocks
From: Fabien DESSENNE @ 2019-08-26 13:30 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Suman Anna, Ohad Ben-Cohen, Rob Herring, Mark Rutland,
Maxime Coquelin, Alexandre TORGUE, Jonathan Corbet,
linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
Benjamin GAIGNARD
In-Reply-To: <20190808153721.GI26807@tuxbook-pro>
Hi Bjorn,
On 08/08/2019 5:37 PM, Bjorn Andersson wrote:
> On Thu 08 Aug 05:52 PDT 2019, Fabien DESSENNE wrote:
>
>> On 07/08/2019 6:19 PM, Suman Anna wrote:
>>> Hi Fabien,
>>>
>>> On 8/7/19 3:39 AM, Fabien DESSENNE wrote:
>>>> Hi
>>>>
>>>> On 06/08/2019 11:30 PM, Suman Anna wrote:
>>>>> On 8/6/19 1:21 PM, Bjorn Andersson wrote:
>>>>>> On Tue 06 Aug 10:38 PDT 2019, Suman Anna wrote:
>>>>>>
>>>>>>> Hi Fabien,
>>>>>>>
>>>>>>> On 8/5/19 12:46 PM, Bjorn Andersson wrote:
>>>>>> I agree that we shouldn't specify this property in DT - if anything it
>>>>>> should be a variant of the API.
>>>> If we decide to add a 'shared' API, then, what about the generic regmap
>>>> driver?
>>>>
>>>> In the context of above example1, this would require to update the
>>>> regmap driver.
>>>>
>>>> But would this be acceptable for any driver using syscon/regmap?
>>>>
>>>>
>>>> I think it is better to keep the existing API (modifying it so it always
>>>> allows
>>>>
>>>> hwlocks sharing, so no need for bindings update) than adding another API.
>>> For your usecases, you would definitely need the syscon/regmap behavior
>>> to be shared right. Whether we introduce a 'shared' API or an
>>> 'exclusive' API and change the current API behavior to shared, it is
>>> definitely a case-by-case usage scenario for the existing drivers and
>>> usage right. The main contention point is what to do with the
>>> unprotected usecases like Bjorn originally pointed out.
>> OK, I see : the hwspinlock framework does not offer any lock protection
>> with the RAW/IN_ATOMIC modes.
>> This is an issue if several different 'local' drivers try to get a
>> shared lock in the same time.
>> And this is a personal problem since I need to use shared locks in
>> ...atomic mode.
>>
> Why can't you use HWLOCK_IRQSTATE in this mode?
>
>> I have tried to see how it is possible to put a constraint on the
>> callers, just like this is documented for the RAW mode which is:
>> "Caution: If the mode is HWLOCK_RAW, that means user must protect
>> the routine
>> of getting hardware lock with mutex or spinlock.."
>> I do not think that it is acceptable to ask several drivers to share a
>> common mutex/spinlock for shared locks.
> No it's not.
>
>> But I think about another option: the driver implementing the trylock
>> ops may offer such protection. This is the case if the driver returns
>> "busy" if the lock is already taken, not only by the remote processor,
>> but also by the local host.
>>
> I think it's typical for hwspinlock hardware to not be able to
> distinguish between different clients within Linux, so we would need to
Agree with that, let's forget this idea.
> wrap the usage in some construct that ensures mutual exclusion in Linux
> - like a spinlock...
>
>> So what do you think about adding such a documentation note :
>> "Caution : the HWLOCK_RAW / HWLOCK_IN_ATOMIC modes shall not be used
>> with shared locks unless the hwspinlock driver supports local lock
>> protection"
>>
> But having local lock protection in the hwspinlock driver would defeat
> the purpose of HWLOCK_RAW.
My understanding is that the purpose of the RAW mode is to allow the
user to do some time-consuming or sleepable operations under the
hardware spinlock protection.
This is probably the reason why the RAW mode does not uses any spinlock
is used in RAW mode.
But I do not think that this is a requirement to not use any local
protection.
So, in this mode, instead of using a spinlock, what about calling the
atomic bitop test_and_set_bit() ?
This would ensure safe concurrency between the hwspinlock linux users,
and will respect the purpose of the RAW mode.
Let me know if this is acceptable.
BR
Fabien
>
> Also this kind of warning will at best be consumed by the client driver
> authors, it will not be read by the dts authors.
>
> Regards,
> Bjorn
>
>> Optionally, we may add a "local_lock_protection" flag in the
>> hwspinlock_device struct, set by the driver before it calls
>> hwspin_lock_register().
>> This flag can then be checked by hwspinlock core to allow/deny use of
>> shared locks in the raw/atomic modes.
>>
>> Let me know what you think about it.
>>
>> BR
>>
>> Fabien
>>
>>> regards
>>> Suman
>>>
>>>>
>>>>>>> If you are sharing a hwlock on the Linux side, surely your driver should
>>>>>>> be aware that it is a shared lock. The tag can be set during the first
>>>>>>> request API, and you look through both tags when giving out a handle.
>>>>>>>
>>>>>> Why would the driver need to know about it?
>>>>> Just the semantics if we were to support single user vs multiple users
>>>>> on Linux-side to even get a handle. Your point is that this may be moot
>>>>> since we have protection anyway other than the raw cases. But we need to
>>>>> be able to have the same API work across all cases.
>>>>>
>>>>> So far, it had mostly been that there would be one user on Linux
>>>>> competing with other equivalent peer entities on different processors.
>>>>> It is not common to have multiple users since these protection schemes
>>>>> are usually needed only at the lowest levels of a stack, so the
>>>>> exclusive handle stuff had been sufficient.
>>>>>
>>>>>>> Obviously, the hwspin_lock_request() API usage semantics always had the
>>>>>>> implied additional need for communicating the lock id to the other peer
>>>>>>> entity, so a realistic usage is most always the specific API variant. I
>>>>>>> doubt this API would be of much use for the shared driver usage. This
>>>>>>> also implies that the client user does not care about specifying a lock
>>>>>>> in DT.
>>>>>>>
>>>>>> Afaict if the lock are shared then there shouldn't be a problem with
>>>>>> some clients using the request API and others request_specific(). As any
>>>>>> collisions would simply mean that there are more contention on the lock.
>>>>>>
>>>>>> With the current exclusive model that is not possible and the success of
>>>>>> the request_specific will depend on probe order.
>>>>>>
>>>>>> But perhaps it should be explicitly prohibited to use both APIs on the
>>>>>> same hwspinlock instance?
>>>>> Yeah, they are meant to be complimentary usage, though I doubt we will
>>>>> ever have any realistic users for the generic API if we haven't had a
>>>>> usage so far. I had posted a concept of reserved locks long back [1] to
>>>>> keep away certain locks from the generic requestor, but dropped it since
>>>>> we did not have an actual use-case needing it.
>>>>>
>>>>> regards
>>>>> Suman
>>>>>
>>>>> [1] https://lwn.net/Articles/611944/
^ permalink raw reply
* Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
From: Enrico Weigelt, metux IT consult @ 2019-08-26 13:29 UTC (permalink / raw)
To: Uwe Kleine-König, Andrew Morton
Cc: Jonathan Corbet, linux-doc, linux-kernel, linux-gpio,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <20190824233724.1775-1-uwe@kleine-koenig.org>
On 25.08.19 01:37, Uwe Kleine-König wrote:
Hi,
> +static noinline_for_stack > +char *errstr(char *buf, char *end, unsigned long long num,> +
struct printf_spec spec)> +{
#1: why not putting that into some separate strerror() lib function ?
This is something I've been looking for quite some time (actually
already hacked it up somewhere, sometime, but forgotten ...)
#2: why not just having a big case statement and leave the actual lookup
logic to the compiler ? IMHO, could be written in a very compact way
by some macro magic
> + for (i = 0; i < ARRAY_SIZE(errorcodes); ++i) { > + if (num == errorcodes[i].err || num == -errorcodes[i].err) {
why not taking the abs value only once, instead of duplicate comp on
each iteration ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: [RFC PATCH v3 01/19] skc: Add supplemental kernel cmdline support
From: Rob Herring @ 2019-08-26 13:27 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Frank Rowand, Ingo Molnar, Namhyung Kim, Tim Bird,
Jiri Olsa, Arnaldo Carvalho de Melo, Tom Zanussi, Andrew Morton,
Thomas Gleixner, Greg Kroah-Hartman, Alexey Dobriyan,
Jonathan Corbet, Linus Torvalds, Linux Doc Mailing List,
linux-fsdevel, linux-kernel@vger.kernel.org
In-Reply-To: <156678934990.21459.10847677747264952252.stgit@devnote2>
On Sun, Aug 25, 2019 at 10:15 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Supplemental kernel command line (SKC) allows admin to pass a
> tree-structured supplemental kernel commandline file (SKC file)
> when boot up kernel. This expands the kernel command line in
> efficient way.
>
> SKC file will contain some key-value commands, e.g.
>
> key.word = value1;
> another.key.word = value2;
>
> It can fold same keys with braces, also you can write array
> data. For example,
>
> key {
> word1 {
> setting1 = data;
> setting2;
> }
> word2.array = "val1", "val2";
> }
Why invent a custom file format? You could use YAML (or JSON):
key:
word1:
setting1: data
setting2: true
word2:
- val1
- val2
That would allow you to define a schema for defined options and can
easily be manipulated with python (or any language with dictionaries
and lists). That does imply adding a YAML parser to the kernel which
I'm not sure is a great idea. There is a C parser lib, but working
with YAML in C is not that great compared to python.
Another option would be using the DTS format, but as a separate file.
That's not unprecedented as u-boot FIT image is a DTB. Then the kernel
already has the parser. And you could still have schema now.
A new interface will take a lot of bootloader work to make it easy to
use given the user has to manually load some file in the bootloader
and know a good address to load it to. Between that and rebuilding the
kernel with the configuration, I'd pick rebuilding the kernel. Perhaps
this version will highlight that the original proposal was not so bad.
Another thought, maybe you could process the configuration file that's
in a readable/editable format into a flat representation that could
simply be added to the kernel command line:
key.word1.setting1=data key.word1.setting2 key.word2=val1,val2
That would then use an existing interface and probably simplify the
kernel parsing.
Rob
^ permalink raw reply
* [PATCH] nvmem: core: add nvmem_device_find
From: Thomas Bogendoerfer @ 2019-08-26 13:08 UTC (permalink / raw)
To: Jonathan Corbet, Srinivas Kandagatla, linux-doc, linux-kernel
nvmem_device_find provides a way to search for nvmem devices with
the help of a match function simlair to bus_find_device.
Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
Documentation/driver-api/nvmem.rst | 2 ++
drivers/nvmem/core.c | 61 +++++++++++++++++---------------------
include/linux/nvmem-consumer.h | 9 ++++++
3 files changed, 38 insertions(+), 34 deletions(-)
diff --git a/Documentation/driver-api/nvmem.rst b/Documentation/driver-api/nvmem.rst
index d9d958d5c824..287e86819640 100644
--- a/Documentation/driver-api/nvmem.rst
+++ b/Documentation/driver-api/nvmem.rst
@@ -129,6 +129,8 @@ To facilitate such consumers NVMEM framework provides below apis::
struct nvmem_device *nvmem_device_get(struct device *dev, const char *name);
struct nvmem_device *devm_nvmem_device_get(struct device *dev,
const char *name);
+ struct nvmem_device *nvmem_device_find(void *data,
+ int (*match)(struct device *dev, const void *data));
void nvmem_device_put(struct nvmem_device *nvmem);
int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset,
size_t bytes, void *buf);
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 057d1ff87d5d..9f1ee9c766ec 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -76,33 +76,6 @@ static struct bus_type nvmem_bus_type = {
.name = "nvmem",
};
-static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
-{
- struct device *d;
-
- if (!nvmem_np)
- return NULL;
-
- d = bus_find_device_by_of_node(&nvmem_bus_type, nvmem_np);
-
- if (!d)
- return NULL;
-
- return to_nvmem_device(d);
-}
-
-static struct nvmem_device *nvmem_find(const char *name)
-{
- struct device *d;
-
- d = bus_find_device_by_name(&nvmem_bus_type, NULL, name);
-
- if (!d)
- return NULL;
-
- return to_nvmem_device(d);
-}
-
static void nvmem_cell_drop(struct nvmem_cell *cell)
{
blocking_notifier_call_chain(&nvmem_notifier, NVMEM_CELL_REMOVE, cell);
@@ -532,13 +505,16 @@ int devm_nvmem_unregister(struct device *dev, struct nvmem_device *nvmem)
}
EXPORT_SYMBOL(devm_nvmem_unregister);
-static struct nvmem_device *__nvmem_device_get(struct device_node *np,
- const char *nvmem_name)
+static struct nvmem_device *__nvmem_device_get(void *data,
+ int (*match)(struct device *dev, const void *data))
{
struct nvmem_device *nvmem = NULL;
+ struct device *dev;
mutex_lock(&nvmem_mutex);
- nvmem = np ? of_nvmem_find(np) : nvmem_find(nvmem_name);
+ dev = bus_find_device(&nvmem_bus_type, NULL, data, match);
+ if (dev)
+ nvmem = to_nvmem_device(dev);
mutex_unlock(&nvmem_mutex);
if (!nvmem)
return ERR_PTR(-EPROBE_DEFER);
@@ -587,7 +563,7 @@ struct nvmem_device *of_nvmem_device_get(struct device_node *np, const char *id)
if (!nvmem_np)
return ERR_PTR(-ENOENT);
- return __nvmem_device_get(nvmem_np, NULL);
+ return __nvmem_device_get(nvmem_np, device_match_of_node);
}
EXPORT_SYMBOL_GPL(of_nvmem_device_get);
#endif
@@ -613,10 +589,26 @@ struct nvmem_device *nvmem_device_get(struct device *dev, const char *dev_name)
}
- return __nvmem_device_get(NULL, dev_name);
+ return __nvmem_device_get((void *)dev_name, device_match_name);
}
EXPORT_SYMBOL_GPL(nvmem_device_get);
+/**
+ * nvmem_device_find() - Find nvmem device with matching function
+ *
+ * @data: Data to pass to match function
+ * @match: Callback function to check device
+ *
+ * Return: ERR_PTR() on error or a valid pointer to a struct nvmem_device
+ * on success.
+ */
+struct nvmem_device *nvmem_device_find(void *data,
+ int (*match)(struct device *dev, const void *data))
+{
+ return __nvmem_device_get(data, match);
+}
+EXPORT_SYMBOL_GPL(nvmem_device_find);
+
static int devm_nvmem_device_match(struct device *dev, void *res, void *data)
{
struct nvmem_device **nvmem = res;
@@ -710,7 +702,8 @@ nvmem_cell_get_from_lookup(struct device *dev, const char *con_id)
if ((strcmp(lookup->dev_id, dev_id) == 0) &&
(strcmp(lookup->con_id, con_id) == 0)) {
/* This is the right entry. */
- nvmem = __nvmem_device_get(NULL, lookup->nvmem_name);
+ nvmem = __nvmem_device_get((void *)lookup->nvmem_name,
+ device_match_name);
if (IS_ERR(nvmem)) {
/* Provider may not be registered yet. */
cell = ERR_CAST(nvmem);
@@ -780,7 +773,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct device_node *np, const char *id)
if (!nvmem_np)
return ERR_PTR(-EINVAL);
- nvmem = __nvmem_device_get(nvmem_np, NULL);
+ nvmem = __nvmem_device_get(nvmem_np, device_match_of_node);
of_node_put(nvmem_np);
if (IS_ERR(nvmem))
return ERR_CAST(nvmem);
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index 8f8be5b00060..02dc4aa992b2 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -89,6 +89,9 @@ void nvmem_del_cell_lookups(struct nvmem_cell_lookup *entries,
int nvmem_register_notifier(struct notifier_block *nb);
int nvmem_unregister_notifier(struct notifier_block *nb);
+struct nvmem_device *nvmem_device_find(void *data,
+ int (*match)(struct device *dev, const void *data));
+
#else
static inline struct nvmem_cell *nvmem_cell_get(struct device *dev,
@@ -204,6 +207,12 @@ static inline int nvmem_unregister_notifier(struct notifier_block *nb)
return -EOPNOTSUPP;
}
+static inline struct nvmem_device *nvmem_device_find(void *data,
+ int (*match)(struct device *dev, const void *data))
+{
+ return NULL;
+}
+
#endif /* CONFIG_NVMEM */
#if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
--
2.13.7
^ permalink raw reply related
* Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
From: Petr Mladek @ 2019-08-26 12:05 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andrew Morton, Bartosz Golaszewski, Sergey Senozhatsky,
Steven Rostedt, Linus Walleij, Jonathan Corbet, linux-doc,
linux-gpio, linux-kernel
In-Reply-To: <20190825091442.GA5817@taurus.defre.kleine-koenig.org>
On Sun 2019-08-25 11:14:42, Uwe Kleine-König wrote:
> Hello Andrew,
>
> On Sat, Aug 24, 2019 at 04:58:29PM -0700, Andrew Morton wrote:
> > (cc printk maintainers).
>
> Ah, I wasn't aware there is something like them. Thanks
>
> > On Sun, 25 Aug 2019 01:37:23 +0200 Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> >
> > > pr_info("probing failed (%dE)\n", ret);
> > >
> > > expands to
> > >
> > > probing failed (EIO)
> > >
> > > if ret holds -EIO (or EIO). This introduces an array of error codes. If
> > > the error code is missing, %dE falls back to %d and so prints the plain
> > > number.
What was the motivation for this patch, please?
Did it look like a good idea?
Did anyone got tired by searching for the error codes many
times a day?
Did the idea came from a developer, support, or user, please?
> add/remove: 2/0 grow/shrink: 4/2 up/down: 1488/-8 (1480)
> Function old new delta
> errorcodes - 1200 +1200
> errstr - 200 +200
> vsnprintf 884 960 +76
> set_precision 148 152 +4
> resource_string 1380 1384 +4
> flags_string 400 404 +4
> num_to_str 288 284 -4
> format_decode 1024 1020 -4
> Total: Before=21686, After=23166, chg +6.82%
>
> But that doesn't seem to include the size increase for all the added
> strings which seems to be around another 1300 bytes.
This non-trivial increase of the size and the table still
includes only part of the error codes.
The array is long, created by cpu&paste, the index of each code
is not obvious.
There are ideas to make the code even more tricky to reduce
the size, keep it fast.
Both, %dE modifier and the output format (ECODE) is non-standard.
Upper letters gain a lot of attention. But the error code is
only helper information. Also many error codes are misleading because
they are used either wrongly or there was no better available.
There is no proof that this approach would be widely acceptable for
subsystem maintainers. Some might not like mass and "blind" code
changes. Some might not like the output at all.
I am not persuaded that all this is worth it. Also I do not like
the non-standard solution.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v3] powerpc/fadump: sysfs for fadump memory reservation
From: Hari Bathini @ 2019-08-26 11:16 UTC (permalink / raw)
To: Sourabh Jain, mpe; +Cc: mahesh, linux-doc, linuxppc-dev, linux-kernel, corbet
In-Reply-To: <f53e4cfe-57cb-d8a6-385a-fa6243940573@linux.ibm.com>
On 26/08/19 4:14 PM, Sourabh Jain wrote:
>
>
> On 8/26/19 3:46 PM, Sourabh Jain wrote:
>>
>>
>> On 8/26/19 3:29 PM, Hari Bathini wrote:
>>>
>>>
>>> On 10/08/19 11:29 PM, Sourabh Jain wrote:
>>>> Add a sys interface to allow querying the memory reserved by
>>>> fadump for saving the crash dump.
>>>>
>>>> Add an ABI doc entry for new sysfs interface.
>>>> - /sys/kernel/fadump_mem_reserved
>>>>
>>>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>>> ---
>>>> Changelog:
>>>> v1 -> v2:
>>>> - Added ABI doc for new sysfs interface.
>>>>
>>>> v2 -> v3:
>>>> - Updated the ABI documentation.
>>>> ---
>>>>
>>>> Documentation/ABI/testing/sysfs-kernel-fadump | 6 ++++++
>>>
>>> Shouldn't this be Documentation/ABI/testing/sysfs-kernel-fadump_mem_reserved?
>
> How about documenting fadump_mem_reserved and other sysfs attributes suggested
> by you in a single file Documentation/ABI/testing/sysfs-kernel-fadump?
I wouldn't mind that but please do check if it is breaking a convention..
- Hari
^ permalink raw reply
* Re: [PATCH v3] powerpc/fadump: sysfs for fadump memory reservation
From: Sourabh Jain @ 2019-08-26 10:44 UTC (permalink / raw)
To: Hari Bathini, mpe; +Cc: mahesh, corbet, linuxppc-dev, linux-kernel, linux-doc
In-Reply-To: <cf4fdb60-438c-bc4e-d759-1fbb27364c50@linux.ibm.com>
On 8/26/19 3:46 PM, Sourabh Jain wrote:
>
>
> On 8/26/19 3:29 PM, Hari Bathini wrote:
>>
>>
>> On 10/08/19 11:29 PM, Sourabh Jain wrote:
>>> Add a sys interface to allow querying the memory reserved by
>>> fadump for saving the crash dump.
>>>
>>> Add an ABI doc entry for new sysfs interface.
>>> - /sys/kernel/fadump_mem_reserved
>>>
>>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>> ---
>>> Changelog:
>>> v1 -> v2:
>>> - Added ABI doc for new sysfs interface.
>>>
>>> v2 -> v3:
>>> - Updated the ABI documentation.
>>> ---
>>>
>>> Documentation/ABI/testing/sysfs-kernel-fadump | 6 ++++++
>>
>> Shouldn't this be Documentation/ABI/testing/sysfs-kernel-fadump_mem_reserved?
How about documenting fadump_mem_reserved and other sysfs attributes suggested
by you in a single file Documentation/ABI/testing/sysfs-kernel-fadump?
>>
>>> +++ b/Documentation/ABI/testing/sysfs-kernel-fadump
>>> @@ -0,0 +1,6 @@
>>> +What: /sys/kernel/fadump_mem_reserved
>>> +Date: August 2019
>>> +Contact: linuxppc-dev@lists.ozlabs.org
>>> +Description: read only
>>> + Provide information about the amount of memory
>>> + reserved by fadump to save the crash dump.
>>
>> Split this up into a separate patch and have ABI documentation for
>> fadump_reserved & fadump_registered as well..
- Sourabh Jain
^ permalink raw reply
* Re: [PATCH v3] powerpc/fadump: sysfs for fadump memory reservation
From: Sourabh Jain @ 2019-08-26 10:16 UTC (permalink / raw)
To: Hari Bathini, mpe; +Cc: linuxppc-dev, mahesh, linux-kernel, linux-doc, corbet
In-Reply-To: <53311fa4-2cce-1eb6-1aae-0c835e06eb24@linux.ibm.com>
On 8/26/19 3:29 PM, Hari Bathini wrote:
>
>
> On 10/08/19 11:29 PM, Sourabh Jain wrote:
>> Add a sys interface to allow querying the memory reserved by
>> fadump for saving the crash dump.
>>
>> Add an ABI doc entry for new sysfs interface.
>> - /sys/kernel/fadump_mem_reserved
>>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>> Changelog:
>> v1 -> v2:
>> - Added ABI doc for new sysfs interface.
>>
>> v2 -> v3:
>> - Updated the ABI documentation.
>> ---
>>
>> Documentation/ABI/testing/sysfs-kernel-fadump | 6 ++++++
>
> Shouldn't this be Documentation/ABI/testing/sysfs-kernel-fadump_mem_reserved?
>
>> +++ b/Documentation/ABI/testing/sysfs-kernel-fadump
>> @@ -0,0 +1,6 @@
>> +What: /sys/kernel/fadump_mem_reserved
>> +Date: August 2019
>> +Contact: linuxppc-dev@lists.ozlabs.org
>> +Description: read only
>> + Provide information about the amount of memory
>> + reserved by fadump to save the crash dump.
>
> Split this up into a separate patch and have ABI documentation for
> fadump_reserved & fadump_registered as well..
Sure. Thanks for review.
- Sourabh Jain
^ permalink raw reply
* Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
From: Rasmus Villemoes @ 2019-08-26 10:13 UTC (permalink / raw)
To: Uwe Kleine-König, Andrew Morton
Cc: Jonathan Corbet, linux-doc, linux-kernel, linux-gpio,
Linus Walleij, Bartosz Golaszewski
In-Reply-To: <20190824233724.1775-1-uwe@kleine-koenig.org>
On 25/08/2019 01.37, Uwe Kleine-König wrote:
> pr_info("probing failed (%dE)\n", ret);
>
> expands to
>
> probing failed (EIO)
>
> if ret holds -EIO (or EIO). This introduces an array of error codes. If
> the error code is missing, %dE falls back to %d and so prints the plain
> number.
>
> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
> ---
> Hello
>
> there are many code sites that benefit from this. Just grep for
> "(%d)" ...
>
> As an example the follow up patch converts a printk to use this new
> format escape.
>
> Best regards
> Uwe
>
> Documentation/core-api/printk-formats.rst | 3 +
> lib/vsprintf.c | 193 +++++++++++++++++++++-
> 2 files changed, 195 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index c6224d039bcb..81002414f956 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -35,6 +35,9 @@ Integer types
> u64 %llu or %llx
>
>
> +To print the name that corresponds to an integer error constant, use %dE and
> +pass the int.
> +
> If <type> is dependent on a config option for its size (e.g., sector_t,
> blkcnt_t) or is architecture-dependent for its size (e.g., tcflag_t), use a
> format specifier of its largest possible type and explicitly cast to it.
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index b0967cf17137..672eab8dab84 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -533,6 +533,192 @@ char *number(char *buf, char *end, unsigned long long num,
> return buf;
> }
>
> +#define ERRORCODE(x) { .str = #x, .err = x }
> +
> +static const struct {
> + const char *str;
> + int err;
> +} errorcodes[] = {
> + ERRORCODE(EPERM),
...
> + ERRORCODE(ERECALLCONFLICT),
> +};
> +
> +static noinline_for_stack
> +char *errstr(char *buf, char *end, unsigned long long num,
> + struct printf_spec spec)
> +{
> + char *errname = NULL;
> + size_t errnamelen, copy;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(errorcodes); ++i) {
> + if (num == errorcodes[i].err || num == -errorcodes[i].err) {
> + errname = errorcodes[i].str;
> + break;
> + }
> + }
Not sure I'm a fan of iterating this array. Yes, the errno values are a
bit sparse, but maybe one could use a dense array with O(1) lookup for
those < 128 and then have an exceptional table like yours for the rest.
But if you do keep this whole array thing, please do as Andrew suggested
and compact it somewhat (4 bytes per entry plus the storage for the
strings themselves is enough, see e.g. e1f0bce3), and put EINVAL and
other common things near the start (at least EINVAL is a lot more common
than ENOEXEC).
> + if (!errname) {
> + /* fall back to ordinary number */
> + return number(buf, end, num, spec);
> + }
> +
> + copy = errnamelen = strlen(errname);
> + if (copy > end - buf)
> + copy = end - buf;
> + buf = memcpy(buf, errname, copy);
This is buggy, AFAICT. buf may be beyond end (that's the convention), so
end-buf (which is a ptrdiff_t, which is probably a signed type, but it
gets converted to a size_t when compared to copy) can be a huge number,
so copy>end-buf is false.
Please just use the string() helper that gets used for printing other
fixed strings (as well as %s input).
And add tests to lib/test_printf.c, that would catch this sort of thing
immediately.
> + return buf + errnamelen;
> +}
> +
> static noinline_for_stack
> char *special_hex_number(char *buf, char *end, unsigned long long num, int size)
> {
> @@ -2566,7 +2752,12 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
> num = va_arg(args, unsigned int);
> }
>
> - str = number(str, end, num, spec);
> + if (spec.type == FORMAT_TYPE_INT && *fmt == 'E') {
> + fmt++;
> + str = errstr(str, end, num, spec);
drivers/staging/speakup/speakup_bns.c has a %dE, have you checked
whether you're breaking that one? It's hard to grep for all the
variations, %-0*.5dE is also legal and would be caught here.
This has previously been suggested as a %p extension, and I think users
would just as often have an ERR_PTR() as a plain int or long. Since we
already have %p[alphanumeric] as a special kernel thing, why not do
that? It's more convenient to convert from ints/longs to error pointers
pr_err("Uh-oh: %pE", ERR_PTR(ret));
than the other way around
pr_err("Uh-oh: %dE", PTR_ERR(p)); // oops, must be %ldE
Kernel size impact? Have you considered making it possible to opt-out
and have %dE just mean %d?
Rasmus
^ permalink raw reply
* Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
From: Jani Nikula @ 2019-08-26 10:04 UTC (permalink / raw)
To: Andrew Morton, Uwe Kleine-König
Cc: Jonathan Corbet, linux-doc, linux-kernel, linux-gpio,
Linus Walleij, Bartosz Golaszewski, Petr Mladek,
Sergey Senozhatsky, Steven Rostedt
In-Reply-To: <87o90c9rkc.fsf@intel.com>
On Mon, 26 Aug 2019, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Sat, 24 Aug 2019, Andrew Morton <akpm@linux-foundation.org> wrote:
>>> --- a/lib/vsprintf.c
>>> +++ b/lib/vsprintf.c
>>> @@ -533,6 +533,192 @@ char *number(char *buf, char *end, unsigned long long num,
>>> return buf;
>>> }
>>>
>>> +#define ERRORCODE(x) { .str = #x, .err = x }
>>> +
>>> +static const struct {
>>> + const char *str;
>>> + int err;
>>> +} errorcodes[] = {
>>
>> It's a bit of a hack, but an array of char*'s and a separate array of
>> ushorts would save a bit of space.
>
> Or just
>
> #define ERRORCODE(x) [x] = #x
>
> static const char * const errorcodes[] = {
> ERRORCODE(EPERM),
> ERRORCODE(ENOENT),
> ...
> };
>
> Saves space, faster lookup, discovers at build time why EWOULDBLOCK
> would always show up as EAGAIN in the logs. We don't have holes to speak
> of in the error codes.
Meh, failed to notice the range ERESTARTSYS..ERECALLCONFLICT. Other than
that, it's nicer. ;)
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply
* Re: [PATCH v3] powerpc/fadump: sysfs for fadump memory reservation
From: Hari Bathini @ 2019-08-26 9:59 UTC (permalink / raw)
To: Sourabh Jain, mpe; +Cc: linuxppc-dev, mahesh, linux-kernel, linux-doc, corbet
In-Reply-To: <20190810175905.7761-1-sourabhjain@linux.ibm.com>
On 10/08/19 11:29 PM, Sourabh Jain wrote:
> Add a sys interface to allow querying the memory reserved by
> fadump for saving the crash dump.
>
> Add an ABI doc entry for new sysfs interface.
> - /sys/kernel/fadump_mem_reserved
>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
> Changelog:
> v1 -> v2:
> - Added ABI doc for new sysfs interface.
>
> v2 -> v3:
> - Updated the ABI documentation.
> ---
>
> Documentation/ABI/testing/sysfs-kernel-fadump | 6 ++++++
Shouldn't this be Documentation/ABI/testing/sysfs-kernel-fadump_mem_reserved?
> +++ b/Documentation/ABI/testing/sysfs-kernel-fadump
> @@ -0,0 +1,6 @@
> +What: /sys/kernel/fadump_mem_reserved
> +Date: August 2019
> +Contact: linuxppc-dev@lists.ozlabs.org
> +Description: read only
> + Provide information about the amount of memory
> + reserved by fadump to save the crash dump.
Split this up into a separate patch and have ABI documentation for
fadump_reserved & fadump_registered as well..
Thanks
Hari
^ permalink raw reply
* Re: [PATCH v1 1/2] vsprintf: introduce %dE for error constants
From: Jani Nikula @ 2019-08-26 9:58 UTC (permalink / raw)
To: Andrew Morton, Uwe Kleine-König
Cc: Jonathan Corbet, linux-doc, linux-kernel, linux-gpio,
Linus Walleij, Bartosz Golaszewski, Petr Mladek,
Sergey Senozhatsky, Steven Rostedt
In-Reply-To: <20190824165829.7d330367992c62dab87f6652@linux-foundation.org>
On Sat, 24 Aug 2019, Andrew Morton <akpm@linux-foundation.org> wrote:
>> --- a/lib/vsprintf.c
>> +++ b/lib/vsprintf.c
>> @@ -533,6 +533,192 @@ char *number(char *buf, char *end, unsigned long long num,
>> return buf;
>> }
>>
>> +#define ERRORCODE(x) { .str = #x, .err = x }
>> +
>> +static const struct {
>> + const char *str;
>> + int err;
>> +} errorcodes[] = {
>
> It's a bit of a hack, but an array of char*'s and a separate array of
> ushorts would save a bit of space.
Or just
#define ERRORCODE(x) [x] = #x
static const char * const errorcodes[] = {
ERRORCODE(EPERM),
ERRORCODE(ENOENT),
...
};
Saves space, faster lookup, discovers at build time why EWOULDBLOCK
would always show up as EAGAIN in the logs. We don't have holes to speak
of in the error codes.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply
* FAILED: patch "[PATCH] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h" failed to apply to 4.9-stable tree
From: gregkh @ 2019-08-26 8:59 UTC (permalink / raw)
To: thomas.lendacky, akpm, andrew.cooper3, bp, corbet, hpa, jgross,
jpoimboe, keescook, linux-doc, linux-pm, mingo, natechancellor,
pavel, pbonzini, rjw, stable, tglx, x86, yu.c.chen
Cc: stable
The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From c49a0a80137c7ca7d6ced4c812c9e07a949f6f24 Mon Sep 17 00:00:00 2001
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Mon, 19 Aug 2019 15:52:35 +0000
Subject: [PATCH] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
There have been reports of RDRAND issues after resuming from suspend on
some AMD family 15h and family 16h systems. This issue stems from a BIOS
not performing the proper steps during resume to ensure RDRAND continues
to function properly.
RDRAND support is indicated by CPUID Fn00000001_ECX[30]. This bit can be
reset by clearing MSR C001_1004[62]. Any software that checks for RDRAND
support using CPUID, including the kernel, will believe that RDRAND is
not supported.
Update the CPU initialization to clear the RDRAND CPUID bit for any family
15h and 16h processor that supports RDRAND. If it is known that the family
15h or family 16h system does not have an RDRAND resume issue or that the
system will not be placed in suspend, the "rdrand=force" kernel parameter
can be used to stop the clearing of the RDRAND CPUID bit.
Additionally, update the suspend and resume path to save and restore the
MSR C001_1004 value to ensure that the RDRAND CPUID setting remains in
place after resuming from suspend.
Note, that clearing the RDRAND CPUID bit does not prevent a processor
that normally supports the RDRAND instruction from executing it. So any
code that determined the support based on family and model won't #UD.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: <stable@vger.kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "x86@kernel.org" <x86@kernel.org>
Link: https://lkml.kernel.org/r/7543af91666f491547bd86cebb1e17c66824ab9f.1566229943.git.thomas.lendacky@amd.com
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 47d981a86e2f..4c1971960afa 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4090,6 +4090,13 @@
Run specified binary instead of /init from the ramdisk,
used for early userspace startup. See initrd.
+ rdrand= [X86]
+ force - Override the decision by the kernel to hide the
+ advertisement of RDRAND support (this affects
+ certain AMD processors because of buggy BIOS
+ support, specifically around the suspend/resume
+ path).
+
rdt= [HW,X86,RDT]
Turn on/off individual RDT features. List is:
cmt, mbmtotal, mbmlocal, l3cat, l3cdp, l2cat, l2cdp,
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 6b4fc2788078..271d837d69a8 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -381,6 +381,7 @@
#define MSR_AMD64_PATCH_LEVEL 0x0000008b
#define MSR_AMD64_TSC_RATIO 0xc0000104
#define MSR_AMD64_NB_CFG 0xc001001f
+#define MSR_AMD64_CPUID_FN_1 0xc0011004
#define MSR_AMD64_PATCH_LOADER 0xc0010020
#define MSR_AMD64_OSVW_ID_LENGTH 0xc0010140
#define MSR_AMD64_OSVW_STATUS 0xc0010141
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 8d4e50428b68..68c363c341bf 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -804,6 +804,64 @@ static void init_amd_ln(struct cpuinfo_x86 *c)
msr_set_bit(MSR_AMD64_DE_CFG, 31);
}
+static bool rdrand_force;
+
+static int __init rdrand_cmdline(char *str)
+{
+ if (!str)
+ return -EINVAL;
+
+ if (!strcmp(str, "force"))
+ rdrand_force = true;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("rdrand", rdrand_cmdline);
+
+static void clear_rdrand_cpuid_bit(struct cpuinfo_x86 *c)
+{
+ /*
+ * Saving of the MSR used to hide the RDRAND support during
+ * suspend/resume is done by arch/x86/power/cpu.c, which is
+ * dependent on CONFIG_PM_SLEEP.
+ */
+ if (!IS_ENABLED(CONFIG_PM_SLEEP))
+ return;
+
+ /*
+ * The nordrand option can clear X86_FEATURE_RDRAND, so check for
+ * RDRAND support using the CPUID function directly.
+ */
+ if (!(cpuid_ecx(1) & BIT(30)) || rdrand_force)
+ return;
+
+ msr_clear_bit(MSR_AMD64_CPUID_FN_1, 62);
+
+ /*
+ * Verify that the CPUID change has occurred in case the kernel is
+ * running virtualized and the hypervisor doesn't support the MSR.
+ */
+ if (cpuid_ecx(1) & BIT(30)) {
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, but hypervisor does not support hiding RDRAND via CPUID.\n");
+ return;
+ }
+
+ clear_cpu_cap(c, X86_FEATURE_RDRAND);
+ pr_info_once("BIOS may not properly restore RDRAND after suspend, hiding RDRAND via CPUID. Use rdrand=force to reenable.\n");
+}
+
+static void init_amd_jg(struct cpuinfo_x86 *c)
+{
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
+}
+
static void init_amd_bd(struct cpuinfo_x86 *c)
{
u64 value;
@@ -818,6 +876,13 @@ static void init_amd_bd(struct cpuinfo_x86 *c)
wrmsrl_safe(MSR_F15H_IC_CFG, value);
}
}
+
+ /*
+ * Some BIOS implementations do not restore proper RDRAND support
+ * across suspend and resume. Check on whether to hide the RDRAND
+ * instruction support via CPUID.
+ */
+ clear_rdrand_cpuid_bit(c);
}
static void init_amd_zn(struct cpuinfo_x86 *c)
@@ -860,6 +925,7 @@ static void init_amd(struct cpuinfo_x86 *c)
case 0x10: init_amd_gh(c); break;
case 0x12: init_amd_ln(c); break;
case 0x15: init_amd_bd(c); break;
+ case 0x16: init_amd_jg(c); break;
case 0x17: init_amd_zn(c); break;
}
diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 24b079e94bc2..c9ef6a7a4a1a 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -12,6 +12,7 @@
#include <linux/smp.h>
#include <linux/perf_event.h>
#include <linux/tboot.h>
+#include <linux/dmi.h>
#include <asm/pgtable.h>
#include <asm/proto.h>
@@ -23,7 +24,7 @@
#include <asm/debugreg.h>
#include <asm/cpu.h>
#include <asm/mmu_context.h>
-#include <linux/dmi.h>
+#include <asm/cpu_device_id.h>
#ifdef CONFIG_X86_32
__visible unsigned long saved_context_ebx;
@@ -397,15 +398,14 @@ static int __init bsp_pm_check_init(void)
core_initcall(bsp_pm_check_init);
-static int msr_init_context(const u32 *msr_id, const int total_num)
+static int msr_build_context(const u32 *msr_id, const int num)
{
- int i = 0;
+ struct saved_msrs *saved_msrs = &saved_context.saved_msrs;
struct saved_msr *msr_array;
+ int total_num;
+ int i, j;
- if (saved_context.saved_msrs.array || saved_context.saved_msrs.num > 0) {
- pr_err("x86/pm: MSR quirk already applied, please check your DMI match table.\n");
- return -EINVAL;
- }
+ total_num = saved_msrs->num + num;
msr_array = kmalloc_array(total_num, sizeof(struct saved_msr), GFP_KERNEL);
if (!msr_array) {
@@ -413,19 +413,30 @@ static int msr_init_context(const u32 *msr_id, const int total_num)
return -ENOMEM;
}
- for (i = 0; i < total_num; i++) {
- msr_array[i].info.msr_no = msr_id[i];
+ if (saved_msrs->array) {
+ /*
+ * Multiple callbacks can invoke this function, so copy any
+ * MSR save requests from previous invocations.
+ */
+ memcpy(msr_array, saved_msrs->array,
+ sizeof(struct saved_msr) * saved_msrs->num);
+
+ kfree(saved_msrs->array);
+ }
+
+ for (i = saved_msrs->num, j = 0; i < total_num; i++, j++) {
+ msr_array[i].info.msr_no = msr_id[j];
msr_array[i].valid = false;
msr_array[i].info.reg.q = 0;
}
- saved_context.saved_msrs.num = total_num;
- saved_context.saved_msrs.array = msr_array;
+ saved_msrs->num = total_num;
+ saved_msrs->array = msr_array;
return 0;
}
/*
- * The following section is a quirk framework for problematic BIOSen:
+ * The following sections are a quirk framework for problematic BIOSen:
* Sometimes MSRs are modified by the BIOSen after suspended to
* RAM, this might cause unexpected behavior after wakeup.
* Thus we save/restore these specified MSRs across suspend/resume
@@ -440,7 +451,7 @@ static int msr_initialize_bdw(const struct dmi_system_id *d)
u32 bdw_msr_id[] = { MSR_IA32_THERM_CONTROL };
pr_info("x86/pm: %s detected, MSR saving is needed during suspending.\n", d->ident);
- return msr_init_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
+ return msr_build_context(bdw_msr_id, ARRAY_SIZE(bdw_msr_id));
}
static const struct dmi_system_id msr_save_dmi_table[] = {
@@ -455,9 +466,58 @@ static const struct dmi_system_id msr_save_dmi_table[] = {
{}
};
+static int msr_save_cpuid_features(const struct x86_cpu_id *c)
+{
+ u32 cpuid_msr_id[] = {
+ MSR_AMD64_CPUID_FN_1,
+ };
+
+ pr_info("x86/pm: family %#hx cpu detected, MSR saving is needed during suspending.\n",
+ c->family);
+
+ return msr_build_context(cpuid_msr_id, ARRAY_SIZE(cpuid_msr_id));
+}
+
+static const struct x86_cpu_id msr_save_cpu_table[] = {
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x15,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {
+ .vendor = X86_VENDOR_AMD,
+ .family = 0x16,
+ .model = X86_MODEL_ANY,
+ .feature = X86_FEATURE_ANY,
+ .driver_data = (kernel_ulong_t)msr_save_cpuid_features,
+ },
+ {}
+};
+
+typedef int (*pm_cpu_match_t)(const struct x86_cpu_id *);
+static int pm_cpu_check(const struct x86_cpu_id *c)
+{
+ const struct x86_cpu_id *m;
+ int ret = 0;
+
+ m = x86_match_cpu(msr_save_cpu_table);
+ if (m) {
+ pm_cpu_match_t fn;
+
+ fn = (pm_cpu_match_t)m->driver_data;
+ ret = fn(m);
+ }
+
+ return ret;
+}
+
static int pm_check_save_msr(void)
{
dmi_check_system(msr_save_dmi_table);
+ pm_cpu_check(msr_save_cpu_table);
+
return 0;
}
^ permalink raw reply related
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