diff mbox series

[kvmtool] Fixes: 0febaae00bb6 ("Add asm/kernel.h for riscv")

Message ID 20220520180946.104214-1-daolu@rivosinc.com (mailing list archive)
State New, archived
Headers show
Series [kvmtool] Fixes: 0febaae00bb6 ("Add asm/kernel.h for riscv") | expand

Commit Message

Dao Lu May 20, 2022, 6:09 p.m. UTC 
Signed-off-by: Dao Lu <daolu@rivosinc.com>
---
 riscv/include/asm/kernel.h | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 riscv/include/asm/kernel.h

Comments

Alexandru Elisei May 23, 2022, 9:31 a.m. UTC | #1 
Hi,

When I started working on the heterogeneous PMU series, support for the
riscv architecture wasn't merged in kvmtool, and after riscv was merged I
missed adding the header file.

This indeed fixes this compilation error:

In file included from include/linux/rbtree.h:32,
                 from include/kvm/devices.h:4,
                 from include/kvm/pci.h:10,
                 from include/kvm/vfio.h:6,
                 from include/kvm/kvm-config.h:5,
                 from include/kvm/kvm.h:6:
include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file or directory
    5 | #include "asm/kernel.h"
      |          ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
compilation terminated.
make: *** [Makefile:484: builtin-balloon.o] Error 1

Would be nice to include it in the commit message, so people googling for
that exact error message can come across this commit.

On Fri, May 20, 2022 at 11:09:46AM -0700, Dao Lu wrote:
> Signed-off-by: Dao Lu <daolu@rivosinc.com>
> ---
>  riscv/include/asm/kernel.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>  create mode 100644 riscv/include/asm/kernel.h
> 
> diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
> new file mode 100644
> index 0000000..a2a8d9e
> --- /dev/null
> +++ b/riscv/include/asm/kernel.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __ASM_KERNEL_H
> +#define __ASM_KERNEL_H
> +
> +#define NR_CPUS	4096

In arch/riscv/Kconfig I see this:

config NR_CPUS
	int "Maximum number of CPUs (2-32)"
	range 2 32
	depends on SMP
	default "8"

Would you mind explaining where the 4096 number of CPUs comes from?

Thanks,
Alex

> +
> +#endif /* __ASM_KERNEL_H */
> -- 
> 2.36.0
>
Alexandru Elisei May 23, 2022, 9:34 a.m. UTC | #2 
Adding the kvmtool maintainers, I just noticed that they were missing.

On Mon, May 23, 2022 at 10:31:34AM +0100, Alexandru Elisei wrote:
> Hi,
> 
> When I started working on the heterogeneous PMU series, support for the
> riscv architecture wasn't merged in kvmtool, and after riscv was merged I
> missed adding the header file.
> 
> This indeed fixes this compilation error:
> 
> In file included from include/linux/rbtree.h:32,
>                  from include/kvm/devices.h:4,
>                  from include/kvm/pci.h:10,
>                  from include/kvm/vfio.h:6,
>                  from include/kvm/kvm-config.h:5,
>                  from include/kvm/kvm.h:6:
> include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file or directory
>     5 | #include "asm/kernel.h"
>       |          ^~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> compilation terminated.
> make: *** [Makefile:484: builtin-balloon.o] Error 1
> 
> Would be nice to include it in the commit message, so people googling for
> that exact error message can come across this commit.
> 
> On Fri, May 20, 2022 at 11:09:46AM -0700, Dao Lu wrote:
> > Signed-off-by: Dao Lu <daolu@rivosinc.com>
> > ---
> >  riscv/include/asm/kernel.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >  create mode 100644 riscv/include/asm/kernel.h
> > 
> > diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
> > new file mode 100644
> > index 0000000..a2a8d9e
> > --- /dev/null
> > +++ b/riscv/include/asm/kernel.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __ASM_KERNEL_H
> > +#define __ASM_KERNEL_H
> > +
> > +#define NR_CPUS	4096
> 
> In arch/riscv/Kconfig I see this:
> 
> config NR_CPUS
> 	int "Maximum number of CPUs (2-32)"
> 	range 2 32
> 	depends on SMP
> 	default "8"
> 
> Would you mind explaining where the 4096 number of CPUs comes from?
> 
> Thanks,
> Alex
> 
> > +
> > +#endif /* __ASM_KERNEL_H */
> > -- 
> > 2.36.0
> >
Dao Lu May 23, 2022, 4:04 p.m. UTC | #3 
Hi Alex,

