[PATCH] KVM: PPC: E500 compile fix

[PATCH] KVM: PPC: E500 compile fix

[Alexander Graf]

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.

Signed-off-by: Alexander Graf <agraf@suse.de>
CC: Liu Yu <yu.liu@freescale.com>
---
 arch/powerpc/kvm/e500_tlb.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index 6a7fc01..99a830b 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -225,8 +225,11 @@ static void kvmppc_e500_stlbe_invalidate(struct kvmppc_vcpu_e500 *vcpu_e500,
 
 	kvmppc_e500_shadow_release(vcpu_e500, tlbsel, esel);
 	stlbe->mas1 = 0;
+	/* XXX doesn't compile */
+#if 0
 	trace_kvm_stlb_inval(index_of(tlbsel, esel), stlbe->mas1, stlbe->mas2,
 			     stlbe->mas3, stlbe->mas7);
+#endif
 }
 
 static void kvmppc_e500_tlb1_invalidate(struct kvmppc_vcpu_e500 *vcpu_e500,
-- 
1.6.0.2

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

[Liu Yu-B13201]

 

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de] 
> Sent: Monday, January 11, 2010 1:02 AM
> To: kvm@vger.kernel.org
> Cc: kvm-ppc; Liu Yu-B13201
> Subject: [PATCH] KVM: PPC: E500 compile fix
> 
> 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.
> 

Hrr...
I sent it before, but the whole series is blocked.
http://marc.info/?l=kvm-ppc&m=124929800623835&w=2

Anyway the series needs to be resent, since there's a lot changes.

Thanks for doing this for me!

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

[Alexander Graf]

On 11.01.2010, at 08:05, Liu Yu-B13201 wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf-l3A5Bk7waGM@public.gmane.org] 
>> Sent: Monday, January 11, 2010 1:02 AM
>> To: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: kvm-ppc; Liu Yu-B13201
>> Subject: [PATCH] KVM: PPC: E500 compile fix
>> 
>> 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.
>> 
> 
> Hrr...
> I sent it before, but the whole series is blocked.
> http://marc.info/?l=kvm-ppc&m=124929800623835&w=2
> 
> Anyway the series needs to be resent, since there's a lot changes.

What did it block on? Please rebase and resend - the more working targets we have on KVM the better :).

Alex

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

[Avi Kivity]

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.

-- 
error compiling committee.c: too many arguments to function

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

[Bruce Majia]

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?

Thanks,
-bm

> 
> -- 
> error compiling committee.c: too many arguments to function
> 
> --
> 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

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

[Alexander Graf]

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

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

[Liu Yu-B13201]

 

> -----Original Message-----
> From: Alexander Graf [mailto:agraf-l3A5Bk7waGM@public.gmane.org] 
> Sent: Friday, March 26, 2010 5:10 PM
> To: Bruce Majia
> Cc: Avi Kivity; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-ppc; Liu Yu-B13201
> Subject: Re: [PATCH] KVM: PPC: E500 compile fix
> 
> 
> 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 :-).
> 

The trace info here is not that important.
And will be removed in future patches...

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

[Bruce Majia]

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;
 	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
 }
 
 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;
 
 	if (run->mmio.len > sizeof(gpr)) {
 		printk(KERN_ERR "bad MMIO length: %d\n", run->mmio.len);
-- 

Do we need a *temporary* patch for the situation?

Regards,
-bm

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

[Alexander Graf]

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.

> }
> 
> 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 :)


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

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

[Bruce Majia]

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