Re: [Qemu-devel] [PATCH v2 16/16] target-s390x: fix MVC instruction when areas overlap

[Richard Henderson <rth@twiddle.net>]

On 06/03/2015 02:09 PM, Aurelien Jarno wrote:
> The MVC instruction and the memmove C funtion do not have the same
> semantic when memory areas overlap:
>
> MVC: When the operands overlap, the result is obtained as if the
> operands were processed one byte at a time and each result byte were
> stored immediately after fetching the necessary operand byte.
>
> memmove: Copying takes place as though the bytes in src are first copied
> into a temporary array that does not overlap src or dest, and the bytes
> are then copied from the temporary array to dest.
>
> The behaviour is therefore the same when the destination is at a lower
> address than the source, but not in the other case. This is actually a
> trick for propagating a value to an area. While the current code detects
> that and call memset in that case, it only does for 1-byte value. This
> trick can and is used for propagating two or more bytes to an area.
>
> In the softmmu case, the call to mvc_fast_memmove is correct as the
> above tests verify that source and destination are each within a page,
> and both in a different page. The part doing the move 8 bytes by 8 bytes
> is wrong and we need to check that if the source and destination
> overlap, they do with a distance of minimum 8 bytes before copying 8
> bytes at a time.
>
> In the user code, we should check check that the destination is at a
> lower address than source or than the end of the source is at a lower
> address than the destination before calling memmove. In the opposite
> case we fallback to the same code as the softmmu one. Note that l
> represents (length - 1).
>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>   target-s390x/mem_helper.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~