>>> On 02.02.18 at 14:41, <andrew.coop...@citrix.com> wrote: > On 07/12/17 14:11, Jan Beulich wrote: >> Signed-off-by: Jan Beulich <jbeul...@suse.com> >> --- >> v3: New. >> >> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >> @@ -5047,6 +5047,24 @@ x86_emulate( >> goto done; >> break; >> >> + case 0xf8: /* swapgs */ >> + generate_exception_if(!mode_64bit(), EXC_UD); >> + generate_exception_if(!mode_ring0(), EXC_GP, 0); >> + fail_if(!ops->read_segment || !ops->read_msr || >> + !ops->write_segment || !ops->write_msr); >> + if ( (rc = ops->read_segment(x86_seg_gs, &sreg, >> + ctxt)) != X86EMUL_OKAY || >> + (rc = ops->read_msr(MSR_SHADOW_GS_BASE, &msr_val, >> + ctxt)) != X86EMUL_OKAY || >> + (rc = ops->write_msr(MSR_SHADOW_GS_BASE, sreg.base, >> + ctxt)) != X86EMUL_OKAY ) > > We need to unwind this write in the case of write_segment failing, or > when the instruction restarts, state will be corrupt.
We don't do similar restoring anywhere else iirc, so I'm not sure I want to start doing so here. Multi-element updates really need to be converted to go through a staging layer, where the checks are done right away, but the commit happens only at the end. One of the reasons I decided against doing what you suggest (indeed I had considered that) is that this other write may then be failing, too. Let me know. Jan >> + goto done; >> + sreg.base = msr_val; >> + if ( (rc = ops->write_segment(x86_seg_gs, &sreg, >> + ctxt)) != X86EMUL_OKAY ) >> + goto done; >> + break; >> + >> case 0xf9: /* rdtscp */ >> fail_if(ops->read_msr == NULL); >> if ( (rc = ops->read_msr(MSR_TSC_AUX, >> >> >> _______________________________________________ Xen-devel mailing list Xenfirstname.lastname@example.org https://lists.xenproject.org/mailman/listinfo/xen-devel