Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Wei Liu <wl@xen.org>, Jan Beulich <jbeulich@suse.com>,
	"Nakajima, Jun" <jun.nakajima@intel.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [Xen-devel] [PATCH v3 3/4] x86/nvmx: split updating RVI from SVI in nvmx_update_apicv
Date: Fri, 27 Mar 2020 10:48:35 +0100
Message-ID: <20200327094835.GM28601@Air-de-Roger> (raw)
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D19D7EC9E0@SHSMSX104.ccr.corp.intel.com>

On Fri, Mar 27, 2020 at 02:21:46AM +0000, Tian, Kevin wrote:
> > From: Roger Pau Monne <roger.pau@citrix.com>
> > Sent: Thursday, March 26, 2020 11:27 PM
> > 
> > Updating SVI is required when an interrupt has been injected using the
> > Ack on exit VMEXIT feature, so that the in service interrupt in the
> > GUEST_INTR_STATUS matches the vector that is signaled in
> > VM_EXIT_INTR_INFO.
> > 
> > Updating RVI however is not tied to the Ack on exit feature, as it
> > signals the next vector to be injected, and hence should always be
> > updated to the next pending vector, regardless of whether Ack on exit
> > is enabled.
> > 
> > When not using the Ack on exit feature preserve the previous vector in
> > SVI, so that it's not lost when RVI is updated to contain the pending
> > vector to inject.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>, with one small comment:
> 
> > ---
> > Changes since v2:
> >  - Return early if the exit reason != EXTERNAL_INTERRUPT.
> >  - Reduce the number of vmwrites by accumulating the changes to a
> >    local variable which is flushed at the end of the function.
> >  - Attempt to preserve the exiting SVI if Ack on exit is not enabled.
> > ---
> >  xen/arch/x86/hvm/vmx/vvmx.c | 33 ++++++++++++++++++++++++---------
> >  1 file changed, 24 insertions(+), 9 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> > index 1753005c91..39fb553590 100644
> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> > @@ -1384,28 +1384,43 @@ static void nvmx_update_apicv(struct vcpu *v)
> >      struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> >      unsigned long reason = get_vvmcs(v, VM_EXIT_REASON);
> >      unsigned long intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO);
> > +    unsigned long status;
> > +    int rvi;
> > 
> > -    if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
> > -         nvmx->intr.source == hvm_intsrc_lapic &&
> > +    if ( reason != EXIT_REASON_EXTERNAL_INTERRUPT )
> > +        return;
> 
> can we also exit if source is not lapic? as we discussed in another
> thread, the whole logic here is only for lapic not others...

Right, in any case the code below will only update GUEST_INTR_STATUS
(either SVI or RVI) for lapic interrupts, but returning early if the
source is not the lapic will do no harm AFAICT.

Will send v4 with this changed.

Thanks, Roger.


  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26 15:27 [Xen-devel] [PATCH v3 0/4] x86/nvmx: fixes for interrupt injection Roger Pau Monne
2020-03-26 15:27 ` [Xen-devel] [PATCH v3 1/4] Revert "x86/vvmx: fix virtual interrupt injection when Ack on exit control is used" Roger Pau Monne
2020-03-26 15:27 ` [Xen-devel] [PATCH v3 2/4] x86/nvmx: only update SVI when using Ack on exit Roger Pau Monne
2020-03-26 15:27 ` [Xen-devel] [PATCH v3 3/4] x86/nvmx: split updating RVI from SVI in nvmx_update_apicv Roger Pau Monne
2020-03-27  2:21   ` Tian, Kevin
2020-03-27  9:48     ` Roger Pau Monné [this message]
2020-03-26 15:27 ` [Xen-devel] [PATCH v3 4/4] x86/nvmx: update exit bitmap when using virtual interrupt delivery Roger Pau Monne
2020-03-27  2:22   ` Tian, Kevin
2020-03-26 15:41 ` [Xen-devel] [PATCH v3 0/4] x86/nvmx: fixes for interrupt injection Jan Beulich
2020-03-26 15:49   ` Roger Pau Monné

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=20200327094835.GM28601@Air-de-Roger \
    --to=roger.pau@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git