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

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

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;
     }


r~