[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86emul: fix SYSENTER/SYSCALL switching into 64-bit mode
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |