This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE :Bug in CtrlC handling when sharing Ethernet btw Redboot/eCos


The solution works. Here is the corresponding patch.

To eCos maintainers : my opinion is that it should be included in CVS
tree to provide an always working ctrlc when ethernet is shared between
eCos app and Redboot/stubs. The drawback is the break always occuring in
the function cyg_hal_user_break(), but it is still useful since the user
can then navigate between the different threads with the 'thread
awreness' functionality included in stubs/GDB. This drawback is
unfortunately necessary since it's not possible with the current
Ethernet driver architecture, to check Ethernet frames directly in the
ISR, which would be necessary for a cleaner solution.

Regards,
Arnaud Chataignier.

-----Message d'origine-----
De : ecos-discuss-owner@ecos.sourceware.org
[mailto:ecos-discuss-owner@ecos.sourceware.org] De la part de Arnaud
Chataignier
Envoyé : mardi 2 novembre 2004 09:48
À : 'Bart Veer'
Cc : ecos-discuss@sources.redhat.com
Objet : [ECOS] RE : Bug in CtrlC handling when sharing Ethernet btw
Redboot/eCos


Dear Bart,

Thank you for your answer on this topic. I agree with your conclusions.

Concerning the thread priority, however, the 'Network alarm support'
thread has the highest priority in my app (lowest priority value), so I
think that the area pointed to by hal_saved_interrupt_state is rewritten
by another interrupt. But I admit that the real reason of this
corruption is not crystal clear in my spirit. Hint: I use a separate
stack for interrupts. May be the timer interrupt trigs and rewrites the
area pointed to by hal_saved_interrupt_state.

Concerning the resolution of the problem, I agree there is no perfect
solution. We have two choices:

1 - We save the area pointed to by hal_saved_interrupt_state (at least
the PC reg value). This solution will not necessarily work in every
situations since this leads in setting a breakpoint in some function
that may already have been called, if the current thread has a higher
priority than the Network alarm thread. So this is not deterministic,
and this is not a really 'always working' solution.

2 - as you say, we can also clear hal_saved_interrupt_state before in
eth_drv_run_deliveries() before calling HAL_CTRLC_CHECK. This will lead
to always break execution in eth_drv_run_deliveries(), but the user can
still see the state of other threads by using the 'thread awareness'
functionnality of GDB/stubs. Note that just clearing the variable
hal_saved_interrupt_state in eth_drv_run_deliveries() is not enough,
since a new interrupt (timer for instance) can occur between the
variable clearance, and the call to cyg_hal_user_break()(). But I know
how to overcome this problem by using a global variable (the ugly
solution I described below).

I will try solution 2, and will post a patch if it works. Then it will
be up to eCos maintainers to decide if this not perfect, but
deterministic solution, is better or not than the 'crash sometimes'
current implementation.

Regards,
Arnaud.

-----Message d'origine-----
De : Bart Veer [mailto:bartv@ecoscentric.com] 
Envoyé : mardi 2 novembre 2004 00:45
À : achataignier@neotion.com
Cc : ecos-discuss@sources.redhat.com
Objet : Re: [ECOS] [ECOS/STUB] Bug in CtrlC handling when sharing
Ethernet btw Redboot/eCos


>>>>> "Arnaud" == achataignier  <achataignier@neotion.com> writes:

    Arnaud> I found a bug in the CtrlC handling when sharing the
    Arnaud> Ethernet connection between eCos application and
    Arnaud> Redboot/stubs.

    Arnaud> What happens is that HAL_CTRLC_CHECK is called from the
    Arnaud> function eth_drv_run_deliveries() in
    Arnaud> io/eth/current/src/net/eth_drv.c. This function is in a
    Arnaud> thread context, which is not compatible with the way
    Arnaud> hal_ctrlc_check calls internally cyg_hal_user_break(),
    Arnaud> since it assumes it is being called from an ISR.

I did not write the code in question, but at first glance this is not
necessarily true. hal_ctrlc_check() passes the current value of
hal_saved_interrupt_state and cyg_hal_user_break() checks for a null
pointer in case it is not being called from an ISR. However...

    Arnaud> The result is CtrlC not working sometimes, particularly
    Arnaud> when some time happens between the Ethernet ISR, and the
    Arnaud> moment cyg_hal_user_break() is called, since the stack
    Arnaud> area pointed to by hal_saved_interrupt_state may have been
    Arnaud> rewritten in between. This is the case for instance if the
    Arnaud> host sends an ARP request to Redboot before sending the
    Arnaud> Ctrlc.

    Arnaud> I see a ugly solution by adding a global variable, set by
    Arnaud> eth_drv_run_deliveries() and checked by
    Arnaud> cyg_hal_user_break(), but this is not as clean as I would
    Arnaud> want it to be.

    Arnaud> Any suggestion for a clean fix ?

I think this problem can occur whenever there is runnable thread with a
higher priority than the TCP/IP delivery thread. Unfortunately there may
not be a good solution. One approach would be to clear
hal_saved_interrupt_state, probably in the kernel's interrupt_end()
routine. That ensures the ctrl-C code will not try to interpret a bogus
save state. Unfortunately it will also mean that the gdb stubs code will
no longer know which thread was interrupted by the ctrl-C. Instead it
will always seem to be the delivery thread that is being interrupted.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Attachment: hal_if.patch
Description: Binary data

Attachment: net_eth_drv.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]