* Re: [PATCH v3 3/6] modules: Introduce data_layout
From: Christophe Leroy @ 2022-02-03 6:58 UTC (permalink / raw)
To: Luis Chamberlain
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <YfsYaDyqrFyVypkv@bombadil.infradead.org>
Le 03/02/2022 à 00:48, Luis Chamberlain a écrit :
> On Sat, Jan 29, 2022 at 05:02:07PM +0000, Christophe Leroy wrote:
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 163e32e39064..11f51e17fb9f 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -81,6 +81,8 @@
>> /* If this is set, the section belongs in the init part of the module */
>> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
>>
>> +#define data_layout core_layout
>> +
>> /*
>> * Mutex protects:
>> * 1) List of modules (also safely readable with preempt_disable),
>> @@ -2451,7 +2454,10 @@ static void layout_sections(struct module *mod, struct load_info *info)
>> || s->sh_entsize != ~0UL
>> || module_init_layout_section(sname))
>> continue;
>> - s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
>> + if (m)
>> + s->sh_entsize = get_offset(mod, &mod->data_layout.size, s, i);
>> + else
>> + s->sh_entsize = get_offset(mod, &mod->core_layout.size, s, i);
>> pr_debug("\t%s\n", sname);
>
> Huh why is this branching here, given you just used mod->data_layout in
> all other areas?
The module text remains in core_layout, so the text section still needs
core_layout. In the masks[][] table, it corresponds to the first line,
which has flag SHF_EXECINSTR. In the loop that's when 'm' is 0.
In the following switch/case, case 0 still uses core_layout.
>
>> @@ -3468,6 +3474,8 @@ static int move_module(struct module *mod, struct load_info *info)
>> if (shdr->sh_entsize & INIT_OFFSET_MASK)
>> dest = mod->init_layout.base
>> + (shdr->sh_entsize & ~INIT_OFFSET_MASK);
>> + else if (!(shdr->sh_flags & SHF_EXECINSTR))
>> + dest = mod->data_layout.base + shdr->sh_entsize;
>> else
>> dest = mod->core_layout.base + shdr->sh_entsize;
>>
>
> Likewise here.
Same here, the section with flag SHF_EXECINSTR is a text section, it
stays in core_layout.
Christophe
^ permalink raw reply
* Re: [PATCH v3 4/6] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Christophe Leroy @ 2022-02-03 7:05 UTC (permalink / raw)
To: Luis Chamberlain, Aaron Tomlin
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, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <YfsbcXD74BwJ9ci2@bombadil.infradead.org>
Le 03/02/2022 à 01:01, Luis Chamberlain a écrit :
> On Sat, Jan 29, 2022 at 05:02:09PM +0000, Christophe Leroy wrote:
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 11f51e17fb9f..f3758115ebaa 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -81,7 +81,9 @@
>> /* If this is set, the section belongs in the init part of the module */
>> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
>>
>> +#ifndef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
>> #define data_layout core_layout
>> +#endif
>>
>> /*
>> * Mutex protects:
>> @@ -111,6 +113,12 @@ static struct mod_tree_root {
>> #define module_addr_min mod_tree.addr_min
>> #define module_addr_max mod_tree.addr_max
>>
>> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
>> +static struct mod_tree_root mod_data_tree __cacheline_aligned = {
>> + .addr_min = -1UL,
>> +};
>> +#endif
>> +
>> #ifdef CONFIG_MODULES_TREE_LOOKUP
>>
>> /*
>> @@ -186,6 +194,11 @@ static void mod_tree_insert(struct module *mod)
>> __mod_tree_insert(&mod->core_layout.mtn, &mod_tree);
>> if (mod->init_layout.size)
>> __mod_tree_insert(&mod->init_layout.mtn, &mod_tree);
>> +
>> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
>> + mod->data_layout.mtn.mod = mod;
>> + __mod_tree_insert(&mod->data_layout.mtn, &mod_data_tree);
>> +#endif
>
>
> kernel/ directory has quite a few files, module.c is the second to
> largest file, and it has tons of stuff. Aaron is doing work to
> split things out to make code easier to read and so that its easier
> to review changes. See:
>
> https://lkml.kernel.org/r/20220130213214.1042497-1-atomlin@redhat.com
>
> I think this is a good patch example which could benefit from that work.
> So I'd much prefer to see that work go in first than this, so to see if
> we can make the below changes more compartamentalized.
>
> Curious, how much testing has been put into this series?
I tested the change up to (including) patch 4 to verify it doesn't
introduce regression when not using
CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC,
Then I tested with patch 5. I first tried with the 'hello world' test
module. After that I loaded several important modules and checked I
didn't get any regression, both with and without STRICT_MODULES_RWX and
I checked the consistency in /proc/vmallocinfo
/proc/modules /sys/class/modules/*
I also tested with a hacked module_alloc() to force branch trampolines.
Christophe
^ permalink raw reply
* Re: [PATCH v2 5/5] powerpc: Select ARCH_WANTS_MODULES_DATA_IN_VMALLOC on book3s/32 and 8xx
From: Christophe Leroy @ 2022-02-03 7:07 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Luis Chamberlain, Paul Mackerras, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <87h79gmrux.fsf@mpe.ellerman.id.au>
Le 03/02/2022 à 06:39, Michael Ellerman a écrit :
> Luis Chamberlain <mcgrof@kernel.org> writes:
>> On Thu, Jan 27, 2022 at 11:28:12AM +0000, Christophe Leroy wrote:
>>> book3s/32 and 8xx have a separate area for allocating modules,
>>> defined by MODULES_VADDR / MODULES_END.
>>>
>>> On book3s/32, it is not possible to protect against execution
>>> on a page basis. A full 256M segment is either Exec or NoExec.
>>> The module area is in an Exec segment while vmalloc area is
>>> in a NoExec segment.
>>>
>>> In order to protect module data against execution, select
>>> ARCH_WANTS_MODULES_DATA_IN_VMALLOC.
>>>
>>> For the 8xx (and possibly other 32 bits platform in the future),
>>> there is no such constraint on Exec/NoExec protection, however
>>> there is a critical distance between kernel functions and callers
>>> that needs to remain below 32Mbytes in order to avoid costly
>>> trampolines. By allocating data outside of module area, we
>>> increase the chance for module text to remain within acceptable
>>> distance from kernel core text.
>>>
>>> So select ARCH_WANTS_MODULES_DATA_IN_VMALLOC for 8xx as well.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>
>> Cc list first and then the SOB.
>
> Just delete the Cc: list, it's meaningless.
>
Was an easy way to copy you automatically with 'git send-email', but
getting it through linuxppc-dev list is enough I guess ?
Christophe
^ permalink raw reply
* Re: [PATCH v5 3/6] kexec_file: Don't opencode appended signature verification.
From: Michal Suchánek @ 2022-02-03 10:49 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Nayna, Mimi Zohar, Sven Schnelle, David Howells, keyrings,
Paul Mackerras, Alexander Gordeev, Rob Herring, Herbert Xu,
Baoquan He, Christian Borntraeger, James Morris,
Lakshmi Ramasubramanian, Christian Borntraeger, Serge E. Hallyn,
Vasily Gorbik, linux-s390, Heiko Carstens, Dmitry Kasatkin,
Hari Bathini, Daniel Axtens, Philipp Rudo, Frank van der Linden,
kexec, linux-kernel, linux-security-module, linux-crypto,
Jessica Yu, linux-integrity, linuxppc-dev, David S. Miller,
Thiago Jung Bauermann, buendgen
In-Reply-To: <YfBafIXgnLzf0QMb@bombadil.infradead.org>
Hello,
thanks for the review.
On Tue, Jan 25, 2022 at 12:15:56PM -0800, Luis Chamberlain wrote:
> On Tue, Jan 11, 2022 at 12:37:45PM +0100, Michal Suchanek wrote:
> > diff --git a/include/linux/verification.h b/include/linux/verification.h
> > index a655923335ae..32db9287a7b0 100644
> > --- a/include/linux/verification.h
> > +++ b/include/linux/verification.h
> > @@ -60,5 +60,8 @@ extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
> > enum key_being_used_for usage);
> > #endif
> >
> > +int verify_appended_signature(const void *data, unsigned long *len,
> > + struct key *trusted_keys, const char *what);
> > +
>
> Looks very non-module specific.
Which it is now that the same signature format is used for kernels.
>
> > diff --git a/kernel/module_signing.c b/kernel/module_signing.c
> > index 8723ae70ea1f..30149969f21f 100644
> > --- a/kernel/module_signing.c
> > +++ b/kernel/module_signing.c
> > @@ -14,32 +14,38 @@
> > #include <crypto/public_key.h>
> > #include "module-internal.h"
> >
> > -/*
> > - * Verify the signature on a module.
> > +/**
> > + * verify_appended_signature - Verify the signature on a module with the
> > + * signature marker stripped.
> > + * @data: The data to be verified
> > + * @len: Size of @data.
> > + * @trusted_keys: Keyring to use for verification
> > + * @what: Informational string for log messages
> > */
> > -int mod_verify_sig(const void *mod, struct load_info *info)
> > +int verify_appended_signature(const void *data, unsigned long *len,
> > + struct key *trusted_keys, const char *what)
> > {
> > - struct module_signature ms;
> > - size_t sig_len, modlen = info->len;
> > + struct module_signature *ms;
>
> There goes the abstraction, so why not make this clear where we re-use
> the struct module_signature for various things and call it as it is,
> verify_mod_appended_signature() or some such?
It sounds like the abstraction is actually improved by callers no longer
dealing with struct module_signature when verifying signature on a
kernel. That is the structure is misnamed but it is now hidden behind
an abstraction.
Or am I missing something?
Thanks
Michal
^ permalink raw reply
* [PATCH v1] drivers/base/node: consolidate node device subsystem initialization in node_dev_init()
From: David Hildenbrand @ 2022-02-03 10:52 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Hocko, linux-ia64, David Hildenbrand, Dave Hansen, x86,
linux-mm, Rich Felker, Paul Mackerras, sparclinux, linux-riscv,
Will Deacon, linux-s390, Yoshinori Sato, linux-sh,
Rafael J. Wysocki, Anatoly Pugachev, Ingo Molnar, Catalin Marinas,
Albert Ou, Vasily Gorbik, Heiko Carstens, Borislav Petkov,
Paul Walmsley, Thomas Gleixner, linux-arm-kernel, Oscar Salvador,
Thomas Bogendoerfer, Greg Kroah-Hartman, linux-mips,
Palmer Dabbelt, Andrew Morton, linuxppc-dev, David S. Miller,
Mike Rapoport
... and call node_dev_init() after memory_dev_init() from driver_init(),
so before any of the existing arch/subsys calls. All online nodes should
be known at that point: early during boot, arch code determines node and
zone ranges and sets the relevant nodes online; usually this happens in
setup_arch().
This is in line with memory_dev_init(), which initializes the memory
device subsystem and creates all memory block devices.
Similar to memory_dev_init(), panic() if anything goes wrong, we don't
want to continue with such basic initialization errors.
The important part is that node_dev_init() gets called after
memory_dev_init() and after cpu_dev_init(), but before any of the
relevant archs call register_cpu() to register the new cpu device under
the node device. The latter should be the case for the current users
of topology_init().
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Tested-by: Anatoly Pugachev <matorola@gmail.com> (sparc64)
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-mm@kvack.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
RFC v1 -> v1:
* Extended patch description slighly
* Tested on x86-64, s390x, ppc64 and aarch64
* Added Rb and Tested-by.
---
arch/arm64/kernel/setup.c | 3 ---
arch/ia64/kernel/topology.c | 10 ----------
arch/mips/kernel/topology.c | 5 -----
arch/powerpc/kernel/sysfs.c | 17 -----------------
arch/riscv/kernel/setup.c | 3 ---
arch/s390/kernel/numa.c | 7 -------
arch/sh/kernel/topology.c | 5 -----
arch/sparc/kernel/sysfs.c | 12 ------------
arch/x86/kernel/topology.c | 5 -----
drivers/base/init.c | 1 +
drivers/base/node.c | 30 +++++++++++++++++-------------
include/linux/node.h | 4 ++++
12 files changed, 22 insertions(+), 80 deletions(-)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index f70573928f1b..3505789cf4bd 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -406,9 +406,6 @@ static int __init topology_init(void)
{
int i;
- for_each_online_node(i)
- register_one_node(i);
-
for_each_possible_cpu(i) {
struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
cpu->hotpluggable = cpu_can_disable(i);
diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index e4992917a24b..94a848b06f15 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -70,16 +70,6 @@ static int __init topology_init(void)
{
int i, err = 0;
-#ifdef CONFIG_NUMA
- /*
- * MCD - Do we want to register all ONLINE nodes, or all POSSIBLE nodes?
- */
- for_each_online_node(i) {
- if ((err = register_one_node(i)))
- goto out;
- }
-#endif
-
sysfs_cpus = kcalloc(NR_CPUS, sizeof(struct ia64_cpu), GFP_KERNEL);
if (!sysfs_cpus)
panic("kzalloc in topology_init failed - NR_CPUS too big?");
diff --git a/arch/mips/kernel/topology.c b/arch/mips/kernel/topology.c
index 08ad6371fbe0..9429d85a4703 100644
--- a/arch/mips/kernel/topology.c
+++ b/arch/mips/kernel/topology.c
@@ -12,11 +12,6 @@ static int __init topology_init(void)
{
int i, ret;
-#ifdef CONFIG_NUMA
- for_each_online_node(i)
- register_one_node(i);
-#endif /* CONFIG_NUMA */
-
for_each_present_cpu(i) {
struct cpu *c = &per_cpu(cpu_devices, i);
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index d45a415d5374..2069bbb90a9a 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -1110,14 +1110,6 @@ EXPORT_SYMBOL_GPL(cpu_remove_dev_attr_group);
/* NUMA stuff */
#ifdef CONFIG_NUMA
-static void __init register_nodes(void)
-{
- int i;
-
- for (i = 0; i < MAX_NUMNODES; i++)
- register_one_node(i);
-}
-
int sysfs_add_device_to_node(struct device *dev, int nid)
{
struct node *node = node_devices[nid];
@@ -1132,13 +1124,6 @@ void sysfs_remove_device_from_node(struct device *dev, int nid)
sysfs_remove_link(&node->dev.kobj, kobject_name(&dev->kobj));
}
EXPORT_SYMBOL_GPL(sysfs_remove_device_from_node);
-
-#else
-static void __init register_nodes(void)
-{
- return;
-}
-
#endif
/* Only valid if CPU is present. */
@@ -1155,8 +1140,6 @@ static int __init topology_init(void)
{
int cpu, r;
- register_nodes();
-
for_each_possible_cpu(cpu) {
struct cpu *c = &per_cpu(cpu_devices, cpu);
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index b42bfdc67482..834eb652a7b9 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -301,9 +301,6 @@ static int __init topology_init(void)
{
int i, ret;
- for_each_online_node(i)
- register_one_node(i);
-
for_each_possible_cpu(i) {
struct cpu *cpu = &per_cpu(cpu_devices, i);
diff --git a/arch/s390/kernel/numa.c b/arch/s390/kernel/numa.c
index 51c5a9f6e525..23ab9f02f278 100644
--- a/arch/s390/kernel/numa.c
+++ b/arch/s390/kernel/numa.c
@@ -33,10 +33,3 @@ void __init numa_setup(void)
NODE_DATA(0)->node_spanned_pages = memblock_end_of_DRAM() >> PAGE_SHIFT;
NODE_DATA(0)->node_id = 0;
}
-
-static int __init numa_init_late(void)
-{
- register_one_node(0);
- return 0;
-}
-arch_initcall(numa_init_late);
diff --git a/arch/sh/kernel/topology.c b/arch/sh/kernel/topology.c
index 76af6db9daa2..2d2a7509b565 100644
--- a/arch/sh/kernel/topology.c
+++ b/arch/sh/kernel/topology.c
@@ -46,11 +46,6 @@ static int __init topology_init(void)
{
int i, ret;
-#ifdef CONFIG_NUMA
- for_each_online_node(i)
- register_one_node(i);
-#endif
-
for_each_present_cpu(i) {
struct cpu *c = &per_cpu(cpu_devices, i);
diff --git a/arch/sparc/kernel/sysfs.c b/arch/sparc/kernel/sysfs.c
index 6d60d416f0dd..f19487e4cc71 100644
--- a/arch/sparc/kernel/sysfs.c
+++ b/arch/sparc/kernel/sysfs.c
@@ -244,22 +244,10 @@ static void __init check_mmu_stats(void)
mmu_stats_supported = 1;
}
-static void register_nodes(void)
-{
-#ifdef CONFIG_NUMA
- int i;
-
- for (i = 0; i < MAX_NUMNODES; i++)
- register_one_node(i);
-#endif
-}
-
static int __init topology_init(void)
{
int cpu, ret;
- register_nodes();
-
check_mmu_stats();
for_each_possible_cpu(cpu) {
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index bd83748e2bde..8617d1ed9d31 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -154,11 +154,6 @@ static int __init topology_init(void)
{
int i;
-#ifdef CONFIG_NUMA
- for_each_online_node(i)
- register_one_node(i);
-#endif
-
for_each_present_cpu(i)
arch_register_cpu(i);
diff --git a/drivers/base/init.c b/drivers/base/init.c
index a9f57c22fb9e..d8d0fe687111 100644
--- a/drivers/base/init.c
+++ b/drivers/base/init.c
@@ -35,5 +35,6 @@ void __init driver_init(void)
auxiliary_bus_init();
cpu_dev_init();
memory_dev_init();
+ node_dev_init();
container_dev_init();
}
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 87acc47e8951..a133981a12fc 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -1065,26 +1065,30 @@ static const struct attribute_group *cpu_root_attr_groups[] = {
};
#define NODE_CALLBACK_PRI 2 /* lower than SLAB */
-static int __init register_node_type(void)
+void __init node_dev_init(void)
{
- int ret;
+ static struct notifier_block node_memory_callback_nb = {
+ .notifier_call = node_memory_callback,
+ .priority = NODE_CALLBACK_PRI,
+ };
+ int ret, i;
BUILD_BUG_ON(ARRAY_SIZE(node_state_attr) != NR_NODE_STATES);
BUILD_BUG_ON(ARRAY_SIZE(node_state_attrs)-1 != NR_NODE_STATES);
ret = subsys_system_register(&node_subsys, cpu_root_attr_groups);
- if (!ret) {
- static struct notifier_block node_memory_callback_nb = {
- .notifier_call = node_memory_callback,
- .priority = NODE_CALLBACK_PRI,
- };
- register_hotmemory_notifier(&node_memory_callback_nb);
- }
+ if (ret)
+ panic("%s() failed to register subsystem: %d\n", __func__, ret);
+
+ register_hotmemory_notifier(&node_memory_callback_nb);
/*
- * Note: we're not going to unregister the node class if we fail
- * to register the node state class attribute files.
+ * Create all node devices, which will properly link the node
+ * to applicable memory block devices and already created cpu devices.
*/
- return ret;
+ for_each_online_node(i) {
+ ret = register_one_node(i);
+ if (ret)
+ panic("%s() failed to add node: %d\n", __func__, ret);
+ }
}
-postcore_initcall(register_node_type);
diff --git a/include/linux/node.h b/include/linux/node.h
index bb21fd631b16..f3be6ccfebed 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -112,6 +112,7 @@ static inline void link_mem_sections(int nid, unsigned long start_pfn,
extern void unregister_node(struct node *node);
#ifdef CONFIG_NUMA
+extern void node_dev_init(void);
/* Core of the node registration - only memory hotplug should use this */
extern int __register_one_node(int nid);
@@ -149,6 +150,9 @@ extern void register_hugetlbfs_with_node(node_registration_func_t doregister,
node_registration_func_t unregister);
#endif
#else
+static inline void node_dev_init(void)
+{
+}
static inline int __register_one_node(int nid)
{
return 0;
base-commit: 26291c54e111ff6ba87a164d85d4a4e134b7315c
--
2.34.1
^ permalink raw reply related
* Re: powerpc: Set crashkernel offset to mid of RMA region
From: Michael Ellerman @ 2022-02-03 11:07 UTC (permalink / raw)
To: Sourabh Jain, linuxppc-dev; +Cc: mahesh, hbathini, Abdul haleem
In-Reply-To: <1c61a544-7ec9-5e5a-4b76-4725675cde7a@linux.ibm.com>
Sourabh Jain <sourabhjain@linux.ibm.com> writes:
> On 01/02/22 17:14, Michael Ellerman wrote:
>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>> 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;
>>> +
>> As you'd have seen this breaks the 32-bit build. It will need an #ifdef
>> CONFIG_PPC64 around it.
>>
>>> 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));
>> I think this will break on machines using Radix won't it? At this point
>> in boot ppc64_rma_size will be == 0. Because we won't call into
>> hash__setup_initial_memory_limit().
>>
>> That's not changed by your patch, but seems like this code needs to be
>> more careful/clever.
>
> Interesting, but in my testing, I found that ppc64_rma_size
> did get initialized before reserve_crashkernel() using radix on LPAR.
>
> I am not sure why but hash__setup_initial_memory_limit() function is
> gets called
> regardless of radix or hash. Not sure whether it is by design but here
> is the flow:
It sort of is by design. See:
103a8542cb35 ("powerpc/book3s64/radix: Fix boot failure with large amount of guest memory")
Basically the hash restrictions are more strict, so we apply them until
we know we will use radix.
But ...
> setup_initial_memory_limit()
>
> static inline void setup_initial_memory_limit()
> (arch/powerpc/include/asm/book3s/64/mmu.h)
>
> if (!early_radix_enabled()) // FALSE regardless of radix is enabled or not
You mean early_radix_enabled() is False regardless. But that's not true
in all cases.
We can now build the kernel without hash MMU support at all, see:
387e220a2e5e ("powerpc/64s: Move hash MMU support code under CONFIG_PPC_64S_HASH_MMU")
In which case early_radix_enabled() will be true here, because it's hard
coded to be true at build time.
> hash__setup_initial_memory_limit() // initialize ppc64_rma_size
>
> reserve_crashkernel() // initialize crashkernel offset to mid of RMA size.
>
>
> For the sack of understanding even if we restrict crashkernel offset
> setting to mid RMA (i.e. ppc64_rma_size/2) for
> only hash it may not save radix because even today we are assigning
> crashkernel offset using
> ppc64_rma_size variable.
Yes. There's already a bug there, your patch doesn't make it better or worse.
> Is the current flow of initializing ppc64_rma_size variable before
> reserve_crashkernel() for radix expected?
>
> Please provide your input.
I wonder if we're better off moving the crash kernel reservation later,
once we've discovered what MMU we're using.
I can't immediately see why that would be a problem, as long as we do
the reservation before we do any (many?) allocations. I'll have to think
about it a bit more though, these boot ordering things are always
subtle.
For now I think this patch is OK if you send a v2 to fix the compile
error.
cheers
^ permalink raw reply
* [Bug 215217] Kernel fails to boot at an early stage when built with GCC_PLUGIN_LATENT_ENTROPY=y (PowerMac G4 3,6)
From: bugzilla-daemon @ 2022-02-03 17:29 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-215217-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=215217
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |CODE_FIX
--- Comment #15 from Erhard F. (erhard_f@mailbox.org) ---
Fix landed in kernel 5.16.5. Thanks!
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
From: bugzilla-daemon @ 2022-02-03 17:33 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-210749-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210749
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |OBSOLETE
--- Comment #11 from Erhard F. (erhard_f@mailbox.org) ---
Have not seen this on kernels 5.15.x and 5.16.x.
Closing as obsolete.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Catalin Marinas @ 2022-02-03 17:51 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linuxppc-dev, linux-kernel, linux-mm, Paul Mackerras, sparclinux,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel,
Mike Kravetz
In-Reply-To: <1643780286-18798-1-git-send-email-anshuman.khandual@arm.com>
On Wed, Feb 02, 2022 at 11:08:06AM +0530, Anshuman Khandual wrote:
> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
> This updates generic fallback stub for arch_make_huge_pte() and available
> platforms definitions. This makes huge pte creation much cleaner and easier
> to follow.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
For arm64:
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* Re: [PATCH V2] mm: Merge pte_mkhuge() call into arch_make_huge_pte()
From: Catalin Marinas @ 2022-02-03 17:54 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linuxppc-dev, linux-kernel, linux-mm, Paul Mackerras, sparclinux,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel,
Mike Kravetz
In-Reply-To: <1643860669-26307-1-git-send-email-anshuman.khandual@arm.com>
On Thu, Feb 03, 2022 at 09:27:49AM +0530, Anshuman Khandual wrote:
> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
> This updates generic fallback stub for arch_make_huge_pte() and available
> platforms definitions. This makes huge pte creation much cleaner and easier
> to follow.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Mike Kravetz <mike.kravetz@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Acking v2 as well:
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* Re: [RFC V1 04/31] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT
From: Mike Rapoport @ 2022-02-03 18:15 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-kernel, hch, linux-mm, Paul Mackerras, akpm, linuxppc-dev
In-Reply-To: <1643029028-12710-5-git-send-email-anshuman.khandual@arm.com>
On Mon, Jan 24, 2022 at 06:26:41PM +0530, Anshuman Khandual wrote:
> This defines and exports a platform specific custom vm_get_page_prot() via
> subscribing ARCH_HAS_VM_GET_PAGE_PROT. Subsequently all __SXXX and __PXXX
> macros can be dropped which are no longer needed. While here, this also
> localizes arch_vm_get_page_prot() as powerpc_vm_get_page_prot().
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/mman.h | 3 +-
> arch/powerpc/include/asm/pgtable.h | 19 ------------
> arch/powerpc/mm/mmap.c | 47 ++++++++++++++++++++++++++++++
> 4 files changed, 49 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index b779603978e1..ddb4a3687c05 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -135,6 +135,7 @@ config PPC
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> + select ARCH_HAS_VM_GET_PAGE_PROT
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> select ARCH_KEEP_MEMBLOCK
> select ARCH_MIGHT_HAVE_PC_PARPORT
> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
> index 7cb6d18f5cd6..7b10c2031e82 100644
> --- a/arch/powerpc/include/asm/mman.h
> +++ b/arch/powerpc/include/asm/mman.h
> @@ -24,7 +24,7 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
> }
> #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
>
> -static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
> +static inline pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
> {
> #ifdef CONFIG_PPC_MEM_KEYS
> return (vm_flags & VM_SAO) ?
> @@ -34,7 +34,6 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
> return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
> #endif
> }
> -#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
>
> static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
> {
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index d564d0ecd4cd..3cbb6de20f9d 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -20,25 +20,6 @@ struct mm_struct;
> #include <asm/nohash/pgtable.h>
> #endif /* !CONFIG_PPC_BOOK3S */
>
> -/* Note due to the way vm flags are laid out, the bits are XWR */
> -#define __P000 PAGE_NONE
> -#define __P001 PAGE_READONLY
> -#define __P010 PAGE_COPY
> -#define __P011 PAGE_COPY
> -#define __P100 PAGE_READONLY_X
> -#define __P101 PAGE_READONLY_X
> -#define __P110 PAGE_COPY_X
> -#define __P111 PAGE_COPY_X
> -
> -#define __S000 PAGE_NONE
> -#define __S001 PAGE_READONLY
> -#define __S010 PAGE_SHARED
> -#define __S011 PAGE_SHARED
> -#define __S100 PAGE_READONLY_X
> -#define __S101 PAGE_READONLY_X
> -#define __S110 PAGE_SHARED_X
> -#define __S111 PAGE_SHARED_X
> -
> #ifndef __ASSEMBLY__
>
> #ifndef MAX_PTRS_PER_PGD
> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
> index c475cf810aa8..7f05e7903bd2 100644
> --- a/arch/powerpc/mm/mmap.c
> +++ b/arch/powerpc/mm/mmap.c
> @@ -254,3 +254,50 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
> mm->get_unmapped_area = arch_get_unmapped_area_topdown;
> }
> }
> +
> +static inline pgprot_t __vm_get_page_prot(unsigned long vm_flags)
> +{
> + switch (vm_flags & (VM_READ | VM_WRITE | VM_EXEC | VM_SHARED)) {
> + case VM_NONE:
> + return PAGE_NONE;
> + case VM_READ:
> + return PAGE_READONLY;
> + case VM_WRITE:
> + return PAGE_COPY;
> + case VM_READ | VM_WRITE:
> + return PAGE_COPY;
> + case VM_EXEC:
> + return PAGE_READONLY_X;
> + case VM_EXEC | VM_READ:
> + return PAGE_READONLY_X;
> + case VM_EXEC | VM_WRITE:
> + return PAGE_COPY_X;
> + case VM_EXEC | VM_READ | VM_WRITE:
> + return PAGE_COPY_X;
> + case VM_SHARED:
> + return PAGE_NONE;
> + case VM_SHARED | VM_READ:
> + return PAGE_READONLY;
> + case VM_SHARED | VM_WRITE:
> + return PAGE_SHARED;
> + case VM_SHARED | VM_READ | VM_WRITE:
> + return PAGE_SHARED;
> + case VM_SHARED | VM_EXEC:
> + return PAGE_READONLY_X;
> + case VM_SHARED | VM_EXEC | VM_READ:
> + return PAGE_READONLY_X;
> + case VM_SHARED | VM_EXEC | VM_WRITE:
> + return PAGE_SHARED_X;
> + case VM_SHARED | VM_EXEC | VM_READ | VM_WRITE:
> + return PAGE_SHARED_X;
> + default:
> + BUILD_BUG();
> + }
> +}
> +
> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
> +{
> + return __pgprot(pgprot_val(__vm_get_page_prot(vm_flags)) |
> + pgprot_val(powerpc_vm_get_page_prot(vm_flags)));
Any reason to keep powerpc_vm_get_page_prot() rather than open code it
here?
This applies to other architectures that implement arch_vm_get_page_prot()
and/or arch_filter_pgprot() as well.
> +}
> +EXPORT_SYMBOL(vm_get_page_prot);
> --
> 2.25.1
>
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v3 4/6] modules: Add CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
From: Luis Chamberlain @ 2022-02-03 19:51 UTC (permalink / raw)
To: Christophe Leroy, Lucas De Marchi, Lucas De Marchi, Aaron Tomlin,
Michal Suchánek
Cc: linux-arch@vger.kernel.org, Daniel Thompson,
kgdb-bugreport@lists.sourceforge.net, Aaron Tomlin,
linux-kernel@vger.kernel.org, Douglas Anderson, Jason Wessel,
linux-mm@kvack.org, Jessica Yu, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <228849f5-f6a4-eb45-5e1e-a9b3eccb28b3@csgroup.eu>
On Thu, Feb 03, 2022 at 07:05:13AM +0000, Christophe Leroy wrote:
>
>
> Le 03/02/2022 à 01:01, Luis Chamberlain a écrit :
> > On Sat, Jan 29, 2022 at 05:02:09PM +0000, Christophe Leroy wrote:
> >> diff --git a/kernel/module.c b/kernel/module.c
> >> index 11f51e17fb9f..f3758115ebaa 100644
> >> --- a/kernel/module.c
> >> +++ b/kernel/module.c
> >> @@ -81,7 +81,9 @@
> >> /* If this is set, the section belongs in the init part of the module */
> >> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
> >>
> >> +#ifndef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> >> #define data_layout core_layout
> >> +#endif
> >>
> >> /*
> >> * Mutex protects:
> >> @@ -111,6 +113,12 @@ static struct mod_tree_root {
> >> #define module_addr_min mod_tree.addr_min
> >> #define module_addr_max mod_tree.addr_max
> >>
> >> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> >> +static struct mod_tree_root mod_data_tree __cacheline_aligned = {
> >> + .addr_min = -1UL,
> >> +};
> >> +#endif
> >> +
> >> #ifdef CONFIG_MODULES_TREE_LOOKUP
> >>
> >> /*
> >> @@ -186,6 +194,11 @@ static void mod_tree_insert(struct module *mod)
> >> __mod_tree_insert(&mod->core_layout.mtn, &mod_tree);
> >> if (mod->init_layout.size)
> >> __mod_tree_insert(&mod->init_layout.mtn, &mod_tree);
> >> +
> >> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> >> + mod->data_layout.mtn.mod = mod;
> >> + __mod_tree_insert(&mod->data_layout.mtn, &mod_data_tree);
> >> +#endif
> >
> >
> > kernel/ directory has quite a few files, module.c is the second to
> > largest file, and it has tons of stuff. Aaron is doing work to
> > split things out to make code easier to read and so that its easier
> > to review changes. See:
> >
> > https://lkml.kernel.org/r/20220130213214.1042497-1-atomlin@redhat.com
> >
> > I think this is a good patch example which could benefit from that work.
> > So I'd much prefer to see that work go in first than this, so to see if
> > we can make the below changes more compartamentalized.
> >
> > Curious, how much testing has been put into this series?
>
>
> I tested the change up to (including) patch 4 to verify it doesn't
> introduce regression when not using
> CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC,
> Then I tested with patch 5. I first tried with the 'hello world' test
> module. After that I loaded several important modules and checked I
> didn't get any regression, both with and without STRICT_MODULES_RWX and
> I checked the consistency in /proc/vmallocinfo
> /proc/modules /sys/class/modules/*
I wonder if we have a test for STRICT_MODULES_RWX.
> I also tested with a hacked module_alloc() to force branch trampolines.
So to verify that reducing these trampolines actually helps on an
architecture? I wonder if we can generalize this somehow to let archs
verify such strategies can help.
I was hoping for a bit more wider testing, like actually users, etc.
It does not seem like so. So we can get to that by merging this soon
into modules-next and having this bleed out issues with linux-next.
We are in good time to do this now.
The kmod tree has tons of tests:
https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/
Can you use that to verify there are no regressions?
Aaron, Michal, if you can do the same that'd be appreciated.
Luis
^ permalink raw reply
* [PATCH AUTOSEL 5.16 22/52] powerpc/fixmap: Fix VM debug warning on unmap
From: Sasha Levin @ 2022-02-03 20:29 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, anshuman.khandual, aneesh.kumar, palmerdabbelt,
npiggin, geert, joel, Maxime Bizon, akpm, linuxppc-dev, shorne
In-Reply-To: <20220203202947.2304-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
[ Upstream commit aec982603aa8cc0a21143681feb5f60ecc69d718 ]
Unmapping a fixmap entry is done by calling __set_fixmap()
with FIXMAP_PAGE_CLEAR as flags.
Today, powerpc __set_fixmap() calls map_kernel_page().
map_kernel_page() is not happy when called a second time
for the same page.
WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:194 set_pte_at+0xc/0x1e8
CPU: 0 PID: 1 Comm: swapper Not tainted 5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty #682
NIP: c0017cd4 LR: c00187f0 CTR: 00000010
REGS: e1011d50 TRAP: 0700 Not tainted (5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty)
MSR: 00029032 <EE,ME,IR,DR,RI> CR: 42000208 XER: 00000000
GPR00: c0165fec e1011e10 c14c0000 c0ee2550 ff800000 c0f3d000 00000000 c001686c
GPR08: 00001000 b00045a9 00000001 c0f58460 c0f50000 00000000 c0007e10 00000000
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
GPR24: 00000000 00000000 c0ee2550 00000000 c0f57000 00000ff8 00000000 ff800000
NIP [c0017cd4] set_pte_at+0xc/0x1e8
LR [c00187f0] map_kernel_page+0x9c/0x100
Call Trace:
[e1011e10] [c0736c68] vsnprintf+0x358/0x6c8 (unreliable)
[e1011e30] [c0165fec] __set_fixmap+0x30/0x44
[e1011e40] [c0c13bdc] early_iounmap+0x11c/0x170
[e1011e70] [c0c06cb0] ioremap_legacy_serial_console+0x88/0xc0
[e1011e90] [c0c03634] do_one_initcall+0x80/0x178
[e1011ef0] [c0c0385c] kernel_init_freeable+0xb4/0x250
[e1011f20] [c0007e34] kernel_init+0x24/0x140
[e1011f30] [c0016268] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
7fe3fb78 48019689 80010014 7c630034 83e1000c 5463d97e 7c0803a6 38210010
4e800020 81250000 712a0001 41820008 <0fe00000> 9421ffe0 93e1001c 48000030
Implement unmap_kernel_page() which clears an existing pte.
Reported-by: Maxime Bizon <mbizon@freebox.fr>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Tested-by: Maxime Bizon <mbizon@freebox.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b0b752f6f6ecc60653e873f385c6f0dce4e9ab6a.1638789098.git.christophe.leroy@csgroup.eu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 1 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 2 ++
arch/powerpc/include/asm/fixmap.h | 6 ++++--
arch/powerpc/include/asm/nohash/32/pgtable.h | 1 +
arch/powerpc/include/asm/nohash/64/pgtable.h | 1 +
arch/powerpc/mm/pgtable.c | 9 +++++++++
6 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 609c80f671943..f8b94f78403f1 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -178,6 +178,7 @@ static inline bool pte_user(pte_t pte)
#ifndef __ASSEMBLY__
int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 33e073d6b0c41..875730d5af408 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1082,6 +1082,8 @@ static inline int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t p
return hash__map_kernel_page(ea, pa, prot);
}
+void unmap_kernel_page(unsigned long va);
+
static inline int __meminit vmemmap_create_mapping(unsigned long start,
unsigned long page_size,
unsigned long phys)
diff --git a/arch/powerpc/include/asm/fixmap.h b/arch/powerpc/include/asm/fixmap.h
index 947b5b9c44241..a832aeafe5601 100644
--- a/arch/powerpc/include/asm/fixmap.h
+++ b/arch/powerpc/include/asm/fixmap.h
@@ -111,8 +111,10 @@ static inline void __set_fixmap(enum fixed_addresses idx,
BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
else if (WARN_ON(idx >= __end_of_fixed_addresses))
return;
-
- map_kernel_page(__fix_to_virt(idx), phys, flags);
+ if (pgprot_val(flags))
+ map_kernel_page(__fix_to_virt(idx), phys, flags);
+ else
+ unmap_kernel_page(__fix_to_virt(idx));
}
#define __early_set_fixmap __set_fixmap
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
index b67742e2a9b22..d959c2a73fbf4 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -64,6 +64,7 @@ extern int icache_44x_need_flush;
#ifndef __ASSEMBLY__
int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h
index 9d2905a474103..2225991c69b55 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -308,6 +308,7 @@ static inline void __ptep_set_access_flags(struct vm_area_struct *vma,
#define __swp_entry_to_pte(x) __pte((x).val)
int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
extern int __meminit vmemmap_create_mapping(unsigned long start,
unsigned long page_size,
unsigned long phys);
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index ce94823831442..b7385e637e3e3 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -203,6 +203,15 @@ void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
__set_pte_at(mm, addr, ptep, pte, 0);
}
+void unmap_kernel_page(unsigned long va)
+{
+ pmd_t *pmdp = pmd_off_k(va);
+ pte_t *ptep = pte_offset_kernel(pmdp, va);
+
+ pte_clear(&init_mm, va, ptep);
+ flush_tlb_kernel_range(va, va + PAGE_SIZE);
+}
+
/*
* This is called when relaxing access to a PTE. It's also called in the page
* fault path when we don't hit any of the major fault cases, ie, a minor
--
2.34.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 22/41] powerpc/fixmap: Fix VM debug warning on unmap
From: Sasha Levin @ 2022-02-03 20:32 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, sfr, anshuman.khandual, aneesh.kumar, palmerdabbelt,
npiggin, geert, guoren, joel, Maxime Bizon, akpm, linuxppc-dev
In-Reply-To: <20220203203245.3007-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
[ Upstream commit aec982603aa8cc0a21143681feb5f60ecc69d718 ]
Unmapping a fixmap entry is done by calling __set_fixmap()
with FIXMAP_PAGE_CLEAR as flags.
Today, powerpc __set_fixmap() calls map_kernel_page().
map_kernel_page() is not happy when called a second time
for the same page.
WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:194 set_pte_at+0xc/0x1e8
CPU: 0 PID: 1 Comm: swapper Not tainted 5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty #682
NIP: c0017cd4 LR: c00187f0 CTR: 00000010
REGS: e1011d50 TRAP: 0700 Not tainted (5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty)
MSR: 00029032 <EE,ME,IR,DR,RI> CR: 42000208 XER: 00000000
GPR00: c0165fec e1011e10 c14c0000 c0ee2550 ff800000 c0f3d000 00000000 c001686c
GPR08: 00001000 b00045a9 00000001 c0f58460 c0f50000 00000000 c0007e10 00000000
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
GPR24: 00000000 00000000 c0ee2550 00000000 c0f57000 00000ff8 00000000 ff800000
NIP [c0017cd4] set_pte_at+0xc/0x1e8
LR [c00187f0] map_kernel_page+0x9c/0x100
Call Trace:
[e1011e10] [c0736c68] vsnprintf+0x358/0x6c8 (unreliable)
[e1011e30] [c0165fec] __set_fixmap+0x30/0x44
[e1011e40] [c0c13bdc] early_iounmap+0x11c/0x170
[e1011e70] [c0c06cb0] ioremap_legacy_serial_console+0x88/0xc0
[e1011e90] [c0c03634] do_one_initcall+0x80/0x178
[e1011ef0] [c0c0385c] kernel_init_freeable+0xb4/0x250
[e1011f20] [c0007e34] kernel_init+0x24/0x140
[e1011f30] [c0016268] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
7fe3fb78 48019689 80010014 7c630034 83e1000c 5463d97e 7c0803a6 38210010
4e800020 81250000 712a0001 41820008 <0fe00000> 9421ffe0 93e1001c 48000030
Implement unmap_kernel_page() which clears an existing pte.
Reported-by: Maxime Bizon <mbizon@freebox.fr>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Tested-by: Maxime Bizon <mbizon@freebox.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b0b752f6f6ecc60653e873f385c6f0dce4e9ab6a.1638789098.git.christophe.leroy@csgroup.eu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 1 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 2 ++
arch/powerpc/include/asm/fixmap.h | 6 ++++--
arch/powerpc/include/asm/nohash/32/pgtable.h | 1 +
arch/powerpc/include/asm/nohash/64/pgtable.h | 1 +
arch/powerpc/mm/pgtable.c | 9 +++++++++
6 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 609c80f671943..f8b94f78403f1 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -178,6 +178,7 @@ static inline bool pte_user(pte_t pte)
#ifndef __ASSEMBLY__
int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 5d34a8646f081..6866d860d4f30 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1082,6 +1082,8 @@ static inline int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t p
return hash__map_kernel_page(ea, pa, prot);
}
+void unmap_kernel_page(unsigned long va);
+
static inline int __meminit vmemmap_create_mapping(unsigned long start,
unsigned long page_size,
unsigned long phys)
diff --git a/arch/powerpc/include/asm/fixmap.h b/arch/powerpc/include/asm/fixmap.h
index 947b5b9c44241..a832aeafe5601 100644
--- a/arch/powerpc/include/asm/fixmap.h
+++ b/arch/powerpc/include/asm/fixmap.h
@@ -111,8 +111,10 @@ static inline void __set_fixmap(enum fixed_addresses idx,
BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
else if (WARN_ON(idx >= __end_of_fixed_addresses))
return;
-
- map_kernel_page(__fix_to_virt(idx), phys, flags);
+ if (pgprot_val(flags))
+ map_kernel_page(__fix_to_virt(idx), phys, flags);
+ else
+ unmap_kernel_page(__fix_to_virt(idx));
}
#define __early_set_fixmap __set_fixmap
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
index d6ba821a56ced..63ea4693ccea6 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -64,6 +64,7 @@ extern int icache_44x_need_flush;
#ifndef __ASSEMBLY__
int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h
index 9d2905a474103..2225991c69b55 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -308,6 +308,7 @@ static inline void __ptep_set_access_flags(struct vm_area_struct *vma,
#define __swp_entry_to_pte(x) __pte((x).val)
int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
extern int __meminit vmemmap_create_mapping(unsigned long start,
unsigned long page_size,
unsigned long phys);
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index cd16b407f47e1..9a93c1a5aa1d1 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -203,6 +203,15 @@ void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
__set_pte_at(mm, addr, ptep, pte, 0);
}
+void unmap_kernel_page(unsigned long va)
+{
+ pmd_t *pmdp = pmd_off_k(va);
+ pte_t *ptep = pte_offset_kernel(pmdp, va);
+
+ pte_clear(&init_mm, va, ptep);
+ flush_tlb_kernel_range(va, va + PAGE_SIZE);
+}
+
/*
* This is called when relaxing access to a PTE. It's also called in the page
* fault path when we don't hit any of the major fault cases, ie, a minor
--
2.34.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 12/25] powerpc/fixmap: Fix VM debug warning on unmap
From: Sasha Levin @ 2022-02-03 20:34 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, catalin.marinas, anshuman.khandual, aneesh.kumar,
geert, npiggin, Maxime Bizon, akpm, linuxppc-dev, rppt, joel
In-Reply-To: <20220203203447.3570-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@csgroup.eu>
[ Upstream commit aec982603aa8cc0a21143681feb5f60ecc69d718 ]
Unmapping a fixmap entry is done by calling __set_fixmap()
with FIXMAP_PAGE_CLEAR as flags.
Today, powerpc __set_fixmap() calls map_kernel_page().
map_kernel_page() is not happy when called a second time
for the same page.
WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pgtable.c:194 set_pte_at+0xc/0x1e8
CPU: 0 PID: 1 Comm: swapper Not tainted 5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty #682
NIP: c0017cd4 LR: c00187f0 CTR: 00000010
REGS: e1011d50 TRAP: 0700 Not tainted (5.16.0-rc3-s3k-dev-01993-g350ff07feb7d-dirty)
MSR: 00029032 <EE,ME,IR,DR,RI> CR: 42000208 XER: 00000000
GPR00: c0165fec e1011e10 c14c0000 c0ee2550 ff800000 c0f3d000 00000000 c001686c
GPR08: 00001000 b00045a9 00000001 c0f58460 c0f50000 00000000 c0007e10 00000000
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
GPR24: 00000000 00000000 c0ee2550 00000000 c0f57000 00000ff8 00000000 ff800000
NIP [c0017cd4] set_pte_at+0xc/0x1e8
LR [c00187f0] map_kernel_page+0x9c/0x100
Call Trace:
[e1011e10] [c0736c68] vsnprintf+0x358/0x6c8 (unreliable)
[e1011e30] [c0165fec] __set_fixmap+0x30/0x44
[e1011e40] [c0c13bdc] early_iounmap+0x11c/0x170
[e1011e70] [c0c06cb0] ioremap_legacy_serial_console+0x88/0xc0
[e1011e90] [c0c03634] do_one_initcall+0x80/0x178
[e1011ef0] [c0c0385c] kernel_init_freeable+0xb4/0x250
[e1011f20] [c0007e34] kernel_init+0x24/0x140
[e1011f30] [c0016268] ret_from_kernel_thread+0x5c/0x64
Instruction dump:
7fe3fb78 48019689 80010014 7c630034 83e1000c 5463d97e 7c0803a6 38210010
4e800020 81250000 712a0001 41820008 <0fe00000> 9421ffe0 93e1001c 48000030
Implement unmap_kernel_page() which clears an existing pte.
Reported-by: Maxime Bizon <mbizon@freebox.fr>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Tested-by: Maxime Bizon <mbizon@freebox.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/b0b752f6f6ecc60653e873f385c6f0dce4e9ab6a.1638789098.git.christophe.leroy@csgroup.eu
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 1 +
arch/powerpc/include/asm/book3s/64/pgtable.h | 2 ++
arch/powerpc/include/asm/fixmap.h | 6 ++++--
arch/powerpc/include/asm/nohash/32/pgtable.h | 1 +
arch/powerpc/include/asm/nohash/64/pgtable.h | 1 +
arch/powerpc/mm/pgtable.c | 9 +++++++++
6 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 523d3e6e24009..94c5c66231a8c 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -142,6 +142,7 @@ static inline bool pte_user(pte_t pte)
#ifndef __ASSEMBLY__
int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 4a3dca0271f1e..71e2c524f1eea 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1054,6 +1054,8 @@ static inline int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t p
return hash__map_kernel_page(ea, pa, prot);
}
+void unmap_kernel_page(unsigned long va);
+
static inline int __meminit vmemmap_create_mapping(unsigned long start,
unsigned long page_size,
unsigned long phys)
diff --git a/arch/powerpc/include/asm/fixmap.h b/arch/powerpc/include/asm/fixmap.h
index 591b2f4deed53..897cc68758d44 100644
--- a/arch/powerpc/include/asm/fixmap.h
+++ b/arch/powerpc/include/asm/fixmap.h
@@ -111,8 +111,10 @@ static inline void __set_fixmap(enum fixed_addresses idx,
BUILD_BUG_ON(idx >= __end_of_fixed_addresses);
else if (WARN_ON(idx >= __end_of_fixed_addresses))
return;
-
- map_kernel_page(__fix_to_virt(idx), phys, flags);
+ if (pgprot_val(flags))
+ map_kernel_page(__fix_to_virt(idx), phys, flags);
+ else
+ unmap_kernel_page(__fix_to_virt(idx));
}
#define __early_set_fixmap __set_fixmap
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
index 96522f7f0618a..e53cc07e6b9ec 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -65,6 +65,7 @@ extern int icache_44x_need_flush;
#ifndef __ASSEMBLY__
int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h
index 57cd3892bfe05..1eacff0fff029 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -311,6 +311,7 @@ static inline void __ptep_set_access_flags(struct vm_area_struct *vma,
#define __swp_entry_to_pte(x) __pte((x).val)
int map_kernel_page(unsigned long ea, unsigned long pa, pgprot_t prot);
+void unmap_kernel_page(unsigned long va);
extern int __meminit vmemmap_create_mapping(unsigned long start,
unsigned long page_size,
unsigned long phys);
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index 15555c95cebc7..faaf33e204de1 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -194,6 +194,15 @@ void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
__set_pte_at(mm, addr, ptep, pte, 0);
}
+void unmap_kernel_page(unsigned long va)
+{
+ pmd_t *pmdp = pmd_off_k(va);
+ pte_t *ptep = pte_offset_kernel(pmdp, va);
+
+ pte_clear(&init_mm, va, ptep);
+ flush_tlb_kernel_range(va, va + PAGE_SIZE);
+}
+
/*
* This is called when relaxing access to a PTE. It's also called in the page
* fault path when we don't hit any of the major fault cases, ie, a minor
--
2.34.1
^ permalink raw reply related
* Re: [RFC V1 04/31] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT
From: Anshuman Khandual @ 2022-02-04 2:57 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-kernel, hch, linux-mm, Paul Mackerras, akpm, linuxppc-dev
In-Reply-To: <Yfwbz5qu20bjFZOP@kernel.org>
On 2/3/22 11:45 PM, Mike Rapoport wrote:
> On Mon, Jan 24, 2022 at 06:26:41PM +0530, Anshuman Khandual wrote:
>> This defines and exports a platform specific custom vm_get_page_prot() via
>> subscribing ARCH_HAS_VM_GET_PAGE_PROT. Subsequently all __SXXX and __PXXX
>> macros can be dropped which are no longer needed. While here, this also
>> localizes arch_vm_get_page_prot() as powerpc_vm_get_page_prot().
>>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> arch/powerpc/Kconfig | 1 +
>> arch/powerpc/include/asm/mman.h | 3 +-
>> arch/powerpc/include/asm/pgtable.h | 19 ------------
>> arch/powerpc/mm/mmap.c | 47 ++++++++++++++++++++++++++++++
>> 4 files changed, 49 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index b779603978e1..ddb4a3687c05 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -135,6 +135,7 @@ config PPC
>> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>> select ARCH_HAS_UACCESS_FLUSHCACHE
>> select ARCH_HAS_UBSAN_SANITIZE_ALL
>> + select ARCH_HAS_VM_GET_PAGE_PROT
>> select ARCH_HAVE_NMI_SAFE_CMPXCHG
>> select ARCH_KEEP_MEMBLOCK
>> select ARCH_MIGHT_HAVE_PC_PARPORT
>> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
>> index 7cb6d18f5cd6..7b10c2031e82 100644
>> --- a/arch/powerpc/include/asm/mman.h
>> +++ b/arch/powerpc/include/asm/mman.h
>> @@ -24,7 +24,7 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
>> }
>> #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
>>
>> -static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
>> +static inline pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
>> {
>> #ifdef CONFIG_PPC_MEM_KEYS
>> return (vm_flags & VM_SAO) ?
>> @@ -34,7 +34,6 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
>> return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
>> #endif
>> }
>> -#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
>>
>> static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
>> {
>> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
>> index d564d0ecd4cd..3cbb6de20f9d 100644
>> --- a/arch/powerpc/include/asm/pgtable.h
>> +++ b/arch/powerpc/include/asm/pgtable.h
>> @@ -20,25 +20,6 @@ struct mm_struct;
>> #include <asm/nohash/pgtable.h>
>> #endif /* !CONFIG_PPC_BOOK3S */
>>
>> -/* Note due to the way vm flags are laid out, the bits are XWR */
>> -#define __P000 PAGE_NONE
>> -#define __P001 PAGE_READONLY
>> -#define __P010 PAGE_COPY
>> -#define __P011 PAGE_COPY
>> -#define __P100 PAGE_READONLY_X
>> -#define __P101 PAGE_READONLY_X
>> -#define __P110 PAGE_COPY_X
>> -#define __P111 PAGE_COPY_X
>> -
>> -#define __S000 PAGE_NONE
>> -#define __S001 PAGE_READONLY
>> -#define __S010 PAGE_SHARED
>> -#define __S011 PAGE_SHARED
>> -#define __S100 PAGE_READONLY_X
>> -#define __S101 PAGE_READONLY_X
>> -#define __S110 PAGE_SHARED_X
>> -#define __S111 PAGE_SHARED_X
>> -
>> #ifndef __ASSEMBLY__
>>
>> #ifndef MAX_PTRS_PER_PGD
>> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
>> index c475cf810aa8..7f05e7903bd2 100644
>> --- a/arch/powerpc/mm/mmap.c
>> +++ b/arch/powerpc/mm/mmap.c
>> @@ -254,3 +254,50 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
>> mm->get_unmapped_area = arch_get_unmapped_area_topdown;
>> }
>> }
>> +
>> +static inline pgprot_t __vm_get_page_prot(unsigned long vm_flags)
>> +{
>> + switch (vm_flags & (VM_READ | VM_WRITE | VM_EXEC | VM_SHARED)) {
>> + case VM_NONE:
>> + return PAGE_NONE;
>> + case VM_READ:
>> + return PAGE_READONLY;
>> + case VM_WRITE:
>> + return PAGE_COPY;
>> + case VM_READ | VM_WRITE:
>> + return PAGE_COPY;
>> + case VM_EXEC:
>> + return PAGE_READONLY_X;
>> + case VM_EXEC | VM_READ:
>> + return PAGE_READONLY_X;
>> + case VM_EXEC | VM_WRITE:
>> + return PAGE_COPY_X;
>> + case VM_EXEC | VM_READ | VM_WRITE:
>> + return PAGE_COPY_X;
>> + case VM_SHARED:
>> + return PAGE_NONE;
>> + case VM_SHARED | VM_READ:
>> + return PAGE_READONLY;
>> + case VM_SHARED | VM_WRITE:
>> + return PAGE_SHARED;
>> + case VM_SHARED | VM_READ | VM_WRITE:
>> + return PAGE_SHARED;
>> + case VM_SHARED | VM_EXEC:
>> + return PAGE_READONLY_X;
>> + case VM_SHARED | VM_EXEC | VM_READ:
>> + return PAGE_READONLY_X;
>> + case VM_SHARED | VM_EXEC | VM_WRITE:
>> + return PAGE_SHARED_X;
>> + case VM_SHARED | VM_EXEC | VM_READ | VM_WRITE:
>> + return PAGE_SHARED_X;
>> + default:
>> + BUILD_BUG();
>> + }
>> +}
>> +
>> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
>> +{
>> + return __pgprot(pgprot_val(__vm_get_page_prot(vm_flags)) |
>> + pgprot_val(powerpc_vm_get_page_prot(vm_flags)));
> Any reason to keep powerpc_vm_get_page_prot() rather than open code it
> here?
>
> This applies to other architectures that implement arch_vm_get_page_prot()
> and/or arch_filter_pgprot() as well.
Just to minimize the code churn ! But I will be happy to open code them
here (and in other platforms) if that will be preferred.
^ permalink raw reply
* [PATCH kernel v2] powerpc/64: Add UADDR64 relocation support
From: Alexey Kardashevskiy @ 2022-02-04 3:48 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, Alan Modra, Nicholas Piggin, Paul Mackerras
When ld detects unaligned relocations, it emits R_PPC64_UADDR64
relocations instead of R_PPC64_RELATIVE. Currently R_PPC64_UADDR64 are
detected by arch/powerpc/tools/relocs_check.sh and expected not to work.
Below is a simple chunk to trigger this behaviour (this disables
optimization for the demonstration purposes only, this also happens with
-O1/-O2 when CONFIG_PRINTK_INDEX=y, for example):
\#pragma GCC push_options
\#pragma GCC optimize ("O0")
struct entry {
const char *file;
int line;
} __attribute__((packed));
static const struct entry e1 = { .file = __FILE__, .line = __LINE__ };
static const struct entry e2 = { .file = __FILE__, .line = __LINE__ };
...
prom_printf("e1=%s %lx %lx\n", e1.file, (unsigned long) e1.file, mfmsr());
prom_printf("e2=%s %lx\n", e2.file, (unsigned long) e2.file);
\#pragma GCC pop_options
This adds support for UADDR64 for 64bit. This reuses __dynamic_symtab
from the 32bit which supports more relocation types already.
Because RELACOUNT includes only R_PPC64_RELATIVE, this replaces it with
RELASZ which is the size of all relocation records.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* replaced RELACOUNT with RELASZ/RELAENT
* removed FIXME
---
Tested via qemu gdb stub (the kernel is loaded at 0x400000).
Disasm:
c000000001a804d0 <e1>:
c000000001a804d0: b0 04 a8 01 .long 0x1a804b0
c000000001a804d0: R_PPC64_RELATIVE *ABS*-0x3ffffffffe57fb50
c000000001a804d4: 00 00 00 c0 lfs f0,0(0)
c000000001a804d8: fa 08 00 00 .long 0x8fa
c000000001a804dc <e2>:
...
c000000001a804dc: R_PPC64_UADDR64 .rodata+0x4b0
Before relocation:
>>> p *(unsigned long *) 0x1e804d0
$1 = 0xc000000001a804b0
>>> p *(unsigned long *) 0x1e804dc
$2 = 0x0
After relocation in __boot_from_prom:
>>> p *(unsigned long *) 0x1e804d0
$1 = 0x1e804b0
>>> p *(unsigned long *) 0x1e804dc
$2 = 0x1e804b0
After relocation in __after_prom_start:
>>> p *(unsigned long *) 0x1e804d0
$1 = 0xc000000001a804b0
>>> p *(unsigned long *) 0x1e804dc
$2 = 0xc000000001a804b0
>>>
---
arch/powerpc/kernel/reloc_64.S | 56 ++++++++++++++++++++----------
arch/powerpc/kernel/vmlinux.lds.S | 2 --
arch/powerpc/tools/relocs_check.sh | 7 +---
3 files changed, 39 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/reloc_64.S b/arch/powerpc/kernel/reloc_64.S
index 02d4719bf43a..f7dcc25e93d0 100644
--- a/arch/powerpc/kernel/reloc_64.S
+++ b/arch/powerpc/kernel/reloc_64.S
@@ -8,8 +8,10 @@
#include <asm/ppc_asm.h>
RELA = 7
-RELACOUNT = 0x6ffffff9
+RELASZ = 8
+RELAENT = 9
R_PPC64_RELATIVE = 22
+R_PPC64_UADDR64 = 43
/*
* r3 = desired final address of kernel
@@ -25,29 +27,36 @@ _GLOBAL(relocate)
add r9,r9,r12 /* r9 has runtime addr of .rela.dyn section */
ld r10,(p_st - 0b)(r12)
add r10,r10,r12 /* r10 has runtime addr of _stext */
+ ld r13,(p_sym - 0b)(r12)
+ add r13,r13,r12 /* r13 has runtime addr of .dynsym */
/*
- * Scan the dynamic section for the RELA and RELACOUNT entries.
+ * Scan the dynamic section for the RELA, RELASZ and RELAENT entries.
*/
li r7,0
li r8,0
1: ld r6,0(r11) /* get tag */
cmpdi r6,0
- beq 4f /* end of list */
+ beq 5f /* end of list */
cmpdi r6,RELA
bne 2f
ld r7,8(r11) /* get RELA pointer in r7 */
- b 3f
-2: addis r6,r6,(-RELACOUNT)@ha
- cmpdi r6,RELACOUNT@l
+ b 4f
+2: cmpdi r6,RELASZ
bne 3f
- ld r8,8(r11) /* get RELACOUNT value in r8 */
-3: addi r11,r11,16
+ ld r8,8(r11) /* get RELASZ value in r8 */
+ b 4f
+3: cmpdi r6,RELAENT
+ bne 4f
+ ld r12,8(r11) /* get RELAENT value in r12 */
+4: addi r11,r11,16
b 1b
-4: cmpdi r7,0 /* check we have both RELA and RELACOUNT */
+5: cmpdi r7,0 /* check we have RELA, RELASZ, RELAENT */
cmpdi cr1,r8,0
- beq 6f
- beq cr1,6f
+ beq 10f
+ beq cr1,10f
+ cmpdi r12,0
+ beq 10f
/*
* Work out linktime address of _stext and hence the
@@ -62,23 +71,34 @@ _GLOBAL(relocate)
/*
* Run through the list of relocations and process the
- * R_PPC64_RELATIVE ones.
+ * R_PPC64_RELATIVE and R_PPC64_UADDR64 ones.
*/
+ divd r8,r8,r12 /* RELASZ / RELAENT */
mtctr r8
-5: ld r0,8(9) /* ELF64_R_TYPE(reloc->r_info) */
+5: lwa r0,8(r9) /* ELF64_R_TYPE(reloc->r_info) */
cmpdi r0,R_PPC64_RELATIVE
- bne 6f
+ bne 7f
ld r6,0(r9) /* reloc->r_offset */
ld r0,16(r9) /* reloc->r_addend */
- add r0,r0,r3
+ b 8f
+7: cmpdi r0,R_PPC64_UADDR64
+ bne 9f
+ ld r6,0(r9)
+ ld r0,16(r9)
+ lwa r14,12(r9) /* ELF64_R_SYM(reloc->r_info) */
+ mulli r14,r14,24 /* 24 == sizeof(elf64_sym) */
+ add r14,r14,r13 /* elf64_sym[ELF64_R_SYM] */
+ ld r14,8(r14)
+ add r0,r0,r14
+8: add r0,r0,r3
stdx r0,r7,r6
- addi r9,r9,24
+9: add r9,r9,r12
bdnz 5b
-
-6: blr
+10: blr
.balign 8
p_dyn: .8byte __dynamic_start - 0b
p_rela: .8byte __rela_dyn_start - 0b
+p_sym: .8byte __dynamic_symtab - 0b
p_st: .8byte _stext - 0b
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 2bcca818136a..fe22d940412f 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -281,9 +281,7 @@ SECTIONS
. = ALIGN(8);
.dynsym : AT(ADDR(.dynsym) - LOAD_OFFSET)
{
-#ifdef CONFIG_PPC32
__dynamic_symtab = .;
-#endif
*(.dynsym)
}
.dynstr : AT(ADDR(.dynstr) - LOAD_OFFSET) { *(.dynstr) }
diff --git a/arch/powerpc/tools/relocs_check.sh b/arch/powerpc/tools/relocs_check.sh
index 014e00e74d2b..63792af00417 100755
--- a/arch/powerpc/tools/relocs_check.sh
+++ b/arch/powerpc/tools/relocs_check.sh
@@ -39,6 +39,7 @@ $objdump -R "$vmlinux" |
# R_PPC_NONE
grep -F -w -v 'R_PPC64_RELATIVE
R_PPC64_NONE
+R_PPC64_UADDR64
R_PPC_ADDR16_LO
R_PPC_ADDR16_HI
R_PPC_ADDR16_HA
@@ -54,9 +55,3 @@ fi
num_bad=$(echo "$bad_relocs" | wc -l)
echo "WARNING: $num_bad bad relocations"
echo "$bad_relocs"
-
-# If we see this type of relocation it's an idication that
-# we /may/ be using an old version of binutils.
-if echo "$bad_relocs" | grep -q -F -w R_PPC64_UADDR64; then
- echo "WARNING: You need at least binutils >= 2.19 to build a CONFIG_RELOCATABLE kernel"
-fi
--
2.30.2
^ permalink raw reply related
* [PATCH] powerpc/64s/hash: Make hash faults work in NMI context
From: Nicholas Piggin @ 2022-02-04 3:53 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Aneesh Kumar K . V, Laurent Dufour, Nicholas Piggin
Hash faults are not resoved in NMI context, instead causing the access
to fail. This is done because perf interrupts can get backtraces
including walking the user stack, and taking a hash fault on those could
deadlock on the HPTE lock if the perf interrupt hits while the same HPTE
lock is being held by the hash fault code. The user-access for the stack
walking will notice the access failed and deal with that in the perf
code.
The reason to allow perf interrupts in is to better profile hash faults.
The problem with this is any hash fault on a kernel access that happens
in NMI context will crash, because kernel accesses must not fail.
Hard lockups, system reset, machine checks that access vmalloc space
including modules and including stack backtracing and symbol lookup in
modules, per-cpu data, etc could all run into this problem.
Fix this by disallowing perf interrupts in the hash fault code (the
direct hash fault is covered by MSR[EE]=0 so the PMI disable just needs
to extend to the preload case). This simplifies the tricky logic in hash
faults and perf, at the cost of reduced profiling of hash faults.
perf can still latch addresses when interrupts are disabled, it just
won't get the stack trace at that point, so it would still find hot
spots, just sometimes with confusing stack chains.
An alternative could be to allow perf interrupts here but always do the
slowpath stack walk if we are in nmi context, but that slows down all
perf interrupt stack walking on hash though and it does not remove as
much tricky code.
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: Laurent Dufour <ldufour@linux.ibm.com>
Tested-by: Laurent Dufour <ldufour@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/interrupt.h | 2 +-
arch/powerpc/mm/book3s64/hash_utils.c | 54 ++++-----------------------
arch/powerpc/perf/callchain.h | 9 +----
arch/powerpc/perf/callchain_64.c | 27 --------------
4 files changed, 10 insertions(+), 82 deletions(-)
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index fc28f46d2f9d..5404f7abbcf8 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -612,7 +612,7 @@ DECLARE_INTERRUPT_HANDLER_RAW(do_slb_fault);
DECLARE_INTERRUPT_HANDLER(do_bad_segment_interrupt);
/* hash_utils.c */
-DECLARE_INTERRUPT_HANDLER_RAW(do_hash_fault);
+DECLARE_INTERRUPT_HANDLER(do_hash_fault);
/* fault.c */
DECLARE_INTERRUPT_HANDLER(do_page_fault);
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 7abf82a698d3..985cabdd7f67 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1621,8 +1621,7 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap,
}
EXPORT_SYMBOL_GPL(hash_page);
-DECLARE_INTERRUPT_HANDLER(__do_hash_fault);
-DEFINE_INTERRUPT_HANDLER(__do_hash_fault)
+DEFINE_INTERRUPT_HANDLER(do_hash_fault)
{
unsigned long ea = regs->dar;
unsigned long dsisr = regs->dsisr;
@@ -1681,35 +1680,6 @@ DEFINE_INTERRUPT_HANDLER(__do_hash_fault)
}
}
-/*
- * The _RAW interrupt entry checks for the in_nmi() case before
- * running the full handler.
- */
-DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
-{
- /*
- * If we are in an "NMI" (e.g., an interrupt when soft-disabled), then
- * don't call hash_page, just fail the fault. This is required to
- * prevent re-entrancy problems in the hash code, namely perf
- * interrupts hitting while something holds H_PAGE_BUSY, and taking a
- * hash fault. See the comment in hash_preload().
- *
- * We come here as a result of a DSI at a point where we don't want
- * to call hash_page, such as when we are accessing memory (possibly
- * user memory) inside a PMU interrupt that occurred while interrupts
- * were soft-disabled. We want to invoke the exception handler for
- * the access, or panic if there isn't a handler.
- */
- if (unlikely(in_nmi())) {
- do_bad_page_fault_segv(regs);
- return 0;
- }
-
- __do_hash_fault(regs);
-
- return 0;
-}
-
#ifdef CONFIG_PPC_MM_SLICES
static bool should_hash_preload(struct mm_struct *mm, unsigned long ea)
{
@@ -1776,26 +1746,18 @@ static void hash_preload(struct mm_struct *mm, pte_t *ptep, unsigned long ea,
#endif /* CONFIG_PPC_64K_PAGES */
/*
- * __hash_page_* must run with interrupts off, as it sets the
- * H_PAGE_BUSY bit. It's possible for perf interrupts to hit at any
- * time and may take a hash fault reading the user stack, see
- * read_user_stack_slow() in the powerpc/perf code.
- *
- * If that takes a hash fault on the same page as we lock here, it
- * will bail out when seeing H_PAGE_BUSY set, and retry the access
- * leading to an infinite loop.
+ * __hash_page_* must run with interrupts off, including PMI interrupts
+ * off, as it sets the H_PAGE_BUSY bit.
*
- * Disabling interrupts here does not prevent perf interrupts, but it
- * will prevent them taking hash faults (see the NMI test in
- * do_hash_page), then read_user_stack's copy_from_user_nofault will
- * fail and perf will fall back to read_user_stack_slow(), which
- * walks the Linux page tables.
+ * It's otherwise possible for perf interrupts to hit at any time and
+ * may take a hash fault reading the user stack, which could take a
+ * hash miss and deadlock on the same H_PAGE_BUSY bit.
*
* Interrupts must also be off for the duration of the
* mm_is_thread_local test and update, to prevent preempt running the
* mm on another CPU (XXX: this may be racy vs kthread_use_mm).
*/
- local_irq_save(flags);
+ powerpc_local_irq_pmu_save(flags);
/* Is that local to this CPU ? */
if (mm_is_thread_local(mm))
@@ -1820,7 +1782,7 @@ static void hash_preload(struct mm_struct *mm, pte_t *ptep, unsigned long ea,
mm_ctx_user_psize(&mm->context),
pte_val(*ptep));
- local_irq_restore(flags);
+ powerpc_local_irq_pmu_restore(flags);
}
/*
diff --git a/arch/powerpc/perf/callchain.h b/arch/powerpc/perf/callchain.h
index d6fa6e25234f..19a8d051ddf1 100644
--- a/arch/powerpc/perf/callchain.h
+++ b/arch/powerpc/perf/callchain.h
@@ -2,7 +2,6 @@
#ifndef _POWERPC_PERF_CALLCHAIN_H
#define _POWERPC_PERF_CALLCHAIN_H
-int read_user_stack_slow(const void __user *ptr, void *buf, int nb);
void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs);
void perf_callchain_user_32(struct perf_callchain_entry_ctx *entry,
@@ -26,17 +25,11 @@ static inline int __read_user_stack(const void __user *ptr, void *ret,
size_t size)
{
unsigned long addr = (unsigned long)ptr;
- int rc;
if (addr > TASK_SIZE - size || (addr & (size - 1)))
return -EFAULT;
- rc = copy_from_user_nofault(ret, ptr, size);
-
- if (IS_ENABLED(CONFIG_PPC64) && !radix_enabled() && rc)
- return read_user_stack_slow(ptr, ret, size);
-
- return rc;
+ return copy_from_user_nofault(ret, ptr, size);
}
#endif /* _POWERPC_PERF_CALLCHAIN_H */
diff --git a/arch/powerpc/perf/callchain_64.c b/arch/powerpc/perf/callchain_64.c
index 8d0df4226328..488e8a21a11e 100644
--- a/arch/powerpc/perf/callchain_64.c
+++ b/arch/powerpc/perf/callchain_64.c
@@ -18,33 +18,6 @@
#include "callchain.h"
-/*
- * On 64-bit we don't want to invoke hash_page on user addresses from
- * interrupt context, so if the access faults, we read the page tables
- * to find which page (if any) is mapped and access it directly. Radix
- * has no need for this so it doesn't use read_user_stack_slow.
- */
-int read_user_stack_slow(const void __user *ptr, void *buf, int nb)
-{
-
- unsigned long addr = (unsigned long) ptr;
- unsigned long offset;
- struct page *page;
- void *kaddr;
-
- if (get_user_page_fast_only(addr, FOLL_WRITE, &page)) {
- kaddr = page_address(page);
-
- /* align address to page boundary */
- offset = addr & ~PAGE_MASK;
-
- memcpy(buf, kaddr + offset, nb);
- put_page(page);
- return 0;
- }
- return -EFAULT;
-}
-
static int read_user_stack_64(const unsigned long __user *ptr, unsigned long *ret)
{
return __read_user_stack(ptr, ret, sizeof(*ret));
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] powerpc/64s/hash: Make hash faults work in NMI context
From: Aneesh Kumar K.V @ 2022-02-04 4:27 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Laurent Dufour, Nicholas Piggin
In-Reply-To: <20220204035348.545435-1-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Hash faults are not resoved in NMI context, instead causing the access
> to fail. This is done because perf interrupts can get backtraces
> including walking the user stack, and taking a hash fault on those could
> deadlock on the HPTE lock if the perf interrupt hits while the same HPTE
> lock is being held by the hash fault code. The user-access for the stack
> walking will notice the access failed and deal with that in the perf
> code.
>
> The reason to allow perf interrupts in is to better profile hash faults.
>
> The problem with this is any hash fault on a kernel access that happens
> in NMI context will crash, because kernel accesses must not fail.
>
> Hard lockups, system reset, machine checks that access vmalloc space
> including modules and including stack backtracing and symbol lookup in
> modules, per-cpu data, etc could all run into this problem.
>
> Fix this by disallowing perf interrupts in the hash fault code (the
> direct hash fault is covered by MSR[EE]=0 so the PMI disable just needs
> to extend to the preload case). This simplifies the tricky logic in hash
> faults and perf, at the cost of reduced profiling of hash faults.
>
> perf can still latch addresses when interrupts are disabled, it just
> won't get the stack trace at that point, so it would still find hot
> spots, just sometimes with confusing stack chains.
>
> An alternative could be to allow perf interrupts here but always do the
> slowpath stack walk if we are in nmi context, but that slows down all
> perf interrupt stack walking on hash though and it does not remove as
> much tricky code.
>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Reported-by: Laurent Dufour <ldufour@linux.ibm.com>
> Tested-by: Laurent Dufour <ldufour@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 2 +-
> arch/powerpc/mm/book3s64/hash_utils.c | 54 ++++-----------------------
> arch/powerpc/perf/callchain.h | 9 +----
> arch/powerpc/perf/callchain_64.c | 27 --------------
> 4 files changed, 10 insertions(+), 82 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index fc28f46d2f9d..5404f7abbcf8 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -612,7 +612,7 @@ DECLARE_INTERRUPT_HANDLER_RAW(do_slb_fault);
> DECLARE_INTERRUPT_HANDLER(do_bad_segment_interrupt);
>
> /* hash_utils.c */
> -DECLARE_INTERRUPT_HANDLER_RAW(do_hash_fault);
> +DECLARE_INTERRUPT_HANDLER(do_hash_fault);
>
> /* fault.c */
> DECLARE_INTERRUPT_HANDLER(do_page_fault);
> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
> index 7abf82a698d3..985cabdd7f67 100644
> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@ -1621,8 +1621,7 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap,
> }
> EXPORT_SYMBOL_GPL(hash_page);
>
> -DECLARE_INTERRUPT_HANDLER(__do_hash_fault);
> -DEFINE_INTERRUPT_HANDLER(__do_hash_fault)
> +DEFINE_INTERRUPT_HANDLER(do_hash_fault)
> {
> unsigned long ea = regs->dar;
> unsigned long dsisr = regs->dsisr;
> @@ -1681,35 +1680,6 @@ DEFINE_INTERRUPT_HANDLER(__do_hash_fault)
> }
> }
>
> -/*
> - * The _RAW interrupt entry checks for the in_nmi() case before
> - * running the full handler.
> - */
> -DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
> -{
> - /*
> - * If we are in an "NMI" (e.g., an interrupt when soft-disabled), then
> - * don't call hash_page, just fail the fault. This is required to
> - * prevent re-entrancy problems in the hash code, namely perf
> - * interrupts hitting while something holds H_PAGE_BUSY, and taking a
> - * hash fault. See the comment in hash_preload().
> - *
> - * We come here as a result of a DSI at a point where we don't want
> - * to call hash_page, such as when we are accessing memory (possibly
> - * user memory) inside a PMU interrupt that occurred while interrupts
> - * were soft-disabled. We want to invoke the exception handler for
> - * the access, or panic if there isn't a handler.
> - */
> - if (unlikely(in_nmi())) {
> - do_bad_page_fault_segv(regs);
> - return 0;
> - }
> -
> - __do_hash_fault(regs);
> -
> - return 0;
> -}
> -
> #ifdef CONFIG_PPC_MM_SLICES
> static bool should_hash_preload(struct mm_struct *mm, unsigned long ea)
> {
> @@ -1776,26 +1746,18 @@ static void hash_preload(struct mm_struct *mm, pte_t *ptep, unsigned long ea,
> #endif /* CONFIG_PPC_64K_PAGES */
>
> /*
> - * __hash_page_* must run with interrupts off, as it sets the
> - * H_PAGE_BUSY bit. It's possible for perf interrupts to hit at any
> - * time and may take a hash fault reading the user stack, see
> - * read_user_stack_slow() in the powerpc/perf code.
> - *
> - * If that takes a hash fault on the same page as we lock here, it
> - * will bail out when seeing H_PAGE_BUSY set, and retry the access
> - * leading to an infinite loop.
> + * __hash_page_* must run with interrupts off, including PMI interrupts
> + * off, as it sets the H_PAGE_BUSY bit.
> *
> - * Disabling interrupts here does not prevent perf interrupts, but it
> - * will prevent them taking hash faults (see the NMI test in
> - * do_hash_page), then read_user_stack's copy_from_user_nofault will
> - * fail and perf will fall back to read_user_stack_slow(), which
> - * walks the Linux page tables.
> + * It's otherwise possible for perf interrupts to hit at any time and
> + * may take a hash fault reading the user stack, which could take a
> + * hash miss and deadlock on the same H_PAGE_BUSY bit.
> *
> * Interrupts must also be off for the duration of the
> * mm_is_thread_local test and update, to prevent preempt running the
> * mm on another CPU (XXX: this may be racy vs kthread_use_mm).
> */
> - local_irq_save(flags);
> + powerpc_local_irq_pmu_save(flags);
>
> /* Is that local to this CPU ? */
> if (mm_is_thread_local(mm))
> @@ -1820,7 +1782,7 @@ static void hash_preload(struct mm_struct *mm, pte_t *ptep, unsigned long ea,
> mm_ctx_user_psize(&mm->context),
> pte_val(*ptep));
>
> - local_irq_restore(flags);
> + powerpc_local_irq_pmu_restore(flags);
> }
>
> /*
> diff --git a/arch/powerpc/perf/callchain.h b/arch/powerpc/perf/callchain.h
> index d6fa6e25234f..19a8d051ddf1 100644
> --- a/arch/powerpc/perf/callchain.h
> +++ b/arch/powerpc/perf/callchain.h
> @@ -2,7 +2,6 @@
> #ifndef _POWERPC_PERF_CALLCHAIN_H
> #define _POWERPC_PERF_CALLCHAIN_H
>
> -int read_user_stack_slow(const void __user *ptr, void *buf, int nb);
> void perf_callchain_user_64(struct perf_callchain_entry_ctx *entry,
> struct pt_regs *regs);
> void perf_callchain_user_32(struct perf_callchain_entry_ctx *entry,
> @@ -26,17 +25,11 @@ static inline int __read_user_stack(const void __user *ptr, void *ret,
> size_t size)
> {
> unsigned long addr = (unsigned long)ptr;
> - int rc;
>
> if (addr > TASK_SIZE - size || (addr & (size - 1)))
> return -EFAULT;
>
> - rc = copy_from_user_nofault(ret, ptr, size);
> -
> - if (IS_ENABLED(CONFIG_PPC64) && !radix_enabled() && rc)
> - return read_user_stack_slow(ptr, ret, size);
> -
> - return rc;
> + return copy_from_user_nofault(ret, ptr, size);
> }
>
> #endif /* _POWERPC_PERF_CALLCHAIN_H */
> diff --git a/arch/powerpc/perf/callchain_64.c b/arch/powerpc/perf/callchain_64.c
> index 8d0df4226328..488e8a21a11e 100644
> --- a/arch/powerpc/perf/callchain_64.c
> +++ b/arch/powerpc/perf/callchain_64.c
> @@ -18,33 +18,6 @@
>
> #include "callchain.h"
>
> -/*
> - * On 64-bit we don't want to invoke hash_page on user addresses from
> - * interrupt context, so if the access faults, we read the page tables
> - * to find which page (if any) is mapped and access it directly. Radix
> - * has no need for this so it doesn't use read_user_stack_slow.
> - */
> -int read_user_stack_slow(const void __user *ptr, void *buf, int nb)
> -{
> -
> - unsigned long addr = (unsigned long) ptr;
> - unsigned long offset;
> - struct page *page;
> - void *kaddr;
> -
> - if (get_user_page_fast_only(addr, FOLL_WRITE, &page)) {
> - kaddr = page_address(page);
> -
> - /* align address to page boundary */
> - offset = addr & ~PAGE_MASK;
> -
> - memcpy(buf, kaddr + offset, nb);
> - put_page(page);
> - return 0;
> - }
> - return -EFAULT;
> -}
> -
> static int read_user_stack_64(const unsigned long __user *ptr, unsigned long *ret)
> {
> return __read_user_stack(ptr, ret, sizeof(*ret));
> --
> 2.23.0
^ permalink raw reply
* Re: [RFC V1 04/31] powerpc/mm: Enable ARCH_HAS_VM_GET_PAGE_PROT
From: Mike Rapoport @ 2022-02-04 4:44 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-kernel, hch, linux-mm, Paul Mackerras, akpm, linuxppc-dev
In-Reply-To: <46e15116-78fb-e6fe-e0f0-fe776f9348c3@arm.com>
On Fri, Feb 04, 2022 at 08:27:37AM +0530, Anshuman Khandual wrote:
>
> On 2/3/22 11:45 PM, Mike Rapoport wrote:
> > On Mon, Jan 24, 2022 at 06:26:41PM +0530, Anshuman Khandual wrote:
> >> This defines and exports a platform specific custom vm_get_page_prot() via
> >> subscribing ARCH_HAS_VM_GET_PAGE_PROT. Subsequently all __SXXX and __PXXX
> >> macros can be dropped which are no longer needed. While here, this also
> >> localizes arch_vm_get_page_prot() as powerpc_vm_get_page_prot().
> >>
> >> Cc: Michael Ellerman <mpe@ellerman.id.au>
> >> Cc: Paul Mackerras <paulus@samba.org>
> >> Cc: linuxppc-dev@lists.ozlabs.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >> arch/powerpc/Kconfig | 1 +
> >> arch/powerpc/include/asm/mman.h | 3 +-
> >> arch/powerpc/include/asm/pgtable.h | 19 ------------
> >> arch/powerpc/mm/mmap.c | 47 ++++++++++++++++++++++++++++++
> >> 4 files changed, 49 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >> index b779603978e1..ddb4a3687c05 100644
> >> --- a/arch/powerpc/Kconfig
> >> +++ b/arch/powerpc/Kconfig
> >> @@ -135,6 +135,7 @@ config PPC
> >> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> >> select ARCH_HAS_UACCESS_FLUSHCACHE
> >> select ARCH_HAS_UBSAN_SANITIZE_ALL
> >> + select ARCH_HAS_VM_GET_PAGE_PROT
> >> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> >> select ARCH_KEEP_MEMBLOCK
> >> select ARCH_MIGHT_HAVE_PC_PARPORT
> >> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
> >> index 7cb6d18f5cd6..7b10c2031e82 100644
> >> --- a/arch/powerpc/include/asm/mman.h
> >> +++ b/arch/powerpc/include/asm/mman.h
> >> @@ -24,7 +24,7 @@ static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
> >> }
> >> #define arch_calc_vm_prot_bits(prot, pkey) arch_calc_vm_prot_bits(prot, pkey)
> >>
> >> -static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
> >> +static inline pgprot_t powerpc_vm_get_page_prot(unsigned long vm_flags)
> >> {
> >> #ifdef CONFIG_PPC_MEM_KEYS
> >> return (vm_flags & VM_SAO) ?
> >> @@ -34,7 +34,6 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
> >> return (vm_flags & VM_SAO) ? __pgprot(_PAGE_SAO) : __pgprot(0);
> >> #endif
> >> }
> >> -#define arch_vm_get_page_prot(vm_flags) arch_vm_get_page_prot(vm_flags)
> >>
> >> static inline bool arch_validate_prot(unsigned long prot, unsigned long addr)
> >> {
> >> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> >> index d564d0ecd4cd..3cbb6de20f9d 100644
> >> --- a/arch/powerpc/include/asm/pgtable.h
> >> +++ b/arch/powerpc/include/asm/pgtable.h
> >> @@ -20,25 +20,6 @@ struct mm_struct;
> >> #include <asm/nohash/pgtable.h>
> >> #endif /* !CONFIG_PPC_BOOK3S */
> >>
> >> -/* Note due to the way vm flags are laid out, the bits are XWR */
> >> -#define __P000 PAGE_NONE
> >> -#define __P001 PAGE_READONLY
> >> -#define __P010 PAGE_COPY
> >> -#define __P011 PAGE_COPY
> >> -#define __P100 PAGE_READONLY_X
> >> -#define __P101 PAGE_READONLY_X
> >> -#define __P110 PAGE_COPY_X
> >> -#define __P111 PAGE_COPY_X
> >> -
> >> -#define __S000 PAGE_NONE
> >> -#define __S001 PAGE_READONLY
> >> -#define __S010 PAGE_SHARED
> >> -#define __S011 PAGE_SHARED
> >> -#define __S100 PAGE_READONLY_X
> >> -#define __S101 PAGE_READONLY_X
> >> -#define __S110 PAGE_SHARED_X
> >> -#define __S111 PAGE_SHARED_X
> >> -
> >> #ifndef __ASSEMBLY__
> >>
> >> #ifndef MAX_PTRS_PER_PGD
> >> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
> >> index c475cf810aa8..7f05e7903bd2 100644
> >> --- a/arch/powerpc/mm/mmap.c
> >> +++ b/arch/powerpc/mm/mmap.c
> >> @@ -254,3 +254,50 @@ void arch_pick_mmap_layout(struct mm_struct *mm, struct rlimit *rlim_stack)
> >> mm->get_unmapped_area = arch_get_unmapped_area_topdown;
> >> }
> >> }
> >> +
> >> +static inline pgprot_t __vm_get_page_prot(unsigned long vm_flags)
> >> +{
> >> + switch (vm_flags & (VM_READ | VM_WRITE | VM_EXEC | VM_SHARED)) {
> >> + case VM_NONE:
> >> + return PAGE_NONE;
> >> + case VM_READ:
> >> + return PAGE_READONLY;
> >> + case VM_WRITE:
> >> + return PAGE_COPY;
> >> + case VM_READ | VM_WRITE:
> >> + return PAGE_COPY;
> >> + case VM_EXEC:
> >> + return PAGE_READONLY_X;
> >> + case VM_EXEC | VM_READ:
> >> + return PAGE_READONLY_X;
> >> + case VM_EXEC | VM_WRITE:
> >> + return PAGE_COPY_X;
> >> + case VM_EXEC | VM_READ | VM_WRITE:
> >> + return PAGE_COPY_X;
> >> + case VM_SHARED:
> >> + return PAGE_NONE;
> >> + case VM_SHARED | VM_READ:
> >> + return PAGE_READONLY;
> >> + case VM_SHARED | VM_WRITE:
> >> + return PAGE_SHARED;
> >> + case VM_SHARED | VM_READ | VM_WRITE:
> >> + return PAGE_SHARED;
> >> + case VM_SHARED | VM_EXEC:
> >> + return PAGE_READONLY_X;
> >> + case VM_SHARED | VM_EXEC | VM_READ:
> >> + return PAGE_READONLY_X;
> >> + case VM_SHARED | VM_EXEC | VM_WRITE:
> >> + return PAGE_SHARED_X;
> >> + case VM_SHARED | VM_EXEC | VM_READ | VM_WRITE:
> >> + return PAGE_SHARED_X;
> >> + default:
> >> + BUILD_BUG();
> >> + }
> >> +}
> >> +
> >> +pgprot_t vm_get_page_prot(unsigned long vm_flags)
> >> +{
> >> + return __pgprot(pgprot_val(__vm_get_page_prot(vm_flags)) |
> >> + pgprot_val(powerpc_vm_get_page_prot(vm_flags)));
> > Any reason to keep powerpc_vm_get_page_prot() rather than open code it
> > here?
> >
> > This applies to other architectures that implement arch_vm_get_page_prot()
> > and/or arch_filter_pgprot() as well.
>
> Just to minimize the code churn ! But I will be happy to open code them
> here (and in other platforms) if that will be preferred.
I think this will be clearer because all the processing will be at one place.
Besides, this way include/asm/pgtable.h becomes shorter and less crowded.
--
Sincerely yours,
Mike.
^ permalink raw reply
* [powerpc/perf] WARN_ONCE arch/powerpc/include/asm/interrupt.h:365 with perf tests
From: Sachin Sant @ 2022-02-04 7:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Athira Rajeev, Madhavan Srinivasan, Nicholas Piggin
While running perftool [1] test against 5.17-rc2 booted on Power 9 LPAR
following warning is seen:
[ 442.002150] ------------[ cut here ]------------
[ 442.002164] WARNING: CPU: 7 PID: 76 at arch/powerpc/include/asm/interrupt.h:365 interrupt_nmi_exit_prepare+0x150/0x160
[ 442.002175] Modules linked in: dm_mod nft_compat nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables nfnetlink pseries_rng uio_pdrv_genirq uio nfsd auth_rpcgss nfs_acl lockd drm grace sunrpc sch_fq_codel drm_panel_orientation_quirks i2c_core xfs libcrc32c sr_mod sd_mod cdrom t10_pi xts ibmvscsi ibmveth scsi_transport_srp vmx_crypto fuse
[ 442.002216] CPU: 7 PID: 76 Comm: kworker/7:1 Kdump: loaded Not tainted 5.17.0-rc2-00167-gdcb85f85fa6f #1
[ 442.002222] Workqueue: events perf_sched_delayed
[ 442.002228] NIP: c00000000002b840 LR: c00000000002b818 CTR: c000000000343240
[ 442.002232] REGS: c00000002c08f460 TRAP: 0700 Not tainted (5.17.0-rc2-00167-gdcb85f85fa6f)
[ 442.002237] MSR: 8000000000021033 <SF,ME,IR,DR,RI,LE> CR: 88002484 XER: 000000cf
[ 442.002247] CFAR: c00000000002b7ac IRQMASK: 3
[ 442.002247] GPR00: c00000000002b818 c00000002c08f700 c000000002a1fe00 0000000000000000
[ 442.002247] GPR04: c00000002c08f758 0000000000000008 00000003fdcc0000 00000003fdcc0000
[ 442.002247] GPR08: 0000000000000001 c000000007fc6280 0000000000000021 000000003b9aca00
[ 442.002247] GPR12: 0000000000002000 c000000007fc6280 c0000000001925f8 c000000028ce0100
[ 442.002247] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 442.002247] GPR20: 0000000000000000 c000000002a53a00 c000000002b0a8a0 0000000000000000
[ 442.002247] GPR24: c000000002550de0 0000000000000001 c000000001059d60 0000000000000000
[ 442.002247] GPR28: 0000000060000000 c008000000100000 c00000002c08f810 c00000002c08f758
[ 442.002299] NIP [c00000000002b840] interrupt_nmi_exit_prepare+0x150/0x160
[ 442.002305] LR [c00000000002b818] interrupt_nmi_exit_prepare+0x128/0x160
[ 442.002310] Call Trace:
[ 442.002312] [c00000002c08f700] [c00000000013bcac] perf_event_interrupt+0x3c/0x70 (unreliable)
[ 442.002320] [c00000002c08f730] [c00000000002b8c4] performance_monitor_exception_nmi+0x74/0xb0
[ 442.002327] [c00000002c08f780] [c00000000002bd44] performance_monitor_exception+0x44/0x60
[ 442.002332] [c00000002c08f7a0] [c00000000000af28] performance_monitor_common_virt+0x208/0x210
[ 442.002339] --- interrupt: f00 at __patch_instruction+0x10/0x60
[ 442.002344] NIP: c0000000000b0bc0 LR: c0000000000b1080 CTR: 000000000000ae84
[ 442.002348] REGS: c00000002c08f810 TRAP: 0f00 Not tainted (5.17.0-rc2-00167-gdcb85f85fa6f)
[ 442.002352] MSR: 800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 44002484 XER: 000000cf
[ 442.002365] CFAR: 0000000000000000 IRQMASK: 1
[ 442.002365] GPR00: c0000000000b1114 c00000002c08fab0 c000000002a1fe00 c0000000001a59f8
[ 442.002365] GPR04: 0000000060000000 c0080000001059f8 8e011a00000000c0 ffffffffffffffff
[ 442.002365] GPR08: c0000000001a018e 0000000060000000 00000000000000c0 0000000000000040
[ 442.002365] GPR12: 0000000084002484 c000000007fc6280 c0000000001925f8 c000000028ce0100
[ 442.002365] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 442.002365] GPR20: 0000000000000000 c000000002a53a00 c000000002b0a8a0 0000000000000000
[ 442.002365] GPR24: c000000002550de0 0000000000000001 c000000001059d60 0000000000000000
[ 442.002365] GPR28: 0000000060000000 c008000000100000 0000000000000000 c0000000001a59f8
[ 442.002414] NIP [c0000000000b0bc0] __patch_instruction+0x10/0x60
[ 442.002419] LR [c0000000000b1080] patch_instruction+0xf0/0x1c0
[ 442.002423] --- interrupt: f00
[ 442.002426] [c00000002c08fab0] [c0000000000b1114] patch_instruction+0x184/0x1c0 (unreliable)
[ 442.002432] [c00000002c08fb00] [c00000000005dc48] arch_jump_label_transform+0x38/0x78
[ 442.002438] [c00000002c08fb20] [c00000000036a948] __jump_label_update+0x148/0x180
[ 442.002444] [c00000002c08fbc0] [c00000000036afd8] static_key_disable_cpuslocked+0xe8/0x130
[ 442.002450] [c00000002c08fc30] [c00000000036b050] static_key_disable+0x30/0x50
[ 442.002454] [c00000002c08fc60] [c000000000343978] perf_sched_delayed+0x98/0xc0
[ 442.002460] [c00000002c08fc90] [c0000000001849dc] process_one_work+0x27c/0x550
[ 442.002466] [c00000002c08fd30] [c000000000184d58] worker_thread+0xa8/0x620
[ 442.002472] [c00000002c08fdc0] [c000000000192710] kthread+0x120/0x130
[ 442.002477] [c00000002c08fe10] [c00000000000ce64] ret_from_kernel_thread+0x5c/0x64
[ 442.002482] Instruction dump:
[ 442.002486] e87e0100 480893a5 60000000 2c230000 4182ff58 39400000 f87e0100 b14d0930
[ 442.002494] 4bffff48 60000000 60000000 60420000 <0fe00000> 60000000 60000000 60420000
[ 442.002503] ---[ end trace 0000000000000000 ]---
[ 442.002511] Can't find PMC that caused IRQ
Since this warning is printed only once I can’t recreate it easily. I have seen this only
on P9 (hash MMU) and only with 5.17-rc1 onwards.
- Sachin
[1] - https://github.com/sacsant/avocado-misc-tests/blob/ci/perf/perftool.py
^ permalink raw reply
* powerpc: Set crashkernel offset to mid of RMA region
From: Sourabh Jain @ 2022-02-04 8:56 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 the 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.
This patch is dependent on changes to paca allocation for boot CPU. It
expect boot CPU to discover 1T segment support which is introduced by
the patch posted here:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239175.html
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reported-by: Abdul haleem <abdhalee@linux.vnet.ibm.com>
---
arch/powerpc/kernel/rtas.c | 6 ++++++
arch/powerpc/kexec/core.c | 15 +++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
---
Chnages in v4:
- fix build issue for 32-bit.
Changes in v3:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239371.html
---
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 733e6ef36758..1f42aabbbab3 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1313,6 +1313,12 @@ 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);
+#ifdef CONFIG_PPC64
+ /* 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;
+#endif
+
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
* Re: powerpc: Set crashkernel offset to mid of RMA region
From: Sourabh Jain @ 2022-02-04 9:00 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <d523c2d6-2a12-77aa-75a9-942cff5574bd@linux.ibm.com>
On 01/02/22 16:40, Hari Bathini wrote:
>
>
> On 28/01/22 3:34 pm, Sourabh Jain wrote:
>> 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
>>
>
> This dependency info must be captured somewhere within the changelog to
> be useful.
Added about the dependent patch in v4 patch commit message.
v4 patch link:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-February/239642.html
Thanks for the review Hari.
- Sourabh Jain
^ permalink raw reply
* Re: powerpc: Set crashkernel offset to mid of RMA region
From: Sourabh Jain @ 2022-02-04 9:14 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <87czk4mco0.fsf@mpe.ellerman.id.au>
On 03/02/22 16:37, Michael Ellerman wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>> On 01/02/22 17:14, Michael Ellerman wrote:
>>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>>> 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;
>>>> +
>>> As you'd have seen this breaks the 32-bit build. It will need an #ifdef
>>> CONFIG_PPC64 around it.
>>>
>>>> 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));
>>> I think this will break on machines using Radix won't it? At this point
>>> in boot ppc64_rma_size will be == 0. Because we won't call into
>>> hash__setup_initial_memory_limit().
>>>
>>> That's not changed by your patch, but seems like this code needs to be
>>> more careful/clever.
>> Interesting, but in my testing, I found that ppc64_rma_size
>> did get initialized before reserve_crashkernel() using radix on LPAR.
>>
>> I am not sure why but hash__setup_initial_memory_limit() function is
>> gets called
>> regardless of radix or hash. Not sure whether it is by design but here
>> is the flow:
> It sort of is by design. See:
>
> 103a8542cb35 ("powerpc/book3s64/radix: Fix boot failure with large amount of guest memory")
>
> Basically the hash restrictions are more strict, so we apply them until
> we know we will use radix.
>
> But ...
>
>> setup_initial_memory_limit()
>>
>> static inline void setup_initial_memory_limit()
>> (arch/powerpc/include/asm/book3s/64/mmu.h)
>>
>> if (!early_radix_enabled()) // FALSE regardless of radix is enabled or not
> You mean early_radix_enabled() is False regardless. But that's not true
> in all cases.
>
> We can now build the kernel without hash MMU support at all, see:
>
> 387e220a2e5e ("powerpc/64s: Move hash MMU support code under CONFIG_PPC_64S_HASH_MMU")
>
> In which case early_radix_enabled() will be true here, because it's hard
> coded to be true at build time.
Sorry, my bad I was not aware of that. But when both hash and radix
are enabled the early_radix_enabled return FALSE regardless it is disabled
using kernel command line or not is confusing to me. Maybe it is too early
in the boot sequence...
>
>> hash__setup_initial_memory_limit() // initialize ppc64_rma_size
>>
>> reserve_crashkernel() // initialize crashkernel offset to mid of RMA size.
>>
>>
>> For the sack of understanding even if we restrict crashkernel offset
>> setting to mid RMA (i.e. ppc64_rma_size/2) for
>> only hash it may not save radix because even today we are assigning
>> crashkernel offset using
>> ppc64_rma_size variable.
> Yes. There's already a bug there, your patch doesn't make it better or worse.
>
>> Is the current flow of initializing ppc64_rma_size variable before
>> reserve_crashkernel() for radix expected?
>>
>> Please provide your input.
> I wonder if we're better off moving the crash kernel reservation later,
> once we've discovered what MMU we're using.
>
> I can't immediately see why that would be a problem, as long as we do
> the reservation before we do any (many?) allocations. I'll have to think
> about it a bit more though, these boot ordering things are always
> subtle.
Agree we have space to improve this piece of code. Let me know
if I can help you to make this better.
> For now I think this patch is OK if you send a v2 to fix the compile
> error
I sent the next version in the mailing list. Thanks for the support.
- Sourabh Jain
^ permalink raw reply
* Re: [PATCH] powerpc: Fix xmon ml command to work with 64 bit values.
From: Christophe Leroy @ 2022-02-04 9:48 UTC (permalink / raw)
To: Rashmica Gupta, linuxppc-dev; +Cc: jwboyer
In-Reply-To: <1448406993-7888-1-git-send-email-rashmicy@gmail.com>
Le 25/11/2015 à 00:16, Rashmica Gupta a écrit :
> The ml command in xmon currently only works for 32-bit values and so fails
> to find 64-bit values on a ppc64 machine. So change to work for 64-bit
> values.
>
> This is based off a patch by Josh Boyer.
>
> Signed-off-by: Rashmica Gupta <rashmicy@gmail.com>
> ---
>
> Based off this patch: http://patchwork.ozlabs.org/patch/90309/
>
> arch/powerpc/xmon/xmon.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 786bf01691c9..df05bd2fca07 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -184,6 +184,12 @@ extern void xmon_leave(void);
> #define GETWORD(v) (((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
> #endif
>
> +#if BITS_PER_LONG == 64
> +#define GETLONG(v) (((unsigned long) GETWORD(v)) << 32 | GETWORD(v+4))
> +#else
> +#define GETLONG(v) GETWORD(v)
> +#endif
> +
memlocate() is the only place when GETWORD() is used. Shouldn't we just
replace GETWORD() by GETLONG() instead of doing a GETLONG() with GETWORD() ?
Also, can we use CONFIG_PPC64 instead of BITS_PER_LONG == 64 ?
> static char *help_string = "\
> Commands:\n\
> b show breakpoints\n\
> @@ -2447,14 +2453,15 @@ memdiffs(unsigned char *p1, unsigned char *p2, unsigned nb, unsigned maxpr)
> printf("Total of %d differences\n", prt);
> }
>
> -static unsigned mend;
> -static unsigned mask;
> +static unsigned long mend;
> +static unsigned long mask;
>
> static void
> memlocate(void)
> {
> - unsigned a, n;
> - unsigned char val[4];
> + unsigned long a, n;
> + int size = sizeof(unsigned long);
> + unsigned char val[size];
>
> last_cmd = "ml";
> scanhex((void *)&mdest);
> @@ -2470,10 +2477,10 @@ memlocate(void)
> }
> }
> n = 0;
> - for (a = mdest; a < mend; a += 4) {
> - if (mread(a, val, 4) == 4
> - && ((GETWORD(val) ^ mval) & mask) == 0) {
> - printf("%.16x: %.16x\n", a, GETWORD(val));
> + for (a = mdest; a < mend; a += size) {
> + if (mread(a, val, size) == size
> + && ((GETLONG(val) ^ mval) & mask) == 0){
> + printf("%.16lx: %.16lx\n", a, GETLONG(val));
> if (++n >= 10)
> break;
> }
^ 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