Thanks for pointing that out - I wasn't sure where the number came
from so I basically copied from the arm one just so the compilation
can pass.

I am happy to fix up the number to 32 and add the compile error
message to the commit message like you said - would something like
this work?
-------
Fixes the following compilation issue:

include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file
or directory
    5 | #include "asm/kernel.h"
-------
Thanks,
Dao

On Mon, May 23, 2022 at 2:33 AM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Adding the kvmtool maintainers, I just noticed that they were missing.
>
> On Mon, May 23, 2022 at 10:31:34AM +0100, Alexandru Elisei wrote:
> > Hi,
> >
> > When I started working on the heterogeneous PMU series, support for the
> > riscv architecture wasn't merged in kvmtool, and after riscv was merged I
> > missed adding the header file.
> >
> > This indeed fixes this compilation error:
> >
> > In file included from include/linux/rbtree.h:32,
> >                  from include/kvm/devices.h:4,
> >                  from include/kvm/pci.h:10,
> >                  from include/kvm/vfio.h:6,
> >                  from include/kvm/kvm-config.h:5,
> >                  from include/kvm/kvm.h:6:
> > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file or directory
> >     5 | #include "asm/kernel.h"
> >       |          ^~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> > compilation terminated.
> > make: *** [Makefile:484: builtin-balloon.o] Error 1
> >
> > Would be nice to include it in the commit message, so people googling for
> > that exact error message can come across this commit.
> >
> > On Fri, May 20, 2022 at 11:09:46AM -0700, Dao Lu wrote:
> > > Signed-off-by: Dao Lu <daolu@rivosinc.com>
> > > ---
> > >  riscv/include/asm/kernel.h | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >  create mode 100644 riscv/include/asm/kernel.h
> > >
> > > diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
> > > new file mode 100644
> > > index 0000000..a2a8d9e
> > > --- /dev/null
> > > +++ b/riscv/include/asm/kernel.h
> > > @@ -0,0 +1,8 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +#ifndef __ASM_KERNEL_H
> > > +#define __ASM_KERNEL_H
> > > +
> > > +#define NR_CPUS    4096
> >
> > In arch/riscv/Kconfig I see this:
> >
> > config NR_CPUS
> >       int "Maximum number of CPUs (2-32)"
> >       range 2 32
> >       depends on SMP
> >       default "8"
> >
> > Would you mind explaining where the 4096 number of CPUs comes from?
> >
> > Thanks,
> > Alex
> >
> > > +
> > > +#endif /* __ASM_KERNEL_H */
> > > --
> > > 2.36.0
> > >
Alexandru Elisei May 23, 2022, 4:28 p.m. UTC | #4 
Hi,

On Mon, May 23, 2022 at 09:04:04AM -0700, Dao Lu wrote:
> Hi Alex,
> 
> Thanks for pointing that out - I wasn't sure where the number came
> from so I basically copied from the arm one just so the compilation
> can pass.

I see, I was worried that I was looking in the wrong place.

> 
> I am happy to fix up the number to 32 and add the compile error
> message to the commit message like you said - would something like
> this work?
> -------
> Fixes the following compilation issue:
> 
> include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file
> or directory
>     5 | #include "asm/kernel.h"
> -------

Sounds good, thanks:
Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>

With the error message added:
Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex

