From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH v11 2/3] x86, apicv: add virtual x2apic support
Date: Mon, 21 Jan 2013 19:21:13 -0200
Message-ID: <20130121212113.GC7110@amt.cnet>
References: <1358331672-32384-1-git-send-email-yang.z.zhang@intel.com>
 <1358331672-32384-3-git-send-email-yang.z.zhang@intel.com>
 <20130121195907.GA3561@amt.cnet>
 <20130121202114.GE25818@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Yang Zhang <yang.z.zhang@intel.com>, kvm@vger.kernel.org,
	haitao.shan@intel.com, xiantao.zhang@intel.com,
	Kevin Tian <kevin.tian@intel.com>
To: Gleb Natapov <gleb@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:65107 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751569Ab3AUWPp (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 21 Jan 2013 17:15:45 -0500
Content-Disposition: inline
In-Reply-To: <20130121202114.GE25818@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Jan 21, 2013 at 10:21:14PM +0200, Gleb Natapov wrote:
> > >  	}
> > > +
> > > +	vcpu->arch.apic_base = value;
> > 
> > Simpler to have
> > 
> > if (apic_x2apic_mode(apic)) {
> > 	...
> > 	kvm_x86_ops->set_virtual_x2apic_mode(vcpu, true);
> > } else {
> > 	kvm_x86_ops->set_virtual_x2apic_mode(vcpu, false);
> > }
> > 
> This will not work during cpu init. That was discussed on one of
> the previous iterations of the patch. When this code is called during
> vcpu init vmcs is not loaded yet so set_virtual_x2apic_mode() cannot
> write into it.

Are you saying that the logic to write on bit value change is due to 
ordering with cpu init or that the callback is at the wrong place?

> > 
> > Why not disable write intercept for all MSRs which represent APIC registers
> > that are virtualized? Why TPR is special?
> > 
> This patch goes before vid is enabled. At this point only TPR is
> vitalized. If APIC_WRITE exit will be generated on unhandled MSR write
> then we can disable intercept for all x2apic MSRs here.

-ENOPARSE, please be more verbose. "unhandled MSR write" ?