Re: [Xen-devel] [PATCH v3 11/12] fuzz/x86_emulate: Set and fuzz more CPU state

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.10.17 at 18:52, <george.dunlap@xxxxxxxxxx> wrote:
> On 10/11/2017 10:31 AM, Jan Beulich wrote:
>>>>> On 10.10.17 at 18:20, <george.dunlap@xxxxxxxxxx> wrote:
>>> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
>>> @@ -40,6 +40,8 @@ struct fuzz_state
>>>      uint64_t msr[MSR_INDEX_MAX];
>>>      struct segment_register segments[SEG_NUM];
>>>      struct cpu_user_regs regs;
>>> +    char fxsave[512] __attribute__((aligned(16)));
>>> +
>>>  
>>>      /* Fuzzer's input data. */
>> 
>> No double blank lines please.
> 
> <smacks head>

Please don't.

>>> +        if ( write )
>>> +        {
>>> +            char null[512] __attribute__((aligned(16))) = { };
>>> +            
>>> +            fxs->mxcsr &= mxcsr_mask;
>>> +
>>> +            asm volatile( "fxrstor %0" :: "m" (*null) );
>>> +            asm volatile( "fxrstor %0" :: "m" (*fxs) );
>> 
>> Without a comment I still don't really understand what good this
>> double load is intended to do. In particular I don't think there are
>> any state components that the first may alter but the second
>> won't. Quite the opposite, on AMD I think you may end up not
>> altering FIP/FDP/FOP despite this double load (iirc they're being
>> loaded only when an exception is indicated to be pending in the
>> status word; see fpu_fxrstor() in the hypervisor).
> 
> As I said, somewhere online I think I read that doing an fxrstor with a
> certain part of the state as all zeros would "reset" the fpu.  But I
> don't see that in the manual, so it's probably wrong (or at least not
> architectural); in which case I should just remove the first fxrstor.
> 
> But you haven't given me clear direction about what I should do instead.
>  Should I attempt to copy the functionality of fpu_fxrstor() somehow?

Well, what (if anything) to do about this depends on what you
want to achieve. Random input to FXRSTOR isn't going to be a
problem if you don't expect said fields to never change across
emulation. FPU insns may result in the fields changing in any
event. If the fields appearing to have changed without any FPU
insn having been emulated is not a problem, not doing anything
may be good enough. Otherwise cloning some of the logic from
fpu_fxrstor() may be necessary, but to give more precise advice
I'd prefer to know some more detail on what unreliable state
transition is seen with what insn sequence.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel