LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Wincy Van <fanwenyi0529@gmail.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "gleb@kernel.org" <gleb@kernel.org>,
	"Zhang, Yang Z" <yang.z.zhang@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Wanpeng Li <wanpeng.li@linux.intel.com>,
	Jan Kiszka <jan.kiszka@web.de>
Subject: Re: [PATCH v4 6/6] KVM: nVMX: Enable nested posted interrupt processing
Date: Mon, 2 Feb 2015 23:33:03 +0800	[thread overview]
Message-ID: <CACzj_yUF+GHZ9Y0wc=6On2G+L0+B7g1VHybcXjwHpzxfo9ZzTA@mail.gmail.com> (raw)
In-Reply-To: <54CF5971.8090407@redhat.com>

On Mon, Feb 2, 2015 at 7:03 PM, Paolo Bonzini <pbonzini@redhat.com> 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

  reply	other threads:[~2015-02-02 15:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-28 16:02 Wincy Van
2015-02-02 11:03 ` Paolo Bonzini
2015-02-02 15:33   ` Wincy Van [this message]
2015-02-02 16:14     ` Paolo Bonzini
2015-02-03  1:21       ` Zhang, Yang Z
2015-02-03  3:32         ` Wincy Van

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CACzj_yUF+GHZ9Y0wc=6On2G+L0+B7g1VHybcXjwHpzxfo9ZzTA@mail.gmail.com' \
    --to=fanwenyi0529@gmail.com \
    --cc=gleb@kernel.org \
    --cc=jan.kiszka@web.de \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=wanpeng.li@linux.intel.com \
    --cc=yang.z.zhang@intel.com \
    --subject='Re: [PATCH v4 6/6] KVM: nVMX: Enable nested posted interrupt processing' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).