> Thanks,
> Dao
> 
> On Mon, May 23, 2022 at 2:33 AM Alexandru Elisei
> <alexandru.elisei@arm.com> wrote:
> >
> > Adding the kvmtool maintainers, I just noticed that they were missing.
> >
> > On Mon, May 23, 2022 at 10:31:34AM +0100, Alexandru Elisei wrote:
> > > Hi,
> > >
> > > When I started working on the heterogeneous PMU series, support for the
> > > riscv architecture wasn't merged in kvmtool, and after riscv was merged I
> > > missed adding the header file.
> > >
> > > This indeed fixes this compilation error:
> > >
> > > In file included from include/linux/rbtree.h:32,
> > >                  from include/kvm/devices.h:4,
> > >                  from include/kvm/pci.h:10,
> > >                  from include/kvm/vfio.h:6,
> > >                  from include/kvm/kvm-config.h:5,
> > >                  from include/kvm/kvm.h:6:
> > > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file or directory
> > >     5 | #include "asm/kernel.h"
> > >       |          ^~~~~~~~~~~~~~
> > > cc1: all warnings being treated as errors
> > > compilation terminated.
> > > make: *** [Makefile:484: builtin-balloon.o] Error 1
> > >
> > > Would be nice to include it in the commit message, so people googling for
> > > that exact error message can come across this commit.
> > >
> > > On Fri, May 20, 2022 at 11:09:46AM -0700, Dao Lu wrote:
> > > > Signed-off-by: Dao Lu <daolu@rivosinc.com>
> > > > ---
> > > >  riscv/include/asm/kernel.h | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >  create mode 100644 riscv/include/asm/kernel.h
> > > >
> > > > diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
> > > > new file mode 100644
> > > > index 0000000..a2a8d9e
> > > > --- /dev/null
> > > > +++ b/riscv/include/asm/kernel.h
> > > > @@ -0,0 +1,8 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +
> > > > +#ifndef __ASM_KERNEL_H
> > > > +#define __ASM_KERNEL_H
> > > > +
> > > > +#define NR_CPUS    4096
> > >
> > > In arch/riscv/Kconfig I see this:
> > >
> > > config NR_CPUS
> > >       int "Maximum number of CPUs (2-32)"
> > >       range 2 32
> > >       depends on SMP
> > >       default "8"
> > >
> > > Would you mind explaining where the 4096 number of CPUs comes from?
> > >
> > > Thanks,
> > > Alex
> > >
> > > > +
> > > > +#endif /* __ASM_KERNEL_H */
> > > > --
> > > > 2.36.0
> > > >
Dao Lu May 23, 2022, 7:15 p.m. UTC | #5 
Hi Alex,

After talking with my colleague I have some additional questions about
what number we want to put there, as of now there is already a patch
that will increase the range in Kconfig to 2-512:

https://lore.kernel.org/lkml/CAOnJCUJrN4frY_OdQzO-yr5CrDLvj=ge9KY2d=XnGvAF-uQNvQ@mail.gmail.com/T/

It seems like a moving target and as riscv develops we kinda expect
this number will grow further. Do you think it is ok for me to at
least set it to 512, if not 4096 at this time?

Thanks,
Dao

