From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751824AbcEQRuE (ORCPT ); Tue, 17 May 2016 13:50:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36628 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750800AbcEQRuB (ORCPT ); Tue, 17 May 2016 13:50:01 -0400 Subject: Re: [PATCH] kvm-pr: manage illegal instructions To: Alexander Graf , Thomas Huth References: <1458073117-24652-1-git-send-email-lvivier@redhat.com> <57330AFE.1050209@suse.de> <57331C3F.2000708@suse.de> <57344C7E.5010202@suse.de> <374d3217-cf6c-fa2e-d24a-e9ee8b1f4534@redhat.com> <3f99bce1-6fb7-b81d-26ca-24f1ca426c3e@redhat.com> <573AD836.4090006@suse.de> Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, Michael Ellerman , Benjamin Herrenschmidt , Gleb Natapov , linuxppc-dev@lists.ozlabs.org, Paolo Bonzini , Paul Mackerras , linux-kernel@vger.kernel.org From: Laurent Vivier Message-ID: Date: Tue, 17 May 2016 19:49:51 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <573AD836.4090006@suse.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 17 May 2016 17:50:00 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17/05/2016 10:37, Alexander Graf wrote: > On 05/17/2016 10:35 AM, Laurent Vivier wrote: >> >> On 12/05/2016 16:23, Laurent Vivier wrote: >>> >>> On 12/05/2016 11:27, Alexander Graf wrote: >>>> On 05/12/2016 11:10 AM, Laurent Vivier wrote: >>>>> On 11/05/2016 13:49, Alexander Graf wrote: >>>>>> On 05/11/2016 01:14 PM, Laurent Vivier wrote: >>>>>>> On 11/05/2016 12:35, Alexander Graf wrote: >>>>>>>> On 03/15/2016 09:18 PM, Laurent Vivier wrote: >>>>>>>>> While writing some instruction tests for kvm-unit-tests for >>>>>>>>> powerpc, >>>>>>>>> I've found that illegal instructions are not managed correctly >>>>>>>>> with >>>>>>>>> kvm-pr, >>>>>>>>> while it is fine with kvm-hv. >>>>>>>>> >>>>>>>>> When an illegal instruction (like ".long 0") is processed by >>>>>>>>> kvm-pr, >>>>>>>>> the kernel logs are filled with: >>>>>>>>> >>>>>>>>> Couldn't emulate instruction 0x00000000 (op 0 xop 0) >>>>>>>>> kvmppc_handle_exit_pr: emulation at 700 failed (00000000) >>>>>>>>> >>>>>>>>> While the exception handler receives an interrupt for each >>>>>>>>> instruction >>>>>>>>> executed after the illegal instruction. >>>>>>>>> >>>>>>>>> Signed-off-by: Laurent Vivier >>>>>>>>> --- >>>>>>>>> arch/powerpc/kvm/book3s_emulate.c | 4 +++- >>>>>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/arch/powerpc/kvm/book3s_emulate.c >>>>>>>>> b/arch/powerpc/kvm/book3s_emulate.c >>>>>>>>> index 2afdb9c..4ee969d 100644 >>>>>>>>> --- a/arch/powerpc/kvm/book3s_emulate.c >>>>>>>>> +++ b/arch/powerpc/kvm/book3s_emulate.c >>>>>>>>> @@ -99,7 +99,6 @@ int kvmppc_core_emulate_op_pr(struct kvm_run >>>>>>>>> *run, >>>>>>>>> struct kvm_vcpu *vcpu, >>>>>>>>> switch (get_op(inst)) { >>>>>>>>> case 0: >>>>>>>>> - emulated = EMULATE_FAIL; >>>>>>>>> if ((kvmppc_get_msr(vcpu) & MSR_LE) && >>>>>>>>> (inst == swab32(inst_sc))) { >>>>>>>>> /* >>>>>>>>> @@ -112,6 +111,9 @@ int kvmppc_core_emulate_op_pr(struct kvm_run >>>>>>>>> *run, >>>>>>>>> struct kvm_vcpu *vcpu, >>>>>>>>> kvmppc_set_gpr(vcpu, 3, EV_UNIMPLEMENTED); >>>>>>>>> kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4); >>>>>>>>> emulated = EMULATE_DONE; >>>>>>>>> + } else { >>>>>>>>> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >>>>>>>> But isn't that exactly what the semantic of EMULATE_FAIL is? >>>>>>>> Fixing it >>>>>>>> up in book3s_emulate.c is definitely the wrong spot. >>>>>>>> >>>>>>>> So what is the problem you're trying to solve? Is the SRR0 at the >>>>>>>> wrong >>>>>>>> spot or are the log messages the problem? >>>>>>> No, the problem is the host kernel logs are filled by the message >>>>>>> and >>>>>>> the execution hangs. And the host becomes unresponsiveness, even >>>>>>> after >>>>>>> the end of the tests. >>>>>>> >>>>>>> Please, try to run kvm-unit-tests (the emulator test) on a KVM-PR >>>>>>> host, >>>>>>> and check the kernel logs (dmesg), then try to ssh to the host... >>>>>> Ok, so the log messages are the problem. Please fix the message >>>>>> output >>>>>> then - or remove it altogether. Or if you like, create a module >>>>>> parameter that allows you to emit them. >>>>>> >>>>>> I personally think the best solution would be to just convert the >>>>>> message into a trace point. >>>>>> >>>>>> While at it, please see whether the guest can trigger similar host >>>>>> log >>>>>> output excess in other code paths. >>>>> The problem is not really with the log messages: they are >>>>> consequence of >>>>> the bug I try to fix. >>>>> >>>>> What happens is once kvm_pr decodes an invalid instruction all the >>>>> valid >>>>> following instructions trigger a Program exception to the guest >>>>> (but are >>>>> executed correctly). It has no real consequence on big machine like >>>>> POWER8, except that the guest become very slow and the log files of >>>>> the >>>>> host are filled with messages (and qemu uses 100% of the CPU). On a >>>>> smaller machine like a PowerMac G5, the machine becomes simply >>>>> unusable. >>>> It's probably more related to your verbosity level of kernel messages. >>>> If you pass loglevel=0 (or quiet) to you kernel cmdline you won't get >>>> the messages printed to serial which is what's slowing you down. >>>> >>>> The other problem sounds pretty severe, but the only thing your patch >>>> does any different from the current code flow would be the patch below. >>>> Or did I miss anything? >>>> >>>> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c >>>> index 5cc2e7a..4672bc2 100644 >>>> --- a/arch/powerpc/kvm/emulate.c >>>> +++ b/arch/powerpc/kvm/emulate.c >>>> @@ -302,7 +302,11 @@ int kvmppc_emulate_instruction(struct kvm_run >>>> *run, >>>> struct kvm_vcpu *vcpu) >>>> advance = 0; >>>> printk(KERN_ERR "Couldn't emulate instruction >>>> 0x%08x " >>>> "(op %d xop %d)\n", inst, get_op(inst), >>>> get_xop(inst)); >>>> +#ifdef CONFIG_PPC_BOOK3S >>>> + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >>>> +#else >>>> kvmppc_core_queue_program(vcpu, 0); >>>> +#endif >>>> } >>>> } >>>> >> Do you want I send an updated patch with your changes? > > Well, you reported the issue and narrowed it down, so feel free to send > it under your name :). I merely simplified your patch a bit. Well, while I was trying to update the patch, I've re-tested this... and it fails. I don't know what I'm doing bad now or what I did bad before but it seems it doesn't work. :( Thomas, could try the patch from Alex? I'm trying to understand what happens... Laurent