Patchwork [v4,3/3] KVM: PPC: epapr: install ev_idle hcall for e500 guest

login
register
mail settings
Submitter Liu Yu-B13201
Date Feb. 16, 2012, 9:26 a.m.
Message ID <1329384365-4028-3-git-send-email-yu.liu@freescale.com>
Download mbox | patch
Permalink /patch/141539/
State New
Headers show

Comments

Liu Yu-B13201 - Feb. 16, 2012, 9:26 a.m.
If the guest hypervisor node contains "has-idle" property.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
v4:
1. discard the CONFIG_E500 to make code for all powerpc platform
2. code cleanup

 arch/powerpc/kernel/epapr.S      |   29 +++++++++++++++++++++++++++++
 arch/powerpc/kernel/epapr_para.c |   13 ++++++++++++-
 2 files changed, 41 insertions(+), 1 deletions(-)
Alexander Graf - Feb. 16, 2012, 10:24 a.m.
On 16.02.2012, at 10:26, Liu Yu <yu.liu@freescale.com> wrote:

> If the guest hypervisor node contains "has-idle" property.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> v4:
> 1. discard the CONFIG_E500 to make code for all powerpc platform
> 2. code cleanup
> 
> arch/powerpc/kernel/epapr.S      |   29 +++++++++++++++++++++++++++++
> arch/powerpc/kernel/epapr_para.c |   13 ++++++++++++-
> 2 files changed, 41 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/epapr.S b/arch/powerpc/kernel/epapr.S
> index 697b390..f06d636 100644
> --- a/arch/powerpc/kernel/epapr.S
> +++ b/arch/powerpc/kernel/epapr.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

Can't you just bl into epapr_hypercall_start? You don't even have to save the old lr. because we never return anyways :)

Alex

