[PATCH 3/3] libchip/network/if_fxp.c: do not use rtems_interrupt_disable.
Pavel Pisa
ppisa4lists at pikron.com
Thu Oct 13 09:26:14 UTC 2016
Some typo corrections of e-mail written when I have returned
late in night from meeting with friends.
And some more clarification as well.
On Thursday 13 of October 2016 01:55:30 Pavel Pisa wrote:
> Hello Chris,
>
> On Wednesday 12 of October 2016 23:05:30 Chris Johns wrote:
> > On 13/10/2016 03:22, Pavel Pisa wrote:
> > > But RTEMS i8269 support has been broken to disable
> > > vector for level triggered interrupts in generic
> > > IRQ processing code.
> >
> > I am not sure where the blame should be placed. We need to disable at
> > the PIC when using libbsd with shared PCI interrupts because an
> > interrupt server is used that is common to a few architectures. Some
> > legacy drivers like this one assume processing inside the interrupt
> > context. It is not clear to me shared interrupts were ever supported
> > with these drivers. I would assume it means some type of per driver
> > interrupt chaining.
> >
> > > So I have introduced reenable
> > > bsp_interrupt_vector_enable to ensure that driver
> > > can work even with that setup.
> >
> > I am not sure we can mix both models without some changes.
>
> I hope that interrupt server should work after the committed change.
> At least, I have feeling that it has been outcome of previous debate.
>
> The IRQ server bsp_interrupt_server_trigger() disables given IRQ
> vector on PIC level in hardware IRQ context by
> bsp_interrupt_vector_disable()
>
> See
>
>
> https://git.rtems.org/rtems/tree/c/src/lib/libbsp/shared/src/irq-server.c#n
>64
>
> I would not push the changes if it has not be a case.
>
> > > classic networking: adapt FXP driver to work with actual PCI and IRQ
> > > code.
> > >
> > > The hack is not required after
> >
> > Which hack?
>
> The reenabling of PIC level ISR in driver code. Generally I consider
> the functions bsp_interrupt_vector_disable() and
> bsp_interrupt_vector_enable() should be used as paired and use should allow
> to use them even
> if implemented as counting disable clock> .
spellchecker ...
s/clock/lock/
The counting of disable calls is required if vector is shared
because if multiple hard IRQ handlers needs to disable
source at controller level (generally bad practice, better is handling
on device level if possible) then if source is enabled on controller level
when the first worker theread finishes processing and cause of the shared
level triggered IRQ is other device for which threaded handler
dis not finish yet then there is complete system dead/livelock.
Linux provides next functions to maintain controller side
interrupt disable and enable
https://www.kernel.org/doc/htmldocs/kernel-api/hardware.html#idp11592384
disable_irq - this function guarantees that after it finishes
corresponding IRQ source handlers are not invoked
and not running in parallel (wait for their finish)
This function cannot be called in the handler itself
(deadlock).
disable_irq_nosync - disables vector at controller level, does not guarantee
that the last actually running instances of each of shared
handlers is finished before call return. This can be called
for source from its own hard context handler.
enable_irq - undoes effect of the corresponding disable_irq, it is necessary
to call it as many times as disable_irq has been called before.
Only when all calls are balanced then controller enables
source.
I this that when all possible HW and SW constellations are possible then
this is only usable API.
And yes, there are strange thing in the world.
I have debugged over e-mail my CAN driver at another university when I have
found that PCI card has level triggered IRQ output but multiple CAN
controllers connected to local bus behind card PCI bridge has shared
interrupts and bridge has asserted interrupt only at shared signal
raising edge. So if PCI interrupt processing finished without beeing
sure that all chips behind bridge has their output inactive then
the device and CAN control/monitoring inside some intelligent van
on the street has been lost. Fortunately not without driver at that time.
> That is implementation where bsp_interrupt_vector_enable() enables vector
> only after same number of calls as was the number of calls
> bsp_interrupt_vector_enable()
>
> > > bsps/i386: Separate variable for i8259 IRQs disable due to in progress
> > > state.
> > >
> > > so I have removed unneeded reenable from daemon hot path.
> > > I have left it in the setup to be sure that it is enabled
> > > after some driver stop start cycles.
> > >
> > > In theory, this occurrence should be deleted as well.
> > >
> > > Generally, I am not sure if/how much I have broken/I am
> > > breaking i386 support by all these changes.
> >
> > I have not testing the i386 with libbsd with your recent changes. I will
> > see what I can do. I did not notice the enables/disables had been
> > changed.
> >
> > > I believe only, hat it is the right direction.
> >
> > I am sorry it is not clear to me what direction this is.
>
> I have on mind changes required for i386 BSP SMP build and generally
> that all RTEMS code (not only x86) should eliminate constructs based
> on UP assumptions or directly incompatible with SMP. That is
> rtems_interrupt_lock_acquire should be used to protect low level operations
> which has to be serialized (short system level/hardware critical sections).
>
> i386 support needs to move forward. basic SMP support and LAPIC seems
> to be included. So it needs to enable its testing. The proposed changes
> allows the build. I have tested UP and SMP i386 builds only under QEMU.
I have tuneled to school an manager remote PC test system
to run the build on actual HW. So some following results have
been captured over serial port from PC box controlled by
project written by my colleague
https://github.com/wentasah/novaboot
to report real HW results.
> My test of networking, RTL and some others seems to work for both
> builds. UP and SMP build single core applications seems to run
> on real HW.
>
> SMP build with 1 CPU output on real HW
>
> -----------------------------------------------------------------
> novaboot: Serial line interaction (press Ctrl-C to interrupt)...
> Scanning from 0x9D400 for 1024 bytes
> Scanning from 0xF0000 for 65536 bytes
> Found MP Floating Structure Pointer at FDA40
> Intel MultiProcessor Spec 1.4 BIOS support detected
> APIC config: "Virtual Wire mode" Local APIC address: 0xFEE00000
> OEM id: CBX3 Product id: DELL
> Bus id 0 is PCI
> Bus id 1 is PCI
> Bus id 2 is PCI
> Bus id 3 is PCI
> Bus id 4 is ISA
> I/O APIC id 2 ver 32, address: 0xFEC00000
> WARNING!! Found more CPUs (4) than configured for (1)!!
>
>
> RTEMS v 4.11.99
> Starting application appfoo v 0.1.0
> *** Starting up Task_1 ***
> Task_1 woken
>
> RTEMS Shell on /dev/console. Use 'help' to list commands.
> [/] # Task_1 woken
> Task_1 woken
> -----------------------------------------------------------------
>
> When I try to enable two CPUs and try to run SMP build on QEMU
> and real HW then it breaks. REAL HW output
>
> -----------------------------------------------------------------
> i386: isr=0 irr=1
> Scanning from 0x9D400 for 1024 bytes
> Scanning from 0xF0000 for 65536 bytes
> Found MP Floating Structure Pointer at FDA40
> Intel MultiProcessor Spec 1.4 BIOS support detected
> APIC config: "Virtual Wire mode" Local APIC address: 0xFEE00000
> OEM id: CBX3 Product id: DELL
> Processor [APIC id 0 ver 21]: #0 BootStrap Processor (BSP)
> Processor [APIC id 2 ver 21]:
>
> i386: isr=0 irr=1
> Scanning from 0x9D400 for 1024 bytes
> Scanning from 0xF0000 for 65536 bytes
> Found MP Floating Structure Pointer at FDA40
> Intel MultiProcessor Spec 1.4 BIOS support detected
> APIC config: "Virtual Wire mode" Local APIC address: 0xFEE00000
> OEM id: CBX3 Product id: DELL
> Processor [APIC id 0 ver 21]: #0 BootStrap Processor (BSP)
> Processor [APIC id 2 ver 21]:
>
> -----------------------------------------------------------------
>
> Run on QEMU output
>
> -----------------------------------------------------------------
>
> i386: isr=0 irr=1
> Scanning from 0x9FC00 for 1024 bytes
> Scanning from 0xF0000 for 65536 bytes
> Found MP Floating Structure Pointer at F6BC0
> Intel MultiProcessor Spec 1.4 BIOS support detected
> APIC config: "Virtual Wire mode" Local APIC address: 0xFEE00000
> OEM id: BOCHSCPU Product id: 0.1
> Processor [APIC id 0 ver 20]: #0 BootStrap Processor (BSP)
> Processor [APIC id 1 ver 20]:
> Bus id 0 is PCI
> Bus id 1 is ISA
> I/O APIC id 0 ver 17, address: 0xFEC00000
>
> -----------------------------------------------------------------
>
> But it at least can be build and starts to some level.
>
> Is there some previous record of running RTEMS on SMP?
> To which point support has been reached?
> Are patches used to allow SMP build at that time
> archived somewhere?
>
> This round of changes tries to make i8259 PIC functions
> usable on SMP system. The longer term goal should be
> to move completely away from PIC where it is possible.
>
> If we do not need PIC then Jailhouse support should be
> much more feasible goal. I think that parallel run of RTEMS
> and Linux under Jailhouse can be good not only for testing
> but could have real applications use.
>
> All that is long term aim, I cannot promise to have time
> any of these steps. These are possible tasks for next GSoCs,
> research or industrial partners projects.
>
> But when I have tried to help with networking Saeed and prepared
> some changes I try to integrate these fixes/enhancements
> to can be used as small steps in direction which I take as logical
> and probably the best option for future i386 work.
>
> Best wishes,
>
> Pavel
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list