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

[Peter Maydell]

On Wed, 20 Mar 2024 at 02:40, Song Gao <gaosong@loongson.cn> wrote:
>
> From: Xianglai Li <lixianglai@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 <lixianglai@loongson.cn>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> Message-Id: <20240318070332.1273939-1-lixianglai@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 ?

thanks
-- PMM