Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Zhangjin Wu <falcon@tinylab.org>
To:     w@1wt.eu
Cc:     ammarfaizi2@gnuweeb.org, arnd@arndb.de, falcon@tinylab.org,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        svens@linux.ibm.com, thomas@t-8ch.de
Subject: Re: [PATCH v1 1/8] tools/nolibc: add support for powerpc
Date:   Wed, 26 Jul 2023 04:52:02 +0800
Message-Id: <20230725205202.36788-1-falcon@tinylab.org>
In-Reply-To: <20230725182736.GA24943@1wt.eu>
References: <20230725182736.GA24943@1wt.eu>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Feedback-ID: bizesmtp:tinylab.org:qybglogicsvrgz:qybglogicsvrgz5a-1
Precedence: bulk

Hi Willy,

> On Tue, Jul 25, 2023 at 07:02:55PM +0800, Zhangjin Wu wrote:
> > > > With this new syscall.h, to support my_syscall<N>, the arch-<ARCH>.h
> > > > will only require to add ~10 lines to define their own syscall
> > > > instructions, registers and clobberlist, which looks like this (for
> > > > powerpc):
> > > > 
> > > >     #define _NOLIBC_SYSCALL_CALL "sc; bns+ 1f; neg  %0, %0; 1:"
> > > > 
> > > >     /* PowerPC doesn't always restore r3-r12 for us */
> > > >     #define _NOLIBC_SYSCALL_CLOBBERLIST 
> > > >     	"memory", "cr0", "r12", "r11", "r10", "r9", "r8", "r7", "r6", "r5", "r4"
> > > > 
> > > >     /* PowerPC write GPRS in kernel side but not restore them */
> > > >     #define _NOLIBC_GPRS_AS_OUTPUT_OPERANDS
> > > >     
> > > >     #define _NOLIBC_REG_NUM  "r0"
> > > >     #define _NOLIBC_REG_RET  "r3"
> > > >     #define _NOLIBC_REG_arg1 "r3"
> > > >     #define _NOLIBC_REG_arg2 "r4"
> > > >     #define _NOLIBC_REG_arg3 "r5"
> > > >     #define _NOLIBC_REG_arg4 "r6"
> > > >     #define _NOLIBC_REG_arg5 "r7"
> > > >     #define _NOLIBC_REG_arg6 "r8"
> > > > 
> > > > Before:
> > > > 
> > > >     $ ls tools/include/nolibc/arch-*.h | while read f; do git show dfef4fc45d5713eb23d87f0863aff9c33bd4bfaf:$f 2>/dev/null | wc -l | tr -d '\n'; echo " $f"; done
> > > >     157 tools/include/nolibc/arch-aarch64.h
> > > >     199 tools/include/nolibc/arch-arm.h
> > > >     178 tools/include/nolibc/arch-i386.h
> > > >     164 tools/include/nolibc/arch-loongarch.h
> > > >     195 tools/include/nolibc/arch-mips.h
> > > >     0 tools/include/nolibc/arch-powerpc.h
> > > >     160 tools/include/nolibc/arch-riscv.h
> > > >     186 tools/include/nolibc/arch-s390.h
> > > >     176 tools/include/nolibc/arch-x86_64.h
> > > > 
> > > > After:
> > > > 
> > > >     $ wc -l tools/include/nolibc/arch-*.h
> > > >        54 tools/include/nolibc/arch-aarch64.h
> > > >        84 tools/include/nolibc/arch-arm.h
> > > >        90 tools/include/nolibc/arch-i386.h                        /* the last one use stack to pass arguments, reserve as-is */
> > > >        59 tools/include/nolibc/arch-loongarch.h
> > > >       120 tools/include/nolibc/arch-mips.h                        /* the last two use stack to pass arguments, reserve as-is */
> > > >        73 tools/include/nolibc/arch-powerpc.h
> > > >        58 tools/include/nolibc/arch-riscv.h
> > > >        87 tools/include/nolibc/arch-s390.h
> > > >        67 tools/include/nolibc/arch-x86_64.h
> > > > 
> > > > syscall.h itself:
> > > > 
> > > >     $ wc -l tools/include/nolibc/syscall.h
> > > >     112 tools/include/nolibc/syscall.h 
> > > 
>
> [...]
>
> Hoping this clarifies my position on this.
>

Willy, Thanks very much for your detailed reply, based on your reply, I
plan to renew the powerpc patchset itself at first since both you and
Thomas have already reviewed it carefully.

After that, I will come back to read your reply again and discuss more
about our new syscall.h, I still think it is something valuable to take
a look at, although something about it still need more attention,
perhaps a RFC patchset is better for more discuss, it may show us the
profile easily.

Best regards,
Zhangjin

> Thanks,
> Willy