Re: [PATCH] arch: powerpc: kernel: fixed some typos
On Thu, Dec 21, 2023 at 4:55 PM Michael Ellerman wrote: > > Ghanshyam Agrawal writes: > > Fixed some typos > > > > Signed-off-by: Ghanshyam Agrawal > > --- > > arch/powerpc/kernel/eeh_pe.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > Please also fix the case in arch/powerpc/include/asm/eeh.h > > The subject should use the correct prefix. You can see what it should be > using: > > $ git log --oneline arch/powerpc/kernel/eeh_pe.c > > Please give the patch a better subject, not "some typos", tell me what > misspelling you're fixing. Same comment for the commit description. > > > diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c > > index e0ce81279624..8e0c1a8b8641 100644 > > --- a/arch/powerpc/kernel/eeh_pe.c > > +++ b/arch/powerpc/kernel/eeh_pe.c > > @@ -24,10 +24,10 @@ static int eeh_pe_aux_size = 0; > > static LIST_HEAD(eeh_phb_pe); > > > > /** > > - * eeh_set_pe_aux_size - Set PE auxillary data size > > - * @size: PE auxillary data size > > + * eeh_set_pe_aux_size - Set PE auxiliary data size > > + * @size: PE auxiliary data size > > While you're changing it you could also mention what the units of the > size are. > > > * > > - * Set PE auxillary data size > > + * Set PE auxiliary data size > > This should gain a full stop at the end of the sentence. > > > */ > > void eeh_set_pe_aux_size(int size) > > { > > -- > > 2.25.1 > > > cheers Hi Michael, Thank you very much for your suggestions. I will implement them and send a v2 patch. You mentioned I need to specify the units of "PE auxiliary data size". Is the unit BYTES? Sorry for the silly question, I am only beginning to contribute to the linux kernel. Thanks & Regards, Ghanshyam Agrawal
Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time
On December 21, 2023 4:16:56 AM PST, Michael Ellerman wrote: >Cc +Kees > >Christophe Leroy writes: >> Declaring rodata_enabled and mark_rodata_ro() at all time >> helps removing related #ifdefery in C files. >> >> Signed-off-by: Christophe Leroy >> --- >> include/linux/init.h | 4 >> init/main.c | 21 +++-- >> 2 files changed, 7 insertions(+), 18 deletions(-) >> >> diff --git a/include/linux/init.h b/include/linux/init.h >> index 01b52c9c7526..d2b47be38a07 100644 >> --- a/include/linux/init.h >> +++ b/include/linux/init.h >> @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; >> >> extern struct file_system_type rootfs_fs_type; >> >> -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) >> extern bool rodata_enabled; >> -#endif >> -#ifdef CONFIG_STRICT_KERNEL_RWX >> void mark_rodata_ro(void); >> -#endif >> >> extern void (*late_time_init)(void); >> >> diff --git a/init/main.c b/init/main.c >> index e24b0780fdff..807df08c501f 100644 >> --- a/init/main.c >> +++ b/init/main.c >> @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) >> early_param("rodata", set_debug_rodata); >> #endif >> >> -#ifdef CONFIG_STRICT_KERNEL_RWX >> static void mark_readonly(void) >> { >> -if (rodata_enabled) { >> +if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { I think this will break without rodata_enabled actual existing on other architectures. (Only declaration was made visible, not the definition, which is above here and still behind ifdefs?) -Kees >> /* >> * load_module() results in W+X mappings, which are cleaned >> * up with call_rcu(). Let's make sure that queued work is >> @@ -1409,20 +1408,14 @@ static void mark_readonly(void) >> rcu_barrier(); >> mark_rodata_ro(); >> rodata_test(); >> -} else >> +} else if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { >> pr_info("Kernel memory protection disabled.\n"); >> +} else if (IS_ENABLED(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)) { >> +pr_warn("Kernel memory protection not selected by kernel >> config.\n"); >> +} else { >> +pr_warn("This architecture does not have kernel memory >> protection.\n"); >> +} >> } >> -#elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX) >> -static inline void mark_readonly(void) >> -{ >> -pr_warn("Kernel memory protection not selected by kernel config.\n"); >> -} >> -#else >> -static inline void mark_readonly(void) >> -{ >> -pr_warn("This architecture does not have kernel memory protection.\n"); >> -} >> -#endif >> >> void __weak free_initmem(void) >> { >> -- >> 2.41.0 -- Kees Cook
Re: [PATCH 00/13] mm/gup: Unify hugetlb, part 2
Copy Muchun, which I forgot since the start, sorry. -- Peter Xu
Re: [PATCH] powerpc/6xx: set High BAT Enable flag on G2 cores
Matthias Schiffer writes: > MMU_FTR_USE_HIGH_BATS is set for G2-based cores (G2_LE, e300cX), but the > high BATs need to be enabled in HID2 to work. Add register definitions > and introduce a G2 variant of __setup_cpu_603. > > This fixes boot on CPUs like the MPC5200B with STRICT_KERNEL_RWX enabled. Nice find. Minor nit on naming. The 32-bit code mostly uses the numeric names, eg. 603, 603e, 604 etc. Can we stick with that, rather than using "G2"? Wikipedia says G2 == 603e. But looking at your patch you're not changing all the 603e cores, so I guess it's not that clear cut? If using "G2" makes the most sense then it would be nice to update Documentation/arch/powerpc/cpu_families.rst to mention it (not asking you to do it necessarily, more a note for us). cheers
Re: [PATCH v14 3/6] crash: add a new kexec flag for FDT update
On 12/21/23 at 11:36am, Sourabh Jain wrote: .. > > > > diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h > > > > index 3d5b3d757bed..df6a6505e267 100644 > > > > --- a/include/uapi/linux/kexec.h > > > > +++ b/include/uapi/linux/kexec.h > > > > @@ -13,7 +13,7 @@ > > > >#define KEXEC_ON_CRASH 0x0001 > > > >#define KEXEC_PRESERVE_CONTEXT 0x0002 > > > > -#define KEXEC_UPDATE_FDT 0x0008 > > > > +#define KEXEC_CRASH_HOTPLUG_UPDATE 0x0004 > > > >#define KEXEC_UPDATE_ELFCOREHDR0x0004 > > > >#define KEXEC_ARCH_MASK0x > > > >/* > > > > > > > > With my understanding, the kexec flag should be indicating the action, > > > > the mem/cpu hotplug, but not relating to any detail. Imagine later > > > > another segment need be skipped on one ARCH again, then another flag > > > > need be added, this sounds not reasonable. > > > I strongly agree with you. The KEXEC_CRASH_HOTPLUG_UPDATE kexec flag > > > should be sufficient to inform the kernel that the kexec tool has been > > > updated > > > to support CPU/Memory hotplug for the kexec_load system call. > > > Unfortunately, > > > we cannot use the 0x0004 kexec flags bit for > > > KEXEC_CRASH_HOTPLUG_UPDATE > > > at the moment. > > I am fine with 0x0008 and a new flag, it has the same effect as > > #define KEXEC_CRASH_HOTPLUG_UPDATE 0x0004 > > > > I am worried about the header file incompatiblity. > > If we are OK to have KEXEC_CRASH_HOTPLUG_UPDATE 0x0008 as new bit > to introduce CPU/Memory hotplug feature for kexec_load syscall, we will not > have > compatibility issue. > > Let me write next version for this patch with KEXEC_CRASH_HOTPLUG_UPDATE > 0x0008 > as new flag bit and show how it will be handled. I will also share kexec > code for clarity. It's great we are in the same page about segments excluding done in arch function. While It's a little unclear to me why we can't reuse 0x0004 flag value. Then KEXEC_UPDATE_ELFCOREHDR will only exist in v6.6 kernel, and that bit won't be used in v6.7 and future version. Except of the existence in kexec-tools utility for XEN, do you see other barrier? I would like to know so that one day I can explain KEXEC_UPDATE_ELFCOREHDR to someone else if asked.
Re: [PATCH] powerpc: Split PAGE_SHIFT/SIZE into vdso/page.h
Linus Walleij writes: > On Thu, Dec 21, 2023 at 1:04 PM Michael Ellerman wrote: > >> The VDSO needs PAGE_SHIFT/SIZE defined, so it includes asm/page.h. >> >> However when COMPAT=y the VDSO is built 32-bit, even though the kernel >> is 64-bit. That can lead to odd warnings because some kernel constants >> are 64-bit, but unsigned long is 32-bit, for example: >> >> VDSO32C arch/powerpc/kernel/vdso/vgettimeofday-32.o >> In file included from :4: >> In file included from /home/michael/linux/lib/vdso/gettimeofday.c:5: >> In file included from ../include/vdso/datapage.h:137: >> In file included from ../arch/powerpc/include/asm/vdso/gettimeofday.h:7: >> ../arch/powerpc/include/asm/page.h:230:9: warning: result of comparison of >> constant 13835058055282163712 with expression of type 'unsigned long' is >> always true >> 230 | return __pa(kaddr) >> PAGE_SHIFT; >> |^~~ >> >> Fix it by moving the PAGE_SHIFT/SIZE constants into a separate header, >> which can be included by the VDSO, and also by the existing kernel >> headers. That avoids exposing the rest of the header to the non-standard >> build environment of the compat VDSO. >> >> The particular warning above was introduced by commit 58b6fed89ab0 >> ("powerpc: Make virt_to_pfn() a static inline"), though it is not at >> fault, it just exposed the fact that the VDSO was including parts of >> page.h that weren't needed or appropriate for the VDSO. >> >> Don't copy the comment about page sizes, it just risks becoming >> outdated, that information is better available in the Kconfig >> dependencies and help text. >> >> Fixes: 58b6fed89ab0 ("powerpc: Make virt_to_pfn() a static inline") >> Reported-by: kernel test robot >> Closes: >> https://lore.kernel.org/oe-kbuild-all/202311061940.4pbrm44u-...@intel.com/ >> Signed-off-by: Michael Ellerman > > Clearly a working solution! > Acked-by: Linus Walleij Thanks. > Just a note: > >> diff --git a/arch/powerpc/include/asm/page.h >> b/arch/powerpc/include/asm/page.h > (...) >> +#include > > (...) >> +++ b/arch/powerpc/include/asm/vdso/page.h >> @@ -0,0 +1,10 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> +#ifndef _ASM_POWERPC_VDSO_PAGE_H >> +#define _ASM_POWERPC_VDSO_PAGE_H >> + >> +#include > > Now the whole kernel includes , is this necessary? It's already included by approximately the whole kernel via: include/linux/kernel.h - include/uapi/linux/kernel.h - include/linux/const.h - include/vdso/const.h And arch/powerpc/include/asm/page.h already includes linux/kernel.h, so includers of page.h should not see any new headers other than asm/vdso/page.h. cheers
Re: [PATCH] perf vendor events: Remove UTF-8 characters from cmn.json
Em Thu, Dec 21, 2023 at 02:03:13PM +0800, Jing Zhang escreveu: > cmn.json contains UTF-8 characters in brief description which > could break the perf build on some distros. > > Fix this issue by removing the UTF-8 characters from cmn.json. > > without the fix: > $find tools/perf/pmu-events/ -name "*.json" | xargs file -i | grep -v us-ascii > tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json: > application/json; charset=utf-8 > > with the fix: > $file -i tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json > tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json: text/plain; > charset=us-ascii Thanks, applied! - Arnaldo > Fixes: 0b4de7bdf46c5215 ("perf jevents: Add support for Arm CMN PMU aliasing") > Reported-by: Arnaldo Carvalho de Melo > Signed-off-by: Jing Zhang > --- > tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json > b/tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json > index 428605c..5ec157c 100644 > --- a/tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json > +++ b/tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json > @@ -107,7 +107,7 @@ > "EventName": "hnf_qos_hh_retry", > "EventidCode": "0xe", > "NodeType": "0x5", > - "BriefDescription": "Counts number of times a HighHigh priority > request is protocolretried at the HN‑F.", > + "BriefDescription": "Counts number of times a HighHigh priority > request is protocolretried at the HN-F.", > "Unit": "arm_cmn", > "Compat": "(434|436|43c|43a).*" > }, > -- > 1.8.3.1 > -- - Arnaldo
Re: [RFC PATCH 6/9] powerpc/ftrace: Update and move function profile instructions out-of-line
Le 21/12/2023 à 15:25, Steven Rostedt a écrit : > On Thu, 21 Dec 2023 10:46:08 + > Christophe Leroy wrote: > >>> To enable ftrace, the nop at function entry is changed to an >>> unconditional branch to 'tramp'. The call to ftrace_caller() may be >>> updated to ftrace_regs_caller() depending on the registered ftrace ops. >>> On 64-bit powerpc, we additionally change the instruction at 'tramp' to >>> 'mflr r0' from an unconditional branch back to func+4. This is so that >>> functions entered through the GEP can skip the function profile sequence >>> unless ftrace is enabled. >>> >>> With the context_switch microbenchmark on a P9 machine, there is a >>> performance improvement of ~6% with this patch applied, going from 650k >>> context switches to 690k context switches without ftrace enabled. With >>> ftrace enabled, the performance was similar at 86k context switches. >> >> Wondering how significant that context_switch micorbenchmark is. >> >> I ran it on both mpc885 and mpc8321 and I'm a bit puzzled by some of the >> results: >> # ./context_switch --no-fp >> Using threads with yield on cpus 0/0 touching FP:no altivec:no vector:no >> vdso:no >> >> On 885, I get the following results before and after your patch. >> >> CONFIG_FTRACE not selected : 44,9k >> CONFIG_FTRACE selected, before : 32,8k >> CONFIG_FTRACE selected, after : 33,6k >> >> All this is with CONFIG_INIT_STACK_ALL_ZERO which is the default. But >> when I select CONFIG_INIT_STACK_NONE, the CONFIG_FTRACE not selected >> result is only 34,4. >> >> On 8321: >> >> CONFIG_FTRACE not selected : 100,3k >> CONFIG_FTRACE selected, before : 72,5k >> CONFIG_FTRACE selected, after : 116k >> >> So the results look odd to me. > > > BTW, CONFIG_FTRACE just enables the tracing system (I would like to change > that to CONFIG_TRACING, but not sure if I can without breaking .configs all > over the place). > > The nops for ftrace is enabled with CONFIG_FUNCTION_TRACER. Yes I selected both CONFIG_FTRACE and CONFIG_FUNCTION_TRACER Christophe
Re: [PATCH v4 00/10] devm_led_classdev_register() usage problem
On Thu, 14 Dec 2023, George Stark wrote: > This patch series fixes the problem of devm_led_classdev_register misusing. > > The basic problem is described in [1]. Shortly when > devm_led_classdev_register() > is used then led_classdev_unregister() called after driver's remove() > callback. > led_classdev_unregister() calls driver's brightness_set callback and that > callback > may use resources which were destroyed already in driver's remove(). > > After discussion with maintainers [2] [3] we decided: > 1) don't touch led subsytem core code and don't remove led_set_brightness() > from it > but fix drivers > 2) don't use devm_led_classdev_unregister > > So the solution is to use devm wrappers for all resources > driver's brightness_set() depends on. And introduce dedicated devm wrapper > for mutex as it's often used resource. > > [1] > https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895...@salutedevices.com/T/ > [2] > https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895...@salutedevices.com/T/#mc132b9b350fa51931b4fcfe14705d9f06e91421f > [3] > https://lore.kernel.org/lkml/8704539b-ed3b-44e6-aa82-586e2f895...@salutedevices.com/T/#mdbf572a85c33f869a553caf986b6228bb65c8383 > > Changelog: > v1->v2: > revise patch series completely > > v2->v3: > locking: add define if mutex_destroy() is not an empty function > new patch, discussed here [8] > > devm-helpers: introduce devm_mutex_init > previous version [4] > - revise code based on mutex_destroy define > - update commit message > - update devm_mutex_init()'s description > > leds: aw2013: unlock mutex before destroying it > previous version [5] > - make this patch first in the series > - add tags Fixes and RvB by Andy > > leds: aw2013: use devm API to cleanup module's resources > previous version [6] > - make aw2013_chip_disable_action()'s body oneline > - don't shadow devm_mutex_init() return code > > leds: aw200xx: use devm API to cleanup module's resources > previous version [7] > - make aw200xx_*_action()'s bodies oneline > - don't shadow devm_mutex_init() return code > > leds: lm3532: use devm API to cleanup module's resources > leds: nic78bx: use devm API to cleanup module's resources > leds: mlxreg: use devm_mutex_init for mutex initializtion > leds: an30259a: use devm_mutext_init for mutext initialization > leds: powernv: add LED_RETAIN_AT_SHUTDOWN flag for leds > - those pathes were planned but not sent in the series #2 due to mail > server > problem on my side. I revised them according to the comments. > > v3->v4: > locking: introduce devm_mutex_init > new patch > - move devm_mutex_init implementation completely from devm-helpers.h to > mutex.h > > locking: add define if mutex_destroy() is not an empty function > drop the patch [9] > > devm-helpers: introduce devm_mutex_init > drop the patch [10] > > leds: aw2013: use devm API to cleanup module's resources > - add tag Tested-by: Nikita Travkin > > [4] > https://lore.kernel.org/lkml/20231204180603.470421-1-gnst...@salutedevices.com/T/#mf500af0eda2a9ffc95594607dbe4cb64f2e3c9a8 > [5] > https://lore.kernel.org/lkml/20231204180603.470421-1-gnst...@salutedevices.com/T/#mc92df4fb4f7d4187fb01cc1144acfa5fb5230dd2 > [6] > https://lore.kernel.org/lkml/20231204180603.470421-1-gnst...@salutedevices.com/T/#m300df89710c43cc2ab598baa16c68dd0a0d7d681 > [7] > https://lore.kernel.org/lkml/20231204180603.470421-1-gnst...@salutedevices.com/T/#m8e5c65e0c6b137c91fa00bb9320ad581164d1d0b > [8] > https://lore.kernel.org/lkml/377e4437-7051-4d88-ae68-1460bcd69...@redhat.com/T/#m5f84a4a2f387d49678783e652b9e658e02c27450 > [9] > https://lore.kernel.org/lkml/20231213223020.2713164-1-gnst...@salutedevices.com/T/#m19ad1fc04c560012c1e27418e3156d0c9306dd84 > [10] > https://lore.kernel.org/lkml/20231213223020.2713164-1-gnst...@salutedevices.com/T/#m63126025f5d1bdcef69bcad50f2e58274d42e2d7 > > George Stark (10): > leds: aw2013: unlock mutex before destroying it > locking: introduce devm_mutex_init > leds: aw2013: use devm API to cleanup module's resources > leds: aw200xx: use devm API to cleanup module's resources > leds: lp3952: use devm API to cleanup module's resources > leds: lm3532: use devm API to cleanup module's resources > leds: nic78bx: use devm API to cleanup module's resources > leds: mlxreg: use devm_mutex_init for mutex initializtion > leds: an30259a: use devm_mutext_init for mutext initialization > leds: powernv: use LED_RETAIN_AT_SHUTDOWN flag for leds > > drivers/leds/leds-an30259a.c | 15 +-- > drivers/leds/leds-aw200xx.c | 33 ++--- > drivers/leds/leds-aw2013.c | 27 +++ > drivers/leds/leds-lm3532.c | 30 ++ > drivers/leds/leds-lp3952.c | 21 +++-- > drivers/leds/leds-mlxreg.c | 17 ++--- > d
Re: [RFC PATCH 6/9] powerpc/ftrace: Update and move function profile instructions out-of-line
On Thu, 21 Dec 2023 10:46:08 + Christophe Leroy wrote: > > To enable ftrace, the nop at function entry is changed to an > > unconditional branch to 'tramp'. The call to ftrace_caller() may be > > updated to ftrace_regs_caller() depending on the registered ftrace ops. > > On 64-bit powerpc, we additionally change the instruction at 'tramp' to > > 'mflr r0' from an unconditional branch back to func+4. This is so that > > functions entered through the GEP can skip the function profile sequence > > unless ftrace is enabled. > > > > With the context_switch microbenchmark on a P9 machine, there is a > > performance improvement of ~6% with this patch applied, going from 650k > > context switches to 690k context switches without ftrace enabled. With > > ftrace enabled, the performance was similar at 86k context switches. > > Wondering how significant that context_switch micorbenchmark is. > > I ran it on both mpc885 and mpc8321 and I'm a bit puzzled by some of the > results: > # ./context_switch --no-fp > Using threads with yield on cpus 0/0 touching FP:no altivec:no vector:no > vdso:no > > On 885, I get the following results before and after your patch. > > CONFIG_FTRACE not selected : 44,9k > CONFIG_FTRACE selected, before : 32,8k > CONFIG_FTRACE selected, after : 33,6k > > All this is with CONFIG_INIT_STACK_ALL_ZERO which is the default. But > when I select CONFIG_INIT_STACK_NONE, the CONFIG_FTRACE not selected > result is only 34,4. > > On 8321: > > CONFIG_FTRACE not selected : 100,3k > CONFIG_FTRACE selected, before : 72,5k > CONFIG_FTRACE selected, after : 116k > > So the results look odd to me. BTW, CONFIG_FTRACE just enables the tracing system (I would like to change that to CONFIG_TRACING, but not sure if I can without breaking .configs all over the place). The nops for ftrace is enabled with CONFIG_FUNCTION_TRACER. -- Steve
Re: [PATCH] powerpc/6xx: set High BAT Enable flag on G2 cores
Le 21/12/2023 à 13:45, Matthias Schiffer a écrit : > MMU_FTR_USE_HIGH_BATS is set for G2-based cores (G2_LE, e300cX), but the > high BATs need to be enabled in HID2 to work. Add register definitions > and introduce a G2 variant of __setup_cpu_603. Well spotted. I have a mpc8321, hence e300c2. I never had the problem you had. But ... looks like U-boot configuration has CONFIG_HID2_HBE so that's set by U-boot indeed, that's the reason why I never had that problem. > > This fixes boot on CPUs like the MPC5200B with STRICT_KERNEL_RWX enabled. > > Fixes: e4d6654ebe6e ("powerpc/mm/32s: rework mmu_mapin_ram()") > Signed-off-by: Matthias Schiffer > --- > arch/powerpc/include/asm/cpu_setup.h | 1 + > arch/powerpc/include/asm/reg.h| 2 ++ > arch/powerpc/kernel/cpu_setup_6xx.S | 25 ++- > arch/powerpc/kernel/cpu_specs_book3s_32.h | 10 - > 4 files changed, 32 insertions(+), 6 deletions(-) > > > I have only tested this on the MPC5200B (G2_LE), but according to the > e300 manual, e300cX cores should behave the same. > > The Fixes tag is the best I could come up with - I believe that the > underlying issue of setting USE_HIGH_BATS without actually enabling them > is as old as Linux's PowerPC implementation, but the specific code > causing the boot failure was added in the mentioned commit. Agreed, before that only BATs 0 to 3 were used anyway. There was also BAT 4 used by platforms/embedded6xx/wii.c , but that's probably not a G2 ? > > Another issue I found in the code is that > arch/powerpc/platforms/52xx/lite5200_sleep.S uses the SPRN_HID2 definition > which does not refer to HID2 on the 5200... but that will be for someone > else to fix, if there is still anyone left using that platform. Maybe file an issue for it at https://github.com/linuxppc/issues/issues if you don't plan to fix it ? By the way, it looks like the SPRN_HID2 definition we have is very specific to the IBM 750. MPC 750 has SPRN_HID2 as 1011 == 0x3f3 like others. > > > diff --git a/arch/powerpc/include/asm/cpu_setup.h > b/arch/powerpc/include/asm/cpu_setup.h > index 30e2fe3895024..68d804e74d221 100644 > --- a/arch/powerpc/include/asm/cpu_setup.h > +++ b/arch/powerpc/include/asm/cpu_setup.h > @@ -35,6 +35,7 @@ void __setup_cpu_750fx(unsigned long offset, struct > cpu_spec *spec); > void __setup_cpu_7400(unsigned long offset, struct cpu_spec *spec); > void __setup_cpu_7410(unsigned long offset, struct cpu_spec *spec); > void __setup_cpu_745x(unsigned long offset, struct cpu_spec *spec); > +void __setup_cpu_g2(unsigned long offset, struct cpu_spec *spec); > > void __setup_cpu_ppc970(unsigned long offset, struct cpu_spec *spec); > void __setup_cpu_ppc970MP(unsigned long offset, struct cpu_spec *spec); > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h > index 4ae4ab9090a2d..f5641fcd1da85 100644 > --- a/arch/powerpc/include/asm/reg.h > +++ b/arch/powerpc/include/asm/reg.h > @@ -617,6 +617,8 @@ > #endif > #define SPRN_HID2 0x3F8 /* Hardware Implementation Register > 2 */ Should that HID2 be renamed to SPRN_HID2_750 to avoid confusion ? > #define SPRN_HID2_GEKKO0x398 /* Gekko HID2 Register */ > +#define SPRN_HID2_G2 0x3F3 /* G2 HID2 Register */ > +#define HID2_HBE_G2 (1<<18) /* High BAT Enable (G2) */ > #define SPRN_IABR 0x3F2 /* Instruction Address Breakpoint Register */ > #define SPRN_IABR2 0x3FA /* 83xx */ > #define SPRN_IBCR 0x135 /* 83xx Insn Breakpoint Control Reg > */ > diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S > b/arch/powerpc/kernel/cpu_setup_6xx.S > index f29ce3dd6140f..c67d32e04df9c 100644 > --- a/arch/powerpc/kernel/cpu_setup_6xx.S > +++ b/arch/powerpc/kernel/cpu_setup_6xx.S > @@ -81,6 +81,20 @@ _GLOBAL(__setup_cpu_745x) > bl setup_745x_specifics > mtlrr5 > blr > +_GLOBAL(__setup_cpu_g2) > + mflrr5 > +BEGIN_MMU_FTR_SECTION > + li r10,0 > + mtspr SPRN_SPRG_603_LRU,r10 /* init SW LRU tracking */ > +END_MMU_FTR_SECTION_IFSET(MMU_FTR_NEED_DTLB_SW_LRU) MMU_FTR_NEED_DTLB_SW_LRU is dedicated to e300 core. You should also remove it from __setup_cpu_603 > + > +BEGIN_FTR_SECTION > + bl __init_fpu_registers > +END_FTR_SECTION_IFCLR(CPU_FTR_FPU_UNAVAILABLE) > + bl setup_common_caches > + bl setup_g2_hid2 > + mtlrr5 > + blr > > /* Enable caches for 603's, 604, 750 & 7400 */ > SYM_FUNC_START_LOCAL(setup_common_caches) > @@ -115,6 +129,16 @@ SYM_FUNC_START_LOCAL(setup_604_hid0) > blr > SYM_FUNC_END(setup_604_hid0) > > +/* Enable high BATs for G2 (G2_LE, e300cX) */ > +SYM_FUNC_START_LOCAL(setup_g2_hid2) > + mfspr r11,SPRN_HID2_G2 > + orisr11,r11,HID2_HBE_G2@h > + mtspr SPRN_HID2_G2,r11 > + sync > + isync > + blr > +SYM_FUNC_END(setup_g2_hid
[PATCH] powerpc/6xx: set High BAT Enable flag on G2 cores
MMU_FTR_USE_HIGH_BATS is set for G2-based cores (G2_LE, e300cX), but the high BATs need to be enabled in HID2 to work. Add register definitions and introduce a G2 variant of __setup_cpu_603. This fixes boot on CPUs like the MPC5200B with STRICT_KERNEL_RWX enabled. Fixes: e4d6654ebe6e ("powerpc/mm/32s: rework mmu_mapin_ram()") Signed-off-by: Matthias Schiffer --- arch/powerpc/include/asm/cpu_setup.h | 1 + arch/powerpc/include/asm/reg.h| 2 ++ arch/powerpc/kernel/cpu_setup_6xx.S | 25 ++- arch/powerpc/kernel/cpu_specs_book3s_32.h | 10 - 4 files changed, 32 insertions(+), 6 deletions(-) I have only tested this on the MPC5200B (G2_LE), but according to the e300 manual, e300cX cores should behave the same. The Fixes tag is the best I could come up with - I believe that the underlying issue of setting USE_HIGH_BATS without actually enabling them is as old as Linux's PowerPC implementation, but the specific code causing the boot failure was added in the mentioned commit. Another issue I found in the code is that arch/powerpc/platforms/52xx/lite5200_sleep.S uses the SPRN_HID2 definition which does not refer to HID2 on the 5200... but that will be for someone else to fix, if there is still anyone left using that platform. diff --git a/arch/powerpc/include/asm/cpu_setup.h b/arch/powerpc/include/asm/cpu_setup.h index 30e2fe3895024..68d804e74d221 100644 --- a/arch/powerpc/include/asm/cpu_setup.h +++ b/arch/powerpc/include/asm/cpu_setup.h @@ -35,6 +35,7 @@ void __setup_cpu_750fx(unsigned long offset, struct cpu_spec *spec); void __setup_cpu_7400(unsigned long offset, struct cpu_spec *spec); void __setup_cpu_7410(unsigned long offset, struct cpu_spec *spec); void __setup_cpu_745x(unsigned long offset, struct cpu_spec *spec); +void __setup_cpu_g2(unsigned long offset, struct cpu_spec *spec); void __setup_cpu_ppc970(unsigned long offset, struct cpu_spec *spec); void __setup_cpu_ppc970MP(unsigned long offset, struct cpu_spec *spec); diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 4ae4ab9090a2d..f5641fcd1da85 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -617,6 +617,8 @@ #endif #define SPRN_HID2 0x3F8 /* Hardware Implementation Register 2 */ #define SPRN_HID2_GEKKO0x398 /* Gekko HID2 Register */ +#define SPRN_HID2_G2 0x3F3 /* G2 HID2 Register */ +#define HID2_HBE_G2 (1<<18) /* High BAT Enable (G2) */ #define SPRN_IABR 0x3F2 /* Instruction Address Breakpoint Register */ #define SPRN_IABR2 0x3FA /* 83xx */ #define SPRN_IBCR 0x135 /* 83xx Insn Breakpoint Control Reg */ diff --git a/arch/powerpc/kernel/cpu_setup_6xx.S b/arch/powerpc/kernel/cpu_setup_6xx.S index f29ce3dd6140f..c67d32e04df9c 100644 --- a/arch/powerpc/kernel/cpu_setup_6xx.S +++ b/arch/powerpc/kernel/cpu_setup_6xx.S @@ -81,6 +81,20 @@ _GLOBAL(__setup_cpu_745x) bl setup_745x_specifics mtlrr5 blr +_GLOBAL(__setup_cpu_g2) + mflrr5 +BEGIN_MMU_FTR_SECTION + li r10,0 + mtspr SPRN_SPRG_603_LRU,r10 /* init SW LRU tracking */ +END_MMU_FTR_SECTION_IFSET(MMU_FTR_NEED_DTLB_SW_LRU) + +BEGIN_FTR_SECTION + bl __init_fpu_registers +END_FTR_SECTION_IFCLR(CPU_FTR_FPU_UNAVAILABLE) + bl setup_common_caches + bl setup_g2_hid2 + mtlrr5 + blr /* Enable caches for 603's, 604, 750 & 7400 */ SYM_FUNC_START_LOCAL(setup_common_caches) @@ -115,6 +129,16 @@ SYM_FUNC_START_LOCAL(setup_604_hid0) blr SYM_FUNC_END(setup_604_hid0) +/* Enable high BATs for G2 (G2_LE, e300cX) */ +SYM_FUNC_START_LOCAL(setup_g2_hid2) + mfspr r11,SPRN_HID2_G2 + orisr11,r11,HID2_HBE_G2@h + mtspr SPRN_HID2_G2,r11 + sync + isync + blr +SYM_FUNC_END(setup_g2_hid2) + /* 7400 <= rev 2.7 and 7410 rev = 1.0 suffer from some * erratas we work around here. * Moto MPC710CE.pdf describes them, those are errata @@ -495,4 +519,3 @@ _GLOBAL(__restore_cpu_setup) mtcrr7 blr _ASM_NOKPROBE_SYMBOL(__restore_cpu_setup) - diff --git a/arch/powerpc/kernel/cpu_specs_book3s_32.h b/arch/powerpc/kernel/cpu_specs_book3s_32.h index 3714634d194a1..83f054fcf837c 100644 --- a/arch/powerpc/kernel/cpu_specs_book3s_32.h +++ b/arch/powerpc/kernel/cpu_specs_book3s_32.h @@ -69,7 +69,7 @@ static struct cpu_spec cpu_specs[] __initdata = { .mmu_features = MMU_FTR_USE_HIGH_BATS, .icache_bsize = 32, .dcache_bsize = 32, - .cpu_setup = __setup_cpu_603, + .cpu_setup = __setup_cpu_g2, .machine_check = machine_check_generic, .platform = "ppc603", }, @@ -83,7 +83,7 @@ static struct cpu_spec cpu_specs[] __initdata = {
Re: [PATCH] powerpc: Split PAGE_SHIFT/SIZE into vdso/page.h
On Thu, Dec 21, 2023 at 1:04 PM Michael Ellerman wrote: > The VDSO needs PAGE_SHIFT/SIZE defined, so it includes asm/page.h. > > However when COMPAT=y the VDSO is built 32-bit, even though the kernel > is 64-bit. That can lead to odd warnings because some kernel constants > are 64-bit, but unsigned long is 32-bit, for example: > > VDSO32C arch/powerpc/kernel/vdso/vgettimeofday-32.o > In file included from :4: > In file included from /home/michael/linux/lib/vdso/gettimeofday.c:5: > In file included from ../include/vdso/datapage.h:137: > In file included from ../arch/powerpc/include/asm/vdso/gettimeofday.h:7: > ../arch/powerpc/include/asm/page.h:230:9: warning: result of comparison of > constant 13835058055282163712 with expression of type 'unsigned long' is > always true > 230 | return __pa(kaddr) >> PAGE_SHIFT; > |^~~ > > Fix it by moving the PAGE_SHIFT/SIZE constants into a separate header, > which can be included by the VDSO, and also by the existing kernel > headers. That avoids exposing the rest of the header to the non-standard > build environment of the compat VDSO. > > The particular warning above was introduced by commit 58b6fed89ab0 > ("powerpc: Make virt_to_pfn() a static inline"), though it is not at > fault, it just exposed the fact that the VDSO was including parts of > page.h that weren't needed or appropriate for the VDSO. > > Don't copy the comment about page sizes, it just risks becoming > outdated, that information is better available in the Kconfig > dependencies and help text. > > Fixes: 58b6fed89ab0 ("powerpc: Make virt_to_pfn() a static inline") > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202311061940.4pbrm44u-...@intel.com/ > Signed-off-by: Michael Ellerman Clearly a working solution! Acked-by: Linus Walleij Just a note: > diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h (...) > +#include (...) > +++ b/arch/powerpc/include/asm/vdso/page.h > @@ -0,0 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +#ifndef _ASM_POWERPC_VDSO_PAGE_H > +#define _ASM_POWERPC_VDSO_PAGE_H > + > +#include Now the whole kernel includes , is this necessary? Yours, Linus Walleij
Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time
Cc +Kees Christophe Leroy writes: > Declaring rodata_enabled and mark_rodata_ro() at all time > helps removing related #ifdefery in C files. > > Signed-off-by: Christophe Leroy > --- > include/linux/init.h | 4 > init/main.c | 21 +++-- > 2 files changed, 7 insertions(+), 18 deletions(-) > > diff --git a/include/linux/init.h b/include/linux/init.h > index 01b52c9c7526..d2b47be38a07 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; > > extern struct file_system_type rootfs_fs_type; > > -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) > extern bool rodata_enabled; > -#endif > -#ifdef CONFIG_STRICT_KERNEL_RWX > void mark_rodata_ro(void); > -#endif > > extern void (*late_time_init)(void); > > diff --git a/init/main.c b/init/main.c > index e24b0780fdff..807df08c501f 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) > early_param("rodata", set_debug_rodata); > #endif > > -#ifdef CONFIG_STRICT_KERNEL_RWX > static void mark_readonly(void) > { > - if (rodata_enabled) { > + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { > /* >* load_module() results in W+X mappings, which are cleaned >* up with call_rcu(). Let's make sure that queued work is > @@ -1409,20 +1408,14 @@ static void mark_readonly(void) > rcu_barrier(); > mark_rodata_ro(); > rodata_test(); > - } else > + } else if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { > pr_info("Kernel memory protection disabled.\n"); > + } else if (IS_ENABLED(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)) { > + pr_warn("Kernel memory protection not selected by kernel > config.\n"); > + } else { > + pr_warn("This architecture does not have kernel memory > protection.\n"); > + } > } > -#elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX) > -static inline void mark_readonly(void) > -{ > - pr_warn("Kernel memory protection not selected by kernel config.\n"); > -} > -#else > -static inline void mark_readonly(void) > -{ > - pr_warn("This architecture does not have kernel memory protection.\n"); > -} > -#endif > > void __weak free_initmem(void) > { > -- > 2.41.0
Re: [PATCH 02/13] hvc: Fix use of uninitialized array in udbg_hvc_putc
Christophe Leroy writes: > Le 14/12/2023 à 06:55, Nicholas Miehlbradt a écrit : >> All elements of bounce_buffer are eventually read and passed to the >> hypervisor so it should probably be fully initialized. > > should or shall ? > >> >> Signed-off-by: Nicholas Miehlbradt > > Should be a Fixed: tag ? > >> --- >> drivers/tty/hvc/hvc_vio.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c >> index 736b230f5ec0..1e88bfcdde20 100644 >> --- a/drivers/tty/hvc/hvc_vio.c >> +++ b/drivers/tty/hvc/hvc_vio.c >> @@ -227,7 +227,7 @@ static const struct hv_ops hvterm_hvsi_ops = { >> static void udbg_hvc_putc(char c) >> { >> int count = -1; >> -unsigned char bounce_buffer[16]; >> +unsigned char bounce_buffer[16] = { 0 }; > > Why 16 while we have a count of 1 in the call to hvterm_raw_put_chars() ? Because hvterm_raw_put_chars() calls hvc_put_chars() which requires a 16 byte buffer, because it passes the buffer directly to firmware which expects a 16 byte buffer. It's a pretty horrible calling convention, but I guess it's to avoid needing another bounce buffer inside hvc_put_chars(). We should probably do the change below, to at least document the interface better. cheers diff --git a/arch/powerpc/include/asm/hvconsole.h b/arch/powerpc/include/asm/hvconsole.h index ccb2034506f0..0ee7ed019e23 100644 --- a/arch/powerpc/include/asm/hvconsole.h +++ b/arch/powerpc/include/asm/hvconsole.h @@ -22,7 +22,7 @@ * parm is included to conform to put_chars() function pointer template */ extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); -extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); +extern int hvc_put_chars(uint32_t vtermno, const char buf[16], int count); /* Provided by HVC VIO */ void hvc_vio_init_early(void); diff --git a/arch/powerpc/platforms/pseries/hvconsole.c b/arch/powerpc/platforms/pseries/hvconsole.c index 1ac52963e08b..c40a82e49d59 100644 --- a/arch/powerpc/platforms/pseries/hvconsole.c +++ b/arch/powerpc/platforms/pseries/hvconsole.c @@ -52,7 +52,7 @@ EXPORT_SYMBOL(hvc_get_chars); * firmware. Must be at least 16 bytes, even if count is less than 16. * @count: Send this number of characters. */ -int hvc_put_chars(uint32_t vtermno, const char *buf, int count) +int hvc_put_chars(uint32_t vtermno, const char buf[16], int count) { unsigned long *lbuf = (unsigned long *) buf; long ret; diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c index 736b230f5ec0..011b239a7e52 100644 --- a/drivers/tty/hvc/hvc_vio.c +++ b/drivers/tty/hvc/hvc_vio.c @@ -115,7 +115,7 @@ static int hvterm_raw_get_chars(uint32_t vtermno, char *buf, int count) * you are sending fewer chars. * @count: number of chars to send. */ -static int hvterm_raw_put_chars(uint32_t vtermno, const char *buf, int count) +static int hvterm_raw_put_chars(uint32_t vtermno, const char buf[16], int count) { struct hvterm_priv *pv = hvterm_privs[vtermno];
[PATCH] powerpc: Split PAGE_SHIFT/SIZE into vdso/page.h
The VDSO needs PAGE_SHIFT/SIZE defined, so it includes asm/page.h. However when COMPAT=y the VDSO is built 32-bit, even though the kernel is 64-bit. That can lead to odd warnings because some kernel constants are 64-bit, but unsigned long is 32-bit, for example: VDSO32C arch/powerpc/kernel/vdso/vgettimeofday-32.o In file included from :4: In file included from /home/michael/linux/lib/vdso/gettimeofday.c:5: In file included from ../include/vdso/datapage.h:137: In file included from ../arch/powerpc/include/asm/vdso/gettimeofday.h:7: ../arch/powerpc/include/asm/page.h:230:9: warning: result of comparison of constant 13835058055282163712 with expression of type 'unsigned long' is always true 230 | return __pa(kaddr) >> PAGE_SHIFT; |^~~ Fix it by moving the PAGE_SHIFT/SIZE constants into a separate header, which can be included by the VDSO, and also by the existing kernel headers. That avoids exposing the rest of the header to the non-standard build environment of the compat VDSO. The particular warning above was introduced by commit 58b6fed89ab0 ("powerpc: Make virt_to_pfn() a static inline"), though it is not at fault, it just exposed the fact that the VDSO was including parts of page.h that weren't needed or appropriate for the VDSO. Don't copy the comment about page sizes, it just risks becoming outdated, that information is better available in the Kconfig dependencies and help text. Fixes: 58b6fed89ab0 ("powerpc: Make virt_to_pfn() a static inline") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202311061940.4pbrm44u-...@intel.com/ Signed-off-by: Michael Ellerman --- arch/powerpc/include/asm/page.h | 10 +- arch/powerpc/include/asm/vdso/gettimeofday.h | 2 +- arch/powerpc/include/asm/vdso/page.h | 10 ++ arch/powerpc/kernel/vdso/vdso32.lds.S| 2 +- arch/powerpc/kernel/vdso/vdso64.lds.S| 2 +- arch/powerpc/kernel/vdso32_wrapper.S | 2 +- arch/powerpc/kernel/vdso64_wrapper.S | 2 +- 7 files changed, 16 insertions(+), 14 deletions(-) create mode 100644 arch/powerpc/include/asm/vdso/page.h diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h index e5fcc79b5bfb..79b463f6f0d7 100644 --- a/arch/powerpc/include/asm/page.h +++ b/arch/powerpc/include/asm/page.h @@ -14,15 +14,7 @@ #include #endif #include - -/* - * On regular PPC32 page size is 4K (but we support 4K/16K/64K/256K pages - * on PPC44x and 4K/16K on 8xx). For PPC64 we support either 4K or 64K software - * page size. When using 64K pages however, whether we are really supporting - * 64K pages in HW or not is irrelevant to those definitions. - */ -#define PAGE_SHIFT CONFIG_PPC_PAGE_SHIFT -#define PAGE_SIZE (ASM_CONST(1) << PAGE_SHIFT) +#include #ifndef __ASSEMBLY__ #ifndef CONFIG_HUGETLB_PAGE diff --git a/arch/powerpc/include/asm/vdso/gettimeofday.h b/arch/powerpc/include/asm/vdso/gettimeofday.h index f0a4cf01e85c..81e82777b0f9 100644 --- a/arch/powerpc/include/asm/vdso/gettimeofday.h +++ b/arch/powerpc/include/asm/vdso/gettimeofday.h @@ -4,7 +4,7 @@ #ifndef __ASSEMBLY__ -#include +#include #include #include #include diff --git a/arch/powerpc/include/asm/vdso/page.h b/arch/powerpc/include/asm/vdso/page.h new file mode 100644 index ..fa30ad64c88e --- /dev/null +++ b/arch/powerpc/include/asm/vdso/page.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#ifndef _ASM_POWERPC_VDSO_PAGE_H +#define _ASM_POWERPC_VDSO_PAGE_H + +#include + +#define PAGE_SHIFT CONFIG_PPC_PAGE_SHIFT +#define PAGE_SIZE (UL(1) << PAGE_SHIFT) + +#endif // _ASM_POWERPC_VDSO_PAGE_H diff --git a/arch/powerpc/kernel/vdso/vdso32.lds.S b/arch/powerpc/kernel/vdso/vdso32.lds.S index 426e1ccc6971..3a6b232c4147 100644 --- a/arch/powerpc/kernel/vdso/vdso32.lds.S +++ b/arch/powerpc/kernel/vdso/vdso32.lds.S @@ -4,7 +4,7 @@ * library */ #include -#include +#include #include #ifdef __LITTLE_ENDIAN__ diff --git a/arch/powerpc/kernel/vdso/vdso64.lds.S b/arch/powerpc/kernel/vdso/vdso64.lds.S index bda6c8cdd459..806005092197 100644 --- a/arch/powerpc/kernel/vdso/vdso64.lds.S +++ b/arch/powerpc/kernel/vdso/vdso64.lds.S @@ -4,7 +4,7 @@ * library */ #include -#include +#include #include #ifdef __LITTLE_ENDIAN__ diff --git a/arch/powerpc/kernel/vdso32_wrapper.S b/arch/powerpc/kernel/vdso32_wrapper.S index 10f92f265d51..3d9d0824a91c 100644 --- a/arch/powerpc/kernel/vdso32_wrapper.S +++ b/arch/powerpc/kernel/vdso32_wrapper.S @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 */ #include -#include +#include __PAGE_ALIGNED_DATA diff --git a/arch/powerpc/kernel/vdso64_wrapper.S b/arch/powerpc/kernel/vdso64_wrapper.S index 839d1a61411d..0e3ff78d7c58 100644 --- a/arch/powerpc/kernel/vdso64_wrapper.S +++ b/arch/powerpc/kernel/vdso64_wrapper.S @@ -1,6 +1,6 @@ /* SPDX-License-Identifier
[PATCH] soc: fsl: dpio: fix kcalloc() arguments order
When compiling with gcc version 14.0.0 20231220 (experimental) and W=1, I've noticed the following warning: drivers/soc/fsl/dpio/dpio-service.c: In function 'dpaa2_io_service_enqueue_multiple_desc_fq': drivers/soc/fsl/dpio/dpio-service.c:526:29: warning: 'kcalloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args] 526 | ed = kcalloc(sizeof(struct qbman_eq_desc), 32, GFP_KERNEL); | ^~ Since 'n' and 'size' arguments of 'kcalloc()' are multiplied to calculate the final size, their actual order doesn't affect the result and so this is not a bug. But it's still worth to fix it. Signed-off-by: Dmitry Antipov --- drivers/soc/fsl/dpio/dpio-service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c index 1d2b27e3ea63..b811446e0fa5 100644 --- a/drivers/soc/fsl/dpio/dpio-service.c +++ b/drivers/soc/fsl/dpio/dpio-service.c @@ -523,7 +523,7 @@ int dpaa2_io_service_enqueue_multiple_desc_fq(struct dpaa2_io *d, struct qbman_eq_desc *ed; int i, ret; - ed = kcalloc(sizeof(struct qbman_eq_desc), 32, GFP_KERNEL); + ed = kcalloc(32, sizeof(struct qbman_eq_desc), GFP_KERNEL); if (!ed) return -ENOMEM; -- 2.43.0
Re: [PATCH] arch: powerpc: kernel: fixed some typos
Ghanshyam Agrawal writes: > Fixed some typos > > Signed-off-by: Ghanshyam Agrawal > --- > arch/powerpc/kernel/eeh_pe.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Please also fix the case in arch/powerpc/include/asm/eeh.h The subject should use the correct prefix. You can see what it should be using: $ git log --oneline arch/powerpc/kernel/eeh_pe.c Please give the patch a better subject, not "some typos", tell me what misspelling you're fixing. Same comment for the commit description. > diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c > index e0ce81279624..8e0c1a8b8641 100644 > --- a/arch/powerpc/kernel/eeh_pe.c > +++ b/arch/powerpc/kernel/eeh_pe.c > @@ -24,10 +24,10 @@ static int eeh_pe_aux_size = 0; > static LIST_HEAD(eeh_phb_pe); > > /** > - * eeh_set_pe_aux_size - Set PE auxillary data size > - * @size: PE auxillary data size > + * eeh_set_pe_aux_size - Set PE auxiliary data size > + * @size: PE auxiliary data size While you're changing it you could also mention what the units of the size are. > * > - * Set PE auxillary data size > + * Set PE auxiliary data size This should gain a full stop at the end of the sentence. > */ > void eeh_set_pe_aux_size(int size) > { > -- > 2.25.1 cheers
Re: powerpc: several early boot regressions on MPC52xx
Le 21/12/2023 à 11:33, Matthias Schiffer a écrit : > [Vous ne recevez pas souvent de courriers de > matthias.schif...@ew.tq-group.com. Découvrez pourquoi ceci est important à > https://aka.ms/LearnAboutSenderIdentification ] > > On Wed, 2023-12-20 at 14:55 +, Christophe Leroy wrote: >>> Le 19/12/2023 à 14:34, Matthias Schiffer a écrit : > [Vous ne recevez pas souvent de courriers de > matthias.schif...@ew.tq-group.com. Découvrez pourquoi ceci est important > à https://aka.ms/LearnAboutSenderIdentification ] > > On Mon, 2023-12-18 at 19:48 +, Christophe Leroy wrote: >>> Hi Matthias, >>> >>> Le 18/12/2023 à 14:48, Matthias Schiffer a écrit : > Hi all, > > I'm currently in the process of porting our ancient TQM5200 SoM to a > modern kernel, and I've > identified a number of regressions that cause early boot failures > (before the UART console has been > initialized) with arch/powerpc/configs/52xx/tqm5200_defconfig. >>> >>> "modern" kernel ==> which version ? > > Hi Christophe, > > I was testing with torvalds/master as of yesterday, and bisected > everything from 4.14 to identify > the commits related to the issues. For my current project, 6.1.y or 6.6.y > will likely be our kernel > of choice, but I'd also like to get mainline to a working state again if > possible. > >>> > > Issue 1) Boot fails with CONFIG_PPC_KUAP enabled (enabled by default > since 9f5bd8f1471d7 > "powerpc/32s: Activate KUAP and KUEP by default"). The reason is a > number of of_iomap() calls in > arch/powerpc/platforms/52xx that should be early_ioremap(). >>> >>> Can you give more details and what leads you to that conclusion ? >>> >>> There should be no relation between KUAP and of_iomap()/early_ioremap(). >>> Problem is likely somewhere else. > > You are entirely right, the warnings about early_ioremap() were a red > hering. I can't reproduce any > difference in boot behavior anymore I thought I was seeing when changing > the of_iomap() to > early_ioremap(). I assume I got confused by testing for too many > variables at once (kernel version + > 2 Kconfig settings). > > >>> > > I can fix this up easy enough for mpc5200_simple by changing > mpc5200_setup_xlb_arbiter() to use > early_ioremap() and moving mpc52xx_map_common_devices() from the > setup_arch to the init hook (one > side effect is that mpc52xx_restart() only works a bit later, as it > requires the mpc52xx_wdt mapping > from mpc52xx_map_common_devices(); I'm not sure if there is a better > solution). > > For the other 52xx platforms (efika, lite5200, media5200) things are > a bit more chaotic, and they > create several more temporary mappings from setup_arch. Either they > should all be moved to the init > hook as well, or be converted to early_ioremap(), but I can't tell > which is more appropriate. As a > first step, I would propose a patch that fixes this for the simple > platforms and leaves the other > ones unchanged. > > (Side note: I also found that before 16132529cee58 ("powerpc/32s: > Rework Kernel Userspace Access > Protection"), boot would succeed even with KUAP enabled without > changing the incorrect of_iomap(); I > guess the old implementation was more lenient about the incorrect > calls that the kernel warns > about?) >>> >>> Interesting. >>> Again, there shouldn't be any impact of those incorrect calls. They are >>> correct calls, it is just an historical method that we want to get rid >>> of on day. >>> Could you then provide the dmesg of what/how it works here ? And then >>> I'd also be interested in a dump of /sys/kernel/debug/kernel_page_tables >>> and /sys/kernel/debug/powerpc/block_address_translation >>> and /sys/kernel/debug/powerpc/segment_registers >>> >>> For that you'll need CONFIG_PTDUMP_DEBUGFS > > As it turns out, whatever issue existed with KUAP at the time when it was > changed to enabled by > default for 32s (which was in 5.14) has been resolved in current > mainline. Current torvalds/master > boots fine with KUAP enabled, and only CONFIG_STRICT_KERNEL_RWX breaks > the boot. > >>> > > Issue 2) Boot fails with CONFIG_STRICT_KERNEL_RWX enabled, which is > also the default nowadays. > > I have not found the cause of this boot failure yet; is there any way > to debug this if the failure > happens before the UART is available and I currently don't have JTAG > for this hardware? >>> >>> Shouldn't happen
Re: [PATCH 3/3] powerpc: ps3: Add missing set_freezable() for ps3_probe_thread()
Geoff Levand writes: > Hi Kevin, > > On 12/21/23 13:45, Kevin Hao wrote: >> The kernel thread function ps3_probe_thread() invokes the try_to_freeze() >> in its loop. But all the kernel threads are non-freezable by default. >> So if we want to make a kernel thread to be freezable, we have to invoke >> set_freezable() explicitly. >> >> Signed-off-by: Kevin Hao >> --- >> arch/powerpc/platforms/ps3/device-init.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/powerpc/platforms/ps3/device-init.c >> b/arch/powerpc/platforms/ps3/device-init.c >> index e87360a0fb40..878bc160246e 100644 >> --- a/arch/powerpc/platforms/ps3/device-init.c >> +++ b/arch/powerpc/platforms/ps3/device-init.c >> @@ -827,6 +827,7 @@ static int ps3_probe_thread(void *data) >> if (res) >> goto fail_free_irq; >> >> +set_freezable(); >> /* Loop here processing the requested notification events. */ >> do { >> try_to_freeze(); > > Seems like a reasonable addition. > > Signed-off-by: Geoff Levand I turned that into an Acked-by, which I think is what you meant :) cheers
Re: [PATCH] MAINTAINERS: powerpc: Add Aneesh & Naveen
On Tue, 05 Dec 2023 16:11:05 +1100, Michael Ellerman wrote: > Aneesh and Naveen are helping out with some aspects of upstream > maintenance, add them as reviewers. > > Applied to powerpc/fixes. [1/1] MAINTAINERS: powerpc: Add Aneesh & Naveen https://git.kernel.org/powerpc/c/d2441d3e8c0c076d0a2e705fa235c76869a85140 cheers
Re: [PATCH v5] powerpc/pseries/vas: Migration suspend waits for no in-progress open windows
On Sat, 25 Nov 2023 15:51:04 -0800, Haren Myneni wrote: > The hypervisor returns migration failure if all VAS windows are not > closed. During pre-migration stage, vas_migration_handler() sets > migration_in_progress flag and closes all windows from the list. > The allocate VAS window routine checks the migration flag, setup > the window and then add it to the list. So there is possibility of > the migration handler missing the window that is still in the > process of setup. > > [...] Applied to powerpc/fixes. [1/1] powerpc/pseries/vas: Migration suspend waits for no in-progress open windows https://git.kernel.org/powerpc/c/0cf72f7f14d12cb065c3d01954cf42fc5638aa69 cheers
Re: [RFC PATCH 6/9] powerpc/ftrace: Update and move function profile instructions out-of-line
Le 08/12/2023 à 17:30, Naveen N Rao a écrit : > Function profile sequence on powerpc includes two instructions at the > beginning of each function: > > mflrr0 > bl ftrace_caller > > The call to ftrace_caller() gets nop'ed out during kernel boot and is > patched in when ftrace is enabled. > > There are two issues with this: > 1. The 'mflr r0' instruction at the beginning of each function remains > even though ftrace is not being used. > 2. When ftrace is activated, we return from ftrace_caller() with a bctr > instruction to preserve r0 and LR, resulting in the link stack > becoming unbalanced. > > To address (1), we have tried to nop'out the 'mflr r0' instruction when > nop'ing out the call to ftrace_caller() and restoring it when enabling > ftrace. But, that required additional synchronization slowing down > ftrace activation. It also left an additional nop instruction at the > beginning of each function and that wasn't desirable on 32-bit powerpc. > > Instead of that, move the function profile sequence out-of-line leaving > a single nop at function entry. On ftrace activation, the nop is changed > to an unconditional branch to the out-of-line sequence that in turn > calls ftrace_caller(). This removes the need for complex synchronization > during ftrace activation and simplifies the code. More importantly, this > improves performance of the kernel when ftrace is not in use. > > To address (2), change the ftrace trampoline to return with a 'blr' > instruction with the original return address in r0 intact. Then, an > additional 'mtlr r0' instruction in the function profile sequence can > move the correct return address back to LR. > > With the above two changes, the function profile sequence now looks like > the following: > > [func: # GEP -- 64-bit powerpc, optional > addis r2,r12,imm1 > addir2,r2,imm2] >tramp: > mflrr0 > bl ftrace_caller > mtlrr0 > b func > nop > [nop] # 64-bit powerpc only >func: # LEP > nop > > On 32-bit powerpc, the ftrace mcount trampoline is now completely > outside the function. This is also the case on 64-bit powerpc for > functions that do not need a GEP. However, for functions that need a > GEP, the additional instructions are inserted between the GEP and the > LEP. Since we can only have a fixed number of instructions between GEP > and LEP, we choose to emit 6 instructions. Four of those instructions > are used for the function profile sequence and two instruction slots are > reserved for implementing support for DYNAMIC_FTRACE_WITH_CALL_OPS. On > 32-bit powerpc, we emit one additional nop for this purpose resulting in > a total of 5 nops before function entry. > > To enable ftrace, the nop at function entry is changed to an > unconditional branch to 'tramp'. The call to ftrace_caller() may be > updated to ftrace_regs_caller() depending on the registered ftrace ops. > On 64-bit powerpc, we additionally change the instruction at 'tramp' to > 'mflr r0' from an unconditional branch back to func+4. This is so that > functions entered through the GEP can skip the function profile sequence > unless ftrace is enabled. > > With the context_switch microbenchmark on a P9 machine, there is a > performance improvement of ~6% with this patch applied, going from 650k > context switches to 690k context switches without ftrace enabled. With > ftrace enabled, the performance was similar at 86k context switches. Wondering how significant that context_switch micorbenchmark is. I ran it on both mpc885 and mpc8321 and I'm a bit puzzled by some of the results: # ./context_switch --no-fp Using threads with yield on cpus 0/0 touching FP:no altivec:no vector:no vdso:no On 885, I get the following results before and after your patch. CONFIG_FTRACE not selected : 44,9k CONFIG_FTRACE selected, before : 32,8k CONFIG_FTRACE selected, after : 33,6k All this is with CONFIG_INIT_STACK_ALL_ZERO which is the default. But when I select CONFIG_INIT_STACK_NONE, the CONFIG_FTRACE not selected result is only 34,4. On 8321: CONFIG_FTRACE not selected : 100,3k CONFIG_FTRACE selected, before : 72,5k CONFIG_FTRACE selected, after : 116k So the results look odd to me. > > The downside of this approach is the increase in vmlinux size, > especially on 32-bit powerpc. We now emit 3 additional instructions for > each function (excluding the one or two instructions for supporting > DYNAMIC_FTRACE_WITH_CALL_OPS). On 64-bit powerpc with the current > implementation of -fpatchable-function-entry though, this is not > avoidable since we are forced to emit 6 instructions between the GEP and > the LEP even if we are to only support DYNAMIC_FTRACE_WITH_CALL_OPS. The increase is almost 5% on the few 32 bits defconfig I have tested. That's a lot. On 32 bits powerpc, only the e500 has a link stack that could end up being unbalanced. Could we keep the bctr and
Re: [PATCH 00/12] KVM: PPC: Nested APIv2 : Performance improvements
On Fri, 01 Dec 2023 18:56:05 +0530, Vaibhav Jain wrote: > This patch series introduces series of performance improvements to recently > added support for Nested APIv2 PPC64 Guests via [1]. Details for Nested > APIv2 for PPC64 Guests is available in Documentation/powerpc/kvm-nested.rst. > > This patch series introduces various optimizations for a Nested APIv2 > guests namely: > > [...] Applied to powerpc/topic/ppc-kvm. [01/12] KVM: PPC: Book3S HV nestedv2: Invalidate RPT before deleting a guest https://git.kernel.org/powerpc/c/7d370e1812b9a5f5cc68aaa5991bf7d31d8ff52c [02/12] KVM: PPC: Book3S HV nestedv2: Avoid reloading the tb offset https://git.kernel.org/powerpc/c/e0d4acbcba3f2d63dc15bc5432c8e26fc9e19675 [03/12] KVM: PPC: Book3S HV nestedv2: Do not check msr on hcalls https://git.kernel.org/powerpc/c/63ccae78cd88b52fb1d598ae33fa8408ce067b30 [04/12] KVM: PPC: Book3S HV nestedv2: Get the PID only if needed to copy tofrom a guest https://git.kernel.org/powerpc/c/e678748a8dca5b57041a84a66577f6168587b3f7 [05/12] KVM: PPC: Book3S HV nestedv2: Ensure LPCR_MER bit is passed to the L0 https://git.kernel.org/powerpc/c/ec0f6639fa8853cf6bfdfc3588aada7eeb7e5e37 [06/12] KVM: PPC: Book3S HV: Handle pending exceptions on guest entry with MSR_EE https://git.kernel.org/powerpc/c/ecd10702baae5c16a91d139bde7eff84ce55daee [07/12] KVM: PPC: Book3S HV nestedv2: Do not inject certain interrupts https://git.kernel.org/powerpc/c/df938a5576f3f3b08e1f217c660385c0d58a0b91 [08/12] KVM: PPC: Book3S HV nestedv2: Avoid msr check in kvmppc_handle_exit_hv() https://git.kernel.org/powerpc/c/a9a3de530d7531bf6cd3f6ccda769cd94c1105a0 [09/12] KVM: PPC: Book3S HV nestedv2: Do not call H_COPY_TOFROM_GUEST https://git.kernel.org/powerpc/c/4bc8ff6f170c78f64446c5d5f9ef6771eefd3416 [10/12] KVM: PPC: Book3S HV nestedv2: Register the VPA with the L0 https://git.kernel.org/powerpc/c/db1dcfae1dae3c042f348175ac0394e2fc14b1b3 [11/12] KVM: PPC: Reduce reliance on analyse_instr() in mmio emulation https://git.kernel.org/powerpc/c/797a5af8fc7297b19e5c6b1713956ebf1e6c1cde [12/12] KVM: PPC: Book3S HV nestedv2: Do not cancel pending decrementer exception https://git.kernel.org/powerpc/c/180c6b072bf360b686e53d893d8dcf7dbbaec6bb cheers
Re: [PATCH v2 1/2] powerpc/book3s/hash: Drop _PAGE_PRIVILEGED from PAGE_NONE
On Mon, 04 Dec 2023 15:06:37 +0530, aneesh.ku...@kernel.org wrote: > There used to be a dependency on _PAGE_PRIVILEGED with pte_savedwrite. > But that got dropped by > commit 6a56ccbcf6c6 ("mm/autonuma: use can_change_(pte|pmd)_writable() to > replace savedwrite") > > With the change in this patch numa fault pte (pte_protnone()) gets mapped as > regular user pte > with RWX cleared (no-access) whereas earlier it used to be mapped > _PAGE_PRIVILEGED. > > [...] Applied to powerpc/next. [1/2] powerpc/book3s/hash: Drop _PAGE_PRIVILEGED from PAGE_NONE https://git.kernel.org/powerpc/c/773b93f1d1c38c5c0d5308b8c9229c7a6ec5b2a0 [2/2] powerpc/book3s64: Avoid __pte_protnone() check in __pte_flags_need_flush() https://git.kernel.org/powerpc/c/a59c14f6b4caad7671dfb81737beba0b313897e4 cheers
Re: [PATCH v5 0/5] powerpc/smp: Topology and shared processor optimizations
On Thu, 14 Dec 2023 23:37:10 +0530, Srikar Dronamraju wrote: > PowerVM systems configured in shared processors mode have some unique > challenges. Some device-tree properties will be missing on a shared > processor. Hence some sched domains may not make sense for shared processor > systems. > > Most shared processor systems are over-provisioned. Underlying PowerVM > Hypervisor would schedule at a Big Core granularity. The most recent power > processors support two almost independent cores. In a lightly loaded > condition, it helps the overall system performance if we pack to lesser > number of Big Cores. > > [...] Applied to powerpc/next. [1/5] powerpc/smp: Enable Asym packing for cores on shared processor https://git.kernel.org/powerpc/c/aa80c6343fcf53cbc29f84ba9f89ca87d4e41350 [2/5] powerpc/smp: Disable MC domain for shared processor https://git.kernel.org/powerpc/c/0e1c1986e0e65746daa05405d7747ce882f83cf1 [3/5] powerpc/smp: Add __ro_after_init attribute https://git.kernel.org/powerpc/c/fd535a858ebeb1f478b1d065b6c057f52aad483a [4/5] powerpc/smp: Avoid asym packing within thread_group of a core https://git.kernel.org/powerpc/c/0e93f1c780e8fd315f1262467b7d35eb6f766d2f [5/5] powerpc/smp: Dynamically build Powerpc topology https://git.kernel.org/powerpc/c/c46975715f5a7b941aa09bc0539a8dbe297f308f cheers
Re: (subset) [RFC PATCH 0/9] powerpc: ftrace updates
On Fri, 08 Dec 2023 22:00:39 +0530, Naveen N Rao wrote: > Early RFC. > > This series attempts to address couple of issues with the existing > support for ftrace on powerpc, with a view towards improving performance > when ftrace is not enabled. See patch 6 for more details. > > Patches 7 and 8 implement support for ftrace direct calls, through > adding support for DYNAMIC_FTRACE_WITH_CALL_OPS. > > [...] Patches 1, 3 and 4 applied to powerpc/next. [1/9] powerpc/ftrace: Fix indentation in ftrace.h https://git.kernel.org/powerpc/c/2ec36570c3581285d15de672eaed10ce7e9218cd [3/9] powerpc/ftrace: Remove nops after the call to ftrace_stub https://git.kernel.org/powerpc/c/ae24db43b3b427eb290b58d55179c32f0a7539d1 [4/9] powerpc/Kconfig: Select FUNCTION_ALIGNMENT_4B https://git.kernel.org/powerpc/c/b20f98e8b3deb50247603f0242ee2d1e38726635 cheers
Re: [PATCH v6 00/13] powerpc/pseries: New character devices for system parameters and VPD
On Tue, 12 Dec 2023 11:01:47 -0600, Nathan Lynch wrote: > Add character devices that expose PAPR-specific system parameters and > VPD to user space. > > The problem: important platform features are enabled on Linux VMs > through the powerpc-specific rtas() syscall in combination with > writeable mappings of /dev/mem. In typical usage, this is encapsulated > behind APIs provided by the librtas library. This paradigm is > incompatible with lockdown, which prohibits /dev/mem access. It also > is too low-level in many cases: a single logical operation may require > multiple sys_rtas() calls in succession to complete. This carries the > risk that a process may exit while leaving an operation unfinished. It > also means that callers must coordinate their use of the syscall for > functions that cannot tolerate multiple concurrent clients, such as > ibm,get-vpd. > > [...] Applied to powerpc/next. [01/13] powerpc/rtas: Avoid warning on invalid token argument to sys_rtas() https://git.kernel.org/powerpc/c/01e346ffefda3a7088afebf02b940614179688e7 [02/13] powerpc/rtas: Add for_each_rtas_function() iterator https://git.kernel.org/powerpc/c/c500c6e736df030f8956080738f59701c0b43dd8 [03/13] powerpc/rtas: Fall back to linear search on failed token->function lookup https://git.kernel.org/powerpc/c/669acc7eec223a81ea5e2420de85b61979ab7dad [04/13] powerpc/rtas: Add function return status constants https://git.kernel.org/powerpc/c/9592aa5ad59e736727fe7894e6e820e2d851abcf [05/13] powerpc/rtas: Move token validation from block_rtas_call() to sys_rtas() https://git.kernel.org/powerpc/c/e7582edb78619abb4ebf0a6e1fed125dcd7243b6 [06/13] powerpc/rtas: Facilitate high-level call sequences https://git.kernel.org/powerpc/c/adf7a019e5f82607fc0f0079926d0178afe8f4ef [07/13] powerpc/rtas: Serialize firmware activation sequences https://git.kernel.org/powerpc/c/dc7637c402b90a197d3f21a3d78f2b00b67ea22a [08/13] powerpc/rtas: Warn if per-function lock isn't held https://git.kernel.org/powerpc/c/e3681107bc9f97c5948a1c8a3a97ac64907210ce [09/13] powerpc/pseries: Add papr-vpd character driver for VPD retrieval https://git.kernel.org/powerpc/c/514f6ff4369a30bf0da71a1a09fd47b2fca5d76f [10/13] powerpc/pseries/papr-sysparm: Validate buffer object lengths https://git.kernel.org/powerpc/c/35aae182bd7b422be3cefc08c12207bf2b973364 [11/13] powerpc/pseries/papr-sysparm: Expose character device to user space https://git.kernel.org/powerpc/c/905b9e48786ec55b2c469db77fb46e20bf3e4901 [12/13] powerpc/selftests: Add test for papr-vpd https://git.kernel.org/powerpc/c/9118c5d32bddb5f75bc4f9f31218e70317702502 [13/13] powerpc/selftests: Add test for papr-sysparm https://git.kernel.org/powerpc/c/76b2ec3faeaa0c8d84705acd64ac0e5a307ce9c2 cheers
Re: [PATCH] MAINTAINERS: powerpc: Transfer PPC83XX to Christophe
On Tue, 05 Dec 2023 16:12:39 +1100, Michael Ellerman wrote: > Christophe volunteered[1] to maintain PPC83XX. > > 1: > https://lore.kernel.org/all/7b1bf4dc-d09d-35b8-f4df-16bf00429...@csgroup.eu/ > > Applied to powerpc/next. [1/1] MAINTAINERS: powerpc: Transfer PPC83XX to Christophe https://git.kernel.org/powerpc/c/4cb3e3ec23fa33a0f58ff133d5aedd26a50356ef cheers
Re: [PATCH 1/5] selftests/powerpc: Fix error handling in FPU/VMX preemption tests
On Wed, 29 Nov 2023 00:27:44 +1100, Michael Ellerman wrote: > The FPU & VMX preemption tests do not check for errors returned by the > low-level asm routines, preempt_fpu() / preempt_vsx() respectively. > That means any register corruption detected by the asm routines does not > result in a test failure. > > Fix it by returning the return value of the asm routines from the > pthread child routines. > > [...] Applied to powerpc/next. [1/5] selftests/powerpc: Fix error handling in FPU/VMX preemption tests https://git.kernel.org/powerpc/c/9dbd5927408c4a0707de73ae9dd9306b184e8fee [2/5] selftests/powerpc: Check all FPRs in fpu_preempt https://git.kernel.org/powerpc/c/e5d00aaac651a69b8016d355123438387bfd37be [3/5] selftests/powerpc: Generate better bit patterns for FPU tests https://git.kernel.org/powerpc/c/2ba107f6795d780bb54b85040a9b2c6a60fccb15 [4/5] selftests/powerpc: Run fpu_preempt test for 60 seconds https://git.kernel.org/powerpc/c/60d2c3af9a0c04dc2d34a62e9f5e7033b40dfed8 [5/5] selftests/powerpc: Check all FPRs in fpu_syscall test https://git.kernel.org/powerpc/c/1bdf22580b794a498b17617bcd7ae417d6b78444 cheers
Re: [PATCH 1/4] powerpc/Makefile: Don't use $(ARCH) unnecessarily
On Wed, 06 Dec 2023 22:55:45 +1100, Michael Ellerman wrote: > There's no need to use $(ARCH) for references to the arch directory in > the source tree, it is always arch/powerpc. > > Applied to powerpc/next. [1/4] powerpc/Makefile: Don't use $(ARCH) unnecessarily https://git.kernel.org/powerpc/c/dc420877b5bd92db5d80df6b117c7a0f987290af [2/4] powerpc/vdso: No need to undef powerpc for 64-bit build https://git.kernel.org/powerpc/c/42449052c94f22e18e01d71147d8fd75cb58132a [3/4] powerpc/Makefile: Default to ppc64le_defconfig when cross building https://git.kernel.org/powerpc/c/22f17b02f88b48c01d3ac38d40d2b0b695ab2d10 [4/4] powerpc/Makefile: Auto detect cross compiler https://git.kernel.org/powerpc/c/402928b58ec62b42b11992a7b51ff2f56ed65a18 cheers
Re: (subset) [PATCH 1/2] powerpc/hv-gpci: Add return value check in affinity_domain_via_partition_show function
On Thu, 16 Nov 2023 17:50:32 +0530, Kajol Jain wrote: > To access hv-gpci kernel interface files data, the > "Enable Performance Information Collection" option has to be set > in hmc. Incase that option is not set and user try to read > the interface files, it should give error message as > operation not permitted. > > Result of accessing added interface files with disabled > performance collection option: > > [...] Applied to powerpc/next. [1/2] powerpc/hv-gpci: Add return value check in affinity_domain_via_partition_show function https://git.kernel.org/powerpc/c/070b71f428facd9130319707db854ed8bd24637a cheers
Re: [RESEND PATCH] powerpc/fsl: fix the schema check errors for fsl,tmu-calibration
On Tue, 12 Dec 2023 19:44:58 +0100, David Heidelberg wrote: > fsl,tmu-calibration is in u32-matrix. Use matching property syntax. > No functional changes. Fixes warnings as: > ... > > [...] Applied to powerpc/next. [1/1] powerpc/fsl: fix the schema check errors for fsl,tmu-calibration https://git.kernel.org/powerpc/c/9ec1d7486e2520b4898d7f8e1ec3acc7c13c8dc8 cheers
Re: [PATCH v3] powerpc/mm: Fix null-pointer dereference in pgtable_cache_add
On Mon, 04 Dec 2023 10:32:23 +0800, Kunwu Chan wrote: > kasprintf() returns a pointer to dynamically allocated memory > which can be NULL upon failure. Ensure the allocation was successful > by checking the pointer validity. > > Applied to powerpc/next. [1/1] powerpc/mm: Fix null-pointer dereference in pgtable_cache_add https://git.kernel.org/powerpc/c/f46c8a75263f97bda13c739ba1c90aced0d3b071 cheers
Re: [PATCH] powerpc/powernv: Fix null pointer dereference in opal_powercap_init
On Sun, 26 Nov 2023 17:57:39 +0800, Kunwu Chan wrote: > kasprintf() returns a pointer to dynamically allocated memory > which can be NULL upon failure. > > Applied to powerpc/next. [1/1] powerpc/powernv: Fix null pointer dereference in opal_powercap_init https://git.kernel.org/powerpc/c/e123015c0ba859cf48aa7f89c5016cc6e98e018d cheers
Re: [PATCH] powerpc/powernv: Add a null pointer check to scom_debug_init_one
On Fri, 08 Dec 2023 16:59:37 +0800, Kunwu Chan wrote: > kasprintf() returns a pointer to dynamically allocated memory > which can be NULL upon failure. > Add a null pointer check, and release 'ent' to avoid memory leaks. > > Applied to powerpc/next. [1/1] powerpc/powernv: Add a null pointer check to scom_debug_init_one https://git.kernel.org/powerpc/c/9a260f2dd827bbc82cc60eb4f4d8c22707d80742 cheers
Re: [PATCH] powerpc/powernv: Add a null pointer check in opal_event_init
On Mon, 27 Nov 2023 11:07:55 +0800, Kunwu Chan wrote: > kasprintf() returns a pointer to dynamically allocated memory > which can be NULL upon failure. > > Applied to powerpc/next. [1/1] powerpc/powernv: Add a null pointer check in opal_event_init https://git.kernel.org/powerpc/c/8649829a1dd25199bbf557b2621cedb4bf9b3050 cheers
Re: [PATCH] powerpc/imc-pmu: Fix null pointer dereference in update_events_in_group
On Sun, 26 Nov 2023 17:37:19 +0800, Kunwu Chan wrote: > kasprintf() returns a pointer to dynamically allocated memory > which can be NULL upon failure. > > Applied to powerpc/next. [1/1] powerpc/imc-pmu: Fix null pointer dereference in update_events_in_group https://git.kernel.org/powerpc/c/0a233867a39078ebb0f575e2948593bbff5826b3 cheers
Re: [PATCH] powerpc/sched: Cleanup vcpu_is_preempted()
On Tue, 14 Nov 2023 12:42:19 +0530, Aneesh Kumar K.V wrote: > No functional change in this patch. A helper is added to find if > vcpu is dispatched by hypervisor. Use that instead of opencoding. > Also clarify some of the comments. > > Applied to powerpc/next. [1/1] powerpc/sched: Cleanup vcpu_is_preempted() https://git.kernel.org/powerpc/c/6f4b7052daa060e7d20d6d599697b8ac702a7e69 cheers
Re: [PATCH v2 1/2] powerpc: add `cur_cpu_spec` symbol to vmcoreinfo
On Wed, 20 Sep 2023 16:27:05 +0530, Aditya Gupta wrote: > Since below commit, address mapping for vmemmap has changed for Radix > MMU, where address mapping is stored in kernel page table itself, > instead of earlier used 'vmemmap_list'. > > commit 368a0590d954 ("powerpc/book3s64/vmemmap: switch radix to use > a different vmemmap handling function") > > [...] Patch 2 applied to powerpc/next. [2/2] powerpc: add cpu_spec.cpu_features to vmcoreinfo https://git.kernel.org/powerpc/c/a143892cb77c5397fd4356bbef9982abe4f3c5a5 cheers
Re: powerpc: several early boot regressions on MPC52xx
On Wed, 2023-12-20 at 14:55 +, Christophe Leroy wrote: > > Le 19/12/2023 à 14:34, Matthias Schiffer a écrit : > > > > [Vous ne recevez pas souvent de courriers de > > > > matthias.schif...@ew.tq-group.com. Découvrez pourquoi ceci est > > > > important à https://aka.ms/LearnAboutSenderIdentification ] > > > > > > > > On Mon, 2023-12-18 at 19:48 +, Christophe Leroy wrote: > > > > > > Hi Matthias, > > > > > > > > > > > > Le 18/12/2023 à 14:48, Matthias Schiffer a écrit : > > > > > > > > Hi all, > > > > > > > > > > > > > > > > I'm currently in the process of porting our ancient TQM5200 SoM > > > > > > > > to a modern kernel, and I've > > > > > > > > identified a number of regressions that cause early boot > > > > > > > > failures (before the UART console has been > > > > > > > > initialized) with arch/powerpc/configs/52xx/tqm5200_defconfig. > > > > > > > > > > > > "modern" kernel ==> which version ? > > > > > > > > Hi Christophe, > > > > > > > > I was testing with torvalds/master as of yesterday, and bisected > > > > everything from 4.14 to identify > > > > the commits related to the issues. For my current project, 6.1.y or > > > > 6.6.y will likely be our kernel > > > > of choice, but I'd also like to get mainline to a working state again > > > > if possible. > > > > > > > > > > > > > > > > > > > > > > > > > > Issue 1) Boot fails with CONFIG_PPC_KUAP enabled (enabled by > > > > > > > > default since 9f5bd8f1471d7 > > > > > > > > "powerpc/32s: Activate KUAP and KUEP by default"). The reason > > > > > > > > is a number of of_iomap() calls in > > > > > > > > arch/powerpc/platforms/52xx that should be early_ioremap(). > > > > > > > > > > > > Can you give more details and what leads you to that conclusion ? > > > > > > > > > > > > There should be no relation between KUAP and > > > > > > of_iomap()/early_ioremap(). > > > > > > Problem is likely somewhere else. > > > > > > > > You are entirely right, the warnings about early_ioremap() were a red > > > > hering. I can't reproduce any > > > > difference in boot behavior anymore I thought I was seeing when > > > > changing the of_iomap() to > > > > early_ioremap(). I assume I got confused by testing for too many > > > > variables at once (kernel version + > > > > 2 Kconfig settings). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I can fix this up easy enough for mpc5200_simple by changing > > > > > > > > mpc5200_setup_xlb_arbiter() to use > > > > > > > > early_ioremap() and moving mpc52xx_map_common_devices() from > > > > > > > > the setup_arch to the init hook (one > > > > > > > > side effect is that mpc52xx_restart() only works a bit later, > > > > > > > > as it requires the mpc52xx_wdt mapping > > > > > > > > from mpc52xx_map_common_devices(); I'm not sure if there is a > > > > > > > > better solution). > > > > > > > > > > > > > > > > For the other 52xx platforms (efika, lite5200, media5200) > > > > > > > > things are a bit more chaotic, and they > > > > > > > > create several more temporary mappings from setup_arch. Either > > > > > > > > they should all be moved to the init > > > > > > > > hook as well, or be converted to early_ioremap(), but I can't > > > > > > > > tell which is more appropriate. As a > > > > > > > > first step, I would propose a patch that fixes this for the > > > > > > > > simple platforms and leaves the other > > > > > > > > ones unchanged. > > > > > > > > > > > > > > > > (Side note: I also found that before 16132529cee58 > > > > > > > > ("powerpc/32s: Rework Kernel Userspace Access > > > > > > > > Protection"), boot would succeed even with KUAP enabled without > > > > > > > > changing the incorrect of_iomap(); I > > > > > > > > guess the old implementation was more lenient about the > > > > > > > > incorrect calls that the kernel warns > > > > > > > > about?) > > > > > > > > > > > > Interesting. > > > > > > Again, there shouldn't be any impact of those incorrect calls. They > > > > > > are > > > > > > correct calls, it is just an historical method that we want to get > > > > > > rid > > > > > > of on day. > > > > > > Could you then provide the dmesg of what/how it works here ? And > > > > > > then > > > > > > I'd also be interested in a dump of > > > > > > /sys/kernel/debug/kernel_page_tables > > > > > > and /sys/kernel/debug/powerpc/block_address_translation > > > > > > and /sys/kernel/debug/powerpc/segment_registers > > > > > > > > > > > > For that you'll need CONFIG_PTDUMP_DEBUGFS > > > > > > > > As it turns out, whatever issue existed with KUAP at the time when it > > > > was changed to enabled by > > > > default for 32s (which was in 5.14) has been resolved in current > > > > mainline. Current torvalds/master > > > > boots fine with KUAP enabled, and only CONFIG_STRICT_KERNEL_RWX breaks > > > > the boot. > > > > > > > > > > > > > > > > > > > > > > > > > > Issue 2) Boot fails with CONFIG_STRICT_KERNEL_RWX enabled, > > > > > > > > which is also the default nowada
[PATCH 3/3] powerpc: Simplify strict_kernel_rwx_enabled()
Now that rodata_enabled is always declared, remove #ifdef and define a single version of strict_kernel_rwx_enabled(). Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/mmu.h | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index d8b7e246a32f..24241995f740 100644 --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h @@ -330,17 +330,10 @@ static __always_inline bool early_radix_enabled(void) return early_mmu_has_feature(MMU_FTR_TYPE_RADIX); } -#ifdef CONFIG_STRICT_KERNEL_RWX static inline bool strict_kernel_rwx_enabled(void) { - return rodata_enabled; + return IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled; } -#else -static inline bool strict_kernel_rwx_enabled(void) -{ - return false; -} -#endif static inline bool strict_module_rwx_enabled(void) { -- 2.41.0
[PATCH 2/3] modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around rodata_enabled
Now that rodata_enabled is declared at all time, the #ifdef CONFIG_STRICT_MODULE_RWX can be removed. Signed-off-by: Christophe Leroy --- kernel/module/strict_rwx.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index a2b656b4e3d2..eadff63b6e80 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -34,12 +34,8 @@ void module_enable_x(const struct module *mod) void module_enable_ro(const struct module *mod, bool after_init) { - if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) - return; -#ifdef CONFIG_STRICT_MODULE_RWX - if (!rodata_enabled) + if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled) return; -#endif module_set_memory(mod, MOD_TEXT, set_memory_ro); module_set_memory(mod, MOD_INIT_TEXT, set_memory_ro); -- 2.41.0
[PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time
Declaring rodata_enabled and mark_rodata_ro() at all time helps removing related #ifdefery in C files. Signed-off-by: Christophe Leroy --- include/linux/init.h | 4 init/main.c | 21 +++-- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/include/linux/init.h b/include/linux/init.h index 01b52c9c7526..d2b47be38a07 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; extern struct file_system_type rootfs_fs_type; -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) extern bool rodata_enabled; -#endif -#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void); -#endif extern void (*late_time_init)(void); diff --git a/init/main.c b/init/main.c index e24b0780fdff..807df08c501f 100644 --- a/init/main.c +++ b/init/main.c @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) early_param("rodata", set_debug_rodata); #endif -#ifdef CONFIG_STRICT_KERNEL_RWX static void mark_readonly(void) { - if (rodata_enabled) { + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { /* * load_module() results in W+X mappings, which are cleaned * up with call_rcu(). Let's make sure that queued work is @@ -1409,20 +1408,14 @@ static void mark_readonly(void) rcu_barrier(); mark_rodata_ro(); rodata_test(); - } else + } else if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { pr_info("Kernel memory protection disabled.\n"); + } else if (IS_ENABLED(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)) { + pr_warn("Kernel memory protection not selected by kernel config.\n"); + } else { + pr_warn("This architecture does not have kernel memory protection.\n"); + } } -#elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX) -static inline void mark_readonly(void) -{ - pr_warn("Kernel memory protection not selected by kernel config.\n"); -} -#else -static inline void mark_readonly(void) -{ - pr_warn("This architecture does not have kernel memory protection.\n"); -} -#endif void __weak free_initmem(void) { -- 2.41.0