From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13325 Path: news.gmane.org!.POSTED!not-for-mail From: Michael Clark Newsgroups: gmane.linux.lib.musl.general Subject: Re: riscv port for review Date: Fri, 28 Sep 2018 18:33:18 +1200 Message-ID: References: <20180928022404.GQ17995@brightrain.aerifal.cx> <20180928024749.GS17995@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 (1.0) Content-Type: multipart/alternative; boundary=Apple-Mail-3D4CE5B4-1398-408C-B478-A2234A8CDEA7 Content-Transfer-Encoding: 7bit X-Trace: blaine.gmane.org 1538116324 19188 195.159.176.226 (28 Sep 2018 06:32:04 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 28 Sep 2018 06:32:04 +0000 (UTC) To: musl@lists.openwall.com, palmer@sifive.com Original-X-From: musl-return-13341-gllmg-musl=m.gmane.org@lists.openwall.com Fri Sep 28 08:32:00 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1g5mJj-0004sO-57 for gllmg-musl@m.gmane.org; Fri, 28 Sep 2018 08:31:59 +0200 Original-Received: (qmail 7514 invoked by uid 550); 28 Sep 2018 06:34:06 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 7490 invoked from network); 28 Sep 2018 06:34:06 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mac.com; s=04042017; t=1538116407; bh=9yD0zABn3nDXtGZ4j6imcmZisonr+DNU+3xfBt+qpsQ=; h=From:Content-type:MIME-version:Date:Subject:Message-id:To; b=f8RfHouuOZTMBktrq8JQ2jpUuq8wMeu/o33624tPbGqvTJZmE2SsN/PvowVW2lLcO aK4zSN5girE/Sbqkl7rKjikqOy+kpYklk37tgZGI39byVbwVmxK3yRCNczDTLQwQhQ JOztsZKjBZeShDdpnQJhhy4CebYPdQWSEJx6nxUmDDznD2+eV2bpqCyq7v8Io9efS2 1HbW8r/1YtIVAEmh/+CkCZQViUOe17WEFpmO/TUIUIAe06xMPr2argFJeed2c0xwRR kBoOoDitib8LCZfzw/a22HxuMRVelNiI3wnKKl3lXQZaFu9iYlH7V7ZsYjTvszFVa2 9iE3I8iBMfDdA== X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1809280071 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-09-28_02:,, signatures=0 In-reply-to: <20180928024749.GS17995@brightrain.aerifal.cx> X-Mailer: iPhone Mail (16A366) Xref: news.gmane.org gmane.linux.lib.musl.general:13325 Archived-At: --Apple-Mail-3D4CE5B4-1398-408C-B478-A2234A8CDEA7 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Rich, > On 28/09/2018, at 2:47 PM, Rich Felker wrote: >=20 >> On Thu, Sep 27, 2018 at 10:24:04PM -0400, Rich Felker wrote: >> Pulled from here: >> https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518= bd2f5f51ee >>=20 >> Attached for review. >=20 >> diff --git a/arch/riscv32/bits/alltypes.h.in b/arch/riscv32/bits/alltypes= .h.in >> new file mode 100644 >> index 0000000..66ca18a >> --- /dev/null >> +++ b/arch/riscv32/bits/alltypes.h.in >> @@ -0,0 +1,26 @@ >> +#define _Addr int >> +#define _Int64 long long >> +#define _Reg int >> + >> +TYPEDEF __builtin_va_list va_list; >> +TYPEDEF __builtin_va_list __isoc_va_list; >> + >> +#ifndef __cplusplus >> +TYPEDEF int wchar_t; >> +#endif >> + >> +TYPEDEF float float_t; >> +TYPEDEF double double_t; >> + >> +TYPEDEF struct { long long __ll; long double __ld; } max_align_t; >> + >> +TYPEDEF long time_t; >=20 > Is riscv32 time_t really 32-bit? If so that's really disappointing, > but presumably unfixable... This definitely is fixable as the riscv32 Linux ABI is not final. This is th= e first time a Linux libc ABI expert has looked closely at the musl port, an= d while riscv32 is present, all of my own testing has been performed on risc= v64 Linux. The RISC-V Glibc port currently only contains riscv64. The riscv32 ABI is st= ill in the process of being finalised. Palmer likely knows the exact status.= SiFive doesn=E2=80=99t have any 32-bit cores with MMUs so it hasn=E2=80=99t= been high on the list of priorities. In any case, now is a very good time to do some cross-checking between musl r= iscv32, qemu-riscv32, the glibc riscv32 port and riscv32 linux-kernel. I thi= nk we are just getting the default asm-generic in QEMU as we have not done a= nything special... yet... I believe there is still an alignment issue with the stat structure in qemu-= riscv32. When we last looked at this issue earlier in the year we didn=E2=80= =99t have linux-kernel support for riscv32, as the toolchain/ABI focus this y= ear was on finalising riscv64 in Glibc and Linux. Debian and Fedora only have riscv64 ports and afaik riscv64 is all that is p= resent in Glibc. Of course linux-kernel is authoritative for the ABI so we n= eed to run tests using riscv32 Linux in full system emulation in qemu-system= -riscv32; qemu-riscv32 also needs to be audited. At the time we were submitt= ing qemu upstream we could build riscv32 kernels. This has solved and we nee= d to get regular build and test for riscv32 and riscv64 Linux and QEMU in CI= ... There is an open bug on riscv-qemu stat which we can look at now that linux-= kernel has initial riscv32 support. We should write a test that prints offse= tof and sizeof for the stat structure members using musl, glibc and linux he= aders to find out what=E2=80=99s happening... I think there is still some time to nail down any remaining issues with risc= v32. We definitely want 64-bit time_t and any other types that should be 64-= bit should be audited now. Given the indirection through multiple levels of t= ypedefs it=E2=80=99s not immediately clear without a bit of analysis. We may do the same thing as was done with glibc and split the port up, first= submitting riscv64, which is locked down. I also had a question regarding whether we needed to #define __ARCH_WANT_STA= T64 in linux/arch/riscv/include/asm/unistd.h or whether this is a legacy fla= g. All of the other 32-bit ports define it, but I do not know if it is neces= sary for a legacy free 32-bit port: https://github.com/torvalds/linux/blob/master/arch/riscv/include/asm/unistd.= h BTW Thanks very much for the review... I=E2=80=99ll go through the remainder= of the patch review comments and work on another revision... __riscv_xlen c= ontains either 32 or 64 at present and is how RISC-V code distinguishes betw= een riscv32 and riscv64. We can remove references to it from the musl riscv p= orts due to them not sharing code. Also, there is no official RISC-V big end= ian so we can remove EB. Big endian may be considered at some point, and it m= ay well have appeared in a previous draft of the ISA manual, but at present,= it is not defined in the ISA manual or toolchain, so we=E2=80=99ll remove i= t. We=E2=80=99ll do some 32-bit testing... now is a good time. SiFive=E2=80=99s= Linux board, the HiFive Unleashed, is riscv64, which likely explains the si= tuation with riscv32. Thanks, Michael= --Apple-Mail-3D4CE5B4-1398-408C-B478-A2234A8CDEA7 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable
Hi R= ich,

On 28/09/2018, at 2:47 PM, Rich Felker <dalias@libc.org> wrote:

On Thu, Sep 27, 2018 at 10= :24:04PM -0400, Rich Felker wrote:
Pulled from here:
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608a= dd4b02f95322518bd2f5f51ee

Attached fo= r review.

= diff --git a/arch/riscv32/bits/alltypes.h.in b/arch/riscv32/bits/allty= pes.h.in
new file mod= e 100644
index 000000= 0..66ca18a
--- /dev/n= ull
+++ b/arch/riscv3= 2/bits/alltypes.h.in
= @@ -0,0 +1,26 @@
+#de= fine _Addr int
+#defi= ne _Int64 long long
+= #define _Reg int
+
+TYPEDEF __builtin_va_l= ist va_list;
+TYPEDEF= __builtin_va_list __isoc_va_list;
+
+#ifnd= ef __cplusplus
+TYPED= EF int wchar_t;
+#end= if
+
+TYPEDEF float float_t;
+TYPEDEF double double_t;
+
+TYPEDEF struct { long long __ll; long double __= ld; } max_align_t;
+<= /span>
+TYPEDEF long time_t;=

Is riscv32 time_t really 32-b= it? If so that's really disappointing,
but presumably unfixa= ble...

This definitely is fixable as t= he riscv32 Linux ABI is not final. This is the first time a Linux libc ABI expert has looked c= losely at the musl port, and while riscv32 is present, all of my own testing= has been performed on riscv64 Linux.

