| Message ID | 1329799573-8820-3-git-send-email-yu.liu@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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 {
> -----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
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
> -----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 --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 {
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(-)