> -----Original Message----- > From: Alexander Graf [mailto:agraf@xxxxxxx] > Sent: Friday, May 10, 2013 4:16 PM > To: Caraman Mihai Claudiu-B02008 > Cc: Wood Scott-B07421; kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; 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@xxxxxxxxxxxxx> > >> Cc: Mihai Caraman <mihai.caraman@xxxxxxxxxxxxx> > >> --- > >> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
