| Message ID | 300B73AA675FCE4A93EB4FC1D42459FF3FE2FA@039-SN2MPN1-011.039d.mgd.msft.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
> -----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 -- 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
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 -- 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
> > 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 -- 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
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 -- 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
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 -- 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
> -----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 -- 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
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 -- 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
> -----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 -- 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
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 -- 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
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 -- 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
> -----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 -- 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
> -----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 -- 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
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 > -- 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
> > > 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 -- 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
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 -- 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
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 -- 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
> > 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 -- 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
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);