On Tue, Dec 22, 2015 at 11:08:10AM +0000, Peter Maydell wrote: > On 22 December 2015 at 09:55, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > > Assuming we trap a coprocessor access, and decide that the access > > is illegal, we will inject an exception in the guest. In this > > case, we shouldn't increment the PC, or the vcpu will miss the > > first instruction of the handler, leading to a mildly confused > > guest. > > > > Solve this by snapshoting PC before the access is performed, > > and checking if it has moved or not before incrementing it. > > > > Reported-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx> > > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> > > --- > > arch/arm/kvm/coproc.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c > > index f3d88dc..f4ad2f2 100644 > > --- a/arch/arm/kvm/coproc.c > > +++ b/arch/arm/kvm/coproc.c > > @@ -447,12 +447,22 @@ static int emulate_cp15(struct kvm_vcpu *vcpu, > > r = find_reg(params, cp15_regs, ARRAY_SIZE(cp15_regs)); > > > > if (likely(r)) { > > + unsigned long pc = *vcpu_pc(vcpu); > > + > > /* If we don't have an accessor, we should never get here! */ > > BUG_ON(!r->access); > > > > if (likely(r->access(vcpu, params, r))) { > > - /* Skip instruction, since it was emulated */ > > - kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); > > + /* > > + * Skip the instruction if it was emulated > > + * without PC having changed. This allows us > > + * to detect a fault being injected > > + * (incrementing the PC here would cause the > > + * vcpu to skip the first instruction of its > > + * fault handler). > > + */ > > + if (pc == *vcpu_pc(vcpu)) > > + kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); > > Won't this result in our incorrectly skipping the first insn > in the fault handler if the original offending instruction > was itself the first insn in the fault handler? > Wouldn't that then loop with the exception forever? -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html