From mboxrd@z Thu Jan 1 00:00:00 1970 From: Haozhong Zhang Subject: Re: [PATCH 1/4] x86/hvm: allow guest to use clflushopt and clwb Date: Wed, 30 Dec 2015 10:16:48 +0800 Message-ID: <20151230021648.GC16809@hz-desktop.sh.intel.com> References: <1451388711-18646-1-git-send-email-haozhong.zhang@intel.com> <1451388711-18646-2-git-send-email-haozhong.zhang@intel.com> <5682AAE6.4070908@citrix.com> <20151230013553.GB16809@hz-desktop.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20151230013553.GB16809@hz-desktop.sh.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , xen-devel@lists.xen.org, Keir Fraser , Jan Beulich , Ian Jackson , Stefano Stabellini , Ian Campbell , Wei Liu , Jun Nakajima , Kevin Tian List-Id: xen-devel@lists.xenproject.org On 12/30/15 09:35, Haozhong Zhang wrote: > On 12/29/15 15:46, Andrew Cooper wrote: > > On 29/12/2015 11:31, Haozhong Zhang wrote: > > >Pass CPU features CLFLUSHOPT and CLWB into HVM domain so that those two > > >instructions can be used by guest. > > > > > >The specification of above two instructions can be found in > > >https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf > > > > > >Signed-off-by: Haozhong Zhang > > > > Please be aware that my cpuid rework series completely changes all of this > > code. As this patch is small and self contained, it would be best to get > > it accepted early and for me to rebase over the result. > > > > I'll split this patch series into two parts and put these two > instruction enabling patches in the first part. > > > As part of my cpuid work, I had come to the conclusion that CLFLUSHOPT, CLWB > > and PCOMMIT were all safe for all guests to use, as they deemed safe for > > cpl3 code to use. Is there any reason why these wouldn't be safe for PV > > guests to use? > > > > Not for safety concern. These three instructions are usually used with > NVDIMM which are only implemented for HVM domains in this patch > series, so I didn't enable them for PV. I think they can be enabled > for PV later by another patch. > > > >--- > > > tools/libxc/xc_cpufeature.h | 3 ++- > > > tools/libxc/xc_cpuid_x86.c | 4 +++- > > > xen/arch/x86/hvm/hvm.c | 7 +++++++ > > > xen/include/asm-x86/cpufeature.h | 5 +++++ > > > 4 files changed, 17 insertions(+), 2 deletions(-) > > > > > >diff --git a/tools/libxc/xc_cpufeature.h b/tools/libxc/xc_cpufeature.h > > >index c3ddc80..5288ac6 100644 > > >--- a/tools/libxc/xc_cpufeature.h > > >+++ b/tools/libxc/xc_cpufeature.h > > >@@ -140,6 +140,7 @@ > > > #define X86_FEATURE_RDSEED 18 /* RDSEED instruction */ > > > #define X86_FEATURE_ADX 19 /* ADCX, ADOX instructions */ > > > #define X86_FEATURE_SMAP 20 /* Supervisor Mode Access Protection */ > > >- > > >+#define X86_FEATURE_CLFLUSHOPT 23 /* CLFLUSHOPT instruction */ > > >+#define X86_FEATURE_CLWB 24 /* CLWB instruction */ > > > #endif /* __LIBXC_CPUFEATURE_H */ > > >diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c > > >index 8882c01..fecfd6c 100644 > > >--- a/tools/libxc/xc_cpuid_x86.c > > >+++ b/tools/libxc/xc_cpuid_x86.c > > >@@ -426,7 +426,9 @@ static void xc_cpuid_hvm_policy(xc_interface *xch, > > > bitmaskof(X86_FEATURE_RDSEED) | > > > bitmaskof(X86_FEATURE_ADX) | > > > bitmaskof(X86_FEATURE_SMAP) | > > >- bitmaskof(X86_FEATURE_FSGSBASE)); > > >+ bitmaskof(X86_FEATURE_FSGSBASE) | > > >+ bitmaskof(X86_FEATURE_CLWB) | > > >+ bitmaskof(X86_FEATURE_CLFLUSHOPT)); > > > } else > > > regs[1] = 0; > > > regs[0] = regs[2] = regs[3] = 0; > > > > The entry for CLFLUSHOPT in the ISA Extension manual (August 2015) talks > > about CPUID.7(ECX=1).EBX[8:15] indicating the cache line size affected by > > the instruction. However, I can't find any other reference to this > > information, nor an extension of the CPUID instruction in the ISA manual. > > Should the Xen cpuid handling code be updated not to clobber this? > > > > Yes, I missed this part and will update in the next version. > I double-checked the manual and it says that "The aligned cache line size affected is also indicated with the CPUID instruction (bits 8 through 15 of the EBX register when the initial value in the EAX register is 1)" so I guess you really meant CPUID.1.EBX[8:15]. The 0x00000001 case branch in xc_cpuid_hvm_policy() (and its callers) has already passed the host CPUID.1.EBX[8:15] to HVM domains, so no more action is needed in this patch. Haozhong