public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: pbonzini@redhat.com (Paolo Bonzini)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6] KVM: arm/arm64: Route vtimer events to user space
Date: Fri, 23 Sep 2016 16:42:44 +0200	[thread overview]
Message-ID: <3f14f9f4-a408-4e16-9fcf-608c2809401d@redhat.com> (raw)
In-Reply-To: <20160923124048.GI9101@cbox>



On 23/09/2016 14:40, Christoffer Dall wrote:
> 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?

It seems clearer at least.

> (By the way, the check should be via a call into the arch timer code,
> kvm_timer_update_user_state() or something like that).

Makes sense.

Paolo

> 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
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

  reply	other threads:[~2016-09-23 14:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-23 11:07 [PATCH v6] KVM: arm/arm64: Route vtimer events to user space Alexander Graf
2016-09-23 12:11 ` Paolo Bonzini
2016-09-23 12:40   ` Christoffer Dall
2016-09-23 14:42     ` Paolo Bonzini [this message]
     [not found]   ` <D82BA95A-A89E-46EB-93D2-197DF44433EC@suse.de>
2016-09-23 12:46     ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3f14f9f4-a408-4e16-9fcf-608c2809401d@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox