[PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Scott Wood]

BookE altivec support brought two new exceptions, but KVM was not
updated, so the build broke for all 64-bit booke with KVM enabled.

Add the new vectors, but there's more than this required to make
Altivec work in the guest, and we can't prevent the guest from turning
on Altivec (which can corrupt host state until state save/restore is
implemented).  Disable e6500 on KVM until this is fixed.

Signed-off-by: Scott Wood <scottwood@freescale.com>
Cc: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kvm/bookehv_interrupts.S |    4 ++++
 arch/powerpc/kvm/e500mc.c             |    2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
index e8ed7d6..6397613 100644
--- a/arch/powerpc/kvm/bookehv_interrupts.S
+++ b/arch/powerpc/kvm/bookehv_interrupts.S
@@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL, EX_PARAMS(CRIT), \
 	SPRN_CSRR0, SPRN_CSRR1, 0
 kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \
 	SPRN_SRR0, SPRN_SRR1, NEED_EMU
+kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
+	SPRN_SRR0, SPRN_SRR1, 0
+kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
+	SPRN_SRR0, SPRN_SRR1, 0
 kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \
 	SPRN_SRR0, SPRN_SRR1, 0
 kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 753cc99..19c8379 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void)
 		r = 0;
 	else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
 		r = 0;
-	else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
-		r = 0;
 	else
 		r = -ENOTSUPP;
 
-- 
1.7.10.4

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, May 10, 2013 6:15 AM
> To: Alexander Graf
> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421;
> Caraman Mihai Claudiu-B02008
> Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and
> disable e6500
> 
> BookE altivec support brought two new exceptions, but KVM was not
> updated, so the build broke for all 64-bit booke with KVM enabled.

We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have
done this in Kumar's tree but we missed that chance. We will face similar
issues every time an exception handler will be added.

> 
> Add the new vectors, but there's more than this required to make
> Altivec work in the guest, and we can't prevent the guest from turning
> on Altivec (which can corrupt host state until state save/restore is
> implemented).  Disable e6500 on KVM until this is fixed.

To be more precise it can corrupt Altivec host state.

> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> Cc: Mihai Caraman <mihai.caraman@freescale.com>
> ---
>  arch/powerpc/kvm/bookehv_interrupts.S |    4 ++++
>  arch/powerpc/kvm/e500mc.c             |    2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
> b/arch/powerpc/kvm/bookehv_interrupts.S
> index e8ed7d6..6397613 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL,
> EX_PARAMS(CRIT), \
>  	SPRN_CSRR0, SPRN_CSRR1, 0
>  kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \
>  	SPRN_SRR0, SPRN_SRR1, NEED_EMU
> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
> +	SPRN_SRR0, SPRN_SRR1, 0
> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
> +	SPRN_SRR0, SPRN_SRR1, 0

Why not NEED_ESR as we did in our SDK?

>  kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \
>  	SPRN_SRR0, SPRN_SRR1, 0
>  kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \
> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> index 753cc99..19c8379 100644
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void)
>  		r = 0;
>  	else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
>  		r = 0;
> -	else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
> -		r = 0;

Hmm ... I made it clear in the commit message that Altivec is not
supported, why couldn't we be more gentle:

diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index c3bdc0a..4f43a36 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void)
                r = 0;
        else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
                r = 0;
+#ifndef CONFIG_ALTIVEC
+/* ALTIVEC is not supported now by KVM so only one of them can work */
        else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
                r = 0;
+#endif
        else
                r = -ENOTSUPP;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 82f155e..bb1a9e0 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
                break;
 #endif
 
+       case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL:
+       case BOOKE_INTERRUPT_ALTIVEC_ASSIST:
+               /*
+                * Guest wants ALTIVEC, but host kernel doesn't support it.
+                * send an "unimplemented operation" program check to the guest.
+                */
+               kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV);
+               r = RESUME_GUEST;
+               break;
+
        case BOOKE_INTERRUPT_DATA_STORAGE:
                kvmppc_core_queue_data_storage(vcpu, vcpu->arch.fault_dear,
                                               vcpu->arch.fault_esr);


Kconfig should also be updated (in both proposals).

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Alexander Graf]

On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote:

>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Friday, May 10, 2013 6:15 AM
>> To: Alexander Graf
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421;
>> Caraman Mihai Claudiu-B02008
>> Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and
>> disable e6500
>> 
>> BookE altivec support brought two new exceptions, but KVM was not
>> updated, so the build broke for all 64-bit booke with KVM enabled.
> 
> We couldn't do that in KVM before having BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
> BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should have
> done this in Kumar's tree but we missed that chance. We will face similar
> issues every time an exception handler will be added.

How hard would it be to add? I suppose it's broken in 3.10, so we need something quick before it gets released?

