* [PATCH v1 6/7] soc: fsl: guts: drop platform driver
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
This driver cannot be unloaded and it will be needed very early in the
boot process because other driver (weakly) depend on it (eg. for chip
errata handling). Drop all the platform driver and devres stuff and
simply make it a core_initcall.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/soc/fsl/guts.c | 127 +++++++++++++++++++++--------------------
1 file changed, 65 insertions(+), 62 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 04c3eb6a6e17..be961a9193f4 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -110,19 +110,55 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
return NULL;
}
-static int fsl_guts_probe(struct platform_device *pdev)
+/*
+ * Table for matching compatible strings, for device tree
+ * guts node, for Freescale QorIQ SOCs.
+ */
+static const struct of_device_id fsl_guts_of_match[] = {
+ { .compatible = "fsl,qoriq-device-config-1.0", },
+ { .compatible = "fsl,qoriq-device-config-2.0", },
+ { .compatible = "fsl,p1010-guts", },
+ { .compatible = "fsl,p1020-guts", },
+ { .compatible = "fsl,p1021-guts", },
+ { .compatible = "fsl,p1022-guts", },
+ { .compatible = "fsl,p1023-guts", },
+ { .compatible = "fsl,p2020-guts", },
+ { .compatible = "fsl,bsc9131-guts", },
+ { .compatible = "fsl,bsc9132-guts", },
+ { .compatible = "fsl,mpc8536-guts", },
+ { .compatible = "fsl,mpc8544-guts", },
+ { .compatible = "fsl,mpc8548-guts", },
+ { .compatible = "fsl,mpc8568-guts", },
+ { .compatible = "fsl,mpc8569-guts", },
+ { .compatible = "fsl,mpc8572-guts", },
+ { .compatible = "fsl,ls1021a-dcfg", },
+ { .compatible = "fsl,ls1043a-dcfg", },
+ { .compatible = "fsl,ls2080a-dcfg", },
+ { .compatible = "fsl,ls1088a-dcfg", },
+ { .compatible = "fsl,ls1012a-dcfg", },
+ { .compatible = "fsl,ls1046a-dcfg", },
+ { .compatible = "fsl,lx2160a-dcfg", },
+ { .compatible = "fsl,ls1028a-dcfg", },
+ {}
+};
+
+static int __init fsl_guts_init(void)
{
- struct device_node *np = pdev->dev.of_node;
struct soc_device_attribute *soc_dev_attr;
static struct soc_device *soc_dev;
- struct device *dev = &pdev->dev;
const struct fsl_soc_die_attr *soc_die;
struct ccsr_guts __iomem *regs;
const char *machine = NULL;
+ struct device_node *np;
bool little_endian;
u32 svr;
+ int ret;
- soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL);
+ np = of_find_matching_node_and_match(NULL, fsl_guts_of_match, NULL);
+ if (!np)
+ return 0;
+
+ soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
if (!soc_dev_attr)
return -ENOMEM;
@@ -138,87 +174,54 @@ static int fsl_guts_probe(struct platform_device *pdev)
svr = ioread32be(®s->svr);
iounmap(regs);
- /* Register soc device */
if (of_property_read_string(of_root, "model", &machine))
of_property_read_string_index(of_root, "compatible", 0, &machine);
if (machine) {
- soc_dev_attr->machine = devm_kstrdup(dev, machine, GFP_KERNEL);
+ soc_dev_attr->machine = kstrdup(machine, GFP_KERNEL);
if (!soc_dev_attr->machine)
return -ENOMEM;
}
soc_die = fsl_soc_die_match(svr, fsl_soc_die);
if (soc_die) {
- soc_dev_attr->family = devm_kasprintf(dev, GFP_KERNEL,
- "QorIQ %s", soc_die->die);
+ soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ %s",
+ soc_die->die);
} else {
- soc_dev_attr->family = devm_kasprintf(dev, GFP_KERNEL, "QorIQ");
+ soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ");
}
if (!soc_dev_attr->family)
- return -ENOMEM;
- soc_dev_attr->soc_id = devm_kasprintf(dev, GFP_KERNEL,
- "svr:0x%08x", svr);
+ goto err_nomem;
+
+ soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", svr);
if (!soc_dev_attr->soc_id)
- return -ENOMEM;
- soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
- (svr >> 4) & 0xf, svr & 0xf);
+ goto err_nomem;
+
+ soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d",
+ (svr >> 4) & 0xf, svr & 0xf);
if (!soc_dev_attr->revision)
- return -ENOMEM;
+ goto err_nomem;
soc_dev = soc_device_register(soc_dev_attr);
- if (IS_ERR(soc_dev))
- return PTR_ERR(soc_dev);
+ if (IS_ERR(soc_dev)) {
+ ret = PTR_ERR(soc_dev);
+ goto err;
+ }
pr_info("Machine: %s\n", soc_dev_attr->machine);
pr_info("SoC family: %s\n", soc_dev_attr->family);
pr_info("SoC ID: %s, Revision: %s\n",
soc_dev_attr->soc_id, soc_dev_attr->revision);
- return 0;
-}
-/*
- * Table for matching compatible strings, for device tree
- * guts node, for Freescale QorIQ SOCs.
- */
-static const struct of_device_id fsl_guts_of_match[] = {
- { .compatible = "fsl,qoriq-device-config-1.0", },
- { .compatible = "fsl,qoriq-device-config-2.0", },
- { .compatible = "fsl,p1010-guts", },
- { .compatible = "fsl,p1020-guts", },
- { .compatible = "fsl,p1021-guts", },
- { .compatible = "fsl,p1022-guts", },
- { .compatible = "fsl,p1023-guts", },
- { .compatible = "fsl,p2020-guts", },
- { .compatible = "fsl,bsc9131-guts", },
- { .compatible = "fsl,bsc9132-guts", },
- { .compatible = "fsl,mpc8536-guts", },
- { .compatible = "fsl,mpc8544-guts", },
- { .compatible = "fsl,mpc8548-guts", },
- { .compatible = "fsl,mpc8568-guts", },
- { .compatible = "fsl,mpc8569-guts", },
- { .compatible = "fsl,mpc8572-guts", },
- { .compatible = "fsl,ls1021a-dcfg", },
- { .compatible = "fsl,ls1043a-dcfg", },
- { .compatible = "fsl,ls2080a-dcfg", },
- { .compatible = "fsl,ls1088a-dcfg", },
- { .compatible = "fsl,ls1012a-dcfg", },
- { .compatible = "fsl,ls1046a-dcfg", },
- { .compatible = "fsl,lx2160a-dcfg", },
- { .compatible = "fsl,ls1028a-dcfg", },
- {}
-};
-MODULE_DEVICE_TABLE(of, fsl_guts_of_match);
+ return 0;
-static struct platform_driver fsl_guts_driver = {
- .driver = {
- .name = "fsl-guts",
- .of_match_table = fsl_guts_of_match,
- },
- .probe = fsl_guts_probe,
-};
+err_nomem:
+ ret = -ENOMEM;
+err:
+ kfree(soc_dev_attr->machine);
+ kfree(soc_dev_attr->family);
+ kfree(soc_dev_attr->soc_id);
+ kfree(soc_dev_attr->revision);
-static int __init fsl_guts_init(void)
-{
- return platform_driver_register(&fsl_guts_driver);
+ return ret;
}
core_initcall(fsl_guts_init);
--
2.30.2
^ permalink raw reply related
* [PATCH v1 5/7] soc: fsl: guts: use of_root instead of own reference
From: Michael Walle @ 2022-01-27 16:41 UTC (permalink / raw)
To: linuxppc-dev, linux-arm-kernel, linux-kernel
Cc: Ulf Hansson, Sudeep Holla, Michael Walle, Arnd Bergmann, Li Yang
In-Reply-To: <20220127164125.3651285-1-michael@walle.cc>
There is already a global of_root reference. Use that instead of getting
one on our own. We don't need to care about the reference count either
this way.
Signed-off-by: Michael Walle <michael@walle.cc>
---
drivers/soc/fsl/guts.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 383b35a4ed58..04c3eb6a6e17 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -112,7 +112,7 @@ static const struct fsl_soc_die_attr *fsl_soc_die_match(
static int fsl_guts_probe(struct platform_device *pdev)
{
- struct device_node *root, *np = pdev->dev.of_node;
+ struct device_node *np = pdev->dev.of_node;
struct soc_device_attribute *soc_dev_attr;
static struct soc_device *soc_dev;
struct device *dev = &pdev->dev;
@@ -139,17 +139,13 @@ static int fsl_guts_probe(struct platform_device *pdev)
iounmap(regs);
/* Register soc device */
- root = of_find_node_by_path("/");
- if (of_property_read_string(root, "model", &machine))
- of_property_read_string_index(root, "compatible", 0, &machine);
+ if (of_property_read_string(of_root, "model", &machine))
+ of_property_read_string_index(of_root, "compatible", 0, &machine);
if (machine) {
soc_dev_attr->machine = devm_kstrdup(dev, machine, GFP_KERNEL);
- if (!soc_dev_attr->machine) {
- of_node_put(root);
+ if (!soc_dev_attr->machine)
return -ENOMEM;
- }
}
- of_node_put(root);
soc_die = fsl_soc_die_match(svr, fsl_soc_die);
if (soc_die) {
--
2.30.2
^ permalink raw reply related
* Re: [PATCH 6/7] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Christophe Leroy @ 2022-01-27 18:04 UTC (permalink / raw)
To: Miroslav Benes
Cc: linux-arch@vger.kernel.org, Daniel Thompson,
kgdb-bugreport@lists.sourceforge.net, Jason Wessel,
linux-kernel@vger.kernel.org, Douglas Anderson,
linux-mm@kvack.org, Luis Chamberlain, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <alpine.LSU.2.21.2201271704470.26559@pobox.suse.cz>
Le 27/01/2022 à 17:05, Miroslav Benes a écrit :
>> @@ -195,6 +208,9 @@ static void mod_tree_remove(struct module *mod)
>> {
>> __mod_tree_remove(&mod->core_layout.mtn, &mod_tree);
>> mod_tree_remove_init(mod);
>> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
>> + __mod_tree_remove(&mod->core_layout.mtn, &mod_data_tree);
>
> s/core_layout/data_layout/ ?
Oops, you are right. I should have awaited a few more hours before
sending v2.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH] ftrace: Have architectures opt-in for mcount build time sorting
From: Mark Rutland @ 2022-01-27 18:23 UTC (permalink / raw)
To: Steven Rostedt
Cc: Kees Cook, LKML, Ingo Molnar, Sachin Sant, Russell King,
Andrew Morton, Yinan Liu, linuxppc-dev, Ard Biesheuvel,
linux-arm-kernel
In-Reply-To: <20220127114249.03b1b52b@gandalf.local.home>
On Thu, Jan 27, 2022 at 11:42:49AM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> First S390 complained that the sorting of the mcount sections at build
> time caused the kernel to crash on their architecture. Now PowerPC is
> complaining about it too. And also ARM64 appears to be having issues.
>
> It may be necessary to also update the relocation table for the values
> in the mcount table. Not only do we have to sort the table, but also
> update the relocations that may be applied to the items in the table.
>
> If the system is not relocatable, then it is fine to sort, but if it is,
> some architectures may have issues (although x86 does not as it shifts all
> addresses the same).
>
> Add a HAVE_BUILDTIME_MCOUNT_SORT that an architecture can set to say it is
> safe to do the sorting at build time.
>
> Also update the config to compile in build time sorting in the sorttable
> code in scripts/ to depend on CONFIG_BUILDTIME_MCOUNT_SORT.
>
> Link: https://lore.kernel.org/all/944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com/
>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Yinan Liu <yinan@linux.alibaba.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Reported-by: Sachin Sant <sachinp@linux.ibm.com>
> Tested-by: Sachin Sant <sachinp@linux.ibm.com>
> Fixes: 72b3942a173c ("scripts: ftrace - move the sort-processing in ftrace_init")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Thanks for this; the rationale and patch makes sense to me. As previously, I
understand that:
* For arch/arm, the build-time sort should be safe as the 32-bit kernel isn't
virtually relocatable, and so the sort affects the final values, and will not
be clobbered later.
* For arch/x86, the build time sort should be safe as the mcount_loc will be
initialazed with values consistent with the default load address, and the
boot-time relocation will add the same offset to all values, so there's no
need to sort the relocs.
So enabling this for arch/arm and arch/x86 makes sense to me.
Similarly, I understand that for arm64 the build-time sort isn't sound due to
not adjusting the relocations, and so it needs to be disabled there (for now at
least).
I gave this patch a spin on arm64 atop v5.17-rc1, using GCC 11.1.0 from the
kernel.org crosstool page:
https://mirrors.edge.kernel.org/pub/tools/crosstool/
... with this applied, I'm no longer seeing a number of ftrace selftest
failures and ftrace internal bugs I previously reported at:
https://lore.kernel.org/all/YfKGKWW5UfZ15kCW@FVFF77S0Q05N/
It looks like there's a trivial bit of whitespace damage in the patch (noted
below), but regardless:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutlad@arm.com> [arm64]
As a heads-up, with this issue out of the way I'm hitting some unrelated issues
when running the ftrace selftests on arm64, which I'll look into. Quick summary
on those below, but I'll start new threads once I've got more detail.
* The duplicate events test seems to fail consistently:
[15] Generic dynamic event - check if duplicate events are caught [FAIL]
* I intermittently see a hang when running the tests. I previously hit that
when originally trying to bisect this issue (and IIRC that bisected down to
some RCU changes, but I need to re-run that). When the tests hang I
magic-srsrq + L tells me:
[ 271.938438] sysrq: Show Blocked State
[ 271.939245] task:ftracetest state:D stack: 0 pid: 5687 ppid: 5627 flags:0x00000200
[ 271.940961] Call trace:
[ 271.941472] __switch_to+0x104/0x160
[ 271.942213] __schedule+0x2b0/0x6e0
[ 271.942933] schedule+0x5c/0xf0
[ 271.943586] schedule_timeout+0x184/0x1c4
[ 271.944410] wait_for_completion+0x8c/0x12c
[ 271.945274] __wait_rcu_gp+0x184/0x190
[ 271.946047] synchronize_rcu_tasks_rude+0x48/0x70
[ 271.947007] update_ftrace_function+0xa4/0xec
[ 271.947897] __unregister_ftrace_function+0xa4/0xf0
[ 271.948898] unregister_ftrace_function+0x34/0x70
[ 271.949857] wakeup_tracer_reset+0x4c/0x100
[ 271.950713] tracing_set_tracer+0xd0/0x2b0
[ 271.951552] tracing_set_trace_write+0xe8/0x150
[ 271.952477] vfs_write+0xfc/0x284
[ 271.953171] ksys_write+0x7c/0x110
[ 271.953874] __arm64_sys_write+0x2c/0x40
[ 271.954678] invoke_syscall+0x5c/0x130
[ 271.955442] el0_svc_common.constprop.0+0x108/0x130
[ 271.956435] do_el0_svc+0x74/0x90
[ 271.957124] el0_svc+0x2c/0x90
[ 271.957757] el0t_64_sync_handler+0xa8/0x12c
[ 271.958629] el0t_64_sync+0x1a0/0x1a4
> ---
> arch/arm/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> kernel/trace/Kconfig | 8 +++++++-
> scripts/Makefile | 2 +-
> 4 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index c2724d986fa0..5256ebe57451 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -82,6 +82,7 @@ config ARM
> select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
> select HAVE_CONTEXT_TRACKING
> select HAVE_C_RECORDMCOUNT
> + select HAVE_BUILDTIME_MCOUNT_SORT
> select HAVE_DEBUG_KMEMLEAK if !XIP_KERNEL
> select HAVE_DMA_CONTIGUOUS if MMU
> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7399327d1eff..46080dea5dba 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -186,6 +186,7 @@ config X86
> select HAVE_CONTEXT_TRACKING_OFFSTACK if HAVE_CONTEXT_TRACKING
> select HAVE_C_RECORDMCOUNT
> select HAVE_OBJTOOL_MCOUNT if STACK_VALIDATION
> + select HAVE_BUILDTIME_MCOUNT_SORT
> select HAVE_DEBUG_KMEMLEAK
> select HAVE_DMA_CONTIGUOUS
> select HAVE_DYNAMIC_FTRACE
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 752ed89a293b..7e5b92090faa 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -70,10 +70,16 @@ config HAVE_C_RECORDMCOUNT
> help
> C version of recordmcount available?
>
> +config HAVE_BUILDTIME_MCOUNT_SORT
> + bool
> + help
> + An architecture selects this if it sorts the mcount_loc section
> + at build time.
Whitespace damage here? The second line has a tab, the first doesn't.
Thanks,
Mark.
> +
> config BUILDTIME_MCOUNT_SORT
> bool
> default y
> - depends on BUILDTIME_TABLE_SORT && !S390
> + depends on HAVE_BUILDTIME_MCOUNT_SORT
> help
> Sort the mcount_loc section at build time.
>
> diff --git a/scripts/Makefile b/scripts/Makefile
> index b082d2f93357..cedc1f0e21d8 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -32,7 +32,7 @@ HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
> HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
> endif
>
> -ifdef CONFIG_DYNAMIC_FTRACE
> +ifdef CONFIG_BUILDTIME_MCOUNT_SORT
> HOSTCFLAGS_sorttable.o += -DMCOUNT_SORT_ENABLED
> endif
>
> --
> 2.33.0
>
^ permalink raw reply
* [PATCH v2] ftrace: Have architectures opt-in for mcount build time sorting
From: Steven Rostedt @ 2022-01-27 20:38 UTC (permalink / raw)
To: LKML
Cc: Mark Rutland, Kees Cook, Russell King, Ard Biesheuvel,
Sachin Sant, Andrew Morton, Yinan Liu, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
From f7d4ef4e77464e08af38789ea5d3a9cfb80a3d78 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Date: Tue, 25 Jan 2022 09:19:10 -0500
Subject: [PATCH] ftrace: Have architectures opt-in for mcount build time
sorting
First S390 complained that the sorting of the mcount sections at build
time caused the kernel to crash on their architecture. Now PowerPC is
complaining about it too. And also ARM64 appears to be having issues.
It may be necessary to also update the relocation table for the values
in the mcount table. Not only do we have to sort the table, but also
update the relocations that may be applied to the items in the table.
If the system is not relocatable, then it is fine to sort, but if it is,
some architectures may have issues (although x86 does not as it shifts all
addresses the same).
Add a HAVE_BUILDTIME_MCOUNT_SORT that an architecture can set to say it is
safe to do the sorting at build time.
Also update the config to compile in build time sorting in the sorttable
code in scripts/ to depend on CONFIG_BUILDTIME_MCOUNT_SORT.
Link: https://lore.kernel.org/all/944D10DA-8200-4BA9-8D0A-3BED9AA99F82@linux.ibm.com/
Cc: Yinan Liu <yinan@linux.alibaba.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Reported-by: Sachin Sant <sachinp@linux.ibm.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com> [arm64]
Tested-by: Sachin Sant <sachinp@linux.ibm.com>
Fixes: 72b3942a173c ("scripts: ftrace - move the sort-processing in ftrace_init")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://lore.kernel.org/all/20220127114249.03b1b52b@gandalf.local.home/
- Have CONFIG_BUILDTIME_MCOUNT_SORT depend on DYNAMIC_FTRACE
otherwise it fails to build when DYNAMIC_FTRACE is not set
because it can not find the mcount section in the sorttable code.
arch/arm/Kconfig | 1 +
arch/x86/Kconfig | 1 +
kernel/trace/Kconfig | 8 +++++++-
scripts/Makefile | 2 +-
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c2724d986fa0..5256ebe57451 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -82,6 +82,7 @@ config ARM
select HAVE_EBPF_JIT if !CPU_ENDIAN_BE32
select HAVE_CONTEXT_TRACKING
select HAVE_C_RECORDMCOUNT
+ select HAVE_BUILDTIME_MCOUNT_SORT
select HAVE_DEBUG_KMEMLEAK if !XIP_KERNEL
select HAVE_DMA_CONTIGUOUS if MMU
select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7399327d1eff..46080dea5dba 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -186,6 +186,7 @@ config X86
select HAVE_CONTEXT_TRACKING_OFFSTACK if HAVE_CONTEXT_TRACKING
select HAVE_C_RECORDMCOUNT
select HAVE_OBJTOOL_MCOUNT if STACK_VALIDATION
+ select HAVE_BUILDTIME_MCOUNT_SORT
select HAVE_DEBUG_KMEMLEAK
select HAVE_DMA_CONTIGUOUS
select HAVE_DYNAMIC_FTRACE
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 752ed89a293b..a5eb5e7fd624 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -70,10 +70,16 @@ config HAVE_C_RECORDMCOUNT
help
C version of recordmcount available?
+config HAVE_BUILDTIME_MCOUNT_SORT
+ bool
+ help
+ An architecture selects this if it sorts the mcount_loc section
+ at build time.
+
config BUILDTIME_MCOUNT_SORT
bool
default y
- depends on BUILDTIME_TABLE_SORT && !S390
+ depends on HAVE_BUILDTIME_MCOUNT_SORT && DYNAMIC_FTRACE
help
Sort the mcount_loc section at build time.
diff --git a/scripts/Makefile b/scripts/Makefile
index b082d2f93357..cedc1f0e21d8 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -32,7 +32,7 @@ HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
endif
-ifdef CONFIG_DYNAMIC_FTRACE
+ifdef CONFIG_BUILDTIME_MCOUNT_SORT
HOSTCFLAGS_sorttable.o += -DMCOUNT_SORT_ENABLED
endif
--
2.33.0
^ permalink raw reply related
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Sven Schnelle @ 2022-01-27 12:04 UTC (permalink / raw)
To: Mark Rutland
Cc: keescook, linux-kernel, Steven Rostedt, Sachin Sant, Yinan Liu,
linuxppc-dev, ardb
In-Reply-To: <YfKGKWW5UfZ15kCW@FVFF77S0Q05N>
Mark Rutland <mark.rutland@arm.com> writes:
>> Isn't x86 relocatable in some configurations (e.g. for KASLR)?
>>
>> I can't see how the sort works for those cases, because the mcount_loc entries
>> are absolute, and either:
>>
>> * The sorted entries will get overwritten by the unsorted relocation entries,
>> and won't be sorted.
>>
>> * The sorted entries won't get overwritten, but then the absolute address will
>> be wrong since they hadn't been relocated.
>>
>> How does that work?
From what i've seen when looking into this ftrace sort problem x86 has a
a relocation tool, which is run before final linking: arch/x86/tools/relocs.c
This tools converts all the required relocations to three types:
- 32 bit relocations
- 64 bit relocations
- inverse 32 bit relocations
These are added to the end of the image.
The decompressor then iterates over that array, and just adds/subtracts
the KASLR offset - see arch/x86/boot/compressed/misc.c, handle_relocations()
So IMHO x86 never uses 'real' relocations during boot, and just
adds/subtracts. That's why the order stays the same, and the compile
time sort works.
/Sven
^ permalink raw reply
* Re: [powerpc] ftrace warning kernel/trace/ftrace.c:2068 with code-patching selftests
From: Sven Schnelle @ 2022-01-27 13:16 UTC (permalink / raw)
To: Mark Rutland
Cc: keescook, hca, linux-kernel, Steven Rostedt, Sachin Sant,
Yinan Liu, linuxppc-dev, ardb
In-Reply-To: <YfKZXvB9vCN1bA1c@FVFF77S0Q05N>
Mark Rutland <mark.rutland@arm.com> writes:
> On Thu, Jan 27, 2022 at 07:46:01AM -0500, Steven Rostedt wrote:
>> On Thu, 27 Jan 2022 12:27:04 +0000
>> Mark Rutland <mark.rutland@arm.com> wrote:
>>
>> > Ah, so those non-ELF relocations for the mcount_loc table just mean "apply the
>> > KASLR offset here", which is equivalent for all entries.
>> >
>> > That makes sense, thanks!
>>
>> And this is why we were having such a hard time understanding each other ;-)
>
> ;)
>
> With that in mind, I think that we understand that the build-time sort works
> for:
>
> * arch/x86, becuase the non-ELF relocations for mcount_loc happen to be
> equivalent.
>
> * arch/arm, because there's no dynamic relocaiton and the mcount_loc entries
> have been finalized prior to sorting.
>
> ... but doesn't work for anyone else (including arm64) because the ELF
> relocations are not equivalent, and need special care that is not yet
> implemented.
For s390 my idea is to just skip the addresses between __start_mcount_loc
and __stop_mcount_loc, because for these addresses we know that they are
64 bits wide, so we just need to add the KASLR offset.
I'm thinking about something like this:
diff --git a/arch/s390/boot/compressed/decompressor.h b/arch/s390/boot/compressed/decompressor.h
index f75cc31a77dd..015d7e2e94ef 100644
--- a/arch/s390/boot/compressed/decompressor.h
+++ b/arch/s390/boot/compressed/decompressor.h
@@ -25,6 +25,8 @@ struct vmlinux_info {
unsigned long rela_dyn_start;
unsigned long rela_dyn_end;
unsigned long amode31_size;
+ unsigned long start_mcount_loc;
+ unsigned long stop_mcount_loc;
};
/* Symbols defined by linker scripts */
diff --git a/arch/s390/boot/startup.c b/arch/s390/boot/startup.c
index 1aa11a8f57dd..7bb0d88db5c6 100644
--- a/arch/s390/boot/startup.c
+++ b/arch/s390/boot/startup.c
@@ -88,6 +88,11 @@ static void handle_relocs(unsigned long offset)
dynsym = (Elf64_Sym *) vmlinux.dynsym_start;
for (rela = rela_start; rela < rela_end; rela++) {
loc = rela->r_offset + offset;
+ if ((loc >= vmlinux.start_mcount_loc) &&
+ (loc < vmlinux.stop_mcount_loc)) {
+ (*(unsigned long *)loc) += offset;
+ continue;
+ }
val = rela->r_addend;
r_sym = ELF64_R_SYM(rela->r_info);
if (r_sym) {
@@ -232,6 +237,8 @@ static void offset_vmlinux_info(unsigned long offset)
vmlinux.rela_dyn_start += offset;
vmlinux.rela_dyn_end += offset;
vmlinux.dynsym_start += offset;
+ vmlinux.start_mcount_loc += offset;
+ vmlinux.stop_mcount_loc += offset;
}
static unsigned long reserve_amode31(unsigned long safe_addr)
diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
index 42c43521878f..51c773405608 100644
--- a/arch/s390/kernel/vmlinux.lds.S
+++ b/arch/s390/kernel/vmlinux.lds.S
@@ -213,6 +213,8 @@ SECTIONS
QUAD(__rela_dyn_start) /* rela_dyn_start */
QUAD(__rela_dyn_end) /* rela_dyn_end */
QUAD(_eamode31 - _samode31) /* amode31_size */
+ QUAD(__start_mcount_loc)
+ QUAD(__stop_mcount_loc)
} :NONE
/* Debugging sections. */
Not sure whether that would also work on power, and also not sure
whether i missed something thinking about that. Maybe it doesn't even
work. ;-)
^ permalink raw reply related
* Re: [PATCH v5 04/21] kernel: Add combined power-off+restart handler call chain API
From: Dmitry Osipenko @ 2022-01-27 14:39 UTC (permalink / raw)
To: Michał Mirosław
Cc: Ulf Hansson, Rich Felker, linux-ia64, Santosh Shilimkar,
Rafael J. Wysocki, Boris Ostrovsky, Linus Walleij, Dave Hansen,
Liam Girdwood, James E.J. Bottomley, Thierry Reding,
Paul Mackerras, Pavel Machek, H. Peter Anvin, linux-riscv,
Vincent Chen, Will Deacon, Greg Ungerer, Stefano Stabellini,
alankao, Yoshinori Sato, Krzysztof Kozlowski, linux-sh,
Helge Deller, x86, Russell King, linux-csky, Jonathan Hunter,
linux-acpi, Ingo Molnar, Geert Uytterhoeven, Catalin Marinas,
xen-devel, linux-mips, Guenter Roeck, Len Brown, Albert Ou,
Lee Jones, linux-m68k, Mark Brown, Borislav Petkov, Greentime Hu,
Paul Walmsley, linux-tegra, Thomas Gleixner, Andy Shevchenko,
Juergen Gross, Thomas Bogendoerfer, Daniel Lezcano, linux-parisc,
linux-pm, Sebastian Reichel, linux-kernel, K . C . Kuen-Chern Lin,
Palmer Dabbelt, Philipp Zabel, Guo Ren, Andrew Morton,
linuxppc-dev, Joshua Thompson
In-Reply-To: <Ydofs2CIfA+r5KAz@qmqm.qmqm.pl>
Hello Michał,
09.01.2022 02:35, Michał Mirosław пишет:
> BTW, I couldn't find a right description of my idea of unifying the
> chains before, so let me sketch it now.
>
> The idea is to have a single system-off chain in which the callback
> gets a mode ({QUERY_*, PREP_*, DO_*} for each of {*_REBOOT, *_POWEROFF, ...?).
> The QUERY_* calls would be made in can_kernel_reboot/poweroff(): all
> would be called, and if at least one returned true, then the shutdown
> mode would continue. All of PREP_* would be called then. After that
> all DO_* would be tried until one doesn't return (succeeded or broke
> the system hard). Classic for(;;); could be a final fallback for the
> case where arch/machine (lowest priority) call would return instead
> of halting the system in machine-dependent way. The QUERY and PREP
> stages could be combined, but I haven't thought about it enough to
> see what conditions would need to be imposed on the callbacks in
> that case (maybe it's not worth the trouble, since it isn't a fast
> path anyway?). The goal here is to have less (duplicated) code in
> kernel, but otherwise it seems equivalent to your API proposal.
Thank you again for yours proposal! IMO, it's much more important to
keep the core code simple and maintainable, rather than try to optimize
it without a very good reason, given that this isn't a hot code path at
all and saving a couple of bytes won't be noticeable. The poweroff,
restart and reboot were separated before this series and I'm finding
that it's easier to follow the code when it's structured that way. I'm
not convinced that we need to change it.
^ permalink raw reply
* Re: Linux kernel: powerpc: KVM guest can trigger host crash on Power8
From: Mike @ 2022-01-27 15:50 UTC (permalink / raw)
To: John Paul Adrian Glaubitz; +Cc: debian-powerpc, open list:LINUX FOR POWERPC...
In-Reply-To: <200a989d-0aa9-c50f-855a-159b4e8ea441@physik.fu-berlin.de>
[-- Attachment #1: Type: text/plain, Size: 1739 bytes --]
I just made the huge mistake of hibernating and resuming, I'm going trough
the process of rescue and all, thankfully I had a 2016 cd in the drive.
I'll read up once the sheer panic settles.
-Michael
On Wed, Jan 26, 2022, 21:22 John Paul Adrian Glaubitz <
glaubitz@physik.fu-berlin.de> wrote:
> Hi Michael!
>
> On 1/13/22 01:17, John Paul Adrian Glaubitz wrote:
> > On 1/9/22 23:17, John Paul Adrian Glaubitz wrote:
> >> On 1/7/22 12:20, John Paul Adrian Glaubitz wrote:
> >>>> Can you separately test with (on the host):
> >>>>
> >>>> # echo 0 > /sys/module/kvm_hv/parameters/dynamic_mt_modes
> >>>
> >>> I'm trying to turn off "dynamic_mt_modes" first and see if that makes
> any difference.
> >>>
> >>> I will report back.
> >>
> >> So far the machine is running stable now and the VM built gcc-9 without
> >> crashing the host. I will continue to monitor the machine and report
> back
> >> if it crashes, but it looks like this could be it.
> >
> > So, it seems that setting "dynamic_mt_modes" actually did the trick, the
> host is no longer
> > crashing. However, I have observed on two occasions now that the build
> VM is just suddenly
> > off as if someone has shut it down using the "force-off" option in the
> virt-manager user
> > interface.
>
> Just as a heads-up. Ever since I set
>
> echo 0 > /sys/module/kvm_hv/parameters/dynamic_mt_modes
>
> on the host machine, I never saw the crash again. So the issue seems to be
> related to the
> dynamic_mt_modes feature.
>
> Thanks,
> Adrian
>
> --
> .''`. John Paul Adrian Glaubitz
> : :' : Debian Developer - glaubitz@debian.org
> `. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
> `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
>
>
[-- Attachment #2: Type: text/html, Size: 2520 bytes --]
^ permalink raw reply
* ftrace hangs waiting for rcu (was: Re: [PATCH] ftrace: Have architectures opt-in for mcount build time sorting)
From: Sven Schnelle @ 2022-01-27 18:42 UTC (permalink / raw)
To: Mark Rutland
Cc: Kees Cook, hca, LKML, Steven Rostedt, Ingo Molnar, linux-s390,
Sachin Sant, Russell King, Andrew Morton, Yinan Liu, linuxppc-dev,
Ard Biesheuvel, linux-arm-kernel
In-Reply-To: <YfLjIOlGfFmbh1Zv@FVFF77S0Q05N>
Hi Mark,
Mark Rutland <mark.rutland@arm.com> writes:
> * I intermittently see a hang when running the tests. I previously hit that
> when originally trying to bisect this issue (and IIRC that bisected down to
> some RCU changes, but I need to re-run that). When the tests hang I
> magic-srsrq + L tells me:
>
> [ 271.938438] sysrq: Show Blocked State
> [ 271.939245] task:ftracetest state:D stack: 0 pid: 5687 ppid: 5627 flags:0x00000200
> [ 271.940961] Call trace:
> [ 271.941472] __switch_to+0x104/0x160
> [ 271.942213] __schedule+0x2b0/0x6e0
> [ 271.942933] schedule+0x5c/0xf0
> [ 271.943586] schedule_timeout+0x184/0x1c4
> [ 271.944410] wait_for_completion+0x8c/0x12c
> [ 271.945274] __wait_rcu_gp+0x184/0x190
> [ 271.946047] synchronize_rcu_tasks_rude+0x48/0x70
> [ 271.947007] update_ftrace_function+0xa4/0xec
> [ 271.947897] __unregister_ftrace_function+0xa4/0xf0
> [ 271.948898] unregister_ftrace_function+0x34/0x70
> [ 271.949857] wakeup_tracer_reset+0x4c/0x100
> [ 271.950713] tracing_set_tracer+0xd0/0x2b0
> [ 271.951552] tracing_set_trace_write+0xe8/0x150
> [ 271.952477] vfs_write+0xfc/0x284
> [ 271.953171] ksys_write+0x7c/0x110
> [ 271.953874] __arm64_sys_write+0x2c/0x40
> [ 271.954678] invoke_syscall+0x5c/0x130
> [ 271.955442] el0_svc_common.constprop.0+0x108/0x130
> [ 271.956435] do_el0_svc+0x74/0x90
> [ 271.957124] el0_svc+0x2c/0x90
> [ 271.957757] el0t_64_sync_handler+0xa8/0x12c
> [ 271.958629] el0t_64_sync+0x1a0/0x1a4
that's interesting. On s390 i'm seeing the same problem in CI, but with
the startup ftrace tests. So that's likely not arm64 spacific.
On s390, the last messages from ftrace are [ 5.663568] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[ 5.667099] futex hash table entries: 65536 (order: 12, 16777216 bytes, vmalloc)
[ 5.739549] Running postponed tracer tests:
[ 5.740662] Testing tracer function: PASSED
[ 6.194635] Testing dynamic ftrace: PASSED
[ 6.471213] Testing dynamic ftrace ops #1:
[ 6.558445] (1 0 1 0 0)
[ 6.558458] (1 1 2 0 0)
[ 6.699135] (2 1 3 0 764347)
[ 6.699252] (2 2 4 0 766466)
[ 6.759857] (3 2 4 0 1159604)
[..] hangs here
The backtrace looks like this, which is very similar to the one above:
crash> bt 1
PID: 1 TASK: 80e68100 CPU: 133 COMMAND: "swapper/0"
#0 [380004df808] __schedule at cda39f0e
#1 [380004df880] schedule at cda3a488
#2 [380004df8b0] schedule_timeout at cda41ef6
#3 [380004df978] wait_for_completion at cda3bd0a
#4 [380004df9d8] __wait_rcu_gp at ccdddd92
#5 [380004dfa30] synchronize_rcu_tasks_generic at ccdde0aa
#6 [380004dfad8] ftrace_shutdown at cce7b050
#7 [380004dfb18] unregister_ftrace_function at cce7b192
#8 [380004dfb50] trace_selftest_ops at cda1e0fa
#9 [380004dfba0] run_tracer_selftest at cda1e4f2
#10 [380004dfc00] trace_selftest_startup_function at ce74355c
#11 [380004dfc58] run_tracer_selftest at cda1e2fc
#12 [380004dfc98] init_trace_selftests at ce742d30
#13 [380004dfcd0] do_one_initcall at cccdca16
#14 [380004dfd68] do_initcalls at ce72e776
#15 [380004dfde0] kernel_init_freeable at ce72ea60
#16 [380004dfe50] kernel_init at cda333fe
#17 [380004dfe68] __ret_from_fork at cccdf920
#18 [380004dfe98] ret_from_fork at cda444ca
I didn't had success reproducing it so far, but it is good to know that
this also happens when running the ftrace testsuite.
I have several crashdumps, so i could try to pull out some information
if someone tells me what to look for.
Thanks,
Sven
^ permalink raw reply
* [powerpc:fixes] BUILD SUCCESS 8defc2a5dd8f4c0cb19ecbaca8d3e89ab98524da
From: kernel test robot @ 2022-01-28 2:31 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes
branch HEAD: 8defc2a5dd8f4c0cb19ecbaca8d3e89ab98524da powerpc/64s/interrupt: Fix decrementer storm
elapsed time: 739m
configs tested: 152
configs skipped: 147
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
i386 randconfig-c001-20220124
powerpc randconfig-c003-20220124
powerpc mpc834x_itx_defconfig
arm at91_dt_defconfig
mips mpc30x_defconfig
xtensa defconfig
mips vocore2_defconfig
arc axs103_smp_defconfig
arm imx_v6_v7_defconfig
m68k m5275evb_defconfig
um i386_defconfig
h8300 h8300h-sim_defconfig
nios2 10m50_defconfig
xtensa audio_kc705_defconfig
powerpc ppc40x_defconfig
powerpc mpc837x_mds_defconfig
s390 zfcpdump_defconfig
arm pleb_defconfig
sparc sparc64_defconfig
sh allmodconfig
powerpc storcenter_defconfig
arm keystone_defconfig
sh sh7710voipgw_defconfig
sh kfr2r09-romimage_defconfig
parisc generic-32bit_defconfig
arm viper_defconfig
sh ecovec24_defconfig
riscv allyesconfig
powerpc rainier_defconfig
mips maltaup_xpa_defconfig
arm omap2plus_defconfig
arm h5000_defconfig
powerpc tqm8541_defconfig
mips xway_defconfig
powerpc ppc6xx_defconfig
arc alldefconfig
arm sunxi_defconfig
arc nsimosci_defconfig
arm lpc18xx_defconfig
i386 defconfig
arm randconfig-c002-20220127
arm randconfig-c002-20220124
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 debian-10.3-kselftests
i386 debian-10.3
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a002-20220124
i386 randconfig-a005-20220124
i386 randconfig-a003-20220124
i386 randconfig-a004-20220124
i386 randconfig-a001-20220124
i386 randconfig-a006-20220124
x86_64 randconfig-a002-20220124
x86_64 randconfig-a003-20220124
x86_64 randconfig-a001-20220124
x86_64 randconfig-a004-20220124
x86_64 randconfig-a005-20220124
x86_64 randconfig-a006-20220124
riscv nommu_k210_defconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
x86_64 allyesconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-func
x86_64 kexec
clang tested configs:
arm randconfig-c002-20220124
riscv randconfig-c006-20220124
i386 randconfig-c001-20220124
powerpc randconfig-c003-20220124
mips randconfig-c004-20220124
x86_64 randconfig-c007-20220124
x86_64 randconfig-c007
arm randconfig-c002-20220125
riscv randconfig-c006-20220125
powerpc randconfig-c003-20220125
mips randconfig-c004-20220125
i386 randconfig-c001
powerpc bluestone_defconfig
mips ip27_defconfig
powerpc ppc44x_defconfig
arm davinci_all_defconfig
arm milbeaut_m10v_defconfig
powerpc allmodconfig
arm defconfig
mips cu1830-neo_defconfig
mips malta_qemu_32r6_defconfig
powerpc mpc832x_mds_defconfig
powerpc katmai_defconfig
arm bcm2835_defconfig
powerpc ebony_defconfig
x86_64 randconfig-a005
x86_64 randconfig-a003
x86_64 randconfig-a001
i386 randconfig-a002
i386 randconfig-a006
i386 randconfig-a004
x86_64 randconfig-a011-20220124
x86_64 randconfig-a013-20220124
x86_64 randconfig-a015-20220124
x86_64 randconfig-a016-20220124
x86_64 randconfig-a014-20220124
x86_64 randconfig-a012-20220124
i386 randconfig-a011-20220124
i386 randconfig-a016-20220124
i386 randconfig-a013-20220124
i386 randconfig-a014-20220124
i386 randconfig-a015-20220124
i386 randconfig-a012-20220124
riscv randconfig-r042-20220126
riscv randconfig-r042-20220124
hexagon randconfig-r045-20220124
hexagon randconfig-r045-20220127
hexagon randconfig-r045-20220126
hexagon randconfig-r041-20220124
hexagon randconfig-r041-20220127
hexagon randconfig-r041-20220126
hexagon randconfig-r045-20220125
hexagon randconfig-r041-20220125
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v2 1/2] PCI/AER: Disable AER service when link is in L2/L3 ready, L2 and L3 state
From: Lu Baolu @ 2022-01-28 2:53 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: Joerg Roedel, Lalithambika Krishnakumar, linuxppc-dev, linux-pci,
linux-kernel, koba.ko, Oliver O'Halloran, bhelgaas,
mika.westerberg, baolu.lu
In-Reply-To: <CAAd53p6+KPAJchh9Jx59Fkkj5FidSxsW0yHjLqooFjvu-Y9u7w@mail.gmail.com>
On 1/27/22 7:14 PM, Kai-Heng Feng wrote:
> On Thu, Jan 27, 2022 at 3:01 PM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>
>> On 2022/1/27 10:54, Kai-Heng Feng wrote:
>>> Commit 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in
>>> hint") enables ACS, and some platforms lose its NVMe after resume from
>>> S3:
>>> [ 50.947816] pcieport 0000:00:1b.0: DPC: containment event, status:0x1f01 source:0x0000
>>> [ 50.947817] pcieport 0000:00:1b.0: DPC: unmasked uncorrectable error detected
>>> [ 50.947829] pcieport 0000:00:1b.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Receiver ID)
>>> [ 50.947830] pcieport 0000:00:1b.0: device [8086:06ac] error status/mask=00200000/00010000
>>> [ 50.947831] pcieport 0000:00:1b.0: [21] ACSViol (First)
>>> [ 50.947841] pcieport 0000:00:1b.0: AER: broadcast error_detected message
>>> [ 50.947843] nvme nvme0: frozen state error detected, reset controller
>>>
>>> It happens right after ACS gets enabled during resume.
>>>
>>> There's another case, when Thunderbolt reaches D3cold:
>>> [ 30.100211] pcieport 0000:00:1d.0: AER: Uncorrected (Non-Fatal) error received: 0000:00:1d.0
>>> [ 30.100251] pcieport 0000:00:1d.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID)
>>> [ 30.100256] pcieport 0000:00:1d.0: device [8086:7ab0] error status/mask=00100000/00004000
>>> [ 30.100262] pcieport 0000:00:1d.0: [20] UnsupReq (First)
>>> [ 30.100267] pcieport 0000:00:1d.0: AER: TLP Header: 34000000 08000052 00000000 00000000
>>> [ 30.100372] thunderbolt 0000:0a:00.0: AER: can't recover (no error_detected callback)
>>> [ 30.100401] xhci_hcd 0000:3e:00.0: AER: can't recover (no error_detected callback)
>>> [ 30.100427] pcieport 0000:00:1d.0: AER: device recovery failed
>>>
>>> So disable AER service to avoid the noises from turning power rails
>>> on/off when the device is in low power states (D3hot and D3cold), as
>>> PCIe spec "5.2 Link State Power Management" states that TLP and DLLP
>>> transmission is disabled for a Link in L2/L3 Ready (D3hot), L2 (D3cold
>>> with aux power) and L3 (D3cold).
>>>
>>> Bugzilla:https://bugzilla.kernel.org/show_bug.cgi?id=209149
>>> Bugzilla:https://bugzilla.kernel.org/show_bug.cgi?id=215453
>>> Fixes: 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint")
>>
>> I don't know what this fix has to do with the commit 50310600ebda.
>
> Commit 50310600ebda only exposed the underlying issue. Do you think
> "Fixes:" tag should change to other commits?
>
>> Commit 50310600ebda only makes sure that PCI ACS is enabled whenever
>> Intel IOMMU is on. Before this commit, PCI ACS could also be enabled
>> and result in the same problem. Or anything I missed?
>
> The system in question didn't enable ACS before commit 50310600ebda.
This commit exposed the issue on your configuration doesn't mean the
fix should be back ported as far as that commit. I believe if you add
intel-iommu=on in the kernel parameter, the issue still exists even you
revert commit 50310600ebda or checkout a tag before it.
Best regards,
baolu
^ permalink raw reply
* Re: [PATCH v2 1/2] PCI/AER: Disable AER service when link is in L2/L3 ready, L2 and L3 state
From: Kai-Heng Feng @ 2022-01-28 3:29 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Lalithambika Krishnakumar, linuxppc-dev, linux-pci,
linux-kernel, koba.ko, Oliver O'Halloran, bhelgaas,
mika.westerberg
In-Reply-To: <11891652-40c6-f111-46b7-e96d1729815e@linux.intel.com>
On Fri, Jan 28, 2022 at 10:57 AM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> On 1/27/22 7:14 PM, Kai-Heng Feng wrote:
> > On Thu, Jan 27, 2022 at 3:01 PM Lu Baolu <baolu.lu@linux.intel.com> wrote:
> >>
> >> On 2022/1/27 10:54, Kai-Heng Feng wrote:
> >>> Commit 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in
> >>> hint") enables ACS, and some platforms lose its NVMe after resume from
> >>> S3:
> >>> [ 50.947816] pcieport 0000:00:1b.0: DPC: containment event, status:0x1f01 source:0x0000
> >>> [ 50.947817] pcieport 0000:00:1b.0: DPC: unmasked uncorrectable error detected
> >>> [ 50.947829] pcieport 0000:00:1b.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Receiver ID)
> >>> [ 50.947830] pcieport 0000:00:1b.0: device [8086:06ac] error status/mask=00200000/00010000
> >>> [ 50.947831] pcieport 0000:00:1b.0: [21] ACSViol (First)
> >>> [ 50.947841] pcieport 0000:00:1b.0: AER: broadcast error_detected message
> >>> [ 50.947843] nvme nvme0: frozen state error detected, reset controller
> >>>
> >>> It happens right after ACS gets enabled during resume.
> >>>
> >>> There's another case, when Thunderbolt reaches D3cold:
> >>> [ 30.100211] pcieport 0000:00:1d.0: AER: Uncorrected (Non-Fatal) error received: 0000:00:1d.0
> >>> [ 30.100251] pcieport 0000:00:1d.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID)
> >>> [ 30.100256] pcieport 0000:00:1d.0: device [8086:7ab0] error status/mask=00100000/00004000
> >>> [ 30.100262] pcieport 0000:00:1d.0: [20] UnsupReq (First)
> >>> [ 30.100267] pcieport 0000:00:1d.0: AER: TLP Header: 34000000 08000052 00000000 00000000
> >>> [ 30.100372] thunderbolt 0000:0a:00.0: AER: can't recover (no error_detected callback)
> >>> [ 30.100401] xhci_hcd 0000:3e:00.0: AER: can't recover (no error_detected callback)
> >>> [ 30.100427] pcieport 0000:00:1d.0: AER: device recovery failed
> >>>
> >>> So disable AER service to avoid the noises from turning power rails
> >>> on/off when the device is in low power states (D3hot and D3cold), as
> >>> PCIe spec "5.2 Link State Power Management" states that TLP and DLLP
> >>> transmission is disabled for a Link in L2/L3 Ready (D3hot), L2 (D3cold
> >>> with aux power) and L3 (D3cold).
> >>>
> >>> Bugzilla:https://bugzilla.kernel.org/show_bug.cgi?id=209149
> >>> Bugzilla:https://bugzilla.kernel.org/show_bug.cgi?id=215453
> >>> Fixes: 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint")
> >>
> >> I don't know what this fix has to do with the commit 50310600ebda.
> >
> > Commit 50310600ebda only exposed the underlying issue. Do you think
> > "Fixes:" tag should change to other commits?
> >
> >> Commit 50310600ebda only makes sure that PCI ACS is enabled whenever
> >> Intel IOMMU is on. Before this commit, PCI ACS could also be enabled
> >> and result in the same problem. Or anything I missed?
> >
> > The system in question didn't enable ACS before commit 50310600ebda.
>
> This commit exposed the issue on your configuration doesn't mean the
> fix should be back ported as far as that commit. I believe if you add
> intel-iommu=on in the kernel parameter, the issue still exists even you
> revert commit 50310600ebda or checkout a tag before it.
That's true.
I guess it's better to drop the "Fixes:" tag.
Bjorn, should I send another version of it?
Kai-Heng
>
> Best regards,
> baolu
^ permalink raw reply
* Re: [PATCH V3 03/17] asm-generic: compat: Cleanup duplicate definitions
From: Guo Ren @ 2022-01-28 7:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-s390, Guo Ren, gregkh, Drew Fustini, Anup Patel,
Wang Junqiang, the arch/x86 maintainers,
Linux Kernel Mailing List, linux-csky, inux-parisc,
Christoph Hellwig, Palmer Dabbelt, liush, sparclinux, linux-riscv,
open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Christoph Hellwig,
Linux ARM, Wei Fu
In-Reply-To: <CAK8P3a1UmnjHk8B6hSULiKv3FKoY5BW9=4=ESerQzc+4=LR5Zw@mail.gmail.com>
On Thu, Jan 20, 2022 at 9:02 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Jan 20, 2022 at 8:38 AM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > There are 7 64bit architectures that support Linux COMPAT mode to
> > run 32bit applications. A lot of definitions are duplicate:
> > - COMPAT_USER_HZ
> > - COMPAT_RLIM_INFINITY
> > - COMPAT_OFF_T_MAX
> > - __compat_uid_t, __compat_uid_t
> > - compat_dev_t
> > - compat_ipc_pid_t
> > - struct compat_flock
> > - struct compat_flock64
> > - struct compat_statfs
> > - struct compat_ipc64_perm, compat_semid64_ds,
> > compat_msqid64_ds, compat_shmid64_ds
> >
> > Cleanup duplicate definitions and merge them into asm-generic.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
>
> > ---
> > arch/arm64/include/asm/compat.h | 108 +++-----------------------
> > arch/mips/include/asm/compat.h | 24 ++----
> > arch/parisc/include/asm/compat.h | 47 ++----------
> > arch/powerpc/include/asm/compat.h | 47 ++----------
> > arch/s390/include/asm/compat.h | 109 +++-----------------------
> > arch/sparc/include/asm/compat.h | 39 ++++------
> > arch/x86/include/asm/compat.h | 114 +++-------------------------
> > include/asm-generic/compat.h | 122 ++++++++++++++++++++++++++++++
> > 8 files changed, 191 insertions(+), 419 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
> > index eaa6ca062d89..f54f295efae3 100644
> > --- a/arch/arm64/include/asm/compat.h
> > +++ b/arch/arm64/include/asm/compat.h
> > @@ -5,9 +5,18 @@
> > #ifndef __ASM_COMPAT_H
> > #define __ASM_COMPAT_H
> >
> > +#define COMPAT_RLIM_INFINITY 0xffffffff
> ...
> > +#ifndef COMPAT_RLIM_INFINITY
> > +#define COMPAT_RLIM_INFINITY 0x7fffffff
> > +#endif
>
> While this is a correct conversion, I think the default should
> be 0xffffffff, to match the asm-generic RLIM_INFINITY
> definition, with only mips and sparc getting the exception
Okay
>
> > -struct compat_flock {
> > - short l_type;
> > - short l_whence;
> > - compat_off_t l_start;
> > - compat_off_t l_len;
> > - compat_pid_t l_pid;
> > -};
> ...
> > +#ifndef compat_flock
> > +struct compat_flock {
> > + compat_short_t l_type;
> > + compat_short_t l_whence;
> > + compat_off_t l_start;
> > + compat_off_t l_len;
> > + compat_pid_t l_pid;
> > +} __attribute__((packed));
> > +#endif
>
> You are adding __attribute__((packed)) here, which I think has
> no effect on the layout on the structure on any of the architectures
> but it does change the alignment requirements needlessly.
>
> Better leave it without the attribute.
Okay
>
> > -struct compat_flock64 {
> > - short l_type;
> > - short l_whence;
> > - compat_loff_t l_start;
> > - compat_loff_t l_len;
> > - compat_pid_t l_pid;
> > -};
> ...
> > +#ifndef compat_flock64
> > +struct compat_flock64 {
> > + compat_short_t l_type;
> > + compat_short_t l_whence;
> > + compat_loff_t l_start;
> > + compat_loff_t l_len;
> > + compat_pid_t l_pid;
> > +} __attribute__((packed));
> > +#endif
>
> This one is different: on all architectures other than x86,
> the added packed attribute changes the size of the
> structure by removing the four padding bytes at the
> end. x86 originally added the attribute here to work around
> the weirdness of the x86-32 ABI that aligns 64-bit values
> on a 4-byte boundary.
>
> The easiest workaround would be to have x86 keep its
> custom definition. A slightly nicer version would drop the
> attribute on x86 as well but instead change the compat_loff_t
> definition to use compat_s64 instead of s64, giving it the
> correct alignment.
Okay, I would leave x86 origin first.
>
> > -struct compat_statfs {
> > - int f_type;
> > - int f_bsize;
> > - int f_blocks;
> > - int f_bfree;
> > - int f_bavail;
> > - int f_files;
> > - int f_ffree;
> > - compat_fsid_t f_fsid;
> > - int f_namelen; /* SunOS ignores this field. */
> > - int f_frsize;
> > - int f_flags;
> > - int f_spare[4];
> > -};
> ...
> > +#ifndef compat_statfs
> > +struct compat_statfs {
> > + compat_uint_t f_type;
> > + compat_uint_t f_bsize;
> > + compat_uint_t f_blocks;
> > + compat_uint_t f_bfree;
> > + compat_uint_t f_bavail;
> > + compat_uint_t f_files;
> > + compat_uint_t f_ffree;
> > + __kernel_fsid_t f_fsid;
> > + compat_uint_t f_namelen;
> > + compat_uint_t f_frsize;
> > + compat_uint_t f_flags;
> > + compat_uint_t f_spare[4];
> > +} __attribute__((packed));
> > +#endif
>
> None of the architectures use the packed attribute at the moment,
> so please don't add one here.
>
> Changing compat_fsid_t to __kernel_fsid_t is harmless, but seems
> unnecessary.
Okay. I would add another
typedef __kernel_fsid_t compat_fsid_t;
in the file.
>
> Changing the signed int to an unsigned int (regardless of notation)
> may be a change in behavior. s390 is the only architecture
> using unsigned members here at the moment, as of b8668fd0a7e1
> ("s390/uapi: change struct statfs[64] member types to unsigned
> values").
> The description of that patch sounds like this was changed to fix
> a bug, but I don't see what the actual problem would be in the
> put_compat_statfs().
>
> For the moment I'd suggest leaving this with the signed version,
> with s390 being another exception next to mips. We can follow-up
> with merging s390 into the common definition using either the
> signed or unsigned types, but I think that needs to be a separate
> patch with a detailed explanation.
Okay, I would leave s390 origin first.
>
> +#ifndef compat_ipc64_perm
> > +struct compat_ipc64_perm {
> > + compat_key_t key;
> > + __compat_uid32_t uid;
> > + __compat_gid32_t gid;
> > + __compat_uid32_t cuid;
> > + __compat_gid32_t cgid;
> > + compat_mode_t mode;
> > + unsigned char __pad1[4 - sizeof(compat_mode_t)];
> > + compat_ushort_t seq;
> > + compat_ushort_t __pad2;
> > + compat_ulong_t unused1;
> > + compat_ulong_t unused2;
> > +} __attribute__((packed));
> > +
> > +struct compat_semid64_ds {
> > + struct compat_ipc64_perm sem_perm;
> > + compat_ulong_t sem_otime;
> > + compat_ulong_t sem_otime_high;
> > + compat_ulong_t sem_ctime;
> > + compat_ulong_t sem_ctime_high;
> > + compat_ulong_t sem_nsems;
> > + compat_ulong_t __unused3;
> > + compat_ulong_t __unused4;
> > +} __attribute__((packed));
> > +
> > +struct compat_msqid64_ds {
> > + struct compat_ipc64_perm msg_perm;
> > + compat_ulong_t msg_stime;
> > + compat_ulong_t msg_stime_high;
> > + compat_ulong_t msg_rtime;
> > + compat_ulong_t msg_rtime_high;
> > + compat_ulong_t msg_ctime;
> > + compat_ulong_t msg_ctime_high;
> > + compat_ulong_t msg_cbytes;
> > + compat_ulong_t msg_qnum;
> > + compat_ulong_t msg_qbytes;
> > + compat_pid_t msg_lspid;
> > + compat_pid_t msg_lrpid;
> > + compat_ulong_t __unused4;
> > + compat_ulong_t __unused5;
> > +} __attribute__((packed));
> > +
> > +struct compat_shmid64_ds {
> > + struct compat_ipc64_perm shm_perm;
> > + compat_size_t shm_segsz;
> > + compat_ulong_t shm_atime;
> > + compat_ulong_t shm_atime_high;
> > + compat_ulong_t shm_dtime;
> > + compat_ulong_t shm_dtime_high;
> > + compat_ulong_t shm_ctime;
> > + compat_ulong_t shm_ctime_high;
> > + compat_pid_t shm_cpid;
> > + compat_pid_t shm_lpid;
> > + compat_ulong_t shm_nattch;
> > + compat_ulong_t __unused4;
> > + compat_ulong_t __unused5;
> > +} __attribute__((packed));
> > +#endif
>
> I checked these in detail, looking at the seven architectures, and your
> conversion looks exactly right (I had initially missed the part about
> compat_mode_t that you got right).
>
> As with compat_flock, the packed attribute has no impact on the layout
> here, but please drop it anyway for consistency.
>
> Arnd
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply
* powerpc: Set crashkernel offset to mid of RMA region
From: Sourabh Jain @ 2022-01-28 10:04 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: mahesh, hbathini, Abdul haleem
On large config LPARs (having 192 and more cores), Linux fails to boot
due to insufficient memory in the first memblock. It is due to the
memory reservation for the crash kernel which starts at 128MB offset of
the first memblock. This memory reservation for the crash kernel doesn't
leave enough space in the first memblock to accommodate other essential
system resources.
The crash kernel start address was set to 128MB offset by default to
ensure that the crash kernel get some memory below the RMA region which
is used to be of size 256MB. But given that the RMA region size can be
512MB or more, setting the crash kernel offset to mid of RMA size will
leave enough space for kernel to allocate memory for other system
resources.
Since the above crash kernel offset change is only applicable to the LPAR
platform, the LPAR feature detection is pushed before the crash kernel
reservation. The rest of LPAR specific initialization will still
be done during pseries_probe_fw_features as usual.
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reported-and-tested-by: Abdul haleem <abdhalee@linux.vnet.ibm.com>
---
arch/powerpc/kernel/rtas.c | 4 ++++
arch/powerpc/kexec/core.c | 15 +++++++++++----
2 files changed, 15 insertions(+), 4 deletions(-)
---
Change in v3:
Dropped 1st and 2nd patch from v2. 1st and 2nd patch from v2 patch
series [1] try to discover 1T segment MMU feature support
BEFORE boot CPU paca allocation ([1] describes why it is needed).
MPE has posted a patch [2] that archives a similar objective by moving
boot CPU paca allocation after mmu_early_init_devtree().
NOTE: This patch is dependent on the patch [2].
[1] https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20211018084434.217772-3-sourabhjain@linux.ibm.com/
[2] https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239175.html
---
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 733e6ef36758..06df7464fb57 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1313,6 +1313,10 @@ int __init early_init_dt_scan_rtas(unsigned long node,
entryp = of_get_flat_dt_prop(node, "linux,rtas-entry", NULL);
sizep = of_get_flat_dt_prop(node, "rtas-size", NULL);
+ /* need this feature to decide the crashkernel offset */
+ if (of_get_flat_dt_prop(node, "ibm,hypertas-functions", NULL))
+ powerpc_firmware_features |= FW_FEATURE_LPAR;
+
if (basep && entryp && sizep) {
rtas.base = *basep;
rtas.entry = *entryp;
diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c
index 8b68d9f91a03..abf5897ae88c 100644
--- a/arch/powerpc/kexec/core.c
+++ b/arch/powerpc/kexec/core.c
@@ -134,11 +134,18 @@ void __init reserve_crashkernel(void)
if (!crashk_res.start) {
#ifdef CONFIG_PPC64
/*
- * On 64bit we split the RMO in half but cap it at half of
- * a small SLB (128MB) since the crash kernel needs to place
- * itself and some stacks to be in the first segment.
+ * On the LPAR platform place the crash kernel to mid of
+ * RMA size (512MB or more) to ensure the crash kernel
+ * gets enough space to place itself and some stack to be
+ * in the first segment. At the same time normal kernel
+ * also get enough space to allocate memory for essential
+ * system resource in the first segment. Keep the crash
+ * kernel starts at 128MB offset on other platforms.
*/
- crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ crashk_res.start = ppc64_rma_size / 2;
+ else
+ crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
#else
crashk_res.start = KDUMP_KERNELBASE;
#endif
--
2.34.1
^ permalink raw reply related
* [PATCHv2] powerpc: mm: radix_tlb: rearrange the if-else block
From: Anders Roxell @ 2022-01-28 10:08 UTC (permalink / raw)
To: nathan, ndesaulniers, mpe
Cc: Anders Roxell, Arnd Bergmann, llvm, linux-kernel, linuxppc-dev
Clang warns:
arch/powerpc/mm/book3s64/radix_tlb.c:1191:23: error: variable 'hstart' is uninitialized when used here [-Werror,-Wuninitialized]
__tlbiel_va_range(hstart, hend, pid,
^~~~~~
arch/powerpc/mm/book3s64/radix_tlb.c:1175:23: note: initialize the variable 'hstart' to silence this warning
unsigned long hstart, hend;
^
= 0
arch/powerpc/mm/book3s64/radix_tlb.c:1191:31: error: variable 'hend' is uninitialized when used here [-Werror,-Wuninitialized]
__tlbiel_va_range(hstart, hend, pid,
^~~~
arch/powerpc/mm/book3s64/radix_tlb.c:1175:29: note: initialize the variable 'hend' to silence this warning
unsigned long hstart, hend;
^
= 0
2 errors generated.
Rework the 'if (IS_ENABLE(CONFIG_TRANSPARENT_HUGEPAGE))' so hstart/hend
always gets initialized, this will silence the warnings. That will also
simplify the 'else' path. Clang is getting confused with these warnings,
but the warnings is a false-positive.
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
arch/powerpc/mm/book3s64/radix_tlb.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
index 7724af19ed7e..7d65965a0688 100644
--- a/arch/powerpc/mm/book3s64/radix_tlb.c
+++ b/arch/powerpc/mm/book3s64/radix_tlb.c
@@ -1174,12 +1174,9 @@ static inline void __radix__flush_tlb_range(struct mm_struct *mm,
bool hflush = false;
unsigned long hstart, hend;
- if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
- hstart = (start + PMD_SIZE - 1) & PMD_MASK;
- hend = end & PMD_MASK;
- if (hstart < hend)
- hflush = true;
- }
+ hstart = (start + PMD_SIZE - 1) & PMD_MASK;
+ hend = end & PMD_MASK;
+ hflush = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && hstart < hend;
if (type == FLUSH_TYPE_LOCAL) {
asm volatile("ptesync": : :"memory");
--
2.34.1
^ permalink raw reply related
* Re: [PATCHv2] powerpc: mm: radix_tlb: rearrange the if-else block
From: Christophe Leroy @ 2022-01-28 10:14 UTC (permalink / raw)
To: Anders Roxell, nathan@kernel.org, ndesaulniers@google.com,
mpe@ellerman.id.au
Cc: llvm@lists.linux.dev, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, Arnd Bergmann
In-Reply-To: <20220128100815.3231551-1-anders.roxell@linaro.org>
Le 28/01/2022 à 11:08, Anders Roxell a écrit :
> Clang warns:
>
> arch/powerpc/mm/book3s64/radix_tlb.c:1191:23: error: variable 'hstart' is uninitialized when used here [-Werror,-Wuninitialized]
> __tlbiel_va_range(hstart, hend, pid,
> ^~~~~~
> arch/powerpc/mm/book3s64/radix_tlb.c:1175:23: note: initialize the variable 'hstart' to silence this warning
> unsigned long hstart, hend;
> ^
> = 0
> arch/powerpc/mm/book3s64/radix_tlb.c:1191:31: error: variable 'hend' is uninitialized when used here [-Werror,-Wuninitialized]
> __tlbiel_va_range(hstart, hend, pid,
> ^~~~
> arch/powerpc/mm/book3s64/radix_tlb.c:1175:29: note: initialize the variable 'hend' to silence this warning
> unsigned long hstart, hend;
> ^
> = 0
> 2 errors generated.
>
> Rework the 'if (IS_ENABLE(CONFIG_TRANSPARENT_HUGEPAGE))' so hstart/hend
> always gets initialized, this will silence the warnings. That will also
> simplify the 'else' path. Clang is getting confused with these warnings,
> but the warnings is a false-positive.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
> arch/powerpc/mm/book3s64/radix_tlb.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 7724af19ed7e..7d65965a0688 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -1174,12 +1174,9 @@ static inline void __radix__flush_tlb_range(struct mm_struct *mm,
> bool hflush = false;
You should then remove the default initialisation of hflush to false
which has become pointless.
With that fixed,
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> unsigned long hstart, hend;
>
> - if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
> - hstart = (start + PMD_SIZE - 1) & PMD_MASK;
> - hend = end & PMD_MASK;
> - if (hstart < hend)
> - hflush = true;
> - }
> + hstart = (start + PMD_SIZE - 1) & PMD_MASK;
> + hend = end & PMD_MASK;
> + hflush = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && hstart < hend;
>
> if (type == FLUSH_TYPE_LOCAL) {
> asm volatile("ptesync": : :"memory");
^ permalink raw reply
* [PATCH V3 1/2] mm/migration: Add trace events for THP migrations
From: Anshuman Khandual @ 2022-01-28 11:09 UTC (permalink / raw)
To: linux-mm, akpm
Cc: Anshuman Khandual, John Hubbard, linux-kernel, Steven Rostedt,
Ingo Molnar, Paul Mackerras, Matthew Wilcox, Zi Yan,
Naoya Horiguchi, linuxppc-dev
In-Reply-To: <1643368182-9588-1-git-send-email-anshuman.khandual@arm.com>
This adds two trace events for PMD based THP migration without split. These
events closely follow the implementation details like setting and removing
of PMD migration entries, which are essential operations for THP migration.
This moves CREATE_TRACE_POINTS into generic THP from powerpc for these new
trace events to be available on other platforms as well.
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/powerpc/mm/book3s64/trace.c | 1 -
include/trace/events/thp.h | 27 +++++++++++++++++++++++++++
mm/huge_memory.c | 5 +++++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/book3s64/trace.c b/arch/powerpc/mm/book3s64/trace.c
index b86e7b906257..ccd64b5e6cac 100644
--- a/arch/powerpc/mm/book3s64/trace.c
+++ b/arch/powerpc/mm/book3s64/trace.c
@@ -3,6 +3,5 @@
* This file is for defining trace points and trace related helpers.
*/
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-#define CREATE_TRACE_POINTS
#include <trace/events/thp.h>
#endif
diff --git a/include/trace/events/thp.h b/include/trace/events/thp.h
index ca3f2767828a..202b3e3e67ff 100644
--- a/include/trace/events/thp.h
+++ b/include/trace/events/thp.h
@@ -48,6 +48,33 @@ TRACE_EVENT(hugepage_update,
TP_printk("hugepage update at addr 0x%lx and pte = 0x%lx clr = 0x%lx, set = 0x%lx", __entry->addr, __entry->pte, __entry->clr, __entry->set)
);
+DECLARE_EVENT_CLASS(migration_pmd,
+
+ TP_PROTO(unsigned long addr, unsigned long pmd),
+
+ TP_ARGS(addr, pmd),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, addr)
+ __field(unsigned long, pmd)
+ ),
+
+ TP_fast_assign(
+ __entry->addr = addr;
+ __entry->pmd = pmd;
+ ),
+ TP_printk("addr=%lx, pmd=%lx", __entry->addr, __entry->pmd)
+);
+
+DEFINE_EVENT(migration_pmd, set_migration_pmd,
+ TP_PROTO(unsigned long addr, unsigned long pmd),
+ TP_ARGS(addr, pmd)
+);
+
+DEFINE_EVENT(migration_pmd, remove_migration_pmd,
+ TP_PROTO(unsigned long addr, unsigned long pmd),
+ TP_ARGS(addr, pmd)
+);
#endif /* _TRACE_THP_H */
/* This part must be outside protection */
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 406a3c28c026..ab49f9a3e420 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -39,6 +39,9 @@
#include <asm/pgalloc.h>
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/thp.h>
+
/*
* By default, transparent hugepage support is disabled in order to avoid
* risking an increased memory footprint for applications that are not
@@ -3173,6 +3176,7 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
set_pmd_at(mm, address, pvmw->pmd, pmdswp);
page_remove_rmap(page, true);
put_page(page);
+ trace_set_migration_pmd(address, pmd_val(pmdswp));
}
void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)
@@ -3206,5 +3210,6 @@ void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new)
if ((vma->vm_flags & VM_LOCKED) && !PageDoubleMap(new))
mlock_vma_page(new);
update_mmu_cache_pmd(vma, address, pvmw->pmd);
+ trace_remove_migration_pmd(address, pmd_val(pmde));
}
#endif
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 0/2] powerpc: Disable syscall emulation and stepping
From: Naveen N. Rao @ 2022-01-28 11:11 UTC (permalink / raw)
To: Christophe Leroy, Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <d352c741-baaf-3be3-ef31-81ce6250876c@csgroup.eu>
[Sorry if you receive this in duplicate. Resending since this message
didn't hit the list]
On 2022-01-25 11:23, Christophe Leroy wrote:
> Le 25/01/2022 à 04:04, Nicholas Piggin a écrit :
>> +Naveen (sorry missed cc'ing you at first)
>>
>> Excerpts from Christophe Leroy's message of January 24, 2022 4:39 pm:
>>>
>>>
>>> Le 24/01/2022 à 06:57, Nicholas Piggin a écrit :
>>>> As discussed previously
>>>>
>>>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/238946.html
>>>>
>>>> I'm wondering whether PPC32 should be returning -1 for syscall
>>>> instructions too here? That could be done in another patch anyway.
>>>>
>>>
>>> The 'Programming Environments Manual for 32-Bit Implementations of
>>> the
>>> PowerPC™ Architecture' says:
>>>
>>> The following are not traced:
>>> • rfi instruction
>>> • sc and trap instructions that trap
>>> • Other instructions that cause interrupts (other than trace
>>> interrupts)
>>> • The first instruction of any interrupt handler
>>> • Instructions that are emulated by software
>>>
>>>
>>> So I think PPC32 should return -1 as well.
>>
>> I agree.
>>
>> What about the trap instructions? analyse_instr returns 0 for them
>> which falls through to return 0 for emulate_step, should they
>> return -1 as well or am I missing something?
Yeah, good point about the trap instructions.
>>
>
> For the traps I don't know. The manual says "trap instructions that
> trap" are not traced. It means that "trap instructions that _don't_
> trap" are traced. Taking into account that trap instructions don't trap
> at least 99.9% of the time, not sure if returning -1 is needed.
>
> Allthought that'd probably be the safest.
'trap' is a special case since it is predominantly used by debuggers
and/or tracing infrastructure. Kprobes and Uprobes do not allow probes
on a trap instruction. But, xmon can be asked to step on a trap
instruction and that can interfere with kprobes in weird ways.
So, I think it is best if we also exclude trap instructions from being
single stepped.
>
> But then what happens with other instruction that will sparsely
> generate
> an exception like a DSI or so ? If we do it for the traps then we
> should
> do it for this as well, and then it becomes a non ending story.
For a DSI, we restart the same instruction after handling the page
fault.
The single step exception is raised on the subsequent successful
completion of the instruction. For most other interrupts (alignment, vsx
unavailable, ...), we end up emulating the single step exception itself
(see emulate_single_step()). So, those are ok if caused by an
instruction
being stepped.
- Naveen
^ permalink raw reply
* Re: [PATCH 0/2] powerpc: Disable syscall emulation and stepping
From: Naveen N. Rao @ 2022-01-28 11:15 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <1643268944.tltfmyu1ey.astroid@bobo.none>
On 2022-01-27 13:09, Nicholas Piggin wrote:
> Excerpts from naverao1's message of January 25, 2022 8:48 pm:
>> On 2022-01-25 11:23, Christophe Leroy wrote:
>>> Le 25/01/2022 à 04:04, Nicholas Piggin a écrit :
>>>> +Naveen (sorry missed cc'ing you at first)
>>>>
>>>> Excerpts from Christophe Leroy's message of January 24, 2022 4:39
>>>> pm:
>>>>>
>>>>>
>>>>> Le 24/01/2022 à 06:57, Nicholas Piggin a écrit :
>>>>>> As discussed previously
>>>>>>
>>>>>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/238946.html
>>>>>>
>>>>>> I'm wondering whether PPC32 should be returning -1 for syscall
>>>>>> instructions too here? That could be done in another patch anyway.
>>>>>>
>>>>>
>>>>> The 'Programming Environments Manual for 32-Bit Implementations of
>>>>> the
>>>>> PowerPC™ Architecture' says:
>>>>>
>>>>> The following are not traced:
>>>>> • rfi instruction
>>>>> • sc and trap instructions that trap
>>>>> • Other instructions that cause interrupts (other than trace
>>>>> interrupts)
>>>>> • The first instruction of any interrupt handler
>>>>> • Instructions that are emulated by software
>>>>>
>>>>>
>>>>> So I think PPC32 should return -1 as well.
>>>>
>>>> I agree.
>>>>
>>>> What about the trap instructions? analyse_instr returns 0 for them
>>>> which falls through to return 0 for emulate_step, should they
>>>> return -1 as well or am I missing something?
>>
>> Yeah, good point about the trap instructions.
>>
>>>>
>>>
>>> For the traps I don't know. The manual says "trap instructions that
>>> trap" are not traced. It means that "trap instructions that _don't_
>>> trap" are traced. Taking into account that trap instructions don't
>>> trap
>>> at least 99.9% of the time, not sure if returning -1 is needed.
>>>
>>> Allthought that'd probably be the safest.
>>
>> 'trap' is a special case since it is predominantly used by debuggers
>> and/or tracing infrastructure. Kprobes and Uprobes do not allow probes
>> on a trap instruction. But, xmon can be asked to step on a trap
>> instruction and that can interfere with kprobes in weird ways.
>>
>> So, I think it is best if we also exclude trap instructions from being
>> single stepped.
>>
>>>
>>> But then what happens with other instruction that will sparsely
>>> generate
>>> an exception like a DSI or so ? If we do it for the traps then we
>>> should
>>> do it for this as well, and then it becomes a non ending story.
>>
>> For a DSI, we restart the same instruction after handling the page
>> fault.
>> The single step exception is raised on the subsequent successful
>> completion of the instruction.
>
> Although it can cause a signal, and the signal handler can decide
> to resume somewhere else.
If a signal is generated while we are single-stepping, we delay signal
delivery (see uprobe_deny_signal()) until after the single stepping.
For fatal signals, single stepping is disabled before we allow the
signal to be delivered.
> Or kernel mode equivalent it can go to a
> fixup handler and resume somewhere else.
For kprobes, we do not allow probing instructions that have an extable
entry.
- Naveen
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/uprobes: Reject uprobe on a system call instruction
From: Naveen N. Rao @ 2022-01-28 11:30 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <1643269209.jj1krtc1vx.astroid@bobo.none>
On 2022-01-27 13:14, Nicholas Piggin wrote:
> Excerpts from Michael Ellerman's message of January 25, 2022 9:45 pm:
>> Nicholas Piggin <npiggin@gmail.com> writes:
>>> Per the ISA, a Trace interrupt is not generated for a system call
>>> [vectored] instruction. Reject uprobes on such instructions as we are
>>> not emulating a system call [vectored] instruction anymore.
>>
>> This should really be patch 1, otherwise there's a single commit
>> window
>> where we allow uprobes on sc but don't honour them.
>
> Yep true. I also messed up Naveen's attribution! Will re-send (or maybe
> Naveen would take over the series).
Yes, let me come up with a better, more complete patch for this.
>
>>
>>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>>> [np: Switch to pr_info_ratelimited]
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>> arch/powerpc/include/asm/ppc-opcode.h | 1 +
>>> arch/powerpc/kernel/uprobes.c | 6 ++++++
>>> 2 files changed, 7 insertions(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>>> b/arch/powerpc/include/asm/ppc-opcode.h
>>> index 9675303b724e..8bbe16ce5173 100644
>>> --- a/arch/powerpc/include/asm/ppc-opcode.h
>>> +++ b/arch/powerpc/include/asm/ppc-opcode.h
>>> @@ -411,6 +411,7 @@
>>> #define PPC_RAW_DCBFPS(a, b) (0x7c0000ac | ___PPC_RA(a) |
>>> ___PPC_RB(b) | (4 << 21))
>>> #define PPC_RAW_DCBSTPS(a, b) (0x7c0000ac | ___PPC_RA(a) |
>>> ___PPC_RB(b) | (6 << 21))
>>> #define PPC_RAW_SC() (0x44000002)
>>> +#define PPC_RAW_SCV() (0x44000001)
>>> #define PPC_RAW_SYNC() (0x7c0004ac)
>>> #define PPC_RAW_ISYNC() (0x4c00012c)
>>>
>>> diff --git a/arch/powerpc/kernel/uprobes.c
>>> b/arch/powerpc/kernel/uprobes.c
>>> index c6975467d9ff..3779fde804bd 100644
>>> --- a/arch/powerpc/kernel/uprobes.c
>>> +++ b/arch/powerpc/kernel/uprobes.c
>>> @@ -41,6 +41,12 @@ int arch_uprobe_analyze_insn(struct arch_uprobe
>>> *auprobe,
>>> if (addr & 0x03)
>>> return -EINVAL;
>>>
>>> + if (ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SC() ||
>>> + ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SCV()) {
>>
>> We should probably reject hypercall too?
>>
>> There's also a lot of reserved fields in `sc`, so doing an exact match
>> like this risks missing instructions that are badly formed but the CPU
>> will happily execute as `sc`.
>
> Yeah, scv as well has lev != 0 unsupported so should be excluded.
>>
>> We'd obviously never expect to see those in compiler generated code,
>> but
>> it'd still be safer to mask. We could probably just reject opcode 17
>> entirely.
Indeed, thanks.
>>
>> And I guess for a subsequent patch, but we should be rejecting some
>> others here as well shouldn't we? Like rfid etc.
>
> Traps under discussion I guess. For uprobe, rfid will be just another
> privilege fault. Is that dealt with somehow or do all privileged and
> illegal instructions also need to be excluded from stepping? (I assume
> we must handle that in a general way somehow)
Yes, this is all handled in our interrupt code if we emulate any of
those
privileged instructions. Otherwise, if a signal is generated, that would
be caught by uprobe_deny_signal().
Thanks,
Naveen
^ permalink raw reply
* Re: [PATCHv2] powerpc: mm: radix_tlb: rearrange the if-else block
From: Anders Roxell @ 2022-01-28 13:07 UTC (permalink / raw)
To: Christophe Leroy
Cc: Arnd Bergmann, llvm@lists.linux.dev, ndesaulniers@google.com,
linux-kernel@vger.kernel.org, nathan@kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <c6beb8a8-ecf3-1c17-f685-f5e69cd6265a@csgroup.eu>
On Fri, 28 Jan 2022 at 11:14, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 28/01/2022 à 11:08, Anders Roxell a écrit :
> > Clang warns:
> >
> > arch/powerpc/mm/book3s64/radix_tlb.c:1191:23: error: variable 'hstart' is uninitialized when used here [-Werror,-Wuninitialized]
> > __tlbiel_va_range(hstart, hend, pid,
> > ^~~~~~
> > arch/powerpc/mm/book3s64/radix_tlb.c:1175:23: note: initialize the variable 'hstart' to silence this warning
> > unsigned long hstart, hend;
> > ^
> > = 0
> > arch/powerpc/mm/book3s64/radix_tlb.c:1191:31: error: variable 'hend' is uninitialized when used here [-Werror,-Wuninitialized]
> > __tlbiel_va_range(hstart, hend, pid,
> > ^~~~
> > arch/powerpc/mm/book3s64/radix_tlb.c:1175:29: note: initialize the variable 'hend' to silence this warning
> > unsigned long hstart, hend;
> > ^
> > = 0
> > 2 errors generated.
> >
> > Rework the 'if (IS_ENABLE(CONFIG_TRANSPARENT_HUGEPAGE))' so hstart/hend
> > always gets initialized, this will silence the warnings. That will also
> > simplify the 'else' path. Clang is getting confused with these warnings,
> > but the warnings is a false-positive.
> >
> > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > Suggested-by: Nathan Chancellor <nathan@kernel.org>
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > ---
> > arch/powerpc/mm/book3s64/radix_tlb.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> > index 7724af19ed7e..7d65965a0688 100644
> > --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> > +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> > @@ -1174,12 +1174,9 @@ static inline void __radix__flush_tlb_range(struct mm_struct *mm,
> > bool hflush = false;
>
> You should then remove the default initialisation of hflush to false
> which has become pointless.
>
> With that fixed,
>
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Thank you for the review.
I will send a v3 shortly with that fixed.
Cheers,
Anders
^ permalink raw reply
* [PATCHv3] powerpc: mm: radix_tlb: rearrange the if-else block
From: Anders Roxell @ 2022-01-28 13:17 UTC (permalink / raw)
To: nathan, ndesaulniers, mpe
Cc: Anders Roxell, Arnd Bergmann, llvm, linux-kernel, linuxppc-dev
Clang warns:
arch/powerpc/mm/book3s64/radix_tlb.c:1191:23: error: variable 'hstart' is uninitialized when used here [-Werror,-Wuninitialized]
__tlbiel_va_range(hstart, hend, pid,
^~~~~~
arch/powerpc/mm/book3s64/radix_tlb.c:1175:23: note: initialize the variable 'hstart' to silence this warning
unsigned long hstart, hend;
^
= 0
arch/powerpc/mm/book3s64/radix_tlb.c:1191:31: error: variable 'hend' is uninitialized when used here [-Werror,-Wuninitialized]
__tlbiel_va_range(hstart, hend, pid,
^~~~
arch/powerpc/mm/book3s64/radix_tlb.c:1175:29: note: initialize the variable 'hend' to silence this warning
unsigned long hstart, hend;
^
= 0
2 errors generated.
Rework the 'if (IS_ENABLE(CONFIG_TRANSPARENT_HUGEPAGE))' so hstart/hend
always gets initialized, this will silence the warnings. That will also
simplify the 'else' path. Clang is getting confused with these warnings,
but the warnings is a false-positive.
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
arch/powerpc/mm/book3s64/radix_tlb.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
index 7724af19ed7e..5172d5cec2c0 100644
--- a/arch/powerpc/mm/book3s64/radix_tlb.c
+++ b/arch/powerpc/mm/book3s64/radix_tlb.c
@@ -1171,15 +1171,12 @@ static inline void __radix__flush_tlb_range(struct mm_struct *mm,
}
}
} else {
- bool hflush = false;
+ bool hflush;
unsigned long hstart, hend;
- if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
- hstart = (start + PMD_SIZE - 1) & PMD_MASK;
- hend = end & PMD_MASK;
- if (hstart < hend)
- hflush = true;
- }
+ hstart = (start + PMD_SIZE - 1) & PMD_MASK;
+ hend = end & PMD_MASK;
+ hflush = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && hstart < hend;
if (type == FLUSH_TYPE_LOCAL) {
asm volatile("ptesync": : :"memory");
--
2.34.1
^ permalink raw reply related
* Re: [BUG] mtd: cfi_cmdset_0002: write regression since v4.17-rc1
From: Ahmad Fatoum @ 2022-01-28 12:55 UTC (permalink / raw)
To: Tokunori Ikegami, Thorsten Leemhuis, linux-mtd, Joakim.Tjernlund,
miquel.raynal, vigneshr, richard, regressions@lists.linux.dev
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, marek.vasut,
Chris Packham, Pengutronix Kernel Team, cyrille.pitchen,
Shaohui.Xie, Brian Norris, David Woodhouse
In-Reply-To: <e11b76dc-5539-fb7e-da1c-a5005713d6b0@gmail.com>
Hello Tokunori-san,
On 15.12.21 18:34, Tokunori Ikegami wrote:
> Hi Ahmad-san,
Thanks for your reply (and Thorsten for the reminder) and sorry for
the delay. I had a lot of backlog after my time off.
> Sorry for the regression issue by the change: dfeae1073583.
> To make sure could you please try with the word write instead of the buffered writes?
The issue is still there with #define FORCE_WORD_WRITE 1:
jffs2: Write clean marker to block at 0x000a0000 failed: -5
MTD do_write_oneword_once(): software timeout
> FYI: There are some changes to disable the buffered writes as below.
> 1. https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/ar71xx/patches-4.9/411-mtd-cfi_cmdset_0002-force-word-write.patch;h=ddd69f17e1ac16e8fc3a694c56231fee1e2ef149;hb=fec8fe806963c96a6506c2aebc3572d3a11f285f
> 2. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mtd/chips/cfi_cmdset_0002.c?h=v5.16-rc5&id=7e4404113686868858a34210c28ae122e967aa64
>
> Note:
> Currently I am not able to investigate the issue on the product for the change before.
>
> By the way in the past I had investigated the similar issue on Buffalo WZR-HP-G300NH using the S29GL256N.
> It was not able to find the root cause by the investigation since not required actually at that time.
> Also actually the buffered writes were disabled on the OpenWrt firmware as the change [1] above.
> But I am not sure the reason detail to disable the buffered writes on the OpenWrt firmware.
> I thought the issue not caused by the change: dfeae1073583 since the issue happened without the change.
>
> So I am not sure why the above change [2] needed to disable the buffered writes on Buffalo WZR-HP-G300NH.
> Probably seems needed to disable the buffered writes on the other firmware also but not OpenWrt firmware.
>
> Anyway there are difference with your regression issue as below.
> 1. Flash device: S29GL064N (Your regression issue), S29GL256N (WZR-HP-G300NH)
> 2. Regression issue: Yes (Your regression issue), No (WZR-HP-G300NH as I investigated before)
Doesn't seem to be a buffered write issue here though as the writes
did work fine before dfeae1073583. Any other ideas?
Cheers,
Ahmad
>
> Regards,
> Ikegami
>
> On 2021/12/14 16:23, Thorsten Leemhuis wrote:
>> [TLDR: adding this regression to regzbot; most of this mail is compiled
>> from a few templates paragraphs some of you might have seen already.]
>>
>> Hi, this is your Linux kernel regression tracker speaking.
>>
>> Top-posting for once, to make this easy accessible to everyone.
>>
>> Thanks for the report.
>>
>> Adding the regression mailing list to the list of recipients, as it
>> should be in the loop for all regressions, as explained here:
>> https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html
>>
>> To be sure this issue doesn't fall through the cracks unnoticed, I'm
>> adding it to regzbot, my Linux kernel regression tracking bot:
>>
>> #regzbot ^introduced dfeae1073583
>> #regzbot title mtd: cfi_cmdset_0002: flash write accesses on the
>> hardware fail on a PowerPC MPC8313 to a 8-bit-parallel S29GL064N flash
>> #regzbot ignore-activity
>>
>> Reminder: when fixing the issue, please add a 'Link:' tag with the URL
>> to the report (the parent of this mail), then regzbot will automatically
>> mark the regression as resolved once the fix lands in the appropriate
>> tree. For more details about regzbot see footer.
>>
>> Sending this to everyone that got the initial report, to make all aware
>> of the tracking. I also hope that messages like this motivate people to
>> directly get at least the regression mailing list and ideally even
>> regzbot involved when dealing with regressions, as messages like this
>> wouldn't be needed then.
>>
>> Don't worry, I'll send further messages wrt to this regression just to
>> the lists (with a tag in the subject so people can filter them away), as
>> long as they are intended just for regzbot. With a bit of luck no such
>> messages will be needed anyway.
>>
>> Ciao, Thorsten (wearing his 'Linux kernel regression tracker' hat).
>>
>> P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
>> on my table. I can only look briefly into most of them. Unfortunately
>> therefore I sometimes will get things wrong or miss something important.
>> I hope that's not the case here; if you think it is, don't hesitate to
>> tell me about it in a public reply. That's in everyone's interest, as
>> what I wrote above might be misleading to everyone reading this; any
>> suggestion I gave thus might sent someone reading this down the wrong
>> rabbit hole, which none of us wants.
>>
>> BTW, I have no personal interest in this issue, which is tracked using
>> regzbot, my Linux kernel regression tracking bot
>> (https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
>> this mail to get things rolling again and hence don't need to be CC on
>> all further activities wrt to this regression.
>>
>> On 13.12.21 14:24, Ahmad Fatoum wrote:
>>> Hi,
>>>
>>> I've been investigating a breakage on a PowerPC MPC8313: The SoC is connected
>>> via the "Enhanced Local Bus Controller" to a 8-bit-parallel S29GL064N flash,
>>> which is represented as a memory-mapped cfi-flash.
>>>
>>> The regression began in v4.17-rc1 with
>>>
>>> dfeae1073583 ("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
>>>
>>> and causes all flash write accesses on the hardware to fail. Example output
>>> after v5.1-rc2[1]:
>>>
>>> root@host:~# mount -t jffs2 /dev/mtdblock0 /mnt
>>> MTD do_write_buffer_wait(): software timeout, address:0x000c000b.
>>> jffs2: Write clean marker to block at 0x000c0000 failed: -5
>>>
>>> This issue still persists with v5.16-rc. Reverting aforementioned patch fixes
>>> it, but I am still looking for a change that keeps both Tokunori's and my
>>> hardware happy.
>>>
>>> What Tokunori's patch did is that it strengthened the success condition
>>> for flash writes:
>>>
>>> - Prior to the patch, DQ polling was done until bits
>>> stopped toggling. This was taken as an indicator that the write succeeded
>>> and was reported up the stack. i.e. success condition is chip_ready()
>>>
>>> - After the patch, polling continues until the just written data is
>>> actually read back, i.e. success condition is chip_good()
>>>
>>> This new condition never holds for me, when DQ stabilizes, it reads 0xFF,
>>> never the just written data. The data is still written and can be read back
>>> on subsequent reads, just not at that point of time in the poll loop.
>>>
>>> We haven't had write issues for the years predating that patch. As the
>>> regression has been mainline for a while, I am wondering what about my setup
>>> that makes it pop up here, but not elsewhere?
>>>
>>> I consulted the data sheet[2] and found Figure 27, which describes DQ polling
>>> during embedded algorithms. DQ switches from status output to "True" (I assume
>>> True == all bits set == 0xFF) until CS# is reasserted.
>>>
>>> I compared with another chip's datasheet, and it (Figure 8.4) doesn't describe
>>> such an intermittent "True" state. In any case, the driver polls a few hundred
>>> times, however, before giving up, so there should be enough CS# toggles.
>>>
>>>
>>> Locally, I'll revert this patch for now. I think accepting 0xFF as a success
>>> condition may be appropriate, but I don't yet have the rationale to back it up.
>>>
>>> I am investigating this some more, probably with a logic trace, but I wanted
>>> to report this in case someone has pointers and in case other people run into
>>> the same issue.
>>>
>>>
>>> Cheers,
>>> Ahmad
>>>
>>> [1] Prior to d9b8a67b3b95 ("mtd: cfi: fix deadloop in cfi_cmdset_0002.c do_write_buffer")
>>> first included with v5.1-rc2, failing writes just hung indefinitely in kernel space.
>>> That's fixed, but the writes still fail.
>>>
>>> [2]: 001-98525 Rev. *B, https://www.infineon.com/dgdl/Infineon-S29GL064N_S29GL032N_64_Mbit_32_Mbit_3_V_Page_Mode_MirrorBit_Flash-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed556fd548b
>>>
>>> [3]: https://www.mouser.com/datasheet/2/268/SST39VF1601C-SST39VF1602C-16-Mbit-x16-Multi-Purpos-709008.pdf
>>> Note that "true data" means valid data here, not all bits one.
>>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v2 4/5] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Daniel Thompson @ 2022-01-28 14:09 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
Jason Wessel, linux-kernel@vger.kernel.org, Douglas Anderson,
linux-mm@kvack.org, Luis Chamberlain, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <af8519537d2a5c36b71a2f48ba9b81c07c93a5c4.1643282353.git.christophe.leroy@csgroup.eu>
On Thu, Jan 27, 2022 at 11:28:09AM +0000, Christophe Leroy wrote:
> Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC to allow architectures
> to request having modules data in vmalloc area instead of module area.
>
> This is required on powerpc book3s/32 in order to set data non
> executable, because it is not possible to set executability on page
> basis, this is done per 256 Mbytes segments. The module area has exec
> right, vmalloc area has noexec.
>
> This can also be useful on other powerpc/32 in order to maximize the
> chance of code being close enough to kernel core to avoid branch
> trampolines.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Jason Wessel <jason.wessel@windriver.com>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Douglas Anderson <dianders@chromium.org>
Thanks for diligence in making sure kdb is up to date!
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
^ 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