[RFC,v7,16/21] um: nommu: plug in the build system

Basic Makefiles for building library of nommu mode. Add a new architecture
specific target for installing the resulting library files and headers.

To make nommu binaries build, UML introduced an additional option, UMMODE
variable, to switch the output file of build: kernel (default), or
library (nommu).  Those modes are not able to be ON at the same time.

To build on library mode, users do the following:

  make defconfig ARCH=um UMMODE=library
  make ARCH=um UMMODE=library

Signed-off-by: Octavian Purdila <tavi.purdila@gmail.com>
Signed-off-by: Hajime Tazaki <thehajime@gmail.com>
---
 arch/um/Kconfig           | 17 ++++++++++++++---
 arch/um/Makefile          | 12 +++++++++++-
 arch/um/kernel/Makefile   | 14 +++++++++-----
 arch/um/nommu/um/Kconfig  | 37 +++++++++++++++++++++++++++++++++++++
 arch/um/nommu/um/Makefile |  4 ++++
 5 files changed, 75 insertions(+), 9 deletions(-)
 create mode 100644 arch/um/nommu/um/Kconfig
 create mode 100644 arch/um/nommu/um/Makefile

On Tue, 2020-10-06 at 18:44 +0900, Hajime Tazaki wrote:
> Basic Makefiles for building library of nommu mode. Add a new architecture
> specific target for installing the resulting library files and headers.
> 
> To make nommu binaries build, UML introduced an additional option, UMMODE
> variable, to switch the output file of build: kernel (default), or
> library (nommu).  Those modes are not able to be ON at the same time.

Here you did that again :)

I think you should really be up-front about it and call it "kernel" and
"library" mode. Then do the ifdefs on library mode etc.

Sure, kernel mode will be CONFIG_MMU=y and library mode will be
!CONFIG_MMU instead, but it really makes more sense to call things
CONFIG_UMMODE_LIB in most cases, I think?

Yes, a lot of stuff *does* depend on CONFIG_MMU, and that still makes
sense. It just feels like a lot of stuff you made depend on CONFIG_MMU
here (or !CONFIG_MMU) really isn't related too much to that, rather than
being related to "library mode".

>   make defconfig ARCH=um UMMODE=library
>   make ARCH=um UMMODE=library

Why not make it a real Kconfig symbol? I find the ARCH=um tricky enough
to remember all the time, so much I usually write a "GNUmakefile" with

export ARCH=um
include Makefile

:-)

> +config UMMODE_LIB
> +	bool "UML mode: library mode"
> +	default y if "$(UMMODE)" = "library"

So wait, you _can_ switch that through Kconfig then, because you made it
a visible option (string after "bool"). But it won't work, because then
you later in the build system etc. still check UMMODE instead of
CONFIG_UMMODE_LIB. Seems like something that ought to be fixed one way
or the other - at the very least hide this symbol if setting it manually
is invalid.

> +	help
> +	 This mode switches a mode to build a library of UML (Linux
> +	 Kernel Library/LKL).  This switch is exclusive to "kernel mode"
> +	 of UML, which is traditional mode of UML.

Not sure if that historically made more sense, but you don't have any
UMMODE_KERNEL option or something like that, so the help text seems
confusing?


> +	 For more detail about LKL, see
> +	 <file:Documentation/virt/uml/lkl.txt>.
> +
>  config MMU
>  	bool
> -	default y
> +	default y if !UMMODE_LIB
>  
>  config NO_IOMEM
>  	def_bool y
> @@ -45,12 +56,12 @@ config LOCKDEP_SUPPORT
>  
>  config STACKTRACE_SUPPORT
>  	bool
> -	default y
> +	default y if MMU

Same as what I said above - there really shouldn't be any inherent
reason why STACKTRACE_SUPPORT depends on CONFIG_MMU, is there? Apart
from not having implemented it in UMMODE_LIB, that is, and then you
should make this if !UMMODE_LIB?

>  config GENERIC_CALIBRATE_DELAY
>  	bool
> -	default y
> +	default y if MMU

Same here. This one _surely_ has no relation to MMU.

> +ifeq ($(UMMODE),library)
> +  SUBARCH := um/nommu
> +endif

>  INSTALL_PATH=$(objtree)/tools/um
> +ifeq ($(UMMODE),library)

Here a few places using UMMODE which must come from the command line.

>  linux.o: vmlinux
>  	@echo '  LINK	$@'
> -	$(Q)$(OBJCOPY) -R .eh_frame $< $@
> +	$(Q)$(OBJCOPY) -R .eh_frame -L sem_init -L sem_post -L sem_wait -L sem_destroy $< $@

Care to explain?

