On 10.05.2013, at 16:11, Caraman Mihai Claudiu-B02008 wrote: >> -----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? 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
