Re: [PATCH V4 1/1] target/loongarch: Fixed tlb huge page loading issue

[lixianglai]

在 2024/3/16 上午1:06, Richard Henderson 写道:
> On 3/14/24 23:01, lixianglai wrote:
>> Hi Richard :
>>
>>> On 3/14/24 20:22, lixianglai wrote:
>>>> Hi Richard:
>>>>> On 3/13/24 15:33, Xianglai Li wrote:
>>>>>> +    if (unlikely((level == 0) || (level > 4))) {
>>>>>> +        return base;
>>>>>> +    }
>>> ...
>>>>> Perhaps it would be worthwhile to add another for the level==0 or > 4 
>>>>> case above?
>>>>>
>>>> A normal level 4 page table should not print an error log,
>>>>
>>>> only if a level 4 page is large, so we should put it in
>>>>
>>>>      if (FIELD_EX64(base, TLBENTRY, HUGE)) {
>>>>          if (unlikely(level == 4)) {
>>>>              qemu_log_mask(LOG_GUEST_ERROR,
>>>>                            "Attempted use of level %lu huge page\n", 
>>>> level);
>>>>          }
>>>>
>>>>          if (FIELD_EX64(base, TLBENTRY, LEVEL)) {
>>>>              return base;
>>>>          } else {
>>>>              return  FIELD_DP64(base, TLBENTRY, LEVEL, level);
>>>>          }
>>>>      }
>>>
>>> A level 5 page table is not normal, nor is a level 0 lddir.
>>>
>>
>> We communicate with the hardware guys that the behavior above level 4 and 
>> lddir 0 is undefined behavior.
>>
>> The result of our test on 3A5000 is that it has no any effect on "base",
>>
>> however in future chips the behavior may change since it may support 5-level 
>> page table and width for level[13:14] may change also.
>>
>>
>> So in this context,I am not sure which level to use to print logs,
>>
>> which content to print, and where to add these prints,
>>
>> any more detailed advice?
>
> Yes, right there in the IF that I quoted at the top.
> What I was trying to spell out is
>
>     if (unlikely(level == 0 || level > 4)) {
>         qemu_log_mask(LOG_GUEST_ERROR,
>                       "Attepted LDDIR with level %"PRId64"\n", level);
>         return base;
>     }
>
Thank you very much, I will modify it in V6 version of patch.

Thanks,
Xianglai.
>
> r~