Re: [PATCH v3 03/11] KVM: nVMX: Re-evaluate L1 pending events when running L2 and L1 got posted-interrupt

[Liran Alon <LIRAN.ALON@ORACLE.COM>]

On 02/01/18 04:45, Quan Xu wrote:
> On 2017/12/25 00:12, Liran Alon wrote:
>
>> In case posted-interrupt was delivered to CPU while it is in host
>> (outside guest), then posted-interrupt delivery will be done by
>> calling sync_pir_to_irr() at vmentry after interrupts are disabled.
>>
>> sync_pir_to_irr() will check vmx->pi_desc.control ON bit and if
>> set, it will sync vmx->pi_desc.pir to IRR and afterwards update RVI to
>> ensure virtual-interrupt-delivery will dispatch interrupt to guest.
>>
>> However, it is possible that L1 will receive a posted-interrupt while
>> CPU runs at host and is about to enter L2. In this case, the call to
>> sync_pir_to_irr() will indeed update the L1's APIC IRR but
>> vcpu_enter_guest() will then just resume into L2 guest without
>> re-evaluating if it should exit from L2 to L1 as a result of this
>> new pending L1 event.
>>
>> To address this case, if sync_pir_to_irr() has a new L1 injectable
>> interrupt and CPU is running L2, we force exit GUEST_MODE which will
>> result in another iteration of vcpu_run() run loop which will call
>> kvm_vcpu_running() which will call check_nested_events() which will
>> handle the pending L1 event properly.
>
>
> I agree with this solution.. However ...
>
>
>> Signed-off-by: Liran Alon <liran.alon@oracle.com>
>> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
>> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
>> Signed-off-by: Liam Merwick <liam.merwick@oracle.com>
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>>   arch/x86/kvm/vmx.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 325608a1ed65..d307bf26462a 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -9032,6 +9032,7 @@ static int vmx_sync_pir_to_irr(struct kvm_vcpu
>> *vcpu)
>>   {
>>       struct vcpu_vmx *vmx = to_vmx(vcpu);
>>       int max_irr;
>> +    bool max_irr_updated;
>>       WARN_ON(!vcpu->arch.apicv_active);
>>       if (pi_test_on(&vmx->pi_desc)) {
>> @@ -9041,7 +9042,16 @@ static int vmx_sync_pir_to_irr(struct kvm_vcpu
>> *vcpu)
>>            * But on x86 this is just a compiler barrier anyway.
>>            */
>>           smp_mb__after_atomic();
>> -        kvm_apic_update_irr(vcpu, vmx->pi_desc.pir, &max_irr);
>> +        max_irr_updated =
>> +            kvm_apic_update_irr(vcpu, vmx->pi_desc.pir, &max_irr);
>> +
>> +        /*
>> +         * If we are running L2 and L1 has a new pending interrupt
>> +         * which can be injected, we should re-evaluate
>> +         * what should be done with this new L1 interrupt.
>> +         */
>> +        if (is_guest_mode(vcpu) && max_irr_updated)
>> +            kvm_vcpu_exiting_guest_mode(vcpu);
>
>
> ...
>
> refer to "Virtual-Interrupt Delivery":
>
> """
>     Vector ← RVI;
>     VISR[Vector] ← 1;
>     SVI ← Vector;
>     VPPR ← Vector & F0H;
>     VIRR[Vector] ← 0;
>     IF any bits set in VIRR
>        THEN RVI ← highest index of bit set in VIRR
>        ELSE RVI ← 0;
>     FI;
>     deliver interrupt with Vector through IDT;
>     cease recognition of any pending virtual interrupt;
> """
>
>
>
> as we synced PIR to L1's APIC IRR, not only the max_irr is injectable
> but also the other vectors in L1's APIC IRR.
>
> So what we need to check is the new L1 injectable interrupt from PIR,
> even if it is not the max_irr..

The vectors which were set in the LAPIC IRR before the sync with PIR 
were already evaluated to check if they should cause #VMExit from L2 to 
L1 (in check_nested_events()).

Therefore, in vmx_sync_pir_to_irr() we only need to check if PIR have a 
new injectable interrupt set.

I failed to understand how this relates to the VID SDM algorithm you 
provided here. Can you clarify?

Thanks,
-Liran

>
> Quan
> Alibaba Cloud