From 29cf2eb342bb3b0787afd35c5f5c9eb6bbb5b079 Mon Sep 17 00:00:00 2001 From: Frank Benkert Date: Fri, 23 Dec 2016 11:45:21 -0600 Subject: [PATCH 1/2] AMV7: CAN: Make delete_filter functions more robust --- arch/arm/src/samv7/sam_mcan.c | 50 ++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/arch/arm/src/samv7/sam_mcan.c b/arch/arm/src/samv7/sam_mcan.c index f6c53306bb..683463be9e 100644 --- a/arch/arm/src/samv7/sam_mcan.c +++ b/arch/arm/src/samv7/sam_mcan.c @@ -1907,16 +1907,35 @@ static int mcan_del_extfilter(FAR struct sam_mcan_s *priv, int ndx) DEBUGASSERT(priv != NULL && priv->config != NULL); config = priv->config; - DEBUGASSERT(ndx < config->nextfilters); + + /* Check user Parameters */ + + DEBUGASSERT(ndx >= 0 || ndx < config->nextfilters); + + if (ndx < 0 || ndx >= config->nextfilters) + { + return -EINVAL; + } /* Get exclusive excess to the MCAN hardware */ mcan_dev_lock(priv); - /* Release the filter */ - word = ndx >> 5; bit = ndx & 0x1f; + + /* Check if this filter is really assigned */ + + if ((priv->extfilters[word] & (1 << bit)) == 0) + { + /* No, error out */ + + mcan_dev_unlock(priv); + return -ENOENT; + } + + /* Release the filter */ + priv->extfilters[word] &= ~(1 << bit); DEBUGASSERT(priv->nextalloc > 0); @@ -2137,16 +2156,35 @@ static int mcan_del_stdfilter(FAR struct sam_mcan_s *priv, int ndx) DEBUGASSERT(priv != NULL && priv->config != NULL); config = priv->config; - DEBUGASSERT(ndx < config->nstdfilters); + + /* Check Userspace Parameters */ + + DEBUGASSERT(ndx >= 0 || ndx < config->nstdfilters); + + if (ndx < 0 || ndx >= config->nstdfilters) + { + return -EINVAL; + } /* Get exclusive excess to the MCAN hardware */ mcan_dev_lock(priv); - /* Release the filter */ - word = ndx >> 5; bit = ndx & 0x1f; + + /* Check if this filter is really assigned */ + + if ((priv->stdfilters[word] & (1 << bit)) == 0) + { + /* No, error out */ + + mcan_dev_unlock(priv); + return -ENOENT; + } + + /* Release the filter */ + priv->stdfilters[word] &= ~(1 << bit); DEBUGASSERT(priv->nstdalloc > 0); From f3d755c16f39e98b83389a48a40985792204fb1b Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Fri, 23 Dec 2016 13:04:33 -0600 Subject: [PATCH 2/2] Some trivial, cosmetic changes for irqlock branch --- arch/arm/src/armv7-a/arm_fullcontextrestore.S | 17 ++++++++++++----- arch/arm/src/armv7-a/arm_testset.S | 6 ++++-- arch/arm/src/armv7-m/up_doirq.c | 19 ++----------------- arch/sim/include/spinlock.h | 1 + arch/sim/src/up_internal.h | 2 +- sched/irq/irq_csection.c | 2 +- 6 files changed, 21 insertions(+), 26 deletions(-) diff --git a/arch/arm/src/armv7-a/arm_fullcontextrestore.S b/arch/arm/src/armv7-a/arm_fullcontextrestore.S index 64f74c8a98..ff9fc71e6b 100644 --- a/arch/arm/src/armv7-a/arm_fullcontextrestore.S +++ b/arch/arm/src/armv7-a/arm_fullcontextrestore.S @@ -77,6 +77,7 @@ up_fullcontextrestore: */ #ifdef CONFIG_ARCH_FPU + /* First, restore the floating point registers. Lets do this before we * restore the ARM registers so that we have plenty of registers to * work with. @@ -96,9 +97,11 @@ up_fullcontextrestore: ldr r2, [r1], #4 /* Fetch the floating point control and status register */ vmsr fpscr, r2 /* Restore the FPCSR */ + #endif #ifdef CONFIG_BUILD_KERNEL + /* For the kernel build, we need to be able to transition gracefully * between kernel- and user-mode tasks. Here we do that with a system * call; the system call will execute in kernel mode and but can return @@ -116,6 +119,7 @@ up_fullcontextrestore: bx lr /* Unnecessary ... will not return */ #else + /* For a flat build, we can do all of this here... Just think of this as * a longjmp() all on steriods. */ @@ -129,11 +133,15 @@ up_fullcontextrestore: sub sp, sp, #(3*4) /* Frame for three registers */ ldr r1, [r0, #(4*REG_R0)] /* Fetch the stored r0 value */ - str r1, [sp] /* Save it at the top of the stack */ + str r1, [sp, #(4*0)] /* Save it at the top of the stack */ ldr r1, [r0, #(4*REG_R1)] /* Fetch the stored r1 value */ - str r1, [sp, #4] /* Save it in the stack */ + str r1, [sp, #(4*1)] /* Save it in the stack */ ldr r1, [r0, #(4*REG_PC)] /* Fetch the stored pc value */ - str r1, [sp, #8] /* Save it at the bottom of the frame */ + str r1, [sp, #(4*2)] /* Save it at the bottom of the frame */ + + /* Recover the saved CPSR value in r1 */ + + ldr r1, [r0, #(4*REG_CPSR)] /* Fetch the stored CPSR value */ /* Now we can restore the CPSR. We wait until we are completely * finished with the context save data to do this. Restore the CPSR @@ -142,14 +150,13 @@ up_fullcontextrestore: * disabled. */ - ldr r1, [r0, #(4*REG_CPSR)] /* Fetch the stored CPSR value */ msr cpsr, r1 /* Set the CPSR */ /* Now recover r0 and r1 */ ldr r0, [sp] ldr r1, [sp, #4] - add sp, sp, #(2*4) + add sp, sp, #(4*2) /* Then return to the address at the stop of the stack, * destroying the stack frame diff --git a/arch/arm/src/armv7-a/arm_testset.S b/arch/arm/src/armv7-a/arm_testset.S index e89cbb3adc..638736e4d6 100644 --- a/arch/arm/src/armv7-a/arm_testset.S +++ b/arch/arm/src/armv7-a/arm_testset.S @@ -75,7 +75,7 @@ * This function must be provided via the architecture-specific logoic. * * Input Parameters: - * lock - The address of spinlock object. + * lock - The address of spinlock object (r0). * * Returned Value: * The spinlock is always locked upon return. The value of previous value @@ -84,6 +84,8 @@ * obtain the lock) or SP_UNLOCKED if the spinlock was previously unlocked * (meaning that we successfully obtained the lock) * + * Modifies: r1, r2, and lr + * ****************************************************************************/ .globl up_testset @@ -98,7 +100,7 @@ up_testset: 1: ldrexb r2, [r0] /* Test if spinlock is locked or not */ cmp r2, r1 /* Already locked? */ - beq 2f /* If alrady locked, return SP_LOCKED */ + beq 2f /* If already locked, return SP_LOCKED */ /* Not locked ... attempt to lock it */ diff --git a/arch/arm/src/armv7-m/up_doirq.c b/arch/arm/src/armv7-m/up_doirq.c index b51c10e558..460295b948 100644 --- a/arch/arm/src/armv7-m/up_doirq.c +++ b/arch/arm/src/armv7-m/up_doirq.c @@ -50,22 +50,6 @@ #include "up_arch.h" #include "up_internal.h" -/**************************************************************************** - * Pre-processor Definitions - ****************************************************************************/ - -/**************************************************************************** - * Public Data - ****************************************************************************/ - -/**************************************************************************** - * Private Data - ****************************************************************************/ - -/**************************************************************************** - * Private Functions - ****************************************************************************/ - /**************************************************************************** * Public Functions ****************************************************************************/ @@ -111,11 +95,12 @@ uint32_t *up_doirq(int irq, uint32_t *regs) /* Restore the previous value of CURRENT_REGS. NULL would indicate that * we are no longer in an interrupt handler. It will be non-NULL if we - * are returning from a nested interrupt. + * are returning from a nested interrupt (which are NOT fully supported). */ CURRENT_REGS = savestate; #endif + board_autoled_off(LED_INIRQ); return regs; } diff --git a/arch/sim/include/spinlock.h b/arch/sim/include/spinlock.h index 350bf24b33..85333276b1 100644 --- a/arch/sim/include/spinlock.h +++ b/arch/sim/include/spinlock.h @@ -46,6 +46,7 @@ /**************************************************************************** * Pre-processor Definitions ****************************************************************************/ + /* Must match definitions in up_testset.c */ #define SP_UNLOCKED false /* The Un-locked state */ diff --git a/arch/sim/src/up_internal.h b/arch/sim/src/up_internal.h index 53f4e2cd42..7ca32a479e 100644 --- a/arch/sim/src/up_internal.h +++ b/arch/sim/src/up_internal.h @@ -52,7 +52,7 @@ # include # ifdef CONFIG_SMP # include -# include +# include # endif #endif diff --git a/sched/irq/irq_csection.c b/sched/irq/irq_csection.c index 935cbfd3af..c9f9f91c18 100644 --- a/sched/irq/irq_csection.c +++ b/sched/irq/irq_csection.c @@ -570,4 +570,4 @@ void leave_critical_section(irqstate_t flags) } #endif -#endif /* CONFIG_SMP || CONFIG_SCHED_INSTRUMENTATION_CSECTION*/ +#endif /* CONFIG_SMP || CONFIG_SCHED_INSTRUMENTATION_CSECTION */