| Message ID | 20220106200304.4070825-7-farosas@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: PPC: MMIO fixes | expand |
On 07/01/2022 07:03, Fabiano Rosas wrote: > If MMIO emulation fails we don't want to crash the whole guest by > returning to userspace. > > The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM > implementation") added a todo: > > /* XXX Deliver Program interrupt to guest. */ > > and later the commit d69614a295ae ("KVM: PPC: Separate loadstore > emulation from priv emulation") added the Program interrupt injection > but in another file, so I'm assuming it was missed that this block > needed to be altered. > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> Looks right. Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> but this means if I want to keep debugging those kvm selftests in comfort, I'll have to have some exception handlers in the vm as otherwise the failing $pc is lost after this change :) > --- > arch/powerpc/kvm/powerpc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index a2e78229d645..50e08635e18a 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -309,7 +309,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu) > kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst); > kvmppc_core_queue_program(vcpu, 0); > pr_info("%s: emulation failed (%08x)\n", __func__, last_inst); > - r = RESUME_HOST; > + r = RESUME_GUEST; > break; > } > default:
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > On 07/01/2022 07:03, Fabiano Rosas wrote: >> If MMIO emulation fails we don't want to crash the whole guest by >> returning to userspace. >> >> The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM >> implementation") added a todo: >> >> /* XXX Deliver Program interrupt to guest. */ >> >> and later the commit d69614a295ae ("KVM: PPC: Separate loadstore >> emulation from priv emulation") added the Program interrupt injection >> but in another file, so I'm assuming it was missed that this block >> needed to be altered. >> >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > > > Looks right. > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > but this means if I want to keep debugging those kvm selftests in > comfort, I'll have to have some exception handlers in the vm as > otherwise the failing $pc is lost after this change :) Yes! But that will be a problem for any test. These kinds of issues is why I wanted a trial period before sending the test infrastructure upstream. Maybe we don't need exception handlers, but just a way to force the test to crash if it tries to fetch from 0x700.
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index a2e78229d645..50e08635e18a 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -309,7 +309,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu) kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst); kvmppc_core_queue_program(vcpu, 0); pr_info("%s: emulation failed (%08x)\n", __func__, last_inst); - r = RESUME_HOST; + r = RESUME_GUEST; break; } default:
If MMIO emulation fails we don't want to crash the whole guest by returning to userspace. The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM implementation") added a todo: /* XXX Deliver Program interrupt to guest. */ and later the commit d69614a295ae ("KVM: PPC: Separate loadstore emulation from priv emulation") added the Program interrupt injection but in another file, so I'm assuming it was missed that this block needed to be altered. Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- arch/powerpc/kvm/powerpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)