From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60982) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuFKZ-0006b7-Nr for qemu-devel@nongnu.org; Thu, 05 Nov 2015 02:51:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZuFKU-0003eY-MF for qemu-devel@nongnu.org; Thu, 05 Nov 2015 02:51:35 -0500 Received: from mail-wm0-x22f.google.com ([2a00:1450:400c:c09::22f]:34914) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuFKU-0003eQ-GL for qemu-devel@nongnu.org; Thu, 05 Nov 2015 02:51:30 -0500 Received: by wmll128 with SMTP id l128so5386866wml.0 for ; Wed, 04 Nov 2015 23:51:29 -0800 (PST) Sender: Richard Henderson References: <1446103899-8644-1-git-send-email-guangrong.xiao@linux.intel.com> <5633D909.3030901@twiddle.net> <20151104193533.GK4180@thinpad.lan.raisama.net> From: Richard Henderson Message-ID: <563B0A7C.2000402@twiddle.net> Date: Thu, 5 Nov 2015 08:51:24 +0100 MIME-Version: 1.0 In-Reply-To: <20151104193533.GK4180@thinpad.lan.raisama.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] target-i386: enable cflushopt/clwb/pcommit instructions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: pbonzini@redhat.com, Xiao Guangrong , qemu-devel@nongnu.org, kvm@vger.kernel.org On 11/04/2015 08:35 PM, Eduardo Habkost wrote: > On Fri, Oct 30, 2015 at 01:54:33PM -0700, Richard Henderson wrote: >> On 10/29/2015 12:31 AM, Xiao Guangrong wrote: >>> These instructions are used by NVDIMM drivers and the specification >>> locates at: >>> https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf >>> >>> There instructions are available on Skylake Server >>> >>> Signed-off-by: Xiao Guangrong >>> --- >>> target-i386/cpu.c | 8 +++++--- >>> target-i386/cpu.h | 3 +++ >>> 2 files changed, 8 insertions(+), 3 deletions(-) >> >> Reviewed-by: Richard Henderson >> >> Although it would be nice to update the comments in translate.c to include the >> new insns, since they overlap mfence and sfence. At present we only check for >> SSE enabled when accepting these; I suppose it's easiest to consider it invalid >> to specify +clwb,-sse? > > I assume you want to add the extra SSE requirement to TCG code, not to > generic x86 code, then I have no objections. I don't really want to add any requirement, just point and laugh at anyone who reports an bug for the above condition. > But in the case of clwb (/6 with a memory operand, modrm != 0xc0), we > are not just requiring SSE2: we are rejecting the instruction unless > modrm == 0xc0. That means TCG is rejecting the clwb instruction, so I > believe we shouldn't add CLWB to TCG_7_0_EBX_FEATURES yet. Hmm, yes. I've cleaned up some of this code on a branch, but it didn't get enough testing or review this cycle, so it's going to wait for the next. I see you've posted a patch for this, which should be good enough until then. r~