Re: [PULL 2/3] target/loongarch: Fix tlb huge page loading issue

[gaosong]

在 2024/7/23 下午11:47, Peter Maydell 写道:

On Wed, 20 Mar 2024 at 02:40, Song Gao <gaos...@loongson.cn> wrote:

From: Xianglai Li <lixiang...@loongson.cn>

When we use qemu tcg simulation, the page size of bios is 4KB.
When using the level 2 super huge page (page size is 1G) to create the page 
table,
it is found that the content of the corresponding address space is abnormal,
resulting in the bios can not start the operating system and graphical 
interface normally.

The lddir and ldpte instruction emulation has
a problem with the use of super huge page processing above level 2.
The page size is not correctly calculated,
resulting in the wrong page size of the table entry found by tlb.

Signed-off-by: Xianglai Li <lixiang...@loongson.cn>
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
Signed-off-by: Song Gao <gaos...@loongson.cn>
Message-Id: <20240318070332.1273939-1-lixiang...@loongson.cn>

Hi; Coverity points out an issue with this patch (Coverity
CID 1547717):

@@ -485,7 +513,25 @@ target_ulong helper_lddir(CPULoongArchState *env, 
target_ulong base,
      target_ulong badvaddr, index, phys, ret;
      int shift;
      uint64_t dir_base, dir_width;
-    bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
+
+    if (unlikely((level == 0) || (level > 4))) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "Attepted LDDIR with level %"PRId64"\n", level);
+        return base;
+    }
+
+    if (FIELD_EX64(base, TLBENTRY, HUGE)) {
+        if (unlikely(level == 4)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "Attempted use of level 4 huge page\n");

Here we log level == 4 as being a guest error, but there is no
early "return <something>" the way the previous error-exit
codepath did above...

+        }
+
+        if (FIELD_EX64(base, TLBENTRY, LEVEL)) {
+            return base;
+        } else {
+            return FIELD_DP64(base, TLBENTRY, LEVEL, level);

...so Coverity complains that here we will try to put that value 4
into a field in the TLBENTRY that is only 2 bits wide.

+        }
+    }

Should the level == 4 if() do a "return base" like the
error cases for level == 0 or > 4 ?

Yes,  we should,  I will send a patch to fix it.

Thanks.
Song Gao