Re: [PATCH v4 6/6] KVM: nVMX: Enable nested posted interrupt processing

[Wincy Van]

On Mon, Feb 2, 2015 at 7:03 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
>
> On 28/01/2015 17:02, Wincy Van wrote:
>> +static int vmx_deliver_nested_posted_interrupt(struct kvm_vcpu *vcpu,
>> +                                               int vector)
>> +{
>> +       if (is_guest_mode(vcpu) &&
>> +           vector == to_vmx(vcpu)->nested.posted_intr_nv &&
>> +           vcpu->mode == IN_GUEST_MODE) {
>> +               /* the PIR and ON have been set by L1. */
>
> What happens if there is a L2->L0->L2 exit on the target VCPU, and the
> guest exits before apic->send_IPI_mask sends the IPI?
>
> The L1 hypervisor might "know" somehow that there cannot be a concurrent
> L2->L1->L2 exit, and not do the equivalent of KVM's
>

In non-nested case, if a posted intr was not accomplished by hardware,
we will sync the pir to irr and set rvi to accomplish it.
Current implementation may lead some of the nested posted intrs delay
for a short time(wait for a nested-vmexit).

>         kvm_make_request(KVM_REQ_EVENT, vcpu);
>
> after it sets ON.
>
> So I think you have to do something like
>
> static bool vmx_is_nested_posted_interrupt(struct kvm_vcpu *vcpu,
>                                            int vector)
> {
>         return (is_guest_mode(vcpu) &&
>                 vector == to_vmx(vcpu)->nested.posted_intr_nv);
> }
>
> and in vmx_deliver_posted_interrupt:
>
>         r = 0;
>         if (!vmx_is_nested_posted_interrupt(vcpu, vector)) {
>                 if (pi_test_and_set_pir(vector, &vmx->pi_desc))
>                         return;
>
>                 r = pi_test_and_set_on(&vmx->pi_desc);
>         }
>         kvm_make_request(KVM_REQ_EVENT, vcpu);
> #ifdef CONFIG_SMP
>         if (!r && (vcpu->mode == IN_GUEST_MODE))
>                 apic->send_IPI_mask(get_cpu_mask(vcpu->cpu),
>                                 POSTED_INTR_VECTOR);
>         else
> #endif
>                 kvm_vcpu_kick(vcpu);
>
>
> What do you think?
>

I think that there would be a way to avoid that delay, but may hurt performance:
When doing nested posted intr, we can set a request bit:

       if (is_guest_mode(vcpu) &&
            vector == to_vmx(vcpu)->nested.posted_intr_nv &&
            vcpu->mode == IN_GUEST_MODE) {
                /* the PIR and ON have been set by L1. */
                apic->send_IPI_mask(get_cpu_mask(vcpu->cpu),
                                POSTED_INTR_VECTOR);
+                kvm_make_request(KVM_REQ_ACCOMP_POSTED_INTR, vcpu);
                return 0;
        }

If a posted intr was not accomplished by hardware,  we can check that
bit before checking KVM_REQ_EVENT, and if that bit is set, we can do:

static void vmx_accomp_nested_posted_intr(struct kvm_vcpu *vcpu)
{
        struct vcpu_vmx *vmx = to_vmx(vcpu);

        if (is_guest_mode(vcpu) &&
            vmx->nested.posted_intr_nv != -1 &&
            pi_test_on(vmx->nested.pi_desc))
                kvm_apic_set_irr(vcpu,
                        vmx->nested.posted_intr_nv);
}

Then we will get an nested-vmexit in vmx_check_nested_events, that
posted intr will be handled by L1 immediately.
This mechanism will also emulate the hardware's behavior: If a posted
intr was not accomplished by hardware, we will get an interrupt with
POSTED_INTR_NV.

Would this be better?

Thanks,
Wincy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/