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 <lvivier@xxxxxxxxxx> >>>>>>>>> --- >>>>>>>>> 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 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html