Re: [PATCH] KVM: PPC: E500 compile fix

[Bruce Majia <bruce.majia-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Fri, Mar 26, 2010 at 11:50:10AM +0100, Alexander Graf wrote:
> 
> On 26.03.2010, at 11:40, Bruce Majia wrote:
> 
> > On Fri, Mar 26, 2010 at 10:10:19AM +0100, Alexander Graf wrote:
> >> 
> >> On 26.03.2010, at 08:15, Bruce Majia wrote:
> >> 
> >>> On Sun, Jan 17, 2010 at 03:06:11PM +0200, Avi Kivity wrote:
> >>>> On 01/10/2010 07:01 PM, Alexander Graf wrote:
> >>>>> While trying to compile an E500 vmlinux, I stumbled across a compilation bug
> >>>>> that was obviously there before I touched any of the code. A trace point
> >>>>> doesn't get the correct arguments.
> >>>>> 
> >>>>> Since that shouldn't be any critical to the functionality of the code, my quick
> >>>>> workaround is to #if 0 it out. I would very much appreciate someone fixing it
> >>>>> properly though.
> >>>>> 
> >>>>> Liu, it would be nice if you could be the one doing that.
> >>>>> 
> >>>> 
> >>>> Applied, thanks.
> >>> 
> >>> Hi Avi,
> >>> 
> >>> Seems the problem still exists in the mainline kernel. Are you guys miss
> >>> something in the past few months? Does it means even if I passed compile
> >>> phase with the '#if 0' workaround, it's functions are not completed yet.
> >>> So its impossible to try kvm on ppc e500 target?
> >> 
> >> If #if 0 only affects you if you want to trace that specific function. It's probably safe to say you don't care, so just #if 0 it out and be good :-).
> >> 
> >> Alex
> > 
> > I'd like to say I am okay with #if 0 stuff. :) But fail to have the
> > source code compiled is bad. Based on latest linus tree, I have to
> > apply following stuff to make KVM + mpc85xx_defconfig pass build phase:
> > 
> > ---
> > arch/powerpc/kvm/booke.c    |    2 +-
> > arch/powerpc/kvm/e500_tlb.c |    2 ++
> > arch/powerpc/kvm/powerpc.c  |    2 +-
> > 3 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index 4d686cc..0cde83b 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -137,7 +137,7 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> >                                         unsigned int priority)
> > {
> > 	int allowed = 0;
> > -	ulong msr_mask;
> > +	ulong msr_mask = 0;
> 
> This is a real bug.
> 
> > 	bool update_esr = false, update_dear = false;
> > 
> > 	switch (priority) {
> > diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
> > index 0d772e6..c52e8c2 100644
> > --- a/arch/powerpc/kvm/e500_tlb.c
> > +++ b/arch/powerpc/kvm/e500_tlb.c
> > @@ -225,8 +225,10 @@ static void kvmppc_e500_stlbe_invalidate(struct kvmppc_vcpu_e500 *vcpu_e500,
> > 
> > 	kvmppc_e500_shadow_release(vcpu_e500, tlbsel, esel);
> > 	stlbe->mas1 = 0;
> > +#if 0
> > 	trace_kvm_stlb_inval(index_of(tlbsel, esel), stlbe->mas1, stlbe->mas2,
> > 			     stlbe->mas3, stlbe->mas7);
> > +#endif
> 
> 
> Avi, please make sure e43f2f741a49483034bf968841275cfa553a4cb3 gets into 2.6.34.

Yeah, thanks very much. I got it:

$ git branch -a --contains=e43f2f741a49483034bf968841275cfa553a4cb3
remotes/kvm/master
remotes/kvm/msr-trace
remotes/kvm/next

> 
> > }
> > 
> > static void kvmppc_e500_tlb1_invalidate(struct kvmppc_vcpu_e500 *vcpu_e500,
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 51aedd7..372b678 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -277,7 +277,7 @@ static void kvmppc_complete_dcr_load(struct kvm_vcpu *vcpu,
> > static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu,
> >                                       struct kvm_run *run)
> > {
> > -	ulong gpr;
> > +	ulong gpr = 0;
> 
> This is a bug, but should never hit anyone. Either way having warnings is bad.
> 
> 
> Bruce, please put the two not initialized variables in a patch, put a proper Signed-off-by with patch description in and submit it to kvm-ppc@vger and kvm@vger. I'll ack it then, but would like to keep the credits to you :)

Got it, Will send it soon.

-bm

> 
> 
> Alex