Re: [RFC v2 3/5] cputlb: Introduce TLB_BSWAP

[Richard Henderson]

On 9/19/19 8:09 AM, David Hildenbrand wrote:
> On 18.09.19 20:02, Richard Henderson wrote:
>> Handle bswap on ram directly in load/store_helper.  This fixes a
>> bug with the previous implementation in that one cannot use the
>> I/O path for RAM.
>>
>> Fixes: a26fc6f5152b47f1
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  include/exec/cpu-all.h |   2 +
>>  accel/tcg/cputlb.c     | 105 +++++++++++++++++++++--------------------
>>  2 files changed, 57 insertions(+), 50 deletions(-)
>>
>> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
>> index d2d443c4f9..3928edab9a 100644
>> --- a/include/exec/cpu-all.h
>> +++ b/include/exec/cpu-all.h
>> @@ -331,6 +331,8 @@ CPUArchState *cpu_copy(CPUArchState *env);
>>  #define TLB_MMIO            (1 << (TARGET_PAGE_BITS - 3))
>>  /* Set if TLB entry contains a watchpoint.  */
>>  #define TLB_WATCHPOINT      (1 << (TARGET_PAGE_BITS - 4))
>> +/* Set if TLB entry requires byte swap.  */
>> +#define TLB_BSWAP           (1 << (TARGET_PAGE_BITS - 5))
> 
> Shouldn't you also take care of TLB_BSWAP in probe_access?

Yes, I should.  I guess like TLB_MMIO and return NULL.  Thanks.

>> +        if (unlikely(tlb_addr & TLB_BSWAP)) {
>> +            return direct_swap(haddr);
>> +        } else {
>> +            return direct(haddr);
>> +        }
> 
> You can drop the else statement and just return.

Sure.

> I assume this patch does not yet result in the issues you are
> experiencing, right?

Correct.  It is only the last patch here that has problems.


r~