MIME-Version: 1.0
References: <20211221163532.2636028-1-guoren@kernel.org> <20211221163532.2636028-4-guoren@kernel.org>
 <CAK8P3a3navO2Z2F5zxisby5EBNDo8rwQ6hxSuyniFgFxrQ5qXQ@mail.gmail.com>
In-Reply-To: <CAK8P3a3navO2Z2F5zxisby5EBNDo8rwQ6hxSuyniFgFxrQ5qXQ@mail.gmail.com>
From:   Guo Ren <guoren@kernel.org>
Date:   Wed, 22 Dec 2021 20:03:46 +0800
Message-ID: <CAJF2gTSssufQOQTXqzRjY8h_JjAmMs8jC6R+6d09K728=4fOOA@mail.gmail.com>
Subject: Re: [PATCH 03/13] riscv: compat: Add basic compat date type implementation
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     Palmer Dabbelt <palmer@dabbelt.com>,
        Anup Patel <anup.patel@wdc.com>,
        gregkh <gregkh@linuxfoundation.org>,
        liush <liush@allwinnertech.com>, Wei Fu <wefu@redhat.com>,
        Drew Fustini <drew@beagleboard.org>,
        Wang Junqiang <wangjunqiang@iscas.ac.cn>,
        =?UTF-8?B?V2VpIFd1ICjlkLTkvJ8p?= <lazyparser@gmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        linux-csky@vger.kernel.org, Guo Ren <guoren@linux.alibaba.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Dec 22, 2021 at 1:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Dec 21, 2021 at 5:35 PM <guoren@kernel.org> wrote:
> >
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Implement asm/compat.h for struct compat_xxx, RLIM_INFINITY,
> > OFF_T_MAX, is_compat_task, compat_user_regset, regset convert.
> >
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > ---
> >  arch/riscv/include/asm/compat.h      | 259 +++++++++++++++++++++++++++
>
> Since both the native and compat side use the generic interface, I think this
> should all be part of asm-generic/compat.h, in case other architectures want to
> share this in the future. Maybe see if any other architectures use the
> same definition
> for some of the structures and then remove the duplicates.
Agree.

>
> > +struct compat_stat {
> > +       compat_ulong_t  st_dev;
> > +       compat_ulong_t  st_ino;
> > +       compat_uint_t   st_mode;
> > +       compat_uint_t   st_nlink;
>
> You should not need a compat_stat, because native rv32 does not have a
> stat() syscall.
We need it:

$ make ARCH=riscv CROSS_COMPILE=riscv64-unknown-linux-gnu-
EXTRA_CFLAGS+=-g O=../build-riscv/ Image -j > /dev/null
/home/guoren/source/kernel/riscv-linux/fs/stat.c: In function 'cp_compat_stat':
/home/guoren/source/kernel/riscv-linux/fs/stat.c:645:21: error:
storage size of 'tmp' isn't known
  645 |  struct compat_stat tmp;
      |                     ^~~
/home/guoren/source/kernel/riscv-linux/fs/stat.c:645:21: warning:
unused variable 'tmp' [-Wunused-variable]
/home/guoren/source/kernel/riscv-linux/fs/stat.c:674:1: error: control
reaches end of non-void function [-Werror=return-type]
  674 | }
      | ^
cc1: some warnings being treated as errors
make[2]: *** [/home/guoren/source/kernel/riscv-linux/scripts/Makefile.build:287:
fs/stat.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/home/guoren/source/kernel/riscv-linux/Makefile:1846: fs] Error 2

>
> > +static inline int is_compat_task(void)
> > +{
> > +       return test_thread_flag(TIF_32BIT);
> > +}
> > +
> > +struct compat_user_regs_struct {
> > +       compat_ulong_t pc;
> > +       compat_ulong_t ra;
>
> These clearly need to stay in arch/riscv
Okay

>
>         Arnd

--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/