On 08.01.2012, at 17:13, Avi Kivity wrote:
> On 01/08/2012 05:37 PM, Alexander Graf wrote:
>> On 08.01.2012, at 16:22, Avi Kivity wrote:
>>
>>> On 01/08/2012 05:11 PM, Alexander Graf wrote:
>>>> On 08.01.2012, at 14:18, Avi Kivity wrote:
>>>>
>>>>> On 01/04/2012 03:10 AM, Alexander Graf wrote:
>>>>>> From: Scott Wood <scottwood@xxxxxxxxxxxxx>
>>>>>>
>>>>>> Currently we check prior to returning from a lightweight exit,
>>>>>> but not prior to initial entry.
>>>>>>
>>>>>> book3s already does a similar test.
>>>>>>
>>>>>> Signed-off-by: Scott Wood <scottwood@xxxxxxxxxxxxx>
>>>>>> Signed-off-by: Alexander Graf <agraf@xxxxxxx>
>>>>>> ---
>>>>>> arch/powerpc/kvm/booke.c | 10 +++++++++-
>>>>>> 1 files changed, 9 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>>>>> index b642200..9c78589 100644
>>>>>> --- a/arch/powerpc/kvm/booke.c
>>>>>> +++ b/arch/powerpc/kvm/booke.c
>>>>>> @@ -322,11 +322,19 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
>>>>>> }
>>>>>>
>>>>>> local_irq_disable();
>>>>>> +
>>>>>> + if (signal_pending(current)) {
>>>>>> + kvm_run->exit_reason = KVM_EXIT_INTR;
>>>>>> + ret = -EINTR;
>>>>>> + goto out;
>>>>>> + }
>>>>>> +
>>>>>> kvm_guest_enter();
>>>>>> ret = __kvmppc_vcpu_run(kvm_run, vcpu);
>>>>>> kvm_guest_exit();
>>>>>> - local_irq_enable();
>>>>>>
>>>>>>
>>>>>
>>>>> In general a single check prior to entry is sufficient (well, in
>>>>> addition to the one in kvm_vcpu_block()).
>>>>
>>>> Yes, and IIUC this is the single check prior to entry. On lightweight exit, we don't return from __kvmppc_vcpu_run, but only call kvmppc_handle_exit() and if that exits that we return to the guest, we stay inside of __kvmppc_vcpu_run and don't return from here.
>>>
>>> It means you check twice per heavyweight exit, no? Once here, and once
>>> when kvmppc_handle_exit() returns. If, instead, you move the check to
>>> just before the lightweight entry, you check just once per entry, like x86.
>>
>> You mean we check twice in case a heavyweight exit occurs right after another heavyweight exit? We need to check whether a signal is pending to determine heavyweight exits, so we definitely have to check at the end of the exit handlers:
>>
>> - check signal (none pending)
>> - enter guest
>> - exit guest because an external interrupt occurs (lightweight)
>> - check signal (pending now because we got preempted)
>> -> make exit heavyweight
>>
>> If however there already is a signal pending before we enter the guest and the guest is running an endless loop and the host doesn't have any timers configured because it's just waiting for the signal to be handled, we would be in a dead loop. So we have to check before entry either way.
>
> Yes, but you're checking in the wrong place.
>
> What you have now is:
>
> if (!signal_pending())
> do
> enter guest
> while (!signal_pending && is_lightweight_exit)
Well, what we really do is
if (!signal_pending())
do
run guest
if (signal_pending())
is_lightweight_exit = 0;
while is_lightweight_exit
but yes :).
>
> What x86 does is
>
> while (!signal_pending() && can_reenter)
> can_reenter = enter guest
>
> well, that's not actually what x86 does, but what it should be doing.
> The point is checking for signals after an exit is meaningless. You've
> exited, so the guest can't be holding off a signal for the host. The
> check after the exit is really meant for the next entry, but it doesn't
> cover the first entry, so you have another check before that.
Yes, but the end of the exit handler is basically the beginning of the entry loop. If we already know that we're in a heavy weight exit, we don't check for signals anymore:
if (!(r & RESUME_HOST)) {
/* To avoid clobbering exit_reason, only check for signals if
* we aren't already exiting to userspace for some other
* reason. */
if (signal_pending(current)) {
run->exit_reason = KVM_EXIT_INTR;
r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
kvmppc_account_exit(vcpu, SIGNAL_EXITS);
}
}
That way we do basically what you're suggesting, but are a bit more explicit, having checking code at the beginning and end and some conditional checking if it's required or not.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
