[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH -next v7 2/7] arm64: entry: Refactor the entry and exit for exceptions from EL1
- To: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>
- From: Ada Couprie Diaz <ada.coupriediaz@xxxxxxx>
- Date: Mon, 11 Aug 2025 17:01:57 +0100
- Cc: catalin.marinas@xxxxxxx, will@xxxxxxxxxx, oleg@xxxxxxxxxx, sstabellini@xxxxxxxxxx, mark.rutland@xxxxxxx, puranjay@xxxxxxxxxx, broonie@xxxxxxxxxx, mbenes@xxxxxxx, ryan.roberts@xxxxxxx, akpm@xxxxxxxxxxxxxxxxxxxx, chenl311@xxxxxxxxxxxxxxx, anshuman.khandual@xxxxxxx, kristina.martsenko@xxxxxxx, liaochang1@xxxxxxxxxx, ardb@xxxxxxxxxx, leitao@xxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, Ada Couprie Diaz <ada.coupriediaz@xxxxxxx>
- Delivery-date: Mon, 11 Aug 2025 16:02:18 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 06/08/2025 03:49, Jinjie Ruan wrote:
On 2025/8/5 23:06, Ada Couprie Diaz wrote:
Hi,
On 29/07/2025 02:54, Jinjie Ruan wrote:
[...]
diff --git a/arch/arm64/kernel/entry-common.c
b/arch/arm64/kernel/entry-common.c
index 8e798f46ad28..97e0741abde1 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
[...]
@@ -475,73 +497,81 @@ UNHANDLED(el1t, 64, error)
static void noinstr el1_abort(struct pt_regs *regs, unsigned long esr)
{
unsigned long far = read_sysreg(far_el1);
+ arm64_irqentry_state_t state;
- enter_from_kernel_mode(regs);
+ state = enter_from_kernel_mode(regs);
Nit: There is some inconsistencies with some functions splitting state's
definition
and declaration (like el1_abort here), while some others do it on the
same line
(el1_undef() below for example).
In some cases it is welcome as the entry function is called after some
other work,
but here for example it doesn't seem to be beneficial ?
Both methods can keep the modifications to `enter_from_kernel_mode()` on
the same line as the original code, which will facilitate code review.
I think it is also fine to do it on the same line here, which can reduce
one line code, which method is better may be a matter of personal opinion.
Fair point !
Then, as mentioned previously, I'm happy to leave my Reviewed-By.
local_daif_inherit(regs);
do_mem_abort(far, esr, regs);
local_daif_mask();
- exit_to_kernel_mode(regs);
+ exit_to_kernel_mode(regs, state);
}
static void noinstr el1_pc(struct pt_regs *regs, unsigned long esr)
{
unsigned long far = read_sysreg(far_el1);
+ arm64_irqentry_state_t state;
- enter_from_kernel_mode(regs);
+ state = enter_from_kernel_mode(regs);
local_daif_inherit(regs);
do_sp_pc_abort(far, esr, regs);
local_daif_mask();
- exit_to_kernel_mode(regs);
+ exit_to_kernel_mode(regs, state);
}
static void noinstr el1_undef(struct pt_regs *regs, unsigned long
esr)
{
- enter_from_kernel_mode(regs);
+ arm64_irqentry_state_t state = enter_from_kernel_mode(regs);
+
local_daif_inherit(regs);
do_el1_undef(regs, esr);
local_daif_mask();
- exit_to_kernel_mode(regs);
+ exit_to_kernel_mode(regs, state);
}
[...]
Other than the small nit:
Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@xxxxxxx>
|
