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

[Paolo Bonzini <pbonzini@redhat.com>]

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.

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

Paolo