This is an automated email from the ASF dual-hosted git repository. archer pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push: new 824dd70617 arch/risc-v/src/mpfs: Move PLIC interrupt enable/disable to mpfs_plic.c and handle pending interrupts 824dd70617 is described below commit 824dd706177444d020ebb20acdc08c294ab0db37 Author: Jukka Laitinen <jukka.laiti...@tii.ae> AuthorDate: Wed Feb 19 10:13:15 2025 +0200 arch/risc-v/src/mpfs: Move PLIC interrupt enable/disable to mpfs_plic.c and handle pending interrupts - Move PLIC interrupt enable and disable functions into mpfs_plic.c - When enabling interrupts, always clear pending interrupt - Remove race conditions between irq enable/disable by adding spinlock - An interrupt may trigger on one hart in the middle of enabling the interrupts - then the interrupt handler might call up_disable_irq. A pending interrupt would trigger immediately when enabling the interrupt source, but this is not expected. The interrupt source is level sensitive, so it should only trigger if the source is active at the time when it is enabled. Not if it was active sometime in the past. Signed-off-by: Jukka Laitinen <jukka.laiti...@tii.ae> --- arch/risc-v/src/mpfs/hardware/mpfs_plic.h | 4 + arch/risc-v/src/mpfs/mpfs_irq.c | 41 +--------- arch/risc-v/src/mpfs/mpfs_plic.c | 122 ++++++++++++++++++++++++++++++ arch/risc-v/src/mpfs/mpfs_plic.h | 26 +++++++ 4 files changed, 155 insertions(+), 38 deletions(-) diff --git a/arch/risc-v/src/mpfs/hardware/mpfs_plic.h b/arch/risc-v/src/mpfs/hardware/mpfs_plic.h index e50e987f93..04fcb4b35c 100644 --- a/arch/risc-v/src/mpfs/hardware/mpfs_plic.h +++ b/arch/risc-v/src/mpfs/hardware/mpfs_plic.h @@ -38,6 +38,10 @@ #define MPFS_PLIC_IP4 (MPFS_PLIC_BASE + 0x001010) #define MPFS_PLIC_I51 (MPFS_PLIC_BASE + 0x001014) +#define MPFS_PLIC_PRIO_DIS 0 +#define MPFS_PLIC_PRIO_MIN 1 +#define MPFS_PLIC_PRIO_MAX 7 + #define MPFS_HART_MIE_OFFSET (0x100) #define MPFS_HART_SIE_OFFSET (0x80) diff --git a/arch/risc-v/src/mpfs/mpfs_irq.c b/arch/risc-v/src/mpfs/mpfs_irq.c index b9a7e4a4eb..01111cef9a 100644 --- a/arch/risc-v/src/mpfs/mpfs_irq.c +++ b/arch/risc-v/src/mpfs/mpfs_irq.c @@ -69,7 +69,7 @@ void up_irqinitialize(void) for (int id = 1; id <= NR_IRQS; id++) { - putreg32(1, MPFS_PLIC_PRIORITY + (4 * id)); + putreg32(MPFS_PLIC_PRIO_MIN, MPFS_PLIC_PRIORITY + (4 * id)); } /* Attach the common interrupt handler */ @@ -103,7 +103,6 @@ void up_irqinitialize(void) void up_disable_irq(int irq) { int extirq = 0; - int i; if (irq == RISCV_IRQ_SOFT) { @@ -125,31 +124,7 @@ void up_disable_irq(int irq) PANIC(); } - /* Disable the irq on all harts */ - - for (i = 0; i < CONFIG_SMP_NCPUS; i++) - { - uintptr_t iebase = mpfs_plic_get_iebase(riscv_cpuid_to_hartid(i)); - uintptr_t claim_address = - mpfs_plic_get_claimbase(riscv_cpuid_to_hartid(i)); - - /* Clear any already claimed IRQ (this must be done BEFORE - * disabling the interrupt source): - * - * To signal the completion of executing an interrupt handler, the - * processor core writes the received interrupt ID to the - * Claim/Complete register. The PLIC does not check whether the - * completion ID is the same as the last claim ID for that target. - * If the completion ID does not match an interrupt source that is - * currently enabled for the target, the completion is ignored. - */ - - putreg32(extirq, claim_address); - - /* Clear enable bit for the irq */ - - modifyreg32(iebase + (4 * (extirq / 32)), 1 << (extirq % 32), 0); - } + mpfs_plic_disable_irq(extirq); } } @@ -164,7 +139,6 @@ void up_disable_irq(int irq) void up_enable_irq(int irq) { int extirq; - int i; if (irq == RISCV_IRQ_SOFT) { @@ -186,16 +160,7 @@ void up_enable_irq(int irq) PANIC(); } - /* Enable the irq on all harts */ - - for (i = 0; i < CONFIG_SMP_NCPUS; i++) - { - uintptr_t iebase = mpfs_plic_get_iebase(riscv_cpuid_to_hartid(i)); - - /* Set enable bit for the irq */ - - modifyreg32(iebase + (4 * (extirq / 32)), 0, 1 << (extirq % 32)); - } + mpfs_plic_clear_and_enable_irq(extirq); } } diff --git a/arch/risc-v/src/mpfs/mpfs_plic.c b/arch/risc-v/src/mpfs/mpfs_plic.c index 5dc9733a25..2e984596df 100644 --- a/arch/risc-v/src/mpfs/mpfs_plic.c +++ b/arch/risc-v/src/mpfs/mpfs_plic.c @@ -30,6 +30,7 @@ #include <stdio.h> #include <assert.h> #include <debug.h> +#include <nuttx/spinlock.h> #include <nuttx/arch.h> #include <arch/irq.h> @@ -53,6 +54,12 @@ # define MPFS_PLIC_THRESHOLDPRIV_OFFSET (0) #endif +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static volatile spinlock_t g_enable_lock = SP_UNLOCKED; + /**************************************************************************** * Private Functions ****************************************************************************/ @@ -215,3 +222,118 @@ uintptr_t mpfs_plic_get_thresholdbase(void) { return get_thresholdbase(up_cpu_index()); } + +/**************************************************************************** + * Name: mpfs_plic_disable_irq(int extirq) + * + * Description: + * Disable interrupt on all harts + * + * Returned Value: + * None + * + ****************************************************************************/ + +void mpfs_plic_disable_irq(int extirq) +{ + uintptr_t iebase; + uintptr_t claim_address; + int i; + + irqstate_t flags = spin_lock_irqsave(&g_enable_lock); + + /* Disable the irq on all harts */ + + for (i = 0; i < CONFIG_SMP_NCPUS; i++) + { + /* Clear any already claimed IRQ (this must be done BEFORE + * disabling the interrupt source): + * + * To signal the completion of executing an interrupt handler, the + * processor core writes the received interrupt ID to the + * Claim/Complete register. The PLIC does not check whether the + * completion ID is the same as the last claim ID for that target. + * If the completion ID does not match an interrupt source that is + * currently enabled for the target, the completion is ignored. + */ + + claim_address = mpfs_plic_get_claimbase(riscv_cpuid_to_hartid(i)); + putreg32(extirq, claim_address); + + /* Clear enable bit for the irq for every hart */ + + iebase = mpfs_plic_get_iebase(riscv_cpuid_to_hartid(i)); + modifyreg32(iebase + (4 * (extirq / 32)), 1 << (extirq % 32), 0); + } + + spin_unlock_irqrestore(&g_enable_lock, flags); +} + +/**************************************************************************** + * Name: mpfs_plic_clear_and_enable_irq + * + * Description: + * Enable interrupt; if it is pending, clear it first + * + * Assumptions: + * - Irq can only be pending, all the irq sources are disabled + * + * Returned Value: + * None + * + ****************************************************************************/ + +void mpfs_plic_clear_and_enable_irq(int extirq) +{ + int hartid = up_cpu_index(); + uintptr_t claim_address = mpfs_plic_get_claimbase(hartid); + uintptr_t iebase = mpfs_plic_get_iebase(hartid); + uintptr_t pending_address = MPFS_PLIC_IP0 + (4 * (extirq / 32)); + int i; + uint32_t claim; + + irqstate_t flags = spin_lock_irqsave(&g_enable_lock); + + /* Check if the extirq is pending */ + + if ((getreg32(pending_address) & (1 << (extirq % 32))) != 0) + { + /* Interrupt is pending. This means that the source is disabled on + * all harts. Only way to clear it is to claim and ack it; do it on + * this hart + * + * First bump the priority of the irq to highest, to get it to the + * head of the claim queue + */ + + putreg32(MPFS_PLIC_PRIO_MAX, MPFS_PLIC_PRIORITY + (4 * extirq)); + + /* Enable the irq on this hart */ + + modifyreg32(iebase + (4 * (extirq / 32)), 0, 1 << (extirq % 32)); + + /* Now we can claim and ack the pending irq */ + + claim = getreg32(claim_address); + + DEBUGASSERT(claim == extirq); + + putreg32(claim, claim_address); + + /* Return the irq priority to mininum */ + + putreg32(MPFS_PLIC_PRIO_MIN, MPFS_PLIC_PRIORITY + (4 * extirq)); + } + + /* Enable the irq on all harts */ + + for (i = 0; i < CONFIG_SMP_NCPUS; i++) + { + /* Set enable bit for the irq */ + + iebase = mpfs_plic_get_iebase(riscv_cpuid_to_hartid(i)); + modifyreg32(iebase + (4 * (extirq / 32)), 0, 1 << (extirq % 32)); + } + + spin_unlock_irqrestore(&g_enable_lock, flags); +} diff --git a/arch/risc-v/src/mpfs/mpfs_plic.h b/arch/risc-v/src/mpfs/mpfs_plic.h index 00aa6ef368..d6563da37c 100644 --- a/arch/risc-v/src/mpfs/mpfs_plic.h +++ b/arch/risc-v/src/mpfs/mpfs_plic.h @@ -88,4 +88,30 @@ uintptr_t mpfs_plic_get_claimbase(uintptr_t hartid); uintptr_t mpfs_plic_get_thresholdbase(void); +/**************************************************************************** + * Name: mpfs_plic_disable_irq(int extirq) + * + * Description: + * Disable interrupt on all harts + * + * Returned Value: + * None + * + ****************************************************************************/ + +void mpfs_plic_disable_irq(int extirq); + +/**************************************************************************** + * Name: mpfs_plic_clear_and_enable_irq + * + * Description: + * Enable interrupt; if it is pending, clear it first + * + * Returned Value: + * None + * + ****************************************************************************/ + +void mpfs_plic_clear_and_enable_irq(int extirq); + #endif /* __ARCH_RISC_V_SRC_MPFS_MPFS_PLIC_H */