From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by sourceware.org (Postfix) with ESMTPS id BC5123858428 for ; Sat, 18 Sep 2021 03:04:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BC5123858428 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sifive.com Received: by mail-pl1-x62f.google.com with SMTP id j14so275400plx.4 for ; Fri, 17 Sep 2021 20:04:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=y8CpYUUSnpjePjVw8eKuWhIToF63b8uB6X5kBIvJ3C4=; b=amvvoRbDcAcCCYLa7H8kaKpjl/oKgGvwQweFSlcHDzb5sNYOSInvodkRg/VJ0g1VDK XC4nT+9HHp17dI8X0M8LTT0aOQ9SX/PxI1oPD0cLg3j1NZ0HUxrFIDC37dVhL5U3y3OV L4r7y7MeyPbplqPs3riA3nA+WGbJF5s5doKh01rVhy7Bfa3CWK63St1uWpbh6zWtEUlW 2cL38Lg7kae9V6xuXcngjctarOD4UUNgY7QMQfQlqy7AiWW9OkpTvWIyi2LiLtn6p61J mEAo/JN7eTCBApCIexKirLlNU2ITiuL2n1DJY9G89IluwXydT1aLrL0tmz7EacYF30BA Tu6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=y8CpYUUSnpjePjVw8eKuWhIToF63b8uB6X5kBIvJ3C4=; b=xJifD4g9G4AFdfx7jeZqIfVhNGWWqUhOKefLZPWuBXglKiWVgNd6Y3KgMvxAoIZNvl JehOJeExDwhb8PKJlHvGsEmuop/NoesyTiuttS962xVyJ6z6dUEgjIUdtja9ElTRbQbW OavWVNAyZ7HRhoND2xCyf14KnK3Xua2iCZBym27Qm6CLsTS28woQy/STxac8T6i8CY4B quo9n3YesHUDsBgcuBL4JR2ZOuzxEYsqjDE/6jURtNfB327XhHFFYoV1kioGFEW1uYNS 5Ib8yJ4ibOOlhSXTDIOOhC4kn0ogbDVdB79Ca0KcXHFw4/+e9B59bv5y1Xotmu7PPENd N/Og== X-Gm-Message-State: AOAM530ZreOML8wMF0oXOhRoKPKbV0yysSgQjLEWd+s5XoHnjpYuxu8T 3OavwGt9mHrmgaQM0I4F+A25VJDfbH2dcSYDPDIoAA== X-Google-Smtp-Source: ABdhPJyKpoh97yb/zC6jYOp210cJVVtnKfn6+5Zf+Gc4WjzVfOw/eKyfO4nUY1xqNZD176wbxCxqCb11OalGOd6+3po= X-Received: by 2002:a17:90b:254:: with SMTP id fz20mr25180994pjb.20.1631934269604; Fri, 17 Sep 2021 20:04:29 -0700 (PDT) MIME-Version: 1.0 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: <87sfy5ndid.fsf@oldenburg.str.redhat.com> From: Vincent Chen Date: Sat, 18 Sep 2021 11:04:18 +0800 Message-ID: Subject: Re: [RFC patch 2/5] RISC-V: Reserve about 5K space in mcontext_t to support future ISA expansion. To: Florian Weimer Cc: Rich Felker , GNU C Library , Andrew Waterman Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 Sep 2021 03:04:32 -0000 On Thu, Sep 16, 2021 at 4:14 PM Florian Weimer wrote: > > * Vincent Chen: > > >> > 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? > >> > 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. > > struct sigcontext is allocated by the kernel, so you can have pointers > in reserved fields to out-of-line start, or after struct sigcontext. > > 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. > I have the same concern as you for reserving a huge space in mcontext_t. If the content of mcontext_t is allowed to be different from the content of sigcontext_t, and it has been confirmed that VCSR should not be saved or restored by the *context function, then there seems to be no need to reserve a space in mcontext to support V extension. I will review it again. Thank you !! > Thanks, > Florian >