Re: [PATCH] powerpc/irq: Modernise inline assembly in irq_soft_mask_{set,return}
On Sat Sep 24, 2022 at 8:15 AM AEST, Segher Boessenkool wrote:
> On Sat, Sep 24, 2022 at 02:26:52AM +1000, Nicholas Piggin wrote:
> > I still don't see what clauses guarantees asm("%0" ::"r"(foo)) to give
> > 13. It doesn't say access via inline assembly is special,
>
> But it is. It is for all register variables, local and global. I agree
> this isn't documented clearly. For local register variables this is the
> *only* thing guaranteed; for global register vars there is more (it
> changes the ABI, there are safe/restore effects, that kind of thing).
>
> Never it is guaranteed that all accesses through this variable will use
> the register directly: this fundamentally cannot work on all archs, and
> also not at -O0. More in general it doesn't work if some basic
> optimisations are not done, be it because of a compiler deficiency, or a
> straight out bug, or maybe it is a conscious choice in some cases.
Right, and we know better than to rely on a spec that is not 100% air
tight with no possibility of lawyering. This may be what the intention is,
it may be what gcc and clang do now, and everybody involved today agrees
with that interpretation. We still have to maintain the kernel tomorrow
though, so explicit r13 it must be.
>
> > I think if it was obviously guaranteed then this might be marginally
> > better than explicit r13 in the asm
> >
> >asm volatile(
> >"stb %0,%2(%1)"
> >:
> >: "r" (mask),
> > "r" (local_paca),
> > "i" (offsetof(struct paca_struct, irq_soft_mask))
> >: "memory");
>
> (Please use "n" instead of "i". Doesn't matter here, but it does in
> many other places.)
What is the difference? Just "i" allows assmebly-time constants?
How about "I"? that looks like it was made for it. Gives much better
errors.
Thanks,
Nick
Re: [PATCH] powerpc/pseries: Move vas_migration_handler early during migration
Haren Myneni writes: > On Thu, 2022-09-22 at 07:14 -0500, Nathan Lynch wrote: >> Haren Myneni writes: >> > When the migration is initiated, the hypervisor changes VAS >> > mappings as part of pre-migration event. Then the OS gets the >> > migration event which closes all VAS windows before the migration >> > starts. NX generates continuous faults until windows are closed >> > and the user space can not differentiate these NX faults coming >> > from the actual migration. So to reduce this time window, close >> > VAS windows first in pseries_migrate_partition(). >> >> I'm concerned that this is only narrowing a window of time where >> undesirable faults occur, and that it may not be sufficient for all >> configurations. Migrations can be in progress for minutes or hours, >> while the time that we wait for the VASI state transition is usually >> seconds or minutes. So I worry that this works around a problem in >> limited cases but doesn't cover them all. >> >> Maybe I don't understand the problem well enough. How does user space >> respond to the NX faults? > > The user space resend the request to NX whenever the request is > returned with NX fault. So the process should be same even for faults > caused by the pre-migration. > > Whereas the paste will be returned with failure when the window is > closed (unmap the paste address) and it can be considered as NX busy. > Up to the user space whether to send the request again after some delay > or fall back to SW compression and send the request again later. > > For the migration, pre-migration event is notified to the hypervisor > and then OS will receive the migration event (SUSPEND) - So this patch > close windows early before VASI so that removing NX fault handling > during the time taken for VASI state transistion. OK, so we can consider this a quality of implementation improvement that allows better behavior and less wasted retries for NX clients in a migration scenario, but there's not a correctness issue, really. With that clarified, I've confirmed that the slightly altered control flow and error handling in pseries_migrate_partition() look correct after your change. Reviewed-by: Nathan Lynch
Re: [PATCH] powerpc/irq: Modernise inline assembly in irq_soft_mask_{set,return}
On Sat, Sep 24, 2022 at 02:26:52AM +1000, Nicholas Piggin wrote:
> I still don't see what clauses guarantees asm("%0" ::"r"(foo)) to give
> 13. It doesn't say access via inline assembly is special,
But it is. It is for all register variables, local and global. I agree
this isn't documented clearly. For local register variables this is the
*only* thing guaranteed; for global register vars there is more (it
changes the ABI, there are safe/restore effects, that kind of thing).
Never it is guaranteed that all accesses through this variable will use
the register directly: this fundamentally cannot work on all archs, and
also not at -O0. More in general it doesn't work if some basic
optimisations are not done, be it because of a compiler deficiency, or a
straight out bug, or maybe it is a conscious choice in some cases.
> I think if it was obviously guaranteed then this might be marginally
> better than explicit r13 in the asm
>
>asm volatile(
>"stb %0,%2(%1)"
>:
>: "r" (mask),
>"r" (local_paca),
> "i" (offsetof(struct paca_struct, irq_soft_mask))
>: "memory");
(Please use "n" instead of "i". Doesn't matter here, but it does in
many other places.)
Segher
[linux-next:master] BUILD REGRESSION aaa11ce2ffc84166d11c4d2ac88c3fcf75425fbd
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: aaa11ce2ffc84166d11c4d2ac88c3fcf75425fbd Add linux-next specific files for 20220923 Error/Warning reports: https://lore.kernel.org/linux-doc/202209231933.vcyettul-...@intel.com https://lore.kernel.org/linux-mm/202209042337.fqi69rlv-...@intel.com https://lore.kernel.org/linux-mm/202209060229.dvuyxjbv-...@intel.com https://lore.kernel.org/linux-mm/202209150141.wgbakqmx-...@intel.com https://lore.kernel.org/linux-mm/202209200603.hpvoa8ii-...@intel.com https://lore.kernel.org/linux-mm/202209200949.vl3xruyd-...@intel.com https://lore.kernel.org/llvm/202209220019.yr2vuxhg-...@intel.com Error/Warning: (recently discovered and may have been fixed) ERROR: modpost: "devm_ioremap_resource" [drivers/dma/fsl-edma.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/dma/idma64.ko] undefined! ERROR: modpost: "devm_ioremap_resource" [drivers/dma/qcom/hdma.ko] undefined! ERROR: modpost: "devm_memremap" [drivers/misc/open-dice.ko] undefined! ERROR: modpost: "devm_memunmap" [drivers/misc/open-dice.ko] undefined! ERROR: modpost: "devm_platform_ioremap_resource" [drivers/char/xillybus/xillybus_of.ko] undefined! ERROR: modpost: "devm_platform_ioremap_resource" [drivers/clk/xilinx/clk-xlnx-clock-wizard.ko] undefined! ERROR: modpost: "ioremap" [drivers/net/ethernet/8390/pcnet_cs.ko] undefined! ERROR: modpost: "ioremap" [drivers/tty/ipwireless/ipwireless.ko] undefined! ERROR: modpost: "iounmap" [drivers/net/ethernet/8390/pcnet_cs.ko] undefined! ERROR: modpost: "iounmap" [drivers/tty/ipwireless/ipwireless.ko] undefined! Warning: Documentation/translations/zh_CN/devicetree/kernel-api.rst references a file that doesn't exist: Documentation/Devicetree/kernel-api.rst arch/arm64/kernel/alternative.c:199:6: warning: no previous prototype for 'apply_alternatives_vdso' [-Wmissing-prototypes] arch/arm64/kernel/alternative.c:295:14: warning: no previous prototype for 'alt_cb_patch_nops' [-Wmissing-prototypes] arch/parisc/lib/iomap.c:363:5: warning: no previous prototype for 'ioread64_lo_hi' [-Wmissing-prototypes] arch/parisc/lib/iomap.c:373:5: warning: no previous prototype for 'ioread64_hi_lo' [-Wmissing-prototypes] arch/parisc/lib/iomap.c:448:6: warning: no previous prototype for 'iowrite64_lo_hi' [-Wmissing-prototypes] arch/parisc/lib/iomap.c:454:6: warning: no previous prototype for 'iowrite64_hi_lo' [-Wmissing-prototypes] drivers/scsi/qla2xxx/qla_os.c:2854:23: warning: assignment to 'struct trace_array *' from 'int' makes pointer from integer without a cast [-Wint-conversion] drivers/scsi/qla2xxx/qla_os.c:2854:25: error: implicit declaration of function 'trace_array_get_by_name'; did you mean 'trace_array_set_clr_event'? [-Werror=implicit-function-declaration] drivers/scsi/qla2xxx/qla_os.c:2869:9: error: implicit declaration of function 'trace_array_put' [-Werror=implicit-function-declaration] grep: smatch_trinity_*: No such file or directory make[5]: *** No rule to make target 'drivers/crypto/aspeed/aspeed_crypto.o', needed by 'drivers/crypto/aspeed/'. mm/hugetlb.c:5539:14: warning: variable 'reserve_alloc' set but not used [-Wunused-but-set-variable] Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-scsi-qla2xxx-qla_os.c:error:implicit-declaration-of-function-trace_array_get_by_name | |-- drivers-scsi-qla2xxx-qla_os.c:error:implicit-declaration-of-function-trace_array_put | `-- drivers-scsi-qla2xxx-qla_os.c:warning:assignment-to-struct-trace_array-from-int-makes-pointer-from-integer-without-a-cast |-- alpha-randconfig-c042-20220923 | `-- make:No-rule-to-make-target-drivers-crypto-aspeed-aspeed_crypto.o-needed-by-drivers-crypto-aspeed-. |-- arc-randconfig-s031-20220923 | |-- drivers-gpu-drm-vkms-vkms_formats.c:sparse:sparse:cast-to-restricted-__le16 | |-- drivers-gpu-drm-vkms-vkms_formats.c:sparse:sparse:incorrect-type-in-assignment-(different-base-types)-expected-unsigned-short-usertype-got-restricted-__le16-usertype | |-- kernel-exit.c:sparse:sparse:incorrect-type-in-initializer-(different-address-spaces)-expected-struct-sighand_struct-sighand-got-struct-sighand_struct-noderef-__rcu-sighand | `-- sound-soc-generic-simple-card-utils.c:sparse:sparse:incorrect-type-in-initializer-(different-base-types)-expected-unsigned-int-usertype-val-got-restricted-snd_pcm_format_t-usertype |-- arm64-allnoconfig | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-alt_cb_patch_nops | `-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-apply_alternatives_vdso |-- arm64-allyesconfig | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-alt_cb_patch_nops | |-- arch-arm64-kernel-alternative.c:warning:no-previous-prototype-for-apply_alternatives_vdso | `-- mm-hugetlb.c
Re: [PATCH 5.15 0/6] arm64: kexec_file: use more system keyrings to verify kernel image signature + dependencies
Hello,
On Fri, Sep 23, 2022 at 03:03:36PM -0400, Mimi Zohar wrote:
> On Fri, 2022-09-23 at 19:10 +0200, Michal Suchanek wrote:
> > Hello,
> >
> > this is backport of commit 0d519cadf751
> > ("arm64: kexec_file: use more system keyrings to verify kernel image
> > signature")
> > to table 5.15 tree including the preparatory patches.
> >
> > Some patches needed minor adjustment for context.
>
> In general when backporting this patch set, there should be a
> dependency on backporting these commits as well. In this instance for
> linux-5.15.y, they've already been backported.
>
> 543ce63b664e ("lockdown: Fix kexec lockdown bypass with ima policy")
> af16df54b89d ("ima: force signature verification when CONFIG_KEXEC_SIG is
> configured")
Thanks for bringing these up. It might be in general useful to backport
these fixes as well.
However, this patchset does one very specific thing: it lifts the x86
kexec_file signature verification to arch-independent and uses it on
arm64 to unify all features (and any existing warts) between EFI
architectures.
So unless I am missing something the fixes you pointed out are
completely independent of this.
Thanks
Michal
Re: [PATCH 5.15 0/6] arm64: kexec_file: use more system keyrings to verify kernel image signature + dependencies
On Fri, 2022-09-23 at 19:10 +0200, Michal Suchanek wrote:
> Hello,
>
> this is backport of commit 0d519cadf751
> ("arm64: kexec_file: use more system keyrings to verify kernel image
> signature")
> to table 5.15 tree including the preparatory patches.
>
> Some patches needed minor adjustment for context.
In general when backporting this patch set, there should be a
dependency on backporting these commits as well. In this instance for
linux-5.15.y, they've already been backported.
543ce63b664e ("lockdown: Fix kexec lockdown bypass with ima policy")
af16df54b89d ("ima: force signature verification when CONFIG_KEXEC_SIG is
configured")
--
thanks,
Mimi
Re: [PATCH] Revert "ASoC: fsl_audmix: make clock and output src write only"
On Tue, 6 Sep 2022 14:49:21 +0800, Shengjiu Wang wrote: > This reverts commit 944c517b8c838832a166f1c89afbf8724f4a6b49. > > There is error after making clock and output src write only > > $amixer -c imxaudmix cset numid=1 1 > amixer: Cannot read the given element from control sysdefault:3 > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] Revert "ASoC: fsl_audmix: make clock and output src write only" commit: 086ceada2107b482df437d76f581062b547eb7f2 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Re: [PATCH 2/2] powerpc/rtas: block error injection when locked down
On Fri, Sep 23, 2022 at 11:40 AM Nathan Lynch wrote:
> Michael Ellerman writes:
> > Paul Moore writes:
> >> On Thu, Sep 22, 2022 at 3:38 PM Nathan Lynch wrote:
> >>>
> >>> The error injection facility on pseries VMs allows corruption of
> >>> arbitrary guest memory, potentially enabling a sufficiently privileged
> >>> user to disable lockdown or perform other modifications of the running
> >>> kernel via the rtas syscall.
> >>>
> >>> Block the PAPR error injection facility from being opened or called
> >>> when locked down.
> >>>
> >>> Signed-off-by: Nathan Lynch
> >>> ---
> >>> arch/powerpc/kernel/rtas.c | 25 -
> >>> include/linux/security.h | 1 +
> >>> security/security.c| 1 +
> >>> 3 files changed, 26 insertions(+), 1 deletion(-)
> >>
> >> ...
> >>
> >>> diff --git a/include/linux/security.h b/include/linux/security.h
> >>> index 1ca8dbacd3cc..b5d5138ae66a 100644
> >>> --- a/include/linux/security.h
> >>> +++ b/include/linux/security.h
> >>> @@ -123,6 +123,7 @@ enum lockdown_reason {
> >>> LOCKDOWN_BPF_WRITE_USER,
> >>> LOCKDOWN_DBG_WRITE_KERNEL,
> >>> LOCKDOWN_DEVICE_TREE,
> >>> + LOCKDOWN_RTAS_ERROR_INJECTION,
> >>
> >> With the understanding that I've never heard of RTAS until now, are
> >> there any other RTAS events that would require a lockdown reason? As
> >> a follow up, is it important to distinguish between different RTAS
> >> lockdown reasons?
>
> 1. Not to my current knowledge.
> 2. Yes, I think so, see below.
>
> >>
> >> I'm trying to determine if we can just call it LOCKDOWN_RTAS.
> >
> > Yes I think we should.
> >
> > Currently it only locks down the error injection calls, not all of RTAS.
> >
> > But firmware can/will add new RTAS calls in future, so it's always
> > possible something will need to be added to the list of things that need
> > to be blocked during lockdown.
> >
> > So I think calling it LOCKDOWN_RTAS would be more general and future
> > proof, and can be read to mean "lockdown the parts of RTAS that need
> > to be locked down".
>
> RTAS provides callable interfaces for a broad variety of functions,
> including device configuration, halt/reboot/suspend, CPU online/offline,
> NVRAM access, firmware upgrade, platform diagnostic data retrieval, and
> others.
>
> Currently I don't know of other RTAS-provided functions that should be
> restricted. But if we were to add more, then -- in answer to Paul -- yes
> I think it would be important to have distinct reasons and
> messages. Taking the point of view of someone diagnosing lockdown denial
> messages and relating them to kernel code and user space activity, I
> would rather we err toward specificity.
As I said before, RTAS is a great mystery to me, if it can be extended
in the future then having a targeted lockdown name makes perfect
sense.
> So a single RTAS catch-all lockdown reason doesn't appeal to me, but
> lockdown reasons and messages aren't ABI (right?) ...
Correct. Or at least that is my understanding, but there have been
some odd rulings on lockdown in the past so my advice would be to make
*very* sure you get this right the first time. From what you and
Michael have said, it seems like a function specific name is the way
to go here, and based on your explanations of the situation it seems
like putting this in the integrity bin is the right way to go.
--
paul-moore.com
[PATCH 5.15 3/6] kexec: drop weak attribute from functions
From: "Naveen N. Rao"
commit 0738eceb6201691534df07e0928d0a6168a35787 upstream.
Drop __weak attribute from functions in kexec_core.c:
- machine_kexec_post_load()
- arch_kexec_protect_crashkres()
- arch_kexec_unprotect_crashkres()
- crash_free_reserved_phys_range()
Link:
https://lkml.kernel.org/r/c0f6219e03cb399d166d518ab505095218a902dd.1656659357.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Naveen N. Rao
Suggested-by: Eric Biederman
Signed-off-by: Andrew Morton
Signed-off-by: Mimi Zohar
Signed-off-by: Michal Suchanek
---
arch/arm64/include/asm/kexec.h | 18 --
arch/powerpc/include/asm/kexec.h | 5 +
arch/s390/include/asm/kexec.h| 11 +++
arch/x86/include/asm/kexec.h | 6 ++
include/linux/kexec.h| 32
kernel/kexec_core.c | 27 ---
6 files changed, 66 insertions(+), 33 deletions(-)
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 91d81824f869..ae3695a15610 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -84,12 +84,28 @@ static inline void crash_setup_regs(struct pt_regs *newregs,
extern bool crash_is_nosave(unsigned long pfn);
extern void crash_prepare_suspend(void);
extern void crash_post_resume(void);
+
+void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
+#define crash_free_reserved_phys_range crash_free_reserved_phys_range
#else
static inline bool crash_is_nosave(unsigned long pfn) {return false; }
static inline void crash_prepare_suspend(void) {}
static inline void crash_post_resume(void) {}
#endif
+struct kimage;
+
+#if defined(CONFIG_KEXEC_CORE)
+int machine_kexec_post_load(struct kimage *image);
+#define machine_kexec_post_load machine_kexec_post_load
+
+void arch_kexec_protect_crashkres(void);
+#define arch_kexec_protect_crashkres arch_kexec_protect_crashkres
+
+void arch_kexec_unprotect_crashkres(void);
+#define arch_kexec_unprotect_crashkres arch_kexec_unprotect_crashkres
+#endif
+
#define ARCH_HAS_KIMAGE_ARCH
struct kimage_arch {
@@ -101,8 +117,6 @@ struct kimage_arch {
#ifdef CONFIG_KEXEC_FILE
extern const struct kexec_file_ops kexec_image_ops;
-struct kimage;
-
int arch_kimage_file_post_load_cleanup(struct kimage *image);
#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 6152fa220054..d8394e77e987 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -97,6 +97,11 @@ static inline bool kdump_in_progress(void)
void relocate_new_kernel(unsigned long indirection_page, unsigned long
reboot_code_buffer,
unsigned long start_address) __noreturn;
+#if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_PPC_RTAS)
+void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
+#define crash_free_reserved_phys_range crash_free_reserved_phys_range
+#endif
+
#ifdef CONFIG_KEXEC_FILE
extern const struct kexec_file_ops kexec_elf64_ops;
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index d13bd221cd37..4f713092e68c 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -85,6 +85,17 @@ struct kimage_arch {
extern const struct kexec_file_ops s390_kexec_image_ops;
extern const struct kexec_file_ops s390_kexec_elf_ops;
+#ifdef CONFIG_CRASH_DUMP
+void crash_free_reserved_phys_range(unsigned long begin, unsigned long end);
+#define crash_free_reserved_phys_range crash_free_reserved_phys_range
+
+void arch_kexec_protect_crashkres(void);
+#define arch_kexec_protect_crashkres arch_kexec_protect_crashkres
+
+void arch_kexec_unprotect_crashkres(void);
+#define arch_kexec_unprotect_crashkres arch_kexec_unprotect_crashkres
+#endif
+
#ifdef CONFIG_KEXEC_FILE
struct purgatory_info;
int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index 5b6e2ae54906..4fd92330f23d 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -186,6 +186,12 @@ extern int arch_kexec_post_alloc_pages(void *vaddr,
unsigned int pages,
extern void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages);
#define arch_kexec_pre_free_pages arch_kexec_pre_free_pages
+void arch_kexec_protect_crashkres(void);
+#define arch_kexec_protect_crashkres arch_kexec_protect_crashkres
+
+void arch_kexec_unprotect_crashkres(void);
+#define arch_kexec_unprotect_crashkres arch_kexec_unprotect_crashkres
+
#ifdef CONFIG_KEXEC_FILE
struct purgatory_info;
int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index f1e5327a7bf8..1638c8d7d216 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -384,7 +384,10 @@ extern void machine_kexec_cleanup(struct kimage *image);
extern
[PATCH 5.15 2/6] kexec_file: drop weak attribute from functions
From: "Naveen N. Rao"
commit 65d9a9a60fd71be964effb2e94747a6acb6e7015 upstream.
As requested
(http://lkml.kernel.org/r/87ee0q7b92@email.froward.int.ebiederm.org),
this series converts weak functions in kexec to use the #ifdef approach.
Quoting the 3e35142ef99fe ("kexec_file: drop weak attribute from
arch_kexec_apply_relocations[_add]") changelog:
: Since commit d1bcae833b32f1 ("ELF: Don't generate unused section symbols")
: [1], binutils (v2.36+) started dropping section symbols that it thought
: were unused. This isn't an issue in general, but with kexec_file.c, gcc
: is placing kexec_arch_apply_relocations[_add] into a separate
: .text.unlikely section and the section symbol ".text.unlikely" is being
: dropped. Due to this, recordmcount is unable to find a non-weak symbol in
: .text.unlikely to generate a relocation record against.
This patch (of 2);
Drop __weak attribute from functions in kexec_file.c:
- arch_kexec_kernel_image_probe()
- arch_kimage_file_post_load_cleanup()
- arch_kexec_kernel_image_load()
- arch_kexec_locate_mem_hole()
- arch_kexec_kernel_verify_sig()
arch_kexec_kernel_image_load() calls into kexec_image_load_default(), so
drop the static attribute for the latter.
arch_kexec_kernel_verify_sig() is not overridden by any architecture, so
drop the __weak attribute.
Link:
https://lkml.kernel.org/r/cover.1656659357.git.naveen.n@linux.vnet.ibm.com
Link:
https://lkml.kernel.org/r/2cd7ca1fe4d6bb6ca38e3283c717878388ed6788.1656659357.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Naveen N. Rao
Suggested-by: Eric Biederman
Signed-off-by: Andrew Morton
Signed-off-by: Mimi Zohar
Signed-off-by: Michal Suchanek
---
arch/arm64/include/asm/kexec.h | 4 ++-
arch/powerpc/include/asm/kexec.h | 9 +++
arch/s390/include/asm/kexec.h| 3 +++
arch/x86/include/asm/kexec.h | 6 +
include/linux/kexec.h| 44 +++-
kernel/kexec_file.c | 35 ++---
6 files changed, 61 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 00dbcc71aeb2..91d81824f869 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -103,7 +103,9 @@ extern const struct kexec_file_ops kexec_image_ops;
struct kimage;
-extern int arch_kimage_file_post_load_cleanup(struct kimage *image);
+int arch_kimage_file_post_load_cleanup(struct kimage *image);
+#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
+
extern int load_other_segments(struct kimage *image,
unsigned long kernel_load_addr, unsigned long kernel_size,
char *initrd, unsigned long initrd_len,
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 88d0d7cf3a79..6152fa220054 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -119,6 +119,15 @@ int setup_purgatory(struct kimage *image, const void
*slave_code,
#ifdef CONFIG_PPC64
struct kexec_buf;
+int arch_kexec_kernel_image_probe(struct kimage *image, void *buf, unsigned
long buf_len);
+#define arch_kexec_kernel_image_probe arch_kexec_kernel_image_probe
+
+int arch_kimage_file_post_load_cleanup(struct kimage *image);
+#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
+
+int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
+#define arch_kexec_locate_mem_hole arch_kexec_locate_mem_hole
+
int load_crashdump_segments_ppc64(struct kimage *image,
struct kexec_buf *kbuf);
int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 63098df81c9f..d13bd221cd37 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -92,5 +92,8 @@ int arch_kexec_apply_relocations_add(struct purgatory_info
*pi,
const Elf_Shdr *relsec,
const Elf_Shdr *symtab);
#define arch_kexec_apply_relocations_add arch_kexec_apply_relocations_add
+
+int arch_kimage_file_post_load_cleanup(struct kimage *image);
+#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
#endif
#endif /*_S390_KEXEC_H */
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index c7c924e15011..5b6e2ae54906 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -193,6 +193,12 @@ int arch_kexec_apply_relocations_add(struct purgatory_info
*pi,
const Elf_Shdr *relsec,
const Elf_Shdr *symtab);
#define arch_kexec_apply_relocations_add arch_kexec_apply_relocations_add
+
+void *arch_kexec_kernel_image_load(struct kimage *image);
+#define arch_kexec_kernel_image_load arch_kexec_kernel_image_load
+
+int arch_kimage_file_post_load_cleanup(struct kimage *image);
+#define
[PATCH 5.15 0/6] arm64: kexec_file: use more system keyrings to verify kernel image signature + dependencies
Hello,
this is backport of commit 0d519cadf751
("arm64: kexec_file: use more system keyrings to verify kernel image signature")
to table 5.15 tree including the preparatory patches.
Some patches needed minor adjustment for context.
Thanks
Michal
Coiby Xu (3):
kexec: clean up arch_kexec_kernel_verify_sig
kexec, KEYS: make the code in bzImage64_verify_sig generic
arm64: kexec_file: use more system keyrings to verify kernel image
signature
Naveen N. Rao (2):
kexec_file: drop weak attribute from functions
kexec: drop weak attribute from functions
Sven Schnelle (1):
s390/kexec_file: move kernel image size check
arch/arm64/include/asm/kexec.h| 20 ++-
arch/arm64/kernel/kexec_image.c | 11 +---
arch/powerpc/include/asm/kexec.h | 14 +
arch/s390/boot/head.S | 2 -
arch/s390/include/asm/kexec.h | 14 +
arch/s390/include/asm/setup.h | 1 -
arch/s390/kernel/machine_kexec_file.c | 17 +-
arch/x86/include/asm/kexec.h | 12
arch/x86/kernel/kexec-bzimage64.c | 20 +--
include/linux/kexec.h | 82 ++
kernel/kexec_core.c | 27 -
kernel/kexec_file.c | 83 ++-
12 files changed, 163 insertions(+), 140 deletions(-)
--
2.35.3
Re: [PATCH] ASoC: fsl: Remove unused inline function imx_pcm_dma_params_init_data()
On Fri, 23 Sep 2022 17:03:55 +0800, Gaosheng Cui wrote:
> The imx_pcm_dma_params_init_data() are no longer used since
> commit c31da0b196f9 ("ASoC: imx-ssi: Remove unused driver"),
> and the function is used to initialize some members of
> "struct imx_dma_data", it's more readable to assign the value
> directly, imx_pcm_dma_params_init_data is useless, so remove it.
>
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl: Remove unused inline function imx_pcm_dma_params_init_data()
commit: 4f865485e8ef1d04de23fc1def1fa4e39fb00b91
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
Re: [PATCH][next] powerpc: Fix fall-through warning for Clang
Applied to powerpc/next. Great. :) Thanks, Michael. -- Gustavo
Re: [RFC PATCH 2/2] powerpc: nop trap instruction after WARN_ONCE fires
Le 23/09/2022 à 17:41, Nicholas Piggin a écrit :
> WARN_ONCE and similar are often used in frequently executed code, and
> should not crash the system. The program check interrupt caused by
> WARN_ON_ONCE can be a significant overhead even when nothing is being
> printed. This can cause performance to become unacceptable, having the
> same effective impact to the user as a BUG_ON().
>
> Avoid this overhead by patching the trap with a nop instruction after a
> "once" trap fires. Conditional warnings that return a result must have
> equivalent compare and branch instructions after the trap, so when it is
> nopped the statement will behave the same way. It's possible the asm
> goto should be removed entirely and this comparison just done in C now.
You mean, just like PPC32 ? (Since db87a7199229 ("powerpc/bug: Remove
specific powerpc BUG_ON() and WARN_ON() on PPC32"))
But I'm having hard time with your change.
You change only WARN_ON()
But WARN_ON_ONCE() calls __WARN_FLAGS()
And WARN_ONCE() calls WARN() via DO_ONCE_LITE_IF()
So I don't see any ..._ONCE something going with WARN_ON().
Am I missing something ?
Otherwise, what you want to do is to patch the 'twi' in __WARN_FLAGS()
by a non conditional branch to __label_warn_on .
Christophe
Re: [PATCH] powerpc/irq: Modernise inline assembly in irq_soft_mask_{set,return}
On Fri Sep 23, 2022 at 10:18 PM AEST, Segher Boessenkool wrote:
> On Fri, Sep 23, 2022 at 05:08:13PM +1000, Nicholas Piggin wrote:
> > On Tue Sep 20, 2022 at 4:41 PM AEST, Christophe Leroy wrote:
> > > local_paca is declared as global register asm("r13"), it is therefore
> > > garantied to always ever be r13.
> > >
> > > It is therefore not required to opencode r13 in the assembly, use
> > > a reference to local_paca->irq_soft_mask instead.
>
> > The code matches the changelog AFAIKS. But I don't know where it is
> > guaranteed it will always be r13 in GCC and Clang. I still don't know
> > where in the specification or documentation suggests this.
>
> "Global Register Variables" in the GCC manual.
>
> > There was some assertion it would always be r13, but that can't be a
> > *general* rule. e.g., the following code:
> >
> > struct foo {
> > #ifdef BIGDISP
> > int array[1024*1024];
> > #endif
> > char bar;
> > };
> >
> > register struct foo *foo asm("r13");
> >
> > static void setval(char val)
> > {
> > asm("stb%X0 %1,%0" : "=m" (foo->bar) : "r" (val));
> > }
> >
> > int main(void)
> > {
> > setval(10);
> > }
>
> Just use r13 directly in the asm, if that is what you want!
>
> > With -O0 this generates stb 9,0(10) for me for GCC 12, and with -O2
> > -DBIGDISP it generates stb 10,0(9). So that makes me nervious. GCC
> > does not have some kind of correctness guarantee here, so it must not
> > have this in its regression tests etc., and who knows about clang.
>
> GCC has all kinds of correctness guarantees, here and elsewhere, that is
> 90% of a compiler's job. But you don't *tell* it what you consider
> "correct" here.
Right, that's what I expect. I think the confusion came from here,
https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-August/247595.html
In any case it is answered now.
> You wrote "foo->bar", and this expression was translated to something
> that derived from r13. If you made the asm something like
> asm("stb%X0 %1,0(%0)" : : "r" (foo), "r" (val) : "memory");
> it would work fine. It would also work fine if you wrote 13 in the
> template directly. These things follow the rules, so are guaranteed.
>
> The most important pieces of doc here may be
>* Accesses to the variable may be optimized as usual and the register
> remains available for allocation and use in any computations,
> provided that observable values of the variable are not affected.
>* If the variable is referenced in inline assembly, the type of
> access must be provided to the compiler via constraints (*note
> Constraints::). Accesses from basic asms are not supported.
> but read the whole "Global Register Variables" chapter?
I still don't see what clauses guarantees asm("%0" ::"r"(foo)) to give
13. It doesn't say access via inline assembly is special, so if
optimized as usual means it could be accessed by any register like
access to a usual variable, then asm could also substitute a different
register for the access by the letter of it AFAIKS.
I think if it was obviously guaranteed then this might be marginally
better than explicit r13 in the asm
asm volatile(
"stb %0,%2(%1)"
:
: "r" (mask),
"r" (local_paca),
"i" (offsetof(struct paca_struct, irq_soft_mask))
: "memory");
But as it is I think we should just stick with explicit r13 base
in the asm.
Thanks,
Nick
Re: [PATCH 1/2] powerpc/pseries: block untrusted device tree changes when locked down
Paul Moore writes:
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 7bd0c490703d..1ca8dbacd3cc 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -122,6 +122,7 @@ enum lockdown_reason {
>> LOCKDOWN_XMON_WR,
>> LOCKDOWN_BPF_WRITE_USER,
>> LOCKDOWN_DBG_WRITE_KERNEL,
>> + LOCKDOWN_DEVICE_TREE,
>
> I would suggest moving LOCKDOWN_DEVICE_TREE to be next to
> LOCKDOWN_ACPI_TABLES. It's not a hard requirement, but it seems like
> a nice idea to group similar things when we can.
>
>> LOCKDOWN_INTEGRITY_MAX,
>> LOCKDOWN_KCORE,
>> LOCKDOWN_KPROBES,
>> diff --git a/security/security.c b/security/security.c
>> index 4b95de24bc8d..2863fc31eec6 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -60,6 +60,7 @@ const char *const
>> lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
>> [LOCKDOWN_XMON_WR] = "xmon write access",
>> [LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
>> [LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write kernel RAM",
>> + [LOCKDOWN_DEVICE_TREE] = "modifying device tree contents",
>
> Might as well move this one too.
Yes, I can do that for v2. Thanks.
Re: [RFC PATCH 1/2] powerpc: Don't use extable for WARN
Le 23/09/2022 à 17:41, Nicholas Piggin a écrit : > extable is used for handling user memory access faults from kernel mode, > as such it is a fast-ish path. Using it in the very slow WARN path > increases the number of extable entries from 1306 to 6516 in a > ppc64le_defconfig vmlinux, increasing the average number of search > steps by ~2.3. > > This patch adds a recovery address to the bug_entry struct instead of > using the extable. The size of the bug entry table plus the extable > go from 137kB to 126kB. That's fine, but it means that some additional work will have to be done in objtool. Christophe
[RFC PATCH 2/2] powerpc: nop trap instruction after WARN_ONCE fires
WARN_ONCE and similar are often used in frequently executed code, and
should not crash the system. The program check interrupt caused by
WARN_ON_ONCE can be a significant overhead even when nothing is being
printed. This can cause performance to become unacceptable, having the
same effective impact to the user as a BUG_ON().
Avoid this overhead by patching the trap with a nop instruction after a
"once" trap fires. Conditional warnings that return a result must have
equivalent compare and branch instructions after the trap, so when it is
nopped the statement will behave the same way. It's possible the asm
goto should be removed entirely and this comparison just done in C now.
XXX: possibly this should schedule the patching to run in a different
context than the program check.
XXX: patching process could allocate memory for patched bugs rather
than keeping a word in the bug entry for all of them. Very few will
ever fire.
XXX: is concurrency okay?
XXX: could avoid clobbering cc in some cases, possibly optimise some
variants.
---
arch/powerpc/include/asm/bug.h | 14 ++--
arch/powerpc/kernel/traps.c| 42 +-
include/asm-generic/bug.h | 9
include/linux/bug.h| 1 +
lib/bug.c | 22 +-
5 files changed, 74 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index dec73137fea0..0ea7b9fe0305 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -14,6 +14,7 @@
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
+.4byte 0
.4byte 0
.4byte 5002f - .
.short \line, \flags
@@ -27,6 +28,7 @@
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
+.4byte 0
.4byte 0
.short \flags
.org 5001b+BUG_ENTRY_SIZE
@@ -48,6 +50,7 @@
#else /* !__ASSEMBLY__ */
struct arch_bug_entry {
signed int recovery_addr_disp;
+ unsigned int insn;
};
/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
@@ -57,6 +60,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte 0\n" \
+ " .4byte 0\n" \
" .4byte %0 - .\n"\
" .short %1, %2\n"\
".org 2b+%3\n" \
@@ -66,6 +70,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte 0\n" \
+ " .4byte 0\n" \
" .short %2\n"\
".org 2b+%3\n" \
".previous\n"
@@ -76,6 +81,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte 1b - %l[" #label "]\n" \
+ " .4byte 0\n" \
" .4byte %0 - .\n"\
" .short %1, %2\n"\
".org 2b+%3\n" \
@@ -85,6 +91,7 @@ struct arch_bug_entry {
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte 1b - %l[" #label "]\n" \
+ " .4byte 0\n" \
" .short %2\n"\
".org 2b+%3\n" \
".previous\n"
@@ -106,7 +113,7 @@ struct arch_bug_entry {
: : "i" (__FILE__), "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
- ##__VA_ARGS__ : : label)
+ ##__VA_ARGS__ : "cr0" : label)
/*
* BUG_ON() and WARN_ON() do their best to cooperate with compile-time
@@ -151,7 +158,7 @@ __label_warn_on:
\
} else {\
__label__ __label_warn_on; \
\
- WARN_ENTRY(PPC_TLNEI " %4, 0", \
+ WARN_ENTRY(PPC_TLNEI " %4, 0 ; cmpdi %4, 0 ; bne
%l[__label_warn_on]", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN),
\
__label_warn_on, \
"r" ((__force long)(x)));\
@@ -178,6 +185,9 @@ __label_warn_on:
\
#define _EMIT_BUG_ENTRY
#define _EMIT_WARN_ENTRY
#endif
+#define arch_generic_bug_entry_clear_once
+void
[RFC PATCH 1/2] powerpc: Don't use extable for WARN
extable is used for handling user memory access faults from kernel mode,
as such it is a fast-ish path. Using it in the very slow WARN path
increases the number of extable entries from 1306 to 6516 in a
ppc64le_defconfig vmlinux, increasing the average number of search
steps by ~2.3.
This patch adds a recovery address to the bug_entry struct instead of
using the extable. The size of the bug entry table plus the extable
go from 137kB to 126kB.
Signed-off-by: Nicholas Piggin
---
arch/powerpc/Kconfig | 4
arch/powerpc/include/asm/bug.h | 31 ---
arch/powerpc/kernel/traps.c| 16 ++--
include/asm-generic/bug.h | 3 +++
4 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 4c466acdc70d..3b379fa15082 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -336,6 +336,10 @@ config GENERIC_BUG_RELATIVE_POINTERS
def_bool y
depends on GENERIC_BUG
+config GENERIC_BUG_ARCH_ENTRY
+ def_bool y
+ depends on GENERIC_BUG
+
config SYS_SUPPORTS_APM_EMULATION
default y if PMAC_APM_EMU
bool
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 61a4736355c2..dec73137fea0 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -14,6 +14,7 @@
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
+.4byte 0
.4byte 5002f - .
.short \line, \flags
.org 5001b+BUG_ENTRY_SIZE
@@ -26,6 +27,7 @@
.macro __EMIT_BUG_ENTRY addr,file,line,flags
.section __bug_table,"aw"
5001: .4byte \addr - .
+.4byte 0
.short \flags
.org 5001b+BUG_ENTRY_SIZE
.previous
@@ -33,7 +35,6 @@
#endif /* verbose */
.macro EMIT_WARN_ENTRY addr,file,line,flags
- EX_TABLE(\addr,\addr+4)
__EMIT_BUG_ENTRY \addr,\file,\line,\flags
.endm
@@ -45,12 +46,17 @@
.endm
#else /* !__ASSEMBLY__ */
+struct arch_bug_entry {
+ signed int recovery_addr_disp;
+};
+
/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
sizeof(struct bug_entry), respectively */
#ifdef CONFIG_DEBUG_BUGVERBOSE
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
+ " .4byte 0\n" \
" .4byte %0 - .\n"\
" .short %1, %2\n"\
".org 2b+%3\n" \
@@ -59,6 +65,26 @@
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
+ " .4byte 0\n" \
+ " .short %2\n"\
+ ".org 2b+%3\n" \
+ ".previous\n"
+#endif
+
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+#define _EMIT_WARN_ENTRY(label)\
+ ".section __bug_table,\"aw\"\n" \
+ "2: .4byte 1b - .\n"\
+ " .4byte 1b - %l[" #label "]\n" \
+ " .4byte %0 - .\n"\
+ " .short %1, %2\n"\
+ ".org 2b+%3\n" \
+ ".previous\n"
+#else
+#define _EMIT_WARN_ENTRY(label)\
+ ".section __bug_table,\"aw\"\n" \
+ "2: .4byte 1b - .\n"\
+ " .4byte 1b - %l[" #label "]\n" \
" .short %2\n"\
".org 2b+%3\n" \
".previous\n"
@@ -76,8 +102,7 @@
#define WARN_ENTRY(insn, flags, label, ...)\
asm_volatile_goto( \
"1: " insn "\n" \
- EX_TABLE(1b, %l[label]) \
- _EMIT_BUG_ENTRY \
+ _EMIT_WARN_ENTRY(label) \
: : "i" (__FILE__), "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index dadfcef5d6db..1e521a432bd0 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1494,13 +1494,17 @@ static void do_program_check(struct pt_regs *regs)
if (!(regs->msr & MSR_PR) && /* not user-mode */
report_bug(bugaddr, regs) == BUG_TRAP_TYPE_WARN) {
- const struct exception_table_entry *entry;
+ const struct bug_entry *bug;
+ unsigned long recov;
- entry = search_exception_tables(bugaddr);
- if (entry) {
- regs_set_return_ip(regs,
Re: [PATCH 2/2] powerpc/rtas: block error injection when locked down
Michael Ellerman writes:
> Paul Moore writes:
>> On Thu, Sep 22, 2022 at 3:38 PM Nathan Lynch wrote:
>>>
>>> The error injection facility on pseries VMs allows corruption of
>>> arbitrary guest memory, potentially enabling a sufficiently privileged
>>> user to disable lockdown or perform other modifications of the running
>>> kernel via the rtas syscall.
>>>
>>> Block the PAPR error injection facility from being opened or called
>>> when locked down.
>>>
>>> Signed-off-by: Nathan Lynch
>>> ---
>>> arch/powerpc/kernel/rtas.c | 25 -
>>> include/linux/security.h | 1 +
>>> security/security.c| 1 +
>>> 3 files changed, 26 insertions(+), 1 deletion(-)
>>
>> ...
>>
>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>> index 1ca8dbacd3cc..b5d5138ae66a 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -123,6 +123,7 @@ enum lockdown_reason {
>>> LOCKDOWN_BPF_WRITE_USER,
>>> LOCKDOWN_DBG_WRITE_KERNEL,
>>> LOCKDOWN_DEVICE_TREE,
>>> + LOCKDOWN_RTAS_ERROR_INJECTION,
>>
>> With the understanding that I've never heard of RTAS until now, are
>> there any other RTAS events that would require a lockdown reason? As
>> a follow up, is it important to distinguish between different RTAS
>> lockdown reasons?
1. Not to my current knowledge.
2. Yes, I think so, see below.
>>
>> I'm trying to determine if we can just call it LOCKDOWN_RTAS.
>
> Yes I think we should.
>
> Currently it only locks down the error injection calls, not all of RTAS.
>
> But firmware can/will add new RTAS calls in future, so it's always
> possible something will need to be added to the list of things that need
> to be blocked during lockdown.
>
> So I think calling it LOCKDOWN_RTAS would be more general and future
> proof, and can be read to mean "lockdown the parts of RTAS that need
> to be locked down".
RTAS provides callable interfaces for a broad variety of functions,
including device configuration, halt/reboot/suspend, CPU online/offline,
NVRAM access, firmware upgrade, platform diagnostic data retrieval, and
others.
Currently I don't know of other RTAS-provided functions that should be
restricted. But if we were to add more, then -- in answer to Paul -- yes
I think it would be important to have distinct reasons and
messages. Taking the point of view of someone diagnosing lockdown denial
messages and relating them to kernel code and user space activity, I
would rather we err toward specificity.
Also: LOCKDOWN_RTAS_ERROR_INJECTION is proposed for lockdown's integrity
mode. But consider that future RTAS-related additions could be proposed
for confidentiality mode, which is more restrictive. (A plausible
example could be platform dump retrieval.) We would need at least two
RTAS reasons and one wouldn't suffice.
So a single RTAS catch-all lockdown reason doesn't appeal to me, but
lockdown reasons and messages aren't ABI (right?) and we could
eventually change whatever decision we reach here. So if you both still
prefer a single LOCKDOWN_RTAS reason, I can do that for v2.
That said, I could see a case for instead adding
LOCKDOWN_HW_ERROR_INJECTION (without the RTAS), to indicate restriction
of hardware- or platform-level error injection. I was a little surprised
to find that an error injection reason doesn't already exist for the
ACPI-based facility (drivers/acpi/apei/einj.c), but since the user
interface is debugfs-based I suppose it's already restricted in practice
by LOCKDOWN_DEBUGFS.
Re: [PATCH] Revert "ASoC: fsl_audmix: make clock and output src write only"
On Tue, Sep 06, 2022 at 02:49:21PM +0800, Shengjiu Wang wrote: > This reverts commit 944c517b8c838832a166f1c89afbf8724f4a6b49. Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone. Please include human readable descriptions of things like commits and issues being discussed in e-mail in your mails, this makes them much easier for humans to read especially when they have no internet access. I do frequently catch up on my mail on flights or while otherwise travelling so this is even more pressing for me than just being about making things a bit easier to read. signature.asc Description: PGP signature
Re: [PATCH 1/2] powerpc/64s: Fix GENERIC_CPU build flags for PPC970 / G5
On Fri, Sep 23, 2022 at 05:17:45PM +1000, Nicholas Piggin wrote:
> On Thu Sep 22, 2022 at 4:50 AM AEST, Segher Boessenkool wrote:
> > On Wed, Sep 21, 2022 at 11:41:02AM +1000, Nicholas Piggin wrote:
> > > Big-endian GENERIC_CPU supports 970, but builds with -mcpu=power5.
> > > POWER5 is ISA v2.02 whereas 970 is v2.01 plus Altivec. 2.02 added
> > > the popcntb instruction which a compiler might use.
> > >
> > > Use -mcpu=power4.
> > >
> > > Fixes: 471d7ff8b51b ("powerpc/64s: Remove POWER4 support")
> > > Signed-off-by: Nicholas Piggin
> >
> > Reviewed-by: Segher Boessenkool
> >
> > Thank you!
> >
> > Maybe superfluous, but some more context: GCC's -mcpu=power4 means
> > POWER4+, ISA 2.01, according to our documentation. There is no
> > difference with ISA 2.00 (what plain POWER4 implements) for anything
> > GCC does.
>
> Huh, okay. Well I guess we are past that point now, interesting that
> another ISA version was done for 4+ though, and then another for 5.
> I don't see a list of changes from 2.00 in the public version, I
> wonder what else changed other than mtmsrd.
I think searching for "POWER4+" will give you everything. I cannot find
a public 2.00 either, yeah. I listed everything I think changed
elsewhere in the thread.
Segher
Re: [PATCH] powerpc/irq: Modernise inline assembly in irq_soft_mask_{set,return}
On Fri, Sep 23, 2022 at 05:08:13PM +1000, Nicholas Piggin wrote:
> On Tue Sep 20, 2022 at 4:41 PM AEST, Christophe Leroy wrote:
> > local_paca is declared as global register asm("r13"), it is therefore
> > garantied to always ever be r13.
> >
> > It is therefore not required to opencode r13 in the assembly, use
> > a reference to local_paca->irq_soft_mask instead.
> The code matches the changelog AFAIKS. But I don't know where it is
> guaranteed it will always be r13 in GCC and Clang. I still don't know
> where in the specification or documentation suggests this.
"Global Register Variables" in the GCC manual.
> There was some assertion it would always be r13, but that can't be a
> *general* rule. e.g., the following code:
>
> struct foo {
> #ifdef BIGDISP
> int array[1024*1024];
> #endif
> char bar;
> };
>
> register struct foo *foo asm("r13");
>
> static void setval(char val)
> {
> asm("stb%X0 %1,%0" : "=m" (foo->bar) : "r" (val));
> }
>
> int main(void)
> {
> setval(10);
> }
Just use r13 directly in the asm, if that is what you want!
> With -O0 this generates stb 9,0(10) for me for GCC 12, and with -O2
> -DBIGDISP it generates stb 10,0(9). So that makes me nervious. GCC
> does not have some kind of correctness guarantee here, so it must not
> have this in its regression tests etc., and who knows about clang.
GCC has all kinds of correctness guarantees, here and elsewhere, that is
90% of a compiler's job. But you don't *tell* it what you consider
"correct" here.
You wrote "foo->bar", and this expression was translated to something
that derived from r13. If you made the asm something like
asm("stb%X0 %1,0(%0)" : : "r" (foo), "r" (val) : "memory");
it would work fine. It would also work fine if you wrote 13 in the
template directly. These things follow the rules, so are guaranteed.
The most important pieces of doc here may be
* Accesses to the variable may be optimized as usual and the register
remains available for allocation and use in any computations,
provided that observable values of the variable are not affected.
* If the variable is referenced in inline assembly, the type of
access must be provided to the compiler via constraints (*note
Constraints::). Accesses from basic asms are not supported.
but read the whole "Global Register Variables" chapter?
> If it is true for some particular subset of cases that we can guarantee,
> e.g., using -O2 and irq_soft_mask offset within range of stb offset and
> we can point to specification such that both GCC and Clang will follow
> it, then okay. Otherwise I still think it's more trouble than it is
> worth.
-O2 makes it much more likely some inline assembler things work as
intended, but it guarantees nothing. Sorry.
Segher
Re: [PATCH] powerpc/pasemi: Use of_root in pas_pci_init()
On Tue, 6 Sep 2022 11:03:13 +1000, Michael Ellerman wrote: > Currently in pas_pci_init() a reference to the root node is leaked due > to a missing of_node_put(). Instead just use of_root directly. > > Note that converting to of_find_compatible_node(NULL, ...) would > not be entirely equivalent, because that would check the compatible > property of the root node, whereas using of_root skips checking the root > node and start the search at the first child of the root. > > [...] Applied to powerpc/next. [1/1] powerpc/pasemi: Use of_root in pas_pci_init() https://git.kernel.org/powerpc/c/c28c2d4abdf95655001992c4f52dc243ba00cac3 cheers
Re: [PATCH -next] powerpc/powernv: add missing of_node_put() in opal_export_attrs()
On Tue, 6 Sep 2022 14:17:03 +, Zheng Yongjun wrote: > After using 'np' returned by of_find_node_by_path(), of_node_put() > need be called to decrease the refcount. > > Applied to powerpc/next. [1/1] powerpc/powernv: add missing of_node_put() in opal_export_attrs() https://git.kernel.org/powerpc/c/71a92e99c47900cc164620948b3863382cec4f1a cheers
Re: [PATCH v2] powerpc/64s: add pte_needs_flush and huge_pmd_needs_flush
On Thu, 1 Sep 2022 21:03:34 +1000, Nicholas Piggin wrote: > Allow PTE changes to avoid flushing the TLB when access permissions are > being relaxed, the dirty bit is being set, and the accessed bit is being > changed. > > Relaxing access permissions and setting dirty and accessed bits do not > require a flush because the MMU will re-load the PTE and notice the > updates (it may also cause a spurious fault). > > [...] Applied to powerpc/next. [1/1] powerpc/64s: add pte_needs_flush and huge_pmd_needs_flush https://git.kernel.org/powerpc/c/b11931e9adc1a439eab75c97ca4b9f15754cdcb1 cheers
Re: [PATCH][next] powerpc: Fix fall-through warning for Clang
On Tue, 6 Sep 2022 22:32:13 +0100, Gustavo A. R. Silva wrote: > Fix the following fallthrough warning: > > arch/powerpc/platforms/85xx/mpc85xx_cds.c:161:3: warning: unannotated > fall-through between switch labels [-Wimplicit-fallthrough] > > Applied to powerpc/next. [1/1] powerpc: Fix fall-through warning for Clang https://git.kernel.org/powerpc/c/d4d944ff68cb1f896d3f3b1af0bc656949dc626a cheers
Re: [PATCH] Revert "powerpc/rtas: Implement reentrant rtas call"
On Wed, 7 Sep 2022 17:01:11 -0500, Nathan Lynch wrote: > At the time this was submitted by Leonardo, I confirmed -- or thought > I had confirmed -- with PowerVM partition firmware development that > the following RTAS functions: > > - ibm,get-xive > - ibm,int-off > - ibm,int-on > - ibm,set-xive > > [...] Applied to powerpc/next. [1/1] Revert "powerpc/rtas: Implement reentrant rtas call" https://git.kernel.org/powerpc/c/f88aabad33ea22be2ce1c60d8901942e4e2a9edb cheers
Re: [PATCH 0/9] Remove unused declarations for powerpc
On Tue, 13 Sep 2022 15:50:20 +0800, Gaosheng Cui wrote: > This series contains a few cleanup patches, to remove unused > declarations which have been removed. Thanks! > > Gaosheng Cui (9): > powerpc/xmon: remove unused ppc_parse_cpu() declaration > powerpc/spufs: remove orphan declarations from spufs.h > powerpc: remove unused chrp_event_scan() declaration > powerpc: remove unused udbg_init_debug_beat() declaration > powerpc/mm: remove orphan declarations from mmu_context.h > powerpc/powernv: remove orphan declarations from opal.h > powerpc/sysdev: remove unused xics_ipi_dispatch() declaration > powerpc/ps3: remove orphan declarations from ps3av.h > KVM: PPC: remove orphan declarations from kvm_ppc.h > > [...] Applied to powerpc/next. [1/9] powerpc/xmon: remove unused ppc_parse_cpu() declaration https://git.kernel.org/powerpc/c/edd100634a5eb99cf97868c419bdf44d44355c4f [2/9] powerpc/spufs: remove orphan declarations from spufs.h https://git.kernel.org/powerpc/c/cf78ddd3a1040a84bf882eecea44626dbad450c4 [3/9] powerpc: remove unused chrp_event_scan() declaration https://git.kernel.org/powerpc/c/29e1eb9169a9c73985ed15361520900ce1cef1d4 [4/9] powerpc: remove unused udbg_init_debug_beat() declaration https://git.kernel.org/powerpc/c/b5a472ad81ba23327ef11ca5b4ba9fd8ed38e45e [5/9] powerpc/mm: remove orphan declarations from mmu_context.h https://git.kernel.org/powerpc/c/d24b8f01fe7b848f88ff0a3204a674a092f365d0 [6/9] powerpc/powernv: remove orphan declarations from opal.h https://git.kernel.org/powerpc/c/77d30535816e90ff6a4466210c403a6b8b42a0a5 [7/9] powerpc/sysdev: remove unused xics_ipi_dispatch() declaration https://git.kernel.org/powerpc/c/3abed8acfe95046b27117f48d52dccd2ea82a322 [8/9] powerpc/ps3: remove orphan declarations from ps3av.h https://git.kernel.org/powerpc/c/b47f0024f990803f36e6a06f82e9d0dbe8424c26 [9/9] KVM: PPC: remove orphan declarations from kvm_ppc.h https://git.kernel.org/powerpc/c/3d7a198cfdb47405cfb4a3ea523876569fe341e6 cheers
Re: [PATCH v1 3/3] checkpatch: warn on usage of VM_BUG_ON() and other BUG variants
And I think that "Do not crash" is a stronger statement than "Avoid crashing" so I prefer the original suggestion but it's not a big deal either way. Yes, stronger wording is better. So how about this: "Do not crash the kernel unless it is absolutely unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants\n" . $herecurr); Okay, let's use that. Thanks! -- Thanks, David / dhildenb
Re: [PATCH v1 1/3] coding-style.rst: document BUG() and WARN() rules ("do not crash the kernel")
On 23.09.22 04:26, John Hubbard wrote: On 9/20/22 05:23, David Hildenbrand wrote: [1] https://lkml.kernel.org/r/CAHk-=wiEAH+ojSpAgx_Ep=nkpwhu8ado3v56bxccsu97oyj...@mail.gmail.com [2] https://lore.kernel.org/r/CAHk-=wg40eazofo16eviaj7mfqdhz2gvebvfsmf6gyzsprj...@mail.gmail.com [2] https://lkml.kernel.org/r/CAHk-=wit-dmhmfqery29jspjfgebx_ld+pnerc4j2ag990w...@mail.gmail.com s/2/3/ Thanks! ... diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst index 03eb53fd029a..e05899cbfd49 100644 --- a/Documentation/process/coding-style.rst +++ b/Documentation/process/coding-style.rst @@ -1186,6 +1186,67 @@ expression used. For instance: #endif /* CONFIG_SOMETHING */ +22) Do not crash the kernel +--- + +In general, it is not the kernel developer's decision to crash the kernel. What do you think of this alternate wording: In general, the decision to crash the kernel belongs to the user, rather than to the kernel developer. Ack [...] I like the wording, it feels familiar somehow! :) :) Reviewed-by: John Hubbard Thanks! -- Thanks, David / dhildenb
[RESEND PATCH] Revert "ASoC: fsl_audmix: make clock and output src write only"
This reverts commit 944c517b8c838832a166f1c89afbf8724f4a6b49.
There is error after making clock and output src write only
$amixer -c imxaudmix cset numid=1 1
amixer: Cannot read the given element from control sysdefault:3
Which is worse than before, so let's revert the change.
Signed-off-by: Shengjiu Wang
---
sound/soc/fsl/fsl_audmix.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index 43857b7a81c9..672148dd4b23 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -199,18 +199,10 @@ static int fsl_audmix_put_out_src(struct snd_kcontrol
*kcontrol,
static const struct snd_kcontrol_new fsl_audmix_snd_controls[] = {
/* FSL_AUDMIX_CTR controls */
- { .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
- .name = "Mixing Clock Source",
- .info = snd_soc_info_enum_double,
- .access = SNDRV_CTL_ELEM_ACCESS_WRITE,
- .put = fsl_audmix_put_mix_clk_src,
- .private_value = (unsigned long)_audmix_enum[0] },
- { .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
- .name = "Output Source",
- .info = snd_soc_info_enum_double,
- .access = SNDRV_CTL_ELEM_ACCESS_WRITE,
- .put = fsl_audmix_put_out_src,
- .private_value = (unsigned long)_audmix_enum[1] },
+ SOC_ENUM_EXT("Mixing Clock Source", fsl_audmix_enum[0],
+snd_soc_get_enum_double, fsl_audmix_put_mix_clk_src),
+ SOC_ENUM_EXT("Output Source", fsl_audmix_enum[1],
+snd_soc_get_enum_double, fsl_audmix_put_out_src),
SOC_ENUM("Output Width", fsl_audmix_enum[2]),
SOC_ENUM("Frame Rate Diff Error", fsl_audmix_enum[3]),
SOC_ENUM("Clock Freq Diff Error", fsl_audmix_enum[4]),
--
2.34.1
[RESEND PATCH] ASoC: fsl_asrc_dma: fully initialize structs
From: Sascha Hauer
The driver uses two statically ininitialized struct dma_slave_config,
but only one of them is initialized to zero. Initialize config_be to
zero as well to make sure that no fields are filled with random values.
Let the compiler do this instead of explicitly calling memset() which
makes it easier to read.
Signed-off-by: Sascha Hauer
Signed-off-by: Shengjiu Wang
---
sound/soc/fsl/fsl_asrc_dma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
index 12ddf2320f2d..3b81a465814a 100644
--- a/sound/soc/fsl/fsl_asrc_dma.c
+++ b/sound/soc/fsl/fsl_asrc_dma.c
@@ -139,7 +139,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component
*component,
struct dma_chan *tmp_chan = NULL, *be_chan = NULL;
struct snd_soc_component *component_be = NULL;
struct fsl_asrc *asrc = pair->asrc;
- struct dma_slave_config config_fe, config_be;
+ struct dma_slave_config config_fe = {}, config_be = {};
struct sdma_peripheral_config audio_config;
enum asrc_pair_index index = pair->index;
struct device *dev = component->dev;
@@ -183,7 +183,6 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component
*component,
return -EINVAL;
}
- memset(_fe, 0, sizeof(config_fe));
ret = snd_dmaengine_pcm_prepare_slave_config(substream, params,
_fe);
if (ret) {
dev_err(dev, "failed to prepare DMA config for Front-End\n");
--
2.34.1
Re: [PATCH v2 1/7] ASoC: dt-bindings: fsl_rpmsg: Add a property to assign the rpmsg channel
On 23/09/2022 11:56, Chancel Liu wrote: >>> Add a string property to assign the rpmsg channel this sound card sits >>> on. It also represents the name of ASoC platform driver. This property >>> can be omitted if there is only one sound card and it sits on >>> "rpmsg-audio-channel". >>> >>> Signed-off-by: Chancel Liu >>> --- >>> .../devicetree/bindings/sound/fsl,rpmsg.yaml | 37 ++- >>> 1 file changed, 35 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml >> b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml >>> index d370c98a62c7..3744ae794c00 100644 >>> --- a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml >>> +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml >>> @@ -11,8 +11,11 @@ maintainers: >>> >>> description: | >>>fsl_rpmsg is a virtual audio device. Mapping to real hardware devices >>> - are SAI, DMA controlled by Cortex M core. What we see from Linux >>> - side is a device which provides audio service by rpmsg channel. >>> + are SAI, MICFIL, DMA controlled by Cortex M core. What we see from >>> + Linux side is a device which provides audio service by rpmsg channel. >>> + We can create different sound cards which access different hardwares >>> + such as SAI, MICFIL, .etc through building rpmsg channels between >>> + Cortex-A and Cortex-M. >>> >>> properties: >>>compatible: >>> @@ -85,6 +88,17 @@ properties: >>>This is a boolean property. If present, the receiving function >>>will be enabled. >>> >>> + fsl,rpmsg-channel-name: >>> +$ref: /schemas/types.yaml#/definitions/string >>> +description: | >>> + A string property to assign rpmsg channel this sound card sits on. >>> + It also represents the name of ASoC platform driver. This property >> >> That's a Linux detail which doesn't belong in DT. >> > > We pass hardware parameters in dts node to set up clocks or other > configurations. These configurations are finally sent to Cortex-M by > rpmsg channel because Cortex-M actually controls real hardware devices. > If there's only one sound card sits on one rpmsg channel we will not > need this property. But if there are several sound cards we need to > specify correct rpmsg channel. Thus hardware configurations can be > properly sent to Cortex-M. From this level to speak, this property is > hardware-related since rpmsg channel represents the real hardware audio > controller. > > Here I attach the discussion in version 1 patches for your information: > --- > This property aims to tell the ASoC driver which rpmsg channel the > sound card depends on. If there are several sound cards sit on rpmsg, > we should pass correct information in dts node to specify the name of > rpmsg channel. That is why I meant to add this property. > > Actually this property is hardware-related. As we discussed before, > this kind of sound card based on rpmsg works under this mechanism > Cortex-A core tells the Cortex-M core configuration of the PCM > parameters then Cortex-M controls real hardware devices. This property > specifying rpmsg channel represents the real hardware audio controller. > --- > > That's my idea adding this property. Do you have any suggstion? I think you misunderstood the comment. Rob's comment was directly below a line which he referred to. That line contained a statement referring to Linux drivers. Anything related to Linux drivers does not belong to DT. Best regards, Krzysztof
RE: Re: [PATCH v2 1/7] ASoC: dt-bindings: fsl_rpmsg: Add a property to assign the rpmsg channel
> > Add a string property to assign the rpmsg channel this sound card sits
> > on. It also represents the name of ASoC platform driver. This property
> > can be omitted if there is only one sound card and it sits on
> > "rpmsg-audio-channel".
> >
> > Signed-off-by: Chancel Liu
> > ---
> > .../devicetree/bindings/sound/fsl,rpmsg.yaml | 37 ++-
> > 1 file changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > index d370c98a62c7..3744ae794c00 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > @@ -11,8 +11,11 @@ maintainers:
> >
> > description: |
> >fsl_rpmsg is a virtual audio device. Mapping to real hardware devices
> > - are SAI, DMA controlled by Cortex M core. What we see from Linux
> > - side is a device which provides audio service by rpmsg channel.
> > + are SAI, MICFIL, DMA controlled by Cortex M core. What we see from
> > + Linux side is a device which provides audio service by rpmsg channel.
> > + We can create different sound cards which access different hardwares
> > + such as SAI, MICFIL, .etc through building rpmsg channels between
> > + Cortex-A and Cortex-M.
> >
> > properties:
> >compatible:
> > @@ -85,6 +88,17 @@ properties:
> >This is a boolean property. If present, the receiving function
> >will be enabled.
> >
> > + fsl,rpmsg-channel-name:
> > +$ref: /schemas/types.yaml#/definitions/string
> > +description: |
> > + A string property to assign rpmsg channel this sound card sits on.
> > + It also represents the name of ASoC platform driver. This property
>
> That's a Linux detail which doesn't belong in DT.
>
We pass hardware parameters in dts node to set up clocks or other
configurations. These configurations are finally sent to Cortex-M by
rpmsg channel because Cortex-M actually controls real hardware devices.
If there's only one sound card sits on one rpmsg channel we will not
need this property. But if there are several sound cards we need to
specify correct rpmsg channel. Thus hardware configurations can be
properly sent to Cortex-M. From this level to speak, this property is
hardware-related since rpmsg channel represents the real hardware audio
controller.
Here I attach the discussion in version 1 patches for your information:
---
This property aims to tell the ASoC driver which rpmsg channel the
sound card depends on. If there are several sound cards sit on rpmsg,
we should pass correct information in dts node to specify the name of
rpmsg channel. That is why I meant to add this property.
Actually this property is hardware-related. As we discussed before,
this kind of sound card based on rpmsg works under this mechanism
Cortex-A core tells the Cortex-M core configuration of the PCM
parameters then Cortex-M controls real hardware devices. This property
specifying rpmsg channel represents the real hardware audio controller.
---
That's my idea adding this property. Do you have any suggstion?
Regards,
Chancel Liu
> > + can be omitted if there is only one sound card and it sits on
> > + "rpmsg-audio-channel".
> > +enum:
> > + - rpmsg-audio-channel
> > + - rpmsg-micfil-channel
> > +
> > required:
> >- compatible
> >- model
> > @@ -107,3 +121,22 @@ examples:
> > < IMX8MN_AUDIO_PLL2_OUT>;
> > clock-names = "ipg", "mclk", "dma", "pll8k", "pll11k";
> > };
> > +
> > + - |
> > +#include
> > +
> > +rpmsg_micfil: audio-controller {
> > +compatible = "fsl,imx8mm-rpmsg-audio";
> > +model = "micfil-audio";
> > +fsl,rpmsg-channel-name = "rpmsg-micfil-channel";
> > +fsl,enable-lpa;
> > +fsl,rpmsg-in;
> > +clocks = < IMX8MM_CLK_PDM_IPG>,
> > + < IMX8MM_CLK_PDM_ROOT>,
> > + < IMX8MM_CLK_SDMA3_ROOT>,
> > + < IMX8MM_AUDIO_PLL1_OUT>,
> > + < IMX8MM_AUDIO_PLL2_OUT>;
> > +clock-names = "ipg", "mclk", "dma", "pll8k", "pll11k";
> > +};
> > +
> > +...
> > --
> > 2.25.1
> >
> >
[PATCH] ASoC: fsl_micfil: Add Hardware Voice Activity Detector support
The Hardware Voice Activity Detector (HWVAD) is a block
responsible for detect voice activity in a channel selected
by the user. It can be configured in Envelope-based or
Energy-based mode.
There are additional two interrupts for HWVAD, one is event
interrupt, another is error interrupt.
Enable hwvad in parallel with recording.
when voice activity detected, HWVAD will be disabled and
alsa control notification is triggerred.
Signed-off-by: Shengjiu Wang
---
sound/soc/fsl/fsl_micfil.c | 434 +
sound/soc/fsl/fsl_micfil.h | 6 +-
2 files changed, 439 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 79ef4e269bc9..eeaa75fb9196 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -47,11 +47,15 @@ struct fsl_micfil {
struct clk *pll11k_clk;
struct snd_dmaengine_dai_dma_data dma_params_rx;
struct sdma_peripheral_config sdmacfg;
+ struct snd_soc_card *card;
unsigned int dataline;
char name[32];
int irq[MICFIL_IRQ_LINES];
enum quality quality;
int dc_remover;
+ int vad_init_mode;
+ int vad_enabled;
+ int vad_detected;
};
struct fsl_micfil_soc_data {
@@ -152,6 +156,152 @@ static int micfil_quality_set(struct snd_kcontrol
*kcontrol,
return micfil_set_quality(micfil);
}
+static const char * const micfil_hwvad_enable[] = {
+ "Disable (Record only)",
+ "Enable (Record with Vad)",
+};
+
+static const char * const micfil_hwvad_init_mode[] = {
+ "Envelope mode", "Energy mode",
+};
+
+static const char * const micfil_hwvad_hpf_texts[] = {
+ "Filter bypass",
+ "Cut-off @1750Hz",
+ "Cut-off @215Hz",
+ "Cut-off @102Hz",
+};
+
+/*
+ * DC Remover Control
+ * Filter Bypassed 1 1
+ * Cut-off @21Hz 0 0
+ * Cut-off @83Hz 0 1
+ * Cut-off @152HZ 1 0
+ */
+static const char * const micfil_dc_remover_texts[] = {
+ "Cut-off @21Hz", "Cut-off @83Hz",
+ "Cut-off @152Hz", "Bypass",
+};
+
+static const struct soc_enum hwvad_enable_enum =
+ SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_hwvad_enable),
+ micfil_hwvad_enable);
+static const struct soc_enum hwvad_init_mode_enum =
+ SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_hwvad_init_mode),
+ micfil_hwvad_init_mode);
+static const struct soc_enum hwvad_hpf_enum =
+ SOC_ENUM_SINGLE(REG_MICFIL_VAD0_CTRL2, 0,
+ ARRAY_SIZE(micfil_hwvad_hpf_texts),
+ micfil_hwvad_hpf_texts);
+static const struct soc_enum fsl_micfil_dc_remover_enum =
+ SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(micfil_dc_remover_texts),
+ micfil_dc_remover_texts);
+
+static int micfil_put_dc_remover_state(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+ struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+ struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+ unsigned int *item = ucontrol->value.enumerated.item;
+ int val = snd_soc_enum_item_to_val(e, item[0]);
+ int i = 0, ret = 0;
+ u32 reg_val = 0;
+
+ if (val < 0 || val > 3)
+ return -EINVAL;
+
+ micfil->dc_remover = val;
+
+ /* Calculate total value for all channels */
+ for (i = 0; i < MICFIL_OUTPUT_CHANNELS; i++)
+ reg_val |= val << MICFIL_DC_CHX_SHIFT(i);
+
+ /* Update DC Remover mode for all channels */
+ ret = snd_soc_component_update_bits(comp, REG_MICFIL_DC_CTRL,
+ MICFIL_DC_CTRL_CONFIG, reg_val);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int micfil_get_dc_remover_state(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+ struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+
+ ucontrol->value.enumerated.item[0] = micfil->dc_remover;
+
+ return 0;
+}
+
+static int hwvad_put_enable(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+ struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+ unsigned int *item = ucontrol->value.enumerated.item;
+ struct fsl_micfil *micfil = snd_soc_component_get_drvdata(comp);
+ int val = snd_soc_enum_item_to_val(e, item[0]);
+
+ micfil->vad_enabled = val;
+
+ return 0;
+}
+
+static int hwvad_get_enable(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_component *comp = snd_kcontrol_chip(kcontrol);
+
[PATCH] powerpc/kprobes: Fix null pointer reference in arch_prepare_kprobe()
I found a null pointer reference in arch_prepare_kprobe(): # echo 'p cmdline_proc_show' > kprobe_events # echo 'p cmdline_proc_show+16' >> kprobe_events [ 67.278533][ T122] Kernel attempted to read user page (0) - exploit attempt? (uid: 0) [ 67.279326][ T122] BUG: Kernel NULL pointer dereference on read at 0x [ 67.279738][ T122] Faulting instruction address: 0xc0050bfc [ 67.280486][ T122] Oops: Kernel access of bad area, sig: 11 [#1] [ 67.280846][ T122] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV [ 67.281435][ T122] Modules linked in: [ 67.281903][ T122] CPU: 0 PID: 122 Comm: sh Not tainted 6.0.0-rc3-7-gdcf8e5633e2e #10 [ 67.282547][ T122] NIP: c0050bfc LR: c0050bec CTR: 5bdc [ 67.282920][ T122] REGS: c000348475b0 TRAP: 0300 Not tainted (6.0.0-rc3-7-gdcf8e5633e2e) [ 67.283424][ T122] MSR: 90009033 CR: 88002444 XER: 20040006 [ 67.284023][ T122] CFAR: c022d100 DAR: DSISR: 4000 IRQMASK: 0 [ 67.284023][ T122] GPR00: c0050bec c00034847850 c13f6100 c1fb7718 [ 67.284023][ T122] GPR04: c0515c10 c0e5fe08 c133da60 c4839300 [ 67.284023][ T122] GPR08: c14ffb98 c0515c0c c0e18576 [ 67.284023][ T122] GPR12: c0e60170 c15a 0001155e0460 [ 67.284023][ T122] GPR16: 7fffe8eeb3c8 000116320728 [ 67.284023][ T122] GPR20: 000116320720 c12fa918 0006 [ 67.284023][ T122] GPR24: c14ffb98 c11ed360 c1fb7928 [ 67.284023][ T122] GPR28: 7c0802a6 c1fb7918 [ 67.287799][ T122] NIP [c0050bfc] arch_prepare_kprobe+0x10c/0x2d0 [ 67.288490][ T122] LR [c0050bec] arch_prepare_kprobe+0xfc/0x2d0 [ 67.289025][ T122] Call Trace: [ 67.289268][ T122] [c00034847850] [c12f77a0] 0xc12f77a0 (unreliable) [ 67.28][ T122] [c000348478d0] [c0231320] register_kprobe+0x3c0/0x7a0 [ 67.290439][ T122] [c00034847940] [c02938c0] __register_trace_kprobe+0x140/0x1a0 [ 67.290898][ T122] [c000348479b0] [c02944c4] __trace_kprobe_create+0x794/0x1040 [ 67.291330][ T122] [c00034847b60] [c02a1614] trace_probe_create+0xc4/0xe0 [ 67.291717][ T122] [c00034847bb0] [c029363c] create_or_delete_trace_kprobe+0x2c/0x80 [ 67.292158][ T122] [c00034847bd0] [c0264420] trace_parse_run_command+0xf0/0x210 [ 67.292611][ T122] [c00034847c70] [c02934a0] probes_write+0x20/0x40 [ 67.292996][ T122] [c00034847c90] [c045e98c] vfs_write+0xfc/0x450 [ 67.293356][ T122] [c00034847d50] [c045eec4] ksys_write+0x84/0x140 [ 67.293716][ T122] [c00034847da0] [c002e4fc] system_call_exception+0x17c/0x3a0 [ 67.294186][ T122] [c00034847e10] [c000c0e8] system_call_vectored_common+0xe8/0x278 [ 67.294680][ T122] --- interrupt: 3000 at 0x7fffa5682de0 [ 67.294937][ T122] NIP: 7fffa5682de0 LR: CTR: [ 67.295313][ T122] REGS: c00034847e80 TRAP: 3000 Not tainted (6.0.0-rc3-7-gdcf8e5633e2e) [ 67.295725][ T122] MSR: 9280f033 CR: 44002408 XER: [ 67.296291][ T122] IRQMASK: 0 [ 67.296291][ T122] GPR00: 0004 7fffe8eeaec0 7fffa5757300 0001 [ 67.296291][ T122] GPR04: 000116329c60 0017 00116329 [ 67.296291][ T122] GPR08: 0006 [ 67.296291][ T122] GPR12: 7fffa580ac60 0001155e0460 [ 67.296291][ T122] GPR16: 7fffe8eeb3c8 000116320728 [ 67.296291][ T122] GPR20: 000116320720 0002 [ 67.296291][ T122] GPR24: 0001163206f0 0020 7fffe8eeafa0 0001 [ 67.296291][ T122] GPR28: 0017 000116329c60 0001 [ 67.299570][ T122] NIP [7fffa5682de0] 0x7fffa5682de0 [ 67.299837][ T122] LR [] 0x0 [ 67.300072][ T122] --- interrupt: 3000 [ 67.300447][ T122] Instruction dump: [ 67.300736][ T122] 386319d8 481342f5 6000 6000 6000 e87f0028 3863fffc 481dc4d1 [ 67.301230][ T122] 6000 2c23 41820018 e9230058 <8129> 552936be 2c090001 4182018c [ 67.302102][ T122] ---[ end trace ]--- [ 67.302496][ T122] The address being probed has some special: cmdline_proc_show: Probe based on ftrace
[PATCH] ASoC: fsl: Remove unused inline function imx_pcm_dma_params_init_data()
The imx_pcm_dma_params_init_data() are no longer used since
commit c31da0b196f9 ("ASoC: imx-ssi: Remove unused driver"),
and the function is used to initialize some members of
"struct imx_dma_data", it's more readable to assign the value
directly, imx_pcm_dma_params_init_data is useless, so remove it.
Signed-off-by: Gaosheng Cui
---
sound/soc/fsl/imx-pcm.h | 9 -
1 file changed, 9 deletions(-)
diff --git a/sound/soc/fsl/imx-pcm.h b/sound/soc/fsl/imx-pcm.h
index 06b25f4b26b6..ac5f57c3cc55 100644
--- a/sound/soc/fsl/imx-pcm.h
+++ b/sound/soc/fsl/imx-pcm.h
@@ -18,15 +18,6 @@
#define IMX_DEFAULT_DMABUF_SIZE(64 * 1024)
-static inline void
-imx_pcm_dma_params_init_data(struct imx_dma_data *dma_data,
- int dma, enum sdma_peripheral_type peripheral_type)
-{
- dma_data->dma_request = dma;
- dma_data->priority = DMA_PRIO_HIGH;
- dma_data->peripheral_type = peripheral_type;
-}
-
struct imx_pcm_fiq_params {
int irq;
void __iomem *base;
--
2.25.1
Re: [PATCH] powerpc/pseries: Move vas_migration_handler early during migration
On Thu, 2022-09-22 at 07:14 -0500, Nathan Lynch wrote: > Haren Myneni writes: > > When the migration is initiated, the hypervisor changes VAS > > mappings as part of pre-migration event. Then the OS gets the > > migration event which closes all VAS windows before the migration > > starts. NX generates continuous faults until windows are closed > > and the user space can not differentiate these NX faults coming > > from the actual migration. So to reduce this time window, close > > VAS windows first in pseries_migrate_partition(). > > I'm concerned that this is only narrowing a window of time where > undesirable faults occur, and that it may not be sufficient for all > configurations. Migrations can be in progress for minutes or hours, > while the time that we wait for the VASI state transition is usually > seconds or minutes. So I worry that this works around a problem in > limited cases but doesn't cover them all. > > Maybe I don't understand the problem well enough. How does user space > respond to the NX faults? The user space resend the request to NX whenever the request is returned with NX fault. So the process should be same even for faults caused by the pre-migration. Whereas the paste will be returned with failure when the window is closed (unmap the paste address) and it can be considered as NX busy. Up to the user space whether to send the request again after some delay or fall back to SW compression and send the request again later. For the migration, pre-migration event is notified to the hypervisor and then OS will receive the migration event (SUSPEND) - So this patch close windows early before VASI so that removing NX fault handling during the time taken for VASI state transistion. Thanks Haren
Re: [PATCH 16/17] powerpc/qspinlock: allow indefinite spinning on a preempted owner
On Fri Sep 23, 2022 at 1:02 AM AEST, Laurent Dufour wrote: > On 28/07/2022 08:31:19, Nicholas Piggin wrote: > > Provide an option that holds off queueing indefinitely while the lock > > owner is preempted. This could reduce queueing latencies for very > > overcommitted vcpu situations. > > > > This is disabled by default. > > Hi Nick, > > I should have missed something here. > > If this option is turned on, CPU trying to lock when there is a preempted > owner will spin checking the lock->val and yielding the lock owner CPU. > Am I right? Yes. > If yes, why not being queued and spin checking its own value, yielding > against the lock owner CPU? I guess the idea is that when we start getting vCPU preemption, queueing behaviour causes this "train wreck" behaviour where lock waiters being preempted can halt lock transfers to other waiters (whereas with simple spinlocks only owner vCPU preemption matters). So the heuristics for paravirt qspinlock basically come down to avoiding queueing and making waiters behave more like a simple spinlock when it matters. That's the case for upstream and this rewrite. > This will generate less cache bouncing, which > is what the queued spinlock is trying to address, isn't it? It could. When the owner is preempted it's not going to be modifying the lock word and probably not surrounding data in the same cache line, and there won't be a lot of other try-lock operations come in (because they'll mostly queue up here as well). So cacheline bouncing shouldn't be the worst problem we face here. But it possibly is a concern. I didn't yet meausre any real improvement from this option, and it possibly has some starvation potential, so it's disabled by default for now. Thanks, Nick
Re: [PATCH v6 22/25] powerpc/64s: Clear user GPRs in syscall interrupt entry
On Wed Sep 21, 2022 at 4:56 PM AEST, Rohan McLure wrote: > Clear user state in gprs (assign to zero) to reduce the influence of user > registers on speculation within kernel syscall handlers. Clears occur > at the very beginning of the sc and scv 0 interrupt handlers, with > restores occurring following the execution of the syscall handler. > > Signed-off-by: Rohan McLure > --- > V2: Update summary > V3: Remove erroneous summary paragraph on syscall_exit_prepare > V4: Use ZEROIZE instead of NULLIFY. Clear r0 also. > V5: Move to end of patch series. > V6: Include clears which were previously in the syscall wrapper patch. > Move comment on r3-r8 register save to when we alter the calling > convention for system_call_exception. The series looks good to here, I just need to find a bit more time to look at the code and do some tests with the next few patches. I don't see much problem with them, looks a lot better now with fewer ifdefs so that's good. Possibly you could share some of those new sanitize macros in a header file but that's a minor nit. Coud we have this zeroize also under the same config option as the next? I figure if we care about speculative security we want both, and if we don't we need neither. Thanks, Nick > --- > arch/powerpc/kernel/interrupt_64.S | 17 - > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/interrupt_64.S > b/arch/powerpc/kernel/interrupt_64.S > index a5dd78bdbe6d..40147558e1a6 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -106,6 +106,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) >* but this is the best we can do. >*/ > > + /* > + * Zero user registers to prevent influencing speculative execution > + * state of kernel code. > + */ > + ZEROIZE_GPR(0) > + ZEROIZE_GPRS(5, 12) > + ZEROIZE_NVGPRS() > bl system_call_exception > > .Lsyscall_vectored_\name\()_exit: > @@ -134,6 +141,7 @@ BEGIN_FTR_SECTION > HMT_MEDIUM_LOW > END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > > + REST_NVGPRS(r1) > cmpdi r3,0 > bne .Lsyscall_vectored_\name\()_restore_regs > > @@ -285,6 +293,13 @@ END_BTB_FLUSH_SECTION > wrteei 1 > #endif > > + /* > + * Zero user registers to prevent influencing speculative execution > + * state of kernel code. > + */ > + ZEROIZE_GPR(0) > + ZEROIZE_GPRS(5, 12) > + ZEROIZE_NVGPRS() > bl system_call_exception > > .Lsyscall_exit: > @@ -325,6 +340,7 @@ BEGIN_FTR_SECTION > stdcx. r0,0,r1 /* to clear the reservation */ > END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS) > > + REST_NVGPRS(r1) > cmpdi r3,0 > bne .Lsyscall_restore_regs > /* Zero volatile regs that may contain sensitive kernel data */ > @@ -352,7 +368,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > .Lsyscall_restore_regs: > ld r3,_CTR(r1) > ld r4,_XER(r1) > - REST_NVGPRS(r1) > mtctr r3 > mtspr SPRN_XER,r4 > REST_GPR(0, r1) > -- > 2.34.1
Re: [PATCH v6 21/25] powerpc: Provide syscall wrapper
On Wed Sep 21, 2022 at 4:56 PM AEST, Rohan McLure wrote:
> Implement syscall wrapper as per s390, x86, arm64. When enabled
> cause handlers to accept parameters from a stack frame rather than
> from user scratch register state. This allows for user registers to be
> safely cleared in order to reduce caller influence on speculation
> within syscall routine. The wrapper is a macro that emits syscall
> handler symbols that call into the target handler, obtaining its
> parameters from a struct pt_regs on the stack.
>
> As registers are already saved to the stack prior to calling
> system_call_exception, it appears that this function is executed more
> efficiently with the new stack-pointer convention than with parameters
> passed by registers, avoiding the allocation of a stack frame for this
> method. On a 32-bit system, we see >20% performance increases on the
> null_syscall microbenchmark, and on a Power 8 the performance gains
> amortise the cost of clearing and restoring registers which is
> implemented at the end of this series, seeing final result of ~5.6%
> performance improvement on null_syscall.
>
> Syscalls are wrapped in this fashion on all platforms except for the
> Cell processor as this commit does not provide SPU support. This can be
> quickly fixed in a successive patch, but requires spu_sys_callback to
> allocate a pt_regs structure to satisfy the wrapped calling convention.
>
> Co-developed-by: Andrew Donnellan
> Signed-off-by: Andrew Donnellan
> Signed-off-by: Rohan McLure
> ---
> V2: Generate prototypes for symbols produced by the wrapper.
> V3: Rebased to remove conflict with 1547db7d1f44
> ("powerpc: Move system_call_exception() to syscall.c"). Also remove copy
> from gpr3 save slot on stackframe to orig_r3's slot. Fix whitespace with
> preprocessor defines in system_call_exception.
> V5: Move systbl.c syscall wrapper support to this patch. Swap
> calling convention for system_call_exception to be (, r0)
> V6: Change calling convention for system_call_exception in another
> patch.
> ---
Nice. Looks very clean.
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index fcca06d200d3..e1f36fd61db3 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -39,6 +39,8 @@
> extern char vdso32_start, vdso32_end;
> extern char vdso64_start, vdso64_end;
>
> +long sys_ni_syscall(void);
> +
> /*
> * The vdso data page (aka. systemcfg for old ppc64 fans) is here.
> * Once the early boot kernel code no longer needs to muck around
What's this doing? Why can't it continue using the declaration in
syscalls.h?
Aside from that and the stray hunk to go in the previous patch,
Reviewed-by: Nicholas Piggin
Thanks,
Nick
Re: [PATCH v6 20/25] powerpc: Change system_call_exception calling convention
On Wed Sep 21, 2022 at 4:56 PM AEST, Rohan McLure wrote:
> Change system_call_exception arguments to pass a pointer to a stack frame
> container caller state, as well as the original r0, which determines the
> number of the syscall. This has been observed to yield improved performance
> to passing them by registers, circumventing the need to allocate a stack
> frame.
>
> Signed-off-by: Rohan McLure
Thanks for splitting it out, I think it does make it nicer to review.
[...]
> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
> index 15af0ed019a7..0e9ba3efee94 100644
> --- a/arch/powerpc/kernel/syscall.c
> +++ b/arch/powerpc/kernel/syscall.c
> @@ -13,9 +13,7 @@
>
>
> /* Has to run notrace because it is entered not completely "reconciled" */
> -notrace long system_call_exception(long r3, long r4, long r5,
> -long r6, long r7, long r8,
> -unsigned long r0, struct pt_regs *regs)
> +notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
> {
> long ret;
> syscall_fn f;
> @@ -136,12 +134,6 @@ notrace long system_call_exception(long r3, long r4,
> long r5,
> r0 = do_syscall_trace_enter(regs);
> if (unlikely(r0 >= NR_syscalls))
> return regs->gpr[3];
> - r3 = regs->gpr[3];
> - r4 = regs->gpr[4];
> - r5 = regs->gpr[5];
> - r6 = regs->gpr[6];
> - r7 = regs->gpr[7];
> - r8 = regs->gpr[8];
>
> } else if (unlikely(r0 >= NR_syscalls)) {
> if (unlikely(trap_is_unsupported_scv(regs))) {
> --
> 2.34.1
This is probably just missing the hunk
+ ret = f(regs->gpr[3], regs->gpr[4], regs->gpr[5],
+ regs->gpr[6], regs->gpr[7], regs->gpr[8]);
which got into your next patch.
Otherwise I think it looks good.
Reviewed-by: Nicholas Piggin
Re: [PATCH v3 1/4] drm/ofdrm: Add ofdrm for Open Firmware framebuffers
Hi Geert
Am 23.09.22 um 09:14 schrieb Geert Uytterhoeven:
Hi Thomas,
On Thu, Sep 22, 2022 at 1:33 PM Thomas Zimmermann wrote:
Open Firmware provides basic display output via the 'display' node.
DT platform code already provides a device that represents the node's
framebuffer. Add a DRM driver for the device. The display mode and
color format is pre-initialized by the system's firmware. Runtime
modesetting via DRM is not possible. The display is useful during
early boot stages or as error fallback.
Similar functionality is already provided by fbdev's offb driver,
which is insufficient for modern userspace. The old driver includes
support for BootX device tree, which can be found on old 32-bit
PowerPC Macintosh systems. If these are still in use, the
functionality can be added to ofdrm or implemented in a new
driver. As with simpledrm, the fbdev driver cannot be selected if
ofdrm is already enabled.
Thanks for your patch!
The driver has been tested on qemu's ppc64le emulation. The device
hand-over has been tested with bochs.
Oh, tested on little-endian only ;-)
I wish it was easier to test. But it's hard to find hardware and a Linux
for PowerPC these days, so I have limited options for testing. It's just
qemu + a compatible distribution for me. My assumption has been that
people who what to use it on anything else would send me a patch.
--- /dev/null
+++ b/drivers/gpu/drm/tiny/ofdrm.c
+static const struct drm_format_info *display_get_validated_format(struct
drm_device *dev,
+ u32 depth)
+{
+ const struct drm_format_info *info;
+ u32 format;
+
+ switch (depth) {
+ case 8:
+ format = drm_mode_legacy_fb_format(8, 8);
+ break;
+ case 15:
+ case 16:
+ format = drm_mode_legacy_fb_format(16, depth);
+ break;
+ case 32:
+ format = drm_mode_legacy_fb_format(32, 24);
Shouldn't all of these use drm_driver_legacy_fb_format() (and the
driver set drm_mode_config.quirk_addfb_prefer_host_byte_order) to have
a chance of working on traditional big-endian PPC?
That's a good point. The offb driver reads the endianess property. Ofdrm
could do this and set the quirk bit accordingly. I won't have the option
to test it, but the code seems easy enough to add it. I'll make an extra
patch for this in the next iteration.
Best regards
Thomas
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v6 19/25] powerpc: Remove high-order word clearing on compat syscall entry
On Wed Sep 21, 2022 at 4:55 PM AEST, Rohan McLure wrote:
> Remove explicit clearing of the high order-word of user parameters when
> handling compatibility syscalls in system_call_exception. The
> COMPAT_SYSCALL_DEFINEx macros handle this clearing through an
> explicit cast to the signature type of the target handler.
>
> Signed-off-by: Rohan McLure
> Reported-by: Nicholas Piggin
Thanks for digging through it to make sure things will work right
without this. Some handlers look problematic without the rest of your
series, right? e.g., upstream compat_sys_mmap2 has long arguments.
Reviewed-by: Nicholas Piggin
> ---
> V6: New patch
> ---
> arch/powerpc/kernel/syscall.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
> index 9875486f6168..15af0ed019a7 100644
> --- a/arch/powerpc/kernel/syscall.c
> +++ b/arch/powerpc/kernel/syscall.c
> @@ -157,14 +157,6 @@ notrace long system_call_exception(long r3, long r4,
> long r5,
>
> if (unlikely(is_compat_task())) {
> f = (void *)compat_sys_call_table[r0];
> -
> - r3 &= 0xULL;
> - r4 &= 0xULL;
> - r5 &= 0xULL;
> - r6 &= 0xULL;
> - r7 &= 0xULL;
> - r8 &= 0xULL;
> -
> } else {
> f = (void *)sys_call_table[r0];
> }
> --
> 2.34.1
Re: [RFC PATCH 1/7] powerpc: use 16-bit immediate for STACK_FRAME_REGS_MARKER
Le 23/09/2022 à 09:32, Michael Ellerman a écrit : > Christophe Leroy writes: >> Le 19/09/2022 à 16:01, Nicholas Piggin a écrit : >>> Using a 16-bit constant for this marker allows it to be loaded with >>> a single 'li' instruction. On 64-bit this avoids a TOC entry and a >>> TOC load that depends on the r2 value that has just been loaded from >>> the PACA. >>> >>> XXX: this probably should be 64-bit change and use 2 instruction >>> sequence that 32-bit uses, to avoid false positives. >> >> Yes would probably be safer ? It is only one instruction more, would >> likely be unnoticeable. > > Yeah "regshere" has definitely saved me some time over the years > starting at memory dumps. > > I'd settle for 0x + "regs". That's not a sign-extended 32 bits value
Re: [RFC PATCH 1/7] powerpc: use 16-bit immediate for STACK_FRAME_REGS_MARKER
Christophe Leroy writes: > Le 19/09/2022 à 16:01, Nicholas Piggin a écrit : >> Using a 16-bit constant for this marker allows it to be loaded with >> a single 'li' instruction. On 64-bit this avoids a TOC entry and a >> TOC load that depends on the r2 value that has just been loaded from >> the PACA. >> >> XXX: this probably should be 64-bit change and use 2 instruction >> sequence that 32-bit uses, to avoid false positives. > > Yes would probably be safer ? It is only one instruction more, would > likely be unnoticeable. Yeah "regshere" has definitely saved me some time over the years starting at memory dumps. I'd settle for 0x + "regs". cheers
Re: [RFC PATCH 5/7] powerpc/64s: update generic cpu option name and compiler flags
On Thu Sep 22, 2022 at 1:22 AM AEST, Segher Boessenkool wrote:
> Hi!
>
> On Wed, Sep 21, 2022 at 11:01:18AM +1000, Nicholas Piggin wrote:
> > On Wed Sep 21, 2022 at 8:16 AM AEST, Segher Boessenkool wrote:
> > > On Tue, Sep 20, 2022 at 12:01:47AM +1000, Nicholas Piggin wrote:
> > > > Update the 64s GENERIC_CPU option. POWER4 support has been dropped, so
> > > > make that clear in the option name.
> > >
> > > AFAIR the minimum now is POWER4+ (ISA 2.01), not POWER5 (ISA 2.02).
> >
> > It's POWER5 now, because of commit 471d7ff8b5 ("powerpc/64s: Remove
> > POWER4 support"), which is misguided about POWER4+ and also introduced
> > the -mcpu=power5 bug on 970 builds :\
>
> ISA 2.01 added just a few things (LPES[0], HDEC, some PM things, but
> crucially also anything that sets MSR[PR] also sets MSR[EE] since then).
Ah, right. Some Book3 cleanups mainly.
> > Not sure it's worth adding POWER4+ support back but if someone has a
> > POWER4+ or adds it to QEMU TCG, I will do the patch.
>
> 970 is 2.01 -- pretending it is 2.02 is a ticking time bomb: the popcntb
> insn will be generated for popcount and parity intrinsics, which can be
> generated by generic code!
Yeah agreed, it was an error on my part with that original patch.
> > > > -mtune= before power8 is dropped because the minimum gcc version
> > > > supports power8, and tuning is made consistent between big and little
> > > > endian.
> > >
> > > Tuning for p8 on e.g. 970 gives quite bad results. No idea if anyone
> > > cares, but this is a serious regression if so.
> >
> > It's for "generic" kernel so we set low minimum but higher tune,
> > assuming that people would usually have newer, so it was already
> > doing -mtune=power7.
> >
> > We could make a specific 970/G5 entry though, since those still
> > have users.
>
> If that uses -mcpu=power4 (which means ISA 2.01 btw!) all is fine
> already? (Or -mcpu=970, same thing really, it just allows VMX as well).
Well it does -mcpu=power4 but the "generic" CPU option also does
the -mtune=power7 or 8. I added a -mcpu=970 version, even though
we don't let the compiler generate VMX in the kernel so I guess
it's not functionally different.
> Thanks for taking care of this Nick, much appreciated!
No problem, thanks for reviewing and finding my errors :P
Thanks,
Nick
Re: [PATCH v3 4/4] drm/ofdrm: Support color management
Hi Thomas,
On Thu, Sep 22, 2022 at 1:33 PM Thomas Zimmermann wrote:
> Support the CRTC's color-management property and implement each model's
> palette support.
>
> The OF hardware has different methods of setting the palette. The
> respective code has been taken from fbdev's offb and refactored into
> per-model device functions. The device functions integrate this
> functionality into the overall modesetting.
>
> As palette handling is a CRTC property that depends on the primary
> plane's color format, the plane's atomic_check helper now updates the
> format field in ofdrm's custom CRTC state. The CRTC's atomic_flush
> helper updates the palette for the format as needed.
>
> v3:
> * lookup CRTC state with drm_atomic_get_new_crtc_state()
> * access HW palette with writeb(), writel(), and readl() (Ben)
> * declare register values as u32
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Javier Martinez Canillas
Thanks for your patch!
> --- a/drivers/gpu/drm/tiny/ofdrm.c
> +++ b/drivers/gpu/drm/tiny/ofdrm.c
> +static void __iomem *ofdrm_qemu_cmap_ioremap(struct ofdrm_device *odev,
> +struct device_node *of_node,
> +u64 fb_base)
> +{
> +#ifdef __BIG_ENDIAN
> + static const __be32 io_of_addr[3] = { 0x0100, 0x0, 0x0 };
> +#else
> + static const __be32 io_of_addr[3] = { 0x0001, 0x0, 0x0 };
> +#endif
You can easily get rid of the #ifdef:
static const __be32 io_of_addr[3] = { cpu_to_be32(0x0100), 0x0, 0x0 };
And probably sparse ("make C=2") will complain about the plain zeros,
so "cpu_to_be32(0x0)" as well.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/2] powerpc/64s: Fix GENERIC_CPU build flags for PPC970 / G5
On Thu Sep 22, 2022 at 4:50 AM AEST, Segher Boessenkool wrote:
> On Wed, Sep 21, 2022 at 11:41:02AM +1000, Nicholas Piggin wrote:
> > Big-endian GENERIC_CPU supports 970, but builds with -mcpu=power5.
> > POWER5 is ISA v2.02 whereas 970 is v2.01 plus Altivec. 2.02 added
> > the popcntb instruction which a compiler might use.
> >
> > Use -mcpu=power4.
> >
> > Fixes: 471d7ff8b51b ("powerpc/64s: Remove POWER4 support")
> > Signed-off-by: Nicholas Piggin
>
> Reviewed-by: Segher Boessenkool
>
> Thank you!
>
> Maybe superfluous, but some more context: GCC's -mcpu=power4 means
> POWER4+, ISA 2.01, according to our documentation. There is no
> difference with ISA 2.00 (what plain POWER4 implements) for anything
> GCC does.
Huh, okay. Well I guess we are past that point now, interesting that
another ISA version was done for 4+ though, and then another for 5.
I don't see a list of changes from 2.00 in the public version, I
wonder what else changed other than mtmsrd.
Thanks,
Nick
Re: [PATCH v3 1/4] drm/ofdrm: Add ofdrm for Open Firmware framebuffers
Hi Thomas,
On Thu, Sep 22, 2022 at 1:33 PM Thomas Zimmermann wrote:
> Open Firmware provides basic display output via the 'display' node.
> DT platform code already provides a device that represents the node's
> framebuffer. Add a DRM driver for the device. The display mode and
> color format is pre-initialized by the system's firmware. Runtime
> modesetting via DRM is not possible. The display is useful during
> early boot stages or as error fallback.
>
> Similar functionality is already provided by fbdev's offb driver,
> which is insufficient for modern userspace. The old driver includes
> support for BootX device tree, which can be found on old 32-bit
> PowerPC Macintosh systems. If these are still in use, the
> functionality can be added to ofdrm or implemented in a new
> driver. As with simpledrm, the fbdev driver cannot be selected if
> ofdrm is already enabled.
Thanks for your patch!
> The driver has been tested on qemu's ppc64le emulation. The device
> hand-over has been tested with bochs.
Oh, tested on little-endian only ;-)
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/ofdrm.c
> +static const struct drm_format_info *display_get_validated_format(struct
> drm_device *dev,
> + u32 depth)
> +{
> + const struct drm_format_info *info;
> + u32 format;
> +
> + switch (depth) {
> + case 8:
> + format = drm_mode_legacy_fb_format(8, 8);
> + break;
> + case 15:
> + case 16:
> + format = drm_mode_legacy_fb_format(16, depth);
> + break;
> + case 32:
> + format = drm_mode_legacy_fb_format(32, 24);
Shouldn't all of these use drm_driver_legacy_fb_format() (and the
driver set drm_mode_config.quirk_addfb_prefer_host_byte_order) to have
a chance of working on traditional big-endian PPC?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 2/2] powerpc/rtas: block error injection when locked down
Paul Moore writes:
> On Thu, Sep 22, 2022 at 3:38 PM Nathan Lynch wrote:
>>
>> The error injection facility on pseries VMs allows corruption of
>> arbitrary guest memory, potentially enabling a sufficiently privileged
>> user to disable lockdown or perform other modifications of the running
>> kernel via the rtas syscall.
>>
>> Block the PAPR error injection facility from being opened or called
>> when locked down.
>>
>> Signed-off-by: Nathan Lynch
>> ---
>> arch/powerpc/kernel/rtas.c | 25 -
>> include/linux/security.h | 1 +
>> security/security.c| 1 +
>> 3 files changed, 26 insertions(+), 1 deletion(-)
>
> ...
>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 1ca8dbacd3cc..b5d5138ae66a 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -123,6 +123,7 @@ enum lockdown_reason {
>> LOCKDOWN_BPF_WRITE_USER,
>> LOCKDOWN_DBG_WRITE_KERNEL,
>> LOCKDOWN_DEVICE_TREE,
>> + LOCKDOWN_RTAS_ERROR_INJECTION,
>
> With the understanding that I've never heard of RTAS until now, are
> there any other RTAS events that would require a lockdown reason? As
> a follow up, is it important to distinguish between different RTAS
> lockdown reasons?
>
> I'm trying to determine if we can just call it LOCKDOWN_RTAS.
Yes I think we should.
Currently it only locks down the error injection calls, not all of RTAS.
But firmware can/will add new RTAS calls in future, so it's always
possible something will need to be added to the list of things that need
to be blocked during lockdown.
So I think calling it LOCKDOWN_RTAS would be more general and future
proof, and can be read to mean "lockdown the parts of RTAS that need
to be locked down".
Similarly we have LOCKDOWN_ACPI_TABLES which locks down modification to
ACPI data, but doesn't disable ACPI use entirely AIUI.
cheers
Re: [PATCH] powerpc/irq: Modernise inline assembly in irq_soft_mask_{set,return}
On Tue Sep 20, 2022 at 4:41 PM AEST, Christophe Leroy wrote:
> local_paca is declared as global register asm("r13"), it is therefore
> garantied to always ever be r13.
>
> It is therefore not required to opencode r13 in the assembly, use
> a reference to local_paca->irq_soft_mask instead.
>
> This also allows removing the 'memory' clobber in irq_soft_mask_set()
> as GCC now knows what is being modified in memory.
The code matches the changelog AFAIKS. But I don't know where it is
guaranteed it will always be r13 in GCC and Clang. I still don't know
where in the specification or documentation suggests this.
There was some assertion it would always be r13, but that can't be a
*general* rule. e.g., the following code:
struct foo {
#ifdef BIGDISP
int array[1024*1024];
#endif
char bar;
};
register struct foo *foo asm("r13");
static void setval(char val)
{
asm("stb%X0 %1,%0" : "=m" (foo->bar) : "r" (val));
}
int main(void)
{
setval(10);
}
With -O0 this generates stb 9,0(10) for me for GCC 12, and with -O2
-DBIGDISP it generates stb 10,0(9). So that makes me nervious. GCC
does not have some kind of correctness guarantee here, so it must not
have this in its regression tests etc., and who knows about clang.
If it is true for some particular subset of cases that we can guarantee,
e.g., using -O2 and irq_soft_mask offset within range of stb offset and
we can point to specification such that both GCC and Clang will follow
it, then okay. Otherwise I still think it's more trouble than it is
worth.
Thanks,
Nick
>
> Using %X modifier will give GCC a bit more flexibility on the code
> generation.
>
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/include/asm/hw_irq.h | 12 ++--
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h
> b/arch/powerpc/include/asm/hw_irq.h
> index e8de249339d8..dbe037ff4474 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -115,10 +115,7 @@ static inline notrace unsigned long
> irq_soft_mask_return(void)
> {
> unsigned long flags;
>
> - asm volatile(
> - "lbz %0,%1(13)"
> - : "=r" (flags)
> - : "i" (offsetof(struct paca_struct, irq_soft_mask)));
> + asm volatile("lbz%X1 %0,%1" : "=r" (flags) : "m"
> (local_paca->irq_soft_mask));
>
> return flags;
> }
> @@ -147,12 +144,7 @@ static inline notrace void irq_soft_mask_set(unsigned
> long mask)
> if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> WARN_ON(mask && !(mask & IRQS_DISABLED));
>
> - asm volatile(
> - "stb %0,%1(13)"
> - :
> - : "r" (mask),
> - "i" (offsetof(struct paca_struct, irq_soft_mask))
> - : "memory");
> + asm volatile("stb%X0 %1,%0" : "=m" (local_paca->irq_soft_mask) : "r"
> (mask));
> }
>
> static inline notrace unsigned long irq_soft_mask_set_return(unsigned long
> mask)
> --
> 2.37.1
Re: [PATCH 1/2] powerpc/pseries: block untrusted device tree changes when locked down
Paul Moore writes: > On Thu, Sep 22, 2022 at 3:38 PM Nathan Lynch wrote: >> >> The /proc/powerpc/ofdt interface allows the root user to freely alter >> the in-kernel device tree, enabling arbitrary physical address writes >> via drivers that could bind to malicious device nodes, thus making it >> possible to disable lockdown. >> >> Historically this interface has been used on the pseries platform to >> facilitate the runtime addition and removal of processor, memory, and >> device resources (aka Dynamic Logical Partitioning or DLPAR). Years >> ago, the processor and memory use cases were migrated to designs that >> happen to be lockdown-friendly: device tree updates are communicated >> directly to the kernel from firmware without passing through untrusted >> user space. I/O device DLPAR via the "drmgr" command in powerpc-utils >> remains the sole legitimate user of /proc/powerpc/ofdt, but it is >> already broken in lockdown since it uses /dev/mem to allocate argument >> buffers for the rtas syscall. So only illegitimate uses of the >> interface should see a behavior change when running on a locked down >> kernel. >> >> Signed-off-by: Nathan Lynch >> --- >> arch/powerpc/platforms/pseries/reconfig.c | 5 + >> include/linux/security.h | 1 + >> security/security.c | 1 + >> 3 files changed, 7 insertions(+) > > A couple of small nits below, but in general this seems reasonable. > However, as we are currently at -rc6 I would like us to wait to merge > this until after the upcoming merge window closes (I don't like > merging new functionality into -next at -rc6). It's a bug fix, not a new feature IMHO. I'd like to take it via the powerpc tree. cheers
Re: [PATCH v5 27/30] RFC: KVM: powerpc: Move processor compatibility check to hardware setup
isaku.yamah...@intel.com writes:
> From: Isaku Yamahata
>
> Move processor compatibility check from kvm_arch_processor_compat() into
^
kvm_arch_check_processor_compat()
> kvm_arch_hardware_setup(). The check does model name comparison with a
> global variable, cur_cpu_spec. There is no point to check it at run time
> on all processors.
A key detail I had to look up is that both kvm_arch_hardware_setup() and
kvm_arch_check_processor_compat() are called from kvm_init(), one after
the other. But the latter is called on each CPU.
And because the powerpc implementation of kvm_arch_check_processor_compat()
just checks a global, there's no need to call it on every CPU.
> kvmppc_core_check_processor_compat() checks the global variable. There are
> five implementation for it as follows.
There are three implementations not five.
> arch/powerpc/include/asm/cputable.h: extern struct cpu_spec *cur_cpu_spec;
> arch/powerpc/kvm/book3s.c: return 0
> arch/powerpc/kvm/e500.c: strcmp(cur_cpu_spec->cpu_name, "e500v2")
> arch/powerpc/kvm/e500mc.c: strcmp(cur_cpu_spec->cpu_name, "e500mc")
> strcmp(cur_cpu_spec->cpu_name, "e5500")
> strcmp(cur_cpu_spec->cpu_name, "e6500")
>
> Suggested-by: Sean Christopherson
> Signed-off-by: Isaku Yamahata
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Fabiano Rosas
> ---
> arch/powerpc/kvm/powerpc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 7b56d6ccfdfb..31dc4f231e9d 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -444,12 +444,12 @@ int kvm_arch_hardware_enable(void)
>
> int kvm_arch_hardware_setup(void *opaque)
> {
> - return 0;
> + return kvmppc_core_check_processor_compat();
> }
>
> int kvm_arch_check_processor_compat(void)
> {
> - return kvmppc_core_check_processor_compat();
> + return 0;
> }
The actual change seems OK. I gave it a quick test boot and ran some
VMs, everything seems to work as before.
Acked-by: Michael Ellerman (powerpc)
cheers
Re: [PATCH v2 3/3] powerpc: mm: support page table check
Le 23/09/2022 à 08:08, Rohan McLure a écrit :
> On creation and clearing of a page table mapping, instrument such calls
> by invoking page_table_check_pte_set and page_table_check_pte_clear
> respectively. These calls serve as a sanity check against illegal
> mappings.
>
> Enable ARCH_SUPPORTS_PAGE_TABLE_CHECK for all ppc64, and 32-bit
> platforms implementing Book3S.
>
> Change pud_pfn to be a runtime bug rather than a build bug as it is
> consumed by page_table_check_pud_{clear,set} which are not called.
>
> See also:
>
> riscv support in commit 3fee229a8eb9 ("riscv/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> arm64 in commit 42b2547137f5 ("arm64/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> x86_64 in commit d283d422c6c4 ("x86: mm: add x86_64 support for page table
> check")
>
> Signed-off-by: Rohan McLure
> ---
> V2: Update spacing and types assigned to pte_update calls > ---
> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h
> b/arch/powerpc/include/asm/nohash/32/pgtable.h
> index 9091e4904a6b..45ba36d968aa 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
> @@ -166,6 +166,7 @@ void unmap_kernel_page(unsigned long va);
> #define _PAGE_CHG_MASK (PTE_RPN_MASK | _PAGE_DIRTY | _PAGE_ACCESSED |
> _PAGE_SPECIAL)
>
> #ifndef __ASSEMBLY__
> +#include
>
> #define pte_clear(mm, addr, ptep) \
> do { pte_update(mm, addr, ptep, ~0, 0, 0); } while (0)
> @@ -305,7 +306,11 @@ static inline int __ptep_test_and_clear_young(struct
> mm_struct *mm,
> static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long
> addr,
> pte_t *ptep)
> {
> - return __pte(pte_update(mm, addr, ptep, ~0, 0, 0));
> + unsigned long old = pte_update(mm, addr, ptep, ~0, 0, 0);
You missed that one, should be pte_t to avoid twice __pte(old)
> +
> + page_table_check_pte_clear(mm, addr, __pte(old));
> +
> + return __pte(old);
> }
>
> #define __HAVE_ARCH_PTEP_SET_WRPROTECT
Re: [PATCH v2 2/3] powerpc: mm: add p{te,md,ud}_user_accessible_page helpers
Le 23/09/2022 à 08:08, Rohan McLure a écrit :
> Add the following helpers for detecting whether a page table entry
> is a leaf and is accessible to user space.
>
> * pte_user_accessible_page
> * pmd_user_accessible_page
> * pud_user_accessible_page
>
> Also implement missing pud_user definitions for both Book3S/E 64-bit
> systems.
>
> Signed-off-by: Rohan McLure
> ---
> V2: Provide missing pud_user implementations, use p{u,m}d_is_leaf.
> ---
> diff --git a/arch/powerpc/include/asm/pgtable.h
> b/arch/powerpc/include/asm/pgtable.h
> index 36956fb440e1..69eed4e03da0 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -172,6 +172,39 @@ static inline int pud_pfn(pud_t pud)
> }
> #endif
>
> +static inline bool pte_user_accessible_page(pte_t pte)
> +{
> + return pte_present(pte) && pte_user(pte);
> +}
> +
> +#ifdef CONFIG_PPC64
> +
> +static inline bool pmd_user_accessible_page(pmd_t pmd)
> +{
> + return pmd_is_leaf(pmd) && pmd_present(pmd) && pmd_user(pmd);
> +}
> +
> +static inline bool pud_user_accessible_page(pud_t pud)
> +{
> + return pud_is_leaf(pud) && pud_present(pud) && pud_user(pud);
> +}
> +
> +#else
> +
> +static inline bool pmd_user_accessible_page(pmd_t pmd)
> +{
> + WARN(1, "pmd: multi-level paging unsupported on ppc32");
> + return false;
> +}
> +
> +static inline bool pud_user_accessible_page(pud_t pud)
> +{
> + WARN(1, "pud: multi-level paging unsupported on ppc32");
> + return false;
> +}
> +
> +#endif /* CONFIG_PPC64 */
> +
I can't see the point in this splitted implementation PPC64/PPC32.
The warning has no added value from my point of view.
And multi-level paging IS supported on PPC32, that's 2 level paging, the
PMD is folder into the PGD.
pmd_is_leaf() and pud_is_leaf() are PPC64 specific.
The following could be common to PPC32 and PPC64:
+static inline bool pmd_user_accessible_page(pmd_t pmd)
+{
+ return pmd_leaf(pmd) && pmd_present(pmd) && pmd_user(pmd);
+}
+
+static inline bool pud_user_accessible_page(pud_t pud)
+{
+ return pud_leaf(pud) && pud_present(pud) && pud_user(pud);
+}
+
pud_user() is defined in include/asm-generic/pgtable-nopmd.h for PPC32.
All you have to do is to define a stub pmd_user() for book3s/32 and
nohash/32.
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_PGTABLE_H */
Re: [PATCH] powerpc/irq: Refactor irq_soft_mask_{set,or}_return()
On Tue Sep 20, 2022 at 4:41 PM AEST, Christophe Leroy wrote:
> This partialy reapply commit ef5b570d3700 ("powerpc/irq: Don't
> open code irq_soft_mask helpers") which was reverted by
> commit 684c68d92e2e ("Revert "powerpc/irq: Don't open code
> irq_soft_mask helpers"")
>
> irq_soft_mask_set_return() and irq_soft_mask_or_return()
> are overset of irq_soft_mask_set().
>
> Have them use irq_soft_mask_set() instead of duplicating it.
>
> Signed-off-by: Christophe Leroy
Reviewed-by: Nicholas Piggin
> ---
> arch/powerpc/include/asm/hw_irq.h | 26 --
> 1 file changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h
> b/arch/powerpc/include/asm/hw_irq.h
> index 983551859891..e8de249339d8 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -157,36 +157,18 @@ static inline notrace void irq_soft_mask_set(unsigned
> long mask)
>
> static inline notrace unsigned long irq_soft_mask_set_return(unsigned long
> mask)
> {
> - unsigned long flags;
> -
> -#ifdef CONFIG_PPC_IRQ_SOFT_MASK_DEBUG
> - WARN_ON(mask && !(mask & IRQS_DISABLED));
> -#endif
> + unsigned long flags = irq_soft_mask_return();
>
> - asm volatile(
> - "lbz %0,%1(13); stb %2,%1(13)"
> - : "=" (flags)
> - : "i" (offsetof(struct paca_struct, irq_soft_mask)),
> - "r" (mask)
> - : "memory");
> + irq_soft_mask_set(mask);
>
> return flags;
> }
>
> static inline notrace unsigned long irq_soft_mask_or_return(unsigned long
> mask)
> {
> - unsigned long flags, tmp;
> + unsigned long flags = irq_soft_mask_return();
>
> - asm volatile(
> - "lbz %0,%2(13); or %1,%0,%3; stb %1,%2(13)"
> - : "=" (flags), "=r" (tmp)
> - : "i" (offsetof(struct paca_struct, irq_soft_mask)),
> - "r" (mask)
> - : "memory");
> -
> -#ifdef CONFIG_PPC_IRQ_SOFT_MASK_DEBUG
> - WARN_ON((mask | flags) && !((mask | flags) & IRQS_DISABLED));
> -#endif
> + irq_soft_mask_set(flags | mask);
>
> return flags;
> }
> --
> 2.37.1
Re: [PATCH 1/2] tools/perf/tests: Fix string substitutions in build id test
On 22/09/22 22:15, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 21, 2022 at 10:38:38PM +0530, Athira Rajeev escreveu:
>> The perf test named “build id cache operations” skips with below
>> error on some distros:
>
> I wonder if we shouldn't instead state that bash is needed?
>
> ⬢[acme@toolbox perf-urgent]$ head -1 tools/perf/tests/shell/*.sh | grep ^#
> #!/bin/sh
> #!/bin/bash
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/bash
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/bash
> #!/bin/sh
> #!/bin/bash
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> #!/bin/sh
> ⬢[acme@toolbox perf-urgent]$
>
> Opinions?
>
Please don't change tools/perf/tests/shell/test_intel_pt.sh
I started using shellcheck on that with the "perf test:
test_intel_pt.sh: Add per-thread test" patch set that I sent.
FYI, if you use shellcheck on buildid.sh, it shows up issues even
after changing to bash:
*** Before ***
$ shellcheck -S warning tools/perf/tests/shell/buildid.sh
In tools/perf/tests/shell/buildid.sh line 69:
link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
^---^ SC2039: In POSIX sh, string
indexing is undefined.
^-^ SC2039: In POSIX sh,
string indexing is undefined.
In tools/perf/tests/shell/buildid.sh line 77:
file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
^---^ SC2039: In POSIX sh, string
indexing is undefined.
In tools/perf/tests/shell/buildid.sh line 123:
echo "running: perf record $@"
^-- SC2145: Argument mixes string and array.
Use * or separate argument.
In tools/perf/tests/shell/buildid.sh line 124:
${perf} record --buildid-all -o ${data} $@ &> ${log}
^-- SC2068: Double quote array
expansions to avoid re-splitting elements.
^---^ SC2039: In POSIX
sh, &> is undefined.
In tools/perf/tests/shell/buildid.sh line 126:
echo "failed: record $@"
^-- SC2145: Argument mixes string and
array. Use * or separate argument.
In tools/perf/tests/shell/buildid.sh line 131:
check ${@: -1}
^--^ SC2068: Double quote array expansions to avoid
re-splitting elements.
^--^ SC2039: In POSIX sh, string indexing is undefined.
In tools/perf/tests/shell/buildid.sh line 158:
exit ${err}
^^ SC2154: err is referenced but not assigned.
For more information:
https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
https://www.shellcheck.net/wiki/SC2039 -- In POSIX sh, &> is undefined.
*** After ***
$ shellcheck -S warning tools/perf/tests/shell/buildid.sh
In tools/perf/tests/shell/buildid.sh line 123:
echo "running: perf record $@"
^-- SC2145: Argument mixes string and array.
Use * or separate argument.
In tools/perf/tests/shell/buildid.sh line 124:
${perf} record --buildid-all -o ${data} $@ &> ${log}
^-- SC2068: Double quote array
expansions to avoid re-splitting elements.
In tools/perf/tests/shell/buildid.sh line 126:
echo "failed: record $@"
^-- SC2145: Argument mixes string and
array. Use * or separate argument.
In tools/perf/tests/shell/buildid.sh line 131:
check ${@: -1}
^--^ SC2068: Double quote array expansions to avoid
re-splitting elements.
In tools/perf/tests/shell/buildid.sh line 158:
exit ${err}
^^ SC2154: err is referenced but not assigned.
For more information:
https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
https://www.shellcheck.net/wiki/SC2154 -- err is referenced but not assigned.
Re: [PATCH] powerpc/64s: POWER10 CPU Kconfig build option
On Fri Sep 23, 2022 at 3:46 PM AEST, Christophe Leroy wrote: > > > Le 23/09/2022 à 05:30, Nicholas Piggin a écrit : > > This adds basic POWER10_CPU option, which builds with -mcpu=power10. > > > > Signed-off-by: Nicholas Piggin > > --- > > There's quite a lot of asm and linker changes slated for the next merge > > window already so I may leave the pcrel patch for next time. I think we > > can add the basic POWER10 build option though. > > > > Thanks, > > Nick > > > > arch/powerpc/Makefile | 7 ++- > > arch/powerpc/platforms/Kconfig.cputype | 8 +++- > > 2 files changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile > > index 8a3d69b02672..ea88af26f8c6 100644 > > --- a/arch/powerpc/Makefile > > +++ b/arch/powerpc/Makefile > > @@ -192,9 +192,14 @@ ifdef CONFIG_476FPE_ERR46 > > -T $(srctree)/arch/powerpc/platforms/44x/ppc476_modules.lds > > endif > > > > -# No AltiVec or VSX instructions when building kernel > > +# No prefix or pcrel > > +KBUILD_CFLAGS += $(call cc-option,-mno-prefixed) > > We have lots of code to handle prefixed instructions in code_patching, > and that code complexifies stuff and has a performance impact. > And it is only partially taken into account, areas like ftrace don't > properly take care of prefixed instructions. > > Should we get rid of prefixed instruction support completely in the > kernel, and come back to more simple code ? I would rather complete prefixed support in the kernel and use pcrel addressing. Actually even if we don't compile with pcrel or prefixed, there are some instructions and we will probably get more that require prefixed, possible we might want to use them in kernel. Some of it is required to handle user mode instructions too. So I think removing it is premature, but I guess it's up for debate. Thanks, Nick
Re: [PATCH 1/5] powerpc/64: use 32-bit immediate for STACK_FRAME_REGS_MARKER
On Fri Sep 23, 2022 at 3:34 PM AEST, Christophe Leroy wrote: > > > Le 23/09/2022 à 05:25, Nicholas Piggin a écrit : > > Using a 32-bit constant for this marker allows it to be loaded with > > two ALU instructions, like 32-bit. This avoids a TOC entry and a > > TOC load that depends on the r2 value that has just been loaded from > > the PACA. > > > > This changes the value for 32-bit as well, so both have the same > > value in the low 4 bytes and 64-bit has 0x in the top bytes. > > > > Signed-off-by: Nicholas Piggin > > --- > > arch/powerpc/include/asm/ptrace.h| 5 +++-- > > arch/powerpc/kernel/entry_32.S | 6 +++--- > > arch/powerpc/kernel/exceptions-64e.S | 8 +--- > > arch/powerpc/kernel/exceptions-64s.S | 2 +- > > arch/powerpc/kernel/head_64.S| 7 --- > > arch/powerpc/kernel/interrupt_64.S | 6 +++--- > > 6 files changed, 11 insertions(+), 23 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/ptrace.h > > b/arch/powerpc/include/asm/ptrace.h > > index a03403695cd4..49d720bb888b 100644 > > --- a/arch/powerpc/include/asm/ptrace.h > > +++ b/arch/powerpc/include/asm/ptrace.h > > @@ -99,6 +99,9 @@ struct pt_regs > > > > #define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct > > pt_regs)) > > > > +/* 0x8d9a988d on 64-bit */ > > +#define STACK_FRAME_REGS_MARKERASM_CONST(-0x72656773) /* 0x8d9a988d */ > > + > > 0x72656773 is "REGS" in ASCII (Big Endian) and you can spot it > immediatly in a memory dump. > 0x7265677368657265 is "REGSHERE". > > 0x8d9a988d is not printable. > > Don't know if it can be a problem. Oh. I guess it doesn't really matter if it has zeroes or f in the top 4 bytes so I should keep it "REGS" then. I was thinking about later moving it into the reserved word next to the CR word on 64-bit so we would only have 4 bytes for it anyway. Thanks, Nick
[PATCH v2 1/3] powerpc: Add common pud_pfn stub for all platforms
Prior to this commit, pud_pfn was implemented with BUILD_BUG as the inline
function for 64-bit Book3S systems but is never included, as its
invocations in generic code are guarded by calls to pud_devmap which return
zero on such systems. A future patch will provide support for page table
checks, the generic code for which depends on a pud_pfn stub being
implemented, even while the patch will not interact with puds directly.
Remove the 64-bit Book3S stub and define pud_pfn to warn on all
platforms. pud_pfn may be defined properly on a per-platform basis
should it grow real usages in future.
Signed-off-by: Rohan McLure
---
V2: Remove conditional BUILD_BUG and BUG. Instead warn on usage.
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 10 --
arch/powerpc/include/asm/pgtable.h | 14 ++
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 486902aff040..f9aefa492df0 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1413,16 +1413,6 @@ static inline int pgd_devmap(pgd_t pgd)
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-static inline int pud_pfn(pud_t pud)
-{
- /*
-* Currently all calls to pud_pfn() are gated around a pud_devmap()
-* check so this should never be used. If it grows another user we
-* want to know about it.
-*/
- BUILD_BUG();
- return 0;
-}
#define __HAVE_ARCH_PTEP_MODIFY_PROT_TRANSACTION
pte_t ptep_modify_prot_start(struct vm_area_struct *, unsigned long, pte_t *);
void ptep_modify_prot_commit(struct vm_area_struct *, unsigned long,
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index 33f4bf8d22b0..36956fb440e1 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -158,6 +158,20 @@ struct seq_file;
void arch_report_meminfo(struct seq_file *m);
#endif /* CONFIG_PPC64 */
+/*
+ * Currently only consumed by page_table_check_pud_{set,clear}. Since clears
+ * and sets to page table entries at any level are done through
+ * page_table_check_pte_{set,clear}, provide stub implementation.
+ */
+#ifndef pud_pfn
+#define pud_pfn pud_pfn
+static inline int pud_pfn(pud_t pud)
+{
+ WARN(1, "pud: platform does not use pud entries directly");
+ return 0;
+}
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_PGTABLE_H */
--
2.34.1
[PATCH v2 2/3] powerpc: mm: add p{te,md,ud}_user_accessible_page helpers
Add the following helpers for detecting whether a page table entry
is a leaf and is accessible to user space.
* pte_user_accessible_page
* pmd_user_accessible_page
* pud_user_accessible_page
Also implement missing pud_user definitions for both Book3S/E 64-bit
systems.
Signed-off-by: Rohan McLure
---
V2: Provide missing pud_user implementations, use p{u,m}d_is_leaf.
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 10 ++
arch/powerpc/include/asm/nohash/64/pgtable.h | 10 ++
arch/powerpc/include/asm/pgtable.h | 33 ++
3 files changed, 53 insertions(+)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index f9aefa492df0..3083111f9d0a 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -621,6 +621,16 @@ static inline bool pte_user(pte_t pte)
return !(pte_raw(pte) & cpu_to_be64(_PAGE_PRIVILEGED));
}
+static inline bool pmd_user(pmd_t pmd)
+{
+ return !(pmd_raw(pmd) & cpu_to_be64(_PAGE_PRIVILEGED));
+}
+
+static inline bool pud_user(pud_t pud)
+{
+ return !(pud_raw(pud) & cpu_to_be64(_PAGE_PRIVILEGED));
+}
+
#define pte_access_permitted pte_access_permitted
static inline bool pte_access_permitted(pte_t pte, bool write)
{
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h
b/arch/powerpc/include/asm/nohash/64/pgtable.h
index 599921cc257e..23c5135178d1 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -123,6 +123,11 @@ static inline pte_t pmd_pte(pmd_t pmd)
return __pte(pmd_val(pmd));
}
+static inline bool pmd_user(pmd_t pmd)
+{
+ return (pmd_val(pmd) & _PAGE_USER) == _PAGE_USER;
+}
+
#define pmd_none(pmd) (!pmd_val(pmd))
#definepmd_bad(pmd)(!is_kernel_addr(pmd_val(pmd)) \
|| (pmd_val(pmd) & PMD_BAD_BITS))
@@ -158,6 +163,11 @@ static inline pte_t pud_pte(pud_t pud)
return __pte(pud_val(pud));
}
+static inline bool pud_user(pud_t pud)
+{
+ return (pud_val(pud) & _PAGE_USER) == _PAGE_USER;
+}
+
static inline pud_t pte_pud(pte_t pte)
{
return __pud(pte_val(pte));
diff --git a/arch/powerpc/include/asm/pgtable.h
b/arch/powerpc/include/asm/pgtable.h
index 36956fb440e1..69eed4e03da0 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -172,6 +172,39 @@ static inline int pud_pfn(pud_t pud)
}
#endif
+static inline bool pte_user_accessible_page(pte_t pte)
+{
+ return pte_present(pte) && pte_user(pte);
+}
+
+#ifdef CONFIG_PPC64
+
+static inline bool pmd_user_accessible_page(pmd_t pmd)
+{
+ return pmd_is_leaf(pmd) && pmd_present(pmd) && pmd_user(pmd);
+}
+
+static inline bool pud_user_accessible_page(pud_t pud)
+{
+ return pud_is_leaf(pud) && pud_present(pud) && pud_user(pud);
+}
+
+#else
+
+static inline bool pmd_user_accessible_page(pmd_t pmd)
+{
+ WARN(1, "pmd: multi-level paging unsupported on ppc32");
+ return false;
+}
+
+static inline bool pud_user_accessible_page(pud_t pud)
+{
+ WARN(1, "pud: multi-level paging unsupported on ppc32");
+ return false;
+}
+
+#endif /* CONFIG_PPC64 */
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_PGTABLE_H */
--
2.34.1
[PATCH v2 3/3] powerpc: mm: support page table check
On creation and clearing of a page table mapping, instrument such calls
by invoking page_table_check_pte_set and page_table_check_pte_clear
respectively. These calls serve as a sanity check against illegal
mappings.
Enable ARCH_SUPPORTS_PAGE_TABLE_CHECK for all ppc64, and 32-bit
platforms implementing Book3S.
Change pud_pfn to be a runtime bug rather than a build bug as it is
consumed by page_table_check_pud_{clear,set} which are not called.
See also:
riscv support in commit 3fee229a8eb9 ("riscv/mm: enable
ARCH_SUPPORTS_PAGE_TABLE_CHECK")
arm64 in commit 42b2547137f5 ("arm64/mm: enable
ARCH_SUPPORTS_PAGE_TABLE_CHECK")
x86_64 in commit d283d422c6c4 ("x86: mm: add x86_64 support for page table
check")
Signed-off-by: Rohan McLure
---
V2: Update spacing and types assigned to pte_update calls.
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/book3s/32/pgtable.h | 9 -
arch/powerpc/include/asm/book3s/64/pgtable.h | 18 +++---
arch/powerpc/include/asm/nohash/32/pgtable.h | 7 ++-
arch/powerpc/include/asm/nohash/64/pgtable.h | 8 ++--
arch/powerpc/include/asm/nohash/pgtable.h| 1 +
6 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 4c466acdc70d..6c213ac46a92 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -149,6 +149,7 @@ config PPC
select ARCH_STACKWALK
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_SUPPORTS_DEBUG_PAGEALLOCif PPC_BOOK3S || PPC_8xx || 40x
+ select ARCH_SUPPORTS_PAGE_TABLE_CHECK
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
select ARCH_USE_MEMTEST
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h
b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 40041ac713d9..e76aca557d48 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -53,6 +53,8 @@
#ifndef __ASSEMBLY__
+#include
+
static inline bool pte_user(pte_t pte)
{
return pte_val(pte) & _PAGE_USER;
@@ -353,7 +355,11 @@ static inline int __ptep_test_and_clear_young(struct
mm_struct *mm,
static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long
addr,
pte_t *ptep)
{
- return __pte(pte_update(mm, addr, ptep, ~_PAGE_HASHPTE, 0, 0));
+ pte_t old_pte = __pte(pte_update(mm, addr, ptep, ~_PAGE_HASHPTE, 0, 0));
+
+ page_table_check_pte_clear(mm, addr, old_pte);
+
+ return old_pte;
}
#define __HAVE_ARCH_PTEP_SET_WRPROTECT
@@ -541,6 +547,7 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, int percpu)
{
+ page_table_check_pte_set(mm, addr, ptep, pte);
#if defined(CONFIG_SMP) && !defined(CONFIG_PTE_64BIT)
/* First case is 32-bit Hash MMU in SMP mode with 32-bit PTEs. We use
the
* helper pte_update() which does an atomic update. We need to do that
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 3083111f9d0a..b5c5718d9b90 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -181,6 +181,8 @@
#define PAGE_AGP (PAGE_KERNEL_NC)
#ifndef __ASSEMBLY__
+#include
+
/*
* page table defines
*/
@@ -484,8 +486,11 @@ static inline void huge_ptep_set_wrprotect(struct
mm_struct *mm,
static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
unsigned long addr, pte_t *ptep)
{
- unsigned long old = pte_update(mm, addr, ptep, ~0UL, 0, 0);
- return __pte(old);
+ pte_t old_pte = __pte(pte_update(mm, addr, ptep, ~0UL, 0, 0));
+
+ page_table_check_pte_clear(mm, addr, old_pte);
+
+ return old_pte;
}
#define __HAVE_ARCH_PTEP_GET_AND_CLEAR_FULL
@@ -494,11 +499,16 @@ static inline pte_t ptep_get_and_clear_full(struct
mm_struct *mm,
pte_t *ptep, int full)
{
if (full && radix_enabled()) {
+ pte_t old_pte;
+
/*
* We know that this is a full mm pte clear and
* hence can be sure there is no parallel set_pte.
*/
- return radix__ptep_get_and_clear_full(mm, addr, ptep, full);
+ old_pte = radix__ptep_get_and_clear_full(mm, addr, ptep, full);
+ page_table_check_pte_clear(mm, addr, old_pte);
+
+ return old_pte;
}
return ptep_get_and_clear(mm, addr, ptep);
}
@@ -884,6 +894,8 @@ static inline void __set_pte_at(struct mm_struct *mm,
unsigned long addr,
*/
pte = __pte_raw(pte_raw(pte) | cpu_to_be64(_PAGE_PTE));
+ page_table_check_pte_set(mm, addr, ptep, pte);
+
if