> +
> +    /*
> +     * Guard against spurious wakeups (e.g. from a hypervisor) --
> +     * any real 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_para.c b/arch/powerpc/kernel/epapr_para.c
> index ea13cac..adee4f1 100644
> --- a/arch/powerpc/kernel/epapr_para.c
> +++ b/arch/powerpc/kernel/epapr_para.c
> @@ -20,6 +20,10 @@
> #include <linux/of.h>
> #include <asm/epapr_hcalls.h>
> #include <asm/cacheflush.h>
> +#include <asm/machdep.h>
> +
> +extern void epapr_ev_idle(void);
> +extern u32 epapr_ev_idle_start[];
> 
> bool epapr_para_enabled = false;
> 
> @@ -35,10 +39,17 @@ static int __init epapr_para_init(void)
> 
>    insts = of_get_property(hyper_node, "hcall-instructions", &len);
>    if (!(len % 4) && (len >= (4 * 4))) {
> -        for (i = 0; i < (len / 4); i++)
> +        for (i = 0; i < (len / 4); i++) {
>            epapr_hypercall_start[i] = insts[i];
> +            epapr_ev_idle_start[i] = insts[i];
> +        }
>        flush_icache_range((ulong)epapr_hypercall_start,
>                           (ulong)epapr_hypercall_start + len);
> +        flush_icache_range((ulong)epapr_ev_idle_start,
> +                           (ulong)epapr_ev_idle_start + len);
> +
> +        if (of_get_property(hyper_node, "has-idle", NULL))
> +            ppc_md.power_save = epapr_ev_idle;
> 
>        epapr_para_enabled = true;
>    }
> -- 
> 1.7.0.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Feb. 16, 2012, 4:58 p.m.
On 02/16/2012 04:24 AM, Alexander Graf wrote:
> On 16.02.2012, at 10:26, Liu Yu <yu.liu@freescale.com> wrote:
>> +_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
> 
> Can't you just bl into epapr_hypercall_start? You don't even have to save the old lr. because we never return anyways :)

The interrupt will branch to LR, so no, we can't trash it or put it
anywhere else.

-scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Feb. 16, 2012, 5:14 p.m.
On 02/16/2012 03:26 AM, Liu Yu wrote:
> If the guest hypervisor node contains "has-idle" property.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> v4:
> 1. discard the CONFIG_E500 to make code for all powerpc platform
> 2. code cleanup

Is the TLF_NAPPING stuff supported on all powerpc platforms?

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - Feb. 16, 2012, 5:18 p.m.
On 16.02.2012, at 17:58, Scott Wood wrote:

> On 02/16/2012 04:24 AM, Alexander Graf wrote:
>> On 16.02.2012, at 10:26, Liu Yu <yu.liu@freescale.com> wrote:
>>> +_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
>> 
>> Can't you just bl into epapr_hypercall_start? You don't even have to save the old lr. because we never return anyways :)
> 
> The interrupt will branch to LR, so no, we can't trash it or put it
> anywhere else.

Hrm. But we can clobber ctr, right? So how about we make the generic version do a bctr and then just do a small C wrapper that takes lr, moves it to ctr and branches to the generic one?

Then we don't have to replicate the hypercall code all over again for every invocation.

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Feb. 16, 2012, 5:28 p.m.
On 02/16/2012 11:18 AM, Alexander Graf wrote:
> 
> On 16.02.2012, at 17:58, Scott Wood wrote:
> 
>> On 02/16/2012 04:24 AM, Alexander Graf wrote:
>>> On 16.02.2012, at 10:26, Liu Yu <yu.liu@freescale.com> wrote:
>>>> +_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
>>>
>>> Can't you just bl into epapr_hypercall_start? You don't even have to save the old lr. because we never return anyways :)
>>
>> The interrupt will branch to LR, so no, we can't trash it or put it
>> anywhere else.
> 
> Hrm. But we can clobber ctr, right? So how about we make the generic version do a bctr and then just do a small C wrapper that takes lr, moves it to ctr and branches to the generic one?

If it's just for this, I would say don't mess with the normal hcall path
for the sake of idle.  If using CTR would let us get away without
creating a stack frame in call sites, maybe that would be worthwhile,
depending on what sort of hcalls we end up having.

> Then we don't have to replicate the hypercall code all over again for every invocation.

We shouldn't need to do it for every invocation.  Idle is special due to
the TLF_NAPPING hack.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - Feb. 16, 2012, 5:30 p.m.
On 16.02.2012, at 18:28, Scott Wood wrote:

> On 02/16/2012 11:18 AM, Alexander Graf wrote:
>> 
>> On 16.02.2012, at 17:58, Scott Wood wrote:
>> 
>>> On 02/16/2012 04:24 AM, Alexander Graf wrote:
>>>> On 16.02.2012, at 10:26, Liu Yu <yu.liu@freescale.com> wrote:
>>>>> +_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
>>>> 
>>>> Can't you just bl into epapr_hypercall_start? You don't even have to save the old lr. because we never return anyways :)
>>> 
>>> The interrupt will branch to LR, so no, we can't trash it or put it
>>> anywhere else.
>> 
>> Hrm. But we can clobber ctr, right? So how about we make the generic version do a bctr and then just do a small C wrapper that takes lr, moves it to ctr and branches to the generic one?
> 
> If it's just for this, I would say don't mess with the normal hcall path
> for the sake of idle.  If using CTR would let us get away without
> creating a stack frame in call sites, maybe that would be worthwhile,
> depending on what sort of hcalls we end up having.
> 
>> Then we don't have to replicate the hypercall code all over again for every invocation.
> 
> We shouldn't need to do it for every invocation.  Idle is special due to
> the TLF_NAPPING hack.

Famous last words. If it's the only case, duplication should be ok. Let's hope there are no others.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - Feb. 16, 2012, 5:36 p.m.
On 02/16/2012 11:30 AM, Alexander Graf wrote:
> 
> On 16.02.2012, at 18:28, Scott Wood wrote:
> 
>> On 02/16/2012 11:18 AM, Alexander Graf wrote:
>>> Hrm. But we can clobber ctr, right? So how about we make the generic version do a bctr and then just do a small C wrapper that takes lr, moves it to ctr and branches to the generic one?
>>
>> If it's just for this, I would say don't mess with the normal hcall path
>> for the sake of idle.  If using CTR would let us get away without
>> creating a stack frame in call sites, maybe that would be worthwhile,
>> depending on what sort of hcalls we end up having.
>>
>>> Then we don't have to replicate the hypercall code all over again for every invocation.
>>
>> We shouldn't need to do it for every invocation.  Idle is special due to
>> the TLF_NAPPING hack.
> 
> Famous last words. If it's the only case, duplication should be ok. Let's hope there are no others.

Actually, we can't use CTR -- it's volatile in the ePAPR hypercall ABI.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - Feb. 16, 2012, 5:39 p.m.
On 16.02.2012, at 18:36, Scott Wood wrote:

> On 02/16/2012 11:30 AM, Alexander Graf wrote:
>> 
>> On 16.02.2012, at 18:28, Scott Wood wrote:
>> 
>>> On 02/16/2012 11:18 AM, Alexander Graf wrote:
>>>> Hrm. But we can clobber ctr, right? So how about we make the generic version do a bctr and then just do a small C wrapper that takes lr, moves it to ctr and branches to the generic one?
>>> 
>>> If it's just for this, I would say don't mess with the normal hcall path
>>> for the sake of idle.  If using CTR would let us get away without
>>> creating a stack frame in call sites, maybe that would be worthwhile,
>>> depending on what sort of hcalls we end up having.
>>> 
>>>> Then we don't have to replicate the hypercall code all over again for every invocation.
>>> 
>>> We shouldn't need to do it for every invocation.  Idle is special due to
>>> the TLF_NAPPING hack.
>> 
>> Famous last words. If it's the only case, duplication should be ok. Let's hope there are no others.
> 
> Actually, we can't use CTR -- it's volatile in the ePAPR hypercall ABI.

Ugh. Alrighty, let's duplicate the hc code then.


Alex

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

Patch

diff --git a/arch/powerpc/kernel/epapr.S b/arch/powerpc/kernel/epapr.S
index 697b390..f06d636 100644
--- a/arch/powerpc/kernel/epapr.S
+++ b/arch/powerpc/kernel/epapr.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 (e.g. from a hypervisor) --
+	 * any real 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_para.c b/arch/powerpc/kernel/epapr_para.c
index ea13cac..adee4f1 100644
--- a/arch/powerpc/kernel/epapr_para.c
+++ b/arch/powerpc/kernel/epapr_para.c
@@ -20,6 +20,10 @@ 
 #include <linux/of.h>
 #include <asm/epapr_hcalls.h>
 #include <asm/cacheflush.h>
+#include <asm/machdep.h>
+
+extern void epapr_ev_idle(void);
+extern u32 epapr_ev_idle_start[];
 
 bool epapr_para_enabled = false;
 
@@ -35,10 +39,17 @@  static int __init epapr_para_init(void)
 
 	insts = of_get_property(hyper_node, "hcall-instructions", &len);
 	if (!(len % 4) && (len >= (4 * 4))) {
-		for (i = 0; i < (len / 4); i++)
+		for (i = 0; i < (len / 4); i++) {
 			epapr_hypercall_start[i] = insts[i];
+			epapr_ev_idle_start[i] = insts[i];
+		}
 		flush_icache_range((ulong)epapr_hypercall_start,
 		                   (ulong)epapr_hypercall_start + len);
+		flush_icache_range((ulong)epapr_ev_idle_start,
+		                   (ulong)epapr_ev_idle_start + len);
+
+		if (of_get_property(hyper_node, "has-idle", NULL))
+			ppc_md.power_save = epapr_ev_idle;
 
 		epapr_para_enabled = true;
 	}