On Mon, May 23, 2022 at 9:27 AM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> On Mon, May 23, 2022 at 09:04:04AM -0700, Dao Lu wrote:
> > Hi Alex,
> >
> > Thanks for pointing that out - I wasn't sure where the number came
> > from so I basically copied from the arm one just so the compilation
> > can pass.
>
> I see, I was worried that I was looking in the wrong place.
>
> >
> > I am happy to fix up the number to 32 and add the compile error
> > message to the commit message like you said - would something like
> > this work?
> > -------
> > Fixes the following compilation issue:
> >
> > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file
> > or directory
> >     5 | #include "asm/kernel.h"
> > -------
>
> Sounds good, thanks:
> Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
>
> With the error message added:
> Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
>
> Thanks,
> Alex
>
> > Thanks,
> > Dao
> >
> > On Mon, May 23, 2022 at 2:33 AM Alexandru Elisei
> > <alexandru.elisei@arm.com> wrote:
> > >
> > > Adding the kvmtool maintainers, I just noticed that they were missing.
> > >
> > > On Mon, May 23, 2022 at 10:31:34AM +0100, Alexandru Elisei wrote:
> > > > Hi,
> > > >
> > > > When I started working on the heterogeneous PMU series, support for the
> > > > riscv architecture wasn't merged in kvmtool, and after riscv was merged I
> > > > missed adding the header file.
> > > >
> > > > This indeed fixes this compilation error:
> > > >
> > > > In file included from include/linux/rbtree.h:32,
> > > >                  from include/kvm/devices.h:4,
> > > >                  from include/kvm/pci.h:10,
> > > >                  from include/kvm/vfio.h:6,
> > > >                  from include/kvm/kvm-config.h:5,
> > > >                  from include/kvm/kvm.h:6:
> > > > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file or directory
> > > >     5 | #include "asm/kernel.h"
> > > >       |          ^~~~~~~~~~~~~~
> > > > cc1: all warnings being treated as errors
> > > > compilation terminated.
> > > > make: *** [Makefile:484: builtin-balloon.o] Error 1
> > > >
> > > > Would be nice to include it in the commit message, so people googling for
> > > > that exact error message can come across this commit.
> > > >
> > > > On Fri, May 20, 2022 at 11:09:46AM -0700, Dao Lu wrote:
> > > > > Signed-off-by: Dao Lu <daolu@rivosinc.com>
> > > > > ---
> > > > >  riscv/include/asm/kernel.h | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > >  create mode 100644 riscv/include/asm/kernel.h
> > > > >
> > > > > diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
> > > > > new file mode 100644
> > > > > index 0000000..a2a8d9e
> > > > > --- /dev/null
> > > > > +++ b/riscv/include/asm/kernel.h
> > > > > @@ -0,0 +1,8 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > +
> > > > > +#ifndef __ASM_KERNEL_H
> > > > > +#define __ASM_KERNEL_H
> > > > > +
> > > > > +#define NR_CPUS    4096
> > > >
> > > > In arch/riscv/Kconfig I see this:
> > > >
> > > > config NR_CPUS
> > > >       int "Maximum number of CPUs (2-32)"
> > > >       range 2 32
> > > >       depends on SMP
> > > >       default "8"
> > > >
> > > > Would you mind explaining where the 4096 number of CPUs comes from?
> > > >
> > > > Thanks,
> > > > Alex
> > > >
> > > > > +
> > > > > +#endif /* __ASM_KERNEL_H */
> > > > > --
> > > > > 2.36.0
> > > > >
Alexandru Elisei May 24, 2022, 8:11 a.m. UTC | #6 
Hi,

On Mon, May 23, 2022 at 12:15:55PM -0700, Dao Lu wrote:
> Hi Alex,
> 
> After talking with my colleague I have some additional questions about
> what number we want to put there, as of now there is already a patch
> that will increase the range in Kconfig to 2-512:
> 
> https://lore.kernel.org/lkml/CAOnJCUJrN4frY_OdQzO-yr5CrDLvj=ge9KY2d=XnGvAF-uQNvQ@mail.gmail.com/T/
> 
> It seems like a moving target and as riscv develops we kinda expect
> this number will grow further. Do you think it is ok for me to at
> least set it to 512, if not 4096 at this time?

It's up to you in the end, I'm not familiar with the riscv architecture.

NR_CPUS is only used at the moment when creating a cpumask, where it
represents the maximum number of bits in the bitmask (bits, not bytes). So
NR_CPUS=512 means that the cpumask will contain 8 unsigned longs, while with
4096 it will contain 64 unsigned longs.

Cpumasks are only used by the arm64 code so far, so the value that you
choose won't affect existing code.

Thanks,
Alex

