public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Gleb Natapov <gleb@redhat.com>
To: "Zhang, Yang Z" <yang.z.zhang@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Shan, Haitao" <haitao.shan@intel.com>,
	"mtosatti@redhat.com" <mtosatti@redhat.com>,
	"Tian, Kevin" <kevin.tian@intel.com>
Subject: Re: [PATCH v9 2/3] x86, apicv: add virtual x2apic support
Date: Thu, 10 Jan 2013 14:34:53 +0200	[thread overview]
Message-ID: <20130110123453.GB11529@redhat.com> (raw)
In-Reply-To: <A9667DDFB95DB7438FA9D7D576C3D87E2F64DA@SHSMSX101.ccr.corp.intel.com>

On Thu, Jan 10, 2013 at 12:22:51PM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-01-10:
> > On Thu, Jan 10, 2013 at 11:54:21AM +0000, Zhang, Yang Z wrote:
> >>>> The right logic should be:
> >>>> When register virtualization enabled, read all apic msr(except apic id reg and
> >>> tmcct which have the hook) not cause vmexit and only write TPR not cause
> >>> vmexit.
> >>>> When vid enabled, write to TPR, EOI and SELF IPI not cause vmexit.
> >>>> It's better to put the patch of envirtualize x2apic mode as first patch.
> >>>> 
> >>> There is no point whatsoever to enable virtualize x2apic without
> >>> allowing at least one non intercepted access to x2apic MSR range and
> >>> this is what your patch is doing when run on cpu without vid capability.
> >> No, at least read/write TPR will not cause vmexit if virtualize x2apic mode is
> > enabled.
> > For that you need to disable 808H MSR intercept, which your patch does not do.
> I want to do this in next patch.
>  
Then don't. There is no point in the patch that enables virtualize
x2apic and does not allow at least one non-intercepted MSR access.

> >> I am not sure whether I understand your comments right in previous
> >> discussion, here is my thinking: 1. enable virtualize x2apic mode if
> >> guest is really using x2apic and clear TPR in msr read  and write
> >> bitmap. This will benefit reading TPR. 2. If APIC registers
> >> virtualization is enabled, clear all bit in rang
> > 0x800-0x8ff(except apic id reg and tmcct).
> > Clear all read bits in the range.
> > 
> >> 3. If vid is enabled, clear EOI and SELF IPI in msr write map.
> >> 
> > Looks OK.
> > 
> >> One concern you mentioned is that vid enabled and virtualize x2apic is disabled
> > but guest still use x2apic. In this case, we still use MSR bitmap to intercept x2apic.
> > This means the vEOI update will never happen. But we still can benefit from
> > interrupt window.
> >> 
> > What interrupt windows? Without virtualized x2apic TPR/EOI
> > virtualization will not happen and we rely on that in the code.
> Yes, but we can eliminate vmexit of interrupt window. Interrupt still can delivery to guest without vmexit when guest enables interrupt if vid is enabled. See SDM vol 3, 29.2.2.
> 
What does it matter that you eliminated vmexit of interrupt window if
you broke everything else? The vid patch assumes that apic emulation
either entirely happens in a software when vid is disabled or in a CPU
if vid is enabled. Mixed mode will not work (apic->isr_count = 1 trick
will break things for instance). And it is not worth it to complicate
the code to make it work.

--
			Gleb.

  reply	other threads:[~2013-01-10 12:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-10  7:26 [PATCH v9 0/3] x86, apicv: Add APIC virtualization support Yang Zhang
2013-01-10  7:26 ` [PATCH v9 1/3] x86, apicv: add APICv register " Yang Zhang
2013-01-10 20:25   ` Marcelo Tosatti
2013-01-10  7:26 ` [PATCH v9 2/3] x86, apicv: add virtual x2apic support Yang Zhang
2013-01-10  7:55   ` Gleb Natapov
2013-01-10  8:32     ` Zhang, Yang Z
2013-01-10  8:52       ` Gleb Natapov
2013-01-10 11:54         ` Zhang, Yang Z
2013-01-10 12:16           ` Gleb Natapov
2013-01-10 12:22             ` Zhang, Yang Z
2013-01-10 12:34               ` Gleb Natapov [this message]
2013-01-11  7:36                 ` Zhang, Yang Z
2013-01-11 16:54                   ` Gleb Natapov
2013-01-14  1:03                     ` Zhang, Yang Z
2013-01-14  1:14                     ` Zhang, Yang Z
2013-01-11  2:37         ` Zhang, Yang Z
2013-01-11 13:51           ` Gleb Natapov
2013-01-10  8:25   ` Gleb Natapov
2013-01-10  8:31     ` Zhang, Yang Z
2013-01-10  8:53       ` Gleb Natapov
2013-01-10  7:26 ` [PATCH v9 3/3] x86, apicv: add virtual interrupt delivery support Yang Zhang
2013-01-10  8:23   ` Gleb Natapov
2013-01-10 12:04     ` Zhang, Yang Z
2013-01-10 21:36   ` Marcelo Tosatti
2013-01-11 14:09     ` Gleb Natapov
2013-01-11 17:58       ` Marcelo Tosatti

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=20130110123453.GB11529@redhat.com \
    --to=gleb@redhat.com \
    --cc=haitao.shan@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox