Re: [PATCH for-11.0] tcg: Pass host-endian values to plugin_gen_mem_callbacks_*

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 3/24/26 9:59 PM, Richard Henderson wrote:
> On 3/25/26 11:39, Pierrick Bouvier wrote:
>> Hi Richard,
>>
>> On 3/24/26 5:40 PM, Richard Henderson wrote:
>>> If the host does not support swapped-endian loads and stores,
>>> then we emulate those within the tcg expanders with explicit
>>> bswap operations.
>>>
>>> However, we were passing values to the plugin interface in
>>> the middle of those bswap operations, which meant that we
>>> would pass values of the wrong endianness to plugins when
>>> running on hosts without swapped-endian loads and stores.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3351
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>
>>> Hi Pierrick,
>>>
>>> On IRC I expressed the opinion that there was an additional
>>> big-endian bug with how we treat neg.plugin_mem_value_low,
>>> but I now see that isn't true, because of how we adjust the
>>> store address in plugin_gen_mem_callbacks_i32.
>>>
>>
>> Yes, we store the value directly swapped, so there is no further transformation needed.
>
> It's not about storing the value swapped, it's about storing the i32 to the low part of
> the uint64_t for the host endianness.
>

I finally realized the orig_value we had is already in host-endian 
order, while I was assuming it was in target-endian order. Thus the 
confusion in our conversion. All good now.

>> It works, but it is still not clear to me why the value orig_val (which is not swapped and
>> in target-endian order) gets stored swapped correctly in neg.plugin_mem_value_low.
> 
> Again, it is *not* swapped: orig_val is host-endian order, and so is neg.plugin_mem_value_low.
> 
>> I'll probably add another patch to document that better in plugin api, and remove the
>> additional swapping done in tests/tcg/plugins/mem.c.
> 
> What swapping is that?
>

It tracks memory regions we accessed, so which byte was written matters 
in this case, thus the need to swap to target-endian order.

> There's a variable called "swapped_value", but it appears to be mis-named, or a remnant of
> swapping that used to occur.
>

It's basically the value in target-endian order. FROM_BE should to TO_BE 
as well, even though both macros do exactly the same (FROM or TO acting 
as documentation to express the intent).

> 
> r~