[PATCH v3 1/3] KVM: PPC: epapr: Factor out the epapr init

[Liu Yu-B13201]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Saturday, February 11, 2012 2:40 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 v3 1/3] KVM: PPC: epapr: Factor out the epapr init
> 
> On 02/10/2012 04:02 AM, Liu Yu wrote:
> > from the kvm guest paravirt init code.
> >
> > Signed-off-by: Liu Yu <yu.liu at freescale.com>
> > ---
> > v3:
> > apply the epapr init for all ppc platform
> >
> >  arch/powerpc/Kconfig                    |    4 +++
> >  arch/powerpc/include/asm/epapr_hcalls.h |    8 +++++
> >  arch/powerpc/kernel/Makefile            |    1 +
> >  arch/powerpc/kernel/epapr_para.c        |   46
> +++++++++++++++++++++++++++++++
> >  arch/powerpc/kernel/kvm.c               |   13 +++------
> >  arch/powerpc/kvm/Kconfig                |    1 +
> >  6 files changed, 64 insertions(+), 9 deletions(-)  create mode 100644
> > arch/powerpc/kernel/epapr_para.c
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index
> > 47682b6..00bd508 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -196,6 +196,10 @@ config EPAPR_BOOT
> >  	  Used to allow a board to specify it wants an ePAPR compliant
> wrapper.
> >  	default n
> >
> > +config EPAPR_PARA
> > +	bool
> > +	default n
> 
> EPAPR_PARAVIRT
> 
> >  config DEFAULT_UIMAGE
> >  	bool
> >  	help
> > diff --git a/arch/powerpc/include/asm/epapr_hcalls.h
> > b/arch/powerpc/include/asm/epapr_hcalls.h
> > index f3b0c2c..c4b86e4 100644
> > --- a/arch/powerpc/include/asm/epapr_hcalls.h
> > +++ b/arch/powerpc/include/asm/epapr_hcalls.h
> > @@ -148,6 +148,14 @@
> >  #define EV_HCALL_CLOBBERS2 EV_HCALL_CLOBBERS3, "r5"
> >  #define EV_HCALL_CLOBBERS1 EV_HCALL_CLOBBERS2, "r4"
> >
> > +extern u32 *epapr_hcall_insts;
> > +extern int epapr_hcall_insts_len;
> > +
> > +static inline void epapr_get_hcall_insts(u32 **instp, int *lenp) {
> > +	*instp = epapr_hcall_insts;
> > +	*lenp = epapr_hcall_insts_len;
> > +}
> 
> Why do we need a function for this?  Why is the public interface anything
> other than "invoke a hypercall"?
> 
> > +static int __init epapr_para_init(void) {
> > +	struct device_node *hyper_node;
> > +	u32 *insts;
> > +	int len;
> > +
> > +	hyper_node = of_find_node_by_path("/hypervisor");
> > +	if (!hyper_node)
> > +		return -ENODEV;
> > +
> > +	insts = (u32*)of_get_property(hyper_node, "hcall-instructions",
> > +&len);
> 
> Do not cast away that const.
> 
> > @@ -535,18 +536,12 @@ EXPORT_SYMBOL_GPL(kvm_hypercall);  static int
> > kvm_para_setup(void)  {
> >  	extern u32 kvm_hypercall_start;
> > -	struct device_node *hyper_node;
> >  	u32 *insts;
> >  	int len, i;
> >
> > -	hyper_node = of_find_node_by_path("/hypervisor");
> > -	if (!hyper_node)
> > -		return -1;
> > -
> > -	insts = (u32*)of_get_property(hyper_node, "hcall-instructions",
> &len);
> > -	if (len % 4)
> > -		return -1;
> > -	if (len > (4 * 4))
> > +	insts = epapr_hcall_insts;
> > +	len = epapr_hcall_insts_len;
> > +	if (insts == NULL)
> >  		return -1;
> >
> >  	for (i = 0; i < (len / 4); i++)
> 
> Why are you still doing the patching inside kvm.c?
> 

Do you mean we should move kvm_hypercall_start() into epapr bit?

Thanks,
Yu