From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Han, Huaitong" <huaitong.han@intel.com>
Subject: Re: [PATCH V6 2/5] x86/hvm: pkeys,
 add pkeys support for guest_walk_tables
Date: Wed, 27 Jan 2016 03:18:47 +0000
Message-ID: <1453864735.4038.4.camel@intel.com>
References: <1453188659-8908-1-git-send-email-huaitong.han@intel.com>
	<1453188659-8908-3-git-send-email-huaitong.han@intel.com>
	<56A6515602000078000CABC4@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <56A6515602000078000CABC4@prv-mh.provo.novell.com>
Content-Language: en-US
Content-ID: <2F15B5A4D8901A4BA51B0D6C9E4E8FAB@intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: "JBeulich@suse.com" <JBeulich@suse.com>
Cc: "george.dunlap@eu.citrix.com" <george.dunlap@eu.citrix.com>, "andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>, "keir@xen.org" <keir@xen.org>, "wei.liu2@citrix.com" <wei.liu2@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On Mon, 2016-01-25 at 08:46 -0700, Jan Beulich wrote:

> > > > On 19.01.16 at 08:30, <huaitong.han@intel.com> wrote:
> 
> 
> > +    write_cr4(cr4 | X86_CR4_PKE);
> > +    asm volatile (".byte 0x0f,0x01,0xee"
> > +        : "=a" (pkru) : "c" (0) : "dx");
> > +    write_cr4(cr4);
> 
> I think you will want to abstract out the actual writing of CR4 from
> write_cr4(), as updating this_cpu(cr4) back and forth is quite
> pointless here.
> 
Updating this_cpu(cr4) back and forth is pointless, but using 
"asm volatile ( "mov %0,%%cr4" : : "r" (val) )" directly here is also
bad code style, as there are two places you can update cr4 directly.