diff mbox

[v5,3/4] KVM: PPC: epapr: install ev_idle hcall for e500 guest

Message ID 1329799573-8820-3-git-send-email-yu.liu@freescale.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Liu Yu-B13201 Feb. 21, 2012, 4:46 a.m. UTC
If the guest hypervisor node contains "has-idle" property.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
v5: no change

 arch/powerpc/kernel/epapr_hcalls.S   |   29 +++++++++++++++++++++++++++++
 arch/powerpc/kernel/epapr_paravirt.c |   11 ++++++++++-
 2 files changed, 39 insertions(+), 1 deletions(-)

Comments

Tiejun Chen Feb. 21, 2012, 10:53 a.m. UTC | #1
Liu Yu wrote:
> If the guest hypervisor node contains "has-idle" property.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> v5: no change
> 
>  arch/powerpc/kernel/epapr_hcalls.S   |   29 +++++++++++++++++++++++++++++
>  arch/powerpc/kernel/epapr_paravirt.c |   11 ++++++++++-
>  2 files changed, 39 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/epapr_hcalls.S b/arch/powerpc/kernel/epapr_hcalls.S
> index 697b390..72fa234 100644
> --- a/arch/powerpc/kernel/epapr_hcalls.S
> +++ b/arch/powerpc/kernel/epapr_hcalls.S
> @@ -15,6 +15,35 @@
>  #include <asm/ppc_asm.h>
>  #include <asm/asm-offsets.h>
>  
> +#define HC_VENDOR_EPAPR		(1 << 16)
> +#define HC_EV_IDLE		16

Why not use 'EV_IDLE' directly?

> +
> +_GLOBAL(epapr_ev_idle)
> +epapr_ev_idle:
> +	rlwinm	r3,r1,0,0,31-THREAD_SHIFT	/* current thread_info */
> +	lwz	r4,TI_LOCAL_FLAGS(r3)	/* set napping bit */
> +	ori	r4,r4,_TLF_NAPPING	/* so when we take an exception */
> +	stw	r4,TI_LOCAL_FLAGS(r3)	/* it will return to our caller */
> +
> +	wrteei	1
> +
> +idle_loop:
> +	LOAD_REG_IMMEDIATE(r11, HC_VENDOR_EPAPR | HC_EV_IDLE)

And could this line be simplified as something like this:

LOAD_REG_IMMEDIATE(r11, EV_HCALL_TOKEN(EV_IDLE))

If so, even we can remove the previous HC_VENDOR_EPAPR definition as well.

Tiejun

> +
> +.global epapr_ev_idle_start
> +epapr_ev_idle_start:
> +	li	r3, -1
> +	nop
> +	nop
> +	nop
> +
> +	/*
> +	 * Guard against spurious wakeups from a hypervisor --
> +	 * only interrupt will cause us to return to LR due to
> +	 * _TLF_NAPPING.
> +	 */
> +	b	idle_loop
> +
>  /* Hypercall entry point. Will be patched with device tree instructions. */
>  .global epapr_hypercall_start
>  epapr_hypercall_start:
> diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
> index e601da7..43d875e 100644
> --- a/arch/powerpc/kernel/epapr_paravirt.c
> +++ b/arch/powerpc/kernel/epapr_paravirt.c
> @@ -21,6 +21,10 @@
>  #include <asm/epapr_hcalls.h>
>  #include <asm/cacheflush.h>
>  #include <asm/code-patching.h>
> +#include <asm/machdep.h>
> +
> +extern void epapr_ev_idle(void);
> +extern u32 epapr_ev_idle_start[];
>  
>  bool epapr_para_enabled = false;
>  
> @@ -39,8 +43,13 @@ static int __init epapr_para_init(void)
>  
>  	insts = of_get_property(hyper_node, "hcall-instructions", &len);
>  	if (insts && !(len % 4) && len <= (4 * 4)) {
> -		for (i = 0; i < (len / 4); i++)
> +		for (i = 0; i < (len / 4); i++) {
>  			patch_instruction(epapr_hypercall_start + i, insts[i]);
> +			patch_instruction(epapr_ev_idle_start + i, insts[i]);
> +		}
> +
> +		if (of_get_property(hyper_node, "has-idle", NULL))
> +			ppc_md.power_save = epapr_ev_idle;
>  
>  		epapr_para_enabled = true;
>  	} else {
Liu Yu-B13201 Feb. 22, 2012, 2:29 a.m. UTC | #2
> -----Original Message-----
> From: tiejun.chen [mailto:tiejun.chen@windriver.com]
> Sent: Tuesday, February 21, 2012 6:54 PM
> To: Liu Yu-B13201
> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
> linuxppc-dev@ozlabs.org; Wood Scott-B07421
> Subject: Re: [PATCH v5 3/4] KVM: PPC: epapr: install ev_idle hcall for
> e500 guest
> 
> Liu Yu wrote:
> > If the guest hypervisor node contains "has-idle" property.
> >
> > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> > ---
> > v5: no change
> >
> >  arch/powerpc/kernel/epapr_hcalls.S   |   29
> +++++++++++++++++++++++++++++
> >  arch/powerpc/kernel/epapr_paravirt.c |   11 ++++++++++-
> >  2 files changed, 39 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/epapr_hcalls.S
> > b/arch/powerpc/kernel/epapr_hcalls.S
> > index 697b390..72fa234 100644
> > --- a/arch/powerpc/kernel/epapr_hcalls.S
> > +++ b/arch/powerpc/kernel/epapr_hcalls.S
> > @@ -15,6 +15,35 @@
> >  #include <asm/ppc_asm.h>
> >  #include <asm/asm-offsets.h>
> >
> > +#define HC_VENDOR_EPAPR		(1 << 16)
> > +#define HC_EV_IDLE		16
> 
> Why not use 'EV_IDLE' directly?
> 
> > +
> > +_GLOBAL(epapr_ev_idle)
> > +epapr_ev_idle:
> > +	rlwinm	r3,r1,0,0,31-THREAD_SHIFT	/* current thread_info */
> > +	lwz	r4,TI_LOCAL_FLAGS(r3)	/* set napping bit */
> > +	ori	r4,r4,_TLF_NAPPING	/* so when we take an exception */
> > +	stw	r4,TI_LOCAL_FLAGS(r3)	/* it will return to our caller */
> > +
> > +	wrteei	1
> > +
> > +idle_loop:
> > +	LOAD_REG_IMMEDIATE(r11, HC_VENDOR_EPAPR | HC_EV_IDLE)
> 
> And could this line be simplified as something like this:
> 
> LOAD_REG_IMMEDIATE(r11, EV_HCALL_TOKEN(EV_IDLE))
> 
> If so, even we can remove the previous HC_VENDOR_EPAPR definition as well.
> 

Because the epapr_hcalls.h contains C functions,
so it cannot be included by assembly code.

Thanks,
Yu
Tiejun Chen Feb. 22, 2012, 2:51 a.m. UTC | #3
Liu Yu-B13201 wrote:
> 
>> -----Original Message-----
>> From: tiejun.chen [mailto:tiejun.chen@windriver.com]
>> Sent: Tuesday, February 21, 2012 6:54 PM
>> To: Liu Yu-B13201
>> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
>> linuxppc-dev@ozlabs.org; Wood Scott-B07421
>> Subject: Re: [PATCH v5 3/4] KVM: PPC: epapr: install ev_idle hcall for
>> e500 guest
>>
>> Liu Yu wrote:
>>> If the guest hypervisor node contains "has-idle" property.
>>>
>>> Signed-off-by: Liu Yu <yu.liu@freescale.com>
>>> ---
>>> v5: no change
>>>
>>>  arch/powerpc/kernel/epapr_hcalls.S   |   29
>> +++++++++++++++++++++++++++++
>>>  arch/powerpc/kernel/epapr_paravirt.c |   11 ++++++++++-
>>>  2 files changed, 39 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/epapr_hcalls.S
>>> b/arch/powerpc/kernel/epapr_hcalls.S
>>> index 697b390..72fa234 100644
>>> --- a/arch/powerpc/kernel/epapr_hcalls.S
>>> +++ b/arch/powerpc/kernel/epapr_hcalls.S
>>> @@ -15,6 +15,35 @@
>>>  #include <asm/ppc_asm.h>
>>>  #include <asm/asm-offsets.h>
>>>
>>> +#define HC_VENDOR_EPAPR		(1 << 16)
>>> +#define HC_EV_IDLE		16
>> Why not use 'EV_IDLE' directly?
>>
>>> +
>>> +_GLOBAL(epapr_ev_idle)
>>> +epapr_ev_idle:
>>> +	rlwinm	r3,r1,0,0,31-THREAD_SHIFT	/* current thread_info */
>>> +	lwz	r4,TI_LOCAL_FLAGS(r3)	/* set napping bit */
>>> +	ori	r4,r4,_TLF_NAPPING	/* so when we take an exception */
>>> +	stw	r4,TI_LOCAL_FLAGS(r3)	/* it will return to our caller */
>>> +
>>> +	wrteei	1
>>> +
>>> +idle_loop:
>>> +	LOAD_REG_IMMEDIATE(r11, HC_VENDOR_EPAPR | HC_EV_IDLE)
>> And could this line be simplified as something like this:
>>
>> LOAD_REG_IMMEDIATE(r11, EV_HCALL_TOKEN(EV_IDLE))
>>
>> If so, even we can remove the previous HC_VENDOR_EPAPR definition as well.
>>
> 
> Because the epapr_hcalls.h contains C functions,
> so it cannot be included by assembly code.

These common definitions are already covered in epapr_hcalls.h, but looks you
redefine the same items many times, in kvm_para.h/epapr_hcalls.S. And I think
maybe we'll also reuse these generics elsewhere lately.

So can we limit that with __ASSEMBLY__ in epapr_hcalls.h? Or other way. If so it
makes our life easy in the future.

Tiejun
Liu Yu-B13201 Feb. 22, 2012, 2:59 a.m. UTC | #4
> -----Original Message-----
> From: tiejun.chen [mailto:tiejun.chen@windriver.com]
> Sent: Wednesday, February 22, 2012 10:52 AM
> To: Liu Yu-B13201
> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
> linuxppc-dev@ozlabs.org; Wood Scott-B07421
> Subject: Re: [PATCH v5 3/4] KVM: PPC: epapr: install ev_idle hcall for
> e500 guest
> 
> Liu Yu-B13201 wrote:
> >
> >> -----Original Message-----
> >> From: tiejun.chen [mailto:tiejun.chen@windriver.com]
> >> Sent: Tuesday, February 21, 2012 6:54 PM
> >> To: Liu Yu-B13201
> >> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org;
> >> linuxppc-dev@ozlabs.org; Wood Scott-B07421
> >> Subject: Re: [PATCH v5 3/4] KVM: PPC: epapr: install ev_idle hcall
> >> for e500 guest
> >>
> >> Liu Yu wrote:
> >>> If the guest hypervisor node contains "has-idle" property.
> >>>
> >>> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> >>> ---
> >>> v5: no change
> >>>
> >>>  arch/powerpc/kernel/epapr_hcalls.S   |   29
> >> +++++++++++++++++++++++++++++
> >>>  arch/powerpc/kernel/epapr_paravirt.c |   11 ++++++++++-
> >>>  2 files changed, 39 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/kernel/epapr_hcalls.S
> >>> b/arch/powerpc/kernel/epapr_hcalls.S
> >>> index 697b390..72fa234 100644
> >>> --- a/arch/powerpc/kernel/epapr_hcalls.S
> >>> +++ b/arch/powerpc/kernel/epapr_hcalls.S
> >>> @@ -15,6 +15,35 @@
> >>>  #include <asm/ppc_asm.h>
> >>>  #include <asm/asm-offsets.h>
> >>>
> >>> +#define HC_VENDOR_EPAPR		(1 << 16)
> >>> +#define HC_EV_IDLE		16
> >> Why not use 'EV_IDLE' directly?
> >>
> >>> +
> >>> +_GLOBAL(epapr_ev_idle)
> >>> +epapr_ev_idle:
> >>> +	rlwinm	r3,r1,0,0,31-THREAD_SHIFT	/* current thread_info */
> >>> +	lwz	r4,TI_LOCAL_FLAGS(r3)	/* set napping bit */
> >>> +	ori	r4,r4,_TLF_NAPPING	/* so when we take an exception */
> >>> +	stw	r4,TI_LOCAL_FLAGS(r3)	/* it will return to our caller */
> >>> +
> >>> +	wrteei	1
> >>> +
> >>> +idle_loop:
> >>> +	LOAD_REG_IMMEDIATE(r11, HC_VENDOR_EPAPR | HC_EV_IDLE)
> >> And could this line be simplified as something like this:
> >>
> >> LOAD_REG_IMMEDIATE(r11, EV_HCALL_TOKEN(EV_IDLE))
> >>
> >> If so, even we can remove the previous HC_VENDOR_EPAPR definition as
> well.
> >>
> >
> > Because the epapr_hcalls.h contains C functions, so it cannot be
> > included by assembly code.
> 
> These common definitions are already covered in epapr_hcalls.h, but looks
> you redefine the same items many times, in kvm_para.h/epapr_hcalls.S. And
> I think maybe we'll also reuse these generics elsewhere lately.\

The ones in kvm_para.h are alias, not redefine.
Because kvm has its own name rule.

> 
> So can we limit that with __ASSEMBLY__ in epapr_hcalls.h? Or other way.
> If so it makes our life easy in the future.
> 

Yes. This would be helpful.

Thanks,
Yu
diff mbox

Patch

diff --git a/arch/powerpc/kernel/epapr_hcalls.S b/arch/powerpc/kernel/epapr_hcalls.S
index 697b390..72fa234 100644
--- a/arch/powerpc/kernel/epapr_hcalls.S
+++ b/arch/powerpc/kernel/epapr_hcalls.S
@@ -15,6 +15,35 @@ 
 #include <asm/ppc_asm.h>
 #include <asm/asm-offsets.h>
 
+#define HC_VENDOR_EPAPR		(1 << 16)
+#define HC_EV_IDLE		16
+
+_GLOBAL(epapr_ev_idle)
+epapr_ev_idle:
+	rlwinm	r3,r1,0,0,31-THREAD_SHIFT	/* current thread_info */
+	lwz	r4,TI_LOCAL_FLAGS(r3)	/* set napping bit */
+	ori	r4,r4,_TLF_NAPPING	/* so when we take an exception */
+	stw	r4,TI_LOCAL_FLAGS(r3)	/* it will return to our caller */
+
+	wrteei	1
+
+idle_loop:
+	LOAD_REG_IMMEDIATE(r11, HC_VENDOR_EPAPR | HC_EV_IDLE)
+
+.global epapr_ev_idle_start
+epapr_ev_idle_start:
+	li	r3, -1
+	nop
+	nop
+	nop
+
+	/*
+	 * Guard against spurious wakeups from a hypervisor --
+	 * only interrupt will cause us to return to LR due to
+	 * _TLF_NAPPING.
+	 */
+	b	idle_loop
+
 /* Hypercall entry point. Will be patched with device tree instructions. */
 .global epapr_hypercall_start
 epapr_hypercall_start:
diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index e601da7..43d875e 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -21,6 +21,10 @@ 
 #include <asm/epapr_hcalls.h>
 #include <asm/cacheflush.h>
 #include <asm/code-patching.h>
+#include <asm/machdep.h>
+
+extern void epapr_ev_idle(void);
+extern u32 epapr_ev_idle_start[];
 
 bool epapr_para_enabled = false;
 
@@ -39,8 +43,13 @@  static int __init epapr_para_init(void)
 
 	insts = of_get_property(hyper_node, "hcall-instructions", &len);
 	if (insts && !(len % 4) && len <= (4 * 4)) {
-		for (i = 0; i < (len / 4); i++)
+		for (i = 0; i < (len / 4); i++) {
 			patch_instruction(epapr_hypercall_start + i, insts[i]);
+			patch_instruction(epapr_ev_idle_start + i, insts[i]);
+		}
+
+		if (of_get_property(hyper_node, "has-idle", NULL))
+			ppc_md.power_save = epapr_ev_idle;
 
 		epapr_para_enabled = true;
 	} else {