Re: [PATCH v9 2/3] x86, apicv: add virtual x2apic support

[Gleb Natapov <gleb@redhat.com>]

On Fri, Jan 11, 2013 at 07:36:44AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-01-10:
> > 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.
> As I said, read/write TPR will not cause vmexit if virtual x2apic is set.
> 
>From my reading of the spec you need to disable 808H intercept for that.
Is this not the case?

> >>>> 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.
> Yes, you are right. It too complicated.
> Another question? Why not to hide x2apic capability to guest when vid is supported and virtual x2apic mode is not supported? It should be more reasonable than disable vid when virtual x2apic mode is unavailable.
> 
Because I think there will be no such HW. If such HW expected to be
common then we can go this route.

--
			Gleb.