| Message ID | 1458073117-24652-1-git-send-email-lvivier@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi,
as Paolo has merged the test into kvm-unit-tests, this patch (and
original bug) can be now tested with it.
git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git
at least:
be9b007 powerpc: add test to check invalid instruction trap
Run this with KVM-PR and check your dmesg:
qemu-system-ppc64 -machine pseries,accel=kvm \
-bios powerpc/boot_rom.bin \
-display none -serial stdio \
-kernel powerpc/emulator.elf -smp 1
Laurent
On 15/03/2016 21:18, 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);
> + emulated = EMULATE_AGAIN;
> }
> break;
> case 19:
>
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ping? On 15/03/2016 21:18, 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); > + emulated = EMULATE_AGAIN; > } > break; > case 19: > -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15.03.2016 21:18, 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); > + emulated = EMULATE_AGAIN; > } > break; > case 19: > Tested-by: Thomas Huth <thuth@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21.04.2016 11:25, Thomas Huth wrote: > On 15.03.2016 21:18, 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); >> + emulated = EMULATE_AGAIN; >> } >> break; >> case 19: >> > > Tested-by: Thomas Huth <thuth@redhat.com> Ping! Alex, Paul, could you please pick up this patch? This patch is required to get the kvm-unit-tests working properly with kvm-pr, so I'd be glad if we could get this included finally... Thanks, Thomas -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/05/2016 10:14, Thomas Huth wrote: >> > Tested-by: Thomas Huth <thuth@redhat.com> > Ping! > > Alex, Paul, could you please pick up this patch? This patch is required > to get the kvm-unit-tests working properly with kvm-pr, so I'd be glad > if we could get this included finally... I have a pull request for 4.6 final to send tomorrow; do you want me to include it? Thanks, Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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... Laurent -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Please try kvm-unit-tests to see what happens: qemu-system-ppc64 -machine pseries,accel=kvm,kvm-type=PR -bios powerpc/boot_rom.bin -display none -serial stdio -kernel powerpc/emulator.elf -smp 1 --append "-v" Thanks, Laurent -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); + emulated = EMULATE_AGAIN; } break; case 19:
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(-)