[PATCH] KVM: PPC: Book3S HV: Branch inside feature section

[Alexander Graf]

We ended up with code that did a conditional branch inside a feature
section to code outside of the feature section. Depending on how the
object file gets organized, that might mean we exceed the 14bit
relocation limit for conditional branches:

  
arch/powerpc/kvm/built-in.o:arch/powerpc/kvm/book3s_hv_rmhandlers.S:416:(__ftr_alt_97+0x8):
 relocation truncated to fit: R_PPC64_REL14 against `.text'+1ca4

So instead of doing a conditional branch outside of the feature section,
let's just jump at the end of the same, making the branch very short.

Signed-off-by: Alexander Graf <ag...@suse.de>
---
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 9c61f736c75b..1e037326897b 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -413,10 +413,11 @@ FTR_SECTION_ELSE
/* On P9 we use the split_info for coordinating LPCR changes */
lwz r4, KVM_SPLIT_DO_SET(r6)
cmpwi   r4, 0
-   beq 63f
+   beq 1f
mr  r3, r6
bl  kvmhv_p9_set_lpcr
nop
+1:
 ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_300)
 63:
/* Order load of vcpu after load of vcore */
-- 
2.12.3

[PATCH] KVM: PPC: Fix svcpu copying with preemption enabled

[Alexander Graf]

When copying between the vcpu and svcpu, we may get scheduled away onto
a different host CPU which in turn means our svcpu pointer may change.

That means we need to atomically copy to and from the svcpu with preemption
disabled, so that all code around it always sees a coherent state.

Reported-by: Simon Guo <wei.guo.si...@gmail.com>
Fixes: 3d3319b45eea ("KVM: PPC: Book3S: PR: Enable interrupts earlier")
Signed-off-by: Alexander Graf <ag...@suse.de>
---
 arch/powerpc/include/asm/kvm_book3s.h |  6 ++
 arch/powerpc/kvm/book3s_interrupts.S  |  4 +---
 arch/powerpc/kvm/book3s_pr.c  | 20 +---
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
b/arch/powerpc/include/asm/kvm_book3s.h
index 9a667007bff8..376ae803b69c 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -249,10 +249,8 @@ extern int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned 
long cmd);
 extern void kvmppc_pr_init_default_hcalls(struct kvm *kvm);
 extern int kvmppc_hcall_impl_pr(unsigned long cmd);
 extern int kvmppc_hcall_impl_hv_realmode(unsigned long cmd);
-extern void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
-struct kvm_vcpu *vcpu);
-extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
-  struct kvmppc_book3s_shadow_vcpu *svcpu);
+extern void kvmppc_copy_to_svcpu(struct kvm_vcpu *vcpu);
+extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu);
 extern int kvm_irq_bypass;
 
 static inline struct kvmppc_vcpu_book3s *to_book3s(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_interrupts.S 
b/arch/powerpc/kvm/book3s_interrupts.S
index 901e6fe00c39..c18e845019ec 100644
--- a/arch/powerpc/kvm/book3s_interrupts.S
+++ b/arch/powerpc/kvm/book3s_interrupts.S
@@ -96,7 +96,7 @@ kvm_start_entry:
 
 kvm_start_lightweight:
/* Copy registers into shadow vcpu so we can access them in real mode */
-   GET_SHADOW_VCPU(r3)
+   mr  r3, r4
bl  FUNC(kvmppc_copy_to_svcpu)
nop
REST_GPR(4, r1)
@@ -165,9 +165,7 @@ after_sprg3_load:
stw r12, VCPU_TRAP(r3)
 
/* Transfer reg values from shadow vcpu back to vcpu struct */
-   /* On 64-bit, interrupts are still off at this point */
 
-   GET_SHADOW_VCPU(r4)
bl  FUNC(kvmppc_copy_from_svcpu)
nop
 
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 7deaeeb14b93..3ae752314b34 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -121,7 +121,7 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
 #ifdef CONFIG_PPC_BOOK3S_64
struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
if (svcpu->in_use) {
-   kvmppc_copy_from_svcpu(vcpu, svcpu);
+   kvmppc_copy_from_svcpu(vcpu);
}
memcpy(to_book3s(vcpu)->slb_shadow, svcpu->slb, sizeof(svcpu->slb));
to_book3s(vcpu)->slb_shadow_max = svcpu->slb_max;
@@ -143,9 +143,10 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
 }
 
 /* Copy data needed by real-mode code from vcpu to shadow vcpu */
-void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
- struct kvm_vcpu *vcpu)
+void kvmppc_copy_to_svcpu(struct kvm_vcpu *vcpu)
 {
+   struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
+
svcpu->gpr[0] = vcpu->arch.gpr[0];
svcpu->gpr[1] = vcpu->arch.gpr[1];
svcpu->gpr[2] = vcpu->arch.gpr[2];
@@ -177,17 +178,14 @@ void kvmppc_copy_to_svcpu(struct 
kvmppc_book3s_shadow_vcpu *svcpu,
if (cpu_has_feature(CPU_FTR_ARCH_207S))
vcpu->arch.entry_ic = mfspr(SPRN_IC);
svcpu->in_use = true;
+
+   svcpu_put(svcpu);
 }
 
 /* Copy data touched by real-mode code from shadow vcpu back to vcpu */
-void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
-   struct kvmppc_book3s_shadow_vcpu *svcpu)
+void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu)
 {
-   /*
-* vcpu_put would just call us again because in_use hasn't
-* been updated yet.
-*/
-   preempt_disable();
+   struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
 
/*
 * Maybe we were already preempted and synced the svcpu from
@@ -233,7 +231,7 @@ void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
svcpu->in_use = false;
 
 out:
-   preempt_enable();
+   svcpu_put(svcpu);
 }
 
 static int kvmppc_core_check_requests_pr(struct kvm_vcpu *vcpu)
-- 
2.12.3

Re: [PATCH] KVM: PPC: Book3S PR: close a race window when SVCPU pointer is hold before kvmppc_copy_from_svcpu()

[Alexander Graf]

On 31.01.18 05:23, wei.guo.si...@gmail.com wrote:
> From: Simon Guo 
> 
> commit 40fdd8c88c4a ("KVM: PPC: Book3S: PR: Make svcpu -> vcpu store
> preempt savvy") and commit 3d3319b45eea ("KVM: PPC: Book3S: PR: Enable
> interrupts earlier") is trying to turns on preemption early when
> return into highmem guest exit handler.
> 
> However there is a race window in following example at
> arch/powerpc/kvm/book3s_interrupts.S:
> 
> highmem guest exit handler:
> ...
> 195 GET_SHADOW_VCPU(r4)
> 196 bl  FUNC(kvmppc_copy_from_svcpu)
> ...
> 239 bl  FUNC(kvmppc_handle_exit_pr)
> 
> If there comes a preemption between line 195 and 196, line 196
> may hold an invalid SVCPU reference with following sequence:
> 1) Qemu task T1 runs at GET_SHADOW_VCPU(r4) at line 195, on CPU A.
> 2) T1 is preempted and switch out CPU A. As a result, it checks
> CPU A's svcpu->in_use (=1 at present) and flush cpu A's svcpu to
> T1's vcpu.
> 3) Another task T2 switches into CPU A and it may update CPU A's
> svcpu->in_use into 1.
> 4) T1 is scheduled into CPU B. But it still holds CPU A's svcpu
> reference as R4. Then it executes kvmppc_copy_from_svcpu() with
> R4 and it will corrupt T1's VCPU with T2's content. T2's VCPU
> will also be impacted.
> 
> This patch moves the svcpu->in_use into VCPU so that the vcpus
> sharing the same svcpu can work properly and fix the above case.
> 
> Signed-off-by: Simon Guo 

Sorry, the previous version would only compile on 32bit PPC ;). Please
find the fixed one which just uses svcpu_get() and _put() here:


https://github.com/agraf/linux-2.6/commit/f9e3ca44c9a9d4930d6dccaacb518734746059c3


Alex

Re: [PATCH] KVM: PPC: Book3S PR: close a race window when SVCPU pointer is hold before kvmppc_copy_from_svcpu()

[Alexander Graf]

On 31.01.18 05:23, wei.guo.si...@gmail.com wrote:
> From: Simon Guo <wei.guo.si...@gmail.com>
> 
> commit 40fdd8c88c4a ("KVM: PPC: Book3S: PR: Make svcpu -> vcpu store
> preempt savvy") and commit 3d3319b45eea ("KVM: PPC: Book3S: PR: Enable
> interrupts earlier") is trying to turns on preemption early when
> return into highmem guest exit handler.
> 
> However there is a race window in following example at
> arch/powerpc/kvm/book3s_interrupts.S:
> 
> highmem guest exit handler:
> ...
> 195 GET_SHADOW_VCPU(r4)
> 196 bl  FUNC(kvmppc_copy_from_svcpu)
> ...
> 239 bl  FUNC(kvmppc_handle_exit_pr)
> 
> If there comes a preemption between line 195 and 196, line 196
> may hold an invalid SVCPU reference with following sequence:
> 1) Qemu task T1 runs at GET_SHADOW_VCPU(r4) at line 195, on CPU A.
> 2) T1 is preempted and switch out CPU A. As a result, it checks
> CPU A's svcpu->in_use (=1 at present) and flush cpu A's svcpu to
> T1's vcpu.
> 3) Another task T2 switches into CPU A and it may update CPU A's
> svcpu->in_use into 1.
> 4) T1 is scheduled into CPU B. But it still holds CPU A's svcpu
> reference as R4. Then it executes kvmppc_copy_from_svcpu() with
> R4 and it will corrupt T1's VCPU with T2's content. T2's VCPU
> will also be impacted.
> 
> This patch moves the svcpu->in_use into VCPU so that the vcpus
> sharing the same svcpu can work properly and fix the above case.
> 
> Signed-off-by: Simon Guo <wei.guo.si...@gmail.com>

To me the description above sounds like what we really need to do is
move the preempt_disable() before GET_SHADOW_VCPU(). Or alternative put
GET_SHADOW_VCPU into kvmppc_copy_from_svcpu().

I guess along those lines we should also update the comment above
GET_SHADOW_VCPU() that says interrupts are disabled ;).

I would really like to keep the fact that the svcpu is currently used as
close to the svcpu as possible.

Does the untested patch below work for you? Because TB breaks
whitespace, I've also uploaded it to github:


https://github.com/agraf/linux-2.6/commit/2469804ed37b41167e9d97f4d72da24cdcb9f959


Alex


From: Alexander Graf <ag...@suse.de>
Date: Wed, 31 Jan 2018 09:48:09 +0100
Subject: [PATCH] KVM: PPC: Fix svcpu copying with preemption enabled

When copying between the vcpu and svcpu, we may get scheduled away onto
a different host CPU which in turn means our svcpu pointer may change.

That means we need to atomically copy to and from the svcpu with preemption
disabled, so that all code around it always sees a coherent state.

Reported-by: Simon Guo <wei.guo.si...@gmail.com>
Fixes: 3d3319b45eea ("KVM: PPC: Book3S: PR: Enable interrupts earlier")
Signed-off-by: Alexander Graf <ag...@suse.de>

diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index 9a667007bff8..376ae803b69c 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -249,10 +249,8 @@ extern int kvmppc_h_pr(struct kvm_vcpu *vcpu,
unsigned long cmd);
 extern void kvmppc_pr_init_default_hcalls(struct kvm *kvm);
 extern int kvmppc_hcall_impl_pr(unsigned long cmd);
 extern int kvmppc_hcall_impl_hv_realmode(unsigned long cmd);
-extern void kvmppc_copy_to_svcpu(struct kvmppc_book3s_shadow_vcpu *svcpu,
-struct kvm_vcpu *vcpu);
-extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu,
-  struct kvmppc_book3s_shadow_vcpu *svcpu);
+extern void kvmppc_copy_to_svcpu(struct kvm_vcpu *vcpu);
+extern void kvmppc_copy_from_svcpu(struct kvm_vcpu *vcpu);
 extern int kvm_irq_bypass;

 static inline struct kvmppc_vcpu_book3s *to_book3s(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_interrupts.S
b/arch/powerpc/kvm/book3s_interrupts.S
index 901e6fe00c39..c18e845019ec 100644
--- a/arch/powerpc/kvm/book3s_interrupts.S
+++ b/arch/powerpc/kvm/book3s_interrupts.S
@@ -96,7 +96,7 @@ kvm_start_entry:

 kvm_start_lightweight:
/* Copy registers into shadow vcpu so we can access them in real mode */
-   GET_SHADOW_VCPU(r3)
+   mr  r3, r4
bl  FUNC(kvmppc_copy_to_svcpu)
nop
REST_GPR(4, r1)
@@ -165,9 +165,7 @@ after_sprg3_load:
stw r12, VCPU_TRAP(r3)

/* Transfer reg values from shadow vcpu back to vcpu struct */
-   /* On 64-bit, interrupts are still off at this point */

-   GET_SHADOW_VCPU(r4)
bl  FUNC(kvmppc_copy_from_svcpu)
nop

diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 7deaeeb14b93..f5c7797a38d7 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -121,7 +121,7 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu
*vcpu)
 #ifdef CONFIG_PPC_BOOK3S_64
struct kvmppc_book3s_shadow_vcpu *svcpu = svcpu_get(vcpu);
  

[PATCH] KVM: PPC: Book3S HV: Remove vcpu->arch.dec usage

[Alexander Graf]

On Book3S in HV mode, we don't use the vcpu->arch.dec field at all.
Instead, all logic is built around vcpu->arch.dec_expires.

So let's remove the one remaining piece of code that was setting it.

Signed-off-by: Alexander Graf <ag...@suse.de>

---

Looking through the DEC logic, I fail to see any code that allows
save or restore of DEC. Do we maybe miss out on that register for
(live) migration?
---
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 2659844784b8..c8ffd69adfec 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -957,7 +957,6 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_300)
mftbr7
subfr3,r7,r8
mtspr   SPRN_DEC,r3
-   std r3,VCPU_DEC(r4)
 
ld  r5, VCPU_SPRG0(r4)
ld  r6, VCPU_SPRG1(r4)
-- 
2.12.3

Re: [PATCH v2 2/2] powerpc/book3s: Fix TB corruption in guest exit path on HMI interrupt.

[Alexander Graf]

On 05/15/2016 06:14 AM, Mahesh J Salgaonkar wrote:

From: Mahesh Salgaonkar 

When a guest is assigned to a core it converts the host Timebase (TB)
into guest TB by adding guest timebase offset before entering into
guest. During guest exit it restores the guest TB to host TB. This means
under certain conditions (Guest migration) host TB and guest TB can differ.

When we get an HMI for TB related issues the opal HMI handler would
try fixing errors and restore the correct host TB value. With no guest
running, we don't have any issues. But with guest running on the core
we run into TB corruption issues.

If we get an HMI while in the guest, the current HMI handler invokes opal
hmi handler before forcing guest to exit. The guest exit path subtracts
the guest TB offset from the current TB value which may have already
been restored with host value by opal hmi handler. This leads to incorrect
host and guest TB values.

With split-core, things become more complex. With split-core, TB also gets
split and each subcore gets its own TB register. When a hmi handler fixes
a TB error and restores the TB value, it affects all the TB values of
sibling subcores on the same core. On TB errors all the thread in the core
gets HMI. With existing code, the individual threads call opal hmi handle
independently which can easily throw TB out of sync if we have guest
running on subcores. Hence we will need to co-ordinate with all the
threads before making opal hmi handler call followed by TB resync.

This patch introduces a sibling subcore state structure (shared by all
threads in the core) in paca which holds information about whether sibling
subcores are in Guest mode or host mode. An array in_guest[] of size
MAX_SUBCORE_PER_CORE=4 is used to maintain the state of each subcore.
The subcore id is used as index into in_guest[] array. Only primary
thread entering/exiting the guest is responsible to set/unset its
designated array element.

On TB error, we get HMI interrupt on every thread on the core. Upon HMI,
this patch will now force guest to vacate the core/subcore. Primary
thread from each subcore will then turn off its respective bit
from the above bitmap during the guest exit path just after the
guest->host partition switch is complete.

All other threads that have just exited the guest OR were already in host
will wait until all other subcores clears their respective bit.
Once all the subcores turn off their respective bit, all threads will
will make call to opal hmi handler.

It is not necessary that opal hmi handler would resync the TB value for
every HMI interrupts. It would do so only for the HMI caused due to
TB errors. For rest, it would not touch TB value. Hence to make things
simpler, primary thread would call TB resync explicitly once for each
core immediately after opal hmi handler instead of subtracting guest
offset from TB. TB resync call will restore the TB with host value.
Thus we can be sure about the TB state.

One of the primary threads exiting the guest will take up the
responsibility of calling TB resync. It will use one of the top bits
(bit 63) from subcore state flags bitmap to make the decision. The first
primary thread (among the subcores) that is able to set the bit will
have to call the TB resync. Rest all other threads will wait until TB
resync is complete.  Once TB resync is complete all threads will then
proceed.

Signed-off-by: Mahesh Salgaonkar 
---
  arch/powerpc/include/asm/hmi.h  |   45 
  arch/powerpc/include/asm/paca.h |6 +
  arch/powerpc/kernel/Makefile|2
  arch/powerpc/kernel/exceptions-64s.S|4 +
  arch/powerpc/kernel/hmi.c   |   56 ++
  arch/powerpc/kernel/idle_power7.S   |5 +
  arch/powerpc/kernel/traps.c |5 +
  arch/powerpc/kvm/book3s_hv.c|   37 +++
  arch/powerpc/kvm/book3s_hv_ras.c|  176 +++
  arch/powerpc/kvm/book3s_hv_rmhandlers.S |   65 +++
  10 files changed, 396 insertions(+), 5 deletions(-)
  create mode 100644 arch/powerpc/include/asm/hmi.h
  create mode 100644 arch/powerpc/kernel/hmi.c



[...]


diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index e571ad2..0d246fc 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #define VCPU_GPRS_TM(reg) (((reg) * ULONG_SIZE) + VCPU_GPR_TM)
  
@@ -373,6 +374,18 @@ kvm_secondary_got_guest:

lwsync
std r0, HSTATE_KVM_VCORE(r13)
  
+	/*

+* All secondaries exiting guest will fall through this path.
+* Before proceeding, just check for HMI interrupt and
+* invoke opal hmi handler. By now we are sure that the
+* primary thread on this core/subcore has already made partition
+* switch/TB resync and we are good to 

Re: [RFC PATCH kernel] KVM: PPC: Book3S PR: Fix WIMG handling under pHyp

[Alexander Graf]

On 24.11.17 00:33, Greg Kurz wrote:
> On Wed, 22 Nov 2017 14:42:21 +1100
> Alexey Kardashevskiy  wrote:
> 
>> 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits" added WIMG
>> bits preserving but it missed 2 special cases:
>> - a magic page in kvmppc_mmu_book3s_64_xlate() and
>> - guest real mode in kvmppc_handle_pagefault().
>>
>> For these ptes WIMG were 0 and pHyp failed on these causing a guest to
>> stop in the very beginning at NIP=0x100 (due to bd9166ffe
>> "KVM: PPC: Book3S PR: Exit KVM on failed mapping").
>>
>> This initializes WIMG to non-zero value HPTE_R_M. The value is chosen
>> as (0x192 & HPTE_R_WIMG); 0x192 is a magic value from
>> kvmppc_mmu_map_page().
>>
>> Fixes: 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits"
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>
>> This indeed fixes PR KVM + VFIO under pHyp but selection of HPTE_R_M
>> is arguable.
>>
> 
> The initial page fault at 0x100 on machine startup causes H_ENTER to fail in
> pHyp with H_PARAMETER, as described in PAPR:
> 
>  The hypervisor checks that the WIMG bits within the PTE are appropriate for 
> the
>  physical page number else H_Parameter return. (For System Memory pages 
> WIMG=0010,
>  or, 1110 if the SAO option is enabled, and for IO pages WIMG=01**.)
> 
> I'm not aware we care for SAO so HPTE_R_M looks like the only sensible choice,
> or I'm missing something ?
> 
> Anyway, this patch allows to start a PR guest on PowerVM again.
> 
> Reviewed-by: Greg Kurz 
> 
> and
> 
> Tested-by: Greg Kurz 

So doesn't that mean we should pick it up, mark it CC stable and push it
upstream?


Alex

Re: [RFC PATCH kernel] KVM: PPC: Book3S PR: Fix WIMG handling under pHyp

[Alexander Graf]

On 22.11.17 04:42, Alexey Kardashevskiy wrote:
> 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits" added WIMG
> bits preserving but it missed 2 special cases:
> - a magic page in kvmppc_mmu_book3s_64_xlate() and
> - guest real mode in kvmppc_handle_pagefault().
> 
> For these ptes WIMG were 0 and pHyp failed on these causing a guest to
> stop in the very beginning at NIP=0x100 (due to bd9166ffe
> "KVM: PPC: Book3S PR: Exit KVM on failed mapping").
> 
> This initializes WIMG to non-zero value HPTE_R_M. The value is chosen
> as (0x192 & HPTE_R_WIMG); 0x192 is a magic value from
> kvmppc_mmu_map_page().
> 
> Fixes: 96df226 "KVM: PPC: Book3S PR: Preserve storage control bits"
> Signed-off-by: Alexey Kardashevskiy 
> ---
> 
> This indeed fixes PR KVM + VFIO under pHyp but selection of HPTE_R_M
> is arguable.

This does indeed fix the breakage we've seen:

Tested-by: Ruediger Oertel 


Alex

Re: [patch] KVM: PPC: fix a sanity check

[Alexander Graf]

On 07/14/2016 12:15 PM, Dan Carpenter wrote:

We use logical negate where bitwise negate was intended.  It means that
we never return -EINVAL here.

Fixes: ce11e48b7fdd ('KVM: PPC: E500: Add userspace debug stub support')
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>


Oops :).

Reviewed-by: Alexander Graf <ag...@suse.de>


Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] kvm-pr: manage illegal instructions

[Alexander Graf]

On 05/17/2016 10:35 AM, Laurent Vivier wrote:


On 12/05/2016 16:23, Laurent Vivier wrote:


On 12/05/2016 11:27, Alexander Graf wrote:

On 05/12/2016 11:10 AM, Laurent Vivier wrote:

On 11/05/2016 13:49, Alexander Graf wrote:

On 05/11/2016 01:14 PM, Laurent Vivier wrote:

On 11/05/2016 12:35, Alexander Graf wrote:

On 03/15/2016 09:18 PM, Laurent Vivier wrote:

While writing some instruction tests for kvm-unit-tests for powerpc,
I've found that illegal instructions are not managed correctly with
kvm-pr,
while it is fine with kvm-hv.

When an illegal instruction (like ".long 0") is processed by kvm-pr,
the kernel logs are filled with:

 Couldn't emulate instruction 0x (op 0 xop 0)
 kvmppc_handle_exit_pr: emulation at 700 failed ()

While the exception handler receives an interrupt for each
instruction
executed after the illegal instruction.

Signed-off-by: Laurent Vivier <lviv...@redhat.com>
---
 arch/powerpc/kvm/book3s_emulate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_emulate.c
b/arch/powerpc/kvm/book3s_emulate.c
index 2afdb9c..4ee969d 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -99,7 +99,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run,
struct kvm_vcpu *vcpu,
   switch (get_op(inst)) {
 case 0:
-emulated = EMULATE_FAIL;
 if ((kvmppc_get_msr(vcpu) & MSR_LE) &&
 (inst == swab32(inst_sc))) {
 /*
@@ -112,6 +111,9 @@ int kvmppc_core_emulate_op_pr(struct kvm_run
*run,
struct kvm_vcpu *vcpu,
 kvmppc_set_gpr(vcpu, 3, EV_UNIMPLEMENTED);
 kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
 emulated = EMULATE_DONE;
+} else {
+kvmppc_core_queue_program(vcpu, SRR1_PROGILL);

But isn't that exactly what the semantic of EMULATE_FAIL is? Fixing it
up in book3s_emulate.c is definitely the wrong spot.

So what is the problem you're trying to solve? Is the SRR0 at the
wrong
spot or are the log messages the problem?

No, the problem is the host kernel logs are filled by the message and
the execution hangs. And the host becomes unresponsiveness, even after
the end of the tests.

Please, try to run kvm-unit-tests (the emulator test) on a KVM-PR host,
and check the kernel logs (dmesg), then try to ssh to the host...

Ok, so the log messages are the problem. Please fix the message output
then - or remove it altogether. Or if you like, create a module
parameter that allows you to emit them.

I personally think the best solution would be to just convert the
message into a trace point.

While at it, please see whether the guest can trigger similar host log
output excess in other code paths.

The problem is not really with the log messages: they are consequence of
the bug I try to fix.

What happens is once kvm_pr decodes an invalid instruction all the valid
following instructions trigger a Program exception to the guest (but are
executed correctly). It has no real consequence on big machine like
POWER8, except that the guest become very slow and the log files of the
host are filled with messages (and qemu uses 100% of the CPU). On a
smaller machine like a  PowerMac G5, the machine becomes simply unusable.

It's probably more related to your verbosity level of kernel messages.
If you pass loglevel=0 (or quiet) to you kernel cmdline you won't get
the messages printed to serial which is what's slowing you down.

The other problem sounds pretty severe, but the only thing your patch
does any different from the current code flow would be the patch below.
Or did I miss anything?

diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index 5cc2e7a..4672bc2 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -302,7 +302,11 @@ int kvmppc_emulate_instruction(struct kvm_run *run,
struct kvm_vcpu *vcpu)
 advance = 0;
 printk(KERN_ERR "Couldn't emulate instruction
0x%08x "
"(op %d xop %d)\n", inst, get_op(inst),
get_xop(inst));
+#ifdef CONFIG_PPC_BOOK3S
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+#else
 kvmppc_core_queue_program(vcpu, 0);
+#endif
 }
 }


Do you want I send an updated patch with your changes?


Well, you reported the issue and narrowed it down, so feel free to send 
it under your name :). I merely simplified your patch a bit.



Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] kvm-pr: manage illegal instructions

[Alexander Graf]

On 05/12/2016 11:10 AM, Laurent Vivier wrote:


On 11/05/2016 13:49, Alexander Graf wrote:

On 05/11/2016 01:14 PM, Laurent Vivier wrote:

On 11/05/2016 12:35, Alexander Graf wrote:

On 03/15/2016 09:18 PM, Laurent Vivier wrote:

While writing some instruction tests for kvm-unit-tests for powerpc,
I've found that illegal instructions are not managed correctly with
kvm-pr,
while it is fine with kvm-hv.

When an illegal instruction (like ".long 0") is processed by kvm-pr,
the kernel logs are filled with:

Couldn't emulate instruction 0x (op 0 xop 0)
kvmppc_handle_exit_pr: emulation at 700 failed ()

While the exception handler receives an interrupt for each instruction
executed after the illegal instruction.

Signed-off-by: Laurent Vivier <lviv...@redhat.com>
---
arch/powerpc/kvm/book3s_emulate.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_emulate.c
b/arch/powerpc/kvm/book3s_emulate.c
index 2afdb9c..4ee969d 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -99,7 +99,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run,
struct kvm_vcpu *vcpu,
  switch (get_op(inst)) {
case 0:
-emulated = EMULATE_FAIL;
if ((kvmppc_get_msr(vcpu) & MSR_LE) &&
(inst == swab32(inst_sc))) {
/*
@@ -112,6 +111,9 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run,
struct kvm_vcpu *vcpu,
kvmppc_set_gpr(vcpu, 3, EV_UNIMPLEMENTED);
kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
emulated = EMULATE_DONE;
+} else {
+kvmppc_core_queue_program(vcpu, SRR1_PROGILL);

But isn't that exactly what the semantic of EMULATE_FAIL is? Fixing it
up in book3s_emulate.c is definitely the wrong spot.

So what is the problem you're trying to solve? Is the SRR0 at the wrong
spot or are the log messages the problem?

No, the problem is the host kernel logs are filled by the message and
the execution hangs. And the host becomes unresponsiveness, even after
the end of the tests.

Please, try to run kvm-unit-tests (the emulator test) on a KVM-PR host,
and check the kernel logs (dmesg), then try to ssh to the host...

Ok, so the log messages are the problem. Please fix the message output
then - or remove it altogether. Or if you like, create a module
parameter that allows you to emit them.

I personally think the best solution would be to just convert the
message into a trace point.

While at it, please see whether the guest can trigger similar host log
output excess in other code paths.

The problem is not really with the log messages: they are consequence of
the bug I try to fix.

What happens is once kvm_pr decodes an invalid instruction all the valid
following instructions trigger a Program exception to the guest (but are
executed correctly). It has no real consequence on big machine like
POWER8, except that the guest become very slow and the log files of the
host are filled with messages (and qemu uses 100% of the CPU). On a
smaller machine like a  PowerMac G5, the machine becomes simply unusable.


It's probably more related to your verbosity level of kernel messages. 
If you pass loglevel=0 (or quiet) to you kernel cmdline you won't get 
the messages printed to serial which is what's slowing you down.


The other problem sounds pretty severe, but the only thing your patch 
does any different from the current code flow would be the patch below. 
Or did I miss anything?


diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index 5cc2e7a..4672bc2 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -302,7 +302,11 @@ int kvmppc_emulate_instruction(struct kvm_run *run, 
struct kvm_vcpu *vcpu)

advance = 0;
printk(KERN_ERR "Couldn't emulate instruction 
0x%08x "
   "(op %d xop %d)\n", inst, get_op(inst), 
get_xop(inst));

+#ifdef CONFIG_PPC_BOOK3S
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+#else
kvmppc_core_queue_program(vcpu, 0);
+#endif
}
}


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] kvm-pr: manage illegal instructions

[Alexander Graf]

On 05/11/2016 01:14 PM, Laurent Vivier wrote:


On 11/05/2016 12:35, Alexander Graf wrote:

On 03/15/2016 09:18 PM, Laurent Vivier wrote:

While writing some instruction tests for kvm-unit-tests for powerpc,
I've found that illegal instructions are not managed correctly with
kvm-pr,
while it is fine with kvm-hv.

When an illegal instruction (like ".long 0") is processed by kvm-pr,
the kernel logs are filled with:

   Couldn't emulate instruction 0x (op 0 xop 0)
   kvmppc_handle_exit_pr: emulation at 700 failed ()

While the exception handler receives an interrupt for each instruction
executed after the illegal instruction.

Signed-off-by: Laurent Vivier <lviv...@redhat.com>
---
   arch/powerpc/kvm/book3s_emulate.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_emulate.c
b/arch/powerpc/kvm/book3s_emulate.c
index 2afdb9c..4ee969d 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -99,7 +99,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run,
struct kvm_vcpu *vcpu,
 switch (get_op(inst)) {
   case 0:
-emulated = EMULATE_FAIL;
   if ((kvmppc_get_msr(vcpu) & MSR_LE) &&
   (inst == swab32(inst_sc))) {
   /*
@@ -112,6 +111,9 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run,
struct kvm_vcpu *vcpu,
   kvmppc_set_gpr(vcpu, 3, EV_UNIMPLEMENTED);
   kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
   emulated = EMULATE_DONE;
+} else {
+kvmppc_core_queue_program(vcpu, SRR1_PROGILL);

But isn't that exactly what the semantic of EMULATE_FAIL is? Fixing it
up in book3s_emulate.c is definitely the wrong spot.

So what is the problem you're trying to solve? Is the SRR0 at the wrong
spot or are the log messages the problem?

No, the problem is the host kernel logs are filled by the message and
the execution hangs. And the host becomes unresponsiveness, even after
the end of the tests.

Please, try to run kvm-unit-tests (the emulator test) on a KVM-PR host,
and check the kernel logs (dmesg), then try to ssh to the host...


Ok, so the log messages are the problem. Please fix the message output 
then - or remove it altogether. Or if you like, create a module 
parameter that allows you to emit them.


I personally think the best solution would be to just convert the 
message into a trace point.


While at it, please see whether the guest can trigger similar host log 
output excess in other code paths.



Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] kvm-pr: manage illegal instructions

[Alexander Graf]

On 03/15/2016 09:18 PM, Laurent Vivier wrote:

While writing some instruction tests for kvm-unit-tests for powerpc,
I've found that illegal instructions are not managed correctly with kvm-pr,
while it is fine with kvm-hv.

When an illegal instruction (like ".long 0") is processed by kvm-pr,
the kernel logs are filled with:

  Couldn't emulate instruction 0x (op 0 xop 0)
  kvmppc_handle_exit_pr: emulation at 700 failed ()

While the exception handler receives an interrupt for each instruction
executed after the illegal instruction.

Signed-off-by: Laurent Vivier 
---
  arch/powerpc/kvm/book3s_emulate.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_emulate.c 
b/arch/powerpc/kvm/book3s_emulate.c
index 2afdb9c..4ee969d 100644
--- a/arch/powerpc/kvm/book3s_emulate.c
+++ b/arch/powerpc/kvm/book3s_emulate.c
@@ -99,7 +99,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
  
  	switch (get_op(inst)) {

case 0:
-   emulated = EMULATE_FAIL;
if ((kvmppc_get_msr(vcpu) & MSR_LE) &&
(inst == swab32(inst_sc))) {
/*
@@ -112,6 +111,9 @@ int kvmppc_core_emulate_op_pr(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
kvmppc_set_gpr(vcpu, 3, EV_UNIMPLEMENTED);
kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
emulated = EMULATE_DONE;
+   } else {
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);


But isn't that exactly what the semantic of EMULATE_FAIL is? Fixing it 
up in book3s_emulate.c is definitely the wrong spot.


So what is the problem you're trying to solve? Is the SRR0 at the wrong 
spot or are the log messages the problem?



Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Fix definition of SIAR register

[Alexander Graf]

On 04/25/2016 11:16 AM, Thomas Huth wrote:

On 25.04.2016 10:15, Alexander Graf wrote:



Am 25.04.2016 um 10:08 schrieb Madhavan Srinivasan <ma...@linux.vnet.ibm.com>:




On Friday 08 April 2016 09:24 PM, Thomas Huth wrote:
The SIAR register is available twice, one time as SPR 780 (unprivileged,
but read-only), and one time as SPR 796 (privileged, but read and write).
The Linux kernel code currently uses SPR 780 - and while this is OK for
reading, writing to that register of course does not work.
Since the KVM code tries to write to this register, too (see the mtspr
in book3s_hv_rmhandlers.S), the contents of this register sometimes get
lost for the guests, e.g. during migration of a VM.
To fix this issue, simply switch to the other SPR numer 796 instead.

IIUC, SIAR and SDAR are updated by hardware when we take
a pmu exception with sampling mode enabled (based on instr).
And these register contents are mainly for OS consumption.
So, we dont need to restore these register values at all,
kindly correct me if I missing something here.

What if you migrate between a pmu event firing and the os reading siar? Or what 
if the host gets pmu events? Or we migrate the guest to a different pcpu? In 
all those cases we need to ensure the register contents are consistent.

Right. Or a guest could use the SIAR as a temporary scratch register
while not using the performance monitoring stuff. In that case the
contents of the register of course have to be preserved, too.


Signed-off-by: Thomas Huth <th...@redhat.com>
---
Note: The perf code in core-book3s.c also seems to write to the SIAR
   SPR, so that might be affected by this issue, too - but I did
   not test the perf code, so I'm not sure about that part.

Please write a small unit test that fires off pmu events constantly and checks 
whtether they arrive correctly. Run perf in parallel on the host to increase 
the chance for breakage.

I'm not very familiar with that PMU stuff yet, but I can have a try...


arch/powerpc/include/asm/reg.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index f5f4c66..6630420 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -752,13 +752,13 @@
#define SPRN_PMC6792
#define SPRN_PMC7793
#define SPRN_PMC8794
-#define SPRN_SIAR780
#define SPRN_SDAR781
#define SPRN_SIER784
#define   SIER_SIPR0x200/* Sampled MSR_PR */
#define   SIER_SIHV0x100/* Sampled MSR_HV */
#define   SIER_SIAR_VALID0x040/* SIAR contents valid */
#define   SIER_SDAR_VALID0x020/* SDAR contents valid */
+#define SPRN_SIAR796

I'm sure there's a reason (iSeries?) we used the r/o version before. Better 
introduce a new constant that gives us rw access and use that in the kvm 
entry/exit code.

Sure. Any suggestions on the naming? I could either rename the current
SPRN_SIAR to SPRN_USIAR (so that it is named similar to other registers
that behave that way, like SPRN_USPRG3 - and also QEMU uses USIAR for
this already). Or I could leave the old name untouched and use something
like "SPRN_SIAR_WR" for the 796 register. What do you prefer?


I'd defer that decision to Michael :).


By the way, I just noticed that SPRN_SDAR (781) seems to suffer from the
same problem, too!


Great! The more the merrier :)


Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Fix definition of SIAR register

[Alexander Graf]

> Am 25.04.2016 um 10:08 schrieb Madhavan Srinivasan :
> 
> 
> 
>> On Friday 08 April 2016 09:24 PM, Thomas Huth wrote:
>> The SIAR register is available twice, one time as SPR 780 (unprivileged,
>> but read-only), and one time as SPR 796 (privileged, but read and write).
>> The Linux kernel code currently uses SPR 780 - and while this is OK for
>> reading, writing to that register of course does not work.
>> Since the KVM code tries to write to this register, too (see the mtspr
>> in book3s_hv_rmhandlers.S), the contents of this register sometimes get
>> lost for the guests, e.g. during migration of a VM.
>> To fix this issue, simply switch to the other SPR numer 796 instead.
> 
> IIUC, SIAR and SDAR are updated by hardware when we take
> a pmu exception with sampling mode enabled (based on instr).
> And these register contents are mainly for OS consumption.
> So, we dont need to restore these register values at all,
> kindly correct me if I missing something here.

What if you migrate between a pmu event firing and the os reading siar? Or what 
if the host gets pmu events? Or we migrate the guest to a different pcpu? In 
all those cases we need to ensure the register contents are consistent.

> 
> Maddy
> 
>> 
>> Signed-off-by: Thomas Huth 
>> ---
>> Note: The perf code in core-book3s.c also seems to write to the SIAR
>>   SPR, so that might be affected by this issue, too - but I did
>>   not test the perf code, so I'm not sure about that part.

Please write a small unit test that fires off pmu events constantly and checks 
whtether they arrive correctly. Run perf in parallel on the host to increase 
the chance for breakage.

>> 
>> arch/powerpc/include/asm/reg.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>> index f5f4c66..6630420 100644
>> --- a/arch/powerpc/include/asm/reg.h
>> +++ b/arch/powerpc/include/asm/reg.h
>> @@ -752,13 +752,13 @@
>> #define SPRN_PMC6792
>> #define SPRN_PMC7793
>> #define SPRN_PMC8794
>> -#define SPRN_SIAR780
>> #define SPRN_SDAR781
>> #define SPRN_SIER784
>> #define   SIER_SIPR0x200/* Sampled MSR_PR */
>> #define   SIER_SIHV0x100/* Sampled MSR_HV */
>> #define   SIER_SIAR_VALID0x040/* SIAR contents valid */
>> #define   SIER_SDAR_VALID0x020/* SDAR contents valid */
>> +#define SPRN_SIAR796

I'm sure there's a reason (iSeries?) we used the r/o version before. Better 
introduce a new constant that gives us rw access and use that in the kvm 
entry/exit code.

Alex

>> #define SPRN_TACR888
>> #define SPRN_TCSCR889
>> #define SPRN_CSIGR890
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] spapr: Don't set the TM ibm, pa-features bit in PR KVM mode

[Alexander Graf]

> Am 04.04.2016 um 13:09 schrieb Anton Blanchard :
> 
> We don't support transactional memory in PR KVM, so don't tell
> the OS that we do.
> 
> Signed-off-by: Anton Blanchard 
> ---
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e7be21e..538bd87 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -696,6 +696,12 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> *fdt, int offset,
> } else /* env->mmu_model == POWERPC_MMU_2_07 */ {
> pa_features = pa_features_207;
> pa_size = sizeof(pa_features_207);
> +
> +/* Don't enable TM in PR KVM mode */
> +if (kvm_enabled() &&
> +kvm_vm_check_extension(cs->kvm_state, KVM_CAP_PPC_GET_PVINFO))

Does this compile on non-kvm systems?

Alex

> {
> +pa_features[24] &= ~0x80;
> +}
> }
> if (env->ci_large_pages) {
> pa_features[3] |= 0x20;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: commit 9178ba294b6839eeff1a91bed95515d783f3ee6c an A-Eon Tabor Board

[Alexander Graf]

On 02/03/2016 11:15 PM, Julian Margetson wrote:

On 2/3/2016 4:43 PM, Alexander Graf wrote:



On 02/03/2016 10:33 AM, Julian Margetson wrote:
Resending as it was attached to and old thread relating to a 
different motherboard.


On 2/2/2016 9:54 AM, Julian Margetson wrote:


Commit 9178ba294b6839eeff1a91bed95515d783f3ee6c prevents building 
of kernel 4.1 branch on A-Eon Tabor Board.


CC arch/powerpc/math-emu/fsqrt.o
arch/powerpc/platforms/85xx/tabor.c:194:2: error: unknown field 
‘power_off’ specified in initializer


I can't seem to find that file in Linux upstream?


Alex





It may have been discontinued as the patches used were maintained 
along with patches for the (Varisys)  A-Eon Cyrus board

which is officially supported from kernel 4.4.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c383ee84e1d575b09d167185d15df24bde25eb15 



I don't quite understand how an internal API change in Linux breaking 
random external patches is a bug? Either your code is upstream or it can 
break on every git commit done upstream.



Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: commit 9178ba294b6839eeff1a91bed95515d783f3ee6c an A-Eon Tabor Board

[Alexander Graf]

On 02/03/2016 10:33 AM, Julian Margetson wrote:
Resending as it was attached to and old thread relating to a different 
motherboard.


On 2/2/2016 9:54 AM, Julian Margetson wrote:


Commit 9178ba294b6839eeff1a91bed95515d783f3ee6c prevents building of 
kernel 4.1 branch on A-Eon Tabor Board.


CC arch/powerpc/math-emu/fsqrt.o
arch/powerpc/platforms/85xx/tabor.c:194:2: error: unknown field 
‘power_off’ specified in initializer


I can't seem to find that file in Linux upstream?


Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: commit 9178ba294b6839eeff1a91bed95515d783f3ee6c an A-Eon Tabor Board

[Alexander Graf]

On 02/03/2016 11:54 PM, Julian Margetson wrote:

On 2/3/2016 6:20 PM, Alexander Graf wrote:



On 02/03/2016 11:15 PM, Julian Margetson wrote:

On 2/3/2016 4:43 PM, Alexander Graf wrote:



On 02/03/2016 10:33 AM, Julian Margetson wrote:
Resending as it was attached to and old thread relating to a 
different motherboard.


On 2/2/2016 9:54 AM, Julian Margetson wrote:


Commit 9178ba294b6839eeff1a91bed95515d783f3ee6c prevents building 
of kernel 4.1 branch on A-Eon Tabor Board.


CC arch/powerpc/math-emu/fsqrt.o
arch/powerpc/platforms/85xx/tabor.c:194:2: error: unknown field 
‘power_off’ specified in initializer


I can't seem to find that file in Linux upstream?


Alex





It may have been discontinued as the patches used were maintained 
along with patches for the (Varisys)  A-Eon Cyrus board

which is officially supported from kernel 4.4.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c383ee84e1d575b09d167185d15df24bde25eb15 



I don't quite understand how an internal API change in Linux breaking 
random external patches is a bug? Either your code is upstream or it 
can break on every git commit done upstream.



Alex





Sorry I am relatively new at this.
If I manage to pinpoint a problem on my powerpc machines I report it . 
Most of them so far have indeed been bugs.
The random external patches were done by person with far greater 
competence than me who are no longer in the picture.

Any  guidance would be greatly  appreciated.


I think the most important step really is to upstream board support, 
otherwise things will continue to fall apart for sure.


As for the exact breakage you saw, just remove the line and put a line like

  pm_power_off = tabor_power_off;

in your board probe function in tabor.c.


Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] KVM: ppc: Fix size of the PSPB register

[Alexander Graf]

> Am 02.09.2015 um 09:26 schrieb Thomas Huth :
> 
>> On 02/09/15 00:55, Benjamin Herrenschmidt wrote:
>>> On Wed, 2015-09-02 at 08:45 +1000, Paul Mackerras wrote:
>>> On Wed, Sep 02, 2015 at 08:25:05AM +1000, Benjamin Herrenschmidt
>>> wrote:
 On Tue, 2015-09-01 at 23:41 +0200, Thomas Huth wrote:
> The size of the Problem State Priority Boost Register is only
> 32 bits, so let's change the type of the corresponding variable
> accordingly to avoid future trouble.
 
 It's not future trouble, it's broken today for LE and this should
 fix
 it BUT 
>>> 
>>> No, it's broken today for BE hosts, which will always see 0 for the
>>> PSPB register value.  LE hosts are fine.
> 
> Right ... I just meant that nobody really experienced trouble with this
> today yet, but the bug is already present now already of course.

Sounds like a great candidate for kvm-unit-tests then, no? ;)


Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: Build regressions/improvements in v4.2-rc8

[Alexander Graf]

On 24.08.15 10:36, Geert Uytterhoeven wrote:
 On Mon, Aug 24, 2015 at 10:34 AM, Geert Uytterhoeven
 ge...@linux-m68k.org wrote:
 JFYI, when comparing v4.2-rc8[1] to v4.2-rc7[3], the summaries are:
   - build errors: +4/-7
 
 4 regressions:
   + /home/kisskb/slave/src/include/linux/kvm_host.h: error: array
 subscript is above array bounds [-Werror=array-bounds]:  = 430:19
 (arch/powerpc/kvm/book3s_64_mmu.c: In function 'kvmppc_mmu
 _book3s_64_tlbie':)
 
 powerpc-randconfig (seen before in a v3.15-rc1 build?)

I'm not quite sure what's going wrong here. The code in question does

  kvm_for_each_vcpu(i, v, vcpu-kvm)
kvmppc_mmu_pte_vflush(v, va  12, mask);

and IIUC the thing we're potentially running over on would be
kvm-vcpus[i]. But that one is bound by the kvm_for_each_vcpu loop, no?


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 2/4] PCI: pci-host-generic: Fix lookup of linux, pci-probe-only property

[Alexander Graf]

 On 14.08.2015, at 09:43, Will Deacon will.dea...@arm.com wrote:
 
 On Fri, Aug 14, 2015 at 05:40:51PM +0100, Bjorn Helgaas wrote:
 On Fri, Aug 14, 2015 at 05:19:17PM +0100, Marc Zyngier wrote:
 When pci-host-generic looks for the probe-only property, it seems
 to trust the DT to be correctly written, and assumes that there
 is a parameter to the property.
 
 Unfortunately, this is not always the case, and some firmware expose
 this property naked. The driver ends up making a decision based on
 whatever the property pointer points to, which is likely to be junk.
 
 Switch to the common of_pci.c implementation that doesn't suffer
 from this problem.
 
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
 drivers/pci/host/pci-host-generic.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)
 
 diff --git a/drivers/pci/host/pci-host-generic.c 
 b/drivers/pci/host/pci-host-generic.c
 index 265dd25..545ff4e 100644
 --- a/drivers/pci/host/pci-host-generic.c
 +++ b/drivers/pci/host/pci-host-generic.c
 @@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
 int err;
 const char *type;
 const struct of_device_id *of_id;
 -   const int *prop;
 struct device *dev = pdev-dev;
 struct device_node *np = dev-of_node;
 struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
 @@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
 return -EINVAL;
 }
 
 -   prop = of_get_property(of_chosen, linux,pci-probe-only, NULL);
 -   if (prop) {
 -   if (*prop)
 -   pci_add_flags(PCI_PROBE_ONLY);
 -   else
 -   pci_clear_flags(PCI_PROBE_ONLY);
 -   }
 +   of_pci_check_probe_only(of_chosen);
 
 Do we need support for pci-probe-only in pci-host-generic at all?
 You're removing the use in amd-overdrive.dts, and there are no other
 DTs in the kernel tree that mention it.
 
 If we can live without it, that would be nice.  It seems like a relic from
 days when we couldn't reliably assign resources.  (I'm not saying we can do
 that reliably even today, but I'd rather make it reliable than turn it
 off.)
 
 Kvmtool certainly uses it (and generates its own DT, hence why you don't
 see it in mainline). Not sure about qemu, though.

QEMU definitely doesn't do proble-only. Is this driver used on real PPC 
machines too? In that case you also won't see the dt, because it comes with the 
hardware ;).


Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 1/1] KVM: PPC: Book3S: correct width in XER handling

[Alexander Graf]

On 06.08.15 12:16, Laurent Vivier wrote:
 Hi,
 
 I'd also like to see this patch in the mainstream as it fixes a bug
 appearing when we switch from vCPU context to hypervisor context (guest
 crash).

Thanks, applied to kvm-ppc-queue.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] kvm:powerpc:Fix incorrect return statement in the function mpic_set_default_irq_routing

[Alexander Graf]

On 07.08.15 17:54, Nicholas Krause wrote:
 This fixes the incorrect return statement in the function
 mpic_set_default_irq_routing from always returning zero
 to signal success to this function's caller to instead
 return the return value of kvm_set_irq_routing as this
 function can fail and we need to correctly signal the
 caller of mpic_set_default_irq_routing when the call
 to this particular function has failed.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com

I like the patch, but I don't see it on the kvm-ppc mailing list. It
doesn't show up on patchwork or spinics. Did something go wrong while
sending it out?


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] kvm:powerpc:Fix return statements for wrapper functions in the file book3s_64_mmu_hv.c

[Alexander Graf]

On 10.08.15 17:27, Nicholas Krause wrote:
 This fixes the wrapper functions kvm_umap_hva_hv and the function
 kvm_unmap_hav_range_hv to return the return value of the function
 kvm_handle_hva or kvm_handle_hva_range that they are wrapped to
 call internally rather then always making the caller of these
 wrapper functions think they always run successfully by returning
 the value of zero directly.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com

Paul, could you please take on this one?

Thanks,

Alex

 ---
  arch/powerpc/kvm/book3s_64_mmu_hv.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c 
 b/arch/powerpc/kvm/book3s_64_mmu_hv.c
 index dab68b7..0905c8f 100644
 --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
 +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
 @@ -774,14 +774,12 @@ static int kvm_unmap_rmapp(struct kvm *kvm, unsigned 
 long *rmapp,
  
  int kvm_unmap_hva_hv(struct kvm *kvm, unsigned long hva)
  {
 - kvm_handle_hva(kvm, hva, kvm_unmap_rmapp);
 - return 0;
 + return kvm_handle_hva(kvm, hva, kvm_unmap_rmapp);
  }
  
  int kvm_unmap_hva_range_hv(struct kvm *kvm, unsigned long start, unsigned 
 long end)
  {
 - kvm_handle_hva_range(kvm, start, end, kvm_unmap_rmapp);
 - return 0;
 + return kvm_handle_hva_range(kvm, start, end, kvm_unmap_rmapp);
  }
  
  void kvmppc_core_flush_memslot_hv(struct kvm *kvm,
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] kvm:powerpc:Fix incorrect return statement in the function mpic_set_default_irq_routing

[Alexander Graf]

On 12.08.15 21:06, nick wrote:
 
 
 On 2015-08-12 03:05 PM, Alexander Graf wrote:


 On 07.08.15 17:54, Nicholas Krause wrote:
 This fixes the incorrect return statement in the function
 mpic_set_default_irq_routing from always returning zero
 to signal success to this function's caller to instead
 return the return value of kvm_set_irq_routing as this
 function can fail and we need to correctly signal the
 caller of mpic_set_default_irq_routing when the call
 to this particular function has failed.

 Signed-off-by: Nicholas Krause xerofo...@gmail.com

 I like the patch, but I don't see it on the kvm-ppc mailing list. It
 doesn't show up on patchwork or spinics. Did something go wrong while
 sending it out?


 Alex

 Alex,
 Ask Paolo about it as he would be able to explain it better then I.

Well, whatever the reason, I can only apply patches that actually
appeared on the public mailing list. Otherwise people may not get the
chance to review them ;).


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 1/1] KVM: PPC: Book3S: correct width in XER handling

[Alexander Graf]

On 26.05.15 02:27, Sam Bobroff wrote:
 In 64 bit kernels, the Fixed Point Exception Register (XER) is a 64
 bit field (e.g. in kvm_regs and kvm_vcpu_arch) and in most places it is
 accessed as such.
 
 This patch corrects places where it is accessed as a 32 bit field by a
 64 bit kernel.  In some cases this is via a 32 bit load or store
 instruction which, depending on endianness, will cause either the
 lower or upper 32 bits to be missed.  In another case it is cast as a
 u32, causing the upper 32 bits to be cleared.
 
 This patch corrects those places by extending the access methods to
 64 bits.
 
 Signed-off-by: Sam Bobroff sam.bobr...@au1.ibm.com
 ---
 
 v2:
 
 Also extend kvmppc_book3s_shadow_vcpu.xer to 64 bit.
 
  arch/powerpc/include/asm/kvm_book3s.h |4 ++--
  arch/powerpc/include/asm/kvm_book3s_asm.h |2 +-
  arch/powerpc/kvm/book3s_hv_rmhandlers.S   |6 +++---
  arch/powerpc/kvm/book3s_segment.S |4 ++--
  4 files changed, 8 insertions(+), 8 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
 b/arch/powerpc/include/asm/kvm_book3s.h
 index b91e74a..05a875a 100644
 --- a/arch/powerpc/include/asm/kvm_book3s.h
 +++ b/arch/powerpc/include/asm/kvm_book3s.h
 @@ -225,12 +225,12 @@ static inline u32 kvmppc_get_cr(struct kvm_vcpu *vcpu)
   return vcpu-arch.cr;
  }
  
 -static inline void kvmppc_set_xer(struct kvm_vcpu *vcpu, u32 val)
 +static inline void kvmppc_set_xer(struct kvm_vcpu *vcpu, ulong val)

Now we have book3s and booke files with different prototypes on the same
inline function names. That's really ugly. Please keep them in sync ;).


Alex

  {
   vcpu-arch.xer = val;
  }
  
 -static inline u32 kvmppc_get_xer(struct kvm_vcpu *vcpu)
 +static inline ulong kvmppc_get_xer(struct kvm_vcpu *vcpu)
  {
   return vcpu-arch.xer;
  }
 diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h 
 b/arch/powerpc/include/asm/kvm_book3s_asm.h
 index 5bdfb5d..c4ccd2d 100644
 --- a/arch/powerpc/include/asm/kvm_book3s_asm.h
 +++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
 @@ -112,7 +112,7 @@ struct kvmppc_book3s_shadow_vcpu {
   bool in_use;
   ulong gpr[14];
   u32 cr;
 - u32 xer;
 + ulong xer;
   ulong ctr;
   ulong lr;
   ulong pc;
 diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
 b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 index 4d70df2..d75be59 100644
 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 @@ -870,7 +870,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
   blt hdec_soon
  
   ld  r6, VCPU_CTR(r4)
 - lwz r7, VCPU_XER(r4)
 + ld  r7, VCPU_XER(r4)
  
   mtctr   r6
   mtxer   r7
 @@ -1103,7 +1103,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
   mfctr   r3
   mfxer   r4
   std r3, VCPU_CTR(r9)
 - stw r4, VCPU_XER(r9)
 + std r4, VCPU_XER(r9)
  
   /* If this is a page table miss then see if it's theirs or ours */
   cmpwi   r12, BOOK3S_INTERRUPT_H_DATA_STORAGE
 @@ -1675,7 +1675,7 @@ kvmppc_hdsi:
   bl  kvmppc_msr_interrupt
  fast_interrupt_c_return:
  6:   ld  r7, VCPU_CTR(r9)
 - lwz r8, VCPU_XER(r9)
 + ld  r8, VCPU_XER(r9)
   mtctr   r7
   mtxer   r8
   mr  r4, r9
 diff --git a/arch/powerpc/kvm/book3s_segment.S 
 b/arch/powerpc/kvm/book3s_segment.S
 index acee37c..ca8f174 100644
 --- a/arch/powerpc/kvm/book3s_segment.S
 +++ b/arch/powerpc/kvm/book3s_segment.S
 @@ -123,7 +123,7 @@ no_dcbz32_on:
   PPC_LL  r8, SVCPU_CTR(r3)
   PPC_LL  r9, SVCPU_LR(r3)
   lwz r10, SVCPU_CR(r3)
 - lwz r11, SVCPU_XER(r3)
 + PPC_LL  r11, SVCPU_XER(r3)
  
   mtctr   r8
   mtlrr9
 @@ -237,7 +237,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
   mfctr   r8
   mflrr9
  
 - stw r5, SVCPU_XER(r13)
 + PPC_STL r5, SVCPU_XER(r13)
   PPC_STL r6, SVCPU_FAULT_DAR(r13)
   stw r7, SVCPU_FAULT_DSISR(r13)
   PPC_STL r8, SVCPU_CTR(r13)
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/1] KVM: PPC: Book3S: correct width in XER handling

[Alexander Graf]

On 20.05.15 07:26, Sam Bobroff wrote:
 In 64 bit kernels, the Fixed Point Exception Register (XER) is a 64
 bit field (e.g. in kvm_regs and kvm_vcpu_arch) and in most places it is
 accessed as such.
 
 This patch corrects places where it is accessed as a 32 bit field by a
 64 bit kernel.  In some cases this is via a 32 bit load or store
 instruction which, depending on endianness, will cause either the
 lower or upper 32 bits to be missed.  In another case it is cast as a
 u32, causing the upper 32 bits to be cleared.
 
 This patch corrects those places by extending the access methods to
 64 bits.
 
 Signed-off-by: Sam Bobroff sam.bobr...@au1.ibm.com
 ---
 
  arch/powerpc/include/asm/kvm_book3s.h   |4 ++--
  arch/powerpc/kvm/book3s_hv_rmhandlers.S |6 +++---
  arch/powerpc/kvm/book3s_segment.S   |4 ++--
  3 files changed, 7 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
 b/arch/powerpc/include/asm/kvm_book3s.h
 index b91e74a..05a875a 100644
 --- a/arch/powerpc/include/asm/kvm_book3s.h
 +++ b/arch/powerpc/include/asm/kvm_book3s.h
 @@ -225,12 +225,12 @@ static inline u32 kvmppc_get_cr(struct kvm_vcpu *vcpu)
   return vcpu-arch.cr;
  }
  
 -static inline void kvmppc_set_xer(struct kvm_vcpu *vcpu, u32 val)
 +static inline void kvmppc_set_xer(struct kvm_vcpu *vcpu, ulong val)
  {
   vcpu-arch.xer = val;
  }
  
 -static inline u32 kvmppc_get_xer(struct kvm_vcpu *vcpu)
 +static inline ulong kvmppc_get_xer(struct kvm_vcpu *vcpu)
  {
   return vcpu-arch.xer;
  }
 diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
 b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 index 4d70df2..d75be59 100644
 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 @@ -870,7 +870,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
   blt hdec_soon
  
   ld  r6, VCPU_CTR(r4)
 - lwz r7, VCPU_XER(r4)
 + ld  r7, VCPU_XER(r4)
  
   mtctr   r6
   mtxer   r7
 @@ -1103,7 +1103,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
   mfctr   r3
   mfxer   r4
   std r3, VCPU_CTR(r9)
 - stw r4, VCPU_XER(r9)
 + std r4, VCPU_XER(r9)
  
   /* If this is a page table miss then see if it's theirs or ours */
   cmpwi   r12, BOOK3S_INTERRUPT_H_DATA_STORAGE
 @@ -1675,7 +1675,7 @@ kvmppc_hdsi:
   bl  kvmppc_msr_interrupt
  fast_interrupt_c_return:
  6:   ld  r7, VCPU_CTR(r9)
 - lwz r8, VCPU_XER(r9)
 + ld  r8, VCPU_XER(r9)
   mtctr   r7
   mtxer   r8
   mr  r4, r9
 diff --git a/arch/powerpc/kvm/book3s_segment.S 
 b/arch/powerpc/kvm/book3s_segment.S
 index acee37c..ca8f174 100644
 --- a/arch/powerpc/kvm/book3s_segment.S
 +++ b/arch/powerpc/kvm/book3s_segment.S
 @@ -123,7 +123,7 @@ no_dcbz32_on:
   PPC_LL  r8, SVCPU_CTR(r3)
   PPC_LL  r9, SVCPU_LR(r3)
   lwz r10, SVCPU_CR(r3)
 - lwz r11, SVCPU_XER(r3)
 + PPC_LL  r11, SVCPU_XER(r3)

struct kvmppc_book3s_shadow_vcpu {
bool in_use;
ulong gpr[14];
u32 cr;
u32 xer;
[...]

so at least this change looks wrong. Please double-check all fields in
your patch again.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/1] KVM: PPC: Book3S: correct width in XER handling

[Alexander Graf]

On 26.05.15 02:14, Sam Bobroff wrote:
 On Mon, May 25, 2015 at 11:08:08PM +0200, Alexander Graf wrote:


 On 20.05.15 07:26, Sam Bobroff wrote:
 In 64 bit kernels, the Fixed Point Exception Register (XER) is a 64
 bit field (e.g. in kvm_regs and kvm_vcpu_arch) and in most places it is
 accessed as such.

 This patch corrects places where it is accessed as a 32 bit field by a
 64 bit kernel.  In some cases this is via a 32 bit load or store
 instruction which, depending on endianness, will cause either the
 lower or upper 32 bits to be missed.  In another case it is cast as a
 u32, causing the upper 32 bits to be cleared.

 This patch corrects those places by extending the access methods to
 64 bits.

 Signed-off-by: Sam Bobroff sam.bobr...@au1.ibm.com
 ---

  arch/powerpc/include/asm/kvm_book3s.h   |4 ++--
  arch/powerpc/kvm/book3s_hv_rmhandlers.S |6 +++---
  arch/powerpc/kvm/book3s_segment.S   |4 ++--
  3 files changed, 7 insertions(+), 7 deletions(-)

 diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
 b/arch/powerpc/include/asm/kvm_book3s.h
 index b91e74a..05a875a 100644
 --- a/arch/powerpc/include/asm/kvm_book3s.h
 +++ b/arch/powerpc/include/asm/kvm_book3s.h
 @@ -225,12 +225,12 @@ static inline u32 kvmppc_get_cr(struct kvm_vcpu *vcpu)
 return vcpu-arch.cr;
  }
  
 -static inline void kvmppc_set_xer(struct kvm_vcpu *vcpu, u32 val)
 +static inline void kvmppc_set_xer(struct kvm_vcpu *vcpu, ulong val)
  {
 vcpu-arch.xer = val;
  }
  
 -static inline u32 kvmppc_get_xer(struct kvm_vcpu *vcpu)
 +static inline ulong kvmppc_get_xer(struct kvm_vcpu *vcpu)
  {
 return vcpu-arch.xer;
  }
 diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
 b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 index 4d70df2..d75be59 100644
 --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
 @@ -870,7 +870,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
 blt hdec_soon
  
 ld  r6, VCPU_CTR(r4)
 -   lwz r7, VCPU_XER(r4)
 +   ld  r7, VCPU_XER(r4)
  
 mtctr   r6
 mtxer   r7
 @@ -1103,7 +1103,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 mfctr   r3
 mfxer   r4
 std r3, VCPU_CTR(r9)
 -   stw r4, VCPU_XER(r9)
 +   std r4, VCPU_XER(r9)
  
 /* If this is a page table miss then see if it's theirs or ours */
 cmpwi   r12, BOOK3S_INTERRUPT_H_DATA_STORAGE
 @@ -1675,7 +1675,7 @@ kvmppc_hdsi:
 bl  kvmppc_msr_interrupt
  fast_interrupt_c_return:
  6: ld  r7, VCPU_CTR(r9)
 -   lwz r8, VCPU_XER(r9)
 +   ld  r8, VCPU_XER(r9)
 mtctr   r7
 mtxer   r8
 mr  r4, r9
 diff --git a/arch/powerpc/kvm/book3s_segment.S 
 b/arch/powerpc/kvm/book3s_segment.S
 index acee37c..ca8f174 100644
 --- a/arch/powerpc/kvm/book3s_segment.S
 +++ b/arch/powerpc/kvm/book3s_segment.S
 @@ -123,7 +123,7 @@ no_dcbz32_on:
 PPC_LL  r8, SVCPU_CTR(r3)
 PPC_LL  r9, SVCPU_LR(r3)
 lwz r10, SVCPU_CR(r3)
 -   lwz r11, SVCPU_XER(r3)
 +   PPC_LL  r11, SVCPU_XER(r3)

 struct kvmppc_book3s_shadow_vcpu {
 bool in_use;
 ulong gpr[14];
 u32 cr;
 u32 xer;
 [...]

 so at least this change looks wrong. Please double-check all fields in
 your patch again.


 Alex
 
 Thanks for the review and the catch!
 
 The xer field in kvm_vcpu_arch is already ulong, so it looks like the one in
 kvmppc_book3s_shadow_vcpu is the only other case. I'll fix that and repost.

I guess given that the one in pt_regs is also ulong going ulong rather
than u32 is the better choice, yes.

While at it, could you please just do a grep -i xer across all kvm (.c
and .h) files and just sanity check that we're staying in sync?


Thanks!

Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/kvm: Fix SMP=n build error in book3s_xics.c

[Alexander Graf]

On 28.04.15 02:42, Michael Ellerman wrote:
 Commit 34cb7954c0aa Convert ICS mutex lock to spin lock added an
 include of asm/spinlock.h, which does not work in the SMP=n case.
 
 It should instead include linux/spinlock.h
 
 Fixes: 34cb7954c0aa (KVM: PPC: Book3S HV: Convert ICS mutex lock to spin 
 lock)
 Signed-off-by: Michael Ellerman m...@ellerman.id.au

Reviewed-by: Alexander Graf ag...@suse.de


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2] powerpc/book3s: Fix the MCE code to use CONFIG_KVM_BOOK3S_64_HANDLER

[Alexander Graf]

On 17.03.15 11:44, Mahesh J Salgaonkar wrote:
 From: Mahesh Salgaonkar mah...@linux.vnet.ibm.com
 
 commit id 2ba9f0d has changed CONFIG_KVM_BOOK3S_64_HV to tristate to allow
 HV/PR bits to be built as modules. But the MCE code still depends on
 CONFIG_KVM_BOOK3S_64_HV which is wrong. When user selects
 CONFIG_KVM_BOOK3S_64_HV=m to build HV/PR bits as a separate module the
 relevant MCE code gets excluded.
 
 This patch fixes the MCE code to use CONFIG_KVM_BOOK3S_64_HANDLER. This
 makes sure that the relevant MCE code is included when HV/PR bits
 are built as a separate modules.
 
 Signed-off-by: Mahesh Salgaonkar mah...@linux.vnet.ibm.com

Please CC the kvm@vger and kvm-ppc@vger mailing lists on patches that
are KVM related.

In this case, please resend with the proper CCs, so that I can pick it
up into my queue :). Also on resend add the lines that Paul suggested
(CC: stable, Acked-by) after your SoB line.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 2/4] kvm/ppc/mpic: drop unused IRQ_testbit

[Alexander Graf]

On 21.03.15 07:56, Arseny Solokha wrote:
 Drop unused static procedure which doesn't have callers within its
 translation unit. It had been already removed independently in QEMU[1]
 from the OpenPIC implementation borrowed by the kernel.
 
 [1] https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01812.html
 
 v4: Fixed the comment regarding the origination of OpenPIC codebase
 and CC'ed KVM mailing lists, as suggested by Alexander Graf.
 
 v3: In patch 4/4, do not remove fsl_mpic_primary_get_version() from
 arch/powerpc/sysdev/mpic.c because the patch by Jia Hongtao
 (powerpc/85xx: workaround for chips with MSI hardware errata) makes
 use of it.
 
 v2: Added a brief explanation to each patch description of why removed
 functions are unused, as suggested by Michael Ellerman.
 
 Signed-off-by: Arseny Solokha asolo...@kb.kras.ru

Thanks, applied to kvm-ppc-queue (for 4.1).


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/4] kvm/ppc/mpic: drop unused IRQ_testbit

[Alexander Graf]

On 20.03.15 04:56, Arseny Solokha wrote:
 Drop unused static procedure which doesn't have callers within its
 translation unit. It had been already removed independently in QEMU[1]
 from the OpenPIC implementation borrowed from the kernel.

The order was reverse. QEMU's implementation was first and then got
imported into the kernel.

 
 [1] https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01812.html
 
 Signed-off-by: Arseny Solokha asolo...@kb.kras.ru
 Cc: Alexander Graf ag...@suse.de
 Cc: Gleb Natapov g...@kernel.org
 Cc: Paolo Bonzini pbonz...@redhat.com

Please resubmit the patch with CC on kvm@vger and kvm-ppc@vger.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] powerpc/kvm: Limit MAX_VCPUS for guests running on RT Linux

[Alexander Graf]

On 18.02.15 10:32, Bogdan Purcareata wrote:
 Due to the introduction of the raw_spinlock for the KVM openpic, guests with a
 high number of VCPUs may induce great latencies on the underlying RT Linux
 system (e.g. cyclictest reports latencies of ~15ms for guests with 24 VCPUs).
 This can be further aggravated by sending a lot of external interrupts to the
 guest.
 
 A malicious app can abuse this scenario, causing a DoS of the host Linux.
 Until the KVM openpic code is refactored to use finer lock granularity, impose
 a limitation on the number of VCPUs a guest can have when running on a
 PREEMPT_RT_FULL system with KVM_MPIC emulation.
 
 Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
 Signed-off-by: Bogdan Purcareata bogdan.purcare...@freescale.com
 Reviewed-by: Scott Wood scottw...@freescale.com

I don't think this patch is reasonable to take upstream. If we have a
latency issue, whoever spawned KVM VMs made a decision to spawn such big
VMs.

I'd say let's leave it at MAX_VCPUS == NR_CPUS for now and rather get
someone to resolve any locking problems for real ;).


Alex

 ---
  arch/powerpc/include/asm/kvm_host.h | 6 ++
  1 file changed, 6 insertions(+)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h 
 b/arch/powerpc/include/asm/kvm_host.h
 index 8ef0512..6f6b928 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -36,8 +36,14 @@
  #include asm/cacheflush.h
  #include asm/hvcall.h
  
 +#if defined(CONFIG_PREEMPT_RT_FULL)  defined(CONFIG_KVM_MPIC)
 +/* Limit the number of vcpus due to in-kernel mpic concurrency */
 +#define KVM_MAX_VCPUS4
 +#define KVM_MAX_VCORES   4
 +#else
  #define KVM_MAX_VCPUSNR_CPUS
  #define KVM_MAX_VCORES   NR_CPUS
 +#endif
  #define KVM_USER_MEM_SLOTS 32
  #define KVM_MEM_SLOTS_NUM KVM_USER_MEM_SLOTS
  
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/2] powerpc/kvm: Enable running guests on RT Linux

[Alexander Graf]

On 18.02.15 10:32, Bogdan Purcareata wrote:
 This patchset enables running KVM SMP guests with external interrupts on an
 underlying RT-enabled Linux. Previous to this patch, a guest with in-kernel 
 MPIC
 emulation could easily panic the kernel due to preemption when delivering IPIs
 and external interrupts, because of the openpic spinlock becoming a sleeping
 mutex on PREEMPT_RT_FULL Linux.
 
 0001: converts the openpic spinlock to a raw spinlock, in order to circumvent
 this behavior. While this change is targeted for a RT enabled Linux, it has no
 effect on upstream kvm-ppc, so send it upstream for better future maintenance.
 
 0002: introduces a limit on the maximum VCPUs a guest can have, in order to
 prevent potential DoS attack due to large system latencies. This patch is
 targeted to RT (due to CONFIG_PREEMPT_RT_FULL), but it can also be applied on
 upstream Linux, with no effect. Not sure if it's best to send it upstream and
 have a hanging CONFIG_PREEMPT_RT_FULL check there, with no effect, or send it
 against linux-stable-rt. Please apply as you consider appropriate.

Thomas, what is the usual approach for patches like this? Do you take
them into your rt tree or should they get integrated to upstream?


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/2] powerpc/kvm: Enable running guests on RT Linux

[Alexander Graf]

On 20.02.15 15:12, Paolo Bonzini wrote:
 
 
 On 20/02/2015 14:45, Alexander Graf wrote:


 On 18.02.15 10:32, Bogdan Purcareata wrote:
 This patchset enables running KVM SMP guests with external interrupts on an
 underlying RT-enabled Linux. Previous to this patch, a guest with in-kernel 
 MPIC
 emulation could easily panic the kernel due to preemption when delivering 
 IPIs
 and external interrupts, because of the openpic spinlock becoming a sleeping
 mutex on PREEMPT_RT_FULL Linux.

 0001: converts the openpic spinlock to a raw spinlock, in order to 
 circumvent
 this behavior. While this change is targeted for a RT enabled Linux, it has 
 no
 effect on upstream kvm-ppc, so send it upstream for better future 
 maintenance.

 0002: introduces a limit on the maximum VCPUs a guest can have, in order to
 prevent potential DoS attack due to large system latencies. This patch is
 targeted to RT (due to CONFIG_PREEMPT_RT_FULL), but it can also be applied 
 on
 upstream Linux, with no effect. Not sure if it's best to send it upstream 
 and
 have a hanging CONFIG_PREEMPT_RT_FULL check there, with no effect, or send 
 it
 against linux-stable-rt. Please apply as you consider appropriate.

 Thomas, what is the usual approach for patches like this? Do you take
 them into your rt tree or should they get integrated to upstream?
 
 Patch 1 is definitely suitable for upstream, that's the reason why we
 have raw_spin_lock vs. raw_spin_unlock.

I see, perfect :).

Bogdan, please resend patch 1 with CC to kvm-ppc@vger so that I can pick
it up from patchworks.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/5] powerpc: Cleanup KVM emulated load/store endian handling

[Alexander Graf]

On 03.02.15 06:36, David Gibson wrote:
 Sometimes the KVM code on powerpc needs to emulate load or store
 instructions from the guest, which can include both normal and byte
 reversed forms.
 
 We currently (AFAICT) handle this correctly, but some variable names are
 very misleading.  In particular we use is_bigendian in several places to
 actually mean is the IO the same endian as the host, but we now support
 little-endian powerpc hosts.  This also ties into the misleadingly named
 ld_le*() and st_le*() functions, which in fact always byteswap, even on
 an LE host.
 
 This patch cleans this up by renaming to more accurate host_swabbed, and
 uses the generic swab*() functions instead of the powerpc specific and
 misleadingly named ld_le*() and st_le*() functions.
 
 Signed-off-by: David Gibson da...@gibson.dropbear.id.au

Reviewed-by: Alexander Graf ag...@suse.de


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 08/24] powerpc/spapr: vfio: Switch from iommu_table to new powerpc_iommu

[Alexander Graf]

On 03.02.15 01:12, Alex Williamson wrote:
 On Thu, 2015-01-29 at 20:21 +1100, Alexey Kardashevskiy wrote:
 Modern IBM POWERPC systems support multiple (currently two) TCE tables
 per IOMMU group (a.k.a. PE). This adds a powerpc_iommu container
 for TCE tables. Right now just one table is supported.

 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
  arch/powerpc/include/asm/iommu.h|  18 ++--
  arch/powerpc/kernel/eeh.c   |   2 +-
  arch/powerpc/kernel/iommu.c |  34 
  arch/powerpc/platforms/powernv/pci-ioda.c   |  37 +---
  arch/powerpc/platforms/powernv/pci-p5ioc2.c |  16 ++--
  arch/powerpc/platforms/powernv/pci.c|   2 +-
  arch/powerpc/platforms/powernv/pci.h|   4 +-
  arch/powerpc/platforms/pseries/iommu.c  |   9 +-
  drivers/vfio/vfio_iommu_spapr_tce.c | 131 
 
  9 files changed, 170 insertions(+), 83 deletions(-)
 [snip]
 diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
 b/drivers/vfio/vfio_iommu_spapr_tce.c
 index 29d5708..28909e1 100644
 --- a/drivers/vfio/vfio_iommu_spapr_tce.c
 +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
 @@ -84,7 +84,7 @@ static void decrement_locked_vm(long npages)
   */
  struct tce_container {
  struct mutex lock;
 -struct iommu_table *tbl;
 +struct iommu_group *grp;
  bool enabled;
  };
  
 @@ -104,16 +104,40 @@ static bool tce_check_page_size(struct page *page, 
 unsigned page_shift)
  return false;
  }
  
 +static struct iommu_table *spapr_tce_find_table(
 +struct tce_container *container,
 +phys_addr_t ioba)
 +{
 +long i;
 +struct iommu_table *ret = NULL;
 +struct powerpc_iommu *iommu = iommu_group_get_iommudata(container-grp);
 +
 +mutex_lock(container-lock);
 +for (i = 0; i  POWERPC_IOMMU_MAX_TABLES; ++i) {
 +struct iommu_table *tbl = iommu-tables[i];
 +unsigned long entry = ioba  tbl-it_page_shift;
 +unsigned long start = tbl-it_offset;
 +unsigned long end = start + tbl-it_size;
 +
 +if ((start = entry)  (entry  end)) {
 +ret = tbl;
 +break;
 +}
 +}
 +mutex_unlock(container-lock);
 +
 +return ret;
 +}
 +
  static int tce_iommu_enable(struct tce_container *container)
  {
  int ret = 0;
 +struct powerpc_iommu *iommu;
 +struct iommu_table *tbl;
  
 -if (!container-tbl)
 +if (!container-grp)
  return -ENXIO;
  
 -if (!current-mm)
 -return -ESRCH; /* process exited */
 -
  if (container-enabled)
  return -EBUSY;
  
 @@ -142,7 +166,12 @@ static int tce_iommu_enable(struct tce_container 
 *container)
   * as this information is only available from KVM and VFIO is
   * KVM agnostic.
   */
 -ret = try_increment_locked_vm(IOMMU_TABLE_PAGES(container-tbl));
 +iommu = iommu_group_get_iommudata(container-grp);
 +if (!iommu)
 +return -EFAULT;
 +
 +tbl = iommu-tables[0];
 
 
 There should probably be a comment somewhere documenting that tables[0]
 is the small window and presumably [1] will be the DDW.

Rather than a comment, how about an enum?


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/8] current ACCESS_ONCE patch queue

[Alexander Graf]

On 15.01.15 09:58, Christian Borntraeger wrote:
 Folks,
 
 fyi, this is my current patch queue for the next merge window. It
 does contain a patch that will disallow ACCESS_ONCE on non-scalar
 types.
 
 The tree is part of linux-next and can be found at
 git://git.kernel.org/pub/scm/linux/kernel/git/borntraeger/linux.git linux-next

KVM PPC bits are:

 Acked-by: Alexander Graf ag...@suse.de



Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: powernv: Return to cpu offline loop when finished in KVM guest

[Alexander Graf]

On 21.12.14 15:13, Andreas Schwab wrote:
 arch/powerpc/kvm/built-in.o: In function `kvm_no_guest':
 arch/powerpc/kvm/book3s_hv_rmhandlers.o:(.text+0x724): undefined reference to 
 `power7_wakeup_loss'

Ugh. We just removed support for 970 HV mode, but that obviously doesn't
mean you can't compile in support for HV mode without enabling p7.

Paul, what would you think of a patch that makes BOOK3S_HV depend on
PPC_POWERNV?


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info

[Alexander Graf]

On 18.12.14 06:11, Michael Ellerman wrote:
 On Wed, 2014-12-17 at 02:16 +0100, Alexander Graf wrote:
 On 31.10.14 04:47, Anton Blanchard wrote:
 LLVM doesn't support local named register variables and is unlikely
 to. current_thread_info is using one, fix it by moving it out and
 calling it __current_r1().

 I gave it a bit of an obscure name because we don't want anyone else
 using it - they should use current_stack_pointer(). This specific
 case is performance critical and we can't afford to call a function
 to get it. Furthermore it isn't important to know exactly where in
 the stack we are since we mask the lower bits.

 Signed-off-by: Anton Blanchard an...@samba.org

 Git bisect managed to point me to this commit as the offender for OOPSes
 on e5500 and e6500 (and maybe the G4 as well, not sure).

 Doing a git revert of this commit on top of linus/master makes things
 work fine for me again.


 Alex

 Oops: Kernel access of bad area, sig: 11 [#2]
 SMP NR_CPUS=16 CoreNet Generic
 Modules linked in:
 CPU: 1 PID: 339 Comm: kworker/1:1 Tainted: G  D
 3.18.0-09423-g988adfd #1
 Workqueue: rpciod .rpc_async_schedule
 task: c001f6397500 ti: c001f6638000 task.ti: c001f6638000
 NIP: c04817a4 LR: c04817a4 CTR: 
 REGS: c001f663b0e0 TRAP: 0300   Tainted: G  D
 (3.18.0-09423-g988adfd)
 MSR: 80029000 CE,EE,ME  CR: 24ad2e42  XER: 
 DEAR: 202031303438355f ESR:  SOFTE: 1
   = r9 + 40
 
 GPR00: c04817a4 c001f663b360 c0988028 7f24333d
 GPR04: 5ff5738c1f2ebfb1   08f8
 GPR08: c0480ae8 2020313034383537 36204b4220617320 6469726563740a31
 GPR12: 3937302d30312d30 cfff8780 c007f988 c001f64c1600
 
 GPRs 9-12 say:  1048576 KB as direct\n1970-01-0
 
 Which is rarely a good sign :)
 
 Looks like it might be part of your dmesg from setup_page_sizes().
 
 GPR16:    05dc
 GPR20: c09b8028 c0007e034200 0548 c000
 GPR24: c001f663b4b0 b225831e  0080
 GPR28: 0548 08f8 0548 0094
 NIP [c04817a4] .__skb_checksum+0x194/0x378
 LR [c04817a4] .__skb_checksum+0x194/0x378
 Call Trace:
 [c001f663b360] [c04817a4] .__skb_checksum+0x194/0x378
 (unreliable)
 [c001f663b440] [c04819b4] .skb_checksum+0x2c/0x3c
 [c001f663b4c0] [c04fd0a8] .udp4_hwcsum+0xa8/0x16c
 [c001f663b560] [c04fd440] .udp_send_skb+0x2d4/0x370
 [c001f663b600] [c04fd51c] .udp_push_pending_frames+0x40/0x94
 [c001f663b680] [c04fec08] .udp_sendpage+0x150/0x1b4
 [c001f663b770] [c050ae54] .inet_sendpage+0xa0/0x120
 [c001f663b810] [c059c8cc] .xs_sendpages+0x2d0/0x30c
 [c001f663b8d0] [c059cae4] .xs_udp_send_request+0x58/0x120
 [c001f663b970] [c0598f04] .xprt_transmit+0x80/0x36c
 [c001f663ba20] [c05942d8] .call_transmit+0x19c/0x254
 [c001f663bab0] [c059ff64] .__rpc_execute+0xbc/0x3c0
 [c001f663bb90] [c00797f8] .process_one_work+0x1c0/0x474
 [c001f663bc40] [c007a518] .worker_thread+0x17c/0x54c
 [c001f663bd30] [c007fa8c] .kthread+0x104/0x124
 [c001f663be30] [c884] .ret_from_kernel_thread+0x58/0xd4
 Instruction dump:
 7d1f3a14 7c6a1850 e958 7fbd4050 786334e4 e90a 7c63ba14 f8490028
 7c63ea14 7d0903a6 e84a0008 4e800421 e8490028 7c641b78 78270464 e9580008
 
 Which is:
 
 add  r8, r31, r7
 subf r3, r10, r3
 ld  r10, 0(r24)
 subfr29, r29, r8
 rldicr   r3, r3, 6, 51
 ld   r8, 0(r10)
 add  r3, r3, r23
 std  r2, 40(r9)
 add  r3, r3, r29
 mtctrr8
 ld   r2, 8(r10)
 bctrl
 ld   r2, 40(r9)   ---
 mr   r4, r3
 rldicr   r7, r1, 0, 49
 ld  r10, 8(r24)
 
 
 Which looks a bit odd. I'd expect us to be saving/restoring r2 to the stack,
 though maybe r9 was pointing at the stack?
 
 Looking at your vmlinux.broken I don't see the same code gen.
 
 Can you get an oops from a kernel and upload the exact binary? Or just post us
 the full code dump of __skb_checksum() (or wherever it oopses).

Ugh, sorry - I must've copied the wrong one. The serial output below is
from the uImage that (hopefully) is belongs to the vmlinux.broken:

  http://csgraf.de/agraf/current_thread_info/dmesg.txt


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info

[Alexander Graf]

On 18.12.14 07:25, Anton Blanchard wrote:
 On Thu, 18 Dec 2014 16:11:54 +1100
 Michael Ellerman m...@ellerman.id.au wrote:
 
 On Wed, 2014-12-17 at 02:16 +0100, Alexander Graf wrote:
 On 31.10.14 04:47, Anton Blanchard wrote:
 LLVM doesn't support local named register variables and is
 unlikely to. current_thread_info is using one, fix it by moving
 it out and calling it __current_r1().

 I gave it a bit of an obscure name because we don't want anyone
 else using it - they should use current_stack_pointer(). This
 specific case is performance critical and we can't afford to call
 a function to get it. Furthermore it isn't important to know
 exactly where in the stack we are since we mask the lower bits.

 Signed-off-by: Anton Blanchard an...@samba.org

 Git bisect managed to point me to this commit as the offender for
 OOPSes on e5500 and e6500 (and maybe the G4 as well, not sure).

 Doing a git revert of this commit on top of linus/master makes
 things work fine for me again.


 Alex

 Oops: Kernel access of bad area, sig: 11 [#2]
 SMP NR_CPUS=16 CoreNet Generic
 Modules linked in:
 CPU: 1 PID: 339 Comm: kworker/1:1 Tainted: G  D
 3.18.0-09423-g988adfd #1
 Workqueue: rpciod .rpc_async_schedule
 task: c001f6397500 ti: c001f6638000 task.ti:
 c001f6638000 NIP: c04817a4 LR: c04817a4 CTR:
  REGS: c001f663b0e0 TRAP: 0300   Tainted:
 G  D (3.18.0-09423-g988adfd)
 MSR: 80029000 CE,EE,ME  CR: 24ad2e42  XER: 
 DEAR: 202031303438355f ESR:  SOFTE: 1
   = r9 + 40

 GPR00: c04817a4 c001f663b360 c0988028
 7f24333d GPR04: 5ff5738c1f2ebfb1 
  08f8 GPR08: c0480ae8
 2020313034383537 36204b4220617320 6469726563740a31 GPR12:
 3937302d30312d30 cfff8780 c007f988 c001f64c1600

 GPRs 9-12 say:  1048576 KB as direct\n1970-01-0

 Which is rarely a good sign :)

 Looks like it might be part of your dmesg from setup_page_sizes().

 GPR16:   
 05dc GPR20: c09b8028 c0007e034200
 0548 c000 GPR24: c001f663b4b0
 b225831e  0080 GPR28:
 0548 08f8 0548 0094
 NIP [c04817a4] .__skb_checksum+0x194/0x378 LR
 [c04817a4] .__skb_checksum+0x194/0x378 Call Trace:
 [c001f663b360] [c04817a4] .__skb_checksum+0x194/0x378
 (unreliable)
 [c001f663b440] [c04819b4] .skb_checksum+0x2c/0x3c
 [c001f663b4c0] [c04fd0a8] .udp4_hwcsum+0xa8/0x16c
 [c001f663b560] [c04fd440] .udp_send_skb+0x2d4/0x370
 [c001f663b600]
 [c04fd51c] .udp_push_pending_frames+0x40/0x94
 [c001f663b680] [c04fec08] .udp_sendpage+0x150/0x1b4
 [c001f663b770] [c050ae54] .inet_sendpage+0xa0/0x120
 [c001f663b810] [c059c8cc] .xs_sendpages+0x2d0/0x30c
 [c001f663b8d0]
 [c059cae4] .xs_udp_send_request+0x58/0x120
 [c001f663b970] [c0598f04] .xprt_transmit+0x80/0x36c
 [c001f663ba20] [c05942d8] .call_transmit+0x19c/0x254
 [c001f663bab0] [c059ff64] .__rpc_execute+0xbc/0x3c0
 [c001f663bb90] [c00797f8] .process_one_work+0x1c0/0x474
 [c001f663bc40] [c007a518] .worker_thread+0x17c/0x54c
 [c001f663bd30] [c007fa8c] .kthread+0x104/0x124
 [c001f663be30]
 [c884] .ret_from_kernel_thread+0x58/0xd4 Instruction
 dump: 7d1f3a14 7c6a1850 e958 7fbd4050 786334e4 e90a
 7c63ba14 f8490028 7c63ea14 7d0903a6 e84a0008 4e800421 e8490028
 7c641b78 78270464 e9580008

 Which is:

 add  r8, r31, r7
 subf r3, r10, r3
 ld  r10, 0(r24)
 subfr29, r29, r8
 rldicr   r3, r3, 6, 51
 ld   r8, 0(r10)
 add  r3, r3, r23
 std  r2, 40(r9)
 add  r3, r3, r29
 mtctrr8
 ld   r2, 8(r10)
 bctrl
 ld   r2, 40(r9)  ---
 mr   r4, r3
 rldicr   r7, r1, 0, 49
 ld  r10, 8(r24)


 Which looks a bit odd. I'd expect us to be saving/restoring r2 to the
 stack, though maybe r9 was pointing at the stack?
 
 Nice catch! This looks like a compiler bug.
 
 Looking at your vmlinux.broken I don't see the same code gen.
 
 For whatever reason we ended up with r10 this time:
 
 7c 2a 0b 78 mr  r10,r1
 ...
 f8 4a 00 28 std r2,40(r10)
 7c 63 ba 14 add r3,r3,r23
 7c e9 03 a6 mtctr   r7
 7c 63 ea 14 add r3,r3,r29
 38 a0 00 00 li  r5,0
 e8 48 00 08 ld  r2,8(r8)
 4e 80 04 21 bctrl
 e8 4a 00 28 ld  r2,40(r10)
 
 The indirect function call is allowed to clobber r10, gcc is doing
 something very wrong here.

Yeah, I couldn't see why the patch really would break anything either,
but it does for me with this (not quite old) version of gcc.

I also don't see the breakage on LE machines that compile with a newer
version of gcc (4.8.3).


Alex

Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info

[Alexander Graf]

On 17.12.14 04:44, Anton Blanchard wrote:
 Hi Alex,
 
 Git bisect managed to point me to this commit as the offender for
 OOPSes on e5500 and e6500 (and maybe the G4 as well, not sure).

 Doing a git revert of this commit on top of linus/master makes things
 work fine for me again.
 
 Ouch, sorry for that, I'll work to reproduce. What gcc version are you
 using?

I'm running

  gcc (SUSE Linux) 4.7.2 20130108 [gcc-4_7-branch revision 195012]

which is basically the one from openSUSE 12.3 for ppc64.

I've also uploaded 2 builds, one with the patch applied (broken) and one
without (works):

  http://csgraf.de/agraf/current_thread_info/vmlinux.broken.xz
  http://csgraf.de/agraf/current_thread_info/vmlinux.works.xz

Interestingly enough I did not see this on IBM POWER systems, but maybe
that's because most of them compile their own kernels with local,
different compilers in my test runs and the only one that does base on
the same compiler doesn't run on nfsroot.

My iBook G4 target is affected by this as well and reverting this patch
also makes it work again. Maybe we're just running over some stack?


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] arch: powerpc: kvm: book3s_paired_singles.c: Remove unused function

[Alexander Graf]

On 07.12.14 23:29, Rickard Strandqvist wrote:
 Remove the function inst_set_field() that is not used anywhere.
 
 This was partially found by using a static code analysis program called 
 cppcheck.
 
 Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se

Thanks, applied all unused function patches in KVM PPC code to
kvm-ppc-queue.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: powernv: Return to cpu offline loop when finished in KVM guest

[Alexander Graf]

On 03.12.14 04:48, Paul Mackerras wrote:
 When a secondary hardware thread has finished running a KVM guest, we
 currently put that thread into nap mode using a nap instruction in
 the KVM code.  This changes the code so that instead of doing a nap
 instruction directly, we instead cause the call to power7_nap() that
 put the thread into nap mode to return.  The reason for doing this is
 to avoid having the KVM code having to know what low-power mode to
 put the thread into.
 
 In the case of a secondary thread used to run a KVM guest, the thread
 will be offline from the point of view of the host kernel, and the
 relevant power7_nap() call is the one in pnv_smp_cpu_disable().
 In this case we don't want to clear pending IPIs in the offline loop
 in that function, since that might cause us to miss the wakeup for
 the next time the thread needs to run a guest.  To tell whether or
 not to clear the interrupt, we use the SRR1 value returned from
 power7_nap(), and check if it indicates an external interrupt.  We
 arrange that the return from power7_nap() when we have finished running
 a guest returns 0, so pending interrupts don't get flushed in that
 case.
 
 Note that it is important a secondary thread that has finished
 executing in the guest, or that didn't have a guest to run, should
 not return to power7_nap's caller while the kvm_hstate.hwthread_req
 flag in the PACA is non-zero, because the return from power7_nap
 will reenable the MMU, and the MMU might still be in guest context.
 In this situation we spin at low priority in real mode waiting for
 hwthread_req to become zero.
 
 Signed-off-by: Paul Mackerras pau...@samba.org
 ---
 I think this would be best going through the powerpc tree.  Alex,
 if you can give me an acked-by for this that would be appreciated.

Acked-by: Alexander Graf ag...@suse.de


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info

[Alexander Graf]

On 31.10.14 04:47, Anton Blanchard wrote:
 LLVM doesn't support local named register variables and is unlikely
 to. current_thread_info is using one, fix it by moving it out and
 calling it __current_r1().
 
 I gave it a bit of an obscure name because we don't want anyone else
 using it - they should use current_stack_pointer(). This specific
 case is performance critical and we can't afford to call a function
 to get it. Furthermore it isn't important to know exactly where in
 the stack we are since we mask the lower bits.
 
 Signed-off-by: Anton Blanchard an...@samba.org

Git bisect managed to point me to this commit as the offender for OOPSes
on e5500 and e6500 (and maybe the G4 as well, not sure).

Doing a git revert of this commit on top of linus/master makes things
work fine for me again.


Alex

Oops: Kernel access of bad area, sig: 11 [#2]
SMP NR_CPUS=16 CoreNet Generic
Modules linked in:
CPU: 1 PID: 339 Comm: kworker/1:1 Tainted: G  D
3.18.0-09423-g988adfd #1
Workqueue: rpciod .rpc_async_schedule
task: c001f6397500 ti: c001f6638000 task.ti: c001f6638000
NIP: c04817a4 LR: c04817a4 CTR: 
REGS: c001f663b0e0 TRAP: 0300   Tainted: G  D
(3.18.0-09423-g988adfd)
MSR: 80029000 CE,EE,ME  CR: 24ad2e42  XER: 
DEAR: 202031303438355f ESR:  SOFTE: 1
GPR00: c04817a4 c001f663b360 c0988028 7f24333d
GPR04: 5ff5738c1f2ebfb1   08f8
GPR08: c0480ae8 2020313034383537 36204b4220617320 6469726563740a31
GPR12: 3937302d30312d30 cfff8780 c007f988 c001f64c1600
GPR16:    05dc
GPR20: c09b8028 c0007e034200 0548 c000
GPR24: c001f663b4b0 b225831e  0080
GPR28: 0548 08f8 0548 0094
NIP [c04817a4] .__skb_checksum+0x194/0x378
LR [c04817a4] .__skb_checksum+0x194/0x378
Call Trace:
[c001f663b360] [c04817a4] .__skb_checksum+0x194/0x378
(unreliable)
[c001f663b440] [c04819b4] .skb_checksum+0x2c/0x3c
[c001f663b4c0] [c04fd0a8] .udp4_hwcsum+0xa8/0x16c
[c001f663b560] [c04fd440] .udp_send_skb+0x2d4/0x370
[c001f663b600] [c04fd51c] .udp_push_pending_frames+0x40/0x94
[c001f663b680] [c04fec08] .udp_sendpage+0x150/0x1b4
[c001f663b770] [c050ae54] .inet_sendpage+0xa0/0x120
[c001f663b810] [c059c8cc] .xs_sendpages+0x2d0/0x30c
[c001f663b8d0] [c059cae4] .xs_udp_send_request+0x58/0x120
[c001f663b970] [c0598f04] .xprt_transmit+0x80/0x36c
[c001f663ba20] [c05942d8] .call_transmit+0x19c/0x254
[c001f663bab0] [c059ff64] .__rpc_execute+0xbc/0x3c0
[c001f663bb90] [c00797f8] .process_one_work+0x1c0/0x474
[c001f663bc40] [c007a518] .worker_thread+0x17c/0x54c
[c001f663bd30] [c007fa8c] .kthread+0x104/0x124
[c001f663be30] [c884] .ret_from_kernel_thread+0x58/0xd4
Instruction dump:
7d1f3a14 7c6a1850 e958 7fbd4050 786334e4 e90a 7c63ba14 f8490028
7c63ea14 7d0903a6 e84a0008 4e800421 e8490028 7c641b78 78270464 e9580008
---[ end trace 51b7414695b0cafe ]---

note: kworker/1:1[339] exited with preempt_count 1
Unable to handle kernel paging request for data at address
0xffd8


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Patch] :Fix ISR return value in i2c-mpc.c

[Alexander Graf]

 Am 12.12.2014 um 09:08 schrieb Amit Tomar amit.to...@freescale.com:
 
 ISR should return IRQ_HANDLED only in case of handling something.
  
  
 Signed-off-by: Amit Singh Tomar amit.to...@freescale.com

Please just send a v2 of your patch with the below fix included :).

Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Patch] :Fix ISR return value in i2c-mpc.c

[Alexander Graf]

 Am 12.12.2014 um 09:08 schrieb Amit Tomar amit.to...@freescale.com:
 
 ISR should return IRQ_HANDLED only in case of handling something.
  
  
 Signed-off-by: Amit Singh Tomar amit.to...@freescale.com

Oh, this is on the Linux side. Sorry, please ignore my previous email.

Please provide some information on what breakage you're fixing with the patch 
in the patch description.


Alex

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC 00/11]: powerKVM, release the compute power of secondary hwthread on host

[Alexander Graf]

On 16.10.14 21:29, kernelf...@gmail.com wrote:
 Nowadays, when running powerKVM(book3s, hv mode), we should make the 
 secondary hwthread
 offline. Which means that if we run misc tsks other than dedicated KVM (e.g 
 mix java and KVM),
 we will lose the compute power of the secondary hwthread on host env.

I'm personally more concerned about IO threads and the likes blocking
CPUs that could do actual work.

But really, IMHO this should just get fixed in hardware. The patch set
looks like quite a good addition of complexity to an already complex
problem - which means it will definitely break :).

Couldn't we just do something as simple as partition the system into SMT
and non-SMT cores? Then the user can just say keep 2 cores in SMT mode
and we would refuse to run KVM threads on those.

But then again we would bounce on these threads and increase latency on
entry if we happen to get scheduled there, so it's probably not a win
either.

I really don't have a good answer, except for POWER8 wasn't designed
for this.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 22.10.14 06:37, Michael Ellerman wrote:
 The generic Linux framework to power off the machine is a function pointer
 called pm_power_off. The trick about this pointer is that device drivers can
 potentially implement it rather than board files.
 
 Today on powerpc we set pm_power_off to invoke our generic full machine power
 off logic which then calls ppc_md.power_off to invoke machine specific power
 off.
 
 However, when we want to add a power off GPIO via the gpio-poweroff driver,
 this card house falls apart. That driver only registers itself if pm_power_off
 is NULL to ensure it doesn't override board specific logic. However, since we
 always set pm_power_off to the generic power off logic (which will just not
 power off the machine if no ppc_md.power_off call is implemented), we can't
 implement power off via the generic GPIO power off driver.
 
 To fix this up, let's get rid of the ppc_md.power_off logic and just always 
 use
 pm_power_off as was intended. Then individual drivers such as the GPIO power 
 off
 driver can implement power off logic via that function pointer.
 
 With this patch set applied and a few patches on top of QEMU that implement a
 power off GPIO on the virt e500 machine, I can successfully turn off my 
 virtual
 machine after halt.
 
 Signed-off-by: Alexander Graf ag...@suse.de
 [mpe: Squash into one patch and update changelog based on cover letter]
 Signed-off-by: Michael Ellerman m...@ellerman.id.au
 ---
  arch/powerpc/include/asm/machdep.h   |  1 -
  arch/powerpc/kernel/setup-common.c   |  6 +--
  arch/powerpc/platforms/44x/ppc476.c  |  2 +-
  arch/powerpc/platforms/52xx/efika.c  |  3 +-
  arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c   |  8 ++--
  arch/powerpc/platforms/85xx/corenet_generic.c|  2 +-
  arch/powerpc/platforms/85xx/sgy_cts1000.c|  4 +-
  arch/powerpc/platforms/cell/celleb_setup.c   |  4 +-
  arch/powerpc/platforms/cell/qpace_setup.c|  2 +-
  arch/powerpc/platforms/cell/setup.c  |  2 +-
  arch/powerpc/platforms/chrp/setup.c  |  3 +-
  arch/powerpc/platforms/embedded6xx/gamecube.c|  3 +-
  arch/powerpc/platforms/embedded6xx/linkstation.c |  4 +-
  arch/powerpc/platforms/embedded6xx/wii.c |  3 +-
  arch/powerpc/platforms/maple/setup.c |  4 +-
  arch/powerpc/platforms/powermac/setup.c  |  3 +-
  arch/powerpc/platforms/powernv/setup.c   |  4 +-
  arch/powerpc/platforms/ps3/setup.c   |  2 +-
  arch/powerpc/platforms/pseries/setup.c   | 59 
 
  arch/powerpc/sysdev/fsl_soc.c|  2 +-
  arch/powerpc/xmon/xmon.c |  3 +-
  21 files changed, 66 insertions(+), 58 deletions(-)
 
 
 Hi guys, how does this look?
 
 If it's OK I'll put it in a topic branch for Guenter.

Looks good to me. Thanks for doing the cleanup :).


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 04/20] powerpc/52xx/efika: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/52xx/efika.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/52xx/efika.c 
b/arch/powerpc/platforms/52xx/efika.c
index 3feffde..6af651e 100644
--- a/arch/powerpc/platforms/52xx/efika.c
+++ b/arch/powerpc/platforms/52xx/efika.c
@@ -212,6 +212,8 @@ static int __init efika_probe(void)
DMA_MODE_READ = 0x44;
DMA_MODE_WRITE = 0x48;
 
+   pm_power_off = rtas_power_off;
+
return 1;
 }
 
@@ -225,7 +227,6 @@ define_machine(efika)
.init_IRQ   = mpc52xx_init_irq,
.get_irq= mpc52xx_get_irq,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.set_rtc_time   = rtas_set_rtc_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 02/20] powerpc/xmon: Support either ppc_md.power_off or pm_power_off

[Alexander Graf]

Xmon can manually turn off the machine. We now have 2 code paths for this:

  1) ppc_md.power_off
  2) pm_power_off

This patch allows xmon to support both and makes sure it graciously allows
a path to not be implemented.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/xmon/xmon.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index b988b5a..531f649 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -981,7 +981,10 @@ static void bootcmds(void)
else if (cmd == 'h')
ppc_md.halt();
else if (cmd == 'p')
-   ppc_md.power_off();
+   if (ppc_md.power_off)
+   ppc_md.power_off();
+   if (pm_power_off)
+   pm_power_off();
 }
 
 static int cpu_cmd(void)
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

The generic Linux framework to power off the machine is a function pointer
called pm_power_off. The trick about this pointer is that device drivers can
potentially implement it rather than board files.

Today on PowerPC we set pm_power_off to invoke our generic full machine power
off logic which then calls ppc_md.power_off to invoke machine specific power
off.

However, when we want to add a power off GPIO via the gpio-poweroff driver,
this card house falls apart. That driver only registers itself if pm_power_off
is NULL to ensure it doesn't override board specific logic. However, since we
always set pm_power_off to the generic power off logic (which will just not
power off the machine if no ppc_md.power_off call is implemented), we can't
implement power off via the generic GPIO power off driver.

To fix this up, let's get rid of the ppc_md.power_off logic and just always use
pm_power_off as was intended. Then individual drivers such as the GPIO power off
driver can implement power off logic via that function pointer.

With this patch set applied and a few patches on top of QEMU that implement a
power off GPIO on the virt e500 machine, I can successfully turn off my virtual
machine after halt.

Michael / Ben, you can find this patch set as a git branch at the URL below.
When applying it, please use that one to ensure that Guenter can easily merge
his work with my work.

  git://github.com/agraf/linux-2.6.git pm_power_off-v2

Alex

---

v1 - v2:

  - fix typo in 47x
  - put ppc_md static replacement setters into probe function

Alexander Graf (20):
  powerpc: Support override of pm_power_off
  powerpc/xmon: Support either ppc_md.power_off or pm_power_off
  powerpc/47x: Use pm_power_off rather than ppc_md.power_off
  powerpc/52xx/efika: Use pm_power_off rather than ppc_md.power_off
  powerpc/mpc8349emitx: Use pm_power_off rather than ppc_md.power_off
  powerpc/corenet: Use pm_power_off rather than ppc_md.power_off
  powerpc/85xx/sgy_cts1000: Use pm_power_off rather than
ppc_md.power_off
  powerpc/celleb: Use pm_power_off rather than ppc_md.power_off
  powerpc/cell/qpace: Use pm_power_off rather than ppc_md.power_off
  powerpc/cell: Use pm_power_off rather than ppc_md.power_off
  powerpc/chrp: Use pm_power_off rather than ppc_md.power_off
  powerpc/6xx/gamecube: Use pm_power_off rather than ppc_md.power_off
  powerpc/6xx/linkstation: Use pm_power_off rather than ppc_md.power_off
  powerpc/6xx/wii: Use pm_power_off rather than ppc_md.power_off
  powerpc/maple: Use pm_power_off rather than ppc_md.power_off
  powerpc/powermac: Use pm_power_off rather than ppc_md.power_off
  powerpc/powernv: Use pm_power_off rather than ppc_md.power_off
  powerpc/ps3: Use pm_power_off rather than ppc_md.power_off
  powerpc/pseries: Use pm_power_off rather than ppc_md.power_off
  powerpc: Remove ppc_md.power_off

 arch/powerpc/include/asm/machdep.h   |   1 -
 arch/powerpc/kernel/setup-common.c   |   6 +-
 arch/powerpc/platforms/44x/ppc476.c  |   2 +-
 arch/powerpc/platforms/52xx/efika.c  |   3 +-
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c   |   8 +-
 arch/powerpc/platforms/85xx/corenet_generic.c|   2 +-
 arch/powerpc/platforms/85xx/sgy_cts1000.c|   4 +-
 arch/powerpc/platforms/cell/celleb_setup.c   |   4 +-
 arch/powerpc/platforms/cell/qpace_setup.c|   2 +-
 arch/powerpc/platforms/cell/setup.c  |   2 +-
 arch/powerpc/platforms/chrp/setup.c  |   3 +-
 arch/powerpc/platforms/embedded6xx/gamecube.c|   3 +-
 arch/powerpc/platforms/embedded6xx/linkstation.c |   4 +-
 arch/powerpc/platforms/embedded6xx/wii.c |   3 +-
 arch/powerpc/platforms/maple/setup.c |   4 +-
 arch/powerpc/platforms/powermac/setup.c  | 147 ---
 arch/powerpc/platforms/powernv/setup.c   |   4 +-
 arch/powerpc/platforms/ps3/setup.c   |   2 +-
 arch/powerpc/platforms/pseries/setup.c   |  59 -
 arch/powerpc/sysdev/fsl_soc.c|   2 +-
 arch/powerpc/xmon/xmon.c |   3 +-
 21 files changed, 138 insertions(+), 130 deletions(-)

-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 08/20] powerpc/celleb: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/cell/celleb_setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/celleb_setup.c 
b/arch/powerpc/platforms/cell/celleb_setup.c
index 34e8ce2..90be8ec 100644
--- a/arch/powerpc/platforms/cell/celleb_setup.c
+++ b/arch/powerpc/platforms/cell/celleb_setup.c
@@ -142,6 +142,7 @@ static int __init celleb_probe_beat(void)
powerpc_firmware_features |= FW_FEATURE_CELLEB_ALWAYS
| FW_FEATURE_BEAT | FW_FEATURE_LPAR;
hpte_init_beat_v3();
+   pm_power_off = beat_power_off;
 
return 1;
 }
@@ -190,6 +191,7 @@ static int __init celleb_probe_native(void)
 
powerpc_firmware_features |= FW_FEATURE_CELLEB_ALWAYS;
hpte_init_native();
+   pm_power_off = rtas_power_off;
 
return 1;
 }
@@ -204,7 +206,6 @@ define_machine(celleb_beat) {
.setup_arch = celleb_setup_arch_beat,
.show_cpuinfo   = celleb_show_cpuinfo,
.restart= beat_restart,
-   .power_off  = beat_power_off,
.halt   = beat_halt,
.get_rtc_time   = beat_get_rtc_time,
.set_rtc_time   = beat_set_rtc_time,
@@ -230,7 +231,6 @@ define_machine(celleb_native) {
.setup_arch = celleb_setup_arch_native,
.show_cpuinfo   = celleb_show_cpuinfo,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.get_boot_time  = rtas_get_boot_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 03/20] powerpc/47x: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - fix typo
---
 arch/powerpc/platforms/44x/ppc476.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/44x/ppc476.c 
b/arch/powerpc/platforms/44x/ppc476.c
index 58db9d0..c11ce65 100644
--- a/arch/powerpc/platforms/44x/ppc476.c
+++ b/arch/powerpc/platforms/44x/ppc476.c
@@ -94,7 +94,7 @@ static int avr_probe(struct i2c_client *client,
 {
avr_i2c_client = client;
ppc_md.restart = avr_reset_system;
-   ppc_md.power_off = avr_power_off_system;
+   pm_power_off = avr_power_off_system;
return 0;
 }
 
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 07/20] powerpc/85xx/sgy_cts1000: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/85xx/sgy_cts1000.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
b/arch/powerpc/platforms/85xx/sgy_cts1000.c
index 8162b04..e149c9e 100644
--- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
+++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
@@ -120,7 +120,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
 
/* Register our halt function */
ppc_md.halt = gpio_halt_cb;
-   ppc_md.power_off = gpio_halt_cb;
+   pm_power_off = gpio_halt_cb;
 
printk(KERN_INFO gpio-halt: registered GPIO %d (%d trigger, %d
irq).\n, gpio, trigger, irq);
@@ -137,7 +137,7 @@ static int gpio_halt_remove(struct platform_device *pdev)
free_irq(irq, halt_node);
 
ppc_md.halt = NULL;
-   ppc_md.power_off = NULL;
+   pm_power_off = NULL;
 
gpio_free(gpio);
 
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 13/20] powerpc/6xx/linkstation: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/embedded6xx/linkstation.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/embedded6xx/linkstation.c 
b/arch/powerpc/platforms/embedded6xx/linkstation.c
index 168e1d8..540eeb5 100644
--- a/arch/powerpc/platforms/embedded6xx/linkstation.c
+++ b/arch/powerpc/platforms/embedded6xx/linkstation.c
@@ -147,6 +147,9 @@ static int __init linkstation_probe(void)
 
if (!of_flat_dt_is_compatible(root, linkstation))
return 0;
+
+   pm_power_off = linkstation_power_off;
+
return 1;
 }
 
@@ -158,7 +161,6 @@ define_machine(linkstation){
.show_cpuinfo   = linkstation_show_cpuinfo,
.get_irq= mpic_get_irq,
.restart= linkstation_restart,
-   .power_off  = linkstation_power_off,
.halt   = linkstation_halt,
.calibrate_decr = generic_calibrate_decr,
 };
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 05/20] powerpc/mpc8349emitx: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c 
b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 463fa91e..15e8021 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -167,10 +167,10 @@ static int mcu_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
if (ret)
goto err;
 
-   /* XXX: this is potentially racy, but there is no lock for ppc_md */
-   if (!ppc_md.power_off) {
+   /* XXX: this is potentially racy, but there is no lock for pm_power_off 
*/
+   if (!pm_power_off) {
glob_mcu = mcu;
-   ppc_md.power_off = mcu_power_off;
+   pm_power_off = mcu_power_off;
dev_info(client-dev, will provide power-off service\n);
}
 
@@ -197,7 +197,7 @@ static int mcu_remove(struct i2c_client *client)
device_remove_file(client-dev, dev_attr_status);
 
if (glob_mcu == mcu) {
-   ppc_md.power_off = NULL;
+   pm_power_off = NULL;
glob_mcu = NULL;
}
 
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 01/20] powerpc: Support override of pm_power_off

[Alexander Graf]

The pm_power_off callback is what drivers are supposed to modify when they
implement power off support for the system.

Support a modified callback on powerpc. That way power off support code can
now either override ppc_md.power_off or pm_power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/kernel/setup-common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/setup-common.c 
b/arch/powerpc/kernel/setup-common.c
index 1362cd6..6398239 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -141,6 +141,8 @@ void machine_power_off(void)
machine_shutdown();
if (ppc_md.power_off)
ppc_md.power_off();
+   if (pm_power_off != machine_power_off)
+   pm_power_off();
 #ifdef CONFIG_SMP
smp_send_stop();
 #endif
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 06/20] powerpc/corenet: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/85xx/corenet_generic.c | 2 +-
 arch/powerpc/sysdev/fsl_soc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c 
b/arch/powerpc/platforms/85xx/corenet_generic.c
index e56b89a..1f309cc 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -170,7 +170,7 @@ static int __init corenet_generic_probe(void)
 
ppc_md.get_irq = ehv_pic_get_irq;
ppc_md.restart = fsl_hv_restart;
-   ppc_md.power_off = fsl_hv_halt;
+   pm_power_off = fsl_hv_halt;
ppc_md.halt = fsl_hv_halt;
 #ifdef CONFIG_SMP
/*
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index ffd1169..1e04568 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -238,7 +238,7 @@ void fsl_hv_restart(char *cmd)
 /*
  * Halt the current partition
  *
- * This function should be assigned to the ppc_md.power_off and ppc_md.halt
+ * This function should be assigned to the pm_power_off and ppc_md.halt
  * function pointers, to shut down the partition when we're running under
  * the Freescale hypervisor.
  */
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 09/20] powerpc/cell/qpace: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/cell/qpace_setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/cell/qpace_setup.c 
b/arch/powerpc/platforms/cell/qpace_setup.c
index 6e3409d..d328140 100644
--- a/arch/powerpc/platforms/cell/qpace_setup.c
+++ b/arch/powerpc/platforms/cell/qpace_setup.c
@@ -127,6 +127,7 @@ static int __init qpace_probe(void)
return 0;
 
hpte_init_native();
+   pm_power_off = rtas_power_off;
 
return 1;
 }
@@ -137,7 +138,6 @@ define_machine(qpace) {
.setup_arch = qpace_setup_arch,
.show_cpuinfo   = qpace_show_cpuinfo,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.get_boot_time  = rtas_get_boot_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 20/20] powerpc: Remove ppc_md.power_off

[Alexander Graf]

Now that we have all implementations of ppc_md.power_off converted to
pm_power_off we can remove the ppc_md variant.

While at it, also set the default for pm_power_off to NULL so that non
machine drivers can implement overrides.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/include/asm/machdep.h | 1 -
 arch/powerpc/kernel/setup-common.c | 6 ++
 arch/powerpc/xmon/xmon.c   | 2 --
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h 
b/arch/powerpc/include/asm/machdep.h
index 307347f..f15f15c 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -142,7 +142,6 @@ struct machdep_calls {
 #endif
 
void(*restart)(char *cmd);
-   void(*power_off)(void);
void(*halt)(void);
void(*panic)(char *str);
void(*cpu_die)(void);
diff --git a/arch/powerpc/kernel/setup-common.c 
b/arch/powerpc/kernel/setup-common.c
index 6398239..44c8d03 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -139,9 +139,7 @@ void machine_restart(char *cmd)
 void machine_power_off(void)
 {
machine_shutdown();
-   if (ppc_md.power_off)
-   ppc_md.power_off();
-   if (pm_power_off != machine_power_off)
+   if (pm_power_off)
pm_power_off();
 #ifdef CONFIG_SMP
smp_send_stop();
@@ -153,7 +151,7 @@ void machine_power_off(void)
 /* Used by the G5 thermal driver */
 EXPORT_SYMBOL_GPL(machine_power_off);
 
-void (*pm_power_off)(void) = machine_power_off;
+void (*pm_power_off)(void);
 EXPORT_SYMBOL_GPL(pm_power_off);
 
 void machine_halt(void)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 531f649..506d256 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -981,8 +981,6 @@ static void bootcmds(void)
else if (cmd == 'h')
ppc_md.halt();
else if (cmd == 'p')
-   if (ppc_md.power_off)
-   ppc_md.power_off();
if (pm_power_off)
pm_power_off();
 }
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 12/20] powerpc/6xx/gamecube: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/embedded6xx/gamecube.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/embedded6xx/gamecube.c 
b/arch/powerpc/platforms/embedded6xx/gamecube.c
index bd4ba5d..fe0ed6e 100644
--- a/arch/powerpc/platforms/embedded6xx/gamecube.c
+++ b/arch/powerpc/platforms/embedded6xx/gamecube.c
@@ -67,6 +67,8 @@ static int __init gamecube_probe(void)
if (!of_flat_dt_is_compatible(dt_root, nintendo,gamecube))
return 0;
 
+   pm_power_off = gamecube_power_off;
+
return 1;
 }
 
@@ -80,7 +82,6 @@ define_machine(gamecube) {
.probe  = gamecube_probe,
.init_early = gamecube_init_early,
.restart= gamecube_restart,
-   .power_off  = gamecube_power_off,
.halt   = gamecube_halt,
.init_IRQ   = flipper_pic_probe,
.get_irq= flipper_pic_get_irq,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 14/20] powerpc/6xx/wii: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/embedded6xx/wii.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c 
b/arch/powerpc/platforms/embedded6xx/wii.c
index 388e29b..352592d 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -211,6 +211,8 @@ static int __init wii_probe(void)
if (!of_flat_dt_is_compatible(dt_root, nintendo,wii))
return 0;
 
+   pm_power_off = wii_power_off;
+
return 1;
 }
 
@@ -226,7 +228,6 @@ define_machine(wii) {
.init_early = wii_init_early,
.setup_arch = wii_setup_arch,
.restart= wii_restart,
-   .power_off  = wii_power_off,
.halt   = wii_halt,
.init_IRQ   = wii_pic_probe,
.get_irq= flipper_pic_get_irq,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 16/20] powerpc/powermac: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/powermac/setup.c | 147 
 1 file changed, 74 insertions(+), 73 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/setup.c 
b/arch/powerpc/platforms/powermac/setup.c
index b127a29..1b03bc1 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -274,6 +274,78 @@ static void __init l2cr_init(void)
 }
 #endif
 
+#ifdef CONFIG_ADB_CUDA
+static void cuda_restart(void)
+{
+   struct adb_request req;
+
+   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_RESET_SYSTEM);
+   for (;;)
+   cuda_poll();
+}
+
+static void cuda_shutdown(void)
+{
+   struct adb_request req;
+
+   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_POWERDOWN);
+   for (;;)
+   cuda_poll();
+}
+
+#else
+#define cuda_restart()
+#define cuda_shutdown()
+#endif
+
+#ifndef CONFIG_ADB_PMU
+#define pmu_restart()
+#define pmu_shutdown()
+#endif
+
+#ifndef CONFIG_PMAC_SMU
+#define smu_restart()
+#define smu_shutdown()
+#endif
+
+static void pmac_restart(char *cmd)
+{
+   switch (sys_ctrler) {
+   case SYS_CTRLER_CUDA:
+   cuda_restart();
+   break;
+   case SYS_CTRLER_PMU:
+   pmu_restart();
+   break;
+   case SYS_CTRLER_SMU:
+   smu_restart();
+   break;
+   default: ;
+   }
+}
+
+static void pmac_power_off(void)
+{
+   switch (sys_ctrler) {
+   case SYS_CTRLER_CUDA:
+   cuda_shutdown();
+   break;
+   case SYS_CTRLER_PMU:
+   pmu_shutdown();
+   break;
+   case SYS_CTRLER_SMU:
+   smu_shutdown();
+   break;
+   default: ;
+   }
+}
+
+static void
+pmac_halt(void)
+{
+   pmac_power_off();
+}
+
 static void __init pmac_setup_arch(void)
 {
struct device_node *cpu, *ic;
@@ -382,78 +454,6 @@ void __init_refok note_bootable_part(dev_t dev, int part, 
int goodness)
current_root_goodness = goodness;
 }
 
-#ifdef CONFIG_ADB_CUDA
-static void cuda_restart(void)
-{
-   struct adb_request req;
-
-   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_RESET_SYSTEM);
-   for (;;)
-   cuda_poll();
-}
-
-static void cuda_shutdown(void)
-{
-   struct adb_request req;
-
-   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_POWERDOWN);
-   for (;;)
-   cuda_poll();
-}
-
-#else
-#define cuda_restart()
-#define cuda_shutdown()
-#endif
-
-#ifndef CONFIG_ADB_PMU
-#define pmu_restart()
-#define pmu_shutdown()
-#endif
-
-#ifndef CONFIG_PMAC_SMU
-#define smu_restart()
-#define smu_shutdown()
-#endif
-
-static void pmac_restart(char *cmd)
-{
-   switch (sys_ctrler) {
-   case SYS_CTRLER_CUDA:
-   cuda_restart();
-   break;
-   case SYS_CTRLER_PMU:
-   pmu_restart();
-   break;
-   case SYS_CTRLER_SMU:
-   smu_restart();
-   break;
-   default: ;
-   }
-}
-
-static void pmac_power_off(void)
-{
-   switch (sys_ctrler) {
-   case SYS_CTRLER_CUDA:
-   cuda_shutdown();
-   break;
-   case SYS_CTRLER_PMU:
-   pmu_shutdown();
-   break;
-   case SYS_CTRLER_SMU:
-   smu_shutdown();
-   break;
-   default: ;
-   }
-}
-
-static void
-pmac_halt(void)
-{
-   pmac_power_off();
-}
-
 /* 
  * Early initialization.
  */
@@ -632,6 +632,8 @@ static int __init pmac_probe(void)
smu_cmdbuf_abs = memblock_alloc_base(4096, 4096, 0x8000UL);
 #endif /* CONFIG_PMAC_SMU */
 
+   pm_power_off = pmac_power_off;
+
return 1;
 }
 
@@ -663,7 +665,6 @@ define_machine(powermac) {
.get_irq= NULL, /* changed later */
.pci_irq_fixup  = pmac_pci_irq_fixup,
.restart= pmac_restart,
-   .power_off  = pmac_power_off,
.halt   = pmac_halt,
.time_init  = pmac_time_init,
.get_boot_time  = pmac_get_boot_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 11/20] powerpc/chrp: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/chrp/setup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/chrp/setup.c 
b/arch/powerpc/platforms/chrp/setup.c
index 5b77b19..dbe7d16 100644
--- a/arch/powerpc/platforms/chrp/setup.c
+++ b/arch/powerpc/platforms/chrp/setup.c
@@ -585,6 +585,8 @@ static int __init chrp_probe(void)
DMA_MODE_READ = 0x44;
DMA_MODE_WRITE = 0x48;
 
+   pm_power_off = rtas_power_off,
+
return 1;
 }
 
@@ -597,7 +599,6 @@ define_machine(chrp) {
.show_cpuinfo   = chrp_show_cpuinfo,
.init_IRQ   = chrp_init_IRQ,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.time_init  = chrp_time_init,
.set_rtc_time   = chrp_set_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 17/20] powerpc/powernv: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/powernv/setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/setup.c 
b/arch/powerpc/platforms/powernv/setup.c
index 3f9546d..941831d 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -268,7 +268,7 @@ static void __init pnv_setup_machdep_opal(void)
ppc_md.get_rtc_time = opal_get_rtc_time;
ppc_md.set_rtc_time = opal_set_rtc_time;
ppc_md.restart = pnv_restart;
-   ppc_md.power_off = pnv_power_off;
+   pm_power_off = pnv_power_off;
ppc_md.halt = pnv_halt;
ppc_md.machine_check_exception = opal_machine_check;
ppc_md.mce_check_early_recovery = opal_mce_check_early_recovery;
@@ -285,7 +285,7 @@ static void __init pnv_setup_machdep_rtas(void)
ppc_md.set_rtc_time = rtas_set_rtc_time;
}
ppc_md.restart = rtas_restart;
-   ppc_md.power_off = rtas_power_off;
+   pm_power_off = rtas_power_off;
ppc_md.halt = rtas_halt;
 }
 #endif /* CONFIG_PPC_POWERNV_RTAS */
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 15/20] powerpc/maple: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/maple/setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/maple/setup.c 
b/arch/powerpc/platforms/maple/setup.c
index cb1b0b3..56b85cd 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -169,7 +169,7 @@ static void __init 
maple_use_rtas_reboot_and_halt_if_present(void)
if (rtas_service_present(system-reboot) 
rtas_service_present(power-off)) {
ppc_md.restart = rtas_restart;
-   ppc_md.power_off = rtas_power_off;
+   pm_power_off = rtas_power_off;
ppc_md.halt = rtas_halt;
}
 }
@@ -312,6 +312,7 @@ static int __init maple_probe(void)
alloc_dart_table();
 
hpte_init_native();
+   pm_power_off = maple_power_off;
 
return 1;
 }
@@ -325,7 +326,6 @@ define_machine(maple) {
.pci_irq_fixup  = maple_pci_irq_fixup,
.pci_get_legacy_ide_irq = maple_pci_get_legacy_ide_irq,
.restart= maple_restart,
-   .power_off  = maple_power_off,
.halt   = maple_halt,
.get_boot_time  = maple_get_boot_time,
.set_rtc_time   = maple_set_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 10/20] powerpc/cell: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/cell/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/cell/setup.c 
b/arch/powerpc/platforms/cell/setup.c
index 6ae25fb..d62aa98 100644
--- a/arch/powerpc/platforms/cell/setup.c
+++ b/arch/powerpc/platforms/cell/setup.c
@@ -259,6 +259,7 @@ static int __init cell_probe(void)
return 0;
 
hpte_init_native();
+   pm_power_off = rtas_power_off;
 
return 1;
 }
@@ -269,7 +270,6 @@ define_machine(cell) {
.setup_arch = cell_setup_arch,
.show_cpuinfo   = cell_show_cpuinfo,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.get_boot_time  = rtas_get_boot_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 19/20] powerpc/pseries: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/pseries/setup.c | 59 +-
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/setup.c 
b/arch/powerpc/platforms/pseries/setup.c
index 125c589..9ce1cf0 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -659,6 +659,34 @@ static void __init pSeries_init_early(void)
pr_debug( - pSeries_init_early()\n);
 }
 
+/**
+ * pSeries_power_off - tell firmware about how to power off the system.
+ *
+ * This function calls either the power-off rtas token in normal cases
+ * or the ibm,power-off-ups token (if present  requested) in case of
+ * a power failure. If power-off token is used, power on will only be
+ * possible with power button press. If ibm,power-off-ups token is used
+ * it will allow auto poweron after power is restored.
+ */
+static void pSeries_power_off(void)
+{
+   int rc;
+   int rtas_poweroff_ups_token = rtas_token(ibm,power-off-ups);
+
+   if (rtas_flash_term_hook)
+   rtas_flash_term_hook(SYS_POWER_OFF);
+
+   if (rtas_poweron_auto == 0 ||
+   rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
+   rc = rtas_call(rtas_token(power-off), 2, 1, NULL, -1, -1);
+   printk(KERN_INFO RTAS power-off returned %d\n, rc);
+   } else {
+   rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
+   printk(KERN_INFO RTAS ibm,power-off-ups returned %d\n, rc);
+   }
+   for (;;);
+}
+
 /*
  * Called very early, MMU is off, device-tree isn't unflattened
  */
@@ -741,6 +769,8 @@ static int __init pSeries_probe(void)
else
hpte_init_native();
 
+   pm_power_off = pSeries_power_off;
+
pr_debug(Machine is%s LPAR !\n,
 (powerpc_firmware_features  FW_FEATURE_LPAR) ?  :  not);
 
@@ -754,34 +784,6 @@ static int pSeries_pci_probe_mode(struct pci_bus *bus)
return PCI_PROBE_NORMAL;
 }
 
-/**
- * pSeries_power_off - tell firmware about how to power off the system.
- *
- * This function calls either the power-off rtas token in normal cases
- * or the ibm,power-off-ups token (if present  requested) in case of
- * a power failure. If power-off token is used, power on will only be
- * possible with power button press. If ibm,power-off-ups token is used
- * it will allow auto poweron after power is restored.
- */
-static void pSeries_power_off(void)
-{
-   int rc;
-   int rtas_poweroff_ups_token = rtas_token(ibm,power-off-ups);
-
-   if (rtas_flash_term_hook)
-   rtas_flash_term_hook(SYS_POWER_OFF);
-
-   if (rtas_poweron_auto == 0 ||
-   rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
-   rc = rtas_call(rtas_token(power-off), 2, 1, NULL, -1, -1);
-   printk(KERN_INFO RTAS power-off returned %d\n, rc);
-   } else {
-   rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
-   printk(KERN_INFO RTAS ibm,power-off-ups returned %d\n, rc);
-   }
-   for (;;);
-}
-
 #ifndef CONFIG_PCI
 void pSeries_final_fixup(void) { }
 #endif
@@ -796,7 +798,6 @@ define_machine(pseries) {
.pcibios_fixup  = pSeries_final_fixup,
.pci_probe_mode = pSeries_pci_probe_mode,
.restart= rtas_restart,
-   .power_off  = pSeries_power_off,
.halt   = rtas_halt,
.panic  = rtas_os_term,
.get_boot_time  = rtas_get_boot_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 18/20] powerpc/ps3: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - write pm_power_off in probe function
---
 arch/powerpc/platforms/ps3/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/ps3/setup.c 
b/arch/powerpc/platforms/ps3/setup.c
index 3f509f8..009a200 100644
--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -248,6 +248,7 @@ static int __init ps3_probe(void)
ps3_mm_init();
ps3_mm_vas_create(htab_size);
ps3_hpte_init(htab_size);
+   pm_power_off = ps3_power_off;
 
DBG( - %s:%d\n, __func__, __LINE__);
return 1;
@@ -278,7 +279,6 @@ define_machine(ps3) {
.calibrate_decr = ps3_calibrate_decr,
.progress   = ps3_progress,
.restart= ps3_restart,
-   .power_off  = ps3_power_off,
.halt   = ps3_halt,
 #if defined(CONFIG_KEXEC)
.kexec_cpu_down = ps3_kexec_cpu_down,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 07.10.14 08:25, Michael Ellerman wrote:
 On Mon, 2014-10-06 at 12:00 +0200, Alexander Graf wrote:

 On 03.10.14 06:42, Michael Ellerman wrote:
 On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
 The generic Linux framework to power off the machine is a function pointer
 called pm_power_off. The trick about this pointer is that device drivers 
 can
 potentially implement it rather than board files.

 Today on PowerPC we set pm_power_off to invoke our generic full machine 
 power
 off logic which then calls ppc_md.power_off to invoke machine specific 
 power
 off.

 To fix this up, let's get rid of the ppc_md.power_off logic and just 
 always use
 pm_power_off as was intended. Then individual drivers such as the GPIO 
 power off
 driver can implement power off logic via that function pointer.

 This looks OK to me with one caveat.

 In several of the patches you're replacing a static initialisation with a
 runtime one, and you're doing the runtime initialisation in 
 xxx_setup_arch().
 That's reasonably late, so I'd prefer you did it in xxx_probe().

 Heh, I had it in xxx_probe() originally and then realized that

   a) the power off function is basically a driver. Driver initialization
 happens in xxx_setup_arch() and

   b) the maple target already does overwrite its power_off callback in
 xxx_setup_arch and

   c) on all targets xxx_probe() is very slim and doesn't do much

 but I'll happily change it back to put the bits in xxx_probe() instead.
 
 Thanks.
 
 That way you shouldn't be changing behaviour.
 
 It may still be the case that some power off routines don't actually work 
 until
 later, but that's an existing problem. Some power off routines *do* work 
 before
 setup_arch(), so they will continue to work.

Ok, works for me :). Just wanted to make sure you're aware of the
reasoning why I didn't do it in probe().

 Also, how does your series interact with Guenter's that removes pm_power_off ?
 It seems at the moment they are unaware of each other.

Guenters patches convert users of pm_power_off to his new scheme. We're
not even at that stage at all yet in the powerpc tree. Converting
everything to pm_power_off is basically a first step. His patch set
maintains pm_power_off, so there shouldn't be nasty conflicts.

Once we converted from ppc_md tables to actual code, we can just run a
simple coccinelle patch to convert from pm_power_off to his new scheme
later.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 07.10.14 19:00, Guenter Roeck wrote:
 On Tue, Oct 07, 2014 at 01:35:07PM +0200, Alexander Graf wrote:


 On 07.10.14 08:25, Michael Ellerman wrote:
 On Mon, 2014-10-06 at 12:00 +0200, Alexander Graf wrote:

 On 03.10.14 06:42, Michael Ellerman wrote:
 On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
 The generic Linux framework to power off the machine is a function 
 pointer
 called pm_power_off. The trick about this pointer is that device drivers 
 can
 potentially implement it rather than board files.

 Today on PowerPC we set pm_power_off to invoke our generic full machine 
 power
 off logic which then calls ppc_md.power_off to invoke machine specific 
 power
 off.

 To fix this up, let's get rid of the ppc_md.power_off logic and just 
 always use
 pm_power_off as was intended. Then individual drivers such as the GPIO 
 power off
 driver can implement power off logic via that function pointer.

 This looks OK to me with one caveat.

 In several of the patches you're replacing a static initialisation with a
 runtime one, and you're doing the runtime initialisation in 
 xxx_setup_arch().
 That's reasonably late, so I'd prefer you did it in xxx_probe().

 Heh, I had it in xxx_probe() originally and then realized that

   a) the power off function is basically a driver. Driver initialization
 happens in xxx_setup_arch() and

   b) the maple target already does overwrite its power_off callback in
 xxx_setup_arch and

   c) on all targets xxx_probe() is very slim and doesn't do much

 but I'll happily change it back to put the bits in xxx_probe() instead.

 Thanks.

 That way you shouldn't be changing behaviour.

 It may still be the case that some power off routines don't actually work 
 until
 later, but that's an existing problem. Some power off routines *do* work 
 before
 setup_arch(), so they will continue to work.

 Ok, works for me :). Just wanted to make sure you're aware of the
 reasoning why I didn't do it in probe().

 Also, how does your series interact with Guenter's that removes 
 pm_power_off ?
 It seems at the moment they are unaware of each other.

 Guenters patches convert users of pm_power_off to his new scheme. We're
 not even at that stage at all yet in the powerpc tree. Converting
 everything to pm_power_off is basically a first step. His patch set
 maintains pm_power_off, so there shouldn't be nasty conflicts.

 Onlly the first m68k patch, though. The very last patch in the series
 remvoes pm_power_off.

And there go my patch reading skills ... :).

For which window are you targeting this? 3.18 or 3.19? If you're trying
to hit 3.18, I can easily wait with my patch set and base it on top of
yours.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 03.10.14 06:42, Michael Ellerman wrote:
 On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
 The generic Linux framework to power off the machine is a function pointer
 called pm_power_off. The trick about this pointer is that device drivers can
 potentially implement it rather than board files.

 Today on PowerPC we set pm_power_off to invoke our generic full machine power
 off logic which then calls ppc_md.power_off to invoke machine specific power
 off.

 To fix this up, let's get rid of the ppc_md.power_off logic and just always 
 use
 pm_power_off as was intended. Then individual drivers such as the GPIO power 
 off
 driver can implement power off logic via that function pointer.
 
 This looks OK to me with one caveat.
 
 In several of the patches you're replacing a static initialisation with a
 runtime one, and you're doing the runtime initialisation in xxx_setup_arch().
 That's reasonably late, so I'd prefer you did it in xxx_probe().

Heh, I had it in xxx_probe() originally and then realized that

  a) the power off function is basically a driver. Driver initialization
happens in xxx_setup_arch() and

  b) the maple target already does overwrite its power_off callback in
xxx_setup_arch and

  c) on all targets xxx_probe() is very slim and doesn't do much

but I'll happily change it back to put the bits in xxx_probe() instead.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 03/20] powerpc/47x: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

On 05.10.14 02:39, Segher Boessenkool wrote:
 On Wed, Oct 01, 2014 at 03:27:49PM +0200, Alexander Graf wrote:
 diff --git a/arch/powerpc/platforms/44x/ppc476.c 
 b/arch/powerpc/platforms/44x/ppc476.c
 index 33986c1..7027015 100644
 --- a/arch/powerpc/platforms/44x/ppc476.c
 +++ b/arch/powerpc/platforms/44x/ppc476.c
 @@ -94,7 +94,7 @@ static int avr_probe(struct i2c_client *client,
  {
  avr_i2c_client = client;
  ppc_md.restart = avr_reset_system;
 -ppc_md.power_off = avr_power_off_system;
 +pm_power_off = avr_power_off_system
  return 0;
  }
 
 That doesn't compile afaics?

Oops :(. Is there any easy way to test compile all targets?


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 05/20] powerpc/mpc8349emitx: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c 
b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index e238b6a..a3b7a1f 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -166,10 +166,10 @@ static int mcu_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
if (ret)
goto err;
 
-   /* XXX: this is potentially racy, but there is no lock for ppc_md */
-   if (!ppc_md.power_off) {
+   /* XXX: this is potentially racy, but there is no lock for pm_power_off 
*/
+   if (!pm_power_off) {
glob_mcu = mcu;
-   ppc_md.power_off = mcu_power_off;
+   pm_power_off = mcu_power_off;
dev_info(client-dev, will provide power-off service\n);
}
 
@@ -196,7 +196,7 @@ static int mcu_remove(struct i2c_client *client)
device_remove_file(client-dev, dev_attr_status);
 
if (glob_mcu == mcu) {
-   ppc_md.power_off = NULL;
+   pm_power_off = NULL;
glob_mcu = NULL;
}
 
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 07/20] powerpc/85xx/sgy_cts1000: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/85xx/sgy_cts1000.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c 
b/arch/powerpc/platforms/85xx/sgy_cts1000.c
index bb75add..33f16bb 100644
--- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
+++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
@@ -120,7 +120,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
 
/* Register our halt function */
ppc_md.halt = gpio_halt_cb;
-   ppc_md.power_off = gpio_halt_cb;
+   pm_power_off = gpio_halt_cb;
 
printk(KERN_INFO gpio-halt: registered GPIO %d (%d trigger, %d
irq).\n, gpio, trigger, irq);
@@ -137,7 +137,7 @@ static int gpio_halt_remove(struct platform_device *pdev)
free_irq(irq, halt_node);
 
ppc_md.halt = NULL;
-   ppc_md.power_off = NULL;
+   pm_power_off = NULL;
 
gpio_free(gpio);
 
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 08/20] powerpc/celleb: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/cell/celleb_setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/celleb_setup.c 
b/arch/powerpc/platforms/cell/celleb_setup.c
index 1d5a4d8..011cc88 100644
--- a/arch/powerpc/platforms/cell/celleb_setup.c
+++ b/arch/powerpc/platforms/cell/celleb_setup.c
@@ -130,6 +130,7 @@ static void __init celleb_setup_arch_beat(void)
 #endif
 
celleb_setup_arch_common();
+   pm_power_off = beat_power_off;
 }
 
 static int __init celleb_probe_beat(void)
@@ -178,6 +179,7 @@ static void __init celleb_setup_arch_native(void)
/* XXX: nvram initialization should be added */
 
celleb_setup_arch_common();
+   pm_power_off = rtas_power_off;
 }
 
 static int __init celleb_probe_native(void)
@@ -204,7 +206,6 @@ define_machine(celleb_beat) {
.setup_arch = celleb_setup_arch_beat,
.show_cpuinfo   = celleb_show_cpuinfo,
.restart= beat_restart,
-   .power_off  = beat_power_off,
.halt   = beat_halt,
.get_rtc_time   = beat_get_rtc_time,
.set_rtc_time   = beat_set_rtc_time,
@@ -230,7 +231,6 @@ define_machine(celleb_native) {
.setup_arch = celleb_setup_arch_native,
.show_cpuinfo   = celleb_show_cpuinfo,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.get_boot_time  = rtas_get_boot_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 04/20] powerpc/52xx/efika: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/52xx/efika.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/52xx/efika.c 
b/arch/powerpc/platforms/52xx/efika.c
index 3feffde..485a470 100644
--- a/arch/powerpc/platforms/52xx/efika.c
+++ b/arch/powerpc/platforms/52xx/efika.c
@@ -194,6 +194,8 @@ static void __init efika_setup_arch(void)
mpc52xx_pm_init();
 #endif
 
+   pm_power_off = rtas_power_off;
+
if (ppc_md.progress)
ppc_md.progress(Linux/PPC  UTS_RELEASE  running on Efika 
;-)\n, 0x0);
 }
@@ -225,7 +227,6 @@ define_machine(efika)
.init_IRQ   = mpc52xx_init_irq,
.get_irq= mpc52xx_get_irq,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.set_rtc_time   = rtas_set_rtc_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 03/20] powerpc/47x: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/44x/ppc476.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/44x/ppc476.c 
b/arch/powerpc/platforms/44x/ppc476.c
index 33986c1..7027015 100644
--- a/arch/powerpc/platforms/44x/ppc476.c
+++ b/arch/powerpc/platforms/44x/ppc476.c
@@ -94,7 +94,7 @@ static int avr_probe(struct i2c_client *client,
 {
avr_i2c_client = client;
ppc_md.restart = avr_reset_system;
-   ppc_md.power_off = avr_power_off_system;
+   pm_power_off = avr_power_off_system
return 0;
 }
 
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 02/20] powerpc/xmon: Support either ppc_md.power_off or pm_power_off

[Alexander Graf]

Xmon can manually turn off the machine. We now have 2 code paths for this:

  1) ppc_md.power_off
  2) pm_power_off

This patch allows xmon to support both and makes sure it graciously allows
a path to not be implemented.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/xmon/xmon.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index b988b5a..531f649 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -981,7 +981,10 @@ static void bootcmds(void)
else if (cmd == 'h')
ppc_md.halt();
else if (cmd == 'p')
-   ppc_md.power_off();
+   if (ppc_md.power_off)
+   ppc_md.power_off();
+   if (pm_power_off)
+   pm_power_off();
 }
 
 static int cpu_cmd(void)
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 13/20] powerpc/6xx/linkstation: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/embedded6xx/linkstation.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/embedded6xx/linkstation.c 
b/arch/powerpc/platforms/embedded6xx/linkstation.c
index 455e7c08..e397e36 100644
--- a/arch/powerpc/platforms/embedded6xx/linkstation.c
+++ b/arch/powerpc/platforms/embedded6xx/linkstation.c
@@ -70,6 +70,8 @@ static void __init linkstation_setup_arch(void)
for_each_compatible_node(np, pci, mpc10x-pci)
linkstation_add_bridge(np);
 
+   pm_power_off = linkstation_power_off;
+
printk(KERN_INFO BUFFALO Network Attached Storage Series\n);
printk(KERN_INFO (C) 2002-2005 BUFFALO INC.\n);
 }
@@ -158,7 +160,6 @@ define_machine(linkstation){
.show_cpuinfo   = linkstation_show_cpuinfo,
.get_irq= mpic_get_irq,
.restart= linkstation_restart,
-   .power_off  = linkstation_power_off,
.halt   = linkstation_halt,
.calibrate_decr = generic_calibrate_decr,
 };
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 06/20] powerpc/corenet: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/85xx/corenet_generic.c | 2 +-
 arch/powerpc/sysdev/fsl_soc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c 
b/arch/powerpc/platforms/85xx/corenet_generic.c
index d22dd85..89fd568 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -156,7 +156,7 @@ static int __init corenet_generic_probe(void)
 
ppc_md.get_irq = ehv_pic_get_irq;
ppc_md.restart = fsl_hv_restart;
-   ppc_md.power_off = fsl_hv_halt;
+   pm_power_off = fsl_hv_halt;
ppc_md.halt = fsl_hv_halt;
 #ifdef CONFIG_SMP
/*
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index ffd1169..1e04568 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -238,7 +238,7 @@ void fsl_hv_restart(char *cmd)
 /*
  * Halt the current partition
  *
- * This function should be assigned to the ppc_md.power_off and ppc_md.halt
+ * This function should be assigned to the pm_power_off and ppc_md.halt
  * function pointers, to shut down the partition when we're running under
  * the Freescale hypervisor.
  */
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 09/20] powerpc/cell/qpace: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/cell/qpace_setup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/cell/qpace_setup.c 
b/arch/powerpc/platforms/cell/qpace_setup.c
index 6e3409d..8488094 100644
--- a/arch/powerpc/platforms/cell/qpace_setup.c
+++ b/arch/powerpc/platforms/cell/qpace_setup.c
@@ -117,6 +117,8 @@ static void __init qpace_setup_arch(void)
 #ifdef CONFIG_DUMMY_CONSOLE
conswitchp = dummy_con;
 #endif
+
+   pm_power_off = rtas_power_off;
 }
 
 static int __init qpace_probe(void)
@@ -137,7 +139,6 @@ define_machine(qpace) {
.setup_arch = qpace_setup_arch,
.show_cpuinfo   = qpace_show_cpuinfo,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.get_boot_time  = rtas_get_boot_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 01/20] powerpc: Support override of pm_power_off

[Alexander Graf]

The pm_power_off callback is what drivers are supposed to modify when they
implement power off support for the system.

Support a modified callback on powerpc. That way power off support code can
now either override ppc_md.power_off or pm_power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/kernel/setup-common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/setup-common.c 
b/arch/powerpc/kernel/setup-common.c
index 1b0e260..5dfcb28 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -140,6 +140,8 @@ void machine_power_off(void)
machine_shutdown();
if (ppc_md.power_off)
ppc_md.power_off();
+   if (pm_power_off != machine_power_off)
+   pm_power_off();
 #ifdef CONFIG_SMP
smp_send_stop();
 #endif
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 11/20] powerpc/chrp: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/chrp/setup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/chrp/setup.c 
b/arch/powerpc/platforms/chrp/setup.c
index 7044fd3..f9ad816 100644
--- a/arch/powerpc/platforms/chrp/setup.c
+++ b/arch/powerpc/platforms/chrp/setup.c
@@ -356,6 +356,8 @@ void __init chrp_setup_arch(void)
 
pci_create_OF_bus_map();
 
+   pm_power_off = rtas_power_off,
+
/*
 * Print the banner, then scroll down so boot progress
 * can be printed.  -- Cort
@@ -597,7 +599,6 @@ define_machine(chrp) {
.show_cpuinfo   = chrp_show_cpuinfo,
.init_IRQ   = chrp_init_IRQ,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.time_init  = chrp_time_init,
.set_rtc_time   = chrp_set_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 17/20] powerpc/powernv: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/powernv/setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/setup.c 
b/arch/powerpc/platforms/powernv/setup.c
index 5a0e2dc..60d137b 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -260,7 +260,7 @@ static void __init pnv_setup_machdep_opal(void)
ppc_md.get_rtc_time = opal_get_rtc_time;
ppc_md.set_rtc_time = opal_set_rtc_time;
ppc_md.restart = pnv_restart;
-   ppc_md.power_off = pnv_power_off;
+   pm_power_off = pnv_power_off;
ppc_md.halt = pnv_halt;
ppc_md.machine_check_exception = opal_machine_check;
ppc_md.mce_check_early_recovery = opal_mce_check_early_recovery;
@@ -277,7 +277,7 @@ static void __init pnv_setup_machdep_rtas(void)
ppc_md.set_rtc_time = rtas_set_rtc_time;
}
ppc_md.restart = rtas_restart;
-   ppc_md.power_off = rtas_power_off;
+   pm_power_off = rtas_power_off;
ppc_md.halt = rtas_halt;
 }
 #endif /* CONFIG_PPC_POWERNV_RTAS */
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 15/20] powerpc/maple: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/maple/setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/maple/setup.c 
b/arch/powerpc/platforms/maple/setup.c
index cb1b0b3..34a08db 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -169,7 +169,7 @@ static void __init 
maple_use_rtas_reboot_and_halt_if_present(void)
if (rtas_service_present(system-reboot) 
rtas_service_present(power-off)) {
ppc_md.restart = rtas_restart;
-   ppc_md.power_off = rtas_power_off;
+   pm_power_off = rtas_power_off;
ppc_md.halt = rtas_halt;
}
 }
@@ -189,6 +189,7 @@ void __init maple_setup_arch(void)
 #ifdef CONFIG_DUMMY_CONSOLE
conswitchp = dummy_con;
 #endif
+   pm_power_off = maple_power_off;
maple_use_rtas_reboot_and_halt_if_present();
 
printk(KERN_DEBUG Using native/NAP idle loop\n);
@@ -325,7 +326,6 @@ define_machine(maple) {
.pci_irq_fixup  = maple_pci_irq_fixup,
.pci_get_legacy_ide_irq = maple_pci_get_legacy_ide_irq,
.restart= maple_restart,
-   .power_off  = maple_power_off,
.halt   = maple_halt,
.get_boot_time  = maple_get_boot_time,
.set_rtc_time   = maple_set_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 18/20] powerpc/ps3: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/ps3/setup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/ps3/setup.c 
b/arch/powerpc/platforms/ps3/setup.c
index 3f509f8..ce3b455 100644
--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -223,6 +223,8 @@ static void __init ps3_setup_arch(void)
ppc_md.power_save = ps3_power_save;
ps3_os_area_init();
 
+   pm_power_off = ps3_power_off;
+
DBG( - %s:%d\n, __func__, __LINE__);
 }
 
@@ -278,7 +280,6 @@ define_machine(ps3) {
.calibrate_decr = ps3_calibrate_decr,
.progress   = ps3_progress,
.restart= ps3_restart,
-   .power_off  = ps3_power_off,
.halt   = ps3_halt,
 #if defined(CONFIG_KEXEC)
.kexec_cpu_down = ps3_kexec_cpu_down,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

The generic Linux framework to power off the machine is a function pointer
called pm_power_off. The trick about this pointer is that device drivers can
potentially implement it rather than board files.

Today on PowerPC we set pm_power_off to invoke our generic full machine power
off logic which then calls ppc_md.power_off to invoke machine specific power
off.

However, when we want to add a power off GPIO via the gpio-poweroff driver,
this card house falls apart. That driver only registers itself if pm_power_off
is NULL to ensure it doesn't override board specific logic. However, since we
always set pm_power_off to the generic power off logic (which will just not
power off the machine if no ppc_md.power_off call is implemented), we can't
implement power off via the generic GPIO power off driver.

To fix this up, let's get rid of the ppc_md.power_off logic and just always use
pm_power_off as was intended. Then individual drivers such as the GPIO power off
driver can implement power off logic via that function pointer.

With this patch set applied and a few patches on top of QEMU that implement a
power off GPIO on the virt e500 machine, I can successfully turn off my virtual
machine after halt.


Alex

Alexander Graf (20):
  powerpc: Support override of pm_power_off
  powerpc/xmon: Support either ppc_md.power_off or pm_power_off
  powerpc/47x: Use pm_power_off rather than ppc_md.power_off
  powerpc/52xx/efika: Use pm_power_off rather than ppc_md.power_off
  powerpc/mpc8349emitx: Use pm_power_off rather than ppc_md.power_off
  powerpc/corenet: Use pm_power_off rather than ppc_md.power_off
  powerpc/85xx/sgy_cts1000: Use pm_power_off rather than
ppc_md.power_off
  powerpc/celleb: Use pm_power_off rather than ppc_md.power_off
  powerpc/cell/qpace: Use pm_power_off rather than ppc_md.power_off
  powerpc/cell: Use pm_power_off rather than ppc_md.power_off
  powerpc/chrp: Use pm_power_off rather than ppc_md.power_off
  powerpc/6xx/gamecube: Use pm_power_off rather than ppc_md.power_off
  powerpc/6xx/linkstation: Use pm_power_off rather than ppc_md.power_off
  powerpc/6xx/wii: Use pm_power_off rather than ppc_md.power_off
  powerpc/maple: Use pm_power_off rather than ppc_md.power_off
  powerpc/powermac: Use pm_power_off rather than ppc_md.power_off
  powerpc/powernv: Use pm_power_off rather than ppc_md.power_off
  powerpc/ps3: Use pm_power_off rather than ppc_md.power_off
  powerpc/pseries: Use pm_power_off rather than ppc_md.power_off
  powerpc: Remove ppc_md.power_off

 arch/powerpc/include/asm/machdep.h   |   1 -
 arch/powerpc/kernel/setup-common.c   |   6 +-
 arch/powerpc/platforms/44x/ppc476.c  |   2 +-
 arch/powerpc/platforms/52xx/efika.c  |   3 +-
 arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c   |   8 +-
 arch/powerpc/platforms/85xx/corenet_generic.c|   2 +-
 arch/powerpc/platforms/85xx/sgy_cts1000.c|   4 +-
 arch/powerpc/platforms/cell/celleb_setup.c   |   4 +-
 arch/powerpc/platforms/cell/qpace_setup.c|   3 +-
 arch/powerpc/platforms/cell/setup.c  |   2 +-
 arch/powerpc/platforms/chrp/setup.c  |   3 +-
 arch/powerpc/platforms/embedded6xx/gamecube.c|   3 +-
 arch/powerpc/platforms/embedded6xx/linkstation.c |   3 +-
 arch/powerpc/platforms/embedded6xx/wii.c |   3 +-
 arch/powerpc/platforms/maple/setup.c |   4 +-
 arch/powerpc/platforms/powermac/setup.c  | 147 ---
 arch/powerpc/platforms/powernv/setup.c   |   4 +-
 arch/powerpc/platforms/ps3/setup.c   |   3 +-
 arch/powerpc/platforms/pseries/setup.c   |  59 -
 arch/powerpc/sysdev/fsl_soc.c|   2 +-
 arch/powerpc/xmon/xmon.c |   3 +-
 21 files changed, 139 insertions(+), 130 deletions(-)

-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 14/20] powerpc/6xx/wii: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/embedded6xx/wii.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c 
b/arch/powerpc/platforms/embedded6xx/wii.c
index 6d8dadf..24d656d 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -158,6 +158,8 @@ static void __init wii_setup_arch(void)
clrbits32(hw_gpio + HW_GPIO_OUT(0),
  HW_GPIO_SLOT_LED | HW_GPIO_SENSOR_BAR);
}
+
+   pm_power_off = wii_power_off;
 }
 
 static void wii_restart(char *cmd)
@@ -226,7 +228,6 @@ define_machine(wii) {
.init_early = wii_init_early,
.setup_arch = wii_setup_arch,
.restart= wii_restart,
-   .power_off  = wii_power_off,
.halt   = wii_halt,
.init_IRQ   = wii_pic_probe,
.get_irq= flipper_pic_get_irq,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 12/20] powerpc/6xx/gamecube: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/embedded6xx/gamecube.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/embedded6xx/gamecube.c 
b/arch/powerpc/platforms/embedded6xx/gamecube.c
index a138e14..3ee7a8b 100644
--- a/arch/powerpc/platforms/embedded6xx/gamecube.c
+++ b/arch/powerpc/platforms/embedded6xx/gamecube.c
@@ -67,6 +67,8 @@ static int __init gamecube_probe(void)
if (!of_flat_dt_is_compatible(dt_root, nintendo,gamecube))
return 0;
 
+   pm_power_off = gamecube_power_off;
+
return 1;
 }
 
@@ -80,7 +82,6 @@ define_machine(gamecube) {
.probe  = gamecube_probe,
.init_early = gamecube_init_early,
.restart= gamecube_restart,
-   .power_off  = gamecube_power_off,
.halt   = gamecube_halt,
.init_IRQ   = flipper_pic_probe,
.get_irq= flipper_pic_get_irq,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 20/20] powerpc: Remove ppc_md.power_off

[Alexander Graf]

Now that we have all implementations of ppc_md.power_off converted to
pm_power_off we can remove the ppc_md variant.

While at it, also set the default for pm_power_off to NULL so that non
machine drivers can implement overrides.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/include/asm/machdep.h | 1 -
 arch/powerpc/kernel/setup-common.c | 6 ++
 arch/powerpc/xmon/xmon.c   | 2 --
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h 
b/arch/powerpc/include/asm/machdep.h
index b125cea..10dc008 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -144,7 +144,6 @@ struct machdep_calls {
 #endif
 
void(*restart)(char *cmd);
-   void(*power_off)(void);
void(*halt)(void);
void(*panic)(char *str);
void(*cpu_die)(void);
diff --git a/arch/powerpc/kernel/setup-common.c 
b/arch/powerpc/kernel/setup-common.c
index 5dfcb28..58a260f 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -138,9 +138,7 @@ void machine_restart(char *cmd)
 void machine_power_off(void)
 {
machine_shutdown();
-   if (ppc_md.power_off)
-   ppc_md.power_off();
-   if (pm_power_off != machine_power_off)
+   if (pm_power_off)
pm_power_off();
 #ifdef CONFIG_SMP
smp_send_stop();
@@ -152,7 +150,7 @@ void machine_power_off(void)
 /* Used by the G5 thermal driver */
 EXPORT_SYMBOL_GPL(machine_power_off);
 
-void (*pm_power_off)(void) = machine_power_off;
+void (*pm_power_off)(void);
 EXPORT_SYMBOL_GPL(pm_power_off);
 
 void machine_halt(void)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 531f649..506d256 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -981,8 +981,6 @@ static void bootcmds(void)
else if (cmd == 'h')
ppc_md.halt();
else if (cmd == 'p')
-   if (ppc_md.power_off)
-   ppc_md.power_off();
if (pm_power_off)
pm_power_off();
 }
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 10/20] powerpc/cell: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/cell/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/cell/setup.c 
b/arch/powerpc/platforms/cell/setup.c
index 6ae25fb..b6a27d7 100644
--- a/arch/powerpc/platforms/cell/setup.c
+++ b/arch/powerpc/platforms/cell/setup.c
@@ -248,6 +248,7 @@ static void __init cell_setup_arch(void)
 #endif
 
mmio_nvram_init();
+   pm_power_off = rtas_power_off;
 }
 
 static int __init cell_probe(void)
@@ -269,7 +270,6 @@ define_machine(cell) {
.setup_arch = cell_setup_arch,
.show_cpuinfo   = cell_show_cpuinfo,
.restart= rtas_restart,
-   .power_off  = rtas_power_off,
.halt   = rtas_halt,
.get_boot_time  = rtas_get_boot_time,
.get_rtc_time   = rtas_get_rtc_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 16/20] powerpc/powermac: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/powermac/setup.c | 147 
 1 file changed, 74 insertions(+), 73 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/setup.c 
b/arch/powerpc/platforms/powermac/setup.c
index 141f8899..d3b0a87 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -274,6 +274,78 @@ static void __init l2cr_init(void)
 }
 #endif
 
+#ifdef CONFIG_ADB_CUDA
+static void cuda_restart(void)
+{
+   struct adb_request req;
+
+   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_RESET_SYSTEM);
+   for (;;)
+   cuda_poll();
+}
+
+static void cuda_shutdown(void)
+{
+   struct adb_request req;
+
+   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_POWERDOWN);
+   for (;;)
+   cuda_poll();
+}
+
+#else
+#define cuda_restart()
+#define cuda_shutdown()
+#endif
+
+#ifndef CONFIG_ADB_PMU
+#define pmu_restart()
+#define pmu_shutdown()
+#endif
+
+#ifndef CONFIG_PMAC_SMU
+#define smu_restart()
+#define smu_shutdown()
+#endif
+
+static void pmac_restart(char *cmd)
+{
+   switch (sys_ctrler) {
+   case SYS_CTRLER_CUDA:
+   cuda_restart();
+   break;
+   case SYS_CTRLER_PMU:
+   pmu_restart();
+   break;
+   case SYS_CTRLER_SMU:
+   smu_restart();
+   break;
+   default: ;
+   }
+}
+
+static void pmac_power_off(void)
+{
+   switch (sys_ctrler) {
+   case SYS_CTRLER_CUDA:
+   cuda_shutdown();
+   break;
+   case SYS_CTRLER_PMU:
+   pmu_shutdown();
+   break;
+   case SYS_CTRLER_SMU:
+   smu_shutdown();
+   break;
+   default: ;
+   }
+}
+
+static void
+pmac_halt(void)
+{
+   pmac_power_off();
+}
+
 static void __init pmac_setup_arch(void)
 {
struct device_node *cpu, *ic;
@@ -341,6 +413,8 @@ static void __init pmac_setup_arch(void)
__adb_probe_sync = 1;
}
 #endif /* CONFIG_ADB */
+
+   pm_power_off = pmac_power_off;
 }
 
 #ifdef CONFIG_SCSI
@@ -382,78 +456,6 @@ void __init_refok note_bootable_part(dev_t dev, int part, 
int goodness)
current_root_goodness = goodness;
 }
 
-#ifdef CONFIG_ADB_CUDA
-static void cuda_restart(void)
-{
-   struct adb_request req;
-
-   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_RESET_SYSTEM);
-   for (;;)
-   cuda_poll();
-}
-
-static void cuda_shutdown(void)
-{
-   struct adb_request req;
-
-   cuda_request(req, NULL, 2, CUDA_PACKET, CUDA_POWERDOWN);
-   for (;;)
-   cuda_poll();
-}
-
-#else
-#define cuda_restart()
-#define cuda_shutdown()
-#endif
-
-#ifndef CONFIG_ADB_PMU
-#define pmu_restart()
-#define pmu_shutdown()
-#endif
-
-#ifndef CONFIG_PMAC_SMU
-#define smu_restart()
-#define smu_shutdown()
-#endif
-
-static void pmac_restart(char *cmd)
-{
-   switch (sys_ctrler) {
-   case SYS_CTRLER_CUDA:
-   cuda_restart();
-   break;
-   case SYS_CTRLER_PMU:
-   pmu_restart();
-   break;
-   case SYS_CTRLER_SMU:
-   smu_restart();
-   break;
-   default: ;
-   }
-}
-
-static void pmac_power_off(void)
-{
-   switch (sys_ctrler) {
-   case SYS_CTRLER_CUDA:
-   cuda_shutdown();
-   break;
-   case SYS_CTRLER_PMU:
-   pmu_shutdown();
-   break;
-   case SYS_CTRLER_SMU:
-   smu_shutdown();
-   break;
-   default: ;
-   }
-}
-
-static void
-pmac_halt(void)
-{
-   pmac_power_off();
-}
-
 /* 
  * Early initialization.
  */
@@ -663,7 +665,6 @@ define_machine(powermac) {
.get_irq= NULL, /* changed later */
.pci_irq_fixup  = pmac_pci_irq_fixup,
.restart= pmac_restart,
-   .power_off  = pmac_power_off,
.halt   = pmac_halt,
.time_init  = pmac_time_init,
.get_boot_time  = pmac_get_boot_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 19/20] powerpc/pseries: Use pm_power_off rather than ppc_md.power_off

[Alexander Graf]

The generic power off callback is pm_power_off. Use that one rather than
the powerpc specific ppc_md.power_off.

Signed-off-by: Alexander Graf ag...@suse.de
---
 arch/powerpc/platforms/pseries/setup.c | 59 +-
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/setup.c 
b/arch/powerpc/platforms/pseries/setup.c
index e724d31..981e9ee 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -460,6 +460,34 @@ static long pseries_little_endian_exceptions(void)
 }
 #endif
 
+/**
+ * pSeries_power_off - tell firmware about how to power off the system.
+ *
+ * This function calls either the power-off rtas token in normal cases
+ * or the ibm,power-off-ups token (if present  requested) in case of
+ * a power failure. If power-off token is used, power on will only be
+ * possible with power button press. If ibm,power-off-ups token is used
+ * it will allow auto poweron after power is restored.
+ */
+static void pSeries_power_off(void)
+{
+   int rc;
+   int rtas_poweroff_ups_token = rtas_token(ibm,power-off-ups);
+
+   if (rtas_flash_term_hook)
+   rtas_flash_term_hook(SYS_POWER_OFF);
+
+   if (rtas_poweron_auto == 0 ||
+   rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
+   rc = rtas_call(rtas_token(power-off), 2, 1, NULL, -1, -1);
+   printk(KERN_INFO RTAS power-off returned %d\n, rc);
+   } else {
+   rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
+   printk(KERN_INFO RTAS ibm,power-off-ups returned %d\n, rc);
+   }
+   for (;;);
+}
+
 static void __init pSeries_setup_arch(void)
 {
set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);
@@ -504,6 +532,8 @@ static void __init pSeries_setup_arch(void)
%ld\n, rc);
}
}
+
+   pm_power_off = pSeries_power_off;
 }
 
 static int __init pSeries_init_panel(void)
@@ -754,34 +784,6 @@ static int pSeries_pci_probe_mode(struct pci_bus *bus)
return PCI_PROBE_NORMAL;
 }
 
-/**
- * pSeries_power_off - tell firmware about how to power off the system.
- *
- * This function calls either the power-off rtas token in normal cases
- * or the ibm,power-off-ups token (if present  requested) in case of
- * a power failure. If power-off token is used, power on will only be
- * possible with power button press. If ibm,power-off-ups token is used
- * it will allow auto poweron after power is restored.
- */
-static void pSeries_power_off(void)
-{
-   int rc;
-   int rtas_poweroff_ups_token = rtas_token(ibm,power-off-ups);
-
-   if (rtas_flash_term_hook)
-   rtas_flash_term_hook(SYS_POWER_OFF);
-
-   if (rtas_poweron_auto == 0 ||
-   rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
-   rc = rtas_call(rtas_token(power-off), 2, 1, NULL, -1, -1);
-   printk(KERN_INFO RTAS power-off returned %d\n, rc);
-   } else {
-   rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
-   printk(KERN_INFO RTAS ibm,power-off-ups returned %d\n, rc);
-   }
-   for (;;);
-}
-
 #ifndef CONFIG_PCI
 void pSeries_final_fixup(void) { }
 #endif
@@ -796,7 +798,6 @@ define_machine(pseries) {
.pcibios_fixup  = pSeries_final_fixup,
.pci_probe_mode = pSeries_pci_probe_mode,
.restart= rtas_restart,
-   .power_off  = pSeries_power_off,
.halt   = rtas_halt,
.panic  = rtas_os_term,
.get_boot_time  = rtas_get_boot_time,
-- 
1.8.1.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 01.10.14 16:33, Geert Uytterhoeven wrote:
 Hi Alex,
 
 On Wed, Oct 1, 2014 at 3:27 PM, Alexander Graf ag...@suse.de wrote:
 The generic Linux framework to power off the machine is a function pointer
 called pm_power_off. The trick about this pointer is that device drivers can
 potentially implement it rather than board files.

 Today on PowerPC we set pm_power_off to invoke our generic full machine power
 off logic which then calls ppc_md.power_off to invoke machine specific power
 off.

 However, when we want to add a power off GPIO via the gpio-poweroff driver,
 this card house falls apart. That driver only registers itself if 
 pm_power_off
 is NULL to ensure it doesn't override board specific logic. However, since we
 always set pm_power_off to the generic power off logic (which will just not
 power off the machine if no ppc_md.power_off call is implemented), we can't
 implement power off via the generic GPIO power off driver.

 To fix this up, let's get rid of the ppc_md.power_off logic and just always 
 use
 pm_power_off as was intended. Then individual drivers such as the GPIO power 
 off
 driver can implement power off logic via that function pointer.

 With this patch set applied and a few patches on top of QEMU that implement a
 power off GPIO on the virt e500 machine, I can successfully turn off my 
 virtual
 machine after halt.
 
 This is touching the same area as last night's
 [RFC PATCH 00/16] kernel: Add support for poweroff handler call chain
 https://lkml.org/lkml/2014/9/30/575

I agree, and I think your patch set is walking into a reasonable
direction. However, I really think it should convert all users of
pm_power_off - at which point you'll probably get to the same conclusion
that ppc_md.power_off is a bad idea :).

So in a way, this patch set is semantically a prerequisite to the full
conversion you'd probably like to do :).

Also, in your cover letter you describe that some methods power off the
CPU power while others power off the system power. How do you
distinguish between them with a call chain? You probably won't get
around to trigger the system power off callback after the CPU power off
callback ran ;).


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 01.10.14 17:54, Guenter Roeck wrote:
 On Wed, Oct 01, 2014 at 04:47:23PM +0200, Alexander Graf wrote:


 On 01.10.14 16:33, Geert Uytterhoeven wrote:
 Hi Alex,

 On Wed, Oct 1, 2014 at 3:27 PM, Alexander Graf ag...@suse.de wrote:
 The generic Linux framework to power off the machine is a function pointer
 called pm_power_off. The trick about this pointer is that device drivers 
 can
 potentially implement it rather than board files.

 Today on PowerPC we set pm_power_off to invoke our generic full machine 
 power
 off logic which then calls ppc_md.power_off to invoke machine specific 
 power
 off.

 However, when we want to add a power off GPIO via the gpio-poweroff 
 driver,
 this card house falls apart. That driver only registers itself if 
 pm_power_off
 is NULL to ensure it doesn't override board specific logic. However, since 
 we
 always set pm_power_off to the generic power off logic (which will just not
 power off the machine if no ppc_md.power_off call is implemented), we can't
 implement power off via the generic GPIO power off driver.

 To fix this up, let's get rid of the ppc_md.power_off logic and just 
 always use
 pm_power_off as was intended. Then individual drivers such as the GPIO 
 power off
 driver can implement power off logic via that function pointer.

 With this patch set applied and a few patches on top of QEMU that 
 implement a
 power off GPIO on the virt e500 machine, I can successfully turn off my 
 virtual
 machine after halt.

 This is touching the same area as last night's
 [RFC PATCH 00/16] kernel: Add support for poweroff handler call chain
 https://lkml.org/lkml/2014/9/30/575

 I agree, and I think your patch set is walking into a reasonable
 direction. However, I really think it should convert all users of
 pm_power_off - at which point you'll probably get to the same conclusion
 that ppc_md.power_off is a bad idea :).

 Yes, that would be the ultimate goal.
 
 So in a way, this patch set is semantically a prerequisite to the full
 conversion you'd probably like to do :).

 Also, in your cover letter you describe that some methods power off the
 CPU power while others power off the system power. How do you
 distinguish between them with a call chain? You probably won't get
 around to trigger the system power off callback after the CPU power off
 callback ran ;).

 Those are examples. Don't get hung up on it. I may actually replace the
 CPU example with something better in the next version; it is not really
 a good example and may get people stuck on why on earth would anyone want
 or need a means to turn off the CPU power instead of focusing on the problem
 the patch set tries to solve.
 
 The basic problem is that there can be different poweroff handlers,
 some of which may not be available on some systems, and some may not
 be as desirable as others for various reasons. The code registering
 those poweroff handlers does not specify the poweroff method, but its 
 priority. It would be up to the programmer (hopefully together with
 the board designer) to determine which method should have higher priority.
 Taking the above example, the callback to turn off CPU power would presumably
 be one of last resort, and have a very low priority.
 
 A better example may actually be patch 15/16 of the series. The affected
 driver (drivers/power/reset/restart-poweroff.c) does not really power off
 the system, but restarts it instead. Obviously that would only be a poweroff
 handler of last resort, which should only be executed if no other means
 to power off the system is available.

Sounds like a good plan :). You probably want to have some global list
of priority numbers like try this first or this is a non-optimal, but
working method and only ever do this as last resort.

Maybe you could as a first step convert every user of pm_power_off to
this new framework with a global notifier_block, similar to how
pm_power_off is a global today? Then we can at least get rid of
pm_power_off altogether and move to only notifiers, whereas new
notifiers can come before or after the old machine set implementations.

As a nice bonus this automatically converts every user of pm_power_off()
to instead call the notifier chain.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 02.10.14 00:39, Scott Wood wrote:
 On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
 The generic Linux framework to power off the machine is a function pointer
 called pm_power_off. The trick about this pointer is that device drivers can
 potentially implement it rather than board files.

 Today on PowerPC we set pm_power_off to invoke our generic full machine power
 off logic which then calls ppc_md.power_off to invoke machine specific power
 off.

 However, when we want to add a power off GPIO via the gpio-poweroff driver,
 this card house falls apart. That driver only registers itself if 
 pm_power_off
 is NULL to ensure it doesn't override board specific logic. However, since we
 always set pm_power_off to the generic power off logic (which will just not
 power off the machine if no ppc_md.power_off call is implemented), we can't
 implement power off via the generic GPIO power off driver.

 To fix this up, let's get rid of the ppc_md.power_off logic and just always 
 use
 pm_power_off as was intended. Then individual drivers such as the GPIO power 
 off
 driver can implement power off logic via that function pointer.

 With this patch set applied and a few patches on top of QEMU that implement a
 power off GPIO on the virt e500 machine, I can successfully turn off my 
 virtual
 machine after halt.
 
 Are there any plans to handle restart similarly?

I don't see an immediate need for it, as we do have access to reset via
the guts device.

I also don't see any generic driver in Linux that would implement reset
via a gpio controller device, so apparently it's not incredibly common
to implement reset via gpio.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 00/20] powerpc: Convert power off logic to pm_power_off

[Alexander Graf]

On 02.10.14 01:28, Scott Wood wrote:
 On Thu, 2014-10-02 at 01:21 +0200, Alexander Graf wrote:

 On 02.10.14 00:39, Scott Wood wrote:
 On Wed, 2014-10-01 at 15:27 +0200, Alexander Graf wrote:
 The generic Linux framework to power off the machine is a function pointer
 called pm_power_off. The trick about this pointer is that device drivers 
 can
 potentially implement it rather than board files.

 Today on PowerPC we set pm_power_off to invoke our generic full machine 
 power
 off logic which then calls ppc_md.power_off to invoke machine specific 
 power
 off.

 However, when we want to add a power off GPIO via the gpio-poweroff 
 driver,
 this card house falls apart. That driver only registers itself if 
 pm_power_off
 is NULL to ensure it doesn't override board specific logic. However, since 
 we
 always set pm_power_off to the generic power off logic (which will just not
 power off the machine if no ppc_md.power_off call is implemented), we can't
 implement power off via the generic GPIO power off driver.

 To fix this up, let's get rid of the ppc_md.power_off logic and just 
 always use
 pm_power_off as was intended. Then individual drivers such as the GPIO 
 power off
 driver can implement power off logic via that function pointer.

 With this patch set applied and a few patches on top of QEMU that 
 implement a
 power off GPIO on the virt e500 machine, I can successfully turn off my 
 virtual
 machine after halt.

 Are there any plans to handle restart similarly?

 I don't see an immediate need for it, as we do have access to reset via
 the guts device.
 
 The guts device is problematic from a hardware description perspective,
 as we advertise a bunch of things to the guest that we don't implement.
 E.g. the only reason we don't currently have a problem with Linux trying
 to use guts to sync the timebase across cores is that by chance we
 happen to expose a guts that corresponds to a single-cpu SoC.

True, and it is slightly ugly to expose a specific SoC's guts in our
generic pv machine type.

 
 I also don't see any generic driver in Linux that would implement reset
 via a gpio controller device, so apparently it's not incredibly common
 to implement reset via gpio.
 
 There wasn't a generic gpio-power-off driver either until a couple years
 ago...  Regardless of whether it's done via gpio, using ppc_md for it is
 bad for the same reasons as for power-off.

I agree. Today, only ARM has a comparable mechanism for drivers to hook
a reset handler into the machine specific reset path.

I'd say let's wait for Guenter to finish the power off rework and then
tackle a generic reset call chain based approach next.


Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev