Re: [RFC PATCH v8 11/21] riscv: Add sigcontext save/restore for vector

From: Ley Foon Tan <lftan.linux@gmail.com>
To: Greentime Hu <greentime.hu@sifive.com>
Cc: linux-riscv@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	aou@eecs.berkeley.edu, Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	vincent.chen@sifive.com
Subject: Re: [RFC PATCH v8 11/21] riscv: Add sigcontext save/restore for vector
Date: Thu, 30 Sep 2021 10:37:39 +0800	[thread overview]
Message-ID: <CAFiDJ5_Lp8ejFiNpZw=YvneyEFRj-PD9667Jv_hHy_-7xGf+FQ@mail.gmail.com> (raw)
In-Reply-To: <82fe07af09d223b33c8d4b8986939bcc0d7180a1.1631121222.git.greentime.hu@sifive.com>

On Thu, Sep 9, 2021 at 1:49 AM Greentime Hu <greentime.hu@sifive.com> wrote:
>
> This patch adds sigcontext save/restore for vector. The vector registers
> will be saved in datap pointer. The datap pointer will be allocated
> dynamically when the task needs in kernel space. The datap pointer will
> be set right after the __riscv_v_state data structure to save all the
> vector registers in the signal handler stack.
>
> Co-developed-by: Vincent Chen <vincent.chen@sifive.com>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> ---
>  arch/riscv/include/uapi/asm/sigcontext.h |  24 ++++
>  arch/riscv/kernel/asm-offsets.c          |   2 +
>  arch/riscv/kernel/setup.c                |   4 +
>  arch/riscv/kernel/signal.c               | 164 ++++++++++++++++++++++-
>  4 files changed, 190 insertions(+), 4 deletions(-)
>

[....]

> +
> +static size_t cal_rt_frame_size(void)
> +{
> +       struct rt_sigframe __user *frame;
> +       static size_t frame_size;
> +       size_t total_context_size = 0;
> +       size_t sc_reserved_size = sizeof(frame->uc.uc_mcontext.__reserved);
> +
> +       if (frame_size)
> +               goto done;
> +
> +       frame_size = sizeof(*frame);
> +
> +       if (has_vector)
> +               total_context_size += rvv_sc_size;
> +       /* Preserved a __riscv_ctx_hdr for END signal context header. */
> +       total_context_size += sizeof(struct __riscv_ctx_hdr);
> +
> +       if (total_context_size > sc_reserved_size)
> +               frame_size += (total_context_size - sc_reserved_size);
> +
> +done:
> +       return round_up(frame_size, 16);

Hi Greentime,

frame_size is static variable here, so it will preserve the value for
the next calling to cal_rt_frame_size().

I think we need update frame_size before return, example:

frame_size =  round_up(frame_size, 16);
return frame_size;

Regards
Ley Foon