The RISC-V Glibc port c= urrently only contains riscv64. The riscv32 ABI is still in the process of b= eing finalised. Palmer likely knows the exact status. SiFive doesn=E2=80=99t have any 32-bit c= ores with MMUs so it hasn=E2=80=99t been high on the list of priorities.

In any case, now is a very good time to do some= cross-checking between musl riscv32, qemu-riscv32, the glibc riscv32 port a= nd riscv32 linux-kernel. I think we are just getting the default asm-generic= in QEMU as we have not done anything special... yet...

=
I believe there is still an alignment issue with the stat structure in q= emu-riscv32. When we last looked at this issue earlier in the year we didn=E2= =80=99t have linux-kernel support for riscv32, as the toolchain/ABI foc= us this year was on finalising riscv64 in Glibc and Linux.

Debian and Fedora only have riscv64 ports and afaik riscv64 is all t= hat is present in Glibc. Of course linux-kernel is authoritative for the ABI= so we need to run tests using riscv32 Linux in full system emulation in qem= u-system-riscv32; qemu-riscv32 also needs to be audited. At the time we were= submitting qemu upstream we could build riscv32 kernels. This has solved an= d we need to get regular build and test for riscv32 and riscv64 Linux and QE= MU in CI...

There is an open bug on riscv-qemu stat= which we can look at now that linux-kernel has initial riscv32 support. We s= hould write a= test that prints offsetof and sizeof for the stat structure members using m= usl, glibc and linux headers to find out what=E2=80=99s happening...<= /div>

I think there is still some time to nail down any r= emaining issues with riscv32. We definitely want 64-bit time_t and any other= types that should be 64-bit should be audited now. Given the indirection th= rough multiple levels of typedefs it=E2=80=99s not immediately clear without= a bit of analysis.

We may do the same thing as was= done with glibc and split the port up, first submitting riscv64, which is l= ocked down.

I also had a question regarding whether= we needed to #define __ARCH_WANT_STAT64 in lin= ux/arch/riscv/include/asm/unistd.h or whether this is a legacy= flag. All of the other 32-bit ports define it, but I do not know if it is n= ecessary for a legacy free 32-bit port:


BTW Thanks very much for the review... I=E2=80=99ll go throu= gh the remainder of the patch review comments and work on another revision..= . __riscv_xlen contains either 32 or 64 at present and is how RISC-V code di= stinguishes between riscv32 and riscv64. We can remove references to it from= the musl riscv ports due to them not sharing code. Also, there is no offici= al RISC-V big endian so we can remove EB. Big endian may be considered at so= me point, and it may well have appeared in a previous draft of the ISA manua= l, but at present, it is not defined in the ISA manual or toolchain, so we=E2= =80=99ll remove it.

We=E2=80=99ll do some 32-bit testing.= .. now is a good time. SiFive=E2=80=99s Linux board, the HiFive Unleashed, i= s riscv64, which likely explains the situation with riscv32.

Thanks,
Michael
= --Apple-Mail-3D4CE5B4-1398-408C-B478-A2234A8CDEA7--