Re: [PATCH v4 01/57] include/exec/memop: Add bits describing atomicity

[Peter Maydell]

On Wed, 3 May 2023 at 08:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> These bits may be used to describe the precise atomicity
> requirements of the guest, which may then be used to
> constrain the methods by which it may be emulated by the host.
>
> For instance, the AArch64 LDP (32-bit) instruction changes
> semantics with ARMv8.4 LSE2, from
>
>   MO_64 | MO_ATMAX_4 | MO_ATOM_IFALIGN
>   (64-bits, single-copy atomic only on 4 byte units,
>    nonatomic if not aligned by 4),
>
> to
>
>   MO_64 | MO_ATMAX_SIZE | MO_ATOM_WITHIN16
>   (64-bits, single-copy atomic within a 16 byte block)
>
> The former may be implemented with two 4 byte loads, or
> a single 8 byte load if that happens to be efficient on
> the host.  The latter may not, and may also require a
> helper when misaligned.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/memop.h | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/include/exec/memop.h b/include/exec/memop.h
> index 25d027434a..04e4048f0b 100644
> --- a/include/exec/memop.h
> +++ b/include/exec/memop.h
> @@ -81,6 +81,42 @@ typedef enum MemOp {
>      MO_ALIGN_32 = 5 << MO_ASHIFT,
>      MO_ALIGN_64 = 6 << MO_ASHIFT,
>
> +    /*
> +     * MO_ATOM_* describes that atomicity requirements of the operation:

"the atomicity requirements"

> +     * MO_ATOM_IFALIGN: the operation must be single-copy atomic if and
> +     *    only if it is aligned; if unaligned there is no atomicity.

Is this really "and only if", ie "must *not* be single-copy-atomic if
non aligned"? Plain old "if" seems more likely...

> +     * MO_ATOM_NONE: the operation has no atomicity requirements.
> +     * MO_ATOM_SUBALIGN: the operation is single-copy atomic by parts
> +     *    by the alignment.  E.g. if the address is 0 mod 4, then each
> +     *    4-byte subobject is single-copy atomic.
> +     *    This is the atomicity of IBM Power and S390X processors.
> +     * MO_ATOM_WITHIN16: the operation is single-copy atomic, even if it
> +     *    is unaligned, so long as it does not cross a 16-byte boundary;
> +     *    if it crosses a 16-byte boundary there is no atomicity.
> +     *    This is the atomicity of Arm FEAT_LSE2.
> +     *
> +     * MO_ATMAX_* describes the maximum atomicity unit required:
> +     * MO_ATMAX_SIZE: the entire operation, i.e. MO_SIZE.
> +     * MO_ATMAX_[248]: units of N bytes.
> +     *
> +     * Note the default (i.e. 0) values are single-copy atomic to the
> +     * size of the operation, if aligned.  This retains the behaviour
> +     * from before these were introduced.
> +     */
> +    MO_ATOM_SHIFT    = 8,
> +    MO_ATOM_MASK     = 0x3 << MO_ATOM_SHIFT,
> +    MO_ATOM_IFALIGN  = 0 << MO_ATOM_SHIFT,
> +    MO_ATOM_NONE     = 1 << MO_ATOM_SHIFT,
> +    MO_ATOM_SUBALIGN = 2 << MO_ATOM_SHIFT,
> +    MO_ATOM_WITHIN16 = 3 << MO_ATOM_SHIFT,
> +
> +    MO_ATMAX_SHIFT = 10,
> +    MO_ATMAX_MASK  = 0x3 << MO_ATMAX_SHIFT,
> +    MO_ATMAX_SIZE  = 0 << MO_ATMAX_SHIFT,
> +    MO_ATMAX_2     = 1 << MO_ATMAX_SHIFT,
> +    MO_ATMAX_4     = 2 << MO_ATMAX_SHIFT,
> +    MO_ATMAX_8     = 3 << MO_ATMAX_SHIFT,
> +

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM