Message ID | 714783a8d1d6aace7d0e315fc12ffc60b5867ada.1601960644.git.thehajime@gmail.com |
---|---|
State | Not Applicable |
Headers | show |
Series | [RFC,v7,01/21] um: split build in kernel and host parts | expand |
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
diff --git a/arch/um/Kconfig b/arch/um/Kconfig index e41d31d0d875..df7daceb095c 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -22,9 +22,20 @@ config UML select TTY # Needed for line.c select MODULE_REL_CRCS if MODVERSIONS +config UMMODE_LIB + bool "UML mode: library mode" + default y if "$(UMMODE)" = "library" + 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. + + 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 select STACKTRACE config GENERIC_CALIBRATE_DELAY bool - default y + default y if MMU config HZ int diff --git a/arch/um/Makefile b/arch/um/Makefile index 8be7bc479442..ee35bd030bf8 100644 --- a/arch/um/Makefile +++ b/arch/um/Makefile @@ -17,6 +17,10 @@ else KBUILD_DEFCONFIG := $(SUBARCH)_defconfig endif +ifeq ($(UMMODE),library) + SUBARCH := um/nommu +endif + ARCH_DIR := arch/um OS := $(shell uname -s) # We require bash because the vmlinux link and loader script cpp use bash @@ -97,8 +101,14 @@ KBUILD_CFLAGS += $(KERNEL_DEFINES) LDFLAGS_vmlinux += -r INSTALL_PATH=$(objtree)/tools/um +ifeq ($(UMMODE),library) +all: install + $(Q)$(MAKE) -C $(srctree)/tools/um +install: linux.o um_headers_install +else all: linux install: linux.o +endif @echo " INSTALL $(INSTALL_PATH)/lib/$<" @mkdir -p $(INSTALL_PATH)/lib/ @cp $< $(INSTALL_PATH)/lib/ @@ -111,7 +121,7 @@ linux: linux.o 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 $< $@ $(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 + select UACCESS_MEMCPY + select ARCH_THREAD_STACK_ALLOCATOR + select ARCH_HAS_SYSCALL_WRAPPER + +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 + +# XXX: need this to work well with tap13.py +config PRINTK_TIME + bool + default y diff --git a/arch/um/nommu/um/Makefile b/arch/um/nommu/um/Makefile new file mode 100644 index 000000000000..b580e0fe97b5 --- /dev/null +++ b/arch/um/nommu/um/Makefile @@ -0,0 +1,4 @@ +include/generated/user_constants.h: $(srctree)/arch/um/nommu/um/user_constants.h + $(Q)cp -f $^ $@ + +obj-y = bootmem.o console.o cpu.o delay.o setup.o syscalls.o threads.o unimplemented.o