From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id 80BFF382CB9C for ; Fri, 9 Dec 2022 03:39:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 80BFF382CB9C 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-x632.google.com with SMTP id jl24so3568116plb.8 for ; Thu, 08 Dec 2022 19:39:50 -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:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=fY6wVAUNUKnxoTPuCZQR8ZcbG7No/qHH6uTZwkQawWQ=; b=wwOS1rX1OZQHLCJU0SRZKe/xDPa2HtWo4sq9r2dOqCGQlaBosK5f3FK9+pN31JyfoG VxGRtbNch5O5ALWr7XXwh8AeWfW2jMFhq2HT78JFKxxtSkwXmh+nHsSFe8KTAkXkTxrJ 4s7tL7ncbrcpS9gIgMlnZjVFUwgwF5SOC5EhrAFLzdPedlWs1L3P6ua5TvMNjiHdYpo0 VDUQu6hlCFhBYTusQGPM1kywKooV4ANeePsDvVivVQDTDAtJKhDZmderkl/CAaLRHKph /K0Uwekim+gtRRz9B8wtGh7s8EKt7otoEz4HtxiHo1GrpCbNt8LX73jco/6FkWVZf4VK /MVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fY6wVAUNUKnxoTPuCZQR8ZcbG7No/qHH6uTZwkQawWQ=; b=ISPrCWeMSx7+q7H5SfPLnKwt/W6K31o+BWxXNZ/DekmIxJ5D0hkgt+1T7Vl1I2AXme PJVsdcdFx4np9OZ1IUcugBNSBzQi3Xsr/QzPNFTUCxF96sUaDSyF2+ihaVFvh61t6gkE z9VBQz/idz5ojoDhdAPPm9J9offgMAg3hezg90KOXyY7NTsSZOWZV+CVNI3KM5jkmtgU t3WFijcPeIaMpzRhdHGmhFyyKk7yewAnrDQSxySDPX6VXsXA0U5GPPvm5Vxd1aVLi+Y+ W6zQJMKg3Kz+OWlXx55myje5cNwiHEhY6EzpLss+Z5vLcwHYc1RR7RtSvGej1ncUIsZy rmmw== X-Gm-Message-State: ANoB5plK7gEsFS69B0WxZ5rlkWk+KEsufG8UeVLvyPlFP1kBRSFF/Nx4 OYP13yXeaIVIL8kCxyd6koPfAA== X-Google-Smtp-Source: AA0mqf7IXNQoFaceipTWNIBxSmAxraIV2W7A2P6ynPuSMwh4+EU7LVM2x6HT2glN2st1j1lOE0YNkQ== X-Received: by 2002:a17:902:fe18:b0:189:9841:8ffe with SMTP id g24-20020a170902fe1800b0018998418ffemr3970118plj.9.1670557189479; Thu, 08 Dec 2022 19:39:49 -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 q14-20020a170902a3ce00b001897de9bae3sm180595plb.204.2022.12.08.19.39.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Dec 2022 19:39:48 -0800 (PST) Message-ID: Date: Thu, 8 Dec 2022 19:39:31 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 From: Vineet Gupta Subject: 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.) 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==?= 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> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,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: 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