f9micro / f9-kernel

An efficient and secure microkernel built for ARM Cortex-M cores, inspired by L4
Other
683 stars 145 forks source link

Lack of a functional CONFIG_DEBUG build option #28

Closed jserv closed 11 years ago

jserv commented 11 years ago

For historical reasons, F9 Microkernel uses two similar definitions DEBUG and CONFIG_DEBUG for picking up the configuration. However, this option is not really functioned because it breaks the build while the option is disabled,

If either DEBUG or CONFIG_DEBUG is disabled, no serial I/O is configured in kernel, but we can still utilize some facilities, such as st-term in stlink package, to verify system.

We should at least decouple DEBUG / CONFIG_DEBUG from build process first and then provide the mechanism for non-serial I/O.

vh21 commented 11 years ago

I think there are some definition which is not so clear, for example

Whatever, I list all the cases which cause compiling error,

kernel/memory.c:356:14: error: 'kdb_mempool_prop' defined but not used [-Werror=unused-function]
kernel/softirq.c:27:14: error: 'softirq_names' defined but not used [-Werror=unused-variable]
kernel/start.c:27:13: error: 'banner' defined but not used [-Werror=unused-variable]
kernel/thread.c: In function 'kdb_dump_threads':
kernel/thread.c:398:8: error: unused variable 'state' [-Werror=unused-variable]
kernel/thread.c: At top level:
kernel/thread.c:375:14: error: 'kdb_get_thread_type' defined but not used [-Werror=unused-function]
jserv commented 11 years ago

@vh21 I think we should provide the dropin replacement for existing dbg_xxx seris and then decouple from each other.

vh21 commented 11 years ago

I have a proposal as follows,

  1. tweak stdio.h to be standard posix naming. For example, use printf() for banner
  2. define some macro, ex DBG_PRINTF, to call dbg_printf().
  3. Use CONFIG_DEBUG in Kconfig to control the switch.

By the way, I think KDB belongs to normal message output, therefore is changed to use printf(). How do you think about it?

jserv commented 11 years ago

@vh21 We expect the implementation functions come with prefix _l4. However, UART is optional regarding the use scenario of deep embedded environments such as IoT. The proper way is either providing build time switch or runtime dispatcher, which determinates the real paths (in-memory logger, UART, etc.).

vh21 commented 11 years ago

The tree for tweaking debug information, and first of all, some commits to clean up config menu

jserv commented 11 years ago

@vh21 Got it. I already merged branch tweak-dbg. However, we do need the further functional facilities even without UART.

vh21 commented 11 years ago

I got some thought about UART, it's under testing now. I will upload a draft these days. Basically I prefer a dynamic way and wrap the selections by config. Then we can have the flexiblility of selections and reduce memory footprint by disabling some config.

vh21 commented 11 years ago

It should be almost done in branch tweak-dbg. Please help to review it but don't merge right now. I will send pull request after fully verified and cleaned up. Please give me one to 2 days.

Basically __l4_putchar() and __l4_getchar() can be re-defined or mapped to debug IO port if CONFIG_STDIO_USE_DBGPORT is chosen. It's the default config now.

banner is changed to use __l4_printf() because I think it should be the normal message, not debug message.

The debug IO is abstracted in platform/debug_device.c, debug_uart belongs to one of it. The debug device selection locates in platform/Kconfig.debug.

More information will be exposed after cleaning up source.

jserv commented 11 years ago

This issue is intended to get closed once we have one working RAM based virtual device as the alternative for UART based implementation.

jserv commented 11 years ago

Conceptual diagram: kdb-uart

arcbbb commented 11 years ago

Execuse me, what is the tool to produce the above conceptual diagram? I thought it's good for communicating and recording ideas while discussing.

vh21 commented 11 years ago

How to get data from RAM devices? by using dbg_getchar()? By the way, while accessing these debugging devices, should we protect them from multitask environment?

jserv commented 11 years ago

@arcbbb I modified the diagram by using GIMP.

jserv commented 11 years ago

@vh21 IMHO, RAM device is of single direction only. stdout is buffered by RAM device. Yes, we have to prevent from concurrent writing in advance.

vh21 commented 11 years ago

For considering that IRQ routine might use debugging IO, I plan to use irq_save() and irq_restore() for concurrent stuff.

jserv commented 11 years ago

@georgekang, can you comment @vh21 's proposal for utilizing irq_save and irq_restore?

vh21 commented 11 years ago

About string library, it seems that re-implementation is required because of license issue. How about to migrate from newlib or olibc? They're BSD license and contain some optimization. Or it's ok to add a small implementation, too. I can try to help it.

The license of newlib is https://sourceware.org/newlib/COPYING.NEWLIB. My example migrated from newlib is d601c8c523d10e4651f74c05a61bbd75c7436744. Just passed compilation and not verified yet.

vh21 commented 11 years ago

@jserv , for the concurrent part, I will try to push current one, no concurrency protection, first and tweak it in the future. How do you think about it?

My thought is that it's more proper to add protection in upper API, or there will be fragmentation in the output string. For exmaple, when some thread is sending debug string like ABCDE. Then it's interrupted by ISR, and ISR try to send 5566, too. The result output might become ABC5566DE, that's not what we'd like to see.

However I think it's not a good idea to put the protection in __l4_vprintf(), too. It will induce to much overhead in critical area.

Therefore I think we should take more effort to tweak it.

jserv commented 11 years ago

@vh21 I agree with you. We need another BSD License compatible implementation of C string functions.

jserv commented 11 years ago

@vh21 No concurrency support in first version looks fine, and there might be always one active log client which throws buffered string.

vh21 commented 11 years ago

Tweak the license in new commit e1ebea10e59f2fa45472761ef00bf8e3764cfe68.

I am not sure how to deal with license file. Therefore it's added in kernel/lib/LICENSE.NEWLIB. Within the file, I specify the files from newlib, the license of newlib and the license of F9 Microkernel. Please feel free to let me know if anything to modify.

By the way, will it get problem if there is some sources contains 3-clause BSD Lincense?

Viller

jserv commented 11 years ago

3-clause BSD license is compatible to F9 microkernel. We only have to mention it properly.

vh21 commented 11 years ago

New integration from musl libc, vh21/f9-kernel@f143bea911031a75c7314c0abc5c74afaf547bda. I pull in all the sources in string/ but only add memcpy, memset, and strcmp to building environment, as well as the license notice is added to top level LICENSE file. Only passed compiling and is not verified yet.

jserv commented 11 years ago

Let's close this issue, OK?

vh21 commented 11 years ago

It sounds good to me.