lkml.org 
[lkml]   [2016]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] kvm-pr: manage illegal instructions
From
Date


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@redhat.com>
>>>>>>>>> ---
>>>>>>>>> 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

\
 
 \ /
  Last update: 2016-05-17 20:01    [W:0.053 / U:1.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site