On Tue, 2020-10-06 at 18:44 +0900, Hajime Tazaki wrote:
> This patch introduces the nommu operation with UML code so that host
> interface can be shrineked for broader environments support.
shrunk
> The nommu mode is implemneted as SUBARCH of arch/um, which places at
implemented
> arch/um/nommu. This SUBARCH defines mode-specific code of memory
> management as well as thread implementation, along with the uapi headers
> to be exported to users.
>
> The headers we introduce in this patch are simple wrappers to the
> asm-generic headers or stubs for things we don't support, such as
> ptrace, DMA, signals, ELF handling and low level processor operations.
>
> nommu mode shares most of arch/um code (irq, thread_info, process
> scheduling) but implements its own facilities, which are memory
> management (nommu), thread primitives (struct arch_thread), system call
> interface, and console.
>
> The outlook of updated directory structure is as follows:
>
> arch/um
> |-- configs (untouched)
> |-- drivers unuse stdio_console.c for !MMU
> |-- include
> | |-- asm updated with !CONFIG_MMU
> | |-- linux (untouched)
> | `-- shared updated with new functions
> | `-- skas (untouched)
> | `-- uapi added for upai header installation
> |-- kernel updated to integrate with !MMU
> | `-- skas (untouched, don't use for !MMU)
> |-- nommu SUBARCH dir (internally =um/nommu)
> | |-- include
> | | |-- asm headers for subarch specific info
> | | `-- uapi headers for user-visible APIs
> | `-- um
> | `-- shared headers for subarch specific info
> `-- scripts added a script for header installation
That seems awkward. Might be nice now for "as little changes as
possible", but eventually it seems it would be better to separate that
out into arch/um/{common,nommu,mmu}/ or something like that?
Or perhaps something like
arch/um/{common,lkl,full}
or something? Not sure I like 'full' though. 'vm'? Hmm.
(also, scripts/ doesn't exist in this patch?)
> +++ b/arch/um/nommu/Makefile
> @@ -0,0 +1 @@
> +#
hmm? maybe add a comment why an empty makefile is needed.
> diff --git a/arch/um/nommu/Makefile.um b/arch/um/nommu/Makefile.um
> new file mode 100644
> index 000000000000..3808462d8283
> --- /dev/null
> +++ b/arch/um/nommu/Makefile.um
[...]
> +ifeq ($(shell uname -s), Linux)
> +NPROC=$(shell nproc)
> +else # e.g., FreeBSD
> +NPROC=$(shell sysctl -n hw.ncpu)
> +endif
That seems very inappropriate here.
> +
> +um_headers_install: $(objtree)/$(HOST_DIR)/include/generated/uapi/asm/syscall_defs.h headers
> +# if syscall_defs.h is newer than generated headers (autoconf.h)
> + $(Q)if [ ! -r $(objtree)/tools/um/include/lkl/autoconf.h ] || \
> + [ $< -nt $(objtree)/tools/um/include/lkl/autoconf.h ]; then \
> + $(srctree)/$(ARCH_DIR)/scripts/headers_install.py \
> + $(subst -j,-j$(NPROC),$(findstring -j,$(MAKEFLAGS))) \
Yeah ... just don't.
> +++ b/arch/um/nommu/include/asm/atomic.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __UM_NOMMU_ATOMIC_H
> +#define __UM_NOMMU_ATOMIC_H
> +
> +#include <asm-generic/atomic.h>
> +
> +#ifndef CONFIG_GENERIC_ATOMIC64
> +#include "atomic64.h"
> +#endif /* !CONFIG_GENERIC_ATOMIC64 */
> +
> +#endif
> diff --git a/arch/um/nommu/include/asm/atomic64.h b/arch/um/nommu/include/asm/atomic64.h
> new file mode 100644
> index 000000000000..949360dea7af
> --- /dev/null
> +++ b/arch/um/nommu/include/asm/atomic64.h
That doesn't make sense to me, you can control CONFIG_GENERIC_ATOMIC64
to be on, and don't need the ifdef and this file?
> diff --git a/arch/um/nommu/include/asm/bitsperlong.h b/arch/um/nommu/include/asm/bitsperlong.h
> new file mode 100644
> index 000000000000..a150cee41e75
> --- /dev/null
> +++ b/arch/um/nommu/include/asm/bitsperlong.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __UM_NOMMU_BITSPERLONG_H
> +#define __UM_NOMMU_BITSPERLONG_H
> +
> +#include <uapi/asm/bitsperlong.h>
> +
> +#define BITS_PER_LONG __BITS_PER_LONG
> +
> +#define BITS_PER_LONG_LONG 64
> +
> +#endif
That's very similar to the contents of include/asm-
generic/bitsperlong.h, add comments why it's needed?
> diff --git a/arch/um/nommu/include/asm/byteorder.h b/arch/um/nommu/include/asm/byteorder.h
> new file mode 100644
> index 000000000000..920a5fd26cad
> --- /dev/null
> +++ b/arch/um/nommu/include/asm/byteorder.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __UM_NOMMU_BYTEORDER_H
> +#define __UM_NOMMU_BYTEORDER_H
> +
> +#include <uapi/asm/byteorder.h>
> +
> +#endif
Not sure why this file even exists, it doesn't for any other arch
(except for h8300 which seems odd).
> diff --git a/arch/um/nommu/include/asm/elf.h b/arch/um/nommu/include/asm/elf.h
> new file mode 100644
> index 000000000000..edcf63edeed1
> --- /dev/null
> +++ b/arch/um/nommu/include/asm/elf.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __UM_NOMMU_ELF_H
> +#define __UM_NOMMU_ELF_H
> +
> +#define elf_check_arch(x) 0
Please add a comment ...
> +#ifdef CONFIG_64BIT
> +#define ELF_CLASS ELFCLASS64
> +#else
> +#define ELF_CLASS ELFCLASS32
> +#endif
> +
> +#define elf_gregset_t long
> +#define elf_fpregset_t double
Also here.
> +#ifndef __UM_NOMMU_SCHED_H
> +#define __UM_NOMMU_SCHED_H
> +
> +#include <linux/sched.h>
> +#include <uapi/asm/host_ops.h>
> +
> +static inline void thread_sched_jb(void)
> +{
> + if (test_ti_thread_flag(current_thread_info(), TIF_HOST_THREAD)) {
> + set_ti_thread_flag(current_thread_info(), TIF_SCHED_JB);
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + lkl_ops->jmp_buf_set(¤t_thread_info()->task->thread.arch.sched_jb,
> + schedule);
> + } else {
> + lkl_bug("%s can be used only for host task", __func__);
> + }
> +}
What's "thread_sched_jb"? Doesn't really seem to exist? Neither does
"TIF_SCHED_JB", at least in my kernel?
Looks like that's added by a later patch, should probably then add this
file also only later so it's actually consistent and one can review all
the pieces together.
> +++ b/arch/um/nommu/include/uapi/asm/Kbuild
> @@ -0,0 +1,6 @@
> +# UAPI Header export list
> +
> +generated-y += syscall_defs.h
> +
> +# no generated-y since we need special user headers handling
> +# see arch/um/script/headers_install.py
Also doesn't exist yet.
> +++ b/arch/um/nommu/um/delay.c
why the double um/ path?
> +/* skas/process.c */
> +void halt_skas(void)
> +{}
nit: missing blank line after this
> +int is_skas_winch(int pid, int fd, void *data)
> +{
> + return 0;
> +}
> +void reboot_skas(void)
> +{}
> +
> +int __init start_uml(void)
> +{
> + return 0;
> +}
That seems odd?
Might be better to have an own os.h, or split that into os-uml.h and
os-lkl.h or so?
johannes
