Re: [v2, 5/5] riscv: vector: allow kernel-mode Vector with preemption

["Björn Töpel" <bjorn@kernel.org>]

Andy Chiu <andy.chiu@sifive.com> writes:

> Add kernel_vstate to keep track of kernel-mode Vector registers when
> trap introduced context switch happens. Also, provide trap_pt_regs to
> let context save/restore routine reference status.VS at which the trap
> takes place. The thread flag TIF_RISCV_V_KERNEL_MODE indicates whether
> a task is running in kernel-mode Vector with preemption 'ON'. So context
> switch routines know and would save V-regs to kernel_vstate and restore
> V-regs immediately from kernel_vstate if the bit is set.
>
> Apart from a task's preemption status, the capability of
> running preemptive kernel-mode Vector is jointly controlled by the
> RISCV_V_VSTATE_CTRL_PREEMPTIBLE mask in the task's
> thread.vstate_ctrl. This bit is masked whenever a trap takes place in
> kernel mode while executing preemptive Vector code.
>
> Also, provide a config CONFIG_RISCV_ISA_V_PREEMPTIVE to give users an
> option to disable preemptible kernel-mode Vector at build time. Users
> with constraint memory may want to disable this config as preemptible
> kernel-mode Vector needs extra space for tracking per thread's
> kernel-mode V context. Or, users might as well want to disable it if all
> kernel-mode Vector code is time sensitive and cannot tolerate context
> swicth overhead.

Nice idea! Did you perform any benchmarking? Would be really interesting
to get some numbers.

Nit: "switch"

I like that the most "controversial" patch is last, so it can easily be
dropped if the discussions doesn't settle! It would be nice with kernel
vector support in 6.6!

> diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
[...]
> @@ -70,11 +109,14 @@ void kernel_vector_begin(void)
>  
>  	riscv_v_vstate_save(&current->thread.vstate, task_pt_regs(current));
>  
> -	get_cpu_vector_context();
> +	if (!preemptible() || !kernel_vector_preemptible()) {
> +		get_cpu_vector_context();
> +	} else {
> +		if (riscv_v_start_kernel_context())
> +			get_cpu_vector_context();
> +	}

Wdyt about replacing this with:
        if (!preemptible() || !kernel_vector_preemptible() || riscv_v_start_kernel_context())
                get_cpu_vector_context();

Björn

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv