Re: [Qemu-devel] [PATCH v4] Improve the alignment check infrastructure

[Richard Henderson <rth@twiddle.net>]

On 06/23/2016 12:18 PM, Richard Henderson wrote:
> On 06/23/2016 11:16 AM, Sergey Sorokin wrote:
>> +#if defined(CONFIG_SOFTMMU)
>> +/**
>> + * get_alignment_bits
>> + * @memop: TCGMemOp value
>> + *
>> + * Extract the alignment size from the memop.
>> + *
>> + * Returns: 0 in case of byte access (which is always aligned);
>> + *          positive value - number of alignment bits;
>> + *          negative value if unaligned access enabled
>> + *          and this is not a byte access.
>> + */
>> +static inline int get_alignment_bits(TCGMemOp memop)
>> +{
>> +    int a = memop & MO_AMASK;
>> +    int s = memop & MO_SIZE;
>> +
>> +    if (a == MO_UNALN) {
>> +        /* Negative value if unaligned access enabled,
>> +         * or zero value in case of byte access.
>> +         */
>> +        return -s;
>> +    } else if (a == MO_ALIGN) {
>> +        tcg_debug_assert((TLB_FLAGS_MASK & ((1 << s) - 1)) == 0);
>> +        /* A natural alignment: return a number of access size bits */
>> +        return s;
>> +    } else {
>> +        /* Specific alignment size. It must be equal or greater
>> +         * than the access size.
>> +         */
>> +        a >>= MO_ASHIFT;
>> +        tcg_debug_assert(a >= s);
>> +        tcg_debug_assert((TLB_FLAGS_MASK & ((1 << a) - 1)) == 0);
>> +        return a;
>> +    }
>> +}
>> +#endif  /* CONFIG_SOFTMMU */
>
> While it's true that usermode doesn't support alignment checks at all (either
> direction, I'd prefer to leave the function available and isolate the one
> assert that caused your build problem.  E.g.
>
> static inline int get_alignment_bits(TCGMemOp memop)
> {
>     int a = memop & MO_AMASK;
>     int s = memop & MO_SIZE;
>     int r;
>
>     ...
>     } else if (a == MO_ALIGN) {
>         /* A natural alignment: return a number of access size bits */
>         r = s;
>     } else {
>         /* Specific alignment size. It must be equal or greater
>          * than the access size.
>          */
>         r = a >> MO_ASHIFT;
>         tcg_debug_assert(r >= s);
>     }
> #ifdef CONFIG_SOFTMMU
>     /* Make sure requested alignment doesn't overlap TLB flags.  */
>     tcg_debug_assert((TLB_FLAGS_MASK & ((1 << r) - 1)) == 0);
> #endif
>     return r;
> }
>
>
> I'll give this a test with some target-sparc changes where this will be useful.

I've merged the patch with the above change for tcg-next.


r~