From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id 4406E38AEB75 for ; Fri, 9 Dec 2022 04:03:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4406E38AEB75 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-pl1-x633.google.com with SMTP id s7so3622257plk.5 for ; Thu, 08 Dec 2022 20:03:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:in-reply-to:references:cc:to:from :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=QmLk0KPGD9HjYKcK+FaEaw3ZqkS6q9nQEvDUVOs5KJM=; b=GOZ0Xds/zYwXDgeMnjQvykSqM5wM7DIv5JgIB3TvNtmEfWEhoAXwzBlnk2vXcnG47D Xz6tOlPF5wrz8XOFhZJj2a1JQ9zKd2uJZGoGXC6TJprejEYqMR25ApH5vsvdeKC3bu93 Lungzh1PQrcdDNTuWNUhGvwbR9JIgQqn9Vr8Y/uC1u0VLQS7NLFJ7QjrxYeX1u00FqLg 3ti7xmtJoG+h67AWa2BrRbPne0mkeR7VrzyMHPxKgHNEA9XYupkwUfiaqYR2b6Xz+1V4 mrMUpVv+Y28jIMCL6+DOLcsIYRDJeDyPPL/zDWfC9v7vkLf0Afwc5I4LTaKibKk/5Zqh nChA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:references:cc:to:from :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QmLk0KPGD9HjYKcK+FaEaw3ZqkS6q9nQEvDUVOs5KJM=; b=Axyb0oEPurcLE/g5JKs81V86OCl208bEuiQFmBN4wknm0RFurDMBQxxpA1iN78OPeY mJrCzbGwnoXguQU1GFFEC9CIZNUtQ8x8AIntzefs2F25sOMBfZ2gcor87NYd5xGmENQa 3WMwC7cS3CR5qmkuQMT22RvW+504t6Ng3IFTmxqivUTmLQfT9+Y3wjltWXgFqGSQN7E5 +HMoXovS41KPJFMkXf8eTAhh1ImgF/EE7l1RDhu2iBQFO2ASFWRY+GjYKATZCwkdB+Bj pF5x0Yj8dsrZh4GxPTQF2Go/C1NLt34cJKDXf91sicKT1OJn9BW9ky6cl0C5JHEaJf0v dqrg== X-Gm-Message-State: ANoB5pky/EjhlXjcZQy8l/ForXLGhS6FJF5/MtlESFtcekdZ/rS2jsvV /elBZS9Rsvl+7e7OngydH7IQ0ygKsNfODcFyfJY= X-Google-Smtp-Source: AA0mqf7dcCJ3iSwtP0DFkNuLnvm8m+qZzAneU1VVgzD3lYl6MX59Jwa+eTXYv8aj2nIj83qCqf14OA== X-Received: by 2002:a17:902:e194:b0:189:aab9:cd80 with SMTP id y20-20020a170902e19400b00189aab9cd80mr4338099pla.64.1670558596178; Thu, 08 Dec 2022 20:03:16 -0800 (PST) Received: from [192.168.50.116] (c-24-4-73-83.hsd1.ca.comcast.net. [24.4.73.83]) by smtp.gmail.com with ESMTPSA id i14-20020a170902c94e00b00189618fc2d8sm206902pla.242.2022.12.08.20.03.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Dec 2022 20:03:15 -0800 (PST) Message-ID: <37552b4f-15c0-9747-5c35-d1e4887fbfe8@rivosinc.com> Date: Thu, 8 Dec 2022 20:03:14 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: RISCV kernel struct sigcontext expansion for V regs and potential glibc ABI break (was Re: [RFC patch 2/5] RISC-V: Reserve about 5K space in mcontext_t to support future ISA expansion.) Content-Language: en-US From: Vineet Gupta To: Vincent Chen , Florian Weimer Cc: Rich Felker , GNU C Library , Andrew Waterman , Palmer Dabbelt , Kito Cheng , =?UTF-8?Q?Christoph_M=c3=bcllner?= , davidlt@rivosinc.com, Arnd Bergmann , =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , Greentime Hu , Andy Chiu References: <1631497278-29829-1-git-send-email-vincent.chen@sifive.com> <1631497278-29829-3-git-send-email-vincent.chen@sifive.com> <871r5sd1zq.fsf@oldenburg.str.redhat.com> <20210913135247.GL13220@brightrain.aerifal.cx> <87sfy5ndid.fsf@oldenburg.str.redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=0.5 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: +CC Greentime and Andy On 12/8/22 19:39, Vineet Gupta wrote: > Hi Florian, > > P.S. Since I'm revisiting a year old thread with some new CC > recipients, here's the link to original patch/thread [1] > > On 9/17/21 20:04, Vincent Chen wrote: >> On Thu, Sep 16, 2021 at 4:14 PM Florian Weimer >> wrote: >>>>>> This changes the size of struct ucontext_t, which is an ABI break >>>>>> (getcontext callers are supposed to provide their own object). >>>>>> >>>> The riscv vector registers are all caller-saved registers except for >>>> VCSR. Therefore, the struct mcontext_t needs to reserve a space for >>>> it. In addition, RISCV ISA is growing, so I also hope the struct >>>> mcontext_t has a space for future expansion. Based on the above ideas, >>>> I reserved a 5K space here. >>> You have reserved space in ucontext_t that you could use for this. >>> >> Sorry, I cannot really understand what you mean. The following is the >> contents of ucontext_t >> typedef struct ucontext_t >>    { >>      unsigned long int  __uc_flags; >>      struct ucontext_t *uc_link; >>      stack_t            uc_stack; >>      sigset_t           uc_sigmask; >>      /* There's some padding here to allow sigset_t to be expanded in >> the >>         future.  Though this is unlikely, other architectures put >> uc_sigmask >>         at the end of this structure and explicitly state it can be >>         expanded, so we didn't want to box ourselves in here. */ >>      char               __glibc_reserved[1024 / 8 - sizeof (sigset_t)]; >>      /* We can't put uc_sigmask at the end of this structure because >> we need >>         to be able to expand sigcontext in the future.  For example, the >>         vector ISA extension will almost certainly add ISA state.  We >> want >>         to ensure all user-visible ISA state can be saved and >> restored via a >>         ucontext, so we're putting this at the end in order to allow for >>         infinite extensibility.  Since we know this will be extended >> and we >>         assume sigset_t won't be extended an extreme amount, we're >>         prioritizing this.  */ >>      mcontext_t uc_mcontext; >>    } ucontext_t; >> >> Currently, we only reserve a space, __glibc_reserved[], for the future >> expansion of sigset_t. >> Do you mean I could use __glibc_reserved[] to for future expansion of >> ISA as well? > > Given unlikely sigset expansion, we could in theory use some of those > reserved fields to store pointers (offsets) to actual V state, but not > for actual V state which is way too large for non-embedded machines > with typical 128 or even wider V regs. > > >> >>>>>> This shouldn't be necessary if the additional vector registers are >>>>>> caller-saved. >>>> Here I am a little confused about the usage of struct mcontext_t. As >>>> far as I know, the struct mcontext_t is used to save the >>>> machine-specific information in user context operation. Therefore, in >>>> this case, the struct mcontext_t is allowed to reserve the space only >>>> for saving caller-saved registers. However, in the signal handler, the >>>> user seems to be allowed to use uc_mcontext whose data type is struct >>>> mcontext_t to access the content of the signal context. In this case, >>>> the struct mcontext_t may need to be the same as the struct sigcontext >>>> defined at kernel. However, it will have a conflict with your >>>> suggestion because the struct sigcontext cannot just reserve a space >>>> for saving caller-saved registers. Could you help me point out my >>>> misunderstanding? Thank you. > > I think the confusion comes from apparent equivalence of kernel struct > sigcontext and glibc mcontext_t as they appear in respective struct > ucontext definitions. > I've enumerated the actual RV structs below to keep them handy in one > place for discussion. > >>> struct sigcontext is allocated by the kernel, so you can have pointers >>> in reserved fields to out-of-line start, or after struct sigcontext. > > In this scheme, would the actual V regfile contents (at the > out-of-line location w.r.t kernel sigcontext) be anonymous for glibc > i.e. do we not need to expose them to glibc userspace ABI ? > > >>> I don't know how the kernel implements this, but there is considerable >>> flexibility and extensibility.  The main issues comes from small stacks >>> which are incompatible with large register files. > > Simplistically, Linux kernel needs to preserve the V regfile across > task switch. The necessary evil that follows is preserving V across > signal-handling (sigaction/sigreturn). > > In RV kernel we have following: > > struct rt_sigframe { >   struct siginfo info; >   struct ucontext uc; > }; > > struct ucontext { >    unsigned long uc_flags; >    struct ucontext *uc_link; >    stack_t uc_stack; >    sigset_t uc_sigmask; >    __u8 __unused[1024 / 8 - sizeof(sigset_t)];     // this is for > sigset_t expansion >    struct sigcontext uc_mcontext; > }; > > struct sigcontext { >    struct user_regs_struct sc_regs; >    union __riscv_fp_state sc_fpregs; > +  __u8 sc_extn[4096+128] __attribute__((__aligned__(16)));   // > handle 128B V regs > }; > > The sc_extn[] would have V state (regfile + control state) in kernel > defined format. > > As I understand it, you are suggesting to prevent ABI break, we should > not add anything to kernel struct sigcontext i.e. do something like this > > struct rt_sigframe { >   struct siginfo info; >   struct ucontext uc; > +__u8 sc_extn[4096+128] __attribute__((__aligned__(16))); > } > > So kernel sig handling can continue to save/restore the V regfile on > user stack, w/o making it part of actual struct sigcontext. > So they are not explicitly visible to userspace at all - is that > feasible ? I know that SA_SIGINFO handlers can access the scalar/fp > regs, they won't do it V. > Is there a POSIX req for SA_SIGINFO handlers being able to access all > machine regs saved by signal handling. > > An alternate approach is what Vincent did originally, to add sc_exn to > struct sigcontext. Here to prevent ABI breakage, we can choose to not > reflect this in the glibc sigcontext. But the question remains, is > that OK ? > > The other topic is changing glibc mcontext_t to add V-regs. It would > seem one has to as mcontext is "visually equivalent" to struct > sigcontext in the respective ucontext structs. But in unserspace > *context routine semantics only require callee-regs to be saved, which > V regs are not per psABI [2]. So looks like this can be avoided which > is what Vincent did in v2 series [3] > > > [1] > https://sourceware.org/pipermail/libc-alpha/2021-September/130899.html > [2] > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc > [3] https://sourceware.org/pipermail/libc-alpha/2022-January/135416.html