| Message ID | 1491400731-2492-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05.04.2017 15:58, Thomas Huth wrote: > According to the PowerISA 2.07, mtspr and mfspr should not always > generate an illegal instruction exception when being used with an > undefined SPR, but rather treat the instruction as a NOP or inject a > privilege exception in some cases, too - depending on the SPR number. > Also turn the printk here into a ratelimited print statement, so that > the guest can not flood the dmesg log of the host by issueing lots of > illegal mtspr/mfspr instruction here. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v3: > - Make sure that we do not advance the program counter after we've > already changed it due to injecting a program interrupt > > v2: > - Inject illegal instruction program interrupt instead of emulation > assist interrupt (according to the last programming note in section > 6.5.9 of Book III of the PowerISA v2.07) > > arch/powerpc/kvm/book3s_emulate.c | 34 ++++++++++++++++++++++++++-------- > arch/powerpc/kvm/emulate.c | 8 ++++++++ > 2 files changed, 34 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c > index 8359752..68d6898 100644 > --- a/arch/powerpc/kvm/book3s_emulate.c > +++ b/arch/powerpc/kvm/book3s_emulate.c > @@ -503,10 +503,18 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) > break; > unprivileged: > default: > - printk(KERN_INFO "KVM: invalid SPR write: %d\n", sprn); > -#ifndef DEBUG_SPR > - emulated = EMULATE_FAIL; > -#endif > + pr_info_ratelimited("KVM: invalid SPR write: %d\n", sprn); > + if (sprn & 0x10) { > + if (kvmppc_get_msr(vcpu) & MSR_PR) { > + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV); > + emulated = EMULATE_AGAIN; > + } > + } else { > + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0) { > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > + emulated = EMULATE_AGAIN; > + } > + } > break; > } > > @@ -648,10 +656,20 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val > break; > default: > unprivileged: > - printk(KERN_INFO "KVM: invalid SPR read: %d\n", sprn); > -#ifndef DEBUG_SPR > - emulated = EMULATE_FAIL; > -#endif > + pr_info_ratelimited("KVM: invalid SPR read: %d\n", sprn); > + if (sprn & 0x10) { > + if (kvmppc_get_msr(vcpu) & MSR_PR) { > + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV); > + emulated = EMULATE_AGAIN; > + } > + } else { > + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0 || > + sprn == 4 || sprn == 5 || sprn == 6) { > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > + emulated = EMULATE_AGAIN; > + } > + } > + > break; > } > > diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c > index b379146..c873ffe 100644 > --- a/arch/powerpc/kvm/emulate.c > +++ b/arch/powerpc/kvm/emulate.c > @@ -259,10 +259,18 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu) > > case OP_31_XOP_MFSPR: > emulated = kvmppc_emulate_mfspr(vcpu, sprn, rt); > + if (emulated == EMULATE_AGAIN) { > + emulated = EMULATE_DONE; > + advance = 0; > + } > break; > > case OP_31_XOP_MTSPR: > emulated = kvmppc_emulate_mtspr(vcpu, sprn, rs); > + if (emulated == EMULATE_AGAIN) { > + emulated = EMULATE_DONE; > + advance = 0; > + } > break; > > case OP_31_XOP_TLBSYNC: > Ping! Paul, does this now look OK for you this way? Thomas
On Tue, Apr 25, 2017 at 09:14:01AM +0200, Thomas Huth wrote: > On 05.04.2017 15:58, Thomas Huth wrote: > > According to the PowerISA 2.07, mtspr and mfspr should not always > > generate an illegal instruction exception when being used with an > > undefined SPR, but rather treat the instruction as a NOP or inject a > > privilege exception in some cases, too - depending on the SPR number. > > Also turn the printk here into a ratelimited print statement, so that > > the guest can not flood the dmesg log of the host by issueing lots of > > illegal mtspr/mfspr instruction here. > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > v3: > > - Make sure that we do not advance the program counter after we've > > already changed it due to injecting a program interrupt > > > > v2: > > - Inject illegal instruction program interrupt instead of emulation > > assist interrupt (according to the last programming note in section > > 6.5.9 of Book III of the PowerISA v2.07) > > > > arch/powerpc/kvm/book3s_emulate.c | 34 ++++++++++++++++++++++++++-------- > > arch/powerpc/kvm/emulate.c | 8 ++++++++ > > 2 files changed, 34 insertions(+), 8 deletions(-) > > > > diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c > > index 8359752..68d6898 100644 > > --- a/arch/powerpc/kvm/book3s_emulate.c > > +++ b/arch/powerpc/kvm/book3s_emulate.c > > @@ -503,10 +503,18 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) > > break; > > unprivileged: > > default: > > - printk(KERN_INFO "KVM: invalid SPR write: %d\n", sprn); > > -#ifndef DEBUG_SPR > > - emulated = EMULATE_FAIL; > > -#endif > > + pr_info_ratelimited("KVM: invalid SPR write: %d\n", sprn); > > + if (sprn & 0x10) { > > + if (kvmppc_get_msr(vcpu) & MSR_PR) { > > + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV); > > + emulated = EMULATE_AGAIN; > > + } > > + } else { > > + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0) { > > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > > + emulated = EMULATE_AGAIN; > > + } > > + } > > break; > > } > > > > @@ -648,10 +656,20 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val > > break; > > default: > > unprivileged: > > - printk(KERN_INFO "KVM: invalid SPR read: %d\n", sprn); > > -#ifndef DEBUG_SPR > > - emulated = EMULATE_FAIL; > > -#endif > > + pr_info_ratelimited("KVM: invalid SPR read: %d\n", sprn); > > + if (sprn & 0x10) { > > + if (kvmppc_get_msr(vcpu) & MSR_PR) { > > + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV); > > + emulated = EMULATE_AGAIN; > > + } > > + } else { > > + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0 || > > + sprn == 4 || sprn == 5 || sprn == 6) { > > + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); > > + emulated = EMULATE_AGAIN; > > + } > > + } > > + > > break; > > } > > > > diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c > > index b379146..c873ffe 100644 > > --- a/arch/powerpc/kvm/emulate.c > > +++ b/arch/powerpc/kvm/emulate.c > > @@ -259,10 +259,18 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu) > > > > case OP_31_XOP_MFSPR: > > emulated = kvmppc_emulate_mfspr(vcpu, sprn, rt); > > + if (emulated == EMULATE_AGAIN) { > > + emulated = EMULATE_DONE; > > + advance = 0; > > + } > > break; > > > > case OP_31_XOP_MTSPR: > > emulated = kvmppc_emulate_mtspr(vcpu, sprn, rs); > > + if (emulated == EMULATE_AGAIN) { > > + emulated = EMULATE_DONE; > > + advance = 0; > > + } > > break; > > > > case OP_31_XOP_TLBSYNC: > > > > Ping! > > Paul, does this now look OK for you this way? Yes, and I put it in, see commit feafd13c96d6. Paul.
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c index 8359752..68d6898 100644 --- a/arch/powerpc/kvm/book3s_emulate.c +++ b/arch/powerpc/kvm/book3s_emulate.c @@ -503,10 +503,18 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) break; unprivileged: default: - printk(KERN_INFO "KVM: invalid SPR write: %d\n", sprn); -#ifndef DEBUG_SPR - emulated = EMULATE_FAIL; -#endif + pr_info_ratelimited("KVM: invalid SPR write: %d\n", sprn); + if (sprn & 0x10) { + if (kvmppc_get_msr(vcpu) & MSR_PR) { + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV); + emulated = EMULATE_AGAIN; + } + } else { + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0) { + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); + emulated = EMULATE_AGAIN; + } + } break; } @@ -648,10 +656,20 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val break; default: unprivileged: - printk(KERN_INFO "KVM: invalid SPR read: %d\n", sprn); -#ifndef DEBUG_SPR - emulated = EMULATE_FAIL; -#endif + pr_info_ratelimited("KVM: invalid SPR read: %d\n", sprn); + if (sprn & 0x10) { + if (kvmppc_get_msr(vcpu) & MSR_PR) { + kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV); + emulated = EMULATE_AGAIN; + } + } else { + if ((kvmppc_get_msr(vcpu) & MSR_PR) || sprn == 0 || + sprn == 4 || sprn == 5 || sprn == 6) { + kvmppc_core_queue_program(vcpu, SRR1_PROGILL); + emulated = EMULATE_AGAIN; + } + } + break; } diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c index b379146..c873ffe 100644 --- a/arch/powerpc/kvm/emulate.c +++ b/arch/powerpc/kvm/emulate.c @@ -259,10 +259,18 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu) case OP_31_XOP_MFSPR: emulated = kvmppc_emulate_mfspr(vcpu, sprn, rt); + if (emulated == EMULATE_AGAIN) { + emulated = EMULATE_DONE; + advance = 0; + } break; case OP_31_XOP_MTSPR: emulated = kvmppc_emulate_mtspr(vcpu, sprn, rs); + if (emulated == EMULATE_AGAIN) { + emulated = EMULATE_DONE; + advance = 0; + } break; case OP_31_XOP_TLBSYNC:
According to the PowerISA 2.07, mtspr and mfspr should not always generate an illegal instruction exception when being used with an undefined SPR, but rather treat the instruction as a NOP or inject a privilege exception in some cases, too - depending on the SPR number. Also turn the printk here into a ratelimited print statement, so that the guest can not flood the dmesg log of the host by issueing lots of illegal mtspr/mfspr instruction here. Signed-off-by: Thomas Huth <thuth@redhat.com> --- v3: - Make sure that we do not advance the program counter after we've already changed it due to injecting a program interrupt v2: - Inject illegal instruction program interrupt instead of emulation assist interrupt (according to the last programming note in section 6.5.9 of Book III of the PowerISA v2.07) arch/powerpc/kvm/book3s_emulate.c | 34 ++++++++++++++++++++++++++-------- arch/powerpc/kvm/emulate.c | 8 ++++++++ 2 files changed, 34 insertions(+), 8 deletions(-)