Re: [PATCH v3 7/8] accel/tcg: Move PageDesc tree into tb-maint.c for system

[Richard Henderson <richard.henderson@linaro.org>]

On 12/9/22 01:22, Philippe Mathieu-Daudé wrote:
> On 9/12/22 06:19, Richard Henderson wrote:
>> Now that PageDesc is not used for user-only, and for system
>> it is only used for tb maintenance, move the implementation
>> into tb-main.c appropriately ifdefed.
>>
>> We have not yet eliminated all references to PageDesc for
>> user-only, so retain a typedef to the structure without definition.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   accel/tcg/internal.h      |  49 +++-----------
>>   accel/tcg/tb-maint.c      | 130 ++++++++++++++++++++++++++++++++++++--
>>   accel/tcg/translate-all.c |  95 ----------------------------
>>   3 files changed, 134 insertions(+), 140 deletions(-)
> 
> 
>> -/*
>> - * In system mode we want L1_MAP to be based on ram offsets,
>> - * while in user mode we want it to be based on virtual addresses.
>> - *
>> - * TODO: For user mode, see the caveat re host vs guest virtual
>> - * address spaces near GUEST_ADDR_MAX.
>> - */
>> -#if !defined(CONFIG_USER_ONLY)
>> -#if HOST_LONG_BITS < TARGET_PHYS_ADDR_SPACE_BITS
>> -# define L1_MAP_ADDR_SPACE_BITS  HOST_LONG_BITS
>> -#else
>> -# define L1_MAP_ADDR_SPACE_BITS  TARGET_PHYS_ADDR_SPACE_BITS
>> -#endif
>> -#else
>> -# define L1_MAP_ADDR_SPACE_BITS  MIN(HOST_LONG_BITS, TARGET_ABI_BITS)
>> -#endif
> 
> 
>> diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
>> index 20e86c813d..9b996bbeb2 100644
>> --- a/accel/tcg/tb-maint.c
>> +++ b/accel/tcg/tb-maint.c
>> @@ -127,6 +127,121 @@ static PageForEachNext foreach_tb_next(PageForEachNext tb,
>>   }
>>   #else
>> +/*
>> + * In system mode we want L1_MAP to be based on ram offsets.
>> + */
>> +#if HOST_LONG_BITS < TARGET_PHYS_ADDR_SPACE_BITS
>> +# define L1_MAP_ADDR_SPACE_BITS  HOST_LONG_BITS
>> +#else
>> +# define L1_MAP_ADDR_SPACE_BITS  TARGET_PHYS_ADDR_SPACE_BITS
>> +#endif
> So you removed L1_MAP_ADDR_SPACE_BITS in this patch. If you ever respin,
> I'd rather have it cleaned in the previous patch, along with the comment
> updated and TODO removed.

I don't agree.  I move all of the PageDesc symbols together in this patch.  I think that 
it would get in the way of the main point of the previous patch.


r~