> 
>> 
>> Add the new vectors, but there's more than this required to make
>> Altivec work in the guest, and we can't prevent the guest from turning
>> on Altivec (which can corrupt host state until state save/restore is
>> implemented).  Disable e6500 on KVM until this is fixed.
> 
> To be more precise it can corrupt Altivec host state.
> 
>> 
>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>> Cc: Mihai Caraman <mihai.caraman@freescale.com>
>> ---
>> arch/powerpc/kvm/bookehv_interrupts.S |    4 ++++
>> arch/powerpc/kvm/e500mc.c             |    2 --
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
>> b/arch/powerpc/kvm/bookehv_interrupts.S
>> index e8ed7d6..6397613 100644
>> --- a/arch/powerpc/kvm/bookehv_interrupts.S
>> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
>> @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL,
>> EX_PARAMS(CRIT), \
>> 	SPRN_CSRR0, SPRN_CSRR1, 0
>> kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \
>> 	SPRN_SRR0, SPRN_SRR1, NEED_EMU
>> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
>> +	SPRN_SRR0, SPRN_SRR1, 0
>> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
>> +	SPRN_SRR0, SPRN_SRR1, 0
> 
> Why not NEED_ESR as we did in our SDK?
> 
>> kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \
>> 	SPRN_SRR0, SPRN_SRR1, 0
>> kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \
>> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
>> index 753cc99..19c8379 100644
>> --- a/arch/powerpc/kvm/e500mc.c
>> +++ b/arch/powerpc/kvm/e500mc.c
>> @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void)
>> 		r = 0;
>> 	else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
>> 		r = 0;
>> -	else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
>> -		r = 0;
> 
> Hmm ... I made it clear in the commit message that Altivec is not
> supported, why couldn't we be more gentle:
> 
> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> index c3bdc0a..4f43a36 100644
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void)
>                r = 0;
>        else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
>                r = 0;
> +#ifndef CONFIG_ALTIVEC

This would still be wrong, since running 2 guests with altivec would break each other.

Alex

> +/* ALTIVEC is not supported now by KVM so only one of them can work */
>        else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
>                r = 0;
> +#endif
>        else
>                r = -ENOTSUPP;
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 82f155e..bb1a9e0 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
>                break;
> #endif
> 
> +       case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL:
> +       case BOOKE_INTERRUPT_ALTIVEC_ASSIST:
> +               /*
> +                * Guest wants ALTIVEC, but host kernel doesn't support it.
> +                * send an "unimplemented operation" program check to the guest.
> +                */
> +               kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV);
> +               r = RESUME_GUEST;
> +               break;
> +
>        case BOOKE_INTERRUPT_DATA_STORAGE:
>                kvmppc_core_queue_data_storage(vcpu, vcpu->arch.fault_dear,
>                                               vcpu->arch.fault_esr);
> 
> 
> Kconfig should also be updated (in both proposals).
> 
> -Mike
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" 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/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Friday, May 10, 2013 4:16 PM
> To: Caraman Mihai Claudiu-B02008
> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
> and disable e6500
> 
> 
> On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote:
> 
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Friday, May 10, 2013 6:15 AM
> >> To: Alexander Graf
> >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421;
> >> Caraman Mihai Claudiu-B02008
> >> Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and
> >> disable e6500
> >>
> >> BookE altivec support brought two new exceptions, but KVM was not
> >> updated, so the build broke for all 64-bit booke with KVM enabled.
> >
> > We couldn't do that in KVM before having
> BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
> > BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should
> have
> > done this in Kumar's tree but we missed that chance. We will face
> similar
> > issues every time an exception handler will be added.
> 
> How hard would it be to add? I suppose it's broken in 3.10, so we need
> something quick before it gets released?

Not so hard. Yes. I was surprised by this patch given the fact that we have
planned to send altivec support upstream this days and that we already have
a similar patch from Tiejun on our list.

> 
> >
> >>
> >> Add the new vectors, but there's more than this required to make
> >> Altivec work in the guest, and we can't prevent the guest from turning
> >> on Altivec (which can corrupt host state until state save/restore is
> >> implemented).  Disable e6500 on KVM until this is fixed.
> >
> > To be more precise it can corrupt Altivec host state.
> >
> >>
> >> Signed-off-by: Scott Wood <scottwood@freescale.com>
> >> Cc: Mihai Caraman <mihai.caraman@freescale.com>
> >> ---
> >> arch/powerpc/kvm/bookehv_interrupts.S |    4 ++++
> >> arch/powerpc/kvm/e500mc.c             |    2 --
> >> 2 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
> >> b/arch/powerpc/kvm/bookehv_interrupts.S
> >> index e8ed7d6..6397613 100644
> >> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> >> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> >> @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL,
> >> EX_PARAMS(CRIT), \
> >> 	SPRN_CSRR0, SPRN_CSRR1, 0
> >> kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \
> >> 	SPRN_SRR0, SPRN_SRR1, NEED_EMU
> >> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
> >> +	SPRN_SRR0, SPRN_SRR1, 0
> >> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
> >> +	SPRN_SRR0, SPRN_SRR1, 0
> >
> > Why not NEED_ESR as we did in our SDK?
> >
> >> kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \
> >> 	SPRN_SRR0, SPRN_SRR1, 0
> >> kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \
> >> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> >> index 753cc99..19c8379 100644
> >> --- a/arch/powerpc/kvm/e500mc.c
> >> +++ b/arch/powerpc/kvm/e500mc.c
> >> @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void)
> >> 		r = 0;
> >> 	else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
> >> 		r = 0;
> >> -	else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
> >> -		r = 0;
> >
> > Hmm ... I made it clear in the commit message that Altivec is not
> > supported, why couldn't we be more gentle:
> >
> > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> > index c3bdc0a..4f43a36 100644
> > --- a/arch/powerpc/kvm/e500mc.c
> > +++ b/arch/powerpc/kvm/e500mc.c
> > @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void)
> >                r = 0;
> >        else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
> >                r = 0;
> > +#ifndef CONFIG_ALTIVEC
> 
> This would still be wrong, since running 2 guests with altivec would
> break each other.