> 
> Thanks,
> Dao
> 
> On Mon, May 23, 2022 at 9:27 AM Alexandru Elisei
> <alexandru.elisei@arm.com> wrote:
> >
> > Hi,
> >
> > On Mon, May 23, 2022 at 09:04:04AM -0700, Dao Lu wrote:
> > > Hi Alex,
> > >
> > > Thanks for pointing that out - I wasn't sure where the number came
> > > from so I basically copied from the arm one just so the compilation
> > > can pass.
> >
> > I see, I was worried that I was looking in the wrong place.
> >
> > >
> > > I am happy to fix up the number to 32 and add the compile error
> > > message to the commit message like you said - would something like
> > > this work?
> > > -------
> > > Fixes the following compilation issue:
> > >
> > > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file
> > > or directory
> > >     5 | #include "asm/kernel.h"
> > > -------
> >
> > Sounds good, thanks:
> > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> >
> > With the error message added:
> > Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
> >
> > Thanks,
> > Alex
> >
> > > Thanks,
> > > Dao
> > >
> > > On Mon, May 23, 2022 at 2:33 AM Alexandru Elisei
> > > <alexandru.elisei@arm.com> wrote:
> > > >
> > > > Adding the kvmtool maintainers, I just noticed that they were missing.
> > > >
> > > > On Mon, May 23, 2022 at 10:31:34AM +0100, Alexandru Elisei wrote:
> > > > > Hi,
> > > > >
> > > > > When I started working on the heterogeneous PMU series, support for the
> > > > > riscv architecture wasn't merged in kvmtool, and after riscv was merged I
> > > > > missed adding the header file.
> > > > >
> > > > > This indeed fixes this compilation error:
> > > > >
> > > > > In file included from include/linux/rbtree.h:32,
> > > > >                  from include/kvm/devices.h:4,
> > > > >                  from include/kvm/pci.h:10,
> > > > >                  from include/kvm/vfio.h:6,
> > > > >                  from include/kvm/kvm-config.h:5,
> > > > >                  from include/kvm/kvm.h:6:
> > > > > include/linux/kernel.h:5:10: fatal error: asm/kernel.h: No such file or directory
> > > > >     5 | #include "asm/kernel.h"
> > > > >       |          ^~~~~~~~~~~~~~
> > > > > cc1: all warnings being treated as errors
> > > > > compilation terminated.
> > > > > make: *** [Makefile:484: builtin-balloon.o] Error 1
> > > > >
> > > > > Would be nice to include it in the commit message, so people googling for
> > > > > that exact error message can come across this commit.
> > > > >
> > > > > On Fri, May 20, 2022 at 11:09:46AM -0700, Dao Lu wrote:
> > > > > > Signed-off-by: Dao Lu <daolu@rivosinc.com>
> > > > > > ---
> > > > > >  riscv/include/asm/kernel.h | 8 ++++++++
> > > > > >  1 file changed, 8 insertions(+)
> > > > > >  create mode 100644 riscv/include/asm/kernel.h
> > > > > >
> > > > > > diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
> > > > > > new file mode 100644
> > > > > > index 0000000..a2a8d9e
> > > > > > --- /dev/null
> > > > > > +++ b/riscv/include/asm/kernel.h
> > > > > > @@ -0,0 +1,8 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > > +
> > > > > > +#ifndef __ASM_KERNEL_H
> > > > > > +#define __ASM_KERNEL_H
> > > > > > +
> > > > > > +#define NR_CPUS    4096
> > > > >
> > > > > In arch/riscv/Kconfig I see this:
> > > > >
> > > > > config NR_CPUS
> > > > >       int "Maximum number of CPUs (2-32)"
> > > > >       range 2 32
> > > > >       depends on SMP
> > > > >       default "8"
> > > > >
> > > > > Would you mind explaining where the 4096 number of CPUs comes from?
> > > > >
> > > > > Thanks,
> > > > > Alex
> > > > >
> > > > > > +
> > > > > > +#endif /* __ASM_KERNEL_H */
> > > > > > --
> > > > > > 2.36.0
> > > > > >
diff mbox series

Patch

diff --git a/riscv/include/asm/kernel.h b/riscv/include/asm/kernel.h
new file mode 100644
index 0000000..a2a8d9e
--- /dev/null
+++ b/riscv/include/asm/kernel.h
@@ -0,0 +1,8 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ASM_KERNEL_H
+#define __ASM_KERNEL_H
+
+#define NR_CPUS	4096
+
+#endif /* __ASM_KERNEL_H */