Re: [PATCH v5 19/20] KVM: arm/arm64: Get rid of kvm_timer_flush_hwstate

Christoffer Dall
2017-11-29T17:39:00Z

On Mon, Nov 27, 2017 at 05:50:12PM +0100, Andrew Jones wrote:
> On Fri, Oct 27, 2017 at 10:34:40AM +0200, Christoffer Dall wrote:
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index 132d39a..14c50d1 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -656,7 +656,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> >
> > local_irq_disable();
> >
> > - kvm_timer_flush_hwstate(vcpu);
> > kvm_vgic_flush_hwstate(vcpu);
> >
> > /*
> > --
> > 2.7.4
>
> Hi Christoffer,
>
> I realize this is already merged, but I have a question about the
> above hunk. IIUC, the patch "KVM: arm/arm64: Move timer/vgic
> flush/sync under disabled irq" only moved kvm_vgic_flush_hwstate()
> under disabled irq because it had to be run after
> kvm_timer_flush_hwstate(). Now that kvm_timer_flush_hwstate() is
> gone can/should it be moved back out?
>

That's a great question.

I think my reasoning was, that since we now disable the VCPU interface
(and thereby maintenance interrupts), any logic that relies on a
maintenance interrupt firing and causing a guest exit after we've called
flush, would no longer work if we move this back out where interrupts
are enabled.

Thinking about this a bit more, I don't think that would ever make
sense, and we should be able to move it out.

I've tested these patches pretty heavily and would want such a change to
get the same kind of exposure, so I can start testing it, and if there
are no issues, put it in next later on in the cycle.

Does that sound reasonable?

Thanks,
-Christoffer