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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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
