All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Wincy Van <fanwenyi0529@gmail.com>,
	gleb@kernel.org, yang.z.zhang@intel.com
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Wanpeng Li <wanpeng.li@linux.intel.com>,
	Jan Kiszka <jan.kiszka@web.de>
Subject: Re: [PATCH v2 5/5] KVM: nVMX: Enable nested posted interrupt processing.
Date: Tue, 20 Jan 2015 10:54:16 +0100	[thread overview]
Message-ID: <54BE25C8.4080603@redhat.com> (raw)
In-Reply-To: <CACzj_yWxK9d_47a9ZHqtk2HSJBN6+LvtDScQqWd5znfSt8ia0Q@mail.gmail.com>



On 20/01/2015 09:48, Wincy Van wrote:
> +static int vmx_deliver_nested_posted_interrupt(struct kvm_vcpu *vcpu,
> +                                               int vector)
> +{
> +       int r = 0;
> +       struct vmcs12 *vmcs12;
> +
> +       /*
> +        * Since posted intr delivery is async,
> +        * we must aquire a spin-lock to avoid
> +        * the race of vmcs12.
> +        */
> +       spin_lock(&to_vmx(vcpu)->nested.vmcs12_lock);
> +       vmcs12 = get_vmcs12(vcpu);
> +       if (!is_guest_mode(vcpu) || !vmcs12) {
> +               r = -1;
> +               goto out;
> +       }

is_guest_mode should be checked first outside the lock, to avoid
affecting the non-nested fast path.  You can then recheck it later
inside the lock.

Another way to avoid the spinlock: in prepare_vmcs02 or a similar place,
you can save vmcs12->posted_intr_nv in a new field
vmx->nested.posted_intr_nv; just set it to -1 if
!nested_cpu_has_posted_intr(vmcs12).  In vmclear, again you just set the
field to -1, and here you can do

	if (!is_guest_mode(vcpu) ||
	    vector != to_vmx(vcpu)->nested.posted_intr_nv) {
		r = -1;
		goto out;
	}

You don't need to access vmcs12, and while there is a race, it's okay
because there is no pointer access.

> 
> +               if (vcpu->mode == IN_GUEST_MODE)
> +                       apic->send_IPI_mask(get_cpu_mask(vcpu->cpu),
> +                               POSTED_INTR_VECTOR);

Please add a comment that PIR and ON have been set by the L1 hypervisor.

I'll do a full review the other patches as soon as possible.

Paolo

> +               else {
> +                       r = -1;
> +                       goto out;
> +               }
> +
> +               /*
> +                * if posted intr is done by hardware, the
> +                * corresponding eoi was sent to L0. Thus
> +                * we should send eoi to L1 manually.
> +                */
> +               kvm_apic_set_eoi_accelerated(vcpu,
> +                       vmcs12->posted_intr_nv);

  reply	other threads:[~2015-01-20  9:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-20  8:48 [PATCH v2 5/5] KVM: nVMX: Enable nested posted interrupt processing Wincy Van
2015-01-20  9:54 ` Paolo Bonzini [this message]
2015-01-20 10:28   ` Wincy Van
2015-01-21  8:07 ` Zhang, Yang Z
2015-01-21  8:07   ` Zhang, Yang Z
2015-01-21  8:44   ` Wincy Van
2015-01-21  8:49     ` Zhang, Yang Z
2015-01-21  8:49       ` Zhang, Yang Z
2015-01-21 10:35       ` 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=54BE25C8.4080603@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=fanwenyi0529@gmail.com \
    --cc=gleb@kernel.org \
    --cc=jan.kiszka@web.de \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wanpeng.li@linux.intel.com \
    --cc=yang.z.zhang@intel.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.