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
