* Re: [PATCH] RISC-V: KVM: fixup undefined reference to riscv_cbom_block_size
[not found] ` <20221010013329.199167-1-vernon2gm@gmail.com>
@ 2022-10-10 6:42 ` Conor Dooley
2022-10-10 6:59 ` Andrew Jones
0 siblings, 1 reply; 3+ messages in thread
From: Conor Dooley @ 2022-10-10 6:42 UTC (permalink / raw)
To: Vernon Yang; +Cc: lkp, ajones, anup, atishp, kbuild-all, linux-mm, linux-riscv
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. 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.
That's my 0.02 €..
Thanks,
Conor.
> So add conditional compilation to fix it
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Vernon Yang <vernon2gm@gmail.com>
> ---
> arch/riscv/kvm/vcpu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index a032c4f0d600..08a6c3cb695d 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -256,7 +256,7 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
> unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK |
> KVM_REG_SIZE_MASK |
> KVM_REG_RISCV_CONFIG);
> - unsigned long reg_val;
> + unsigned long reg_val = 0;
>
> if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
> return -EINVAL;
> @@ -268,7 +268,9 @@ static int kvm_riscv_vcpu_get_reg_config(struct kvm_vcpu *vcpu,
> case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
> if (!riscv_isa_extension_available(vcpu->arch.isa, ZICBOM))
> return -EINVAL;
> +#ifdef CONFIG_RISCV_ISA_ZICBOM
> reg_val = riscv_cbom_block_size;
> +#endif
> break;
> default:
> return -EINVAL;
> --
> 2.25.1
>
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] RISC-V: KVM: fixup undefined reference to riscv_cbom_block_size
2022-10-10 6:42 ` [PATCH] RISC-V: KVM: fixup undefined reference to riscv_cbom_block_size Conor Dooley
@ 2022-10-10 6:59 ` Andrew Jones
2022-10-10 7:05 ` Conor Dooley
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Jones @ 2022-10-10 6:59 UTC (permalink / raw)
To: Conor Dooley
Cc: Vernon Yang, lkp, anup, atishp, kbuild-all, linux-mm, linux-riscv
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] RISC-V: KVM: fixup undefined reference to riscv_cbom_block_size
2022-10-10 6:59 ` Andrew Jones
@ 2022-10-10 7:05 ` Conor Dooley
0 siblings, 0 replies; 3+ messages in thread
From: Conor Dooley @ 2022-10-10 7:05 UTC (permalink / raw)
To: Andrew Jones
Cc: Vernon Yang, lkp, anup, atishp, kbuild-all, linux-mm, linux-riscv
On Mon, Oct 10, 2022 at 08:59:49AM +0200, Andrew Jones wrote:
> 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.
Aight chief, you know the code better than I do :)
> > 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.
Fair enough. Maybe once fixes, for-next & some of Heiko's cleanups have
aligned it'll make sense to do such a change.
Sorry for the noise then,
Conor.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-10 7:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <202210091222.xuZquaM9-lkp@intel.com>
[not found] ` <20221010013329.199167-1-vernon2gm@gmail.com>
2022-10-10 6:42 ` [PATCH] RISC-V: KVM: fixup undefined reference to riscv_cbom_block_size Conor Dooley
2022-10-10 6:59 ` Andrew Jones
2022-10-10 7:05 ` Conor Dooley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).