Re: [RFC v8 06/20] um: add UML library mode
From: Johannes Berg <johannes@sipsolutions.net>
Date: 2021-03-14 16:50:16
Also in:
linux-um
On Wed, 2021-01-20 at 11:27 +0900, Hajime Tazaki wrote:
quoted hunk ↗ jump to hunk
+++ b/arch/um/lkl/include/asm/atomic.h@@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __UM_LIBMODE_ATOMIC_H +#define __UM_LIBMODE_ATOMIC_H + +#include <asm-generic/atomic.h> + +#ifndef CONFIG_GENERIC_ATOMIC64 +#include "atomic64.h" +#endif /* !CONFIG_GENERIC_ATOMIC64 */
That's not actually configurable, is it? When is this on or off?
+static inline void cpu_relax(void)
+{
+ unsigned long flags;
+
+ /* since this is usually called in a tight loop waiting for some
+ * external condition (e.g. jiffies) lets run interrupts now to allow
+ * the external condition to propagate
+ */
+ local_irq_save(flags);
+ local_irq_restore(flags);Hmm? If interrupts are enabled, then you'll get them anyway, and if not, then this does nothing?
+static inline void arch_copy_thread(struct arch_thread *from,
+ struct arch_thread *to)
+{
+ panic("unimplemented %s: fork isn't supported yet", __func__);
+}"yet" seems kind of misleading - I mean, it really *won't* be, since that's basically what UML is, no? I mean, in principle, nothing actually stops you from building a linux.so instead of linux binary, and having main() renamed to linux_main() so the application can start up whenever it needs to, or something like that?
quoted hunk ↗ jump to hunk
+#endifdiff --git a/arch/um/lkl/include/asm/ptrace.h b/arch/um/lkl/include/asm/ptrace.h new file mode 100644 index 000000000000..83f4e10fb677 --- /dev/null +++ b/arch/um/lkl/include/asm/ptrace.h@@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __UM_LIBMODE_PTRACE_H +#define __UM_LIBMODE_PTRACE_H + +#include <linux/errno.h> + +static int reg_dummy __attribute__((unused)); + +#define PT_REGS_ORIG_SYSCALL(r) (reg_dummy) +#define PT_REGS_SYSCALL_RET(r) (reg_dummy) +#define PT_REGS_SET_SYSCALL_RETURN(r, res) (reg_dummy = (res)) +#define REGS_SP(r) (reg_dummy) + +#define user_mode(regs) 0 +#define kernel_mode(regs) 1 +#define profile_pc(regs) 0 +#define user_stack_pointer(regs) 0 + +extern void new_thread_handler(void); + +#endifdiff --git a/arch/um/lkl/include/asm/segment.h b/arch/um/lkl/include/asm/segment.h new file mode 100644 index 000000000000..1f6746866b8b --- /dev/null +++ b/arch/um/lkl/include/asm/segment.h@@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __UM_LIBMODE_SEGMENT_H +#define __UM_LIBMODE_SEGMENT_H + +typedef struct { + unsigned long seg; +} mm_segment_t; + +#endif /* _ASM_LKL_SEGMENT_H */diff --git a/arch/um/lkl/include/uapi/asm/bitsperlong.h b/arch/um/lkl/include/uapi/asm/bitsperlong.h new file mode 100644 index 000000000000..f6657ba0ff9b --- /dev/null +++ b/arch/um/lkl/include/uapi/asm/bitsperlong.h@@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __UM_LIBMODE_UAPI_BITSPERLONG_H +#define __UM_LIBMODE_UAPI_BITSPERLONG_H + +#ifdef CONFIG_64BIT +#define __BITS_PER_LONG 64 +#else +#define __BITS_PER_LONG 32 +#endif + +#include <asm-generic/bitsperlong.h>
First, that include does nothing. Second, the comment there says: /* * There seems to be no way of detecting this automatically from user * space, so 64 bit architectures should override this in their * bitsperlong.h. In particular, an architecture that supports * both 32 and 64 bit user space must not rely on CONFIG_64BIT * to decide it, but rather check a compiler provided macro. */ So you're doing exactly what it says *not* to? In fact, that totally makes sense - I mean, this is *uapi* and applications don't have access to CONFIG_ defines etc. Do you expose that somehow to LKL users that makes this OK-ish? But it's still very confusing, and you've not made sure the necessary header is actually included. IIUC, lkl builds a (shared?) library, so it won't really work this way?
quoted hunk ↗ jump to hunk
+++ b/arch/um/lkl/include/uapi/asm/byteorder.h@@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __UM_LIBMODE_UAPI_BYTEORDER_H +#define __UM_LIBMODE_UAPI_BYTEORDER_H + +#if defined(CONFIG_BIG_ENDIAN) +#include <linux/byteorder/big_endian.h>
Same here.
quoted hunk ↗ jump to hunk
+++ b/arch/um/lkl/um/delay.c@@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <linux/jiffies.h> +#include <linux/delay.h> +#include <os.h> + +void __ndelay(unsigned long nsecs) +{ + long long start = os_nsecs(); + + while (os_nsecs() < start + nsecs) + ;
At least do something like cpu_relax()? Although ... you made that do nothing, so you probably want an arch-specific thing here? johannes