From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.5 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A994C433E0 for ; Wed, 10 Feb 2021 14:19:14 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 270C164D9E for ; Wed, 10 Feb 2021 14:19:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 270C164D9E Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.83620.156054 (Exim 4.92) (envelope-from ) id 1l9qKa-00028k-W2; Wed, 10 Feb 2021 14:19:00 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 83620.156054; Wed, 10 Feb 2021 14:19:00 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l9qKa-00028d-Sa; Wed, 10 Feb 2021 14:19:00 +0000 Received: by outflank-mailman (input) for mailman id 83620; Wed, 10 Feb 2021 14:19:00 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1l9qKa-00028Y-A9 for xen-devel@lists.xenproject.org; Wed, 10 Feb 2021 14:19:00 +0000 Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id ba50ed03-d14e-4779-846b-a9a9a5f456e2; Wed, 10 Feb 2021 14:18:59 +0000 (UTC) Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 8BFDFAC97; Wed, 10 Feb 2021 14:18:58 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: ba50ed03-d14e-4779-846b-a9a9a5f456e2 X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1612966738; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5W6aM15Fq4rJcOiUrhfLnfzkJvWfXae7DM4qOUMMC1A=; b=fqYiqRvijDI/AmSr2xK79l0iindQ0hAb9RfszroOmsU1M9Mxb+WXFAzjE11JNsihL7FAaJ tQyiE1nOQW8xB2M5BgKeoOr+Us+A06cUW7ABoje7v73vOvQ5MARqOYmTya4h86rUqJiIvY lzUe53yEZNrLuZ3BaOjyYIu3GRGnNXc= Subject: Re: [PATCH] x86emul: fix SYSENTER/SYSCALL switching into 64-bit mode To: Andrew Cooper Cc: Wei Liu , =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= , "xen-devel@lists.xenproject.org" References: <7ce15e4b-8bf1-0cfd-ca9e-5f6eba12cac1@suse.com> <2eed5630-3e23-3005-245e-989893fc8476@suse.com> From: Jan Beulich Message-ID: <77fda392-6f6a-c8b0-f1ea-15b917245f5e@suse.com> Date: Wed, 10 Feb 2021 15:18:58 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 10.02.2021 15:02, Andrew Cooper wrote: > On 10/02/2021 13:54, Jan Beulich wrote: >> On 10.02.2021 13:28, Andrew Cooper wrote: >>> On 10/02/2021 09:57, Jan Beulich wrote: >>>> When invoked by compat mode, mode_64bit() will be false at the start of >>>> emulation. The logic after complete_insn, however, needs to consider the >>>> mode switched into, in particular to avoid truncating RIP. >>>> >>>> Inspired by / paralleling and extending Linux commit 943dea8af21b ("KVM: >>>> x86: Update emulator context mode if SYSENTER xfers to 64-bit mode"). >>>> >>>> While there, tighten a related assertion in x86_emulate_wrapper() - we >>>> want to be sure to not switch into an impossible mode when the code gets >>>> built for 32-bit only (as is possible for the test harness). >>>> >>>> Signed-off-by: Jan Beulich >>>> --- >>>> In principle we could drop SYSENTER's ctxt->lma dependency when setting >>>> _regs.r(ip). I wasn't certain whether leaving it as is serves as kind of >>>> documentation ... >>>> >>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >>>> @@ -6127,6 +6127,10 @@ x86_emulate( >>>> (rc = ops->write_segment(x86_seg_ss, &sreg, ctxt)) ) >>>> goto done; >>>> >>>> + if ( ctxt->lma ) >>>> + /* In particular mode_64bit() needs to return true from here on. */ >>>> + ctxt->addr_size = ctxt->sp_size = 64; >>> I think this is fine as presented, but don't we want the logical >>> opposite for SYSRET/SYSEXIT ? >>> >>> We truncate rip suitably already, >> This is why I left them alone, i.e. ... >> >>> but don't know what other checks may appear in the future. >> ... I thought we would deal with this if and when such checks >> would appear. > > Ok.  Reviewed-by: Andrew Cooper Thanks. >> Just like considered in the post-description >> remark, we could drop the conditional part from sysexit's >> setting of _regs.r(ip), and _then_ we would indeed need a >> respective change there, for the truncation to happen at >> complete_insn:. > > I think it would look odd changing just rip and not rsp truncation. Yes, this was another consideration of mine as well. But it is a fact that we treat rip and rsp differently in this regard. Perhaps generated code overall could benefit from treating rsp more like rip, but this would need careful looking at all the involved pieces - especially in cases where the updated stack pointer gets further used we may not be able to defer the truncation to complete_insn:. Jan