Re: [Xen-devel] [PATCH v3 5/8] x86emul: support MOVDIR{I, 64B} insns

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.09.2019 12:19, Andrew Cooper wrote:
> On 03/09/2019 13:25, Jan Beulich wrote:
>> On 03.09.2019 12:28, Andrew Cooper wrote:
>>> On 03/09/2019 10:39, Jan Beulich wrote:
>>>> Note that SDM revision 070 doesn't specify exception behavior for
>>>> ModRM.mod != 0b11; assuming #UD here.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> What are we going to do about the ->write() hook atomicity?  I'm happy
>>> to put it on the TODO list, but we can't simply ignore the problem.
>> So do you not agree with my assessment that our memcpy()
>> implementation satisfies the need, and it's just not very
>> nice that the ->write() hook is dependent upon this?
> 
> We use __builtin_memcpy(), not necessarily our own local implementation.
> 
> Our own copy uses rep movsq followed by rep movsb of up to 7 bytes,
> which doesn't handle 2 and 4 byte copies atomically.  More generally,
> rep movs doesn't provide guarantees about the external visibility of
> component writes, owing to the many different ways that such writes may
> be implemented, and optimised.
> 
> Even if the above were fixed (and I'm not sure could make it correct
> even for misaligned writes), we cannot depend on our own memcpy never
> changing in the future.  For one, it should be a straight rep movsb on
> most Intel hardware these days, for performance.

Well, okay, I'll add a prepatch making HVM's ->write() hook not use
memcpy anymore for small aligned accesses. I guess I should also do
this for ->read() then, if it's not the case already.

However, as an implication this means that MOVDIR64B can't use the
->write() hook at all. We'd need to introduce a new hook, but to be
honest I have no good idea how to model it (i.e. what other
operations it might cover later on); possibly we want to come back
to this when deciding how to implement large memory block accessing
insns (FXSAVE/FXRSTOR etc).

Furthermore I don't think we currently have means to make the split
behavior of MOVDIRI correct: By using ->write(), we can't guarantee
it'll be exactly two writes.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel