Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH] LoongArch: Fix irq enable in exception handlers
To:     Jinyang He <hejinyang@loongson.cn>,
        Huacai Chen <chenhuacai@kernel.org>,
        WANG Xuerui <kernel@xen0n.name>
References: <20221221074238.6699-1-hejinyang@loongson.cn>
Cc:     loongarch@lists.linux.dev, linux-kernel@vger.kernel.org
From:   Tiezhu Yang <yangtiezhu@loongson.cn>
Message-ID: <c6477e2e-2950-ac27-97c9-38fb0c089c80@loongson.cn>
Date:   Tue, 27 Dec 2022 14:43:35 +0800
User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <20221221074238.6699-1-hejinyang@loongson.cn>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/21/2022 03:42 PM, Jinyang He wrote:
> The interrupt state can be got by regs->csr_prmd. Once previous
> interrupt state is disable, we shouldn't enable interrupt if we
> triggered exception which can be triggered in kernel mode. So
> conditionally enable interrupt. For those do_\exception which
> can not triggered in kernel mode but need enable interrupt, call
> die_if_kernel() firstly. And for do_lsx, do_lasx and do_lbt cannot
> triggered in kernel mode, too.
>
> Signed-off-by: Jinyang He <hejinyang@loongson.cn>
> ---
>  arch/loongarch/kernel/traps.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/arch/loongarch/kernel/traps.c b/arch/loongarch/kernel/traps.c
> index 1ea14f6c18d3..3ac7b32d1e15 100644
> --- a/arch/loongarch/kernel/traps.c
> +++ b/arch/loongarch/kernel/traps.c
> @@ -340,9 +340,9 @@ asmlinkage void noinstr do_fpe(struct pt_regs *regs, unsigned long fcsr)
>
>  	/* Clear FCSR.Cause before enabling interrupts */
>  	write_fcsr(LOONGARCH_FCSR0, fcsr & ~mask_fcsr_x(fcsr));
> -	local_irq_enable();
>
>  	die_if_kernel("FP exception in kernel code", regs);
> +	local_irq_enable();
>
>  	sig = SIGFPE;
>  	fault_addr = (void __user *) regs->csr_era;
> @@ -432,7 +432,8 @@ asmlinkage void noinstr do_bp(struct pt_regs *regs)
>  	unsigned long era = exception_era(regs);
>  	irqentry_state_t state = irqentry_enter(regs);
>
> -	local_irq_enable();
> +	if (regs->csr_prmd & CSR_PRMD_PIE)
> +		local_irq_enable();
>  	current->thread.trap_nr = read_csr_excode();
>  	if (__get_inst(&opcode, (u32 *)era, user))
>  		goto out_sigsegv;
> @@ -514,7 +515,8 @@ asmlinkage void noinstr do_ri(struct pt_regs *regs)
>  	unsigned int __user *era = (unsigned int __user *)exception_era(regs);
>  	irqentry_state_t state = irqentry_enter(regs);
>
> -	local_irq_enable();
> +	if (regs->csr_prmd & CSR_PRMD_PIE)
> +		local_irq_enable();
>  	current->thread.trap_nr = read_csr_excode();
>
>  	if (notify_die(DIE_RI, "RI Fault", regs, 0, current->thread.trap_nr,
> @@ -606,8 +608,8 @@ asmlinkage void noinstr do_fpu(struct pt_regs *regs)
>  {
>  	irqentry_state_t state = irqentry_enter(regs);
>
> -	local_irq_enable();
>  	die_if_kernel("do_fpu invoked from kernel context!", regs);
> +	local_irq_enable();
>  	BUG_ON(is_lsx_enabled());
>  	BUG_ON(is_lasx_enabled());
>
> @@ -623,13 +625,13 @@ asmlinkage void noinstr do_lsx(struct pt_regs *regs)
>  {
>  	irqentry_state_t state = irqentry_enter(regs);
>
> +	die_if_kernel("do_lsx invoked from kernel context!", regs);
>  	local_irq_enable();
>  	if (!cpu_has_lsx) {
>  		force_sig(SIGILL);
>  		goto out;
>  	}
>
> -	die_if_kernel("do_lsx invoked from kernel context!", regs);
>  	BUG_ON(is_lasx_enabled());
>
>  	preempt_disable();
> @@ -645,14 +647,13 @@ asmlinkage void noinstr do_lasx(struct pt_regs *regs)
>  {
>  	irqentry_state_t state = irqentry_enter(regs);
>
> +	die_if_kernel("do_lasx invoked from kernel context!", regs);
>  	local_irq_enable();
>  	if (!cpu_has_lasx) {
>  		force_sig(SIGILL);
>  		goto out;
>  	}
>
> -	die_if_kernel("do_lasx invoked from kernel context!", regs);
> -
>  	preempt_disable();
>  	init_restore_lasx();
>  	preempt_enable();
> @@ -666,6 +667,7 @@ asmlinkage void noinstr do_lbt(struct pt_regs *regs)
>  {
>  	irqentry_state_t state = irqentry_enter(regs);
>
> +	die_if_kernel("do_lbt invoked from kernel context!", regs);
>  	local_irq_enable();
>  	force_sig(SIGILL);
>  	local_irq_disable();
> @@ -677,7 +679,6 @@ asmlinkage void noinstr do_reserved(struct pt_regs *regs)
>  {
>  	irqentry_state_t state = irqentry_enter(regs);
>
> -	local_irq_enable();
>  	/*
>  	 * Game over - no way to handle this if it ever occurs.	Most probably
>  	 * caused by a fatal error after another hardware/software error.
> @@ -685,8 +686,8 @@ asmlinkage void noinstr do_reserved(struct pt_regs *regs)
>  	pr_err("Caught reserved exception %u on pid:%d [%s] - should not happen\n",
>  		read_csr_excode(), current->pid, current->comm);
>  	die_if_kernel("do_reserved exception", regs);
> +	local_irq_enable();
>  	force_sig(SIGUNUSED);
> -
>  	local_irq_disable();
>
>  	irqentry_exit(regs, state);
>

With this patch, the kprobe hang problem can be fixed, it is better
to merge this patch before the kprobe patches, or I can put it as
the first patch when submit the new kprobe series.

Tested-by: Tiezhu Yang <yangtiezhu@loongson.cn>

Thanks,
Tiezhu