It's wrong if you expect to have altivec supported in the VM which is not
the case. Guests that executes altivec instructions will get a program
(unimplemented op) exception see below.

Do we really need to remove e6500 all together for this?

> 
> Alex
> 
> > +/* ALTIVEC is not supported now by KVM so only one of them can work */
> >        else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
> >                r = 0;
> > +#endif
> >        else
> >                r = -ENOTSUPP;
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index 82f155e..bb1a9e0 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -973,6 +973,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
> >                break;
> > #endif
> >
> > +       case BOOKE_INTERRUPT_ALTIVEC_UNAVAIL:
> > +       case BOOKE_INTERRUPT_ALTIVEC_ASSIST:
> > +               /*
> > +                * Guest wants ALTIVEC, but host kernel doesn't support
> it.
> > +                * send an "unimplemented operation" program check to
> the guest.
> > +                */
> > +               kvmppc_core_queue_program(vcpu, ESR_PUO | ESR_SPV);
> > +               r = RESUME_GUEST;
> > +               break;
> > +
> >        case BOOKE_INTERRUPT_DATA_STORAGE:
> >                kvmppc_core_queue_data_storage(vcpu, vcpu-
> >arch.fault_dear,
> >                                               vcpu->arch.fault_esr);
> >
> >
> > Kconfig should also be updated (in both proposals).
> >

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Alexander Graf]

On 10.05.2013, at 16:11, Caraman Mihai Claudiu-B02008 wrote:

>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Friday, May 10, 2013 4:16 PM
>> To: Caraman Mihai Claudiu-B02008
>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
>> and disable e6500
>> 
>> 
>> On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote:
>> 
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Friday, May 10, 2013 6:15 AM
>>>> To: Alexander Graf
>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421;
>>>> Caraman Mihai Claudiu-B02008
>>>> Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and
>>>> disable e6500
>>>> 
>>>> BookE altivec support brought two new exceptions, but KVM was not
>>>> updated, so the build broke for all 64-bit booke with KVM enabled.
>>> 
>>> We couldn't do that in KVM before having
>> BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
>>> BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should
>> have
>>> done this in Kumar's tree but we missed that chance. We will face
>> similar
>>> issues every time an exception handler will be added.
>> 
>> How hard would it be to add? I suppose it's broken in 3.10, so we need
>> something quick before it gets released?
> 
> Not so hard. Yes. I was surprised by this patch given the fact that we have
> planned to send altivec support upstream this days and that we already have
> a similar patch from Tiejun on our list.
> 
>> 
>>> 
>>>> 
>>>> Add the new vectors, but there's more than this required to make
>>>> Altivec work in the guest, and we can't prevent the guest from turning
>>>> on Altivec (which can corrupt host state until state save/restore is
>>>> implemented).  Disable e6500 on KVM until this is fixed.
>>> 
>>> To be more precise it can corrupt Altivec host state.
>>> 
>>>> 
>>>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>>>> Cc: Mihai Caraman <mihai.caraman@freescale.com>
>>>> ---
>>>> arch/powerpc/kvm/bookehv_interrupts.S |    4 ++++
>>>> arch/powerpc/kvm/e500mc.c             |    2 --
>>>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
>>>> b/arch/powerpc/kvm/bookehv_interrupts.S
>>>> index e8ed7d6..6397613 100644
>>>> --- a/arch/powerpc/kvm/bookehv_interrupts.S
>>>> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
>>>> @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL,
>>>> EX_PARAMS(CRIT), \
>>>> 	SPRN_CSRR0, SPRN_CSRR1, 0
>>>> kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \
>>>> 	SPRN_SRR0, SPRN_SRR1, NEED_EMU
>>>> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
>>>> +	SPRN_SRR0, SPRN_SRR1, 0
>>>> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
>>>> +	SPRN_SRR0, SPRN_SRR1, 0
>>> 
>>> Why not NEED_ESR as we did in our SDK?
>>> 
>>>> kvm_handler BOOKE_INTERRUPT_HV_SYSCALL, EX_PARAMS(GEN), \
>>>> 	SPRN_SRR0, SPRN_SRR1, 0
>>>> kvm_handler BOOKE_INTERRUPT_GUEST_DBELL, EX_PARAMS(GDBELL), \
>>>> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
>>>> index 753cc99..19c8379 100644
>>>> --- a/arch/powerpc/kvm/e500mc.c
>>>> +++ b/arch/powerpc/kvm/e500mc.c
>>>> @@ -177,8 +177,6 @@ int kvmppc_core_check_processor_compat(void)
>>>> 		r = 0;
>>>> 	else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
>>>> 		r = 0;
>>>> -	else if (strcmp(cur_cpu_spec->cpu_name, "e6500") == 0)
>>>> -		r = 0;
>>> 
>>> Hmm ... I made it clear in the commit message that Altivec is not
>>> supported, why couldn't we be more gentle:
>>> 
>>> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
>>> index c3bdc0a..4f43a36 100644
>>> --- a/arch/powerpc/kvm/e500mc.c
>>> +++ b/arch/powerpc/kvm/e500mc.c
>>> @@ -172,8 +172,11 @@ int kvmppc_core_check_processor_compat(void)
>>>               r = 0;
>>>       else if (strcmp(cur_cpu_spec->cpu_name, "e5500") == 0)
>>>               r = 0;
>>> +#ifndef CONFIG_ALTIVEC
>> 
>> This would still be wrong, since running 2 guests with altivec would
>> break each other.
> 
> It's wrong if you expect to have altivec supported in the VM which is not
> the case. Guests that executes altivec instructions will get a program
> (unimplemented op) exception see below.
> 
> Do we really need to remove e6500 all together for this?

No, just send real Altivec support quickly. We can add it to 3.10 through stable if it's too later, but I'd prefer to have it in within the rc phase.


Alex

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> > Do we really need to remove e6500 all together for this?
> 
> No, just send real Altivec support quickly. We can add it to 3.10 through
> stable if it's too later, but I'd prefer to have it in within the rc
> phase.

I should have it by Monday, the smoke tests passed.

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Alexander Graf]

On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote:

>>> Do we really need to remove e6500 all together for this?
>> 
>> No, just send real Altivec support quickly. We can add it to 3.10 through
>> stable if it's too later, but I'd prefer to have it in within the rc
>> phase.
> 
> I should have it by Monday, the smoke tests passed.

Please make sure it also works on 32bit :)


Alex

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> owner@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Friday, May 10, 2013 7:51 PM
> To: Caraman Mihai Claudiu-B02008
> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
> and disable e6500
> 
> 
> On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote:
> 
> >>> Do we really need to remove e6500 all together for this?
> >>
> >> No, just send real Altivec support quickly. We can add it to 3.10
> through
> >> stable if it's too later, but I'd prefer to have it in within the rc
> >> phase.
> >
> > I should have it by Monday, the smoke tests passed.
> 
> Please make sure it also works on 32bit :)

I can make sure that it doesn't break 32-bit. Altivec is not present in
e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are
you looking for G4 support?

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Scott Wood]

On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote:
> > -----Original Message-----
> > From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> > owner@vger.kernel.org] On Behalf Of Alexander Graf
> > Sent: Friday, May 10, 2013 7:51 PM
> > To: Caraman Mihai Claudiu-B02008
> > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from  
> Altivec,
> > and disable e6500
> >
> >
> > On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote:
> >
> > >>> Do we really need to remove e6500 all together for this?
> > >>
> > >> No, just send real Altivec support quickly. We can add it to 3.10
> > through
> > >> stable if it's too later, but I'd prefer to have it in within  
> the rc
> > >> phase.
> > >
> > > I should have it by Monday, the smoke tests passed.
> >
> > Please make sure it also works on 32bit :)
> 
> I can make sure that it doesn't break 32-bit. Altivec is not present  
> in
> e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host.  
> Are
> you looking for G4 support?

Just because Altivec isn't present on 32-bit doesn't mean that a  
particular patch can't break things there.  Just as a patch to deal  
with lazy ee issues can break 32-bit, even though 32-bit doesn't have  
lazy ee. :-)

-Scott

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Alexander Graf]

On 10.05.2013, at 20:06, Scott Wood wrote:

> On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote:
>> > -----Original Message-----
>> > From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
>> > owner@vger.kernel.org] On Behalf Of Alexander Graf
>> > Sent: Friday, May 10, 2013 7:51 PM
>> > To: Caraman Mihai Claudiu-B02008
>> > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>> > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
>> > and disable e6500
>> >
>> >
>> > On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote:
>> >
>> > >>> Do we really need to remove e6500 all together for this?
>> > >>
>> > >> No, just send real Altivec support quickly. We can add it to 3.10
>> > through
>> > >> stable if it's too later, but I'd prefer to have it in within the rc
>> > >> phase.
>> > >
>> > > I should have it by Monday, the smoke tests passed.
>> >
>> > Please make sure it also works on 32bit :)
>> I can make sure that it doesn't break 32-bit. Altivec is not present in
>> e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host. Are
>> you looking for G4 support?
> 
> Just because Altivec isn't present on 32-bit doesn't mean that a particular patch can't break things there.  Just as a patch to deal with lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee. :-)

Yeah, that's what I was referring to :). G4s  use a different code path altogether and already work with Altivec for years. I doubt you'll manage to break anything there :).

So what you should ensure is

  1) 32bit e500 still compiles
  2) 32bit e500 still executes fine, albeit without altivec


Alex

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Friday, May 10, 2013 9:23 PM
> To: Wood Scott-B07421
> Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm-
> ppc@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
> and disable e6500
> 
> 
> On 10.05.2013, at 20:06, Scott Wood wrote:
> 
> > On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote:
> >> > -----Original Message-----
> >> > From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> >> > owner@vger.kernel.org] On Behalf Of Alexander Graf
> >> > Sent: Friday, May 10, 2013 7:51 PM
> >> > To: Caraman Mihai Claudiu-B02008
> >> > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> >> > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from
> Altivec,
> >> > and disable e6500
> >> >
> >> >
> >> > On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote:
> >> >
> >> > >>> Do we really need to remove e6500 all together for this?
> >> > >>
> >> > >> No, just send real Altivec support quickly. We can add it to 3.10
> >> > through
> >> > >> stable if it's too later, but I'd prefer to have it in within the
> rc
> >> > >> phase.
> >> > >
> >> > > I should have it by Monday, the smoke tests passed.
> >> >
> >> > Please make sure it also works on 32bit :)
> >> I can make sure that it doesn't break 32-bit. Altivec is not present
> in
> >> e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host.
> Are
> >> you looking for G4 support?
> >
> > Just because Altivec isn't present on 32-bit doesn't mean that a
> particular patch can't break things there.  Just as a patch to deal with
> lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee.
> :-)
> 
> Yeah, that's what I was referring to :). G4s  use a different code path
> altogether and already work with Altivec for years.

Huh ... I forgot that G4 in handled under Book3S, not so obvious.

> I doubt you'll manage to break anything there :).

If you have stated breaks instead of works would have been more clear where
you were heading :-J

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Alexander Graf]

Am 10.05.2013 um 20:51 schrieb Caraman Mihai Claudiu-B02008 <B02008@freescale.com>:

>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Friday, May 10, 2013 9:23 PM
>> To: Wood Scott-B07421
>> Cc: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm-
>> ppc@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
>> and disable e6500
>> 
>> 
>> On 10.05.2013, at 20:06, Scott Wood wrote:
>> 
>>> On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote:
>>>>> -----Original Message-----
>>>>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
>>>>> owner@vger.kernel.org] On Behalf Of Alexander Graf
>>>>> Sent: Friday, May 10, 2013 7:51 PM
>>>>> To: Caraman Mihai Claudiu-B02008
>>>>> Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>>>>> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from
>> Altivec,
>>>>> and disable e6500
>>>>> 
>>>>> 
>>>>> On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote:
>>>>> 
>>>>>>>> Do we really need to remove e6500 all together for this?
>>>>>>> 
>>>>>>> No, just send real Altivec support quickly. We can add it to 3.10
>>>>> through
>>>>>>> stable if it's too later, but I'd prefer to have it in within the
>> rc
>>>>>>> phase.
>>>>>> 
>>>>>> I should have it by Monday, the smoke tests passed.
>>>>> 
>>>>> Please make sure it also works on 32bit :)
>>>> I can make sure that it doesn't break 32-bit. Altivec is not present
>> in
>>>> e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host.
>> Are
>>>> you looking for G4 support?
>>> 
>>> Just because Altivec isn't present on 32-bit doesn't mean that a
>> particular patch can't break things there.  Just as a patch to deal with
>> lazy ee issues can break 32-bit, even though 32-bit doesn't have lazy ee.
>> :-)
>> 
>> Yeah, that's what I was referring to :). G4s  use a different code path
>> altogether and already work with Altivec for years.
> 
> Huh ... I forgot that G4 in handled under Book3S, not so obvious.
> 
>> I doubt you'll manage to break anything there :).
> 
> If you have stated breaks instead of works would have been more clear where
> you were heading :-J

Ok, next time I'll make it more obvious :).

Alex

> 
> -Mike
> 

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, May 10, 2013 9:06 PM
> To: Caraman Mihai Claudiu-B02008
> Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org;
> kvm@vger.kernel.org
> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
> and disable e6500
> 
> On 05/10/2013 01:03:27 PM, Caraman Mihai Claudiu-B02008 wrote:
> > > -----Original Message-----
> > > From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> > > owner@vger.kernel.org] On Behalf Of Alexander Graf
> > > Sent: Friday, May 10, 2013 7:51 PM
> > > To: Caraman Mihai Claudiu-B02008
> > > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> > > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from
> > Altivec,
> > > and disable e6500
> > >
> > >
> > > On 10.05.2013, at 18:50, Caraman Mihai Claudiu-B02008 wrote:
> > >
> > > >>> Do we really need to remove e6500 all together for this?
> > > >>
> > > >> No, just send real Altivec support quickly. We can add it to 3.10
> > > through
> > > >> stable if it's too later, but I'd prefer to have it in within
> > the rc
> > > >> phase.
> > > >
> > > > I should have it by Monday, the smoke tests passed.
> > >
> > > Please make sure it also works on 32bit :)
> >
> > I can make sure that it doesn't break 32-bit. Altivec is not present
> > in
> > e500v2/e500mc/e5500 cores and e6500 is supported only by 64-bit host.
> > Are
> > you looking for G4 support?
> 
> Just because Altivec isn't present on 32-bit doesn't mean that a
> particular patch can't break things there.  Just as a patch to deal
> with lazy ee issues can break 32-bit, even though 32-bit doesn't have
> lazy ee. :-)

That's exactly what I have said. That patch was intended as a RFC I forgot
to mark it as such under time constraints, however it reached its goal.

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Scott Wood]

On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote:
> > -----Original Message-----
> > From: Alexander Graf [mailto:agraf@suse.de]
> > Sent: Friday, May 10, 2013 4:16 PM
> > To: Caraman Mihai Claudiu-B02008
> > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from  
> Altivec,
> > and disable e6500
> >
> >
> > On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote:
> >
> > >> -----Original Message-----
> > >> From: Wood Scott-B07421
> > >> Sent: Friday, May 10, 2013 6:15 AM
> > >> To: Alexander Graf
> > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood  
> Scott-B07421;
> > >> Caraman Mihai Claudiu-B02008
> > >> Subject: [PATCH] kvm/ppc/booke64: fix build breakage from  
> Altivec, and
> > >> disable e6500
> > >>
> > >> BookE altivec support brought two new exceptions, but KVM was not
> > >> updated, so the build broke for all 64-bit booke with KVM  
> enabled.
> > >
> > > We couldn't do that in KVM before having
> > BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
> > > BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we  
> should
> > have
> > > done this in Kumar's tree but we missed that chance. We will face
> > similar
> > > issues every time an exception handler will be added.
> >
> > How hard would it be to add? I suppose it's broken in 3.10, so we  
> need
> > something quick before it gets released?
> 
> Not so hard. Yes. I was surprised by this patch given the fact that  
> we have
> planned to send altivec support upstream this days and that we  
> already have
> a similar patch from Tiejun on our list.

I didn't see Tiejun's patch...  My goal was just to fix the build break  
without exposing problems, and to encourage a patch to fix it properly  
to happen sooner rather than later.  With Tiejun's patch, which is  
similar to mine except that it doesn't disable e6500 support, a user  
could BUG() the kernel by forcing an Altivec exception in a guest.  I  
didn't want to go further down the road of adding reflectors for those  
exceptions, which could make it look like the problem was dealt with  
even though it's still not done.

-Scott

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, May 10, 2013 8:42 PM
> To: Caraman Mihai Claudiu-B02008
> Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org;
> kvm@vger.kernel.org
> Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,
> and disable e6500
> 
> On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote:
> > > -----Original Message-----
> > > From: Alexander Graf [mailto:agraf@suse.de]
> > > Sent: Friday, May 10, 2013 4:16 PM
> > > To: Caraman Mihai Claudiu-B02008
> > > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
> > > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from
> > Altivec,
> > > and disable e6500
> > >
> > >
> > > On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote:
> > >
> > > >> -----Original Message-----
> > > >> From: Wood Scott-B07421
> > > >> Sent: Friday, May 10, 2013 6:15 AM
> > > >> To: Alexander Graf
> > > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood
> > Scott-B07421;
> > > >> Caraman Mihai Claudiu-B02008
> > > >> Subject: [PATCH] kvm/ppc/booke64: fix build breakage from
> > Altivec, and
> > > >> disable e6500
> > > >>
> > > >> BookE altivec support brought two new exceptions, but KVM was not
> > > >> updated, so the build broke for all 64-bit booke with KVM
> > enabled.
> > > >
> > > > We couldn't do that in KVM before having
> > > BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
> > > > BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we
> > should
> > > have
> > > > done this in Kumar's tree but we missed that chance. We will face
> > > similar
> > > > issues every time an exception handler will be added.
> > >
> > > How hard would it be to add? I suppose it's broken in 3.10, so we
> > need
> > > something quick before it gets released?
> >
> > Not so hard. Yes. I was surprised by this patch given the fact that
> > we have
> > planned to send altivec support upstream this days and that we
> > already have
> > a similar patch from Tiejun on our list.
> 
> I didn't see Tiejun's patch...  My goal was just to fix the build break
> without exposing problems, and to encourage a patch to fix it properly
> to happen sooner rather than later.  With Tiejun's patch, which is
> similar to mine except that it doesn't disable e6500 support, a user
> could BUG() the kernel by forcing an Altivec exception in a guest.  I 
> didn't want to go further down the road of adding reflectors for those
> exceptions, which could make it look like the problem was dealt with
> even though it's still not done.

I agree it's quite annoying to hit a build breakage. Reflection is not
a proper solution for this problem (though we will require it later)
but program exception injection looks feasible as a simple fix. I wouldn't
want to see e6500 removed for this reason.

Thanks,
Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Scott Wood]

On 05/10/2013 01:20:33 PM, Caraman Mihai Claudiu-B02008 wrote:
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, May 10, 2013 8:42 PM
> > To: Caraman Mihai Claudiu-B02008
> > Cc: Alexander Graf; Wood Scott-B07421; kvm-ppc@vger.kernel.org;
> > kvm@vger.kernel.org
> > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from  
> Altivec,
> > and disable e6500
> >
> > On 05/10/2013 09:11:24 AM, Caraman Mihai Claudiu-B02008 wrote:
> > > > -----Original Message-----
> > > > From: Alexander Graf [mailto:agraf@suse.de]
> > > > Sent: Friday, May 10, 2013 4:16 PM
> > > > To: Caraman Mihai Claudiu-B02008
> > > > Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org;  
> kvm@vger.kernel.org
> > > > Subject: Re: [PATCH] kvm/ppc/booke64: fix build breakage from
> > > Altivec,
> > > > and disable e6500
> > > >
> > > >
> > > > On 10.05.2013, at 11:40, Caraman Mihai Claudiu-B02008 wrote:
> > > >
> > > > >> -----Original Message-----
> > > > >> From: Wood Scott-B07421
> > > > >> Sent: Friday, May 10, 2013 6:15 AM
> > > > >> To: Alexander Graf
> > > > >> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood
> > > Scott-B07421;
> > > > >> Caraman Mihai Claudiu-B02008
> > > > >> Subject: [PATCH] kvm/ppc/booke64: fix build breakage from
> > > Altivec, and
> > > > >> disable e6500
> > > > >>
> > > > >> BookE altivec support brought two new exceptions, but KVM  
> was not
> > > > >> updated, so the build broke for all 64-bit booke with KVM
> > > enabled.
> > > > >
> > > > > We couldn't do that in KVM before having
> > > > BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
> > > > > BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we
> > > should
> > > > have
> > > > > done this in Kumar's tree but we missed that chance. We will  
> face
> > > > similar
> > > > > issues every time an exception handler will be added.
> > > >
> > > > How hard would it be to add? I suppose it's broken in 3.10, so  
> we
> > > need
> > > > something quick before it gets released?
> > >
> > > Not so hard. Yes. I was surprised by this patch given the fact  
> that
> > > we have
> > > planned to send altivec support upstream this days and that we
> > > already have
> > > a similar patch from Tiejun on our list.
> >
> > I didn't see Tiejun's patch...  My goal was just to fix the build  
> break
> > without exposing problems, and to encourage a patch to fix it  
> properly
> > to happen sooner rather than later.  With Tiejun's patch, which is
> > similar to mine except that it doesn't disable e6500 support, a user
> > could BUG() the kernel by forcing an Altivec exception in a guest.   
> I
> > didn't want to go further down the road of adding reflectors for  
> those
> > exceptions, which could make it look like the problem was dealt with
> > even though it's still not done.
> 
> I agree it's quite annoying to hit a build breakage. Reflection is not
> a proper solution for this problem (though we will require it later)
> but program exception injection looks feasible as a simple fix.

Program exception injection still doesn't deal with state corruption.

> I wouldn't want to see e6500 removed for this reason.

It's not being removed, just closed while under construction to prevent  
damage to people passing through. :-)

-Scott

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> > > I didn't see Tiejun's patch...  My goal was just to fix the build
> > break
> > > without exposing problems, and to encourage a patch to fix it
> > properly
> > > to happen sooner rather than later.  With Tiejun's patch, which is
> > > similar to mine except that it doesn't disable e6500 support, a user
> > > could BUG() the kernel by forcing an Altivec exception in a guest.
> > I
> > > didn't want to go further down the road of adding reflectors for
> > those
> > > exceptions, which could make it look like the problem was dealt with
> > > even though it's still not done.
> >
> > I agree it's quite annoying to hit a build breakage. Reflection is not
> > a proper solution for this problem (though we will require it later)
> > but program exception injection looks feasible as a simple fix.
> 
> Program exception injection still doesn't deal with state corruption.

Yes but it's not critical for this particular case since nobody is able
to effectively use that state via altivec instructions. Leaking state
however can be a real issue.

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Scott Wood]

On 05/10/2013 02:06:53 PM, Caraman Mihai Claudiu-B02008 wrote:
> > > > I didn't see Tiejun's patch...  My goal was just to fix the  
> build
> > > break
> > > > without exposing problems, and to encourage a patch to fix it
> > > properly
> > > > to happen sooner rather than later.  With Tiejun's patch, which  
> is
> > > > similar to mine except that it doesn't disable e6500 support, a  
> user
> > > > could BUG() the kernel by forcing an Altivec exception in a  
> guest.
> > > I
> > > > didn't want to go further down the road of adding reflectors for
> > > those
> > > > exceptions, which could make it look like the problem was dealt  
> with
> > > > even though it's still not done.
> > >
> > > I agree it's quite annoying to hit a build breakage. Reflection  
> is not
> > > a proper solution for this problem (though we will require it  
> later)
> > > but program exception injection looks feasible as a simple fix.
> >
> > Program exception injection still doesn't deal with state  
> corruption.
> 
> Yes but it's not critical for this particular case since nobody is  
> able
> to effectively use that state via altivec instructions. Leaking state
> however can be a real issue.

