2023-10-09 01:18:33

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] loongarch/mm: disable WUC for pgprot_writecombine() too

Hi, Zheng,

The title can be "LoongArch: Disable WUC for pgprot_writecombine() as
same as ioremap()"

On Sun, Oct 8, 2023 at 9:03 PM Icenowy Zheng <[email protected]> wrote:
>
> Currently the code disables WUC only disables it for ioremap_wc(), which
> is only used when mapping writecombine pages like ioremap() (mapped to
> the kernel space). For VRAM mapped in TTM/GEM, it's mapped with a
> crafted pgprot with pgprot_writecombine() function, which isn't
> corrently disabled now.
>
> Disable WUC for pgprot_writecombine() (fallback to SUC) too.
>
> This improves AMDGPU driver stability on Loongson 3A5000 machines.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> arch/loongarch/include/asm/io.h | 4 +---
> arch/loongarch/include/asm/pgtable-bits.h | 9 ++++++++-
> arch/loongarch/kernel/setup.c | 10 +++++-----
> 3 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/io.h b/arch/loongarch/include/asm/io.h
> index 0dcb36b32cb25..5477d9e3ab072 100644
> --- a/arch/loongarch/include/asm/io.h
> +++ b/arch/loongarch/include/asm/io.h
> @@ -52,10 +52,8 @@ static inline void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
> * @offset: bus address of the memory
> * @size: size of the resource to map
> */
> -extern pgprot_t pgprot_wc;
> -
> #define ioremap_wc(offset, size) \
> - ioremap_prot((offset), (size), pgprot_val(pgprot_wc))
> + ioremap_prot((offset), (size), pgprot_val(PAGE_KERNEL_WC))
>
> #define ioremap_cache(offset, size) \
> ioremap_prot((offset), (size), pgprot_val(PAGE_KERNEL))
> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
> index 35348d4c4209a..c9e3ed330fb97 100644
> --- a/arch/loongarch/include/asm/pgtable-bits.h
> +++ b/arch/loongarch/include/asm/pgtable-bits.h
> @@ -92,6 +92,13 @@
>
> #ifndef __ASSEMBLY__
>
> +extern bool wc_enabled;
> +
> +#define _CACHE_WC (wc_enabled ? _CACHE_WUC : _CACHE_SUC)
> +
> +#define PAGE_KERNEL_WC __pgprot(_PAGE_PRESENT | __READABLE | __WRITEABLE | \
> + _PAGE_GLOBAL | _PAGE_KERN | _CACHE_WC)
If these macros cannot be put close to their friends, I prefer to open code.

Huacai
> +
> #define _PAGE_IOREMAP pgprot_val(PAGE_KERNEL_SUC)
>
> #define pgprot_noncached pgprot_noncached
> @@ -111,7 +118,7 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> {
> unsigned long prot = pgprot_val(_prot);
>
> - prot = (prot & ~_CACHE_MASK) | _CACHE_WUC;
> + prot = (prot & ~_CACHE_MASK) | _CACHE_WC;
>
> return __pgprot(prot);
> }
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 4de32b07c0dcd..b35186f7b2547 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -161,19 +161,19 @@ static void __init smbios_parse(void)
> }
>
> #ifdef CONFIG_ARCH_WRITECOMBINE
> -pgprot_t pgprot_wc = PAGE_KERNEL_WUC;
> +bool wc_enabled = true;
> #else
> -pgprot_t pgprot_wc = PAGE_KERNEL_SUC;
> +bool wc_enabled;
> #endif
>
> -EXPORT_SYMBOL(pgprot_wc);
> +EXPORT_SYMBOL(wc_enabled);
>
> static int __init setup_writecombine(char *p)
> {
> if (!strcmp(p, "on"))
> - pgprot_wc = PAGE_KERNEL_WUC;
> + wc_enabled = true;
> else if (!strcmp(p, "off"))
> - pgprot_wc = PAGE_KERNEL_SUC;
> + wc_enabled = false;
> else
> pr_warn("Unknown writecombine setting \"%s\".\n", p);
>
> --
> 2.39.1
>
>