linux-kernel - Re: [PATCH 4/5] tools/nolibc: add csky support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240930060420.GB5109@1wt.eu>
Date: Mon, 30 Sep 2024 08:04:20 +0200
From: Willy Tarreau <w@....eu>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Guo Ren <guoren@...nel.org>, Shuah Khan <shuah@...nel.org>,
        linux-kernel@...r.kernel.org, linux-csky@...r.kernel.org,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 4/5] tools/nolibc: add csky support

On Mon, Sep 30, 2024 at 07:23:39AM +0200, Thomas Weißschuh wrote:
> > > [0] gcc 13.2.0 and 14.2.0 from kernel.org crosstools
> > > [1] https://github.com/XUANTIE-RV/qemu/
> > > [2]
> > 
> > I think you wanted to place a link or something above for [2].
> 
> [2] was supposed to be inline patches for QEMU, I'll try to make that a
> bit clearer.
> 
> > > diff --git a/target/csky/cpu-param.h b/target/csky/cpu-param.h
> > > index 80554cc0fc03..9181b602a26f 100644
> > > --- a/target/csky/cpu-param.h
> > > +++ b/target/csky/cpu-param.h
> > (...)
> > > diff --git a/target/csky/op_vdsp2.c b/target/csky/op_vdsp2.c
> > > index a9985a03be33..d953f5ea94fe 100644
> > > --- a/target/csky/op_vdsp2.c
> > > +++ b/target/csky/op_vdsp2.c
> > 
> > Also, the first two patches look like fixes for the arch itself, they
> > should really go outside of the nolibc development tree, at least
> > because they might have to be backported to some stable branches,
> > or later fixed/reverted in case they wouldn't be optimal.
> 
> As mentioned above, these are patches for qemu, not Linux.
> I don't know enough about QEMU or C-SKY to know if these are the
> generally correct fixes. But they seem to work well enough for nolibc.

Ah I understand now. The problem with external patches inlined like this
is that it's hard to split them apart from the rest of the patch. For
example just doing patch -p1 < patch.mbox will fail, trying to patch
non-existing files.

What I'm used to doing when quoting code/patches/etc in messages is to
indent them by 2 or more chars. That could be sufficient to explain what
needs to be fixed in the upstream project without being taken for a part
of the patchset, especially for such tiny patches. In this case the quoted
part could include only the strict minimum (i.e. no diff --git header etc).
Just a suggestion.

Cheers,
Willy