Re: [PATCH v2 06/17] linux-user: Do not use guest_addr_valid for h2g_valid

[Richard Henderson]

On 6/25/20 9:34 AM, Peter Maydell wrote:
> On Fri, 5 Jun 2020 at 05:17, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> This is the only use of guest_addr_valid that does not begin
>> with a guest address, but a host address being transformed to
>> a guest address.
>>
>> We will shortly adjust guest_addr_valid to handle guest memory
>> tags, and the host address should not be subjected to that.
>>
>> Move h2g_valid adjacent to the other h2g macros.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/exec/cpu_ldst.h | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
>> index c14a48f65e..3930362e20 100644
>> --- a/include/exec/cpu_ldst.h
>> +++ b/include/exec/cpu_ldst.h
>> @@ -77,15 +77,16 @@ typedef uint64_t abi_ptr;
>>  #else
>>  #define guest_addr_valid(x) ((x) <= GUEST_ADDR_MAX)
>>  #endif
>> -#define h2g_valid(x) guest_addr_valid((unsigned long)(x) - guest_base)
>>
>>  static inline int guest_range_valid(unsigned long start, unsigned long len)
>>  {
>>      return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
>>  }
>>
>> +#define h2g_valid(x)  ((uintptr_t)(x) - guest_base <= GUEST_ADDR_MAX)
> 
> The old implementation returns true for
> HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
> (because there's a different definition of guest_addr_valid() there)
> but this one does a range check even in that case.

It's part and parcel with patch 1, wherein we are in fact attempting to limit
the guest address space to GUEST_ADDR_MAX.

That's why I put patch 1 first, so the behaviour change happens there.

>>  #define h2g_nocheck(x) ({ \
>> -    unsigned long __ret = (unsigned long)(x) - guest_base; \
>> +    uintptr_t __ret = (uintptr_t)(x) - guest_base; \
>>      (abi_ptr)__ret; \
>>  })
> 
> Why the type change? This seems unrelated.

Dropped.  Though at some point we should purge unsigned long, as there is
always a clearer type to use.


r~