Re: stable-rc build: 0 warnings 2 failures (stable-rc/v4.4.63-29-ge636782)

[Michael Ellerman]

Arnd Bergmann  writes:

> On Tue, Apr 25, 2017 at 7:08 PM, Olof's autobuilder  wrote:
>>
>> Failed defconfigs:
>> powerpc.pasemi_defconfig
>> powerpc.ppc64_defconfig
>>
>> ---
>>
>> Errors:
>>
>> /work/build/batch/arch/powerpc/kernel/misc_64.S:72: Error: .localentry 
>> expression for `flush_icache_range' does not evaluate to a constant
>
> This is a regression in 4.4-stable-rc, caused by the backport of commit
> 8f5f525d5b83f7d7 ("powerpc/64: Fix flush_(d|i)cache_range() called from
> modules").
>
> The patch was also backported into v4.9-stable-rc, but did not cause
> problems there. The fixes tag suggests that the patch is needed on
> every version from 3.16 up, but apparently the fix needs to be a little
> different compared to newer kernels.

Yeah, the auto-backport failed, and then I sent a version for 4.4 which
fixed the bug but broke other configs (big endian).

I've asked Greg to drop it for now.

I'll try again when I'm less sleep deprived :)

cheers

Re: [PATCH] powerpc/kprobes: refactor kprobe_lookup_name for safer string operations

[Masami Hiramatsu]

On Tue, 25 Apr 2017 21:37:11 +0530
"Naveen N. Rao"  wrote:

> Use safer string manipulation functions when dealing with a
> user-provided string in kprobe_lookup_name().
> 
> Reported-by: David Laight 
> Signed-off-by: Naveen N. Rao 
> ---
>  arch/powerpc/kernel/kprobes.c | 47 
> ++-
>  1 file changed, 20 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 160ae0fa7d0d..5a17e6467be9 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -53,7 +53,7 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
>  
>  kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
>  {
> - kprobe_opcode_t *addr;
> + kprobe_opcode_t *addr = NULL;
>  
>  #ifdef PPC64_ELF_ABI_v2
>   /* PPC64 ABIv2 needs local entry point */
> @@ -85,36 +85,29 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, 
> unsigned int offset)
>* Also handle  format.
>*/
>   char dot_name[MODULE_NAME_LEN + 1 + KSYM_NAME_LEN];
> - const char *modsym;
>   bool dot_appended = false;
> - if ((modsym = strchr(name, ':')) != NULL) {
> - modsym++;
> - if (*modsym != '\0' && *modsym != '.') {
> - /* Convert to  */
> - strncpy(dot_name, name, modsym - name);
> - dot_name[modsym - name] = '.';
> - dot_name[modsym - name + 1] = '\0';
> - strncat(dot_name, modsym,
> - sizeof(dot_name) - (modsym - name) - 2);
> - dot_appended = true;
> - } else {
> - dot_name[0] = '\0';
> - strncat(dot_name, name, sizeof(dot_name) - 1);
> - }
> - } else if (name[0] != '.') {
> - dot_name[0] = '.';
> - dot_name[1] = '\0';
> - strncat(dot_name, name, KSYM_NAME_LEN - 2);
> + const char *c;
> + ssize_t ret = 0;
> + int len = 0;
> +
> + if ((c = strnchr(name, MODULE_NAME_LEN, ':')) != NULL) {
> + c++;
> + len = c - name;
> + memcpy(dot_name, name, len);
> + } else
> + c = name;
> +
> + if (*c != '\0' && *c != '.') {
> + dot_name[len++] = '.';
>   dot_appended = true;
> - } else {
> - dot_name[0] = '\0';
> - strncat(dot_name, name, KSYM_NAME_LEN - 1);
>   }
> - addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
> - if (!addr && dot_appended) {
> - /* Let's try the original non-dot symbol lookup */
> + ret = strscpy(dot_name + len, c, KSYM_NAME_LEN);
> + if (ret >= 0)

Here, maybe you can skip the case of ret == 0. (Or, would we have
a symbol which only has "."?)

Others look good to me.

Thank you,

> + addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
> +
> + /* Fallback to the original non-dot symbol lookup */
> + if (!addr && dot_appended)
>   addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
> - }
>  #else
>   addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
>  #endif
> -- 
> 2.12.1
> 


-- 
Masami Hiramatsu 

linux-next: manual merge of the tip tree with the powerpc tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the tip tree got a conflict in:

  kernel/kprobes.c

between commits:

  49e0b4658fe6 ("kprobes: Convert kprobe_lookup_name() to a function")
  290e3070762a ("powerpc/kprobes: Fix handling of function offsets on ABIv2")

from the powerpc tree and commit:

  1d585e70905e ("trace/kprobes: Fix check for kretprobe offset within function 
entry")

from the tip tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc kernel/kprobes.c
index 406889889ce5,c7ea9960433a..
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@@ -1395,16 -1394,14 +1398,14 @@@ bool within_kprobe_blacklist(unsigned l
   * This returns encoded errors if it fails to look up symbol or invalid
   * combination of parameters.
   */
- static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
+ static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
+   const char *symbol_name, unsigned int offset)
  {
-   kprobe_opcode_t *addr = p->addr;
- 
-   if ((p->symbol_name && p->addr) ||
-   (!p->symbol_name && !p->addr))
+   if ((symbol_name && addr) || (!symbol_name && !addr))
goto invalid;
  
-   if (p->symbol_name) {
-   addr = kprobe_lookup_name(p->symbol_name, p->offset);
+   if (symbol_name) {
 -  kprobe_lookup_name(symbol_name, addr);
++  addr = kprobe_lookup_name(symbol_name, offset);
if (!addr)
return ERR_PTR(-ENOENT);
}

[PATCH v3] KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller

[Michael Ellerman]

From: Benjamin Herrenschmidt 

This patch makes KVM capable of using the XIVE interrupt controller
to provide the standard PAPR "XICS" style hypercalls. It is necessary
for proper operations when the host uses XIVE natively.

This has been lightly tested on an actual system, including PCI
pass-through with a TG3 device.

Signed-off-by: Benjamin Herrenschmidt 
[mpe: Cleanup pr_xxx(), unsplit pr_xxx() strings, etc., fix build
 failures by adding KVM_XIVE which depends on KVM_XICS and XIVE, and
 adding empty stubs for the kvm_xive_xxx() routines, fixup subject]
Signed-off-by: Michael Ellerman 
---
 arch/powerpc/include/asm/kvm_book3s_asm.h |2 +
 arch/powerpc/include/asm/kvm_host.h   |   28 +-
 arch/powerpc/include/asm/kvm_ppc.h|   74 ++
 arch/powerpc/include/asm/xive.h   |9 +-
 arch/powerpc/kernel/asm-offsets.c |   10 +
 arch/powerpc/kvm/Kconfig  |5 +
 arch/powerpc/kvm/Makefile |4 +-
 arch/powerpc/kvm/book3s.c |   75 +-
 arch/powerpc/kvm/book3s_hv.c  |   51 +-
 arch/powerpc/kvm/book3s_hv_builtin.c  |  101 ++
 arch/powerpc/kvm/book3s_hv_rm_xics.c  |   10 +-
 arch/powerpc/kvm/book3s_hv_rm_xive.c  |   47 +
 arch/powerpc/kvm/book3s_hv_rmhandlers.S   |   62 +-
 arch/powerpc/kvm/book3s_rtas.c|   21 +-
 arch/powerpc/kvm/book3s_xics.c|   35 +-
 arch/powerpc/kvm/book3s_xics.h|5 +
 arch/powerpc/kvm/book3s_xive.c| 1893 +
 arch/powerpc/kvm/book3s_xive.h|  254 
 arch/powerpc/kvm/book3s_xive_template.c   |  503 
 arch/powerpc/kvm/irq.h|1 +
 arch/powerpc/kvm/powerpc.c|   17 +-
 arch/powerpc/platforms/powernv/opal.c |1 +
 arch/powerpc/sysdev/xive/common.c |  142 ++-
 arch/powerpc/sysdev/xive/native.c |   84 +-
 include/linux/kvm_host.h  |1 -
 virt/kvm/kvm_main.c   |4 -
 26 files changed, 3350 insertions(+), 89 deletions(-)
 create mode 100644 arch/powerpc/kvm/book3s_hv_rm_xive.c
 create mode 100644 arch/powerpc/kvm/book3s_xive.c
 create mode 100644 arch/powerpc/kvm/book3s_xive.h
 create mode 100644 arch/powerpc/kvm/book3s_xive_template.c

v3: As mentioned in the change log:
Cleanup pr_xxx(), unsplit pr_xxx() strings, etc., fix build failures by
adding KVM_XIVE which depends on KVM_XICS and XIVE, and adding empty stubs
for the kvm_xive_xxx() routines, fixup subject.

diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h 
b/arch/powerpc/include/asm/kvm_book3s_asm.h
index 0593d9479f74..b148496ffe36 100644
--- a/arch/powerpc/include/asm/kvm_book3s_asm.h
+++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
@@ -111,6 +111,8 @@ struct kvmppc_host_state {
struct kvm_vcpu *kvm_vcpu;
struct kvmppc_vcore *kvm_vcore;
void __iomem *xics_phys;
+   void __iomem *xive_tima_phys;
+   void __iomem *xive_tima_virt;
u32 saved_xirr;
u64 dabr;
u64 host_mmcr[7];   /* MMCR 0,1,A, SIAR, SDAR, MMCR2, SIER */
diff --git a/arch/powerpc/include/asm/kvm_host.h 
b/arch/powerpc/include/asm/kvm_host.h
index 7bba8f415627..5a8ab4a758f1 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -205,6 +205,12 @@ struct kvmppc_spapr_tce_table {
 /* XICS components, defined in book3s_xics.c */
 struct kvmppc_xics;
 struct kvmppc_icp;
+extern struct kvm_device_ops kvm_xics_ops;
+
+/* XIVE components, defined in book3s_xive.c */
+struct kvmppc_xive;
+struct kvmppc_xive_vcpu;
+extern struct kvm_device_ops kvm_xive_ops;
 
 struct kvmppc_passthru_irqmap;
 
@@ -293,6 +299,7 @@ struct kvm_arch {
 #endif
 #ifdef CONFIG_KVM_XICS
struct kvmppc_xics *xics;
+   struct kvmppc_xive *xive;
struct kvmppc_passthru_irqmap *pimap;
 #endif
struct kvmppc_ops *kvm_ops;
@@ -421,7 +428,7 @@ struct kvmppc_passthru_irqmap {
 
 #define KVMPPC_IRQ_DEFAULT 0
 #define KVMPPC_IRQ_MPIC1
-#define KVMPPC_IRQ_XICS2
+#define KVMPPC_IRQ_XICS2 /* Includes a XIVE option */
 
 #define MMIO_HPTE_CACHE_SIZE   4
 
@@ -443,6 +450,21 @@ struct mmio_hpte_cache {
 
 struct openpic;
 
+/* W0 and W1 of a XIVE thread management context */
+union xive_tma_w01 {
+   struct {
+   u8  nsr;
+   u8  cppr;
+   u8  ipb;
+   u8  lsmfb;
+   u8  ack;
+   u8  inc;
+   u8  age;
+   u8  pipr;
+   };
+   __be64 w01;
+};
+
 struct kvm_vcpu_arch {
ulong host_stack;
u32 host_pid;
@@ -688,6 +710,10 @@ struct kvm_vcpu_arch {
struct openpic *mpic;   /* KVM_IRQ_MPIC */
 #ifdef CONFIG_KVM_XICS
struct kvmppc_icp *icp; /* XICS presentation controller */
+   struct kvmppc_xive_vcpu 

Re: [PATCH v3] KVM: PPC: Book3S PR: Do not fail emulation with mtspr/mfspr for unknown SPRs

[Paul Mackerras]

On Tue, Apr 25, 2017 at 09:14:01AM +0200, Thomas Huth wrote:
> On 05.04.2017 15:58, Thomas Huth wrote:
> > According to the PowerISA 2.07, mtspr and mfspr should not always
> > generate an illegal instruction exception when being used with an
> > undefined SPR, but rather treat the instruction as a NOP or inject a
> > privilege exception in some cases, too - depending on the SPR number.
> > Also turn the printk here into a ratelimited print statement, so that
> > the guest can not flood the dmesg log of the host by issueing lots of
> > illegal mtspr/mfspr instruction here.
> > 
> > Signed-off-by: Thomas Huth 
> > ---
> >  v3:
> >  - Make sure that we do not advance the program counter after we've
> >already changed it due to injecting a program interrupt
> > 
> >  v2:
> >  - Inject illegal instruction program interrupt instead of emulation
> >assist interrupt (according to the last programming note in section
> >6.5.9 of Book III of the PowerISA v2.07)
> > 
> >  arch/powerpc/kvm/book3s_emulate.c | 34 ++
> >  arch/powerpc/kvm/emulate.c|  8 
> >  2 files changed, 34 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/powerpc/kvm/book3s_emulate.c 
> > b/arch/powerpc/kvm/book3s_emulate.c
> > index 8359752..68d6898 100644
> > --- a/arch/powerpc/kvm/book3s_emulate.c
> > +++ b/arch/powerpc/kvm/book3s_emulate.c
> > @@ -503,10 +503,18 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu 
> > *vcpu, int sprn, ulong spr_val)
> > break;
> >  unprivileged:
> > default:
> > -   printk(KERN_INFO "KVM: invalid SPR write: %d\n", sprn);
> > -#ifndef DEBUG_SPR
> > -   emulated = EMULATE_FAIL;
> > -#endif
> > +   pr_info_ratelimited("KVM: invalid SPR write: %d\n", sprn);
> > +   if (sprn & 0x10) {
> > +   if (kvmppc_get_msr(vcpu) & MSR_PR) {
> > +   kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV);
> > +   emulated = EMULATE_AGAIN;
> > +   }
> > +   } else {
> > +   if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0) {
> > +   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> > +   emulated = EMULATE_AGAIN;
> > +   }
> > +   }
> > break;
> > }
> >  
> > @@ -648,10 +656,20 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu 
> > *vcpu, int sprn, ulong *spr_val
> > break;
> > default:
> >  unprivileged:
> > -   printk(KERN_INFO "KVM: invalid SPR read: %d\n", sprn);
> > -#ifndef DEBUG_SPR
> > -   emulated = EMULATE_FAIL;
> > -#endif
> > +   pr_info_ratelimited("KVM: invalid SPR read: %d\n", sprn);
> > +   if (sprn & 0x10) {
> > +   if (kvmppc_get_msr(vcpu) & MSR_PR) {
> > +   kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV);
> > +   emulated = EMULATE_AGAIN;
> > +   }
> > +   } else {
> > +   if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0 ||
> > +   sprn == 4 || sprn == 5 || sprn == 6) {
> > +   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> > +   emulated = EMULATE_AGAIN;
> > +   }
> > +   }
> > +
> > break;
> > }
> >  
> > diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> > index b379146..c873ffe 100644
> > --- a/arch/powerpc/kvm/emulate.c
> > +++ b/arch/powerpc/kvm/emulate.c
> > @@ -259,10 +259,18 @@ int kvmppc_emulate_instruction(struct kvm_run *run, 
> > struct kvm_vcpu *vcpu)
> >  
> > case OP_31_XOP_MFSPR:
> > emulated = kvmppc_emulate_mfspr(vcpu, sprn, rt);
> > +   if (emulated == EMULATE_AGAIN) {
> > +   emulated = EMULATE_DONE;
> > +   advance = 0;
> > +   }
> > break;
> >  
> > case OP_31_XOP_MTSPR:
> > emulated = kvmppc_emulate_mtspr(vcpu, sprn, rs);
> > +   if (emulated == EMULATE_AGAIN) {
> > +   emulated = EMULATE_DONE;
> > +   advance = 0;
> > +   }
> > break;
> >  
> > case OP_31_XOP_TLBSYNC:
> > 
> 
> Ping!
> 
> Paul, does this now look OK for you this way?

Yes, and I put it in, see commit feafd13c96d6.

Paul.

Re: stable-rc build: 0 warnings 2 failures (stable-rc/v4.4.63-29-ge636782)

[Arnd Bergmann]

On Tue, Apr 25, 2017 at 7:08 PM, Olof's autobuilder  wrote:
>
> Failed defconfigs:
> powerpc.pasemi_defconfig
> powerpc.ppc64_defconfig
>
> ---
>
> Errors:
>
> /work/build/batch/arch/powerpc/kernel/misc_64.S:72: Error: .localentry 
> expression for `flush_icache_range' does not evaluate to a constant

