[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86emul: fix SYSENTER/SYSCALL switching into 64-bit mode


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 10 Feb 2021 14:02:39 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=EFHZcH+jQ9Eve3do2WdbBwrkpQEDzESAD3lY3yYCHQs=; b=DXlO5xzweYvQQYo5ZeBfftU6SFwxRjKQhzZX/DTV/f4K2PPPOECbbwhJGSk9+S0+MHrODq6q6zXN8k1VH+62yssTyorJHfHJMHPcf2SaErazFxreeS994bj5PozZ107nl+mTgCjYEIOu0q+AM7e/2jBjQTOoP1fESiiTFKwQxNUinhyLvLmnb3x9egojv9SWLqiNnbU/H2IhpmrQfFDW0i5aEbLANwILDJ8UtucsrYai1AypJcj9drww4Qf5QVa1XUEPD3OLnrcCeRcqsFM6vDmgPqO02HtESbeyN/EbzaYuQbRkPGFIowrWMFRhAmB5BptvvT9ovO9bNSi0TxnyZg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=l4Ij01F3MSSL0Ae8LKMI8e6XWowYh/tIDGh66N2HhPR6PSpIkhMqWFiNa4hQDm2pMZqcosuI3oM5p4cwiekcBjTOE5sQikonVnhTHosTdem9GlxRf2u13+LaH+IqZHkngS26INGfueKC498S9NSBPBMrx2xZKD+gOz8EPgieTpNVSf+tSIXj6LY21i8acK8FT+SKY9TITa8z4TwkS+ux1JjpufRPzhSNSOWYjdu9D+V5kGgqfskZYd9IU7bfdtTezaieQ3kGYuO/GCjOrf6d9l2ILtgPb0Rkynu/y0K0xgDaYFbxoqa99zL2gYWDzOe+BCaxg2JP/aNT/GGpAd0VVw==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 10 Feb 2021 14:02:57 +0000
  • Ironport-sdr: 8dmyrFxXGNxlH/wNBL1bpBb/eLntvt9ttSoFrrEtx/RDOZpy969OA/VhcUdnw46Nkrso7lyDv2 kGX2g+v9QkF033I1yUBoCT3dtTF5eJtl8m4ldu4GLQU8H58oqhjYfy/r4DcdXYXAQArDXJ5ZQI IgzcVM9pHFXn7WIfBV2B0F56yQbY3s0Y9Kpm+0j07s27uPz3knOyW/L9VUwYEgzCKibBZhuRw0 w68HjvK1nCK9caZGtI5mv7U/btgJpLX0gxS0YFjQwF6hvieAXVRxanLBbVJtb8HdSI7MYIJgk2 rjg=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <jbeulich@xxxxxxxx>
>>> ---
>>> 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 <andrew.cooper3@xxxxxxxxxxx>

> 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.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.