>>> On 12.01.17 at 16:04, <andrew.coop...@citrix.com> wrote: > On 12/01/17 14:02, Jan Beulich wrote: >> Furthermore I think we have another issue with writes: If the write >> faults, the FSW (or MXCSR, albeit there only for instructions we don't >> emulate yet) register may have been updated already, so we'd need to >> undo that update. > > Do you mean restore the value before we sample it, or before the guest > gets to see it?
Read it, run the stub, call ->write(), and upon failure restore the value read in the first step. > (I can't see what the problem is here.) The stub execution may modify FSW/MXCSR, if the operation causes an exception to be latched (for MXCSR this would need to be a masked exception), but if ->write() fails architecturally the update to FSW/MXCSR should not be committed. >> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >> @@ -3723,6 +3735,8 @@ x86_emulate( >> default: >> generate_exception(EXC_UD); >> } >> + if ( dst.type == OP_MEM && dst.bytes == 4 && !fpu_check_write() >> ) >> + dst.type = OP_NONE; > > This dst.bytes check is rather suspicious, as the size of the operand > has nothing to do with whether the write should be surpressed. > > I presume you actually mean (modrm_reg & 7) < 6 to exclude fnstenv and > fnstcw from triggering the fpu_check_write() logic? I had it this way first, and then thought it's better the way it is now: The cases we want to exclude are the non-register-data stores, and in both groups all register stores are respectively uniform in size. Plus this way the conditional is slightly shorter (i.e. doesn't require splitting across lines). Yet if you strongly prefer the other variant, I can of course switch back. Just let me know. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