>  	$(Q)mkdir -p $(objtree)/tools/um/lib
>  
>  define archhelp
> diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile
> index 9b63831a69e1..01605ed439cb 100644
> --- a/arch/um/kernel/Makefile
> +++ b/arch/um/kernel/Makefile
> @@ -14,17 +14,21 @@ CPPFLAGS_vmlinux.lds := -DSTART=$(LDS_START)		\
>  			$(LDS_EXTRA)
>  extra-y := vmlinux.lds
>  
> -obj-y = config.o exec.o exitcode.o irq.o ksyms.o mem.o \
> -	physmem.o process.o ptrace.o reboot.o sigio.o \
> -	signal.o syscall.o sysrq.o time.o tlb.o trap.o \
> -	um_arch.o umid.o maccess.o kmsg_dump.o skas/
> +obj-y = config.o exitcode.o irq.o ksyms.o \
> +	process.o reboot.o sigio.o \
> +	signal.o syscall.o time.o \
> +	um_arch.o umid.o maccess.o kmsg_dump.o
> +
> +ifdef CONFIG_MMU
> +obj-y += exec.o mem.o physmem.o ptrace.o sysrq.o tlb.o trap.o skas/
> +endif
>  
>  obj-$(CONFIG_BLK_DEV_INITRD) += initrd.o
>  obj-$(CONFIG_GPROF)	+= gprof_syms.o
>  obj-$(CONFIG_GCOV)	+= gmon_syms.o
>  obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>  obj-$(CONFIG_STACKTRACE) += stacktrace.o
> -obj-y += user_syms.o
> +obj-$(CONFIG_MMU) += user_syms.o
>  
>  USER_OBJS := config.o
>  
> diff --git a/arch/um/nommu/um/Kconfig b/arch/um/nommu/um/Kconfig
> new file mode 100644
> index 000000000000..20b3eaccb6f0
> --- /dev/null
> +++ b/arch/um/nommu/um/Kconfig
> @@ -0,0 +1,37 @@
> +config UML_NOMMU
> +	def_bool y
> +	depends on !SMP && !MMU

again, using MMU to mean "LKL" (both UML_NOMMU and !MMU places)

> +	select UACCESS_MEMCPY
> +	select ARCH_THREAD_STACK_ALLOCATOR
> +	select ARCH_HAS_SYSCALL_WRAPPER

You never use this except for the selects, maybe can go elsewhere?

> +config 64BIT
> +	bool
> +	default y
> +
> +config GENERIC_CSUM
> +	def_bool y
> +
> +config GENERIC_ATOMIC64
> +	bool
> +	default y if !64BIT
> +
> +config SECCOMP
> +	bool
> +	default n
> +
> +config GENERIC_HWEIGHT
> +	def_bool y
> +
> +config GENERIC_CALIBRATE_DELAY
> +	bool
> +	default n
> +
> +config STACKTRACE_SUPPORT
> +	bool
> +	default n

You ... were just changing these elsewhere, so one of that isn't needed?

> +# XXX: need this to work well with tap13.py

What's tap13.py??

johannes

(I removed UMMODE_LIB vs !MMU part, but leave replies to the others)

On Thu, 08 Oct 2020 04:20:00 +0900,
Johannes Berg wrote:

> >   make defconfig ARCH=um UMMODE=library
> >   make ARCH=um UMMODE=library
>
> Why not make it a real Kconfig symbol? I find the ARCH=um tricky enough
> to remember all the time, so much I usually write a "GNUmakefile" with
>
> export ARCH=um
> include Makefile
>
> :-)
>
> > +config UMMODE_LIB
> > + bool "UML mode: library mode"
> > + default y if "$(UMMODE)" = "library"
>
> So wait, you _can_ switch that through Kconfig then, because you made it
> a visible option (string after "bool"). But it won't work, because then
> you later in the build system etc. still check UMMODE instead of
> CONFIG_UMMODE_LIB. Seems like something that ought to be fixed one way
> or the other - at the very least hide this symbol if setting it manually
> is invalid.

I now see what I went wrong..
thanks, will fix this with your suggestion not to use the command line argument.

> > + help
> > + This mode switches a mode to build a library of UML (Linux
> > + Kernel Library/LKL).  This switch is exclusive to "kernel mode"
> > + of UML, which is traditional mode of UML.
>
> Not sure if that historically made more sense, but you don't have any
> UMMODE_KERNEL option or something like that, so the help text seems
> confusing?

thanks, we will clarify this text.

> > +ifeq ($(UMMODE),library)
> > +  SUBARCH := um/nommu
> > +endif
>
> >  INSTALL_PATH=$(objtree)/tools/um
> > +ifeq ($(UMMODE),library)
>
> Here a few places using UMMODE which must come from the command line.

will fix this too.

> >  linux.o: vmlinux
> >  @echo '  LINK $@'
> > - $(Q)$(OBJCOPY) -R .eh_frame $< $@
> > + $(Q)$(OBJCOPY) -R .eh_frame -L sem_init -L sem_post -L sem_wait -L sem_destroy $< $@
>
> Care to explain?

If we will link a userspace binary with liblinux.a (LKL) and
libpthread, the the symbols sem_init, etc will conflict with the one
in the kernel (ipc/sem.c).  objcopy command with -L localizes the
symbol specified to avoid this conflict.

We will add the description as a comment.

> > + select UACCESS_MEMCPY
> > + select ARCH_THREAD_STACK_ALLOCATOR
> > + select ARCH_HAS_SYSCALL_WRAPPER
>
> You never use this except for the selects, maybe can go elsewhere?

This is used at the patch 12/21, so add Kconfig earlier and move this
to 12/21 would be better.  We'll work on.

> > +config 64BIT
> > + bool
> > + default y
> > +
> > +config GENERIC_CSUM
> > + def_bool y
> > +
> > +config GENERIC_ATOMIC64
> > + bool
> > + default y if !64BIT
> > +
> > +config SECCOMP
> > + bool
> > + default n
> > +
> > +config GENERIC_HWEIGHT
> > + def_bool y
> > +
> > +config GENERIC_CALIBRATE_DELAY
> > + bool
> > + default n
> > +
> > +config STACKTRACE_SUPPORT
> > + bool
> > + default n
>
> You ... were just changing these elsewhere, so one of that isn't needed?

you're right too. will fix it.

-- Hajime