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

[Liu Yu-B13201]

> -----Original Message-----
> From: tiejun.chen [mailto:tiejun.chen at windriver.com]
> Sent: Wednesday, February 22, 2012 10:52 AM
> To: Liu Yu-B13201
> Cc: agraf at suse.de; kvm-ppc at vger.kernel.org; kvm at vger.kernel.org;
> linuxppc-dev at 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 at windriver.com]
> >> Sent: Tuesday, February 21, 2012 6:54 PM
> >> To: Liu Yu-B13201
> >> Cc: agraf at suse.de; kvm-ppc at vger.kernel.org; kvm at vger.kernel.org;
> >> linuxppc-dev at 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 at 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