From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2C49AC433E7 for ; Fri, 9 Oct 2020 03:38:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BA17F20739 for ; Fri, 9 Oct 2020 03:38:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="bkXblcmQ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731512AbgJIDiy (ORCPT ); Thu, 8 Oct 2020 23:38:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37532 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731511AbgJIDix (ORCPT ); Thu, 8 Oct 2020 23:38:53 -0400 Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D00A3C0613D2 for ; Thu, 8 Oct 2020 20:38:53 -0700 (PDT) Received: by mail-pf1-x443.google.com with SMTP id x13so3083857pfa.9 for ; Thu, 08 Oct 2020 20:38:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:message-id:from:to:cc:subject:in-reply-to:references :user-agent:mime-version; bh=d+8ZR/Hdzfhmx2SRvxSn61kJ30onA81VcKme0467eas=; b=bkXblcmQ92RE99snCmLip0SKT9dVd4d1QXXZGG8FSFKurEkBmtacKnqPLmwQxjyXtf 3eQQ21/Z7EsCx/lFRbtz+742cWzSVFbEryAkMReTV3uv78kokSH3Pwr557BXqAY3ZMQQ Z2w1IHTvDD/N/fD8HvUWCRxklFRXBth2aSBe2JvX1Xz9gWyblMpY7fsfak40Jcb+mGFr vSiC8iNrcds4VBiLiVsqO0V1UHlCE6hkoUbbVzOCSuN9/8Xrf/P/xt3Fgnffs51B0H/0 Hv//45WnpH+WW6wlxxRmu/JggtKtOQ8QfuiurvTjZhes6Qaxlx1nqb8bfxHrb+ez1UtP /7zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:from:to:cc:subject:in-reply-to :references:user-agent:mime-version; bh=d+8ZR/Hdzfhmx2SRvxSn61kJ30onA81VcKme0467eas=; b=oLth/dB1ILYxv3nfwovn/UUZLdD5gJd1DEucNbtZfUfEVpcXvW0ckE6Hd6/TEhv9sB SP7hi693LHTQpmdgwvGQsfuTjgI/EfunRuY4A14FmwS24Vybmuo9+wxW3VMOzwDjjjpu fj75G5c1iwMDp8+jTkSUkNQ2/fZZ80T/HyqRSqLLuowReSHneS7gYCyxVJqUasbPA+yU EmR2gsU/U1YwvM2MH0S0wyCtVJ0mzujwWSY01wuNf1OGGRwFF7H26IaQLeCX6SOsSrq4 TyrA7GSBOYhs2e+M+KPg+HwF4v2iKC3kQQzfxyxw29BPNZBaPaXapAOOcRbnGDu6f0J/ OMHA== X-Gm-Message-State: AOAM532LNcPUUtH3F3XoNVqHZWTz1/xo2O3N1oOghljUU9LB/TgGEAtm vD9HfYa61Y6zZH8Eepa7CKQ= X-Google-Smtp-Source: ABdhPJxwlEKfA7tZu6oQG6gvC/qxRP4lOm5DY19qWXL+Vkx8waGw8yOZ18DJU9CCV0fsCt1n5ba8kg== X-Received: by 2002:a63:4945:: with SMTP id y5mr1745544pgk.181.1602214732974; Thu, 08 Oct 2020 20:38:52 -0700 (PDT) Received: from earth-mac.local.gmail.com (219x123x138x129.ap219.ftth.ucom.ne.jp. [219.123.138.129]) by smtp.gmail.com with ESMTPSA id 3sm9130350pfv.92.2020.10.08.20.38.50 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 08 Oct 2020 20:38:52 -0700 (PDT) Date: Fri, 09 Oct 2020 12:38:46 +0900 Message-ID: From: Hajime Tazaki To: johannes@sipsolutions.net Cc: linux-um@lists.infradead.org, jdike@addtoit.com, richard@nod.at, anton.ivanov@cambridgegreys.com, tavi.purdila@gmail.com, linux-kernel-library@freelists.org, linux-arch@vger.kernel.org, retrage01@gmail.com Subject: Re: [RFC v7 08/21] um: add nommu mode for UML library mode In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/25.3 Mule/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-arch@vger.kernel.org Hello Johannes, On Thu, 08 Oct 2020 00:44:12 +0900, Johannes Berg wrote: > > 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 thanks; will fix both in the next round. > > 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. (I will leave this to the other thread if any) > (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. Thanks, we will. > > 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. Will cut the FreeBSD part. > > + > > +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. Will clean up this part. > > +++ 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 > > + > > +#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? We were suggested to not use GENERIC_ATOMIC64 on 64bit build during our v3 patchset. So we actually control CONFIG_GENERIC_ATOMIC64 to be on only when !64BIT, and thus need atomic64.h for the 64BIT build. https://lwn.net/ml/linux-arch/20200205093454.GG14879@hirez.programming.kicks-ass.net/ > > 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 > > + > > +#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? thanks, it seems that this is unnecessary now. will fix them. > > 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 > > + > > +#endif > > Not sure why this file even exists, it doesn't for any other arch > (except for h8300 which seems odd). You're right. We will remove this. > > 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. thanks, will add the comments. > > +#ifndef __UM_NOMMU_SCHED_H > > +#define __UM_NOMMU_SCHED_H > > + > > +#include > > +#include > > + > > +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. I understand. I will move this file to the later patch (11/21). > > +++ 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. The message is incorrect since uapi header infra has changed. This also applies to the comment in arch/um/nommu/include/asm/Kbuild. Will fix both in the next round. > > +++ b/arch/um/nommu/um/delay.c > > why the double um/ path? This is because now in arch/um/Makefile, - SUBARCH := um/nommu - HEADER_ARCH := $(SUBARCH) - HOST_DIR := arch/$(HEADER_ARCH) - core-y += $(HOST_DIR)/um the last line expands as arch/um/nommu/um. > > +/* skas/process.c */ > > +void halt_skas(void) > > +{} > > nit: missing blank line after this will fix this and other parts of the whole patchset. > > +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? Okay, we will think about this. -- Hajime