Re: [PATCH 2/4] target/i386/emulate: stop overloading decode->op[N].ptr

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 5/5/25 12:00, Paolo Bonzini wrote:
> On 5/5/25 11:43, Philippe Mathieu-Daudé wrote:
>> Hi Paolo,
>>
>> On 2/5/25 23:48, Paolo Bonzini wrote:
>>> decode->op[N].ptr can contain either a host pointer (!) in CPUState
>>> or a guest virtual address.  Pass the whole struct to read_val_ext
>>> and write_val_ext, so that it can decide the contents based on the
>>> operand type.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>   target/i386/emulate/x86_decode.h |   9 ++-
>>>   target/i386/emulate/x86_emu.h    |   8 +--
>>>   target/i386/emulate/x86_decode.c |  74 +++++++++----------
>>>   target/i386/emulate/x86_emu.c    | 119 ++++++++++++++++---------------
>>>   4 files changed, 109 insertions(+), 101 deletions(-)
>>>
>>> diff --git a/target/i386/emulate/x86_decode.h b/target/i386/emulate/ 
>>> x86_decode.h
>>> index 87cc728598d..497cbdef9c7 100644
>>> --- a/target/i386/emulate/x86_decode.h
>>> +++ b/target/i386/emulate/x86_decode.h
>>> @@ -266,7 +266,10 @@ typedef struct x86_decode_op {
>>>       int reg;
>>>       target_ulong val;
>>> -    target_ulong ptr;
>>> +    union {
>>> +        target_ulong addr;
>>
>> Prefer 'vaddr' type for "guest virtual address".
> That would be a semantic change which I really want to avoid in this 
> series.
> 
> I don't think target_long/target_ulong is a big blocker towards single- 
> binary anyway.  The trick is to confine it to target/, making it 
> essentially a #define.  That is, let target/* include one of two new 
> headers target_long_32.h and target_long_64.h.  See lore.kernel.org/ 
> r/68b6c799-6407-43cc-aebc-a0ef6b8b64fa@redhat.com as well.

Yes, I have this tagged to understand and address. Maybe Pierrick
already understood the issue (similar mention from Richard? [1]) and
is addressing it, see [2].

[1] 
https://lore.kernel.org/qemu-devel/5b152664-a752-4be8-aa15-8c71c040b026@linaro.org/
[2] 
https://lore.kernel.org/qemu-devel/20250505015223.3895275-15-pierrick.bouvier@linaro.org/

> 
> For the same reason I think target_long_bits() is a step in the wrong 
> direction.