[PATCH] bsps/i386: Separate variable for i8259 IRQs disable due to in progress state.
Pavel Pisa
pisa at cmp.felk.cvut.cz
Mon Oct 10 10:14:10 UTC 2016
On Sunday 25 of September 2016 02:48:03 Pavel Pisa wrote:
> From: Pavel Pisa <pisa at cmp.felk.cvut.cz>
>
> The global state of enabled and disabled interrupts has to hold
> interrupts really disabled by drivers and system. If the state is
> combined with interrupts temporarily disabled because they are
> processed at given time then it is impossible to maintain state
> by interrupt handlers in drivers.
I have added compiler barrier to make code fully correct
for UP
@@ -346,6 +362,7 @@ void BSP_dispatch_isr(int vector)
*/
irq_count[vector]++;
+ RTEMS_COMPILER_MEMORY_BARRIER();
/*
* Allow nesting.
*/
@@ -358,6 +375,8 @@ void BSP_dispatch_isr(int vector)
*/
__asm__ __volatile__("cli");
+ RTEMS_COMPILER_MEMORY_BARRIER();
+
and I have tested in on real HW, without networking
because I do have there classic PCI E100 card.
But ticker and my projects including VESA graphics
seems to run correctly.
Have somebody else found time to test the code?
Some testing with serial ports (ideally multiple)
would help there. Even that there is risk of some
problems I think that step in this direction
is necessary.
Branch including even VirtIO can be found on my GitHub
sandbox
https://github.com/ppisa/rtems/tree/devel-virtio
I think that i8259 IRQs is needed for mainline and it
would worth to push change.
I can prepare and push or send for review trivial
followup patch to replace
rtems_interrupt_disable(level);
by
RTEMS_INTERRUPT_LOCK_DEFINE
rtems_interrupt_lock_acquire
I think that if i8259 interrupts are routed/delivered
to single core only then such setup can be used even
on i386 SMP system when IPI is made working.
Best wishes,
Pavel
> ---
> c/src/lib/libbsp/i386/shared/irq/irq.c | 51
> +++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 19
> deletions(-)
>
> diff --git a/c/src/lib/libbsp/i386/shared/irq/irq.c
> b/c/src/lib/libbsp/i386/shared/irq/irq.c index 0511257..8316fe6 100644
> --- a/c/src/lib/libbsp/i386/shared/irq/irq.c
> +++ b/c/src/lib/libbsp/i386/shared/irq/irq.c
> @@ -51,7 +51,22 @@ static enum intr_trigger
> irq_trigger[BSP_IRQ_LINES_NUMBER]; * while upper bits are interrupt on the
> slave PIC.
> * This cache is initialized in ldseg.s
> */
> -static rtems_i8259_masks i8259a_cache = 0xFFFB;
> +static rtems_i8259_masks i8259a_imr_cache = 0xFFFB;
> +static rtems_i8259_masks i8259a_in_progress = 0;
> +
> +static inline
> +void BSP_i8259a_irq_update_master_imr( void )
> +{
> + rtems_i8259_masks mask = i8259a_in_progress | i8259a_imr_cache;
> + outport_byte( PIC_MASTER_IMR_IO_PORT, mask & 0xff );
> +}
> +
> +static inline
> +void BSP_i8259a_irq_update_slave_imr( void )
> +{
> + rtems_i8259_masks mask = i8259a_in_progress | i8259a_imr_cache;
> + outport_byte( PIC_SLAVE_IMR_IO_PORT, ( mask >> 8 ) & 0xff );
> +}
>
> /*
> * Print the stats.
> @@ -107,7 +122,7 @@ static inline uint8_t
> BSP_i8259a_irq_in_service_reg(uint32_t ioport)
> /*-------------------------------------------------------------------------
>+
>
> | Function: BSP_irq_disable_at_i8259a
> | Description: Mask IRQ line in appropriate PIC chip.
>
> -| Global Variables: i8259a_cache
> +| Global Variables: i8259a_imr_cache, i8259a_in_progress
>
> | Arguments: vector_offset - number of IRQ line to mask.
> | Returns: 0 is OK.
>
>
> +--------------------------------------------------------------------------
>*/ @@ -119,15 +134,15 @@ static int BSP_irq_disable_at_i8259a(const
> rtems_irq_number irqLine) rtems_interrupt_disable(level);
>
> mask = 1 << irqLine;
> - i8259a_cache |= mask;
> + i8259a_imr_cache |= mask;
>
> if (irqLine < 8)
> {
> - outport_byte(PIC_MASTER_IMR_IO_PORT, i8259a_cache & 0xff);
> + BSP_i8259a_irq_update_master_imr();
> }
> else
> {
> - outport_byte(PIC_SLAVE_IMR_IO_PORT, (i8259a_cache >> 8) & 0xff);
> + BSP_i8259a_irq_update_slave_imr();
> }
>
> rtems_interrupt_enable(level);
> @@ -138,7 +153,7 @@ static int BSP_irq_disable_at_i8259a(const
> rtems_irq_number irqLine)
> /*-------------------------------------------------------------------------
>+
>
> | Function: BSP_irq_enable_at_i8259a
> | Description: Unmask IRQ line in appropriate PIC chip.
>
> -| Global Variables: i8259a_cache
> +| Global Variables: i8259a_imr_cache, i8259a_in_progress
>
> | Arguments: irqLine - number of IRQ line to mask.
> | Returns: Nothing.
>
>
> +--------------------------------------------------------------------------
>*/ @@ -152,19 +167,19 @@ static int BSP_irq_enable_at_i8259a(const
> rtems_irq_number irqLine) rtems_interrupt_disable(level);
>
> mask = 1 << irqLine;
> - i8259a_cache &= ~mask;
> + i8259a_imr_cache &= ~mask;
>
> if (irqLine < 8)
> {
> isr = BSP_i8259a_irq_in_service_reg(PIC_MASTER_COMMAND_IO_PORT);
> irr = BSP_i8259a_irq_int_request_reg(PIC_MASTER_COMMAND_IO_PORT);
> - outport_byte(PIC_MASTER_IMR_IO_PORT, i8259a_cache & 0xff);
> + BSP_i8259a_irq_update_master_imr();
> }
> else
> {
> isr = BSP_i8259a_irq_in_service_reg(PIC_SLAVE_COMMAND_IO_PORT);
> irr = BSP_i8259a_irq_int_request_reg(PIC_SLAVE_COMMAND_IO_PORT);
> - outport_byte(PIC_SLAVE_IMR_IO_PORT, (i8259a_cache >> 8) & 0xff);
> + BSP_i8259a_irq_update_slave_imr();
> }
>
> if (((isr ^ irr) & mask) != 0)
> @@ -298,7 +313,7 @@ void BSP_dispatch_isr(int vector);
>
> void BSP_dispatch_isr(int vector)
> {
> - uint16_t old_imr = 0;
> + rtems_i8259_masks in_progress_save = 0;
>
> if (vector < BSP_IRQ_VECTOR_NUMBER) {
> /*
> @@ -329,10 +344,10 @@ void BSP_dispatch_isr(int vector)
> * vector clear.
> */
> if (vector <= BSP_IRQ_MAX_ON_i8259A) {
> - old_imr = i8259a_cache;
> - i8259a_cache |= irq_mask_or_tbl[vector];
> - outport_byte(PIC_MASTER_IMR_IO_PORT, i8259a_cache & 0xff);
> - outport_byte(PIC_SLAVE_IMR_IO_PORT, (i8259a_cache >> 8) & 0xff);
> + in_progress_save = i8259a_in_progress;
> + i8259a_in_progress |= irq_mask_or_tbl[vector];
> + BSP_i8259a_irq_update_master_imr();
> + BSP_i8259a_irq_update_slave_imr();
> }
>
> /*
> @@ -365,11 +380,9 @@ void BSP_dispatch_isr(int vector)
> * again.
> */
> if (vector <= BSP_IRQ_MAX_ON_i8259A) {
> - if (irq_trigger[vector] == INTR_TRIGGER_LEVEL)
> - old_imr |= 1 << vector;
> - i8259a_cache = old_imr;
> - outport_byte(PIC_MASTER_IMR_IO_PORT, i8259a_cache & 0xff);
> - outport_byte(PIC_SLAVE_IMR_IO_PORT, (i8259a_cache >> 8) & 0xff);
> + i8259a_in_progress = in_progress_save;
> + BSP_i8259a_irq_update_master_imr();
> + BSP_i8259a_irq_update_slave_imr();
> }
> }
> }
--
More information about the devel
mailing list