From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752839AbdAZFyy (ORCPT ); Thu, 26 Jan 2017 00:54:54 -0500 Received: from mga09.intel.com ([134.134.136.24]:54590 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751598AbdAZFyw (ORCPT ); Thu, 26 Jan 2017 00:54:52 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,287,1477983600"; d="scan'208";a="52534133" Message-ID: <1485410090.41148.50.camel@ranerica-desktop> Subject: Re: [v3 PATCH 07/10] x86: Add emulation code for UMIP instructions From: Ricardo Neri To: "H. Peter Anvin" Cc: Ingo Molnar , Thomas Gleixner , Andy Lutomirski , Borislav Petkov , Peter Zijlstra , Andrew Morton , Brian Gerst , Chris Metcalf , Dave Hansen , Paolo Bonzini , Liang Z Li , Masami Hiramatsu , Huang Rui , Jiri Slaby , Jonathan Corbet , "Michael S. Tsirkin" , Paul Gortmaker , Vlastimil Babka , Chen Yucong , Alexandre Julliard , Fenghua Yu , Stas Sergeev , "Ravi V. Shankar" , Shuah Khan , linux-kernel@vger.kernel.org, x86@kernel.org, linux-msdos@vger.kernel.org, wine-devel@winehq.org, Tony Luck Date: Wed, 25 Jan 2017 21:54:50 -0800 In-Reply-To: <145900b0-b386-e32c-cd79-cf7c47c14616@zytor.com> References: <20170125202353.101059-1-ricardo.neri-calderon@linux.intel.com> <20170125202353.101059-8-ricardo.neri-calderon@linux.intel.com> <145900b0-b386-e32c-cd79-cf7c47c14616@zytor.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-01-25 at 12:38 -0800, H. Peter Anvin wrote: > On 01/25/17 12:23, Ricardo Neri wrote: > > + case UMIP_SMSW: > > + dummy_value = CR0_STATE; > > Unless the user space process is running in 64-bit mode this value > should be & 0xffff. But wouldn't that prevent the bits CR0[63:16] or CR0[31:16] from being copied when a register operand is used? According to the Intel Software Development manual, SMSW returns SMSW r16 operand size 16, store CR0[15:0] in r16 SMSW r32 operand size 32, zero-extend CR0[31:0], and store in r32 SMSW r64 operand size 64, zero-extend CR0[63:0], and store in r64 The number of bytes returned by the emulated results is controlled by the data_size variable. If it finds that the result will be saved in a memory location, it will only copy CR0[15:0], which is the expected behavior of SMSW if the result is to be copied in memory. > I'm not sure if we should even support fixing up > UMIP instructions in 64-bit mode. Probably not. The whole point of the emulation was to support virtual-8086 mode and 32-bit mode. > > Also, please put an explicit /* fall through */ here. Will do. > > > + /* > > + * These two instructions return a 16-bit value. We return > > + * all zeros. This is equivalent to a null descriptor for > > + * str and sldt. > > + */ > > + case UMIP_SLDT: > > + case UMIP_STR: > > + /* if operand is a register, it is zero-extended*/ > > + if (X86_MODRM_MOD(insn->modrm.value) == 3) { > > + memset(data, 0, insn->opnd_bytes); > > + *data_size = insn->opnd_bytes; > > + /* if not, only the two least significant bytes are copied */ > > + } else { > > + *data_size = 2; > > + } > > + memcpy(data, &dummy_value, sizeof(dummy_value)); > > + break; The code above controls how many bytes are copied as the result of SMSW. > > + default: > > + return -EINVAL; > > + } > > + return 0; > > > > +bool fixup_umip_exception(struct pt_regs *regs) > > +{ > > + struct insn insn; > > + unsigned char buf[MAX_INSN_SIZE]; > > + /* 10 bytes is the maximum size of the result of UMIP instructions */ > > + unsigned char dummy_data[10] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; > > +#ifdef CONFIG_X86_64 > > + int x86_64 = user_64bit_mode(regs); > > +#else > > + int x86_64 = 0; > > +#endif > > Again, could we simply do: > > if (user_64bit_mode(regs)) > return false; > > or are there known users of these instructions *in 64-bit mode*? I am not aware of any. In that case, I will make this code return in this case. Thanks and BR, Ricardo > > -hpa > >