From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v6] KVM: arm/arm64: Route vtimer events to user space Date: Fri, 23 Sep 2016 14:40:48 +0200 Message-ID: <20160923124048.GI9101@cbox> References: <1474628854-69945-1-git-send-email-agraf@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Alexander Graf , kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, drjones@redhat.com, marc.zyngier@arm.com, peter.maydell@linaro.org To: Paolo Bonzini Return-path: Received: from mail-wm0-f49.google.com ([74.125.82.49]:36745 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932823AbcIWMkl (ORCPT ); Fri, 23 Sep 2016 08:40:41 -0400 Received: by mail-wm0-f49.google.com with SMTP id w84so29377152wmg.1 for ; Fri, 23 Sep 2016 05:40:41 -0700 (PDT) Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Sep 23, 2016 at 02:11:41PM +0200, Paolo Bonzini wrote: > > > On 23/09/2016 13:07, Alexander Graf wrote: > > + timer_ret = kvm_timer_sync_hwstate(vcpu); > > > > kvm_vgic_sync_hwstate(vcpu); > > > > preempt_enable(); > > > > ret = handle_exit(vcpu, run, ret); > > + > > + if ((ret == 1) && timer_ret) { > > + /* > > + * We have to exit straight away to ensure that we only > > + * ever notify user space once about a level change > > + */ > > Is this really a requirement? It complicates the logic noticeably. > If we skip this, then we have to somehow remember that the sync may have updated the line level when we reach the flush state (and didn't exit), and I would like maintaining that state even less than doing this check. What we could do is to compare the timer->irq.level with the kvm_run state outside of the run loop (on the way to userspace) and update the kvm_run state if there's a discrepancy. That way, we can maintain the 'timer->irq.level != kvm_run' means, we have to exit, and whenever we are exiting, we are reporting the most recent state to user space anyway. Would that work? (By the way, the check should be via a call into the arch timer code, kvm_timer_update_user_state() or something like that). Also, the comment above is confusing, because that's not why this check is here, the check is here so that we don't loose track of the timer output level change if there's no exit to userspace. Thanks, -Christoffer