Re: [PATCH] RISC-V: KVM: fixup undefined reference to riscv_cbom_block_size

[Andrew Jones <ajones@ventanamicro.com>]

On Mon, Oct 10, 2022 at 07:42:04AM +0100, Conor Dooley wrote:
> On Mon, Oct 10, 2022 at 09:33:29AM +0800, Vernon Yang wrote:
> > When some RISC-V compilers do not support the Zicbom extension,
> > the build system auto disable the CONFIG_RISCV_ISA_ZICBOM, so the
> > source code of the relevant function is not compiled, resulting
> > in the definition of the riscv_cbom_block_size variable cannot
> > be found
> 
> Hmm, my understanding was that riscv_cbom_block_size was not supposed to
> depend on CONFIG_RISCV_ISA_ZICBOM because the thead is able to use it
> even if the toolchain does not support it.
> 
> The code in cacheflush.h looks like:
> extern unsigned int riscv_cbom_block_size;
> #ifdef CONFIG_RISCV_ISA_ZICBOM
> void riscv_init_cbom_blocksize(void);
> #else
> static inline void riscv_init_cbom_blocksize(void) { }
> #endif
> 
> #ifdef CONFIG_RISCV_DMA_NONCOHERENT
> void riscv_noncoherent_supported(void);
> #endif
> 
> It's early and I only had a quick look but I think that this is not
> defined because RISCV_DMA_NONCOHERENT is not defined, not because of
> RISCV_ISA_ZICBOM.

thead is able to use riscv_cbom_block_size because it does its own
initialization of it and selects RISCV_DMA_NONCOHERENT to get access
to it. KVM depends on the initializer in dma-noncoherent.c, which is
guarded by RISCV_ISA_ZICBOM and does not select RISCV_DMA_NONCOHERENT,
but RISCV_ISA_ZICBOM does. I think guarding use of riscv_cbom_block_size
with RISCV_ISA_ZICBOM in KVM makes sense.

> I'm not the KVM maintainer, but I dislike #ifdefery
> in c files, so it'd be nice I think to sort this out in the header and
> not have to worry about guarding the variable.

I also dislike #ifdefery, but unless we move riscv_cbom_block_size to
an unconditionally built file like cacheflush.c (as Anup once did), then
we don't have much choice.

Thanks,
drew