* Re: [PATCH] mm/kprobes: Add generic kprobe_fault_handler() fallback definition
From: Masami Hiramatsu @ 2019-07-05 10:30 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Kate Stewart, Mark Rutland, Rich Felker, linux-ia64, linux-sh,
Heiko Carstens, linux-mips, linux-mm, Paul Mackerras,
H. Peter Anvin, sparclinux, Will Deacon, linux-s390,
Yoshinori Sato, x86, Russell King, Anil S Keshavamurthy,
Christian Borntraeger, Ingo Molnar, linux-arm-kernel,
Catalin Marinas, James Hogan, linux-snps-arc, Guenter Roeck,
Fenghua Yu, Vasily Gorbik, linuxppc-dev, Naveen N. Rao,
Borislav Petkov, Thomas Gleixner, Allison Randal, Tony Luck,
Richard Fontana, Vineet Gupta, linux-kernel, Ralf Baechle,
Paul Burton, Masami Hiramatsu, Greg Kroah-Hartman, Andrew Morton,
Enrico Weigelt, David S. Miller
In-Reply-To: <1562304629-29376-1-git-send-email-anshuman.khandual@arm.com>
Hi Anshuman,
On Fri, 5 Jul 2019 11:00:29 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> Architectures like parisc enable CONFIG_KROBES without having a definition
> for kprobe_fault_handler() which results in a build failure.
Hmm, as far as I can see, kprobe_fault_handler() is closed inside each arch
specific code. The reason why include/linux/kprobes.h defines
dummy inline function is only for !CONFIG_KPROBES case.
> Arch needs to
> provide kprobe_fault_handler() as it is platform specific and cannot have
> a generic working alternative. But in the event when platform lacks such a
> definition there needs to be a fallback.
Wait, indeed that each arch need to implement it, but that is for calling
kprobe->fault_handler() as user expected.
Hmm, why not fixing those architecture implementations?
> This adds a stub kprobe_fault_handler() definition which not only prevents
> a build failure but also makes sure that kprobe_page_fault() if called will
> always return negative in absence of a sane platform specific alternative.
I don't like introducing this complicated macro only for avoiding (not fixing)
build error. To fix that, kprobes on parisc should implement kprobe_fault_handler
correctly (and call kprobe->fault_handler).
BTW, even if you need such generic stub, please use a weak function instead
of macros for every arch headers.
> While here wrap kprobe_page_fault() in CONFIG_KPROBES. This enables stud
> definitions for generic kporbe_fault_handler() and kprobes_built_in() can
> just be dropped. Only on x86 it needs to be added back locally as it gets
> used in a !CONFIG_KPROBES function do_general_protection().
If you want to remove kprobes_built_in(), you should replace it with
IS_ENABLED(CONFIG_KPROBES), instead of this...
Thank you,
>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Allison Randal <allison@lohutok.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Enrico Weigelt <info@metux.net>
> Cc: Richard Fontana <rfontana@redhat.com>
> Cc: Kate Stewart <kstewart@linuxfoundation.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: x86@kernel.org
> Cc: linux-snps-arc@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linux-mips@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arc/include/asm/kprobes.h | 1 +
> arch/arm/include/asm/kprobes.h | 1 +
> arch/arm64/include/asm/kprobes.h | 1 +
> arch/ia64/include/asm/kprobes.h | 1 +
> arch/mips/include/asm/kprobes.h | 1 +
> arch/powerpc/include/asm/kprobes.h | 1 +
> arch/s390/include/asm/kprobes.h | 1 +
> arch/sh/include/asm/kprobes.h | 1 +
> arch/sparc/include/asm/kprobes.h | 1 +
> arch/x86/include/asm/kprobes.h | 6 ++++++
> include/linux/kprobes.h | 32 ++++++++++++++++++------------
> 11 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arc/include/asm/kprobes.h b/arch/arc/include/asm/kprobes.h
> index 2134721dce44..ee8efe256675 100644
> --- a/arch/arc/include/asm/kprobes.h
> +++ b/arch/arc/include/asm/kprobes.h
> @@ -45,6 +45,7 @@ struct kprobe_ctlblk {
> struct prev_kprobe prev_kprobe;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> int kprobe_fault_handler(struct pt_regs *regs, unsigned long cause);
> void kretprobe_trampoline(void);
> void trap_is_kprobe(unsigned long address, struct pt_regs *regs);
> diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h
> index 213607a1f45c..660f877b989f 100644
> --- a/arch/arm/include/asm/kprobes.h
> +++ b/arch/arm/include/asm/kprobes.h
> @@ -38,6 +38,7 @@ struct kprobe_ctlblk {
> struct prev_kprobe prev_kprobe;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> void arch_remove_kprobe(struct kprobe *);
> int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr);
> int kprobe_exceptions_notify(struct notifier_block *self,
> diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
> index 97e511d645a2..667773f75616 100644
> --- a/arch/arm64/include/asm/kprobes.h
> +++ b/arch/arm64/include/asm/kprobes.h
> @@ -42,6 +42,7 @@ struct kprobe_ctlblk {
> struct kprobe_step_ctx ss_ctx;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> void arch_remove_kprobe(struct kprobe *);
> int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr);
> int kprobe_exceptions_notify(struct notifier_block *self,
> diff --git a/arch/ia64/include/asm/kprobes.h b/arch/ia64/include/asm/kprobes.h
> index c5cf5e4fb338..c321e8585089 100644
> --- a/arch/ia64/include/asm/kprobes.h
> +++ b/arch/ia64/include/asm/kprobes.h
> @@ -106,6 +106,7 @@ struct arch_specific_insn {
> unsigned short slot;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> extern int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data);
> diff --git a/arch/mips/include/asm/kprobes.h b/arch/mips/include/asm/kprobes.h
> index 68b1e5d458cf..d1efe991ea22 100644
> --- a/arch/mips/include/asm/kprobes.h
> +++ b/arch/mips/include/asm/kprobes.h
> @@ -40,6 +40,7 @@ do { \
>
> #define kretprobe_blacklist_size 0
>
> +#define kprobe_fault_handler kprobe_fault_handler
> void arch_remove_kprobe(struct kprobe *p);
> int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
>
> diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
> index 66b3f2983b22..c94f375ec957 100644
> --- a/arch/powerpc/include/asm/kprobes.h
> +++ b/arch/powerpc/include/asm/kprobes.h
> @@ -84,6 +84,7 @@ struct arch_optimized_insn {
> kprobe_opcode_t *insn;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> extern int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data);
> extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> diff --git a/arch/s390/include/asm/kprobes.h b/arch/s390/include/asm/kprobes.h
> index b106aa29bf55..0ecaebb78092 100644
> --- a/arch/s390/include/asm/kprobes.h
> +++ b/arch/s390/include/asm/kprobes.h
> @@ -73,6 +73,7 @@ struct kprobe_ctlblk {
> void arch_remove_kprobe(struct kprobe *p);
> void kretprobe_trampoline(void);
>
> +#define kprobe_fault_handler kprobe_fault_handler
> int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data);
> diff --git a/arch/sh/include/asm/kprobes.h b/arch/sh/include/asm/kprobes.h
> index 6171682f7798..637a698393c0 100644
> --- a/arch/sh/include/asm/kprobes.h
> +++ b/arch/sh/include/asm/kprobes.h
> @@ -45,6 +45,7 @@ struct kprobe_ctlblk {
> struct prev_kprobe prev_kprobe;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> extern int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data);
> diff --git a/arch/sparc/include/asm/kprobes.h b/arch/sparc/include/asm/kprobes.h
> index bfcaa6326c20..9aa4d25a45a8 100644
> --- a/arch/sparc/include/asm/kprobes.h
> +++ b/arch/sparc/include/asm/kprobes.h
> @@ -47,6 +47,7 @@ struct kprobe_ctlblk {
> struct prev_kprobe prev_kprobe;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data);
> int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
> index 5dc909d9ad81..1af2b6db13bd 100644
> --- a/arch/x86/include/asm/kprobes.h
> +++ b/arch/x86/include/asm/kprobes.h
> @@ -101,11 +101,17 @@ struct kprobe_ctlblk {
> struct prev_kprobe prev_kprobe;
> };
>
> +#define kprobe_fault_handler kprobe_fault_handler
> extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> extern int kprobe_exceptions_notify(struct notifier_block *self,
> unsigned long val, void *data);
> extern int kprobe_int3_handler(struct pt_regs *regs);
> extern int kprobe_debug_handler(struct pt_regs *regs);
> +#else
> +static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> +{
> + return 0;
> +}
>
> #endif /* CONFIG_KPROBES */
> #endif /* _ASM_X86_KPROBES_H */
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 04bdaf01112c..e106f3018804 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -182,11 +182,19 @@ DECLARE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> /*
> * For #ifdef avoidance:
> */
> -static inline int kprobes_built_in(void)
> +
> +/*
> + * Architectures need to override this with their own implementation
> + * if they care to call kprobe_page_fault(). This will just ensure
> + * that kprobe_page_fault() returns false when called without having
> + * a proper platform specific definition for kprobe_fault_handler().
> + */
> +#ifndef kprobe_fault_handler
> +static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> {
> - return 1;
> + return 0;
> }
> -
> +#endif
> #ifdef CONFIG_KRETPROBES
> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs);
> @@ -375,14 +383,6 @@ void free_insn_page(void *page);
>
> #else /* !CONFIG_KPROBES: */
>
> -static inline int kprobes_built_in(void)
> -{
> - return 0;
> -}
> -static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> -{
> - return 0;
> -}
> static inline struct kprobe *get_kprobe(void *addr)
> {
> return NULL;
> @@ -458,12 +458,11 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr)
> }
> #endif
>
> +#ifdef CONFIG_KPROBES
> /* Returns true if kprobes handled the fault */
> static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs,
> unsigned int trap)
> {
> - if (!kprobes_built_in())
> - return false;
> if (user_mode(regs))
> return false;
> /*
> @@ -476,5 +475,12 @@ static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs,
> return false;
> return kprobe_fault_handler(regs, trap);
> }
> +#else
> +static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs,
> + unsigned int trap)
> +{
> + return false;
> +}
> +#endif
>
> #endif /* _LINUX_KPROBES_H */
> --
> 2.20.1
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
From: Jarkko Sakkinen @ 2019-07-05 10:42 UTC (permalink / raw)
To: Nayna Jain, linux-integrity, linuxppc-dev, Michal Suchanek
Cc: Sachin Sant, George Wilson, linux-kernel, Mimi Zohar,
Jason Gunthorpe, Peter Huewe, Michal Suchanek
In-Reply-To: <1562211121-2188-1-git-send-email-nayna@linux.ibm.com>
On Wed, 2019-07-03 at 23:32 -0400, Nayna Jain wrote:
> The nr_allocated_banks and allocated banks are initialized as part of
> tpm_chip_register. Currently, this is done as part of auto startup
> function. However, some drivers, like the ibm vtpm driver, do not run
> auto startup during initialization. This results in uninitialized memory
> issue and causes a kernel panic during boot.
>
> This patch moves the pcr allocation outside the auto startup function
> into tpm_chip_register. This ensures that allocated banks are initialized
> in any case.
>
> Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with
> PCR read")
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Please add
Reported-by: Michal Suchanek <msuchanek@suse.de>
It is missing. Michal is there a chance you could try this out once
Nayna send a new version?
> ---
> drivers/char/tpm/tpm-chip.c | 37 +++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm.h | 1 +
> drivers/char/tpm/tpm1-cmd.c | 12 ------------
> drivers/char/tpm/tpm2-cmd.c | 6 +-----
> 4 files changed, 39 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 8804c9e916fd..958508bb8379 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -550,6 +550,39 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
> return hwrng_register(&chip->hwrng);
> }
>
> +/*
> + * tpm_pcr_allocation() - initializes the chip allocated banks for PCRs
> + */
> +static int tpm_pcr_allocation(struct tpm_chip *chip)
Why that name and not tpm_get_pcr_allocation()? Do not get why
"get_" has been dropped. Please add it back.
Would be senseful to create tpm1_get_pcr_allocation() to tpm1-cmd.c
now that a new function needs to be introduced anyway. Please do
it for that for TPM 1.x part.
/Jarkko
^ permalink raw reply
* Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
From: Michal Suchánek @ 2019-07-05 10:51 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sachin Sant, Nayna Jain, linux-kernel, Mimi Zohar,
Jason Gunthorpe, linux-integrity, George Wilson, linuxppc-dev,
Peter Huewe
In-Reply-To: <f315356e7c00378c8785bd20d5869c9046ece2f2.camel@linux.intel.com>
On Fri, 05 Jul 2019 13:42:18 +0300
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> On Wed, 2019-07-03 at 23:32 -0400, Nayna Jain wrote:
> > The nr_allocated_banks and allocated banks are initialized as part of
> > tpm_chip_register. Currently, this is done as part of auto startup
> > function. However, some drivers, like the ibm vtpm driver, do not run
> > auto startup during initialization. This results in uninitialized memory
> > issue and causes a kernel panic during boot.
> >
> > This patch moves the pcr allocation outside the auto startup function
> > into tpm_chip_register. This ensures that allocated banks are initialized
> > in any case.
> >
> > Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with
> > PCR read")
> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>
> Please add
>
> Reported-by: Michal Suchanek <msuchanek@suse.de>
>
> It is missing. Michal is there a chance you could try this out once
> Nayna send a new version?
Should be easy to test. Without the patch the machine crashes on boot
so it is quite obvious case. I have a few VMs with the vTPM available.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
From: Jarkko Sakkinen @ 2019-07-05 11:02 UTC (permalink / raw)
To: Nayna Jain, linux-integrity, linuxppc-dev, Michal Suchanek
Cc: Sachin Sant, linux-kernel, Mimi Zohar, Jason Gunthorpe,
Peter Huewe, George Wilson
In-Reply-To: <f315356e7c00378c8785bd20d5869c9046ece2f2.camel@linux.intel.com>
On Fri, 2019-07-05 at 13:42 +0300, Jarkko Sakkinen wrote:
> On Wed, 2019-07-03 at 23:32 -0400, Nayna Jain wrote:
> > The nr_allocated_banks and allocated banks are initialized as part of
> > tpm_chip_register. Currently, this is done as part of auto startup
> > function. However, some drivers, like the ibm vtpm driver, do not run
> > auto startup during initialization. This results in uninitialized memory
> > issue and causes a kernel panic during boot.
> >
> > This patch moves the pcr allocation outside the auto startup function
> > into tpm_chip_register. This ensures that allocated banks are initialized
> > in any case.
> >
> > Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with
> > PCR read")
> > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>
> Please add
>
> Reported-by: Michal Suchanek <msuchanek@suse.de>
>
> It is missing. Michal is there a chance you could try this out once
> Nayna send a new version?
Hey, I saw Michal's tested-by. I can do that minor reorg cosmetic
bits myeslf and add Micha's tag.
Some issue with the network but I'll push a commit soonish.
/Jarkko
^ permalink raw reply
* Re: [PATCH v12 01/11] MODSIGN: Export module signature definitions
From: Philipp Rudo @ 2019-07-05 13:00 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-s390, Herbert Xu, linux-doc, Dmitry Kasatkin,
David S. Miller, Jonathan Corbet, Heiko Carstens, linux-kernel,
Mimi Zohar, James Morris, David Howells, AKASHI, Takahiro,
linux-security-module, keyrings, linux-crypto, Jessica Yu,
linux-integrity, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <874l41ocf5.fsf@morokweng.localdomain>
Hi Thiago,
On Thu, 04 Jul 2019 15:57:34 -0300
Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:
> Hello Philipp,
>
> Philipp Rudo <prudo@linux.ibm.com> writes:
>
> > Hi Thiago,
> >
> >
> > On Thu, 04 Jul 2019 03:42:57 -0300
> > Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:
> >
> >> Jessica Yu <jeyu@kernel.org> writes:
> >>
> >> > +++ Thiago Jung Bauermann [27/06/19 23:19 -0300]:
> >> >>IMA will use the module_signature format for append signatures, so export
> >> >>the relevant definitions and factor out the code which verifies that the
> >> >>appended signature trailer is valid.
> >> >>
> >> >>Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
> >> >>and be able to use mod_check_sig() without having to depend on either
> >> >>CONFIG_MODULE_SIG or CONFIG_MODULES.
> >> >>
> >> >>Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> >> >>Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> >> >>Cc: Jessica Yu <jeyu@kernel.org>
> >> >>---
> >> >> include/linux/module.h | 3 --
> >> >> include/linux/module_signature.h | 44 +++++++++++++++++++++++++
> >> >> init/Kconfig | 6 +++-
> >> >> kernel/Makefile | 1 +
> >> >> kernel/module.c | 1 +
> >> >> kernel/module_signature.c | 46 ++++++++++++++++++++++++++
> >> >> kernel/module_signing.c | 56 +++++---------------------------
> >> >> scripts/Makefile | 2 +-
> >> >> 8 files changed, 106 insertions(+), 53 deletions(-)
> >> >>
> >> >>diff --git a/include/linux/module.h b/include/linux/module.h
> >> >>index 188998d3dca9..aa56f531cf1e 100644
> >> >>--- a/include/linux/module.h
> >> >>+++ b/include/linux/module.h
> >> >>@@ -25,9 +25,6 @@
> >> >> #include <linux/percpu.h>
> >> >> #include <asm/module.h>
> >> >>
> >> >>-/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
> >> >>-#define MODULE_SIG_STRING "~Module signature appended~\n"
> >> >>-
> >> >
> >> > Hi Thiago, apologies for the delay.
> >>
> >> Hello Jessica, thanks for reviewing the patch!
> >>
> >> > It looks like arch/s390/kernel/machine_kexec_file.c also relies on
> >> > MODULE_SIG_STRING being defined, so module_signature.h will need to be
> >> > included there too, otherwise we'll run into a compilation error.
> >>
> >> Indeed. Thanks for spotting that. The patch below fixes it. It's
> >> identical to the previous version except for the changes in
> >> arch/s390/kernel/machine_kexec_file.c and their description in the
> >> commit message. I'm also copying some s390 people in this email.
> >
> > to me the s390 part looks good but for one minor nit.
>
> Thanks for the prompt review!
>
> > In arch/s390/Kconfig KEXEC_VERIFY_SIG currently depends on
> > SYSTEM_DATA_VERIFICATION. I'd prefer when you update this to the new
> > MODULE_SIG_FORMAT. It shouldn't make any difference right now, as we don't
> > use mod_check_sig in our code path. But it could cause problems in the future,
> > when more code might be shared.
>
> Makes sense. Here is the updated patch with the Kconfig change.
>
The patch looks good now.
Thanks a lot
PHilipp
^ permalink raw reply
* Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
From: Stefan Berger @ 2019-07-05 14:13 UTC (permalink / raw)
To: Nayna Jain, linux-integrity, linuxppc-dev
Cc: Sachin Sant, Michal Suchanek, linux-kernel, Mimi Zohar,
Jason Gunthorpe, Jarkko Sakkinen, Peter Huewe, George Wilson
In-Reply-To: <1562211121-2188-1-git-send-email-nayna@linux.ibm.com>
On 7/3/19 11:32 PM, Nayna Jain wrote:
> The nr_allocated_banks and allocated banks are initialized as part of
> tpm_chip_register. Currently, this is done as part of auto startup
> function. However, some drivers, like the ibm vtpm driver, do not run
> auto startup during initialization. This results in uninitialized memory
> issue and causes a kernel panic during boot.
>
> This patch moves the pcr allocation outside the auto startup function
> into tpm_chip_register. This ensures that allocated banks are initialized
> in any case.
>
> Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with
> PCR read")
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
> drivers/char/tpm/tpm-chip.c | 37 +++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm.h | 1 +
> drivers/char/tpm/tpm1-cmd.c | 12 ------------
> drivers/char/tpm/tpm2-cmd.c | 6 +-----
> 4 files changed, 39 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 8804c9e916fd..958508bb8379 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -550,6 +550,39 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
> return hwrng_register(&chip->hwrng);
> }
>
> +/*
> + * tpm_pcr_allocation() - initializes the chip allocated banks for PCRs
> + */
> +static int tpm_pcr_allocation(struct tpm_chip *chip)
> +{
> + int rc = 0;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> + rc = tpm2_get_pcr_allocation(chip);
> + if (rc)
> + goto out;
> + }
> +
> + /* Initialize TPM 1.2 */
> + chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
> + GFP_KERNEL);
> + if (!chip->allocated_banks) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
> + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
> + chip->nr_allocated_banks = 1;
> +
> + return 0;
> +out:
> + if (rc < 0)
> + rc = -ENODEV;
The old code where you lifted this from said:
out:
if (rc > 0)
rc = -ENODEV;
return rc;
It would not overwrite -ENOMEM with -ENODEV but yours does.
I think the correct fix would be to use:
if (rc > 0)
rc = -ENODEV;
> + return rc;
> +}
> +
> /*
> * tpm_chip_register() - create a character device for the TPM chip
> * @chip: TPM chip to use.
> @@ -573,6 +606,10 @@ int tpm_chip_register(struct tpm_chip *chip)
> if (rc)
> return rc;
Above this is tpm_chip_stop(chip) because (afaik) none of the following
function calls in tpm_chip_register() needed the TPM, but now with
tpm_pcr_allocation() you will need to send a command to the TPM. So I
would say you should move the tpm_chip_stop() into the error branch
visible above and also after the tpm_pcr_allocation().
> + rc = tpm_pcr_allocation(chip);
tpm_chip_stop(chip);
> + if (rc)
> + return rc;
> +
> tpm_sysfs_add_device(chip);
>
> rc = tpm_bios_log_setup(chip);
Stefan
^ permalink raw reply
* Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
From: Nayna @ 2019-07-05 15:32 UTC (permalink / raw)
To: Stefan Berger, Nayna Jain, linux-integrity, linuxppc-dev,
Jarkko Sakkinen
Cc: Sachin Sant, George Wilson, linux-kernel, Mimi Zohar,
Jason Gunthorpe, Peter Huewe, Michal Suchanek
In-Reply-To: <1998ebcf-1521-778f-2c80-55ad2c855023@linux.ibm.com>
On 07/05/2019 10:13 AM, Stefan Berger wrote:
> On 7/3/19 11:32 PM, Nayna Jain wrote:
>> The nr_allocated_banks and allocated banks are initialized as part of
>> tpm_chip_register. Currently, this is done as part of auto startup
>> function. However, some drivers, like the ibm vtpm driver, do not run
>> auto startup during initialization. This results in uninitialized memory
>> issue and causes a kernel panic during boot.
>>
>> This patch moves the pcr allocation outside the auto startup function
>> into tpm_chip_register. This ensures that allocated banks are
>> initialized
>> in any case.
>>
>> Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms
>> with
>> PCR read")
>> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>> ---
>> drivers/char/tpm/tpm-chip.c | 37 +++++++++++++++++++++++++++++++++++++
>> drivers/char/tpm/tpm.h | 1 +
>> drivers/char/tpm/tpm1-cmd.c | 12 ------------
>> drivers/char/tpm/tpm2-cmd.c | 6 +-----
>> 4 files changed, 39 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 8804c9e916fd..958508bb8379 100644
>> --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -550,6 +550,39 @@ static int tpm_add_hwrng(struct tpm_chip *chip)
>> return hwrng_register(&chip->hwrng);
>> }
>>
>> +/*
>> + * tpm_pcr_allocation() - initializes the chip allocated banks for PCRs
>> + */
>> +static int tpm_pcr_allocation(struct tpm_chip *chip)
>> +{
>> + int rc = 0;
>> +
>> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>> + rc = tpm2_get_pcr_allocation(chip);
>> + if (rc)
>> + goto out;
>> + }
>> +
>> + /* Initialize TPM 1.2 */
>> + chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
>> + GFP_KERNEL);
>> + if (!chip->allocated_banks) {
>> + rc = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1;
>> + chip->allocated_banks[0].digest_size =
>> hash_digest_size[HASH_ALGO_SHA1];
>> + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1;
>> + chip->nr_allocated_banks = 1;
>> +
>> + return 0;
>> +out:
>> + if (rc < 0)
>> + rc = -ENODEV;
>
>
> The old code where you lifted this from said:
>
> out:
> if (rc > 0)
> rc = -ENODEV;
> return rc;
>
> It would not overwrite -ENOMEM with -ENODEV but yours does.
>
> I think the correct fix would be to use:
>
> if (rc > 0)
>
> rc = -ENODEV;
>
Yes. I think I misread it. Thanks Stefan. Will fix this..
>
>
>
>
>> + return rc;
>> +}
>> +
>> /*
>> * tpm_chip_register() - create a character device for the TPM chip
>> * @chip: TPM chip to use.
>> @@ -573,6 +606,10 @@ int tpm_chip_register(struct tpm_chip *chip)
>> if (rc)
>> return rc;
>
> Above this is tpm_chip_stop(chip) because (afaik) none of the
> following function calls in tpm_chip_register() needed the TPM, but
> now with tpm_pcr_allocation() you will need to send a command to the
> TPM. So I would say you should move the tpm_chip_stop() into the error
> branch visible above and also after the tpm_pcr_allocation().
>
>
>> + rc = tpm_pcr_allocation(chip);
> tpm_chip_stop(chip);
I am not sure of the purpose of tpm_stop_chip(), so I have left it as it
is. Jarkko, what do you think about the change ?
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
From: Jarkko Sakkinen @ 2019-07-05 17:50 UTC (permalink / raw)
To: Nayna, Stefan Berger, Nayna Jain, linux-integrity, linuxppc-dev
Cc: Sachin Sant, George Wilson, linux-kernel, Mimi Zohar,
Jason Gunthorpe, Peter Huewe, Michal Suchanek
In-Reply-To: <164b9c6e-9b6d-324d-9df8-d2f7d1ac8cfc@linux.vnet.ibm.com>
On Fri, 2019-07-05 at 11:32 -0400, Nayna wrote:
> I am not sure of the purpose of tpm_stop_chip(), so I have left it as it
> is. Jarkko, what do you think about the change ?
Stefan right. Your does not work, or will randomly work or not work
depending on the chip.
You need to turn the TPM on with tpm_chip_start() and turn it off with
tpm_chip_stop() once you are done. This is done in tpm_chip_register()
before calling tpm_auto_startup().
TPM power management was once in tpm_transmit() but not anymore after my
patch set that removed nested tpm_transmit() calls.
While you're on it please take into account my earlier feedback.
Also, short summary could be "tpm: tpm_ibm_vtpm: Fix unallocated banks"
Some oddballs in your patch that I have to ask.
if (chip->flags & TPM_CHIP_FLAG_TPM2) {
rc = tpm2_get_pcr_allocation(chip);
if (rc)
goto out;
}
chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks),
GFP_KERNEL);
if (!chip->allocated_banks) {
rc = -ENOMEM;
goto out;
}
Why you don't return on site and instead jump somewhere? Also the
2nd line for kcalloc() is misaligned.
out:
if (rc < 0)
rc = -ENODEV;
This will cause a new regression i.e. you let TPM error codes
through.
To summarize this patch fixes one regression and introduces two
completely new ones...
/Jarkko`
^ permalink raw reply
* Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
From: Jarkko Sakkinen @ 2019-07-05 18:15 UTC (permalink / raw)
To: Nayna, Stefan Berger, Nayna Jain, linux-integrity, linuxppc-dev
Cc: Sachin Sant, George Wilson, linux-kernel, Mimi Zohar,
Jason Gunthorpe, Peter Huewe, Michal Suchanek
In-Reply-To: <1270cd6ab2ceae1ad01e4b83b75fc4c6fc70027d.camel@linux.intel.com>
On Fri, 2019-07-05 at 20:50 +0300, Jarkko Sakkinen wrote:
> To summarize this patch fixes one regression and introduces two
> completely new ones...
Anyway, take the time and update it. The principle is right
anyway. I'll merge it once it is in a better shape...
/Jarkko
^ permalink raw reply
* Re: [PATCH] i2c: remove casting dma_alloc
From: Wolfram Sang @ 2019-07-05 18:44 UTC (permalink / raw)
To: Vasyl Gomonovych; +Cc: linux-kernel, linuxppc-dev, linux-i2c
In-Reply-To: <20190623211354.24181-1-gomonovych@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 296 bytes --]
On Sun, Jun 23, 2019 at 11:13:53PM +0200, Vasyl Gomonovych wrote:
> From: Vasyl <gomonovych@gmail.com>
>
> Generated by: alloc_cast.cocci
>
> Signed-off-by: Vasyl <gomonovych@gmail.com>
Applied to for-next, thanks! I fixed the missing full name for you, but
please fix your setup.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [v3 1/7] powerpc/mce: Make machine_check_ue_event() static
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-1-santosh@fossix.org>
From: Reza Arbab <arbab@linux.ibm.com>
The function doesn't get used outside this file, so make it static.
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/kernel/mce.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index b18df633eae9..e78c4f18ea0a 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -33,7 +33,7 @@ static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT],
mce_ue_event_queue);
static void machine_check_process_queued_event(struct irq_work *work);
-void machine_check_ue_event(struct machine_check_event *evt);
+static void machine_check_ue_event(struct machine_check_event *evt);
static void machine_process_ue_event(struct work_struct *work);
static struct irq_work mce_event_process_work = {
@@ -203,7 +203,7 @@ void release_mce_event(void)
/*
* Queue up the MCE event which then can be handled later.
*/
-void machine_check_ue_event(struct machine_check_event *evt)
+static void machine_check_ue_event(struct machine_check_event *evt)
{
int index;
--
2.20.1
^ permalink raw reply related
* [v3 0/7] powerpc: implement machine check safe memcpy
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
During a memcpy from a pmem device, if a machine check exception is
generated we end up in a panic. In case of fsdax read, this should
only result in a -EIO. Avoid MCE by implementing memcpy_mcsafe.
Before this patch series:
```
bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/
[ 7621.714094] Disabling lock debugging due to kernel taint
[ 7621.714099] MCE: CPU0: machine check (Severe) Host UE Load/Store [Not recovered]
[ 7621.714104] MCE: CPU0: NIP: [c000000000088978] memcpy_power7+0x418/0x7e0
[ 7621.714107] MCE: CPU0: Hardware error
[ 7621.714112] opal: Hardware platform error: Unrecoverable Machine Check exception
[ 7621.714118] CPU: 0 PID: 1368 Comm: mount Tainted: G M 5.2.0-rc5-00239-g241e39004581 #50
[ 7621.714123] NIP: c000000000088978 LR: c0000000008e16f8 CTR: 00000000000001de
[ 7621.714129] REGS: c0000000fffbfd70 TRAP: 0200 Tainted: G M (5.2.0-rc5-00239-g241e39004581)
[ 7621.714131] MSR: 9000000002209033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 24428840 XER: 00040000
[ 7621.714160] CFAR: c0000000000889a8 DAR: deadbeefdeadbeef DSISR: 00008000 IRQMASK: 0
[ 7621.714171] GPR00: 000000000e000000 c0000000f0b8b1e0 c0000000012cf100 c0000000ed8e1100
[ 7621.714186] GPR04: c000020000001100 0000000000010000 0000000000000200 03fffffff1272000
[ 7621.714201] GPR08: 0000000080000000 0000000000000010 0000000000000020 0000000000000030
[ 7621.714216] GPR12: 0000000000000040 00007fffb8c6d390 0000000000000050 0000000000000060
[ 7621.714232] GPR16: 0000000000000070 0000000000000000 0000000000000001 c0000000f0b8b960
[ 7621.714247] GPR20: 0000000000000001 c0000000f0b8b940 0000000000000001 0000000000010000
[ 7621.714262] GPR24: c000000001382560 c00c0000003b6380 c00c0000003b6380 0000000000010000
[ 7621.714277] GPR28: 0000000000000000 0000000000010000 c000020000000000 0000000000010000
[ 7621.714294] NIP [c000000000088978] memcpy_power7+0x418/0x7e0
[ 7621.714298] LR [c0000000008e16f8] pmem_do_bvec+0xf8/0x430
... <snip> ...
```
After this patch series:
```
bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/
[25302.883978] Buffer I/O error on dev pmem0, logical block 0, async page read
[25303.020816] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25303.021236] EXT4-fs (pmem0): Can't read superblock on 2nd try
[25303.152515] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25303.284031] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[25304.084100] UDF-fs: bad mount option "dax" or missing value
mount: /mnt/pmem: wrong fs type, bad option, bad superblock on /dev/pmem0, missing codepage or helper program, or other error.
```
MCE is injected on a pmem address using mambo. The last patch which adds a nop
is only for testing on mambo, where r13 is not restored upon hitting vector 200.
The memcpy code can be optimised by adding VMX optimizations and GAS macros can
be used to enable code reusablity, which I will send as another series.
---
Change-log:
v3:
* Drop patch which enables DR/IR for external modules
* Drop notifier call chain, we don't want to do that in real mode
* Return remaining bytes from memcpy_mcsafe correctly
* We no longer restore r13 for simulator tests, rather use a nop at
vector 0x200 [workaround for simulator; not to be merged]
v2:
* Don't set RI bit explicitly [mahesh]
* Re-ordered series to get r13 workaround as the last patch
---
Balbir Singh (2):
powerpc/mce: Bug fixes for MCE handling in kernel space
powerpc/memcpy: Add memcpy_mcsafe for pmem
Reza Arbab (2):
powerpc/mce: Make machine_check_ue_event() static
powerpc/64s: save r13 in MCE handler (simulator workaroud)
Santosh Sivaraj (3):
powerpc/mce: Handle UE event for memcpy_mcsafe
powerpc/memcpy_mcsafe: return remaining bytes
powerpc: add machine check safe copy_to_user
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/mce.h | 7 +-
arch/powerpc/include/asm/string.h | 2 +
arch/powerpc/include/asm/uaccess.h | 12 ++
arch/powerpc/kernel/exceptions-64s.S | 1 +
arch/powerpc/kernel/mce.c | 11 +-
arch/powerpc/kernel/mce_power.c | 41 +++--
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/memcpy_mcsafe_64.S | 239 +++++++++++++++++++++++++++
arch/powerpc/platforms/pseries/ras.c | 6 +-
10 files changed, 302 insertions(+), 20 deletions(-)
create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
--
2.20.1
^ permalink raw reply
* [v3 2/7] powerpc/mce: Bug fixes for MCE handling in kernel space
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-1-santosh@fossix.org>
From: Balbir Singh <bsingharora@gmail.com>
The code currently assumes PAGE_SHIFT as the shift value of
the pfn, this works correctly (mostly) for user space pages,
but the correct thing to do is
1. Extract the shift value returned via the pte-walk API's
2. Use the shift value to access the instruction address.
Note, the final physical address still use PAGE_SHIFT for
computation. handle_ierror() is not modified and handle_derror()
is modified just for extracting the correct instruction
address.
This is largely due to __find_linux_pte() returning pfn's
shifted by pdshift. The code is much more generic and can
handle shift values returned.
Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
[arbab@linux.ibm.com: Fixup pseries_do_memory_failure()]
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/include/asm/mce.h | 3 ++-
arch/powerpc/kernel/mce_power.c | 26 ++++++++++++++++----------
arch/powerpc/platforms/pseries/ras.c | 6 ++++--
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index a4c6a74ad2fb..94888a7025b3 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -209,7 +209,8 @@ extern void release_mce_event(void);
extern void machine_check_queue_event(void);
extern void machine_check_print_event_info(struct machine_check_event *evt,
bool user_mode, bool in_guest);
-unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
+unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
+ unsigned int *shift);
#ifdef CONFIG_PPC_BOOK3S_64
void flush_and_reload_slb(void);
#endif /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index e39536aad30d..04666c0b40a8 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -23,7 +23,8 @@
* Convert an address related to an mm to a PFN. NOTE: we are in real
* mode, we could potentially race with page table updates.
*/
-unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
+unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
+ unsigned int *shift)
{
pte_t *ptep;
unsigned long flags;
@@ -36,13 +37,15 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
local_irq_save(flags);
if (mm == current->mm)
- ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
+ ptep = find_current_mm_pte(mm->pgd, addr, NULL, shift);
else
- ptep = find_init_mm_pte(addr, NULL);
+ ptep = find_init_mm_pte(addr, shift);
local_irq_restore(flags);
if (!ptep || pte_special(*ptep))
return ULONG_MAX;
- return pte_pfn(*ptep);
+ if (!*shift)
+ *shift = PAGE_SHIFT;
+ return (pte_val(*ptep) & PTE_RPN_MASK) >> *shift;
}
/* flush SLBs and reload */
@@ -358,15 +361,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr,
unsigned long pfn, instr_addr;
struct instruction_op op;
struct pt_regs tmp = *regs;
+ unsigned int shift;
- pfn = addr_to_pfn(regs, regs->nip);
+ pfn = addr_to_pfn(regs, regs->nip, &shift);
if (pfn != ULONG_MAX) {
- instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
+ instr_addr = (pfn << shift) + (regs->nip & ((1 << shift) - 1));
instr = *(unsigned int *)(instr_addr);
if (!analyse_instr(&op, &tmp, instr)) {
- pfn = addr_to_pfn(regs, op.ea);
+ pfn = addr_to_pfn(regs, op.ea, &shift);
*addr = op.ea;
- *phys_addr = (pfn << PAGE_SHIFT);
+ *phys_addr = (pfn << shift);
return 0;
}
/*
@@ -442,12 +446,14 @@ static int mce_handle_ierror(struct pt_regs *regs,
if (mce_err->sync_error &&
table[i].error_type == MCE_ERROR_TYPE_UE) {
unsigned long pfn;
+ unsigned int shift;
if (get_paca()->in_mce < MAX_MCE_DEPTH) {
- pfn = addr_to_pfn(regs, regs->nip);
+ pfn = addr_to_pfn(regs, regs->nip,
+ &shift);
if (pfn != ULONG_MAX) {
*phys_addr =
- (pfn << PAGE_SHIFT);
+ (pfn << shift);
}
}
}
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index f16fdd0f71f7..5e43283d3300 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -740,12 +740,14 @@ static void pseries_do_memory_failure(struct pt_regs *regs,
paddr = be64_to_cpu(mce_log->logical_address);
} else if (mce_log->sub_err_type & UE_EFFECTIVE_ADDR_PROVIDED) {
unsigned long pfn;
+ unsigned int shift;
pfn = addr_to_pfn(regs,
- be64_to_cpu(mce_log->effective_address));
+ be64_to_cpu(mce_log->effective_address),
+ &shift);
if (pfn == ULONG_MAX)
return;
- paddr = pfn << PAGE_SHIFT;
+ paddr = pfn << shift;
} else {
return;
}
--
2.20.1
^ permalink raw reply related
* [v3 3/7] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-1-santosh@fossix.org>
From: Balbir Singh <bsingharora@gmail.com>
The pmem infrastructure uses memcpy_mcsafe in the pmem
layer so as to convert machine check exceptions into
a return value on failure in case a machine check
exception is encountered during the memcpy.
This patch largely borrows from the copyuser_power7
logic and does not add the VMX optimizations, largely
to keep the patch simple. If needed those optimizations
can be folded in.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
[arbab@linux.ibm.com: Added symbol export]
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/include/asm/string.h | 2 +
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/memcpy_mcsafe_64.S | 215 ++++++++++++++++++++++++++++
3 files changed, 218 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 9bf6dffb4090..b72692702f35 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -53,7 +53,9 @@ void *__memmove(void *to, const void *from, __kernel_size_t n);
#ifndef CONFIG_KASAN
#define __HAVE_ARCH_MEMSET32
#define __HAVE_ARCH_MEMSET64
+#define __HAVE_ARCH_MEMCPY_MCSAFE
+extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index c55f9c27bf79..529d6536eb4a 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
memcpy_power7.o
obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
- memcpy_64.o pmem.o
+ memcpy_64.o pmem.o memcpy_mcsafe_64.o
obj64-$(CONFIG_SMP) += locks.o
obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
new file mode 100644
index 000000000000..50f865db0338
--- /dev/null
+++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
@@ -0,0 +1,215 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) IBM Corporation, 2011
+ * Derived from copyuser_power7.s by Anton Blanchard <anton@au.ibm.com>
+ * Author - Balbir Singh <bsingharora@gmail.com>
+ */
+#include <asm/ppc_asm.h>
+#include <asm/errno.h>
+#include <asm/export.h>
+
+ .macro err1
+100:
+ EX_TABLE(100b,.Ldo_err1)
+ .endm
+
+ .macro err2
+200:
+ EX_TABLE(200b,.Ldo_err2)
+ .endm
+
+.Ldo_err2:
+ ld r22,STK_REG(R22)(r1)
+ ld r21,STK_REG(R21)(r1)
+ ld r20,STK_REG(R20)(r1)
+ ld r19,STK_REG(R19)(r1)
+ ld r18,STK_REG(R18)(r1)
+ ld r17,STK_REG(R17)(r1)
+ ld r16,STK_REG(R16)(r1)
+ ld r15,STK_REG(R15)(r1)
+ ld r14,STK_REG(R14)(r1)
+ addi r1,r1,STACKFRAMESIZE
+.Ldo_err1:
+ li r3,-EFAULT
+ blr
+
+
+_GLOBAL(memcpy_mcsafe)
+ cmpldi r5,16
+ blt .Lshort_copy
+
+.Lcopy:
+ /* Get the source 8B aligned */
+ neg r6,r4
+ mtocrf 0x01,r6
+ clrldi r6,r6,(64-3)
+
+ bf cr7*4+3,1f
+err1; lbz r0,0(r4)
+ addi r4,r4,1
+err1; stb r0,0(r3)
+ addi r3,r3,1
+
+1: bf cr7*4+2,2f
+err1; lhz r0,0(r4)
+ addi r4,r4,2
+err1; sth r0,0(r3)
+ addi r3,r3,2
+
+2: bf cr7*4+1,3f
+err1; lwz r0,0(r4)
+ addi r4,r4,4
+err1; stw r0,0(r3)
+ addi r3,r3,4
+
+3: sub r5,r5,r6
+ cmpldi r5,128
+ blt 5f
+
+ mflr r0
+ stdu r1,-STACKFRAMESIZE(r1)
+ std r14,STK_REG(R14)(r1)
+ std r15,STK_REG(R15)(r1)
+ std r16,STK_REG(R16)(r1)
+ std r17,STK_REG(R17)(r1)
+ std r18,STK_REG(R18)(r1)
+ std r19,STK_REG(R19)(r1)
+ std r20,STK_REG(R20)(r1)
+ std r21,STK_REG(R21)(r1)
+ std r22,STK_REG(R22)(r1)
+ std r0,STACKFRAMESIZE+16(r1)
+
+ srdi r6,r5,7
+ mtctr r6
+
+ /* Now do cacheline (128B) sized loads and stores. */
+ .align 5
+4:
+err2; ld r0,0(r4)
+err2; ld r6,8(r4)
+err2; ld r7,16(r4)
+err2; ld r8,24(r4)
+err2; ld r9,32(r4)
+err2; ld r10,40(r4)
+err2; ld r11,48(r4)
+err2; ld r12,56(r4)
+err2; ld r14,64(r4)
+err2; ld r15,72(r4)
+err2; ld r16,80(r4)
+err2; ld r17,88(r4)
+err2; ld r18,96(r4)
+err2; ld r19,104(r4)
+err2; ld r20,112(r4)
+err2; ld r21,120(r4)
+ addi r4,r4,128
+err2; std r0,0(r3)
+err2; std r6,8(r3)
+err2; std r7,16(r3)
+err2; std r8,24(r3)
+err2; std r9,32(r3)
+err2; std r10,40(r3)
+err2; std r11,48(r3)
+err2; std r12,56(r3)
+err2; std r14,64(r3)
+err2; std r15,72(r3)
+err2; std r16,80(r3)
+err2; std r17,88(r3)
+err2; std r18,96(r3)
+err2; std r19,104(r3)
+err2; std r20,112(r3)
+err2; std r21,120(r3)
+ addi r3,r3,128
+ bdnz 4b
+
+ clrldi r5,r5,(64-7)
+
+ ld r14,STK_REG(R14)(r1)
+ ld r15,STK_REG(R15)(r1)
+ ld r16,STK_REG(R16)(r1)
+ ld r17,STK_REG(R17)(r1)
+ ld r18,STK_REG(R18)(r1)
+ ld r19,STK_REG(R19)(r1)
+ ld r20,STK_REG(R20)(r1)
+ ld r21,STK_REG(R21)(r1)
+ ld r22,STK_REG(R22)(r1)
+ addi r1,r1,STACKFRAMESIZE
+
+ /* Up to 127B to go */
+5: srdi r6,r5,4
+ mtocrf 0x01,r6
+
+6: bf cr7*4+1,7f
+err1; ld r0,0(r4)
+err1; ld r6,8(r4)
+err1; ld r7,16(r4)
+err1; ld r8,24(r4)
+err1; ld r9,32(r4)
+err1; ld r10,40(r4)
+err1; ld r11,48(r4)
+err1; ld r12,56(r4)
+ addi r4,r4,64
+err1; std r0,0(r3)
+err1; std r6,8(r3)
+err1; std r7,16(r3)
+err1; std r8,24(r3)
+err1; std r9,32(r3)
+err1; std r10,40(r3)
+err1; std r11,48(r3)
+err1; std r12,56(r3)
+ addi r3,r3,64
+
+ /* Up to 63B to go */
+7: bf cr7*4+2,8f
+err1; ld r0,0(r4)
+err1; ld r6,8(r4)
+err1; ld r7,16(r4)
+err1; ld r8,24(r4)
+ addi r4,r4,32
+err1; std r0,0(r3)
+err1; std r6,8(r3)
+err1; std r7,16(r3)
+err1; std r8,24(r3)
+ addi r3,r3,32
+
+ /* Up to 31B to go */
+8: bf cr7*4+3,9f
+err1; ld r0,0(r4)
+err1; ld r6,8(r4)
+ addi r4,r4,16
+err1; std r0,0(r3)
+err1; std r6,8(r3)
+ addi r3,r3,16
+
+9: clrldi r5,r5,(64-4)
+
+ /* Up to 15B to go */
+.Lshort_copy:
+ mtocrf 0x01,r5
+ bf cr7*4+0,12f
+err1; lwz r0,0(r4) /* Less chance of a reject with word ops */
+err1; lwz r6,4(r4)
+ addi r4,r4,8
+err1; stw r0,0(r3)
+err1; stw r6,4(r3)
+ addi r3,r3,8
+
+12: bf cr7*4+1,13f
+err1; lwz r0,0(r4)
+ addi r4,r4,4
+err1; stw r0,0(r3)
+ addi r3,r3,4
+
+13: bf cr7*4+2,14f
+err1; lhz r0,0(r4)
+ addi r4,r4,2
+err1; sth r0,0(r3)
+ addi r3,r3,2
+
+14: bf cr7*4+3,15f
+err1; lbz r0,0(r4)
+err1; stb r0,0(r3)
+
+15: li r3,0
+ blr
+
+EXPORT_SYMBOL_GPL(memcpy_mcsafe);
--
2.20.1
^ permalink raw reply related
* [v3 4/7] powerpc/mce: Handle UE event for memcpy_mcsafe
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-1-santosh@fossix.org>
If we take a UE on one of the instructions with a fixup entry, set nip
to continue exucution at the fixup entry. Stop processing the event
further or print it.
Based-on-patch-by: Reza Arbab <arbab@linux.ibm.com>
Cc: Reza Arbab <arbab@linux.ibm.com>
Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/include/asm/mce.h | 4 +++-
arch/powerpc/kernel/mce.c | 7 ++++++-
arch/powerpc/kernel/mce_power.c | 15 +++++++++++++--
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 94888a7025b3..f74257eb013b 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -122,7 +122,8 @@ struct machine_check_event {
enum MCE_UeErrorType ue_error_type:8;
u8 effective_address_provided;
u8 physical_address_provided;
- u8 reserved_1[5];
+ u8 ignore_event;
+ u8 reserved_1[4];
u64 effective_address;
u64 physical_address;
u8 reserved_2[8];
@@ -193,6 +194,7 @@ struct mce_error_info {
enum MCE_Initiator initiator:8;
enum MCE_ErrorClass error_class:8;
bool sync_error;
+ bool ignore_event;
};
#define MAX_MC_EVT 100
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index e78c4f18ea0a..94f2bb307537 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -144,7 +144,9 @@ void save_mce_event(struct pt_regs *regs, long handled,
if (phys_addr != ULONG_MAX) {
mce->u.ue_error.physical_address_provided = true;
mce->u.ue_error.physical_address = phys_addr;
- machine_check_ue_event(mce);
+ mce->u.ue_error.ignore_event = mce_err->ignore_event;
+ if (!mce->u.ue_error.ignore_event)
+ machine_check_ue_event(mce);
}
}
return;
@@ -230,6 +232,9 @@ void machine_check_queue_event(void)
if (!get_mce_event(&evt, MCE_EVENT_RELEASE))
return;
+ if (evt.error_type == MCE_ERROR_TYPE_UE && evt.u.ue_error.ignore_event)
+ return;
+
index = __this_cpu_inc_return(mce_queue_count) - 1;
/* If queue is full, just return for now. */
if (index >= MAX_MC_EVT) {
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 04666c0b40a8..db4aa98a23ac 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -11,6 +11,7 @@
#include <linux/types.h>
#include <linux/ptrace.h>
+#include <linux/extable.h>
#include <asm/mmu.h>
#include <asm/mce.h>
#include <asm/machdep.h>
@@ -18,6 +19,7 @@
#include <asm/pte-walk.h>
#include <asm/sstep.h>
#include <asm/exception-64s.h>
+#include <asm/extable.h>
/*
* Convert an address related to an mm to a PFN. NOTE: we are in real
@@ -565,9 +567,18 @@ static int mce_handle_derror(struct pt_regs *regs,
return 0;
}
-static long mce_handle_ue_error(struct pt_regs *regs)
+static long mce_handle_ue_error(struct pt_regs *regs,
+ struct mce_error_info *mce_err)
{
long handled = 0;
+ const struct exception_table_entry *entry;
+
+ entry = search_exception_tables(regs->nip);
+ if (entry) {
+ mce_err->ignore_event = true;
+ regs->nip = extable_fixup(entry);
+ return 1;
+ }
/*
* On specific SCOM read via MMIO we may get a machine check
@@ -600,7 +611,7 @@ static long mce_handle_error(struct pt_regs *regs,
&phys_addr);
if (!handled && mce_err.error_type == MCE_ERROR_TYPE_UE)
- handled = mce_handle_ue_error(regs);
+ handled = mce_handle_ue_error(regs, &mce_err);
save_mce_event(regs, handled, &mce_err, regs->nip, addr, phys_addr);
--
2.20.1
^ permalink raw reply related
* [v3 5/7] powerpc/memcpy_mcsafe: return remaining bytes
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-1-santosh@fossix.org>
memcpy_mcsafe currently return -EFAULT on a machine check exception, change
it to return the remaining bytes that needs to be copied, so that machine
check safe copy_to_user can maintain the same behavior as copy_to_user.
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/lib/memcpy_mcsafe_64.S | 142 ++++++++++++++++------------
1 file changed, 83 insertions(+), 59 deletions(-)
diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
index 50f865db0338..4d8a3d315992 100644
--- a/arch/powerpc/lib/memcpy_mcsafe_64.S
+++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
@@ -30,11 +30,25 @@
ld r14,STK_REG(R14)(r1)
addi r1,r1,STACKFRAMESIZE
.Ldo_err1:
- li r3,-EFAULT
+ /* Do a byte by byte copy to get the exact remaining size */
+ mtctr r7
+100: EX_TABLE(100b, .Ldone)
+46:
+err1; lbz r0,0(r4)
+ addi r4,r4,1
+err1; stb r0,0(r3)
+ addi r3,r3,1
+ bdnz 46b
+ li r3,0
+ blr
+
+.Ldone:
+ mfctr r3
blr
_GLOBAL(memcpy_mcsafe)
+ mr r7,r5
cmpldi r5,16
blt .Lshort_copy
@@ -49,18 +63,21 @@ err1; lbz r0,0(r4)
addi r4,r4,1
err1; stb r0,0(r3)
addi r3,r3,1
+ subi r7,r7,1
1: bf cr7*4+2,2f
err1; lhz r0,0(r4)
addi r4,r4,2
err1; sth r0,0(r3)
addi r3,r3,2
+ subi r7,r7,2
2: bf cr7*4+1,3f
err1; lwz r0,0(r4)
addi r4,r4,4
err1; stw r0,0(r3)
addi r3,r3,4
+ subi r7,r7,4
3: sub r5,r5,r6
cmpldi r5,128
@@ -87,43 +104,69 @@ err1; stw r0,0(r3)
4:
err2; ld r0,0(r4)
err2; ld r6,8(r4)
-err2; ld r7,16(r4)
-err2; ld r8,24(r4)
-err2; ld r9,32(r4)
-err2; ld r10,40(r4)
-err2; ld r11,48(r4)
-err2; ld r12,56(r4)
-err2; ld r14,64(r4)
-err2; ld r15,72(r4)
-err2; ld r16,80(r4)
-err2; ld r17,88(r4)
-err2; ld r18,96(r4)
-err2; ld r19,104(r4)
-err2; ld r20,112(r4)
-err2; ld r21,120(r4)
+err2; ld r8,16(r4)
+err2; ld r9,24(r4)
+err2; ld r10,32(r4)
+err2; ld r11,40(r4)
+err2; ld r12,48(r4)
+err2; ld r14,56(r4)
+err2; ld r15,64(r4)
+err2; ld r16,72(r4)
+err2; ld r17,80(r4)
+err2; ld r18,88(r4)
+err2; ld r19,96(r4)
+err2; ld r20,104(r4)
+err2; ld r21,112(r4)
+err2; ld r22,120(r4)
addi r4,r4,128
err2; std r0,0(r3)
err2; std r6,8(r3)
-err2; std r7,16(r3)
-err2; std r8,24(r3)
-err2; std r9,32(r3)
-err2; std r10,40(r3)
-err2; std r11,48(r3)
-err2; std r12,56(r3)
-err2; std r14,64(r3)
-err2; std r15,72(r3)
-err2; std r16,80(r3)
-err2; std r17,88(r3)
-err2; std r18,96(r3)
-err2; std r19,104(r3)
-err2; std r20,112(r3)
-err2; std r21,120(r3)
+err2; std r8,16(r3)
+err2; std r9,24(r3)
+err2; std r10,32(r3)
+err2; std r11,40(r3)
+err2; std r12,48(r3)
+err2; std r14,56(r3)
+err2; std r15,64(r3)
+err2; std r16,72(r3)
+err2; std r17,80(r3)
+err2; std r18,88(r3)
+err2; std r19,96(r3)
+err2; std r20,104(r3)
+err2; std r21,112(r3)
+err2; std r22,120(r3)
addi r3,r3,128
+ subi r7,r7,128
bdnz 4b
clrldi r5,r5,(64-7)
- ld r14,STK_REG(R14)(r1)
+ /* Up to 127B to go */
+5: srdi r6,r5,4
+ mtocrf 0x01,r6
+
+6: bf cr7*4+1,7f
+err2; ld r0,0(r4)
+err2; ld r6,8(r4)
+err2; ld r8,16(r4)
+err2; ld r9,24(r4)
+err2; ld r10,32(r4)
+err2; ld r11,40(r4)
+err2; ld r12,48(r4)
+err2; ld r14,56(r4)
+ addi r4,r4,64
+err2; std r0,0(r3)
+err2; std r6,8(r3)
+err2; std r8,16(r3)
+err2; std r9,24(r3)
+err2; std r10,32(r3)
+err2; std r11,40(r3)
+err2; std r12,48(r3)
+err2; std r14,56(r3)
+ addi r3,r3,64
+ subi r7,r7,64
+
+7: ld r14,STK_REG(R14)(r1)
ld r15,STK_REG(R15)(r1)
ld r16,STK_REG(R16)(r1)
ld r17,STK_REG(R17)(r1)
@@ -134,42 +177,19 @@ err2; std r21,120(r3)
ld r22,STK_REG(R22)(r1)
addi r1,r1,STACKFRAMESIZE
- /* Up to 127B to go */
-5: srdi r6,r5,4
- mtocrf 0x01,r6
-
-6: bf cr7*4+1,7f
-err1; ld r0,0(r4)
-err1; ld r6,8(r4)
-err1; ld r7,16(r4)
-err1; ld r8,24(r4)
-err1; ld r9,32(r4)
-err1; ld r10,40(r4)
-err1; ld r11,48(r4)
-err1; ld r12,56(r4)
- addi r4,r4,64
-err1; std r0,0(r3)
-err1; std r6,8(r3)
-err1; std r7,16(r3)
-err1; std r8,24(r3)
-err1; std r9,32(r3)
-err1; std r10,40(r3)
-err1; std r11,48(r3)
-err1; std r12,56(r3)
- addi r3,r3,64
-
/* Up to 63B to go */
-7: bf cr7*4+2,8f
+ bf cr7*4+2,8f
err1; ld r0,0(r4)
err1; ld r6,8(r4)
-err1; ld r7,16(r4)
-err1; ld r8,24(r4)
+err1; ld r8,16(r4)
+err1; ld r9,24(r4)
addi r4,r4,32
err1; std r0,0(r3)
err1; std r6,8(r3)
-err1; std r7,16(r3)
-err1; std r8,24(r3)
+err1; std r8,16(r3)
+err1; std r9,24(r3)
addi r3,r3,32
+ subi r7,r7,32
/* Up to 31B to go */
8: bf cr7*4+3,9f
@@ -179,6 +199,7 @@ err1; ld r6,8(r4)
err1; std r0,0(r3)
err1; std r6,8(r3)
addi r3,r3,16
+ subi r7,r7,16
9: clrldi r5,r5,(64-4)
@@ -192,18 +213,21 @@ err1; lwz r6,4(r4)
err1; stw r0,0(r3)
err1; stw r6,4(r3)
addi r3,r3,8
+ subi r7,r7,8
12: bf cr7*4+1,13f
err1; lwz r0,0(r4)
addi r4,r4,4
err1; stw r0,0(r3)
addi r3,r3,4
+ subi r7,r7,4
13: bf cr7*4+2,14f
err1; lhz r0,0(r4)
addi r4,r4,2
err1; sth r0,0(r3)
addi r3,r3,2
+ subi r7,r7,2
14: bf cr7*4+3,15f
err1; lbz r0,0(r4)
--
2.20.1
^ permalink raw reply related
* [v3 6/7] powerpc: add machine check safe copy_to_user
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-1-santosh@fossix.org>
Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe()
Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/uaccess.h | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8c1c636308c8..a173b392c272 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -134,6 +134,7 @@ config PPC
select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
+ select ARCH_HAS_UACCESS_MCSAFE if PPC64
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARCH_HAS_ZONE_DEVICE if PPC_BOOK3S_64
select ARCH_HAVE_NMI_SAFE_CMPXCHG
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 76f34346b642..f8fcaab4c5bc 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -386,6 +386,18 @@ static inline unsigned long raw_copy_to_user(void __user *to,
return ret;
}
+static __always_inline unsigned long __must_check
+copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n)
+{
+ if (likely(check_copy_size(from, n, true))) {
+ allow_write_to_user(to, n);
+ n = memcpy_mcsafe(to, from, n);
+ prevent_write_to_user(to, n);
+ }
+
+ return n;
+}
+
extern unsigned long __clear_user(void __user *addr, unsigned long size);
static inline unsigned long clear_user(void __user *addr, unsigned long size)
--
2.20.1
^ permalink raw reply related
* [v3 7/7] powerpc/64s: save r13 in MCE handler (simulator workaroud)
From: Santosh Sivaraj @ 2019-07-05 21:26 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-1-santosh@fossix.org>
From: Reza Arbab <arbab@linux.ibm.com>
Testing my memcpy_mcsafe() work in progress with an injected UE, I get
an error like this immediately after the function returns:
BUG: Unable to handle kernel data access at 0x7fff84dec8f8
Faulting instruction address: 0xc0080000009c00b0
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
Modules linked in: mce(O+) vmx_crypto crc32c_vpmsum
CPU: 0 PID: 1375 Comm: modprobe Tainted: G O 5.1.0-rc6 #267
NIP: c0080000009c00b0 LR: c0080000009c00a8 CTR: c000000000095f90
REGS: c0000000ee197790 TRAP: 0300 Tainted: G O (5.1.0-rc6)
MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 88002826 XER: 00040000
CFAR: c000000000095f8c DAR: 00007fff84dec8f8 DSISR: 40000000 IRQMASK: 0
GPR00: 000000006c6c6568 c0000000ee197a20 c0080000009c8400 fffffffffffffff2
GPR04: c0080000009c02e0 0000000000000006 0000000000000000 c000000003c834c8
GPR08: 0080000000000000 776a6681b7fb5100 0000000000000000 c0080000009c01c8
GPR12: c000000000095f90 00007fff84debc00 000000004d071440 0000000000000000
GPR16: 0000000100000601 c0080000009e0000 c000000000c98dd8 c000000000c98d98
GPR20: c000000003bba970 c0080000009c04d0 c0080000009c0618 c0000000001e5820
GPR24: 0000000000000000 0000000000000100 0000000000000001 c000000003bba958
GPR28: c0080000009c02e8 c0080000009c0318 c0080000009c02e0 0000000000000000
NIP [c0080000009c00b0] cause_ue+0xa8/0xe8 [mce]
LR [c0080000009c00a8] cause_ue+0xa0/0xe8 [mce]
After debugging we see that the first instruction at vector 200 is skipped by
the simulator, due to which r13 is not saved. Adding a nop at 0x200 fixes the
issue.
(This commit is needed for testing this series. This should not be taken
into the tree)
---
arch/powerpc/kernel/exceptions-64s.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 73ba246ca11d..8e43abb2a744 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -255,6 +255,7 @@ EXC_REAL_BEGIN(machine_check, 0x200, 0x100)
* some code path might still want to branch into the original
* vector
*/
+ nop
SET_SCRATCH0(r13) /* save r13 */
EXCEPTION_PROLOG_0(PACA_EXMC)
BEGIN_FTR_SECTION
--
2.20.1
^ permalink raw reply related
* [Bug 203839] Kernel 5.2-rc3 fails to boot on a PowerMac G4 3,6: systemd[1]: Failed to bump fs.file-max, ignoring: invalid argument
From: bugzilla-daemon @ 2019-07-06 0:51 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-203839-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=203839
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |CODE_FIX
--- Comment #10 from Erhard F. (erhard_f@mailbox.org) ---
The fix meanwhile found it's way in kernel 5.2-rc7 which boots just fine.
Thanks!
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [v3 1/7] powerpc/mce: Make machine_check_ue_event() static
From: Nicholas Piggin @ 2019-07-06 5:18 UTC (permalink / raw)
To: linuxppc-dev, Santosh Sivaraj
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-2-santosh@fossix.org>
Santosh Sivaraj's on July 6, 2019 7:26 am:
> From: Reza Arbab <arbab@linux.ibm.com>
>
> The function doesn't get used outside this file, so make it static.
>
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply
* Re: [v3 3/7] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Christophe Leroy @ 2019-07-06 6:12 UTC (permalink / raw)
To: Santosh Sivaraj, linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-4-santosh@fossix.org>
Le 05/07/2019 à 23:26, Santosh Sivaraj a écrit :
> From: Balbir Singh <bsingharora@gmail.com>
>
> The pmem infrastructure uses memcpy_mcsafe in the pmem
> layer so as to convert machine check exceptions into
> a return value on failure in case a machine check
> exception is encountered during the memcpy.
>
> This patch largely borrows from the copyuser_power7
> logic and does not add the VMX optimizations, largely
> to keep the patch simple. If needed those optimizations
> can be folded in.
As this patch largely borrows from copyuser_power7(), could we use gas
macro in order to share the source code between the two functions and
limit code source duplication ?
Christophe
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> Acked-by: Nicholas Piggin <npiggin@gmail.com>
> [arbab@linux.ibm.com: Added symbol export]
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
> arch/powerpc/include/asm/string.h | 2 +
> arch/powerpc/lib/Makefile | 2 +-
> arch/powerpc/lib/memcpy_mcsafe_64.S | 215 ++++++++++++++++++++++++++++
> 3 files changed, 218 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S
>
> diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
> index 9bf6dffb4090..b72692702f35 100644
> --- a/arch/powerpc/include/asm/string.h
> +++ b/arch/powerpc/include/asm/string.h
> @@ -53,7 +53,9 @@ void *__memmove(void *to, const void *from, __kernel_size_t n);
> #ifndef CONFIG_KASAN
> #define __HAVE_ARCH_MEMSET32
> #define __HAVE_ARCH_MEMSET64
> +#define __HAVE_ARCH_MEMCPY_MCSAFE
>
> +extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
> extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
> extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
> extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index c55f9c27bf79..529d6536eb4a 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -39,7 +39,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
> memcpy_power7.o
>
> obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
> - memcpy_64.o pmem.o
> + memcpy_64.o pmem.o memcpy_mcsafe_64.o
>
> obj64-$(CONFIG_SMP) += locks.o
> obj64-$(CONFIG_ALTIVEC) += vmx-helper.o
> diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
> new file mode 100644
> index 000000000000..50f865db0338
> --- /dev/null
> +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
> @@ -0,0 +1,215 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) IBM Corporation, 2011
> + * Derived from copyuser_power7.s by Anton Blanchard <anton@au.ibm.com>
> + * Author - Balbir Singh <bsingharora@gmail.com>
> + */
> +#include <asm/ppc_asm.h>
> +#include <asm/errno.h>
> +#include <asm/export.h>
> +
> + .macro err1
> +100:
> + EX_TABLE(100b,.Ldo_err1)
> + .endm
> +
> + .macro err2
> +200:
> + EX_TABLE(200b,.Ldo_err2)
> + .endm
> +
> +.Ldo_err2:
> + ld r22,STK_REG(R22)(r1)
> + ld r21,STK_REG(R21)(r1)
> + ld r20,STK_REG(R20)(r1)
> + ld r19,STK_REG(R19)(r1)
> + ld r18,STK_REG(R18)(r1)
> + ld r17,STK_REG(R17)(r1)
> + ld r16,STK_REG(R16)(r1)
> + ld r15,STK_REG(R15)(r1)
> + ld r14,STK_REG(R14)(r1)
> + addi r1,r1,STACKFRAMESIZE
> +.Ldo_err1:
> + li r3,-EFAULT
> + blr
> +
> +
> +_GLOBAL(memcpy_mcsafe)
> + cmpldi r5,16
> + blt .Lshort_copy
> +
> +.Lcopy:
> + /* Get the source 8B aligned */
> + neg r6,r4
> + mtocrf 0x01,r6
> + clrldi r6,r6,(64-3)
> +
> + bf cr7*4+3,1f
> +err1; lbz r0,0(r4)
> + addi r4,r4,1
> +err1; stb r0,0(r3)
> + addi r3,r3,1
> +
> +1: bf cr7*4+2,2f
> +err1; lhz r0,0(r4)
> + addi r4,r4,2
> +err1; sth r0,0(r3)
> + addi r3,r3,2
> +
> +2: bf cr7*4+1,3f
> +err1; lwz r0,0(r4)
> + addi r4,r4,4
> +err1; stw r0,0(r3)
> + addi r3,r3,4
> +
> +3: sub r5,r5,r6
> + cmpldi r5,128
> + blt 5f
> +
> + mflr r0
> + stdu r1,-STACKFRAMESIZE(r1)
> + std r14,STK_REG(R14)(r1)
> + std r15,STK_REG(R15)(r1)
> + std r16,STK_REG(R16)(r1)
> + std r17,STK_REG(R17)(r1)
> + std r18,STK_REG(R18)(r1)
> + std r19,STK_REG(R19)(r1)
> + std r20,STK_REG(R20)(r1)
> + std r21,STK_REG(R21)(r1)
> + std r22,STK_REG(R22)(r1)
> + std r0,STACKFRAMESIZE+16(r1)
> +
> + srdi r6,r5,7
> + mtctr r6
> +
> + /* Now do cacheline (128B) sized loads and stores. */
> + .align 5
> +4:
> +err2; ld r0,0(r4)
> +err2; ld r6,8(r4)
> +err2; ld r7,16(r4)
> +err2; ld r8,24(r4)
> +err2; ld r9,32(r4)
> +err2; ld r10,40(r4)
> +err2; ld r11,48(r4)
> +err2; ld r12,56(r4)
> +err2; ld r14,64(r4)
> +err2; ld r15,72(r4)
> +err2; ld r16,80(r4)
> +err2; ld r17,88(r4)
> +err2; ld r18,96(r4)
> +err2; ld r19,104(r4)
> +err2; ld r20,112(r4)
> +err2; ld r21,120(r4)
> + addi r4,r4,128
> +err2; std r0,0(r3)
> +err2; std r6,8(r3)
> +err2; std r7,16(r3)
> +err2; std r8,24(r3)
> +err2; std r9,32(r3)
> +err2; std r10,40(r3)
> +err2; std r11,48(r3)
> +err2; std r12,56(r3)
> +err2; std r14,64(r3)
> +err2; std r15,72(r3)
> +err2; std r16,80(r3)
> +err2; std r17,88(r3)
> +err2; std r18,96(r3)
> +err2; std r19,104(r3)
> +err2; std r20,112(r3)
> +err2; std r21,120(r3)
> + addi r3,r3,128
> + bdnz 4b
> +
> + clrldi r5,r5,(64-7)
> +
> + ld r14,STK_REG(R14)(r1)
> + ld r15,STK_REG(R15)(r1)
> + ld r16,STK_REG(R16)(r1)
> + ld r17,STK_REG(R17)(r1)
> + ld r18,STK_REG(R18)(r1)
> + ld r19,STK_REG(R19)(r1)
> + ld r20,STK_REG(R20)(r1)
> + ld r21,STK_REG(R21)(r1)
> + ld r22,STK_REG(R22)(r1)
> + addi r1,r1,STACKFRAMESIZE
> +
> + /* Up to 127B to go */
> +5: srdi r6,r5,4
> + mtocrf 0x01,r6
> +
> +6: bf cr7*4+1,7f
> +err1; ld r0,0(r4)
> +err1; ld r6,8(r4)
> +err1; ld r7,16(r4)
> +err1; ld r8,24(r4)
> +err1; ld r9,32(r4)
> +err1; ld r10,40(r4)
> +err1; ld r11,48(r4)
> +err1; ld r12,56(r4)
> + addi r4,r4,64
> +err1; std r0,0(r3)
> +err1; std r6,8(r3)
> +err1; std r7,16(r3)
> +err1; std r8,24(r3)
> +err1; std r9,32(r3)
> +err1; std r10,40(r3)
> +err1; std r11,48(r3)
> +err1; std r12,56(r3)
> + addi r3,r3,64
> +
> + /* Up to 63B to go */
> +7: bf cr7*4+2,8f
> +err1; ld r0,0(r4)
> +err1; ld r6,8(r4)
> +err1; ld r7,16(r4)
> +err1; ld r8,24(r4)
> + addi r4,r4,32
> +err1; std r0,0(r3)
> +err1; std r6,8(r3)
> +err1; std r7,16(r3)
> +err1; std r8,24(r3)
> + addi r3,r3,32
> +
> + /* Up to 31B to go */
> +8: bf cr7*4+3,9f
> +err1; ld r0,0(r4)
> +err1; ld r6,8(r4)
> + addi r4,r4,16
> +err1; std r0,0(r3)
> +err1; std r6,8(r3)
> + addi r3,r3,16
> +
> +9: clrldi r5,r5,(64-4)
> +
> + /* Up to 15B to go */
> +.Lshort_copy:
> + mtocrf 0x01,r5
> + bf cr7*4+0,12f
> +err1; lwz r0,0(r4) /* Less chance of a reject with word ops */
> +err1; lwz r6,4(r4)
> + addi r4,r4,8
> +err1; stw r0,0(r3)
> +err1; stw r6,4(r3)
> + addi r3,r3,8
> +
> +12: bf cr7*4+1,13f
> +err1; lwz r0,0(r4)
> + addi r4,r4,4
> +err1; stw r0,0(r3)
> + addi r3,r3,4
> +
> +13: bf cr7*4+2,14f
> +err1; lhz r0,0(r4)
> + addi r4,r4,2
> +err1; sth r0,0(r3)
> + addi r3,r3,2
> +
> +14: bf cr7*4+3,15f
> +err1; lbz r0,0(r4)
> +err1; stb r0,0(r3)
> +
> +15: li r3,0
> + blr
> +
> +EXPORT_SYMBOL_GPL(memcpy_mcsafe);
>
^ permalink raw reply
* Re: [v3 5/7] powerpc/memcpy_mcsafe: return remaining bytes
From: Christophe Leroy @ 2019-07-06 6:16 UTC (permalink / raw)
To: Santosh Sivaraj, linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-6-santosh@fossix.org>
Le 05/07/2019 à 23:26, Santosh Sivaraj a écrit :
> memcpy_mcsafe currently return -EFAULT on a machine check exception, change
> it to return the remaining bytes that needs to be copied, so that machine
> check safe copy_to_user can maintain the same behavior as copy_to_user.
AFAIU, this behaviour is the expected behaviour for memcpy_mcsafe(). Why
implement a different behaviour in patch 3 and then change it here.
Can't memcpy_mcsafe() return remaining bytes as expected from patch 3 ?
Christophe
>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
> arch/powerpc/lib/memcpy_mcsafe_64.S | 142 ++++++++++++++++------------
> 1 file changed, 83 insertions(+), 59 deletions(-)
>
> diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S
> index 50f865db0338..4d8a3d315992 100644
> --- a/arch/powerpc/lib/memcpy_mcsafe_64.S
> +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S
> @@ -30,11 +30,25 @@
> ld r14,STK_REG(R14)(r1)
> addi r1,r1,STACKFRAMESIZE
> .Ldo_err1:
> - li r3,-EFAULT
> + /* Do a byte by byte copy to get the exact remaining size */
> + mtctr r7
> +100: EX_TABLE(100b, .Ldone)
> +46:
> +err1; lbz r0,0(r4)
> + addi r4,r4,1
> +err1; stb r0,0(r3)
> + addi r3,r3,1
> + bdnz 46b
> + li r3,0
> + blr
> +
> +.Ldone:
> + mfctr r3
> blr
>
>
> _GLOBAL(memcpy_mcsafe)
> + mr r7,r5
> cmpldi r5,16
> blt .Lshort_copy
>
> @@ -49,18 +63,21 @@ err1; lbz r0,0(r4)
> addi r4,r4,1
> err1; stb r0,0(r3)
> addi r3,r3,1
> + subi r7,r7,1
>
> 1: bf cr7*4+2,2f
> err1; lhz r0,0(r4)
> addi r4,r4,2
> err1; sth r0,0(r3)
> addi r3,r3,2
> + subi r7,r7,2
>
> 2: bf cr7*4+1,3f
> err1; lwz r0,0(r4)
> addi r4,r4,4
> err1; stw r0,0(r3)
> addi r3,r3,4
> + subi r7,r7,4
>
> 3: sub r5,r5,r6
> cmpldi r5,128
> @@ -87,43 +104,69 @@ err1; stw r0,0(r3)
> 4:
> err2; ld r0,0(r4)
> err2; ld r6,8(r4)
> -err2; ld r7,16(r4)
> -err2; ld r8,24(r4)
> -err2; ld r9,32(r4)
> -err2; ld r10,40(r4)
> -err2; ld r11,48(r4)
> -err2; ld r12,56(r4)
> -err2; ld r14,64(r4)
> -err2; ld r15,72(r4)
> -err2; ld r16,80(r4)
> -err2; ld r17,88(r4)
> -err2; ld r18,96(r4)
> -err2; ld r19,104(r4)
> -err2; ld r20,112(r4)
> -err2; ld r21,120(r4)
> +err2; ld r8,16(r4)
> +err2; ld r9,24(r4)
> +err2; ld r10,32(r4)
> +err2; ld r11,40(r4)
> +err2; ld r12,48(r4)
> +err2; ld r14,56(r4)
> +err2; ld r15,64(r4)
> +err2; ld r16,72(r4)
> +err2; ld r17,80(r4)
> +err2; ld r18,88(r4)
> +err2; ld r19,96(r4)
> +err2; ld r20,104(r4)
> +err2; ld r21,112(r4)
> +err2; ld r22,120(r4)
> addi r4,r4,128
> err2; std r0,0(r3)
> err2; std r6,8(r3)
> -err2; std r7,16(r3)
> -err2; std r8,24(r3)
> -err2; std r9,32(r3)
> -err2; std r10,40(r3)
> -err2; std r11,48(r3)
> -err2; std r12,56(r3)
> -err2; std r14,64(r3)
> -err2; std r15,72(r3)
> -err2; std r16,80(r3)
> -err2; std r17,88(r3)
> -err2; std r18,96(r3)
> -err2; std r19,104(r3)
> -err2; std r20,112(r3)
> -err2; std r21,120(r3)
> +err2; std r8,16(r3)
> +err2; std r9,24(r3)
> +err2; std r10,32(r3)
> +err2; std r11,40(r3)
> +err2; std r12,48(r3)
> +err2; std r14,56(r3)
> +err2; std r15,64(r3)
> +err2; std r16,72(r3)
> +err2; std r17,80(r3)
> +err2; std r18,88(r3)
> +err2; std r19,96(r3)
> +err2; std r20,104(r3)
> +err2; std r21,112(r3)
> +err2; std r22,120(r3)
> addi r3,r3,128
> + subi r7,r7,128
> bdnz 4b
>
> clrldi r5,r5,(64-7)
>
> - ld r14,STK_REG(R14)(r1)
> + /* Up to 127B to go */
> +5: srdi r6,r5,4
> + mtocrf 0x01,r6
> +
> +6: bf cr7*4+1,7f
> +err2; ld r0,0(r4)
> +err2; ld r6,8(r4)
> +err2; ld r8,16(r4)
> +err2; ld r9,24(r4)
> +err2; ld r10,32(r4)
> +err2; ld r11,40(r4)
> +err2; ld r12,48(r4)
> +err2; ld r14,56(r4)
> + addi r4,r4,64
> +err2; std r0,0(r3)
> +err2; std r6,8(r3)
> +err2; std r8,16(r3)
> +err2; std r9,24(r3)
> +err2; std r10,32(r3)
> +err2; std r11,40(r3)
> +err2; std r12,48(r3)
> +err2; std r14,56(r3)
> + addi r3,r3,64
> + subi r7,r7,64
> +
> +7: ld r14,STK_REG(R14)(r1)
> ld r15,STK_REG(R15)(r1)
> ld r16,STK_REG(R16)(r1)
> ld r17,STK_REG(R17)(r1)
> @@ -134,42 +177,19 @@ err2; std r21,120(r3)
> ld r22,STK_REG(R22)(r1)
> addi r1,r1,STACKFRAMESIZE
>
> - /* Up to 127B to go */
> -5: srdi r6,r5,4
> - mtocrf 0x01,r6
> -
> -6: bf cr7*4+1,7f
> -err1; ld r0,0(r4)
> -err1; ld r6,8(r4)
> -err1; ld r7,16(r4)
> -err1; ld r8,24(r4)
> -err1; ld r9,32(r4)
> -err1; ld r10,40(r4)
> -err1; ld r11,48(r4)
> -err1; ld r12,56(r4)
> - addi r4,r4,64
> -err1; std r0,0(r3)
> -err1; std r6,8(r3)
> -err1; std r7,16(r3)
> -err1; std r8,24(r3)
> -err1; std r9,32(r3)
> -err1; std r10,40(r3)
> -err1; std r11,48(r3)
> -err1; std r12,56(r3)
> - addi r3,r3,64
> -
> /* Up to 63B to go */
> -7: bf cr7*4+2,8f
> + bf cr7*4+2,8f
> err1; ld r0,0(r4)
> err1; ld r6,8(r4)
> -err1; ld r7,16(r4)
> -err1; ld r8,24(r4)
> +err1; ld r8,16(r4)
> +err1; ld r9,24(r4)
> addi r4,r4,32
> err1; std r0,0(r3)
> err1; std r6,8(r3)
> -err1; std r7,16(r3)
> -err1; std r8,24(r3)
> +err1; std r8,16(r3)
> +err1; std r9,24(r3)
> addi r3,r3,32
> + subi r7,r7,32
>
> /* Up to 31B to go */
> 8: bf cr7*4+3,9f
> @@ -179,6 +199,7 @@ err1; ld r6,8(r4)
> err1; std r0,0(r3)
> err1; std r6,8(r3)
> addi r3,r3,16
> + subi r7,r7,16
>
> 9: clrldi r5,r5,(64-4)
>
> @@ -192,18 +213,21 @@ err1; lwz r6,4(r4)
> err1; stw r0,0(r3)
> err1; stw r6,4(r3)
> addi r3,r3,8
> + subi r7,r7,8
>
> 12: bf cr7*4+1,13f
> err1; lwz r0,0(r4)
> addi r4,r4,4
> err1; stw r0,0(r3)
> addi r3,r3,4
> + subi r7,r7,4
>
> 13: bf cr7*4+2,14f
> err1; lhz r0,0(r4)
> addi r4,r4,2
> err1; sth r0,0(r3)
> addi r3,r3,2
> + subi r7,r7,2
>
> 14: bf cr7*4+3,15f
> err1; lbz r0,0(r4)
>
^ permalink raw reply
* Re: [v3 6/7] powerpc: add machine check safe copy_to_user
From: Christophe Leroy @ 2019-07-06 6:19 UTC (permalink / raw)
To: Santosh Sivaraj, linuxppc-dev
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Nicholas Piggin,
Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-7-santosh@fossix.org>
Le 05/07/2019 à 23:26, Santosh Sivaraj a écrit :
> Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe()
>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/uaccess.h | 12 ++++++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 8c1c636308c8..a173b392c272 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -134,6 +134,7 @@ config PPC
> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION)
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
> + select ARCH_HAS_UACCESS_MCSAFE if PPC64
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARCH_HAS_ZONE_DEVICE if PPC_BOOK3S_64
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 76f34346b642..f8fcaab4c5bc 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -386,6 +386,18 @@ static inline unsigned long raw_copy_to_user(void __user *to,
> return ret;
> }
>
> +static __always_inline unsigned long __must_check
> +copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n)
> +{
> + if (likely(check_copy_size(from, n, true))) {
> + allow_write_to_user(to, n);
> + n = memcpy_mcsafe(to, from, n);
memcpy_mcsafe() doesn't expect a __user pointer. I think at least a cast
is needed.
And should we perform an access_ok() verification too ?
Christophe
> + prevent_write_to_user(to, n);
> + }
> +
> + return n;
> +}
> +
> extern unsigned long __clear_user(void __user *addr, unsigned long size);
>
> static inline unsigned long clear_user(void __user *addr, unsigned long size)
>
^ permalink raw reply
* Re: [PATCH] powerpc/hw_breakpoint: move instruction stepping out of hw_breakpoint_handler()
From: Christophe Leroy @ 2019-07-06 8:26 UTC (permalink / raw)
To: Ravi Bangoria; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <57148696-b9a5-d3c1-1e29-82673c558927@linux.ibm.com>
Le 03/07/2019 à 08:20, Ravi Bangoria a écrit :
>
>
> On 6/28/19 9:25 PM, Christophe Leroy wrote:
>> On 8xx, breakpoints stop after executing the instruction, so
>> stepping/emulation is not needed. Move it into a sub-function and
>> remove the #ifdefs.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>
> Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>
> Just one neat below...
Thanks for the review.
>
> [...]
>
>> -#ifndef CONFIG_PPC_8xx
>> - /* Do not emulate user-space instructions, instead single-step them */
>> - if (user_mode(regs)) {
>> - current->thread.last_hit_ubp = bp;
>> - regs->msr |= MSR_SE;
>> + if (!IS_ENABLED(CONFIG_PPC_8xx) && !stepping_handler(regs, bp, info->address))
>
> May be split this line. It's 86 chars long and checkpatch.pl is warning
> about this:
Didn't you use arch/powerpc/tools/checkpatch.sh ?
powerpc accepts 90 chars per line.
Christophe
>
> WARNING: line over 80 characters
> #257: FILE: arch/powerpc/kernel/hw_breakpoint.c:282:
> + if (!IS_ENABLED(CONFIG_PPC_8xx) && !stepping_handler(regs, bp, info->address))
>
^ permalink raw reply
* Re: [v3 2/7] powerpc/mce: Bug fixes for MCE handling in kernel space
From: Nicholas Piggin @ 2019-07-06 9:44 UTC (permalink / raw)
To: linuxppc-dev, Santosh Sivaraj
Cc: Aneesh Kumar K.V, Mahesh Salgaonkar, Chandan Rajendra, Reza Arbab
In-Reply-To: <20190705212647.21750-3-santosh@fossix.org>
Santosh Sivaraj's on July 6, 2019 7:26 am:
> From: Balbir Singh <bsingharora@gmail.com>
>
> The code currently assumes PAGE_SHIFT as the shift value of
> the pfn,
This comment doesn't really make sense on its own. Linux pfns
are always units of page shift, so if it's not that then it's
not a pfn.
I think you want the information from the last paragraph up the
here because that explains exactly what the problem is.
> this works correctly (mostly) for user space pages,
> but the correct thing to do is
>
> 1. Extract the shift value returned via the pte-walk API's
> 2. Use the shift value to access the instruction address.
>
> Note, the final physical address still use PAGE_SHIFT for
> computation. handle_ierror() is not modified and handle_derror()
> is modified just for extracting the correct instruction
> address.
>
> This is largely due to __find_linux_pte() returning pfn's
> shifted by pdshift. The code is much more generic and can
> handle shift values returned.
>
> Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors")
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> [arbab@linux.ibm.com: Fixup pseries_do_memory_failure()]
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> Signed-off-by: Santosh Sivaraj <santosh@fossix.org>
> ---
> arch/powerpc/include/asm/mce.h | 3 ++-
> arch/powerpc/kernel/mce_power.c | 26 ++++++++++++++++----------
> arch/powerpc/platforms/pseries/ras.c | 6 ++++--
> 3 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
> index a4c6a74ad2fb..94888a7025b3 100644
> --- a/arch/powerpc/include/asm/mce.h
> +++ b/arch/powerpc/include/asm/mce.h
> @@ -209,7 +209,8 @@ extern void release_mce_event(void);
> extern void machine_check_queue_event(void);
> extern void machine_check_print_event_info(struct machine_check_event *evt,
> bool user_mode, bool in_guest);
> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
> +unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
> + unsigned int *shift);
> #ifdef CONFIG_PPC_BOOK3S_64
> void flush_and_reload_slb(void);
> #endif /* CONFIG_PPC_BOOK3S_64 */
> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
> index e39536aad30d..04666c0b40a8 100644
> --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -23,7 +23,8 @@
> * Convert an address related to an mm to a PFN. NOTE: we are in real
> * mode, we could potentially race with page table updates.
> */
> -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> +unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr,
> + unsigned int *shift)
> {
> pte_t *ptep;
> unsigned long flags;
addr_to_pfn is too generic a name for what it does I think. I would
call it ppc_addr_to_pfn maybe. But it has other weirdness too, it's
assuming the kernel can't fault on user addresses AFAIKS.
> @@ -36,13 +37,15 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
>
> local_irq_save(flags);
> if (mm == current->mm)
> - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL);
> + ptep = find_current_mm_pte(mm->pgd, addr, NULL, shift);
> else
> - ptep = find_init_mm_pte(addr, NULL);
> + ptep = find_init_mm_pte(addr, shift);
> local_irq_restore(flags);
> if (!ptep || pte_special(*ptep))
> return ULONG_MAX;
> - return pte_pfn(*ptep);
> + if (!*shift)
> + *shift = PAGE_SHIFT;
> + return (pte_val(*ptep) & PTE_RPN_MASK) >> *shift;
> }
>
> /* flush SLBs and reload */
AFAIKS moving this weirdness into the callers is pointles, fix it up
into a proper Linux pfn in addr_to_pfn.
The duplication and divergence of machine check code between powernv
and pseries is bad, I'm looking through it and from start to end it's
trying to do basically the same thing in slightly different ways, and
we're still got a lot of bugs. I wonder if we shouldn't try to fix it
all and merge them to a single implementation...
Actually your code does not touch the core of it too significantly so
it looks like we don't have to delay your stuff in order to do that.
This is a good fix though, it should go as a fix at the top of the
series.
Thanks,
Nick
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox