Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
On Mar 12, 2018 7:40 AM, "Amaan Cheval" wrote: Agreed. I'll look into fixing CPU_Interrupt_frame up as time permits. If you get it to compile before addressing the context switch synchronization point, one thought is to put an ifdef RTEMS_SMP with a #error where the fix needs to go with the message referencing the ticket number. At least that way, it still doesn't compile until that issue is also fixed. As Sebastian said, that failure is more subtle than a compile error. Appreciate the help and follow-ups, thanks! On Mon, Mar 12, 2018 at 6:06 PM Sebastian Huber < sebastian.hu...@embedded-brains.de> wrote: > On 12/03/18 13:32, Amaan Cheval wrote: > > I originally sent this patch with the intent of merely ridding the i386 > > targets of compiler errors, for anyone interested in looking into SMP > > issues on the arch. > > > > Do you believe that I should look into fixing i386's incomplete SMP > > context-switch support for this patch too, or would that be okay as a > > follow-up later, given that it seems like there's more incompleteness > > regarding SMP for i386? > A compile-time error is a good hint that it is broken. I think this is > more user friendly compared to having to debug a run-time error which > takes place only under certain conditions. > -- > Sebastian Huber, embedded brains GmbH > Address : Dornierstr. 4, D-82178 Puchheim, Germany > Phone : +49 89 189 47 41-16 > Fax : +49 89 189 47 41-09 > E-Mail : sebastian.hu...@embedded-brains.de > PGP : Public key available on request. > Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
Agreed. I'll look into fixing CPU_Interrupt_frame up as time permits. Appreciate the help and follow-ups, thanks! On Mon, Mar 12, 2018 at 6:06 PM Sebastian Huber < sebastian.hu...@embedded-brains.de> wrote: > On 12/03/18 13:32, Amaan Cheval wrote: > > I originally sent this patch with the intent of merely ridding the i386 > > targets of compiler errors, for anyone interested in looking into SMP > > issues on the arch. > > > > Do you believe that I should look into fixing i386's incomplete SMP > > context-switch support for this patch too, or would that be okay as a > > follow-up later, given that it seems like there's more incompleteness > > regarding SMP for i386? > A compile-time error is a good hint that it is broken. I think this is > more user friendly compared to having to debug a run-time error which > takes place only under certain conditions. > -- > Sebastian Huber, embedded brains GmbH > Address : Dornierstr. 4, D-82178 Puchheim, Germany > Phone : +49 89 189 47 41-16 > Fax : +49 89 189 47 41-09 > E-Mail : sebastian.hu...@embedded-brains.de > PGP : Public key available on request. > Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
On 12/03/18 13:32, Amaan Cheval wrote: I originally sent this patch with the intent of merely ridding the i386 targets of compiler errors, for anyone interested in looking into SMP issues on the arch. Do you believe that I should look into fixing i386's incomplete SMP context-switch support for this patch too, or would that be okay as a follow-up later, given that it seems like there's more incompleteness regarding SMP for i386? A compile-time error is a good hint that it is broken. I think this is more user friendly compared to having to debug a run-time error which takes place only under certain conditions. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
Brilliant, thanks a ton! That should keep me occupied for a bit. I originally sent this patch with the intent of merely ridding the i386 targets of compiler errors, for anyone interested in looking into SMP issues on the arch. Do you believe that I should look into fixing i386's incomplete SMP context-switch support for this patch too, or would that be okay as a follow-up later, given that it seems like there's more incompleteness regarding SMP for i386? On Mon, Mar 12, 2018 at 5:54 PM Sebastian Huber < sebastian.hu...@embedded-brains.de> wrote: > On 12/03/18 13:13, Amaan Cheval wrote: > > Hey! Thanks for the guidance! > > > > I did look at cpu_asm.S, but I don't quite get how Interrupt_frame is being > > used, where by "used" I mean it in the sense that fields within it are > > being set to the actual register values, the way they are with stm/ltm for > > the Context_Control structure. > This structure is used as a temporary stack to allow interrupt > processing during a context switch. You find some information about this > here: https://docs.rtems.org/branches/master/c-user/symmetric_multiprocessing_services.html#thread-dispatch-details > > > > The reason I'm looking for ways in which state values are saved into the > > CPU_Interrupt_frame > The interrupt exception prologue saves the volatile context of the > interrupted thread into this structure. I guess this is https://git.rtems.org/rtems/tree/c/src/lib/libbsp/i386/shared/irq/irq_asm.S > I don't know the i386 BSP well and I am not really interested in this > architecture. > > struct is because the comment in > > ./cpukit/score/cpu/no_cpu/include/rtems/score/cpu.h says: > > > > /** > >* @ingroup Management > >* > >* This defines the set of integer and processor state registers that > > must > >* be saved during an interrupt. This set does not include any which > > are > >* in @ref Context_Control. > >*/ > > > > In cpu_asm.S, _CPU_Context_switch has only this bit that's relevant to > > Interrupt_frame: > > > > add sp, r2, #(PER_CPU_INTERRUPT_FRAME_AREA + CPU_INTERRUPT_FRAME_SIZE) > > > > Which is effectively: > > > > sp = GET_SELF_CPU_CONTROL() + Per_CPU_Control.Interrupt_frame + > > sizeof(CPU_Interrupt_frame) > > > > If the stack grows downwards, I guess pushing to the stack would write to > > the Interrupt_frame? > Yes. > > I'll continue looking at it now that I know this is > > the correct place to look; perhaps what I'm missing is just how the stack > > pointer is used or how interrupts are dispatched? > On ARM, it is in this file: > https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/arm_exc_interrupt.S > > > >> SMP context switch code (which is incomplete for i386) > > When you say this, does that mean _CPU_Context_switch's bits with "#ifdef > > RTEMS_SMP" > Yes. > > or do you mean another function altogether? > No, the non-SMP context switch should work fine. > > Sorry if that's a > > stupid question; evidently I'm not familiar enough with RTEMS' internals - > > it's easy to get lost until you find just the right thread to unwind the > > whole thing. :P > > > > P.S. - I'm "amaan" on the #rtems IRC in case you think I've confused myself > > and gone down the wrong path, and it'd be quicker to clarify on chat. > > > > On Mon, Mar 12, 2018 at 12:18 PM Sebastian Huber < > > sebastian.hu...@embedded-brains.de> wrote: > > > >> On 10/03/18 15:11, Amaan Cheval wrote: > >>> CPU_INTERRUPT_FRAME_SIZE needs to also be set to allow the > > RTEMS_STATIC_ASSERT > >>> in percpuasm.c to be fulfilled. > >> The CPU_Interrupt_frame must properly defined. It must be used by the > >> SMP context switch code (which is incomplete for i386). Please have a > >> look at the ARM context switch code as an example: > >> https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/cpu_asm.S > >> -- > >> Sebastian Huber, embedded brains GmbH > >> Address : Dornierstr. 4, D-82178 Puchheim, Germany > >> Phone : +49 89 189 47 41-16 > >> Fax : +49 89 189 47 41-09 > >> E-Mail : sebastian.hu...@embedded-brains.de > >> PGP : Public key available on request. > >> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. > -- > Sebastian Huber, embedded brains GmbH > Address : Dornierstr. 4, D-82178 Puchheim, Germany > Phone : +49 89 189 47 41-16 > Fax : +49 89 189 47 41-09 > E-Mail : sebastian.hu...@embedded-brains.de > PGP : Public key available on request. > Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
On 12/03/18 13:13, Amaan Cheval wrote: Hey! Thanks for the guidance! I did look at cpu_asm.S, but I don't quite get how Interrupt_frame is being used, where by "used" I mean it in the sense that fields within it are being set to the actual register values, the way they are with stm/ltm for the Context_Control structure. This structure is used as a temporary stack to allow interrupt processing during a context switch. You find some information about this here: https://docs.rtems.org/branches/master/c-user/symmetric_multiprocessing_services.html#thread-dispatch-details The reason I'm looking for ways in which state values are saved into the CPU_Interrupt_frame The interrupt exception prologue saves the volatile context of the interrupted thread into this structure. I guess this is https://git.rtems.org/rtems/tree/c/src/lib/libbsp/i386/shared/irq/irq_asm.S I don't know the i386 BSP well and I am not really interested in this architecture. struct is because the comment in ./cpukit/score/cpu/no_cpu/include/rtems/score/cpu.h says: /** * @ingroup Management * * This defines the set of integer and processor state registers that must * be saved during an interrupt. This set does not include any which are * in @ref Context_Control. */ In cpu_asm.S, _CPU_Context_switch has only this bit that's relevant to Interrupt_frame: add sp, r2, #(PER_CPU_INTERRUPT_FRAME_AREA + CPU_INTERRUPT_FRAME_SIZE) Which is effectively: sp = GET_SELF_CPU_CONTROL() + Per_CPU_Control.Interrupt_frame + sizeof(CPU_Interrupt_frame) If the stack grows downwards, I guess pushing to the stack would write to the Interrupt_frame? Yes. I'll continue looking at it now that I know this is the correct place to look; perhaps what I'm missing is just how the stack pointer is used or how interrupts are dispatched? On ARM, it is in this file: https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/arm_exc_interrupt.S SMP context switch code (which is incomplete for i386) When you say this, does that mean _CPU_Context_switch's bits with "#ifdef RTEMS_SMP" Yes. or do you mean another function altogether? No, the non-SMP context switch should work fine. Sorry if that's a stupid question; evidently I'm not familiar enough with RTEMS' internals - it's easy to get lost until you find just the right thread to unwind the whole thing. :P P.S. - I'm "amaan" on the #rtems IRC in case you think I've confused myself and gone down the wrong path, and it'd be quicker to clarify on chat. On Mon, Mar 12, 2018 at 12:18 PM Sebastian Huber < sebastian.hu...@embedded-brains.de> wrote: On 10/03/18 15:11, Amaan Cheval wrote: CPU_INTERRUPT_FRAME_SIZE needs to also be set to allow the RTEMS_STATIC_ASSERT in percpuasm.c to be fulfilled. The CPU_Interrupt_frame must properly defined. It must be used by the SMP context switch code (which is incomplete for i386). Please have a look at the ARM context switch code as an example: https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/cpu_asm.S -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
Hey! Thanks for the guidance! I did look at cpu_asm.S, but I don't quite get how Interrupt_frame is being used, where by "used" I mean it in the sense that fields within it are being set to the actual register values, the way they are with stm/ltm for the Context_Control structure. The reason I'm looking for ways in which state values are saved into the CPU_Interrupt_frame struct is because the comment in ./cpukit/score/cpu/no_cpu/include/rtems/score/cpu.h says: /** * @ingroup Management * * This defines the set of integer and processor state registers that must * be saved during an interrupt. This set does not include any which are * in @ref Context_Control. */ In cpu_asm.S, _CPU_Context_switch has only this bit that's relevant to Interrupt_frame: add sp, r2, #(PER_CPU_INTERRUPT_FRAME_AREA + CPU_INTERRUPT_FRAME_SIZE) Which is effectively: sp = GET_SELF_CPU_CONTROL() + Per_CPU_Control.Interrupt_frame + sizeof(CPU_Interrupt_frame) If the stack grows downwards, I guess pushing to the stack would write to the Interrupt_frame? I'll continue looking at it now that I know this is the correct place to look; perhaps what I'm missing is just how the stack pointer is used or how interrupts are dispatched? > SMP context switch code (which is incomplete for i386) When you say this, does that mean _CPU_Context_switch's bits with "#ifdef RTEMS_SMP" or do you mean another function altogether? Sorry if that's a stupid question; evidently I'm not familiar enough with RTEMS' internals - it's easy to get lost until you find just the right thread to unwind the whole thing. :P P.S. - I'm "amaan" on the #rtems IRC in case you think I've confused myself and gone down the wrong path, and it'd be quicker to clarify on chat. On Mon, Mar 12, 2018 at 12:18 PM Sebastian Huber < sebastian.hu...@embedded-brains.de> wrote: > On 10/03/18 15:11, Amaan Cheval wrote: > > CPU_INTERRUPT_FRAME_SIZE needs to also be set to allow the RTEMS_STATIC_ASSERT > > in percpuasm.c to be fulfilled. > The CPU_Interrupt_frame must properly defined. It must be used by the > SMP context switch code (which is incomplete for i386). Please have a > look at the ARM context switch code as an example: > https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/cpu_asm.S > -- > Sebastian Huber, embedded brains GmbH > Address : Dornierstr. 4, D-82178 Puchheim, Germany > Phone : +49 89 189 47 41-16 > Fax : +49 89 189 47 41-09 > E-Mail : sebastian.hu...@embedded-brains.de > PGP : Public key available on request. > Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
On 10/03/18 15:11, Amaan Cheval wrote: CPU_INTERRUPT_FRAME_SIZE needs to also be set to allow the RTEMS_STATIC_ASSERT in percpuasm.c to be fulfilled. The CPU_Interrupt_frame must properly defined. It must be used by the SMP context switch code (which is incomplete for i386). Please have a look at the ARM context switch code as an example: https://git.rtems.org/rtems/tree/cpukit/score/cpu/arm/cpu_asm.S -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] i386/smp: Define unused CPU_Interrupt_frame to fix compiler error
Context: https://lists.rtems.org/pipermail/devel/2018-March/020409.html
I just realized that it may make more sense to use uint8_t and a
CPU_INTERRUPT_FRAME_SIZE of 1 instead, if we _do_ want to leave it unused
but defined. Another option I considered was simply setting a preprocessor
flag to disable the use of CPU_Interrupt_frame, despite HAS_SMP being set
(for eg. DISABLE_INTERRUPT_FRAME, so only i386 can opt out of that?)
Let me know if either of those seem any better.
On Sat, Mar 10, 2018 at 7:42 PM Amaan Cheval wrote:
> CPU_INTERRUPT_FRAME_SIZE needs to also be set to allow the
RTEMS_STATIC_ASSERT
> in percpuasm.c to be fulfilled.
> Updates #3331
> ---
> cpukit/score/cpu/i386/include/rtems/score/cpu.h | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> diff --git a/cpukit/score/cpu/i386/include/rtems/score/cpu.h
b/cpukit/score/cpu/i386/include/rtems/score/cpu.h
> index f78149c24b..b8ac218acc 100644
> --- a/cpukit/score/cpu/i386/include/rtems/score/cpu.h
> +++ b/cpukit/score/cpu/i386/include/rtems/score/cpu.h
> @@ -271,12 +271,20 @@ typedef void (*cpuExcHandlerType)
(CPU_Exception_frame*);
> extern cpuExcHandlerType _currentExcHandler;
> extern void rtems_exception_init_mngt(void);
> +#ifdef RTEMS_SMP
> + #define CPU_INTERRUPT_FRAME_SIZE 4
> +#endif
> +
> /*
>* This port does not pass any frame info to the
>* interrupt handler.
> + * For SMP, RTEMS assumes CPU_Interrupt_frame exists,
> + * so we make it a fixed size which is never used.
>*/
> -typedef void CPU_Interrupt_frame;
> +typedef struct {
> + uint32_t unused_field;
> +} CPU_Interrupt_frame;
> typedef enum {
> I386_EXCEPTION_DIVIDE_BY_ZERO = 0,
> --
> 2.13.0
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
