[PATCH] Strict thread-stack isolation
Utkarsh Rai
utkarsh.rai60 at gmail.com
Tue Jul 14 16:59:02 UTC 2020
On Tue, Jul 14, 2020 at 7:36 PM Gedare Bloom <gedare at rtems.org> wrote:
> I won't comment on the namespace and coding conventions, other than to
> say you should focus on doing them correctly as you code, rather than
> going back and fixing them later. More below.
>
> On Mon, Jul 13, 2020 at 10:34 AM Utkarsh Rai <utkarsh.rai60 at gmail.com>
> wrote:
> >
> > - This is the complete set of changes for strict isolation of thread
> stacks.
> > - There needs to be a confiuration operation,(#if
> defined(USE_THREAD_STACK_PROTECTION) for simple configuration can be used)
> > - The stack attributes are allocated through malloc, this needs to be
> done through score unlimited objects.
> > ---
> > bsps/arm/headers.am | 1 +
> > .../include/bsp/arm-cp15-set-ttb-entries.h | 7 +
> > .../shared/cp15/arm-cp15-set-ttb-entries.c | 3 +
> > bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c | 72 +++++++++
> > bsps/shared/start/stackalloc.c | 20 ++-
> > c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am | 5 +-
> > cpukit/Makefile.am | 1 +
> > cpukit/headers.am | 2 +
> > cpukit/include/rtems/score/memorymanagement.h | 22 +++
> > cpukit/include/rtems/score/stackmanagement.h | 49 ++++++
> > cpukit/score/cpu/arm/cpu.c | 3 +
> > cpukit/score/cpu/arm/cpu_asm.S | 22 ++-
> > .../score/cpu/arm/include/rtems/score/cpu.h | 20 +++
> > cpukit/score/src/stackmanagement.c | 143 ++++++++++++++++++
> > 14 files changed, 365 insertions(+), 5 deletions(-)
> > create mode 100644 bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
> > create mode 100644 bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> > create mode 100644 cpukit/include/rtems/score/memorymanagement.h
> > create mode 100644 cpukit/include/rtems/score/stackmanagement.h
> > create mode 100644 cpukit/score/src/stackmanagement.c
> >
> > diff --git a/bsps/arm/headers.am b/bsps/arm/headers.am
> > index 3d2b09effa..b1e86f3385 100644
> > --- a/bsps/arm/headers.am
> > +++ b/bsps/arm/headers.am
> > @@ -15,6 +15,7 @@ include_bsp_HEADERS +=
> ../../../../../bsps/arm/include/bsp/arm-a9mpcore-clock.h
> > include_bsp_HEADERS +=
> ../../../../../bsps/arm/include/bsp/arm-a9mpcore-irq.h
> > include_bsp_HEADERS +=
> ../../../../../bsps/arm/include/bsp/arm-a9mpcore-regs.h
> > include_bsp_HEADERS +=
> ../../../../../bsps/arm/include/bsp/arm-a9mpcore-start.h
> > +include_bsp_HEADERS +=
> ../../../../../bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
> > include_bsp_HEADERS +=
> ../../../../../bsps/arm/include/bsp/arm-cp15-start.h
> > include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-errata.h
> > include_bsp_HEADERS += ../../../../../bsps/arm/include/bsp/arm-gic-irq.h
> > diff --git a/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
> b/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
> > new file mode 100644
> > index 0000000000..39170927da
> > --- /dev/null
> > +++ b/bsps/arm/include/bsp/arm-cp15-set-ttb-entries.h
> > @@ -0,0 +1,7 @@
> > +#include<bsp/arm-cp15-start.h>
> > +
> > +uint32_t arm_cp15_set_translation_table_entries(
> > + const void *begin,
> > + const void *end,
> > + uint32_t section_flags
> > +);
> > \ No newline at end of file
> > diff --git a/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> b/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> > index 507277dca1..f5d0494167 100644
> > --- a/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> > +++ b/bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> > @@ -14,6 +14,7 @@
> >
> > #include <rtems.h>
> > #include <libcpu/arm-cp15.h>
> > +#include <bsp/arm-cp15-set-ttb-entries.h>
> > #include <bspopts.h>
> >
> > /*
> > @@ -30,6 +31,8 @@
> > * ARM DDI 0406C.b (ID072512)
> > */
> >
> > +#define ARM_MMU_USE_SMALL_PAGES
> > +
> > static uint32_t set_translation_table_entries(
> > const void *begin,
> > const void *end,
> > diff --git a/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> b/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> > new file mode 100644
> > index 0000000000..978e35b86c
> > --- /dev/null
> > +++ b/bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> > @@ -0,0 +1,72 @@
> > +#include <bsp/arm-cp15-start.h>
> > +#include <bsp/arm-cp15-set-ttb-entries.h>
> > +#include <rtems/score/memorymanagement.h>
> > +#include <libcpu/arm-cp15.h>
> > +#include <rtems.h>
> > +
> > +#ifdef USE_THREAD_STACK_PROTECTION
> > + #define ARM_MMU_USE_SMALL_PAGES
> > +#endif
> > +
> > +void memory_entries_set(uintptr_t begin, size_t size, memory_flags
> flags)
> > +{
> > +
> > + uintptr_t end;
> > + rtems_interrupt_level irq_level;
> > + uint32_t access_flags;
> > +
> > + end = begin + size;
> > + access_flags = memory_translate_flags(flags);
> > +
> > + rtems_interrupt_local_disable(irq_level);
>
> is a local isr critical section sufficient to protect changing the ttb?
>
On close inspection of the reference manual, I realize the mistake. The ARM
reference manual says that we have to disable all the interrupts, so I
guess rtems_interrupt_disable() would be better.
>
> > + arm_cp15_set_translation_table_entries(begin, end, access_flags);
> > + rtems_interrupt_local_enable(irq_level);
> > +}
> > +
> > +void memory_entries_unset(uintptr_t begin, size_t size)
> > +{
> > + uint32_t access_flags;
> > + uintptr_t end;
> > + rtems_interrupt_level irq_level;
> > +
> > + end = begin + size;
> > + access_flags = memory_translate_flags(NO_ACCESS);
> > +
> > + rtems_interrupt_local_disable(irq_level);
> > + arm_cp15_set_translation_table_entries(begin, end, access_flags);
> > + rtems_interrupt_local_enable(irq_level);
> > +}
> > +
> > +
> > +uint32_t memory_translate_flags(memory_flags attr_flags)
>
> This helper function could be static (and at the top of the file).
> +{
> > + uint32_t flags;
> > + switch (attr_flags)
> > + {
> > + case READ_WRITE:
> > + flags = ARMV7_MMU_READ_WRITE;
> > + break;
> > +
> > + case READ_WRITE_CACHED:
> > + flags = ARMV7_MMU_DATA_READ_WRITE_CACHED;
> > + break;
> > +
> > + case READ_ONLY:
> > + flags = ARMV7_MMU_READ_ONLY;
> > + break;
> > +
> > + case READ_ONLY_CACHED:
> > + flags = ARMV7_MMU_READ_ONLY_CACHED;
> > + break;
> > +
> > + case NO_ACCESS:
> > + flags = 0;
> > + break;
> > +
> > + default:
> > + return -1;
>
> It's not great to return a negative value in an unsigned return.
> Especially considering you're not checking them.
>
> The "safe default" to use when modifying protection state is default-deny.
>
> > + break;
> > + }
> > +
> > + return flags;
> > +}
> > \ No newline at end of file
> > diff --git a/bsps/shared/start/stackalloc.c
> b/bsps/shared/start/stackalloc.c
> > index f7cf7be0f1..2ce0d6573b 100644
> > --- a/bsps/shared/start/stackalloc.c
> > +++ b/bsps/shared/start/stackalloc.c
> > @@ -25,6 +25,7 @@
> > #include <rtems.h>
> > #include <rtems/score/heapimpl.h>
> > #include <rtems/score/wkspace.h>
> > +#include <rtems/score/stackmanagement.h>
> >
> > #include <bsp/linker-symbols.h>
> >
> > @@ -42,7 +43,8 @@ void bsp_stack_allocate_init(size_t stack_space_size)
> >
> > void *bsp_stack_allocate(size_t size)
> > {
> > - void *stack = NULL;
> > + void *stack = NULL;
> > + uintptr_t page_table_base;
> These should be two spaces indented
>
> >
> > if (bsp_stack_heap.area_begin != 0) {
> > stack = _Heap_Allocate(&bsp_stack_heap, size);
> > @@ -51,11 +53,23 @@ void *bsp_stack_allocate(size_t size)
> > if (stack == NULL) {
> > stack = _Workspace_Allocate(size);
> > }
> > -
> > +
> Not sure why the above two lines are removing and adding seeming the
> same thing. Watch out for this.
>
> > +#ifdef USE_THREAD_STACK_PROTECTION
> > + /*
> > + This is a hard coded address, assigned just for the
> > + purpose of consistency
> > + */
> Not sure what "consistency" means here. the comment is unclear.
>
> > + page_table_base = (uintptr_t)0x1000;
> Avoid adding extra whitespaces at the end of the line
>
> > + /*
> > + The current way to allocate protected stack is to assign memory
> attributes
> > + to the allocated memory and remove memory-entry of every other stack
> > + */
> > + prot_stack_allocate( (uintptr_t)stack, size, page_table_base );
> if you're not actually allocating something, you may not want to call
> it "allocate" -- think about what it does when you go through your
> renaming
>
> > +#endif
> > return stack;
> > }
> >
> > -void bsp_stack_free(void *stack)
> > +void bsp_stack_free(void *stack)
> avoid (spurious) changes in existing code
>
> > {
> > bool ok = _Heap_Free(&bsp_stack_heap, stack);
> >
> > diff --git a/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am
> b/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am
> > index cfd59475c2..490f99792e 100644
> > --- a/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am
> > +++ b/c/src/lib/libbsp/arm/xilinx-zynq/Makefile.am
> > @@ -74,6 +74,9 @@ librtemsbsp_a_SOURCES +=
> ../../../../../../bsps/arm/xilinx-zynq/i2c/cadence-i2c.
> > # Cache
> > librtemsbsp_a_SOURCES +=
> ../../../../../../bsps/arm/shared/cache/cache-l2c-310.c
> >
> > +#MMU
> > +librtemsbsp_a_SOURCES +=
> ../../../../../../bsps/arm/xilinx-zynq/mmu/bsp-set-mmu-attr.c
> > +
> > # Start hooks
> > librtemsbsp_a_SOURCES +=
> ../../../../../../bsps/arm/xilinx-zynq/start/bspstarthooks.c
> > librtemsbsp_a_SOURCES +=
> ../../../../../../bsps/arm/xilinx-zynq/start/bspstartmmu.c
> > @@ -85,4 +88,4 @@ librtemsbsp_a_SOURCES +=
> ../../../../../../bsps/arm/xilinx-zynq/start/bspstartmm
> >
> > include $(srcdir)/../../../../../../bsps/shared/irq-sources.am
> > include $(srcdir)/../../../../../../bsps/shared/shared-sources.am
> > -include $(srcdir)/../../../../../../bsps/arm/xilinx-zynq/headers.am
> > +include $(srcdir)/../../../../../../bsps/arm/xilinx-zynq/headers.am
> > \ No newline at end of file
> > diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
> > index 51f38c84c7..f3b373376b 100644
> > --- a/cpukit/Makefile.am
> > +++ b/cpukit/Makefile.am
> > @@ -929,6 +929,7 @@ librtemscpu_a_SOURCES +=
> score/src/schedulercbsgetserverid.c
> > librtemscpu_a_SOURCES += score/src/schedulercbssetparameters.c
> > librtemscpu_a_SOURCES += score/src/schedulercbsreleasejob.c
> > librtemscpu_a_SOURCES += score/src/schedulercbsunblock.c
> > +librtemscpu_a_SOURCES += score/src/stackmanagement.c
> > librtemscpu_a_SOURCES += score/src/stackallocator.c
> > librtemscpu_a_SOURCES += score/src/pheapallocate.c
> > librtemscpu_a_SOURCES += score/src/pheapextend.c
> > diff --git a/cpukit/headers.am b/cpukit/headers.am
> > index fcf679f09d..9388aa0c8c 100644
> > --- a/cpukit/headers.am
> > +++ b/cpukit/headers.am
> > @@ -352,6 +352,7 @@ include_rtems_score_HEADERS +=
> include/rtems/score/isr.h
> > include_rtems_score_HEADERS += include/rtems/score/isrlevel.h
> > include_rtems_score_HEADERS += include/rtems/score/isrlock.h
> > include_rtems_score_HEADERS += include/rtems/score/memory.h
> > +include_rtems_score_HEADERS += include/rtems/score/memorymanagement.h
> > include_rtems_score_HEADERS += include/rtems/score/mpci.h
> > include_rtems_score_HEADERS += include/rtems/score/mpciimpl.h
> > include_rtems_score_HEADERS += include/rtems/score/mppkt.h
> > @@ -405,6 +406,7 @@ include_rtems_score_HEADERS +=
> include/rtems/score/smplockstats.h
> > include_rtems_score_HEADERS += include/rtems/score/smplockticket.h
> > include_rtems_score_HEADERS += include/rtems/score/stack.h
> > include_rtems_score_HEADERS += include/rtems/score/stackimpl.h
> > +include_rtems_score_HEADERS += include/rtems/score/stackmanagement.h
> > include_rtems_score_HEADERS += include/rtems/score/states.h
> > include_rtems_score_HEADERS += include/rtems/score/statesimpl.h
> > include_rtems_score_HEADERS += include/rtems/score/status.h
> > diff --git a/cpukit/include/rtems/score/memorymanagement.h
> b/cpukit/include/rtems/score/memorymanagement.h
> > new file mode 100644
> > index 0000000000..c712e7301a
> > --- /dev/null
> > +++ b/cpukit/include/rtems/score/memorymanagement.h
> > @@ -0,0 +1,22 @@
> > +#ifndef _RTEMS_SCORE_MEMORYMANAGEMENT_H
> > +#define _RTEMS_SCORE_MEMORYMANAGEMENT_H
> > +
> > +#include <stdlib.h>
> What do you need this header for?
>
> > +#include <stdint.h>
> This should get picked up if you include basedefs like other score headers
> do.
>
> > +
> > +typedef enum
> > +{
> > + READ_WRITE,
> > + READ_WRITE_CACHED,
> > + READ_ONLY,
> > + READ_ONLY_CACHED,
> > + NO_ACCESS
> > +} memory_flags;
> > +
>
> Need documentation. Look at other score headers to guide you.
>
>
The memory_entries_* need to be implemented for each BSP to expose the
low-level MMU implementation of that BSP. Is score the appropriate
location for this API?
> > +void memory_entries_set(uintptr_t begin_addr, size_t size, memory_flags
> flags);
> > +
> > +void memory_entries_unset(uintptr_t begin_addr, size_t size);
> > +
> > +uint32_t memory_translate_flags(memory_flags flags);
>
> Does this need to be part of the public-facing interface?
> > +
> > +#endif
> > \ No newline at end of file
> > diff --git a/cpukit/include/rtems/score/stackmanagement.h
> b/cpukit/include/rtems/score/stackmanagement.h
> > new file mode 100644
> > index 0000000000..53b6a23af0
> > --- /dev/null
> > +++ b/cpukit/include/rtems/score/stackmanagement.h
> > @@ -0,0 +1,49 @@
> > +#ifndef _RTEMS_SCORE_STACKMANAGEMENT_H
> > +#define _RTEMS_SCORE_STACKMANAGEMENT_H
> > +
> > +#if defined (ASM)
> > +#include <rtems/asm.h>
> > +#else
> This also gets handled by basedefs.
>
> > +
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +#include <rtems/score/memorymanagement.h>
> > +#include <rtems/score/chainimpl.h>
> > +
> > +typedef struct stack_attr
> > +{
> > + Chain_Node node;
> > + uintptr_t stack_address;
> > + size_t size;
> > + uint32_t page_table_base;
> > + memory_flags access_flags;
> > +}stack_attr;
> space after }
>
> > +
> > +typedef struct stack_attr_shared
> > +{
> > + stack_attr Base;
> > + Chain_Control shared_node_control;
> > +}stack_attr_shared;
> > +
> > +typedef struct stack_attr_prot
> > +{
> > + stack_attr_shared *shared_stacks;
> > + stack_attr Base;
> Typically 'Base' should come first in the struct definition. This way,
> the start of the struct is identical to its Base
>
> > + bool current_stack;
> > +} stack_attr_prot;
> > +
> > +
> > +void prot_stack_allocate(uintptr_t stack_address, size_t size,
> uintptr_t page_table_base);
> > +
> > +
> > +void prot_stack_share(stack_attr_shared *shared, stack_attr_prot
> prot_stack);
> > +
> > +stack_attr_prot *prot_stack_context_initialize(void);
> > +
> > +void prot_stack_context_switch(stack_attr_prot *stack_attr);
> > +
> > +void prot_stack_context_restore(stack_attr_prot *stack_attr);
> > +
> > +#endif
> > +
> > +#endif
> > \ No newline at end of file
> > diff --git a/cpukit/score/cpu/arm/cpu.c b/cpukit/score/cpu/arm/cpu.c
> > index 07b9588afd..aa3626965a 100644
> > --- a/cpukit/score/cpu/arm/cpu.c
> > +++ b/cpukit/score/cpu/arm/cpu.c
> > @@ -97,6 +97,9 @@ void _CPU_Context_Initialize(
> > {
> > (void) new_level;
> >
> > + #if defined (USE_THREAD_STACK_PROTECTION)
> > + the_context->stack_attr = prot_stack_context_initialize();
> > + #endif
> > the_context->register_sp = (uint32_t) stack_area_begin +
> stack_area_size;
> > the_context->register_lr = (uint32_t) entry_point;
> > the_context->isr_dispatch_disable = 0;
> > diff --git a/cpukit/score/cpu/arm/cpu_asm.S
> b/cpukit/score/cpu/arm/cpu_asm.S
> > index 66f8ba6032..0133746b82 100644
> > --- a/cpukit/score/cpu/arm/cpu_asm.S
> > +++ b/cpukit/score/cpu/arm/cpu_asm.S
> > @@ -36,7 +36,6 @@
> > #ifdef ARM_MULTILIB_ARCH_V4
> >
> > .text
> > -
> spurious
>
> > /*
> > * void _CPU_Context_switch( run_context, heir_context )
> > * void _CPU_Context_restore( run_context, heir_context )
> > @@ -65,6 +64,16 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)
> > #endif
> >
> > str r3, [r0, #ARM_CONTEXT_CONTROL_ISR_DISPATCH_DISABLE]
> > +#ifdef USE_THREAD_STACK_PROTECTION
> > + mov r8, r0
> > + mov r9, r1
> check alignment
>
> > + mov r10, r2
> > + ldr r0, [r0, #112]
> > + bl prot_stack_context_switch
>
> Is this a function call from inside the context switch code? That does
> not seem like a good design.
>
> > + mov r0, r8
> > + mov r1, r9
> > + mov r2, r10
> > +#endif
> >
> > #ifdef RTEMS_SMP
> > /*
> > @@ -133,6 +142,17 @@ DEFINE_FUNCTION_ARM(_CPU_Context_switch)
> > */
> > DEFINE_FUNCTION_ARM(_CPU_Context_restore)
> > mov r1, r0
> > +#if defined( USE_THREAD_STACK_PROTECTION )
> > + mov r8, r0
> > + mov r9, r1
> > + mov r10, r2
> > + ldr r0, [r0, #ARM_STACK_PROT_ATTR_OFFSET]
> > + bl prot_stack_context_restore
> ditto
>
> > + ldr lr, [r2]
> > + mov r0, r8
> > + mov r1, r9
> > + mov r2, r10
> > +#endif
> > GET_SELF_CPU_CONTROL r2
> > b .L_restore
> >
> > diff --git a/cpukit/score/cpu/arm/include/rtems/score/cpu.h
> b/cpukit/score/cpu/arm/include/rtems/score/cpu.h
> > index b7b48a3ac3..fb91142f35 100644
> > --- a/cpukit/score/cpu/arm/include/rtems/score/cpu.h
> > +++ b/cpukit/score/cpu/arm/include/rtems/score/cpu.h
> > @@ -34,6 +34,7 @@
> > #include <rtems/score/paravirt.h>
> > #endif
> > #include <rtems/score/arm.h>
> > +#include <rtems/score/stackmanagement.h>
> >
> > /**
> > * @addtogroup RTEMSScoreCPUARM
> > @@ -157,6 +158,21 @@
> >
> > #ifdef ARM_MULTILIB_HAS_THREAD_ID_REGISTER
> > #define ARM_CONTEXT_CONTROL_THREAD_ID_OFFSET 44
> > + #ifdef ARM_MULITLIB_VFP
> > + #define ARM_STACK_PROT_ATTR_OFFSET 112
> > + #else
> > + #define ARM_STACK_PROT_ATTR_OFFSET 48
> > + #endif
> > +#endif
> > +
> > +#ifdef USE_THREAD_STACK_PROTECTION
> > + #if defined ARM_MULITLIB_VFP
> > + #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 112
> > + #elif ARM_MULTILIB_HAS_THREAD_ID_REGISTER
> > + #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 48
> > + #else
> > + #define ARM_CONTEXT_CONTROL_STACK_ATTR_OFFSET 44
> > + #endif
> > #endif
> >
> > #ifdef ARM_MULTILIB_VFP
> > @@ -185,6 +201,7 @@
> >
> > #define ARM_VFP_CONTEXT_SIZE 264
> >
> > +
> ws
>
> > #ifndef ASM
> >
> > #ifdef __cplusplus
> > @@ -235,6 +252,9 @@ typedef struct {
> > #ifdef RTEMS_SMP
> > volatile bool is_executing;
> > #endif
> > +#ifdef USE_THREAD_STACK_PROTECTION
> > +stack_attr_prot *stack_attr;
> > +#endif
> > } Context_Control;
> >
> > static inline void _ARM_Data_memory_barrier( void )
> > diff --git a/cpukit/score/src/stackmanagement.c
> b/cpukit/score/src/stackmanagement.c
> > new file mode 100644
> > index 0000000000..ae66099b14
> > --- /dev/null
> > +++ b/cpukit/score/src/stackmanagement.c
> > @@ -0,0 +1,143 @@
> > +#include <rtems/score/stackmanagement.h>
> > +#include <rtems/score/chainimpl.h>
> > +#include <rtems/score/basedefs.h>
> > +
> > +Chain_Control prot_node_control =
> CHAIN_INITIALIZER_EMPTY(prot_node_control);
> > +
> > +Chain_Control *shared_node_control;
> > +
> > +static void prot_stack_prev_entry_remove(void)
> > +{
> > +
> > + Chain_Node *node;
> > + stack_attr_prot *stack_attr;
> > +
> > + if( _Chain_Is_empty(&prot_node_control) == true ) {
> > + _Chain_Initialize_empty(&prot_node_control);
> > + }
> > + node = _Chain_First( &prot_node_control );
> > +
> > + while(_Chain_Is_tail(&prot_node_control, node) == false) {
> > +
> > + stack_attr = RTEMS_CONTAINER_OF(node,stack_attr_prot,
> Base.node);
> > +
> > + if( stack_attr->current_stack == false ) {
>
> Is there more than one current_stack?
>
> If not, why can't you just keep track of it in a variable rather than
> iterate through a list looking for it?
>
> > + memory_entries_unset(stack_attr->Base.stack_address,
> stack_attr->Base.size);
> > + // shared_stack_entry_remove(stack_attr->shared_stacks);
> > +
> > + }
> > + node = _Chain_Immutable_next( node );
> > + }
> > +
> > +}
> > +
> > +/*
> > +Iterate to the end of the chain and mark all the 'currnet' stack as
> false
> > +Append the current stack attribute to the end of the chain
> > +*/
> > +static void prot_stack_chain_append (Chain_Control *control,
> stack_attr_prot *stack_append_attr)
> > +{
> > + Chain_Node *node;
> > + stack_attr_prot *present_stacks_attr;
> > +
> > + if(_Chain_Is_empty(&prot_node_control) == true ) {
> > +
> > + _Chain_Initialize_one(&prot_node_control,
> &stack_append_attr->Base.node);
> > + } else {
> > + node = _Chain_First(&prot_node_control);
> > +
> > + while(_Chain_Is_tail(&prot_node_control,node) == false) {
> > +
> > + present_stacks_attr = RTEMS_CONTAINER_OF(node,
> stack_attr_prot, Base.node);
> > + present_stacks_attr->current_stack = false;
> > + node = _Chain_Immutable_next( node );
> > + }
> > + _Chain_Append_unprotected(&prot_node_control,
> &stack_append_attr->Base.node);
> > + }
> > +
> > +}
> > +
> > +void prot_stack_allocate(uintptr_t stack_address, size_t size,
> uintptr_t page_table_base)
> > +{
> > + stack_attr_prot *stack_attr;
> > +
> > +/*This field will be refactored and score objects will be used for
> dynamic allocation*/
> > + stack_attr = malloc(sizeof(stack_attr_prot));
> > +
> > + if(stack_attr != NULL) {
> > + stack_attr->Base.stack_address = stack_address;
> > + stack_attr->Base.size = size;
> > + stack_attr->Base.page_table_base = page_table_base;
> > + stack_attr->Base.access_flags = READ_WRITE_CACHED;
> > + stack_attr->current_stack = true;
> > + }
> > +
> > + /*
> > + Add the attribute field to the end of the chain, remove the memory
> entries of
> > + previously allocated stack and set the memory entry of the currnet
> stack.
> > + */
> > + prot_stack_chain_append(&prot_node_control, stack_attr );
> > + prot_stack_prev_entry_remove();
> > + memory_entries_set(stack_address, size, READ_WRITE_CACHED);
> > +
> > +}
> > +
> > +stack_attr_prot *prot_stack_context_initialize(void)
> > +{
> > + Chain_Node *node;
> > + stack_attr_prot *stack_attr;
> > +
> > + if( _Chain_Is_empty(&prot_node_control) == false ) {
> > + node = _Chain_First( &prot_node_control );
> > +
> > + while( _Chain_Is_tail(&prot_node_control, node ) == false) {
> > + stack_attr = RTEMS_CONTAINER_OF( node, stack_attr_prot,
> Base.node);
> > +
> > + if(stack_attr->current_stack == true) {
> > + return stack_attr;
> > + } else {
> > + node = _Chain_Immutable_next( node );
> > + }
> > + }
> > + }
> > +
> > + return stack_attr;
> > +}
> > +
> > +void prot_stack_context_switch(stack_attr_prot *stack_attr)
> > +{
> > + void *stack_address;
> > + size_t size;
> > + Chain_Node *node;
> > + Chain_Control *shared_node_control;
> > +
> > + /*
> > + Remove the stacks shared with the current stack by iterating the
> chain
> > + */
> > + if( stack_attr != NULL) {
> > +
> > + stack_address = stack_attr->Base.stack_address;
> > + size = stack_attr->Base.size;
> > +
> > + if(stack_attr->current_stack == true) {
> > + memory_entries_unset(stack_address, size);
> > + stack_attr->current_stack = false;
> > + }
> > +
> > + shared_node_control =
> &stack_attr->shared_stacks->shared_node_control;
> > + }
> > +}
> > +
> > +void prot_stack_context_restore(stack_attr_prot *stack_attr)
> > +{
> > + void *stack_address;
> > + size_t size;
> > + Chain_Node *node;
> > + Chain_Control *shared_node_control;
> > + memory_flags flags;
> > +
> > + if(stack_attr->current_stack == true) {
> > + memory_entries_set(stack_address, size, READ_WRITE_CACHED);
>
> Should you be reading these flags from somewhere
> (stack_attr->Base.access_flags?), instead of hard-coding them?
>
> > + }
> > +
> > +}
> > --
> > 2.17.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20200714/89a72ad1/attachment-0001.html>
More information about the devel
mailing list