Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753884AbdGLRJ7 (ORCPT ); Wed, 12 Jul 2017 13:09:59 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:52632 "EHLO imgpgp01.kl.imgtec.org" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753510AbdGLRJ6 (ORCPT ); Wed, 12 Jul 2017 13:09:58 -0400 X-PGP-Universal: processed; by imgpgp01.kl.imgtec.org on Wed, 12 Jul 2017 19:20:38 +0100 Date: Wed, 12 Jul 2017 18:09:55 +0100 From: James Hogan To: Palmer Dabbelt CC: Olof Johansson , Arnd Bergmann , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH 16/17] RISC-V: User-facing API Message-ID: <20170712170955.GU6973@jhogan-linux.le.imgtec.org> References: <20170712110751.GS6973@jhogan-linux.le.imgtec.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mrGfhPylyqsubxLB" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [192.168.154.110] X-ESG-ENCRYPT-TAG: 1b7d744b Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4385 Lines: 100 --mrGfhPylyqsubxLB Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 12, 2017 at 09:24:24AM -0700, Palmer Dabbelt wrote: > On Wed, 12 Jul 2017 04:07:51 PDT (-0700), james.hogan@imgtec.com wrote: > > On Tue, Jul 11, 2017 at 06:31:29PM -0700, Palmer Dabbelt wrote: > >> diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c > >> new file mode 100644 > >> index 000000000000..e0a1b89583ef > >> --- /dev/null > >> +++ b/arch/riscv/kernel/signal.c > >> @@ -0,0 +1,289 @@ > > > >> +static long setup_sigcontext(struct rt_sigframe __user *frame, > >> + struct pt_regs *regs) > >> +{ > >> + struct sigcontext __user *sc =3D &frame->uc.uc_mcontext; > >> + long err; > >> + size_t i; > >> + /* sc_regs is structured the same as the start of pt_regs */ > >> + err =3D __copy_to_user(&sc->sc_regs, regs, sizeof(sc->sc_regs)); > >> + /* Save the floating-point state. */ > >> + err |=3D save_d_state(regs, &sc->sc_fpregs.d); > >> + /* We support no other extension state at this time. */ > >> + for (i =3D 0; i < ARRAY_SIZE(sc->sc_fpregs.q.reserved); i++) > >> + err |=3D __put_user(0, &sc->sc_fpregs.q.reserved[i]); > > > > How should userland determine how to interpret sc_fpregs? It looks like > > you couldn't add f or q state without using one of these reserved > > fields, so why not just specify a field up front to say which fp format > > (if any) to interpret? >=20 > We considered that, but didn't want to tie ourserves to an extension mech= anism > right now because we don't know what the vector extension is going to look > like. >=20 > > That would allow userland wanting to interpret it to safely check that > > field in a forward and backward compatible way without assuming a > > specific format is in use. >=20 > We set ELF_HWCAP (which percolates to userspace via the auxvec. This con= tains > the entire set of extensions the kernel supports on the current machine, = which > allows userspace to figure out what the format of the floating-point stat= e is. But then (as far as I understand it) software written now could break once support for that extension is made available and the format suddenly changes (or to avoid that breakage you may need to split up vector values, which is not what the current union describes). Wouldn't it be better to define it now in such a way that you hopefully don't need to worry about such ABI breakage in future? E.g. does it make sense to have the fp state as an fcsr and an array of 32 unions, each of which can contain a 32bit, 64-bit, or 128-bit quantity. That assumes the vector state aliases the FP state, such that an FP program on a kernel with vector extensions continues to work, but a program using vector extensions can use the same sigcontext sensibly. Thats how the MIPS SIMD Architecture (MSA) would ideally have worked, but there wasn't space in the fpregs fields, so the upper 64-bits of each vector register needed to be added separately in the sigcontext as an extension, but the lower 64-bits (aliasing FP state) remaining in the fpregs array. Alternatively if even larger vector extensions are expected it might make sense to abstract further and specify the stride between fp registers as another field so it can be made larger in future without breaking software that properly uses the stride, but admitedly that adds complexity. Cheers James --mrGfhPylyqsubxLB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEd80NauSabkiESfLYbAtpk944dnoFAllmV9kACgkQbAtpk944 dnpM7BAAi9++HdjFUWyNHmH0fMJ0wg/3CwrrKEPSP0NUI9heRIRecKCQGSuTsBfr UVf+u7OwZGIPDYDCoOAjiYRrUgbumVoxihoLbS42zHwiEaP0VVvO0cZzXdQEZtyu +u9H+CpJvUf7ygAj0s1AigfiWO+dcA5+9hfNWpsHA7GI/A0gspiZnaNWfA0BvzU/ PRI54cMLxt0gmHQ4iavx5VQai6GrvQYdt+Ni1YLQXjEdpls5Pcsp1RzjcRtZuEKs Q8ufJUw+FM+4/77N/lRpgRfNHOeimp5GhSYxPsWK9Mkk9KBUv5DIxchsheQQu1Je RklaKrt9Jp5hCmY0XH9XbezO+fHvYDwtr8ZrTrgbQG1QVGLJwg1QvF1tf+qpC4AO SLWSW1Ofem5muRPFeUwXuQoYMtY3sY4ILi0jdn9ZeRFGGZohpbYhevoFZ8GIjFFM lzj3bHf1v5ocz0XL7I/9uT4gjI3aNaI26qVRFIAL83QBSxFZfp+saYl2UXg3KiRp jDx3EvcmZPYYle7uUQJVnfgH0nHmRMTIFCxjabOV8hzavPCkdMd+ot9yJMNCy/Cu Bjo7ETMAciam9JByXjGAtxOGqsietew9Z0nc7Wwgs7Nei6yAtZiFsLTVPoLe2AfP 1REtI3ndByhPuxg5DLq/2joRz4tkLUrrSa8RcCRtnq53Z3ejkUg= =yf+H -----END PGP SIGNATURE----- --mrGfhPylyqsubxLB--