This is a regression in 4.4-stable-rc, caused by the backport of commit
8f5f525d5b83f7d7 ("powerpc/64: Fix flush_(d|i)cache_range() called from
modules").

The patch was also backported into v4.9-stable-rc, but did not cause
problems there. The fixes tag suggests that the patch is needed on
every version from 3.16 up, but apparently the fix needs to be a little
different compared to newer kernels.

Arnd

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

[Cédric Le Goater]

On 04/22/2017 08:11 AM, Michael Ellerman wrote:
> Shilpasri G Bhat  writes:
>> On 04/21/2017 05:17 PM, Cédric Le Goater wrote:
>>> On 04/21/2017 06:31 AM, Shilpasri G Bhat wrote:
 diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
 index 6d2e660..1f329fa8 100644
 --- a/drivers/hwmon/ibmpowernv.c
 +++ b/drivers/hwmon/ibmpowernv.c
 @@ -65,7 +66,8 @@ enum sensors {
{"fan", "ibm,opal-sensor-cooling-fan"},
{"temp", "ibm,opal-sensor-amb-temp"},
{"in", "ibm,opal-sensor-power-supply"},
 -  {"power", "ibm,opal-sensor-power"}
 +  {"power", "ibm,opal-sensor-power"},
 +  {"curr"}, /* Follows newer device tree compatible ibm,opal-sensor */
>>>
>>> why isn't there a compatible string ? 
>>
>> Skiboot exports "ibm,opal-sensor" as compatible string for all the inband
>> sensors. Now if I define that as the compatible string here, then all the
>> sensors would get identified as "curr" type of sensor by the driver.
> 
> So fix it in skiboot?


After a memory refresh, this table is to maintain compatibility with 
the support of the FSP sensors in old firmware. These have peculiar
device node names and properties.  

So What the code is doing looks correct. But, you should also modify 
the 'enum sensors' to include a CURRENT value.

Thanks,

C.

RE: [PATCH] kallsyms: optimize kallsyms_lookup_name() for a few cases

[Naveen N. Rao]

Excerpts from David Laight's message of April 25, 2017 22:06:

From: Naveen N. Rao

Sent: 25 April 2017 17:18
1. Fail early for invalid/zero length symbols.
2. Detect names of the form  and skip checking for kernel
symbols in that case.

Signed-off-by: Naveen N. Rao 
---
Masami, Michael,
I have added two very simple checks here, which I felt is good to have,
rather than the elaborate checks in the previous version. Given the
change in module code to use strnchr(), the checks are now safe and
further tests are not probably not that useful.

...

+   if (strnchr(name, MODULE_NAME_LEN, ':'))
+   return module_kallsyms_lookup_name(name);


Should that be MODULE_NAME_LEN - 1 ?


The ':' character _follows_ the module name, which can itself be upto 
MODULE_NAME_LEN - 1 characters. So, we should look for it till 
MODULE_NAME_LEN.


Thanks for the review,
- Naveen

RE: [PATCH] kallsyms: optimize kallsyms_lookup_name() for a few cases

[David Laight]

From: Naveen N. Rao
> Sent: 25 April 2017 17:18
> 1. Fail early for invalid/zero length symbols.
> 2. Detect names of the form  and skip checking for kernel
> symbols in that case.
> 
> Signed-off-by: Naveen N. Rao 
> ---
> Masami, Michael,
> I have added two very simple checks here, which I felt is good to have,
> rather than the elaborate checks in the previous version. Given the
> change in module code to use strnchr(), the checks are now safe and
> further tests are not probably not that useful.
...
> + if (strnchr(name, MODULE_NAME_LEN, ':'))
> + return module_kallsyms_lookup_name(name);

Should that be MODULE_NAME_LEN - 1 ?

David

[PATCH 4/4] powerpc/kprobes: blacklist functions involved when returning from exception

[Naveen N. Rao]

Blacklist all functions involved when we return from a trap. We:
- convert some of the labels into private labels,
- remove the duplicate 'restore' label, and
- blacklist most functions involved during returning from a trap.

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/entry_64.S | 46 --
 arch/powerpc/kernel/traps.c|  1 +
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 22aaa377149f..e7e05eb590a5 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -184,7 +184,7 @@ __system_call:
 #ifdef CONFIG_PPC_BOOK3S
/* No MSR:RI on BookE */
andi.   r10,r8,MSR_RI
-   beq-unrecov_restore
+   beq-.Lunrecov_restore
 #endif
/*
 * Disable interrupts so current_thread_info()->flags can't change,
@@ -643,18 +643,18 @@ _GLOBAL(ret_from_except_lite)
 * Use the internal debug mode bit to do this.
 */
andis.  r0,r3,DBCR0_IDM@h
-   beq restore
+   beq fast_exc_return_irq
mfmsr   r0
rlwinm  r0,r0,0,~MSR_DE /* Clear MSR.DE */
mtmsr   r0
mtspr   SPRN_DBCR0,r3
li  r10, -1
mtspr   SPRN_DBSR,r10
-   b   restore
+   b   fast_exc_return_irq
 #else
addir3,r1,STACK_FRAME_OVERHEAD
bl  restore_math
-   b   restore
+   b   fast_exc_return_irq
 #endif
 1: andi.   r0,r4,_TIF_NEED_RESCHED
beq 2f
@@ -667,7 +667,7 @@ _GLOBAL(ret_from_except_lite)
bne 3f  /* only restore TM if nothing else to do */
addir3,r1,STACK_FRAME_OVERHEAD
bl  restore_tm_state
-   b   restore
+   b   fast_exc_return_irq
 3:
 #endif
bl  save_nvgprs
@@ -719,14 +719,14 @@ resume_kernel:
 #ifdef CONFIG_PREEMPT
/* Check if we need to preempt */
andi.   r0,r4,_TIF_NEED_RESCHED
-   beq+restore
+   beq+fast_exc_return_irq
/* Check that preempt_count() == 0 and interrupts are enabled */
lwz r8,TI_PREEMPT(r9)
cmpwi   cr1,r8,0
ld  r0,SOFTE(r1)
cmpdi   r0,0
crandc  eq,cr1*4+eq,eq
-   bne restore
+   bne fast_exc_return_irq
 
/*
 * Here we are preempting the current task. We want to make
@@ -757,7 +757,6 @@ resume_kernel:
 
.globl  fast_exc_return_irq
 fast_exc_return_irq:
-restore:
/*
 * This is the main kernel exit path. First we check if we
 * are about to re-enable interrupts
@@ -765,11 +764,11 @@ restore:
ld  r5,SOFTE(r1)
lbz r6,PACASOFTIRQEN(r13)
cmpwi   cr0,r5,0
-   beq restore_irq_off
+   beq .Lrestore_irq_off
 
/* We are enabling, were we already enabled ? Yes, just return */
cmpwi   cr0,r6,1
-   beq cr0,do_restore
+   beq cr0,.Ldo_restore
 
/*
 * We are about to soft-enable interrupts (we are hard disabled
@@ -778,14 +777,14 @@ restore:
 */
lbz r0,PACAIRQHAPPENED(r13)
cmpwi   cr0,r0,0
-   bne-restore_check_irq_replay
+   bne-.Lrestore_check_irq_replay
 
/*
 * Get here when nothing happened while soft-disabled, just
 * soft-enable and move-on. We will hard-enable as a side
 * effect of rfi
 */
-restore_no_replay:
+.Lrestore_no_replay:
TRACE_ENABLE_INTS
li  r0,1
stb r0,PACASOFTIRQEN(r13);
@@ -793,7 +792,7 @@ restore_no_replay:
/*
 * Final return path. BookE is handled in a different file
 */
-do_restore:
+.Ldo_restore:
 #ifdef CONFIG_PPC_BOOK3E
b   exception_return_book3e
 #else
@@ -827,7 +826,7 @@ fast_exception_return:
REST_8GPRS(5, r1)
 
andi.   r0,r3,MSR_RI
-   beq-unrecov_restore
+   beq-.Lunrecov_restore
 
/* Load PPR from thread struct before we clear MSR:RI */
 BEGIN_FTR_SECTION
@@ -885,7 +884,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 * make sure that in this case, we also clear PACA_IRQ_HARD_DIS
 * or that bit can get out of sync and bad things will happen
 */
-restore_irq_off:
+.Lrestore_irq_off:
ld  r3,_MSR(r1)
lbz r7,PACAIRQHAPPENED(r13)
andi.   r0,r3,MSR_EE
@@ -895,13 +894,13 @@ restore_irq_off:
 1: li  r0,0
stb r0,PACASOFTIRQEN(r13);
TRACE_DISABLE_INTS
-   b   do_restore
+   b   .Ldo_restore
 
/*
 * Something did happen, check if a re-emit is needed
 * (this also clears paca->irq_happened)
 */
-restore_check_irq_replay:
+.Lrestore_check_irq_replay:
/* XXX: We could implement a fast path here where we check
 * for irq_happened being just 0x01, in which case we can
 * clear it and return. That 

[PATCH 3/4] powerpc/kprobes: blacklist functions invoked on a trap

[Naveen N. Rao]

Blacklist all functions invoked when we get a trap, through to the time
we invoke the kprobe handler.

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/entry_64.S   | 1 +
 arch/powerpc/kernel/exceptions-64s.S | 1 +
 arch/powerpc/kernel/time.c   | 3 +++
 arch/powerpc/kernel/traps.c  | 2 ++
 arch/powerpc/platforms/pseries/dtl.c | 2 ++
 5 files changed, 9 insertions(+)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index e030ce34dd66..22aaa377149f 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -399,6 +399,7 @@ _GLOBAL(save_nvgprs)
clrrdi  r0,r11,1
std r0,_TRAP(r1)
blr
+_ASM_NOKPROBE_SYMBOL(save_nvgprs);
 

 /*
diff --git a/arch/powerpc/kernel/exceptions-64s.S 
b/arch/powerpc/kernel/exceptions-64s.S
index 28f8d7bed6b1..aff23b2288f2 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1534,6 +1534,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 1: addir3,r1,STACK_FRAME_OVERHEAD
bl  kernel_bad_stack
b   1b
+_ASM_NOKPROBE_SYMBOL(bad_stack);
 
 /*
  * Called from arch_local_irq_enable when an interrupt needs
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 07b90725855e..afdec3c00738 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -59,6 +59,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -236,6 +237,7 @@ static u64 scan_dispatch_log(u64 stop_tb)
local_paca->dtl_curr = dtl;
return stolen;
 }
+NOKPROBE_SYMBOL(scan_dispatch_log);
 
 /*
  * Accumulate stolen time by scanning the dispatch trace log.
@@ -263,6 +265,7 @@ void accumulate_stolen_time(void)
 
local_paca->soft_enabled = save_soft_enabled;
 }
+NOKPROBE_SYMBOL(accumulate_stolen_time);
 
 static inline u64 calculate_stolen_time(u64 stop_tb)
 {
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 76f6045b021b..ece130515cd0 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -237,6 +237,7 @@ void die(const char *str, struct pt_regs *regs, long err)
err = 0;
oops_end(flags, regs, err);
 }
+NOKPROBE_SYMBOL(die);
 
 void user_single_step_siginfo(struct task_struct *tsk,
struct pt_regs *regs, siginfo_t *info)
@@ -1981,6 +1982,7 @@ void kernel_bad_stack(struct pt_regs *regs)
   regs->gpr[1], regs->nip);
die("Bad kernel stack pointer", regs, SIGABRT);
 }
+NOKPROBE_SYMBOL(kernel_bad_stack);
 
 void __init trap_init(void)
 {
diff --git a/arch/powerpc/platforms/pseries/dtl.c 
b/arch/powerpc/platforms/pseries/dtl.c
index 18014cdeb590..03d333ff7867 100644
--- a/arch/powerpc/platforms/pseries/dtl.c
+++ b/arch/powerpc/platforms/pseries/dtl.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct dtl {
struct dtl_entry*buf;
@@ -97,6 +98,7 @@ static void consume_dtle(struct dtl_entry *dtle, u64 index)
smp_wmb();
++dtlr->write_index;
 }
+NOKPROBE_SYMBOL(consume_dtle);
 
 static int dtl_start(struct dtl *dtl)
 {
-- 
2.12.1

[PATCH 2/4] powerpc/kprobes: un-blacklist system_call() from kprobes

[Naveen N. Rao]

It is actually safe to probe system_call() in entry_64.S, but only till
.Lsyscall_exit. To allow this, convert .Lsyscall_exit to a non-local
symbol __system_call() and blacklist that symbol, rather than
system_call().

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/entry_64.S | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 380361c0bb6a..e030ce34dd66 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -176,7 +176,7 @@ system_call:/* label this so stack 
traces look sane */
mtctr   r12
bctrl   /* Call handler */
 
-.Lsyscall_exit:
+__system_call:
std r3,RESULT(r1)
CURRENT_THREAD_INFO(r12, r1)
 
@@ -294,12 +294,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
blt+system_call
 
/* Return code is already in r3 thanks to do_syscall_trace_enter() */
-   b   .Lsyscall_exit
+   b   __system_call
 
 
 .Lsyscall_enosys:
li  r3,-ENOSYS
-   b   .Lsyscall_exit
+   b   __system_call

 .Lsyscall_exit_work:
 #ifdef CONFIG_PPC_BOOK3S
@@ -388,7 +388,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
b   .   /* prevent speculative execution */
 #endif
 _ASM_NOKPROBE_SYMBOL(system_call_common);
-_ASM_NOKPROBE_SYMBOL(system_call);
+_ASM_NOKPROBE_SYMBOL(__system_call);
 
 /* Save non-volatile GPRs, if not already saved. */
 _GLOBAL(save_nvgprs)
@@ -413,38 +413,38 @@ _GLOBAL(save_nvgprs)
 _GLOBAL(ppc_fork)
bl  save_nvgprs
bl  sys_fork
-   b   .Lsyscall_exit
+   b   __system_call
 
 _GLOBAL(ppc_vfork)
bl  save_nvgprs
bl  sys_vfork
-   b   .Lsyscall_exit
+   b   __system_call
 
 _GLOBAL(ppc_clone)
bl  save_nvgprs
bl  sys_clone
-   b   .Lsyscall_exit
+   b   __system_call
 
 _GLOBAL(ppc32_swapcontext)
bl  save_nvgprs
bl  compat_sys_swapcontext
-   b   .Lsyscall_exit
+   b   __system_call
 
 _GLOBAL(ppc64_swapcontext)
bl  save_nvgprs
bl  sys_swapcontext
-   b   .Lsyscall_exit
+   b   __system_call
 
 _GLOBAL(ppc_switch_endian)
bl  save_nvgprs
bl  sys_switch_endian
-   b   .Lsyscall_exit
+   b   __system_call
 
 _GLOBAL(ret_from_fork)
bl  schedule_tail
REST_NVGPRS(r1)
li  r3,0
-   b   .Lsyscall_exit
+   b   __system_call
 
 _GLOBAL(ret_from_kernel_thread)
bl  schedule_tail
@@ -456,7 +456,7 @@ _GLOBAL(ret_from_kernel_thread)
 #endif
blrl
li  r3,0
-   b   .Lsyscall_exit
+   b   __system_call
 
 /*
  * This routine switches between two different tasks.  The process
-- 
2.12.1

[PATCH 1/4] powerpc/kprobes: cleanup system_call_common and blacklist it from kprobes

[Naveen N. Rao]

Convert some of the labels into private labels and blacklist
system_call_common() and system_call() from kprobes. We can't take a
trap at parts of these functions as either MSR_RI is unset or the
kernel stack pointer is not yet setup.

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/entry_64.S | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 9b541d22595a..380361c0bb6a 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -52,12 +52,11 @@ exception_marker:
.section".text"
.align 7
 
-   .globl system_call_common
-system_call_common:
+_GLOBAL(system_call_common)
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 BEGIN_FTR_SECTION
extrdi. r10, r12, 1, (63-MSR_TS_T_LG) /* transaction active? */
-   bne tabort_syscall
+   bne .Ltabort_syscall
 END_FTR_SECTION_IFSET(CPU_FTR_TM)
 #endif
andi.   r10,r12,MSR_PR
@@ -152,9 +151,9 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
CURRENT_THREAD_INFO(r11, r1)
ld  r10,TI_FLAGS(r11)
andi.   r11,r10,_TIF_SYSCALL_DOTRACE
-   bne syscall_dotrace /* does not return */
+   bne .Lsyscall_dotrace   /* does not return */
cmpldi  0,r0,NR_syscalls
-   bge-syscall_enosys
+   bge-.Lsyscall_enosys
 
 system_call:   /* label this so stack traces look sane */
 /*
@@ -208,7 +207,7 @@ system_call:/* label this so stack 
traces look sane */
ld  r9,TI_FLAGS(r12)
li  r11,-MAX_ERRNO
andi.   
r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
-   bne-syscall_exit_work
+   bne-.Lsyscall_exit_work
 
andi.   r0,r8,MSR_FP
beq 2f
@@ -232,7 +231,7 @@ system_call:/* label this so stack 
traces look sane */
 
 3: cmpld   r3,r11
ld  r5,_CCR(r1)
-   bge-syscall_error
+   bge-.Lsyscall_error
 .Lsyscall_error_cont:
ld  r7,_NIP(r1)
 BEGIN_FTR_SECTION
@@ -258,14 +257,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
RFI
b   .   /* prevent speculative execution */
 
-syscall_error: 
+.Lsyscall_error:
orisr5,r5,0x1000/* Set SO bit in CR */
neg r3,r3
std r5,_CCR(r1)
b   .Lsyscall_error_cont

 /* Traced system call support */
-syscall_dotrace:
+.Lsyscall_dotrace:
bl  save_nvgprs
addir3,r1,STACK_FRAME_OVERHEAD
bl  do_syscall_trace_enter
@@ -298,11 +297,11 @@ syscall_dotrace:
b   .Lsyscall_exit
 
 
-syscall_enosys:
+.Lsyscall_enosys:
li  r3,-ENOSYS
b   .Lsyscall_exit

-syscall_exit_work:
+.Lsyscall_exit_work:
 #ifdef CONFIG_PPC_BOOK3S
li  r10,MSR_RI
mtmsrd  r10,1   /* Restore RI */
@@ -362,7 +361,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
b   ret_from_except
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-tabort_syscall:
+.Ltabort_syscall:
/* Firstly we need to enable TM in the kernel */
mfmsr   r10
li  r9, 1
@@ -388,6 +387,8 @@ tabort_syscall:
rfid
b   .   /* prevent speculative execution */
 #endif
+_ASM_NOKPROBE_SYMBOL(system_call_common);
+_ASM_NOKPROBE_SYMBOL(system_call);
 
 /* Save non-volatile GPRs, if not already saved. */
 _GLOBAL(save_nvgprs)
-- 
2.12.1

[PATCH 0/4] powerpc: build out kprobes blacklist

[Naveen N. Rao]

This is the second in the series of patches to build out an appropriate
kprobes blacklist. This series blacklists system_call() and functions
involved when handling the trap itself. Not everything is covered, but
this is the first set of functions that I have tested with. More
patches to follow once I expand my tests.

I have converted many labels into private -- these are labels that I
felt are not necessary to read stack traces. If any of those are
important to have, please let me know.

- Naveen

Naveen N. Rao (4):
  powerpc/kprobes: cleanup system_call_common and blacklist it from
kprobes
  powerpc/kprobes: un-blacklist system_call() from kprobes
  powerpc/kprobes: blacklist functions invoked on a trap
  powerpc/kprobes: blacklist functions involved when returning from
exception

 arch/powerpc/kernel/entry_64.S   | 94 +++-
 arch/powerpc/kernel/exceptions-64s.S |  1 +
 arch/powerpc/kernel/time.c   |  3 ++
 arch/powerpc/kernel/traps.c  |  3 ++
 arch/powerpc/platforms/pseries/dtl.c |  2 +
 5 files changed, 60 insertions(+), 43 deletions(-)

-- 
2.12.1

[PATCH] kallsyms: optimize kallsyms_lookup_name() for a few cases

[Naveen N. Rao]

1. Fail early for invalid/zero length symbols.
2. Detect names of the form  and skip checking for kernel
symbols in that case.

Signed-off-by: Naveen N. Rao 
---
Masami, Michael,
I have added two very simple checks here, which I felt is good to have,
rather than the elaborate checks in the previous version. Given the
change in module code to use strnchr(), the checks are now safe and
further tests are not probably not that useful.

- Naveen

 kernel/kallsyms.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 6a3b249a2ae1..d134b060564f 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -205,6 +205,12 @@ unsigned long kallsyms_lookup_name(const char *name)
unsigned long i;
unsigned int off;
 
+   if (!name || *name == '\0')
+   return false;
+
+   if (strnchr(name, MODULE_NAME_LEN, ':'))
+   return module_kallsyms_lookup_name(name);
+
for (i = 0, off = 0; i < kallsyms_num_syms; i++) {
off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
 
-- 
2.12.1

Re: [PATCH] powerpc/mm: Fix possible out-of-bounds shift in arch_mmap_rnd()

[Kees Cook]

On Tue, Apr 25, 2017 at 5:09 AM, Michael Ellerman  wrote:
> The recent patch to add runtime configuration of the ASLR limits added a bug 
> in
> arch_mmap_rnd() where we may shift an integer (32-bits) by up to 33 bits,
> leading to undefined behaviour.
>
> In practice it exhibits as every process seg faulting instantly, presumably
> because the rnd value hasn't been restricited by the modulus at all. We didn't
> notice because it only happens under certain kernel configurations and if the
> number of bits is actually set to a large value.
>
> Fix it by switching to unsigned long.
>
> Fixes: 9fea59bd7ca5 ("powerpc/mm: Add support for runtime configuration of 
> ASLR limits")
> Reported-by: Balbir Singh 
> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/mm/mmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
> index 005aa8a44915..9dbd2a733d6b 100644
> --- a/arch/powerpc/mm/mmap.c
> +++ b/arch/powerpc/mm/mmap.c
> @@ -66,7 +66,7 @@ unsigned long arch_mmap_rnd(void)
> if (is_32bit_task())
> shift = mmap_rnd_compat_bits;
>  #endif
> -   rnd = get_random_long() % (1 << shift);
> +   rnd = get_random_long() % (1ul << shift);
>
> return rnd << PAGE_SHIFT;
>  }
> --
> 2.7.4

Reviewed-by: Kees Cook 

-Kees


-- 
Kees Cook
Pixel Security

[PATCH] powerpc/kprobes: refactor kprobe_lookup_name for safer string operations

[Naveen N. Rao]

Use safer string manipulation functions when dealing with a
user-provided string in kprobe_lookup_name().

Reported-by: David Laight 
Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/kprobes.c | 47 ++-
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 160ae0fa7d0d..5a17e6467be9 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -53,7 +53,7 @@ bool arch_within_kprobe_blacklist(unsigned long addr)
 
 kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
 {
-   kprobe_opcode_t *addr;
+   kprobe_opcode_t *addr = NULL;
 
 #ifdef PPC64_ELF_ABI_v2
/* PPC64 ABIv2 needs local entry point */
@@ -85,36 +85,29 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, 
unsigned int offset)
 * Also handle  format.
 */
char dot_name[MODULE_NAME_LEN + 1 + KSYM_NAME_LEN];
-   const char *modsym;
bool dot_appended = false;
-   if ((modsym = strchr(name, ':')) != NULL) {
-   modsym++;
-   if (*modsym != '\0' && *modsym != '.') {
-   /* Convert to  */
-   strncpy(dot_name, name, modsym - name);
-   dot_name[modsym - name] = '.';
-   dot_name[modsym - name + 1] = '\0';
-   strncat(dot_name, modsym,
-   sizeof(dot_name) - (modsym - name) - 2);
-   dot_appended = true;
-   } else {
-   dot_name[0] = '\0';
-   strncat(dot_name, name, sizeof(dot_name) - 1);
-   }
-   } else if (name[0] != '.') {
-   dot_name[0] = '.';
-   dot_name[1] = '\0';
-   strncat(dot_name, name, KSYM_NAME_LEN - 2);
+   const char *c;
+   ssize_t ret = 0;
+   int len = 0;
+
+   if ((c = strnchr(name, MODULE_NAME_LEN, ':')) != NULL) {
+   c++;
+   len = c - name;
+   memcpy(dot_name, name, len);
+   } else
+   c = name;
+
+   if (*c != '\0' && *c != '.') {
+   dot_name[len++] = '.';
dot_appended = true;
-   } else {
-   dot_name[0] = '\0';
-   strncat(dot_name, name, KSYM_NAME_LEN - 1);
}
-   addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
-   if (!addr && dot_appended) {
-   /* Let's try the original non-dot symbol lookup */
+   ret = strscpy(dot_name + len, c, KSYM_NAME_LEN);
+   if (ret >= 0)
+   addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
+
+   /* Fallback to the original non-dot symbol lookup */
+   if (!addr && dot_appended)
addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
-   }
 #else
addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
 #endif
-- 
2.12.1

[PATCH v2] selftests: splice: override clean in lib.mk to fix warnings

[Shuah Khan]

Add override with EXTRA_CLEAN for lib.mk clean to fix the following
warnings from clean target run.

Makefile:8: warning: overriding recipe for target 'clean'
../lib.mk:55: warning: ignoring old recipe for target 'clean'

Signed-off-by: Shuah Khan 
---
Changes since v1:
- simplified to use EXTRA_CLEAN based on Michael Ellerman's comments.

 tools/testing/selftests/splice/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/splice/Makefile 
b/tools/testing/selftests/splice/Makefile
index 559512c..9fc78e5 100644
--- a/tools/testing/selftests/splice/Makefile
+++ b/tools/testing/selftests/splice/Makefile
@@ -4,5 +4,4 @@ all: $(TEST_PROGS) $(EXTRA)
 
 include ../lib.mk
 
-clean:
-   rm -fr $(EXTRA)
+EXTRA_CLEAN := $(EXTRA)
-- 
2.9.3

[PATCH v2] selftests: sync: override clean in lib.mk to fix warnings

[Shuah Khan]

Add override with EXTRA_CLEAN for lib.mk clean to fix the following
warnings from clean target run.

Makefile:24: warning: overriding recipe for target 'clean'
../lib.mk:55: warning: ignoring old recipe for target 'clean'

Signed-off-by: Shuah Khan 
---
Changes since v1:
- simplified to use EXTRA_CLEAN based on Michael Ellerman's comments.

 tools/testing/selftests/sync/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/sync/Makefile 
b/tools/testing/selftests/sync/Makefile
index 87ac400..4981c6b 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -20,5 +20,4 @@ TESTS += sync_stress_merge.o
 
 sync_test: $(OBJS) $(TESTS)
 
-clean:
-   $(RM) sync_test $(OBJS) $(TESTS)
+EXTRA_CLEAN := sync_test $(OBJS) $(TESTS)
-- 
2.9.3

[PATCH v2] selftests: x86: override clean in lib.mk to fix warnings

[Shuah Khan]

Add override with EXTRA_CLEAN for lib.mk clean to fix the following
warnings from clean target run.

Makefile:44: warning: overriding recipe for target 'clean'
../lib.mk:55: warning: ignoring old recipe for target 'clean'

Signed-off-by: Shuah Khan 
---
Changes since v1:
- simplified to use EXTRA_CLEAN based on Michael Ellerman's comments.

 tools/testing/selftests/x86/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/x86/Makefile 
b/tools/testing/selftests/x86/Makefile
index 38e0a9c..97f187e 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -40,8 +40,7 @@ all_32: $(BINARIES_32)
 
 all_64: $(BINARIES_64)
 
-clean:
-   $(RM) $(BINARIES_32) $(BINARIES_64)
+EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
 
 $(BINARIES_32): $(OUTPUT)/%_32: %.c
$(CC) -m32 -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl -lm
-- 
2.9.3

Re: [PATCH 8/8] selftests: x86: override clean in lib.mk to fix warnings

[Shuah Khan]

On 04/21/2017 11:41 PM, Michael Ellerman wrote:
> Shuah Khan  writes:
> 
>> Add override for lib.mk clean to fix the following warnings from clean
>> target run.
>>
>> Makefile:44: warning: overriding recipe for target 'clean'
>> ../lib.mk:55: warning: ignoring old recipe for target 'clean'
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  tools/testing/selftests/x86/Makefile | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/x86/Makefile 
>> b/tools/testing/selftests/x86/Makefile
>> index 38e0a9c..4d27550 100644
>> --- a/tools/testing/selftests/x86/Makefile
>> +++ b/tools/testing/selftests/x86/Makefile
>> @@ -40,8 +40,9 @@ all_32: $(BINARIES_32)
>>  
>>  all_64: $(BINARIES_64)
>>  
>> -clean:
>> +override define CLEAN
>>  $(RM) $(BINARIES_32) $(BINARIES_64)
>> +endef
> 
> Simpler as:
> 
> EXTRA_CLEAN := $(BINARIES_32) $(BINARIES_64)
> 

Will send v2 with this change.

thanks,
-- Shuah

Re: [PATCH 7/8] selftests: sync: override clean in lib.mk to fix warnings

[Shuah Khan]

On 04/21/2017 11:41 PM, Michael Ellerman wrote:
> Shuah Khan  writes:
> 
>> Add override for lib.mk clean to fix the following warnings from clean
>> target run.
>>
>> Makefile:24: warning: overriding recipe for target 'clean'
>> ../lib.mk:55: warning: ignoring old recipe for target 'clean'
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  tools/testing/selftests/sync/Makefile | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/sync/Makefile 
>> b/tools/testing/selftests/sync/Makefile
>> index 87ac400..f7d250d 100644
>> --- a/tools/testing/selftests/sync/Makefile
>> +++ b/tools/testing/selftests/sync/Makefile
>> @@ -20,5 +20,6 @@ TESTS += sync_stress_merge.o
>>  
>>  sync_test: $(OBJS) $(TESTS)
>>  
>> -clean:
>> +override define CLEAN
>>  $(RM) sync_test $(OBJS) $(TESTS)
>> +endef
> 
> EXTRA_CLEAN := sync_test $(OBJS) $(TESTS)
> 

Will send v2 with this change.

-- Shuah

Re: [PATCH 6/8] selftests: splice: override clean in lib.mk to fix warnings

[Shuah Khan]

On 04/21/2017 11:40 PM, Michael Ellerman wrote:
> Shuah Khan  writes:
> 
>> Add override for lib.mk clean to fix the following warnings from clean
>> target run.
>>
>> Makefile:8: warning: overriding recipe for target 'clean'
>> ../lib.mk:55: warning: ignoring old recipe for target 'clean'
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  tools/testing/selftests/splice/Makefile | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/splice/Makefile 
>> b/tools/testing/selftests/splice/Makefile
>> index 559512c..3f967ba 100644
>> --- a/tools/testing/selftests/splice/Makefile
>> +++ b/tools/testing/selftests/splice/Makefile
>> @@ -4,5 +4,6 @@ all: $(TEST_PROGS) $(EXTRA)
>>  
>>  include ../lib.mk
>>  
>> -clean:
>> +override define CLEAN
>>  rm -fr $(EXTRA)
>> +endef
> 
> Could just be:
> 
> EXTRA_CLEAN := $(EXTRA)
> 

Will send v2 with this change.

-- Shuah

Re: [PATCH 4/8] selftests: gpio: override clean in lib.mk to fix warnings

[Shuah Khan]

On 04/21/2017 11:40 PM, Michael Ellerman wrote:
> Shuah Khan  writes:
> 
>> Add override for lib.mk clean to fix the following warnings from clean
>> target run.
>>
>> Makefile:11: warning: overriding recipe for target 'clean'
>> ../lib.mk:55: warning: ignoring old recipe for target 'clean'
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  tools/testing/selftests/gpio/Makefile | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/gpio/Makefile 
>> b/tools/testing/selftests/gpio/Makefile
>> index 205e4d1..4f6d9e0 100644
>> --- a/tools/testing/selftests/gpio/Makefile
>> +++ b/tools/testing/selftests/gpio/Makefile
>> @@ -7,8 +7,9 @@ include ../lib.mk
>>  
>>  all: $(BINARIES)
>>  
>> -clean:
>> +override define CLEAN
>>  $(RM) $(BINARIES)
>> +endef
> 
> This could be achieved more simply with:
> 
> EXTRA_CLEAN := $(BINARIES)
> 

gpio clean requires special handling. I have one more patch
I sent out that handles that. So I am going to leave this
patch the way with override.

thanks,
-- Shuah

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

[Cédric Le Goater]

On 04/25/2017 04:28 PM, Cédric Le Goater wrote:
> On 04/22/2017 08:11 AM, Michael Ellerman wrote:
>> Shilpasri G Bhat  writes:
>>> On 04/21/2017 05:17 PM, Cédric Le Goater wrote:
 On 04/21/2017 06:31 AM, Shilpasri G Bhat wrote:
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> index 6d2e660..1f329fa8 100644
> --- a/drivers/hwmon/ibmpowernv.c
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -65,7 +66,8 @@ enum sensors {
>   {"fan", "ibm,opal-sensor-cooling-fan"},
>   {"temp", "ibm,opal-sensor-amb-temp"},
>   {"in", "ibm,opal-sensor-power-supply"},
> - {"power", "ibm,opal-sensor-power"}
> + {"power", "ibm,opal-sensor-power"},
> + {"curr"}, /* Follows newer device tree compatible ibm,opal-sensor */

 why isn't there a compatible string ? 
>>>
>>> Skiboot exports "ibm,opal-sensor" as compatible string for all the inband
>>> sensors. Now if I define that as the compatible string here, then all the
>>> sensors would get identified as "curr" type of sensor by the driver.
>>
>> So fix it in skiboot?
> 
> 
> After a memory refresh, this table is to maintain compatibility with 
> the support of the FSP sensors in old firmware. These have peculiar
> device node names and properties.  
> 
> So What the code is doing looks correct. But, you should also modify 
> the 'enum sensors' to include a CURRENT value.

But the patch does that already. I was missing context. This is fine
then.

Thanks,

C.

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

[Cédric Le Goater]

...

> + sdata[count].id = sensor_id;
> + sdata[count].type = type;
> + sdata[count].hwmon_index = sdata[count - 1].hwmon_index;
> + create_hwmon_attr([count], attr_name,
> +   show_sensor);
> + pgroups[type]->attrs[sensor_groups[type].attr_count++] =
> + [count++].dev_attr.attr;
> + }

We are duplicating these lines at least three times. I wonder if we 
could make a routine for them. Don't bother doing so if the number
of arguments is too large.

Thanks,

C.

[REBASED PATCH v4 1/2] powerpc: split ftrace bits into a separate file

[Naveen N. Rao]

entry_*.S now includes a lot more than just kernel entry/exit code. As a
first step at cleaning this up, let's split out the ftrace bits into
separate files. Also move all related tracing code into a new trace/
subdirectory.

No functional changes.

Suggested-by: Michael Ellerman 
Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/Makefile  |   9 +-
 arch/powerpc/kernel/entry_32.S| 107 ---
 arch/powerpc/kernel/entry_64.S| 378 -
 arch/powerpc/kernel/trace/Makefile|  24 ++
 arch/powerpc/kernel/{ => trace}/ftrace.c  |   0
 arch/powerpc/kernel/trace/ftrace_32.S | 118 
 arch/powerpc/kernel/trace/ftrace_64.S | 389 ++
 arch/powerpc/kernel/{ => trace}/trace_clock.c |   0
 8 files changed, 532 insertions(+), 493 deletions(-)
 create mode 100644 arch/powerpc/kernel/trace/Makefile
 rename arch/powerpc/kernel/{ => trace}/ftrace.c (100%)
 create mode 100644 arch/powerpc/kernel/trace/ftrace_32.S
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64.S
 rename arch/powerpc/kernel/{ => trace}/trace_clock.c (100%)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 3e461637b64d..b9db46ae545b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -25,8 +25,6 @@ CFLAGS_REMOVE_cputable.o = -mno-sched-epilog 
$(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_prom_init.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_btext.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_prom.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
-# do not trace tracer code
-CFLAGS_REMOVE_ftrace.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 # timers used by tracing
 CFLAGS_REMOVE_time.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 endif
@@ -119,10 +117,7 @@ obj64-$(CONFIG_AUDIT)  += compat_audit.o
 
 obj-$(CONFIG_PPC_IO_WORKAROUNDS)   += io-workarounds.o
 
-obj-$(CONFIG_DYNAMIC_FTRACE)   += ftrace.o
-obj-$(CONFIG_FUNCTION_GRAPH_TRACER)+= ftrace.o
-obj-$(CONFIG_FTRACE_SYSCALLS)  += ftrace.o
-obj-$(CONFIG_TRACING)  += trace_clock.o
+obj-y  += trace/
 
 ifneq ($(CONFIG_PPC_INDIRECT_PIO),y)
 obj-y  += iomap.o
@@ -143,8 +138,6 @@ obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
 # Disable GCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
 UBSAN_SANITIZE_prom_init.o := n
-GCOV_PROFILE_ftrace.o := n
-UBSAN_SANITIZE_ftrace.o := n
 GCOV_PROFILE_machine_kexec_64.o := n
 UBSAN_SANITIZE_machine_kexec_64.o := n
 GCOV_PROFILE_machine_kexec_32.o := n
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index a38600949f3a..8587059ad848 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -31,7 +31,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -1315,109 +1314,3 @@ machine_check_in_rtas:
/* XXX load up BATs and panic */
 
 #endif /* CONFIG_PPC_RTAS */
-
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-   /*
-* It is required that _mcount on PPC32 must preserve the
-* link register. But we have r0 to play with. We use r0
-* to push the return address back to the caller of mcount
-* into the ctr register, restore the link register and
-* then jump back using the ctr register.
-*/
-   mflrr0
-   mtctr   r0
-   lwz r0, 4(r1)
-   mtlrr0
-   bctr
-
-_GLOBAL(ftrace_caller)
-   MCOUNT_SAVE_FRAME
-   /* r3 ends up with link register */
-   subir3, r3, MCOUNT_INSN_SIZE
-.globl ftrace_call
-ftrace_call:
-   bl  ftrace_stub
-   nop
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-   b   ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
-   MCOUNT_RESTORE_FRAME
-   /* old link register ends up in ctr reg */
-   bctr
-#else
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-
-   MCOUNT_SAVE_FRAME
-
-   subir3, r3, MCOUNT_INSN_SIZE
-   LOAD_REG_ADDR(r5, ftrace_trace_function)
-   lwz r5,0(r5)
-
-   mtctr   r5
-   bctrl
-   nop
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-   b   ftrace_graph_caller
-#endif
-   MCOUNT_RESTORE_FRAME
-   bctr
-#endif
-EXPORT_SYMBOL(_mcount)
-
-_GLOBAL(ftrace_stub)
-   blr
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-_GLOBAL(ftrace_graph_caller)
-   /* load r4 with local address */
-   lwz r4, 44(r1)
-   subir4, r4, MCOUNT_INSN_SIZE
-
-   /* Grab the LR out of the caller stack frame */
-   lwz r3,52(r1)
-
-   bl  prepare_ftrace_return
-   nop
-
-/*
- * prepare_ftrace_return gives us the address we divert to.
- * Change the LR in the callers stack frame to this.
- */
-   stw r3,52(r1)
-
-   

[REBASED PATCH v4 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel

[Naveen N. Rao]

Split ftrace_64.S further retaining the core ftrace 64-bit aspects
in ftrace_64.S and moving ftrace_caller() and ftrace_graph_caller() into
separate files based on -mprofile-kernel. The livepatch routines are all
now contained within the mprofile file.

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/trace/Makefile |   5 +
 arch/powerpc/kernel/trace/ftrace_64.S  | 306 +
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 272 ++
 arch/powerpc/kernel/trace/ftrace_64_pg.S   |  68 ++
 4 files changed, 346 insertions(+), 305 deletions(-)
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64_mprofile.S
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64_pg.S

diff --git a/arch/powerpc/kernel/trace/Makefile 
b/arch/powerpc/kernel/trace/Makefile
index 5f5a35254a9b..729dffc5f7bc 100644
--- a/arch/powerpc/kernel/trace/Makefile
+++ b/arch/powerpc/kernel/trace/Makefile
@@ -11,6 +11,11 @@ endif
 
 obj32-$(CONFIG_FUNCTION_TRACER)+= ftrace_32.o
 obj64-$(CONFIG_FUNCTION_TRACER)+= ftrace_64.o
+ifdef CONFIG_MPROFILE_KERNEL
+obj64-$(CONFIG_FUNCTION_TRACER)+= ftrace_64_mprofile.o
+else
+obj64-$(CONFIG_FUNCTION_TRACER)+= ftrace_64_pg.o
+endif
 obj-$(CONFIG_DYNAMIC_FTRACE)   += ftrace.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)+= ftrace.o
 obj-$(CONFIG_FTRACE_SYSCALLS)  += ftrace.o
diff --git a/arch/powerpc/kernel/trace/ftrace_64.S 
b/arch/powerpc/kernel/trace/ftrace_64.S
index 587e7b5c0aff..e5ccea19821e 100644
--- a/arch/powerpc/kernel/trace/ftrace_64.S
+++ b/arch/powerpc/kernel/trace/ftrace_64.S
@@ -23,233 +23,7 @@ EXPORT_SYMBOL(_mcount)
mtlrr0
bctr
 
-#ifndef CC_USING_MPROFILE_KERNEL
-_GLOBAL_TOC(ftrace_caller)
-   /* Taken from output of objdump from lib64/glibc */
-   mflrr3
-   ld  r11, 0(r1)
-   stdur1, -112(r1)
-   std r3, 128(r1)
-   ld  r4, 16(r11)
-   subir3, r3, MCOUNT_INSN_SIZE
-.globl ftrace_call
-ftrace_call:
-   bl  ftrace_stub
-   nop
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-   b   ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
-   ld  r0, 128(r1)
-   mtlrr0
-   addir1, r1, 112
-
-#else /* CC_USING_MPROFILE_KERNEL */
-/*
- *
- * ftrace_caller() is the function that replaces _mcount() when ftrace is
- * active.
- *
- * We arrive here after a function A calls function B, and we are the trace
- * function for B. When we enter r1 points to A's stack frame, B has not yet
- * had a chance to allocate one yet.
- *
- * Additionally r2 may point either to the TOC for A, or B, depending on
- * whether B did a TOC setup sequence before calling us.
- *
- * On entry the LR points back to the _mcount() call site, and r0 holds the
- * saved LR as it was on entry to B, ie. the original return address at the
- * call site in A.
- *
- * Our job is to save the register state into a struct pt_regs (on the stack)
- * and then arrange for the ftrace function to be called.
- */
-_GLOBAL(ftrace_caller)
-   /* Save the original return address in A's stack frame */
-   std r0,LRSAVE(r1)
-
-   /* Create our stack frame + pt_regs */
-   stdur1,-SWITCH_FRAME_SIZE(r1)
-
-   /* Save all gprs to pt_regs */
-   SAVE_8GPRS(0,r1)
-   SAVE_8GPRS(8,r1)
-   SAVE_8GPRS(16,r1)
-   SAVE_8GPRS(24,r1)
-
-   /* Load special regs for save below */
-   mfmsr   r8
-   mfctr   r9
-   mfxer   r10
-   mfcrr11
-
-   /* Get the _mcount() call site out of LR */
-   mflrr7
-   /* Save it as pt_regs->nip */
-   std r7, _NIP(r1)
-   /* Save the read LR in pt_regs->link */
-   std r0, _LINK(r1)
-
-   /* Save callee's TOC in the ABI compliant location */
-   std r2, 24(r1)
-   ld  r2,PACATOC(r13) /* get kernel TOC in r2 */
-
-   addis   r3,r2,function_trace_op@toc@ha
-   addir3,r3,function_trace_op@toc@l
-   ld  r5,0(r3)
-
-#ifdef CONFIG_LIVEPATCH
-   mr  r14,r7  /* remember old NIP */
-#endif
-   /* Calculate ip from nip-4 into r3 for call below */
-   subir3, r7, MCOUNT_INSN_SIZE
-
-   /* Put the original return address in r4 as parent_ip */
-   mr  r4, r0
-
-   /* Save special regs */
-   std r8, _MSR(r1)
-   std r9, _CTR(r1)
-   std r10, _XER(r1)
-   std r11, _CCR(r1)
-
-   /* Load _regs in r6 for call below */
-   addir6, r1 ,STACK_FRAME_OVERHEAD
-
-   /* ftrace_call(r3, r4, r5, r6) */
-.globl ftrace_call
-ftrace_call:
-   bl  ftrace_stub
-   nop
-
-   /* Load ctr with the possibly modified NIP */
-   ld  r3, _NIP(r1)
-   mtctr   r3
-#ifdef CONFIG_LIVEPATCH
-   cmpdr14,r3  /* has NIP been altered? */
-#endif
-
-   /* Restore gprs 

Re: [PATCH 2/8] selftests: lib.mk: define CLEAN macro to allow Makefiles to override clean

[Shuah Khan]

On 04/21/2017 11:38 PM, Michael Ellerman wrote:
> Shuah Khan  writes:
> 
>> Define CLEAN macro to allow Makefiles to override common clean target
>> in lib.mk. This will help fix the following failures:
>>
>> warning: overriding recipe for target 'clean'
>> ../lib.mk:55: warning: ignoring old recipe for target 'clean'
>>
>> Signed-off-by: Shuah Khan 
> 
> Should probably have:
> 
> Fixes: 88baa78d1f31 ("selftests: remove duplicated all and clean target")

Amended the change log to add the above.

> 
> 
> In hindsight I'm not sure moving the clean target into lib.mk was
> the best idea, but anyway it's a bit late to change our mind on that.

Yeah. Moving clean target to lib.mk ended up to be problematic. However,
there are some advantages as well. It will simplify some Makefiles. One
thing that was missed was that not finding the Makefiles that require
overrides. Anyway live and learn.

> 
> This patch is a good solution to fix the warnings.
> 
> Acked-by: Michael Ellerman 
> 

Thanks. I plan to apply the patch with the amended changelog and your
Ack. Please let me know if you want to see v2 with the change sent out.

thanks,
-- Shuah

Re: [PATCH v2 1/2] fadump: reduce memory consumption for capture kernel

[Michal Suchánek]

On Tue, 25 Apr 2017 18:43:11 +0530
Hari Bathini  wrote:

> On Monday 24 April 2017 07:30 PM, Michal Suchánek wrote:
> > On Mon, 24 Apr 2017 18:26:37 +0530
> > Hari Bathini  wrote:
> >  
> >> Hi Michal.
> >>

> >>> 
> >> I thinks there is a mixup here..
> >> I am no longer batting for handover area approach but
> >> "fadump_append=" approach (suggested by Michael Ellerman in this
> >> thread) where there is no need for a handover area but an
> >> additional kernel parameter fadump_append=numa=off,nr_cpus=1..
> >> which is a comma separated list of parameters the user wants to
> >> enforce when fadump is active.
> >>
> >> I was trying to say that passing additional parameters with
> >> 'fadump_append=' kernel parameter is better over f/w changing the
> >> chosen/bootargs node.  
> >
> > Ok, so a fadump specific parameter was just removed in
> > 25031865553e5a2bd9b6e0a5d044952cfa2925d8 and with this we get one
> > back.  
> 
> I think you mean commit c2afb7ce9d088696427018ba2135b898058507b8
> from linux-next. If so, yes.
> 
> > If this is going to be added as an extra kernel parameter can't
> > this be passed to kdump kernel as well? Does kdump not have the same
> > restrictions?
> >  
> 
> kdump kernel is loaded with kexec and additional parameters can be 
> passed to it at the
> time of loading. But fadump kernel boots like a regular kernel
> (through f/w & bootloader,
> resetting all the h/w) except that f/w guarantees to keep the memory 
> intact. So, there is
> a need for a different approach to pass additional parameters in case
> of fadump..
> 

I see, you can pass different commandline to the kdump kernel while
loading it with kexec but fadump kernel is booted same as normal kernel
until it looks at the devicetree so the extra arguments have to be
passed always and only appended to the commandline in the fadump case.

This sounds reasonable.

Thanks

Michal

Re: [PATCH v2 1/2] fadump: reduce memory consumption for capture kernel

[Hari Bathini]

On Monday 24 April 2017 07:30 PM, Michal Suchánek wrote:

On Mon, 24 Apr 2017 18:26:37 +0530
Hari Bathini  wrote:


Hi Michal.

On Monday 24 April 2017 03:54 PM, Michal Suchánek wrote:

On Fri, 21 Apr 2017 00:19:55 +0530
Hari Bathini  wrote:
  

On Wednesday 19 April 2017 07:38 PM, Michal Suchánek wrote:

On Wed, 19 Apr 2017 14:19:47 +1000
Michael Ellerman  wrote:
 

With the fadump_append= approach I posted in this response thread,
additional parameters are enforced when fadump is active. If f/w
supports appending additional parameters, it has to update
chosen/bootargs whenever fadump is active. Almost the same thing
except the dirty job is now done by f/w? Hence I thought
fadump_append= kernel parameter approach is simple and lesser of
the two evils? Am i missing something here..
  

The firmware already has to set the parameter by which the kernel
can tell it is a fadump kernel. Hence it already modifies the
devicetree for fadump and you have to rely on it to do so.

Right, devicetree is modified to include new 'ibm,kernel-dump' rtas
node informing that fadump is active.


The handover area which stores the additional parameters is
reserved by the running kernel so now you also have to rely on the
running kernel, the running kernel and fadum kernel having the same
idea about the location of handover area, the crashing kernel not
randomly overwriting the handover area, etc.

In short it is additional potential for trouble which can be
avoided.

I don't know if the firmware does protect the fadump reserved area
and devicetree from being randomly overwritten by the crashing
kernel but it is in the position to do so if desired. The handover
area is controlled by the crashing kernel and cannot have this
guarantee.

  

I thinks there is a mixup here..
I am no longer batting for handover area approach but
"fadump_append=" approach (suggested by Michael Ellerman in this
thread) where there is no need for a handover area but an additional
kernel parameter fadump_append=numa=off,nr_cpus=1.. which is a comma
separated list of parameters the user wants to enforce when fadump is
active.

I was trying to say that passing additional parameters with
'fadump_append=' kernel parameter is better over f/w changing the
chosen/bootargs node.


Ok, so a fadump specific parameter was just removed in
25031865553e5a2bd9b6e0a5d044952cfa2925d8 and with this we get one back.


I think you mean commit c2afb7ce9d088696427018ba2135b898058507b8
from linux-next. If so, yes.


If this is going to be added as an extra kernel parameter can't this be
passed to kdump kernel as well? Does kdump not have the same
restrictions?



kdump kernel is loaded with kexec and additional parameters can be 
passed to it at the
time of loading. But fadump kernel boots like a regular kernel (through 
f/w & bootloader,
resetting all the h/w) except that f/w guarantees to keep the memory 
intact. So, there is
a need for a different approach to pass additional parameters in case of 
fadump..


Thanks
Hari

[PATCH] powerpc/mm: Fix possible out-of-bounds shift in arch_mmap_rnd()

[Michael Ellerman]

The recent patch to add runtime configuration of the ASLR limits added a bug in
arch_mmap_rnd() where we may shift an integer (32-bits) by up to 33 bits,
leading to undefined behaviour.

In practice it exhibits as every process seg faulting instantly, presumably
because the rnd value hasn't been restricited by the modulus at all. We didn't
notice because it only happens under certain kernel configurations and if the
number of bits is actually set to a large value.

Fix it by switching to unsigned long.

Fixes: 9fea59bd7ca5 ("powerpc/mm: Add support for runtime configuration of ASLR 
limits")
Reported-by: Balbir Singh 
Signed-off-by: Michael Ellerman 
---
 arch/powerpc/mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
index 005aa8a44915..9dbd2a733d6b 100644
--- a/arch/powerpc/mm/mmap.c
+++ b/arch/powerpc/mm/mmap.c
@@ -66,7 +66,7 @@ unsigned long arch_mmap_rnd(void)
if (is_32bit_task())
shift = mmap_rnd_compat_bits;
 #endif
-   rnd = get_random_long() % (1 << shift);
+   rnd = get_random_long() % (1ul << shift);
 
return rnd << PAGE_SHIFT;
 }
-- 
2.7.4

Re: [PATCH] kvm: powerpc: book3s: use local_paca instead of get_paca

[Michael Ellerman]

Denis Kirjanov  writes:

> On 3/29/17, Denis Kirjanov  wrote:
>> with CONFIG_DEBUG_PREEMPT get_paca produces the following warning
>> in kvmppc_book3s_init_hv since we are getting into the
>> smp_processor_id debugging code
>>
>> There is no real issue with the xics_phys field.
>> If paca->kvm_hstate.xics_phys is non-zero on one cpu, it will be
>> non-zero on them all.  Therefore this is not fixing any actual
>> problem, just the warning.
>
> Hey Michael,
>
> are you going to take the patch?

It's in KVM code, so technically Paul should take it.

I'll see if it is likely to conflict with anything in his tree and if
not I'll merge it.

cheers

Re: [PATCH] kvm: powerpc: book3s: use local_paca instead of get_paca

[Denis Kirjanov]

On 3/29/17, Denis Kirjanov  wrote:
> with CONFIG_DEBUG_PREEMPT get_paca produces the following warning
> in kvmppc_book3s_init_hv since we are getting into the
> smp_processor_id debugging code
>
> There is no real issue with the xics_phys field.
> If paca->kvm_hstate.xics_phys is non-zero on one cpu, it will be
> non-zero on them all.  Therefore this is not fixing any actual
> problem, just the warning.

Hey Michael,

are you going to take the patch?

Thanks!
>
> [  138.521188] BUG: using smp_processor_id() in preemptible [] code:
> modprobe/5596
> [  138.521308] caller is .kvmppc_book3s_init_hv+0x184/0x350 [kvm_hv]
> [  138.521404] CPU: 5 PID: 5596 Comm: modprobe Not tainted
> 4.11.0-rc3-00022-gc7e790c #1
> [  138.521509] Call Trace:
> [  138.521563] [c007d018b810] [c23eef10] .dump_stack+0xe4/0x150
> (unreliable)
> [  138.521694] [c007d018b8a0] [c1f6ec04]
> .check_preemption_disabled+0x134/0x150
> [  138.521829] [c007d018b940] [da010274]
> .kvmppc_book3s_init_hv+0x184/0x350 [kvm_hv]
> [  138.521963] [c007d018ba00] [c191d5cc]
> .do_one_initcall+0x5c/0x1c0
> [  138.522082] [c007d018bad0] [c23e9494]
> .do_init_module+0x84/0x240
> [  138.522201] [c007d018bb70] [c1aade18]
> .load_module+0x1f68/0x2a10
> [  138.522319] [c007d018bd20] [c1aaeb30]
> .SyS_finit_module+0xc0/0xf0
> [  138.522439] [c007d018be30] [c191baec] system_call+0x38/0xfc
>
> Signed-off-by: Denis Kirjanov 
> ---
>  arch/powerpc/kvm/book3s_hv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 1ec86d9..cb4ff36 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3930,7 +3930,7 @@ static int kvmppc_book3s_init_hv(void)
>* indirectly, via OPAL.
>*/
>  #ifdef CONFIG_SMP
> - if (!get_paca()->kvm_hstate.xics_phys) {
> + if (!local_paca->kvm_hstate.xics_phys) {
>   struct device_node *np;
>
>   np = of_find_compatible_node(NULL, NULL, "ibm,opal-intc");
> --
> 1.8.3.1
>
>

Re: [PATCH v3] KVM: PPC: Book3S PR: Do not fail emulation with mtspr/mfspr for unknown SPRs

[Thomas Huth]

On 05.04.2017 15:58, Thomas Huth wrote:
> According to the PowerISA 2.07, mtspr and mfspr should not always
> generate an illegal instruction exception when being used with an
> undefined SPR, but rather treat the instruction as a NOP or inject a
> privilege exception in some cases, too - depending on the SPR number.
> Also turn the printk here into a ratelimited print statement, so that
> the guest can not flood the dmesg log of the host by issueing lots of
> illegal mtspr/mfspr instruction here.
> 
> Signed-off-by: Thomas Huth 
> ---
>  v3:
>  - Make sure that we do not advance the program counter after we've
>already changed it due to injecting a program interrupt
> 
>  v2:
>  - Inject illegal instruction program interrupt instead of emulation
>assist interrupt (according to the last programming note in section
>6.5.9 of Book III of the PowerISA v2.07)
> 
>  arch/powerpc/kvm/book3s_emulate.c | 34 ++
>  arch/powerpc/kvm/emulate.c|  8 
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_emulate.c 
> b/arch/powerpc/kvm/book3s_emulate.c
> index 8359752..68d6898 100644
> --- a/arch/powerpc/kvm/book3s_emulate.c
> +++ b/arch/powerpc/kvm/book3s_emulate.c
> @@ -503,10 +503,18 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, 
> int sprn, ulong spr_val)
>   break;
>  unprivileged:
>   default:
> - printk(KERN_INFO "KVM: invalid SPR write: %d\n", sprn);
> -#ifndef DEBUG_SPR
> - emulated = EMULATE_FAIL;
> -#endif
> + pr_info_ratelimited("KVM: invalid SPR write: %d\n", sprn);
> + if (sprn & 0x10) {
> + if (kvmppc_get_msr(vcpu) & MSR_PR) {
> + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV);
> + emulated = EMULATE_AGAIN;
> + }
> + } else {
> + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0) {
> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> + emulated = EMULATE_AGAIN;
> + }
> + }
>   break;
>   }
>  
> @@ -648,10 +656,20 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, 
> int sprn, ulong *spr_val
>   break;
>   default:
>  unprivileged:
> - printk(KERN_INFO "KVM: invalid SPR read: %d\n", sprn);
> -#ifndef DEBUG_SPR
> - emulated = EMULATE_FAIL;
> -#endif
> + pr_info_ratelimited("KVM: invalid SPR read: %d\n", sprn);
> + if (sprn & 0x10) {
> + if (kvmppc_get_msr(vcpu) & MSR_PR) {
> + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV);
> + emulated = EMULATE_AGAIN;
> + }
> + } else {
> + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0 ||
> + sprn == 4 || sprn == 5 || sprn == 6) {
> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> + emulated = EMULATE_AGAIN;
> + }
> + }
> +
>   break;
>   }
>  
> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> index b379146..c873ffe 100644
> --- a/arch/powerpc/kvm/emulate.c
> +++ b/arch/powerpc/kvm/emulate.c
> @@ -259,10 +259,18 @@ int kvmppc_emulate_instruction(struct kvm_run *run, 
> struct kvm_vcpu *vcpu)
>  
>   case OP_31_XOP_MFSPR:
>   emulated = kvmppc_emulate_mfspr(vcpu, sprn, rt);
> + if (emulated == EMULATE_AGAIN) {
> + emulated = EMULATE_DONE;
> + advance = 0;
> + }
>   break;
>  
>   case OP_31_XOP_MTSPR:
>   emulated = kvmppc_emulate_mtspr(vcpu, sprn, rs);
> + if (emulated == EMULATE_AGAIN) {
> + emulated = EMULATE_DONE;
> + advance = 0;
> + }
>   break;
>  
>   case OP_31_XOP_TLBSYNC:
> 

Ping!

Paul, does this now look OK for you this way?

 Thomas