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

2015-07-16 Thread Thomas Huth
On 05/27/2015 01:56 AM, 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 

Reviewed-by: Thomas Huth 

Actually this patch also fixes a bug that SLOF sometimes crashes when a
vCPU gets kicked out of kernel mode (see the following URL for details:
https://bugzilla.redhat.com/show_bug.cgi?id=1178502 ), and I've just
tested that this bug does not occur with this patch anymore, so also:

Tested-by: Thomas Huth 

--
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


Re: BUG: sleeping function called from ras_epow_interrupt context

2015-07-16 Thread Nathan Fontenot
On 07/16/2015 01:23 AM, Thomas Huth wrote:
> On 07/15/2015 09:58 PM, Nathan Fontenot wrote:
>> On 07/15/2015 09:35 AM, Thomas Huth wrote:
>>> On 07/14/2015 11:22 PM, Benjamin Herrenschmidt wrote:
 On Tue, 2015-07-14 at 20:43 +0200, Thomas Huth wrote:
> Any suggestions how to fix this? Simply revert 587f83e8dd50d? Use
> mdelay() instead of msleep() in rtas_busy_delay()? Something more
> fancy?

 A proper fix would be more fancy, the get_sensor should happen in a
 kernel thread instead.
>>>
>>> I'm not very familiar with this stuff, but isn't the EPOW interrupt
>>> something that is very time-critical? Moving parts of the handler into a
>>> kernel thread then does not sound like a very good idea to me...
>>>
>>> Another question: Can it happen at all that this get-sensor call results
>>> in a sleep condition? Looking at commit ID
>>> 81b73dd92b97423b8f5324a59044da478c04f4c4 ("Fix might-sleep warning on
>>> removing cpus"), which apparently fixed a similar issue for CPU
>>> hot-plugging, indicates that at least some of the rtas calls are never
>>> returning the busy code? In that case we could fix this by introducing a
>>> similar rtas_get_sensor_fast() function? (or simply revert 587f83e8dd50d
>>> which would be quite similar, I think)
>>>
>>
>> Looking at the PAPR, the get-sensor-state rtas call for the EPOW sensor
>> is listed as a fast call and should not return a busy indication.
> 
> Great, good to know, thanks for looking that up! So IMHO we should
> either introduce a rtas_get_sensor_fast() function or revert
> 587f83e8dd50d ... any preferences? Shall I come up with a patch?
>
A quick look at the kernel, I only find three places that rtas_get_sensor
is called. The instance you point out here for the EPOW sensor is the
only time I find it called for a sensor that should not return a busy
indication.

Reverting commit 587f83e8dd50d would solve the issue but not fix any future
users of a fast get-sensor call. I don't have an issue with a patch for a
rtas_get_sensor_fast().

-Nathan
 
>> I'm curious as to why we're getting a busy return indication when
>> making this call.
> 
> Looking at the code again, rtas_busy_delay() likely never slept ... it's
> likely just the "might_sleep()" annotation in that function that causes
> the BUG.
> 
>  Thomas
> 
> 

--
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


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

2015-07-16 Thread Laurent Vivier
On 27/05/2015 01:56, 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 
> ---
> 
> v3:
> Adjust booke set/get xer to match book3s.
> 
> 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/include/asm/kvm_booke.h  |4 ++--
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S   |6 +++---
>  arch/powerpc/kvm/book3s_segment.S |4 ++--
>  5 files changed, 10 insertions(+), 10 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/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/include/asm/kvm_booke.h 
> b/arch/powerpc/include/asm/kvm_booke.h
> index 3286f0d..bc6e29e 100644
> --- a/arch/powerpc/include/asm/kvm_booke.h
> +++ b/arch/powerpc/include/asm/kvm_booke.h
> @@ -54,12 +54,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)
>  
>   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)
> 


Reviewed-by: Laurent Vivier 
--
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


Re: [PATCH 0/2] Two fixes for dynamic micro-threading

2015-07-16 Thread Laurent Vivier


On 16/07/2015 09:11, Paul Mackerras wrote:
> This series contains two fixes for the new dynamic micro-threading
> code that was added recently for HV-mode KVM on Power servers.
> The patches are against Alex Graf's kvm-ppc-queue branch.  Please
> apply.
> 
> Paul.
> 
>  arch/powerpc/kvm/book3s_hv.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Tested-by: Laurent Vivier 

[this series has corrected a host kernel crash when CPU are overcommitted]
--
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


[PATCH 1/2] KVM: PPC: Book3S HV: Fix preempted vcore list locking

2015-07-16 Thread Paul Mackerras
When a vcore gets preempted, we put it on the preempted vcore list for
the current CPU.  The runner task then calls schedule() and comes back
some time later and takes itself off the list.  We need to be careful
to lock the list that it was put onto, which may not be the list for the
current CPU since the runner task may have moved to another CPU.

Signed-off-by: Paul Mackerras 
---
 arch/powerpc/kvm/book3s_hv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 6e3ef30..3d02276 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1962,10 +1962,11 @@ static void kvmppc_vcore_preempt(struct kvmppc_vcore 
*vc)
 
 static void kvmppc_vcore_end_preempt(struct kvmppc_vcore *vc)
 {
-   struct preempted_vcore_list *lp = this_cpu_ptr(&preempted_vcores);
+   struct preempted_vcore_list *lp;
 
kvmppc_core_end_stolen(vc);
if (!list_empty(&vc->preempt_list)) {
+   lp = &per_cpu(preempted_vcores, vc->pcpu);
spin_lock(&lp->lock);
list_del_init(&vc->preempt_list);
spin_unlock(&lp->lock);
-- 
2.1.4

--
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


[PATCH 2/2] KVM: PPC: Book3S HV: Fix preempted vcore stolen time calculation

2015-07-16 Thread Paul Mackerras
Whenever a vcore state is VCORE_PREEMPT we need to be counting stolen
time for it.  This currently isn't the case when we have a vcore that
no longer has any runnable threads in it but still has a runner task,
so we do an explicit call to kvmppc_core_start_stolen() in that case.

Signed-off-by: Paul Mackerras 
---
 arch/powerpc/kvm/book3s_hv.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3d02276..fad52f2 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2283,9 +2283,14 @@ static void post_guest_process(struct kvmppc_vcore *vc, 
bool is_master)
}
list_del_init(&vc->preempt_list);
if (!is_master) {
-   vc->vcore_state = vc->runner ? VCORE_PREEMPT : VCORE_INACTIVE;
-   if (still_running > 0)
+   if (still_running > 0) {
kvmppc_vcore_preempt(vc);
+   } else if (vc->runner) {
+   vc->vcore_state = VCORE_PREEMPT;
+   kvmppc_core_start_stolen(vc);
+   } else {
+   vc->vcore_state = VCORE_INACTIVE;
+   }
if (vc->n_runnable > 0 && vc->runner == NULL) {
/* make sure there's a candidate runner awake */
vcpu = list_first_entry(&vc->runnable_threads,
-- 
2.1.4

--
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


[PATCH 0/2] Two fixes for dynamic micro-threading

2015-07-16 Thread Paul Mackerras
This series contains two fixes for the new dynamic micro-threading
code that was added recently for HV-mode KVM on Power servers.
The patches are against Alex Graf's kvm-ppc-queue branch.  Please
apply.

Paul.

 arch/powerpc/kvm/book3s_hv.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

--
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