Depending on guest behavior it could look like things are working even  
though they aren't (e.g. a guest just enables MSR[VEC] and uses altivec  
instructions, not relying on exceptions).  This really isn't worth  
spending a lot of time debating...  Once Altivec is fixed properly (you  
said that'd be soon, right?), we can add e6500 back to the list.

-Scott

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Scott Wood]

On 05/10/2013 02:15:34 PM, Scott Wood wrote:
> On 05/10/2013 02:06:53 PM, Caraman Mihai Claudiu-B02008 wrote:
>> > > > I didn't see Tiejun's patch...  My goal was just to fix the  
>> build
>> > > break
>> > > > without exposing problems, and to encourage a patch to fix it
>> > > properly
>> > > > to happen sooner rather than later.  With Tiejun's patch,  
>> which is
>> > > > similar to mine except that it doesn't disable e6500 support,  
>> a user
>> > > > could BUG() the kernel by forcing an Altivec exception in a  
>> guest.
>> > > I
>> > > > didn't want to go further down the road of adding reflectors  
>> for
>> > > those
>> > > > exceptions, which could make it look like the problem was  
>> dealt with
>> > > > even though it's still not done.
>> > >
>> > > I agree it's quite annoying to hit a build breakage. Reflection  
>> is not
>> > > a proper solution for this problem (though we will require it  
>> later)
>> > > but program exception injection looks feasible as a simple fix.
>> >
>> > Program exception injection still doesn't deal with state  
>> corruption.
>> 
>> Yes but it's not critical for this particular case since nobody is  
>> able
>> to effectively use that state via altivec instructions. Leaking state
>> however can be a real issue.
> 
> Depending on guest behavior it could look like things are working  
> even though they aren't (e.g. a guest just enables MSR[VEC] and uses  
> altivec instructions, not relying on exceptions).  This really isn't  
> worth spending a lot of time debating...  Once Altivec is fixed  
> properly (you said that'd be soon, right?), we can add e6500 back to  
> the list.

Am I going to see an Altivec patch soon, or should I ask Gleb to take  
this patch instead?

-Scott

RE: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Caraman Mihai Claudiu-B02008]

> > Depending on guest behavior it could look like things are working
> > even though they aren't (e.g. a guest just enables MSR[VEC] and uses
> > altivec instructions, not relying on exceptions).  This really isn't
> > worth spending a lot of time debating...  Once Altivec is fixed
> > properly (you said that'd be soon, right?), we can add e6500 back to
> > the list.
> 
> Am I going to see an Altivec patch soon, or should I ask Gleb to take
> this patch instead?

Yes, I will add ONE_REG support today and send it on the list.

-Mike

Re: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec, and disable e6500

[Scott Wood]

On 05/10/2013 04:40:01 AM, Caraman Mihai Claudiu-B02008 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, May 10, 2013 6:15 AM
> > To: Alexander Graf
> > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Wood Scott-B07421;
> > Caraman Mihai Claudiu-B02008
> > Subject: [PATCH] kvm/ppc/booke64: fix build breakage from Altivec,  
> and
> > disable e6500
> >
> > BookE altivec support brought two new exceptions, but KVM was not
> > updated, so the build broke for all 64-bit booke with KVM enabled.
> 
> We couldn't do that in KVM before having  
> BOOKE_INTERRUPT_ALTIVEC_UNAVAIL/
> BOOKE_INTERRUPT_ALTIVEC_ASSIST. As Tiejun noticed earlier we should  
> have
> done this in Kumar's tree but we missed that chance. We will face  
> similar
> issues every time an exception handler will be added.

Exceptions don't get handled all that often, and ideally we catch it  
when it's added rather than after-the-fact.

> > diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
> > b/arch/powerpc/kvm/bookehv_interrupts.S
> > index e8ed7d6..6397613 100644
> > --- a/arch/powerpc/kvm/bookehv_interrupts.S
> > +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> > @@ -309,6 +309,10 @@ kvm_handler BOOKE_INTERRUPT_DOORBELL_CRITICAL,
> > EX_PARAMS(CRIT), \
> >  	SPRN_CSRR0, SPRN_CSRR1, 0
> >  kvm_handler BOOKE_INTERRUPT_HV_PRIV, EX_PARAMS(GEN), \
> >  	SPRN_SRR0, SPRN_SRR1, NEED_EMU
> > +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
> > +	SPRN_SRR0, SPRN_SRR1, 0
> > +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
> > +	SPRN_SRR0, SPRN_SRR1, 0
> 
> Why not NEED_ESR as we did in our SDK?

This is just to fix the build break -- we don't need ESR yet.  Though I  
did look at the ESR documentation and didn't see where Altivec  
exceptions used it (I do see a couple places now).

-Scott