Il 31/05/2013 06:36, Gleb Natapov ha scritto:
> In my commit message there is two INITs in a row:
> vpu0: vcpu1:
> set INIT
> test_and_clear_bit(KVM_APIC_INIT)
> process INIT
> set INIT
> set SIPI
> test_and_clear_bit(KVM_APIC_SIPI)
> process SIPI
>
> Two INITs before SIPI are essential to trigger the bug
I see now. Let's draw pending_events as well:
event sent event processed pending_events
INIT INIT
INIT 0
INIT INIT
SIPI INIT|SIPI
SIPI INIT
INIT 0
Events are reordered, there is indeed a bug if the second INIT comes at
just the right time. With your patch:
event sent event processed pending_events
INIT INIT
INIT 0
INIT INIT
SIPI INIT|SIPI
SIPI, failed cmpxchg INIT|SIPI
INIT SIPI
SIPI SIPI
The patch introduces a spurious SIPI, that's worse than coalescing.
With my patch:
event sent event processed pending_events
INIT INIT
INIT 0
INIT INIT
SIPI INIT|SIPI
(failed cmpxchg) INIT|SIPI
INIT SIPI
SIPI 0
My patch looks better to me for this scenario.
> and coincidentally this is what spec advices to do.
The spec advises INIT-SIPI-SIPI, not INIT-INIT-SIPI. This is because
the first INIT may have been processed late, and SIPIs are masked if not
in wait-for-SIPI state. You need to send the second just in case. It
is not needed in KVM because INITs effectively unmask the SIPI
immediately, even though the INIT may take place a bit later.
The INIT-SIPI-SIPI sequence is handled correctly by KVM thanks to the
check on the mp-state. But your patch breaks another corner case:
event sent event processed pending_events
INIT INIT
INIT 0
SIPI SIPI
SIPI 0
SIPI SIPI
ignored SIPI SIPI
set_mp_state(INIT_RECEIVED) SIPI
SIPI 0
With my patch, or no patch at all:
event sent event processed pending_events
INIT INIT
INIT 0
SIPI SIPI
SIPI 0
SIPI SIPI
ignored SIPI 0
set_mp_state(INIT_RECEIVED) 0
Though perhaps the real bug here is in kvm_arch_vcpu_ioctl_setmp_state.
Setting the mp_state to anything bug SIPI_RECEIVED should clear the SIPI
event.
Paolo
>> event sent event processed pending_events
>> INIT INIT
>> SIPI INIT|SIPI
>> INIT SIPI
>> SIPI 0
>> INIT INIT
>> SIPI INIT|SIPI
>> INIT SIPI
>> SIPI 0
--
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
