From: Palmer Dabbelt palmer@rivosinc.com
This fixes two issues: I truncated the warning's hart ID when porting to the 64-bit hart ID code, and the original code's warning handling could fire on an uninitialized hart ID.
The biggest change here is that riscv_cbom_block_size is no longer initialized, as IMO the default isn't sane: there's nothing in the ISA that mandates any specific cache block size, so falling back to one will just silently produce the wrong answer on some systems. This also changes the probing order so the cache block size is known before enabling Zicbom support.
CC: stable@vger.kernel.org CC: Andrew Jones ajones@ventanamicro.com CC: Heiko Stuebner heiko@sntech.de CC: Atish Patra atishp@rivosinc.com Fixes: 3aefb2ee5bdd ("riscv: implement Zicbom-based CMO instructions + the t-head variant") Fixes: 1631ba1259d6 ("riscv: Add support for non-coherent devices using zicbom extension") Reported-by: kernel test robot lkp@intel.com Reported-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Palmer Dabbelt palmer@rivosinc.com [Conor: fixed the redefinition errors] Tested-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com --- Hey Palmer, w/ LPC etc I figure it's highly unlikely you'd have this respun in time to have it applied this week. I dropped all the tested and reviewed -by tags since this patch has been changed a fair bit back and forth since the tags were left. Checked it on my D1 to make sure nothing blew up.. if this could make this weeks rc, that'd be great so that the clang allmodconfig builds are working again. Conor. --- arch/riscv/errata/thead/errata.c | 1 + arch/riscv/include/asm/cacheflush.h | 1 + arch/riscv/kernel/setup.c | 2 +- arch/riscv/mm/dma-noncoherent.c | 23 +++++++++++++---------- 4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c index 202c83f677b2..96648c176f37 100644 --- a/arch/riscv/errata/thead/errata.c +++ b/arch/riscv/errata/thead/errata.c @@ -37,6 +37,7 @@ static bool errata_probe_cmo(unsigned int stage, if (stage == RISCV_ALTERNATIVES_EARLY_BOOT) return false;
+ riscv_cbom_block_size = L1_CACHE_BYTES; riscv_noncoherent_supported(); return true; #else diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h index a60acaecfeda..a89c005b4bbf 100644 --- a/arch/riscv/include/asm/cacheflush.h +++ b/arch/riscv/include/asm/cacheflush.h @@ -43,6 +43,7 @@ void flush_icache_mm(struct mm_struct *mm, bool local); #endif /* CONFIG_SMP */
#ifdef CONFIG_RISCV_ISA_ZICBOM +extern unsigned int riscv_cbom_block_size; void riscv_init_cbom_blocksize(void); #else static inline void riscv_init_cbom_blocksize(void) { } diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 95ef6e2bf45c..2dfc463b86bb 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -296,8 +296,8 @@ void __init setup_arch(char **cmdline_p) setup_smp(); #endif
- riscv_fill_hwcap(); riscv_init_cbom_blocksize(); + riscv_fill_hwcap(); apply_boot_alternatives(); }
diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c index cd2225304c82..e3f9bdf47c5f 100644 --- a/arch/riscv/mm/dma-noncoherent.c +++ b/arch/riscv/mm/dma-noncoherent.c @@ -12,7 +12,7 @@ #include <linux/of_device.h> #include <asm/cacheflush.h>
-static unsigned int riscv_cbom_block_size = L1_CACHE_BYTES; +unsigned int riscv_cbom_block_size; static bool noncoherent_supported;
void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, @@ -79,38 +79,41 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, void riscv_init_cbom_blocksize(void) { struct device_node *node; + unsigned long cbom_hartid; + u32 val, probed_block_size; int ret; - u32 val;
+ probed_block_size = 0; for_each_of_cpu_node(node) { unsigned long hartid; - int cbom_hartid;
ret = riscv_of_processor_hartid(node, &hartid); if (ret) continue;
- if (hartid < 0) - continue; - /* set block-size for cbom extension if available */ ret = of_property_read_u32(node, "riscv,cbom-block-size", &val); if (ret) continue;
- if (!riscv_cbom_block_size) { - riscv_cbom_block_size = val; + if (!probed_block_size) { + probed_block_size = val; cbom_hartid = hartid; } else { - if (riscv_cbom_block_size != val) - pr_warn("cbom-block-size mismatched between harts %d and %lu\n", + if (probed_block_size != val) + pr_warn("cbom-block-size mismatched between harts %lu and %lu\n", cbom_hartid, hartid); } } + + if (probed_block_size) + riscv_cbom_block_size = probed_block_size; } #endif
void riscv_noncoherent_supported(void) { + WARN(!riscv_cbom_block_size, + "Non-coherent DMA support enabled without a block size\n"); noncoherent_supported = true; }
Am Dienstag, 13. September 2022, 00:48:01 CEST schrieb Conor Dooley:
From: Palmer Dabbelt palmer@rivosinc.com
This fixes two issues: I truncated the warning's hart ID when porting to the 64-bit hart ID code, and the original code's warning handling could fire on an uninitialized hart ID.
The biggest change here is that riscv_cbom_block_size is no longer initialized, as IMO the default isn't sane: there's nothing in the ISA that mandates any specific cache block size, so falling back to one will just silently produce the wrong answer on some systems. This also changes the probing order so the cache block size is known before enabling Zicbom support.
CC: stable@vger.kernel.org CC: Andrew Jones ajones@ventanamicro.com CC: Heiko Stuebner heiko@sntech.de CC: Atish Patra atishp@rivosinc.com Fixes: 3aefb2ee5bdd ("riscv: implement Zicbom-based CMO instructions + the t-head variant") Fixes: 1631ba1259d6 ("riscv: Add support for non-coherent devices using zicbom extension") Reported-by: kernel test robot lkp@intel.com Reported-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Palmer Dabbelt palmer@rivosinc.com [Conor: fixed the redefinition errors] Tested-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com
Reviewed-by: Heiko Stuebner heiko@sntech.de
On Mon, Sep 12, 2022 at 11:48:01PM +0100, Conor Dooley wrote:
From: Palmer Dabbelt palmer@rivosinc.com
This fixes two issues: I truncated the warning's hart ID when porting to the 64-bit hart ID code, and the original code's warning handling could fire on an uninitialized hart ID.
The biggest change here is that riscv_cbom_block_size is no longer initialized, as IMO the default isn't sane: there's nothing in the ISA that mandates any specific cache block size, so falling back to one will just silently produce the wrong answer on some systems. This also changes the probing order so the cache block size is known before enabling Zicbom support.
CC: stable@vger.kernel.org CC: Andrew Jones ajones@ventanamicro.com CC: Heiko Stuebner heiko@sntech.de CC: Atish Patra atishp@rivosinc.com Fixes: 3aefb2ee5bdd ("riscv: implement Zicbom-based CMO instructions + the t-head variant") Fixes: 1631ba1259d6 ("riscv: Add support for non-coherent devices using zicbom extension") Reported-by: kernel test robot lkp@intel.com Reported-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Palmer Dabbelt palmer@rivosinc.com [Conor: fixed the redefinition errors] Tested-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com
Build-tested-by: Nathan Chancellor nathan@kernel.org
Thanks a lot for continuing to chase this!
Hey Palmer, w/ LPC etc I figure it's highly unlikely you'd have this respun in time to have it applied this week. I dropped all the tested and reviewed -by tags since this patch has been changed a fair bit back and forth since the tags were left. Checked it on my D1 to make sure nothing blew up.. if this could make this weeks rc, that'd be great so that the clang allmodconfig builds are working again. Conor.
arch/riscv/errata/thead/errata.c | 1 + arch/riscv/include/asm/cacheflush.h | 1 + arch/riscv/kernel/setup.c | 2 +- arch/riscv/mm/dma-noncoherent.c | 23 +++++++++++++---------- 4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c index 202c83f677b2..96648c176f37 100644 --- a/arch/riscv/errata/thead/errata.c +++ b/arch/riscv/errata/thead/errata.c @@ -37,6 +37,7 @@ static bool errata_probe_cmo(unsigned int stage, if (stage == RISCV_ALTERNATIVES_EARLY_BOOT) return false;
- riscv_cbom_block_size = L1_CACHE_BYTES; riscv_noncoherent_supported(); return true;
#else diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h index a60acaecfeda..a89c005b4bbf 100644 --- a/arch/riscv/include/asm/cacheflush.h +++ b/arch/riscv/include/asm/cacheflush.h @@ -43,6 +43,7 @@ void flush_icache_mm(struct mm_struct *mm, bool local); #endif /* CONFIG_SMP */ #ifdef CONFIG_RISCV_ISA_ZICBOM +extern unsigned int riscv_cbom_block_size; void riscv_init_cbom_blocksize(void); #else static inline void riscv_init_cbom_blocksize(void) { } diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 95ef6e2bf45c..2dfc463b86bb 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -296,8 +296,8 @@ void __init setup_arch(char **cmdline_p) setup_smp(); #endif
- riscv_fill_hwcap(); riscv_init_cbom_blocksize();
- riscv_fill_hwcap(); apply_boot_alternatives();
} diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c index cd2225304c82..e3f9bdf47c5f 100644 --- a/arch/riscv/mm/dma-noncoherent.c +++ b/arch/riscv/mm/dma-noncoherent.c @@ -12,7 +12,7 @@ #include <linux/of_device.h> #include <asm/cacheflush.h> -static unsigned int riscv_cbom_block_size = L1_CACHE_BYTES; +unsigned int riscv_cbom_block_size; static bool noncoherent_supported; void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, @@ -79,38 +79,41 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, void riscv_init_cbom_blocksize(void) { struct device_node *node;
- unsigned long cbom_hartid;
- u32 val, probed_block_size; int ret;
- u32 val;
- probed_block_size = 0; for_each_of_cpu_node(node) { unsigned long hartid;
int cbom_hartid;ret = riscv_of_processor_hartid(node, &hartid); if (ret) continue;
if (hartid < 0)continue;- /* set block-size for cbom extension if available */ ret = of_property_read_u32(node, "riscv,cbom-block-size", &val); if (ret) continue;
if (!riscv_cbom_block_size) {riscv_cbom_block_size = val;
if (!probed_block_size) {probed_block_size = val; cbom_hartid = hartid;
if (riscv_cbom_block_size != val)pr_warn("cbom-block-size mismatched between harts %d and %lu\n",
if (probed_block_size != val)pr_warn("cbom-block-size mismatched between harts %lu and %lu\n", cbom_hartid, hartid);- if (probed_block_size)
riscv_cbom_block_size = probed_block_size;} #endif void riscv_noncoherent_supported(void) {
- WARN(!riscv_cbom_block_size,
"Non-coherent DMA support enabled without a block size\n");}
2.37.1
On Tue, 13 Sep 2022 02:40:17 PDT (-0700), nathan@kernel.org wrote:
On Mon, Sep 12, 2022 at 11:48:01PM +0100, Conor Dooley wrote:
From: Palmer Dabbelt palmer@rivosinc.com
This fixes two issues: I truncated the warning's hart ID when porting to the 64-bit hart ID code, and the original code's warning handling could fire on an uninitialized hart ID.
The biggest change here is that riscv_cbom_block_size is no longer initialized, as IMO the default isn't sane: there's nothing in the ISA that mandates any specific cache block size, so falling back to one will just silently produce the wrong answer on some systems. This also changes the probing order so the cache block size is known before enabling Zicbom support.
CC: stable@vger.kernel.org CC: Andrew Jones ajones@ventanamicro.com CC: Heiko Stuebner heiko@sntech.de CC: Atish Patra atishp@rivosinc.com Fixes: 3aefb2ee5bdd ("riscv: implement Zicbom-based CMO instructions + the t-head variant") Fixes: 1631ba1259d6 ("riscv: Add support for non-coherent devices using zicbom extension") Reported-by: kernel test robot lkp@intel.com Reported-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Palmer Dabbelt palmer@rivosinc.com [Conor: fixed the redefinition errors] Tested-by: Conor Dooley conor.dooley@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com
Build-tested-by: Nathan Chancellor nathan@kernel.org
Thanks a lot for continuing to chase this!
Ya, thanks. I was actually opening my laptop to try and find the build errors for the one I just sent, but this is way easier. It's on fixes.
Hey Palmer, w/ LPC etc I figure it's highly unlikely you'd have this respun in time to have it applied this week. I dropped all the tested and reviewed -by tags since this patch has been changed a fair bit back and forth since the tags were left. Checked it on my D1 to make sure nothing blew up.. if this could make this weeks rc, that'd be great so that the clang allmodconfig builds are working again. Conor.
arch/riscv/errata/thead/errata.c | 1 + arch/riscv/include/asm/cacheflush.h | 1 + arch/riscv/kernel/setup.c | 2 +- arch/riscv/mm/dma-noncoherent.c | 23 +++++++++++++---------- 4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c index 202c83f677b2..96648c176f37 100644 --- a/arch/riscv/errata/thead/errata.c +++ b/arch/riscv/errata/thead/errata.c @@ -37,6 +37,7 @@ static bool errata_probe_cmo(unsigned int stage, if (stage == RISCV_ALTERNATIVES_EARLY_BOOT) return false;
- riscv_cbom_block_size = L1_CACHE_BYTES; riscv_noncoherent_supported(); return true;
#else diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h index a60acaecfeda..a89c005b4bbf 100644 --- a/arch/riscv/include/asm/cacheflush.h +++ b/arch/riscv/include/asm/cacheflush.h @@ -43,6 +43,7 @@ void flush_icache_mm(struct mm_struct *mm, bool local); #endif /* CONFIG_SMP */
#ifdef CONFIG_RISCV_ISA_ZICBOM +extern unsigned int riscv_cbom_block_size; void riscv_init_cbom_blocksize(void); #else static inline void riscv_init_cbom_blocksize(void) { } diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 95ef6e2bf45c..2dfc463b86bb 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -296,8 +296,8 @@ void __init setup_arch(char **cmdline_p) setup_smp(); #endif
- riscv_fill_hwcap(); riscv_init_cbom_blocksize();
- riscv_fill_hwcap(); apply_boot_alternatives();
}
diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c index cd2225304c82..e3f9bdf47c5f 100644 --- a/arch/riscv/mm/dma-noncoherent.c +++ b/arch/riscv/mm/dma-noncoherent.c @@ -12,7 +12,7 @@ #include <linux/of_device.h> #include <asm/cacheflush.h>
-static unsigned int riscv_cbom_block_size = L1_CACHE_BYTES; +unsigned int riscv_cbom_block_size; static bool noncoherent_supported;
void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, @@ -79,38 +79,41 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, void riscv_init_cbom_blocksize(void) { struct device_node *node;
- unsigned long cbom_hartid;
- u32 val, probed_block_size; int ret;
- u32 val;
- probed_block_size = 0; for_each_of_cpu_node(node) { unsigned long hartid;
int cbom_hartid;ret = riscv_of_processor_hartid(node, &hartid); if (ret) continue;
if (hartid < 0)continue;/* set block-size for cbom extension if available */ ret = of_property_read_u32(node, "riscv,cbom-block-size", &val); if (ret) continue;
if (!riscv_cbom_block_size) {riscv_cbom_block_size = val;
if (!probed_block_size) {probed_block_size = val; cbom_hartid = hartid;
if (riscv_cbom_block_size != val)pr_warn("cbom-block-size mismatched between harts %d and %lu\n",
if (probed_block_size != val)pr_warn("cbom-block-size mismatched between harts %lu and %lu\n", cbom_hartid, hartid);- if (probed_block_size)
riscv_cbom_block_size = probed_block_size;} #endif
void riscv_noncoherent_supported(void) {
- WARN(!riscv_cbom_block_size,
"Non-coherent DMA support enabled without a block size\n");}
2.37.1
linux-stable-mirror@lists.linaro.org
-
Conor Dooley -
Heiko Stuebner -
Nathan Chancellor -
Palmer Dabbelt