Skip to content

native: allow for native to be resetable via SIGUSR1#12517

Merged
miri64 merged 2 commits intoRIOT-OS:masterfrom
miri64:native/enh/reset-command
Dec 3, 2019
Merged

native: allow for native to be resetable via SIGUSR1#12517
miri64 merged 2 commits intoRIOT-OS:masterfrom
miri64:native/enh/reset-command

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Oct 20, 2019

Contribution description

Based on the discussion sparked by this comment #12448 (comment) I decided to provide a simple PoC of how to make native resetable.

Testing procedure

Start a native instance in one terminal (application is arbitrary), find out its PID in another terminal, and then use the PID to reset e.g.:

$ pgrep -f native/hello-world
96937
$ DEBUG_ADAPTER_ID=96937 make -C examples/hello-world reset

The native instance should then reboot (noticeable by a big fat !! REBOOT !! printed)

Issues/PRs references

#12448 (comment)

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: native Platform: This PR/issue effects the native platform Area: boards Area: Board ports State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. labels Oct 20, 2019
@miri64 miri64 added this to the Release 2020.01 milestone Oct 20, 2019
@miri64 miri64 requested review from fjmolinas and kaspar030 October 20, 2019 22:14
@miri64 miri64 mentioned this pull request Oct 20, 2019
Open
9 tasks
@miri64 miri64 force-pushed the native/enh/reset-command branch from 40129dc to 943fd68 Compare October 20, 2019 22:15
@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Dec 3, 2019

Test, works as advertised.

LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-pr-12517)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.


                !! REBOOT !!

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-pr-12517)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.

@smlng
Copy link
Copy Markdown
Member

smlng commented Dec 3, 2019

on FreeBSD I get:

~/devel/RIOT/examples/hello-world/bin/native/hello-world.elf: native_irq_handler: ignoring SIGUSR1

@smlng
Copy link
Copy Markdown
Member

smlng commented Dec 3, 2019

comes from here https://github.com/RIOT-OS/RIOT/blob/master/cpu/native/irq_cpu.c#L266

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/startup.c Outdated

/* should not error as SIGUSR is a valid error code and EINVAL is the only
* specified error code => don't check return value */
signal(SIGUSR1, reset_handler);
Copy link
Copy Markdown
Member

@smlng smlng Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to (should) use: https://github.com/RIOT-OS/RIOT/blob/master/cpu/native/irq_cpu.c#L412 instead

Copy link
Copy Markdown
Member

@smlng smlng Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or additionally?

Copy link
Copy Markdown
Member Author

@miri64 miri64 Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be.

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/startup.c Outdated
extern init_func_t __init_array_end;
#endif

static void reset_handler(int signo)
Copy link
Copy Markdown
Member

@smlng smlng Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to

static void _reset_handler(void)
{
    pm_reboot();
}

and added:

register_interrupt(SIGUSR1, _reset_handler);

below and it works on FreeBSD, the signal(...) is not needed

@smlng
Copy link
Copy Markdown
Member

smlng commented Dec 3, 2019

with the proposed changes it looks good:

/devel/RIOT/examples/hello-world/bin/native/hello-world.elf
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-HEAD)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.


                !! REBOOT !!

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-HEAD)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 3, 2019

I will apply the changes @smlng proposed and retest on Linux.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 3, 2019

I will apply the changes @smlng proposed and retest on Linux.

Done and still works on Linux. Thanks for testing on FreeBSD, @smlng!

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/include/native_internal.h Outdated
*
* @see e.g. https://www.freebsd.org/cgi/man.cgi?query=signal&section=3
*/
typedef void (*real_sig_t) (int);
Copy link
Copy Markdown
Member

@smlng smlng Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed anymore, right?

Copy link
Copy Markdown
Member Author

@miri64 miri64 Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, sorry.... still on my first coffee ^^"

Copy link
Copy Markdown
Member

@smlng smlng Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enjoy that first ☕️

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/include/native_internal.h Outdated
*__restrict value, struct itimerval *__restrict ovalue);
extern int (*real_setsid)(void);
extern int (*real_setsockopt)(int socket, ...);
extern real_sig_t (*real_signal)(int sig, real_sig_t handler);
Copy link
Copy Markdown
Member

@smlng smlng Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here?

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/syscalls.c Outdated
*restrict value, struct itimerval *restrict ovalue);
int (*real_setsid)(void);
int (*real_setsockopt)(int socket, ...);
real_sig_t (*real_signal)(int sig, real_sig_t handler);
Copy link
Copy Markdown
Member

@smlng smlng Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also not needed anymore?

@smlng smlng added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Dec 3, 2019
@smlng
Copy link
Copy Markdown
Member

smlng commented Dec 3, 2019

when done, please squash and trigger Murdock please

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 3, 2019

Cleaned out now unnecessary changes and squashed.

@miri64 miri64 force-pushed the native/enh/reset-command branch from a2ce01d to d670f77 Compare December 3, 2019 08:52
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 3, 2019
smlng
smlng approved these changes Dec 3, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested ACK: Linux and FreeBSD

@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Dec 3, 2019

tested ACK: Linux and FreeBSD

I guess FreeBSD implies OSX?

@smlng
Copy link
Copy Markdown
Member

smlng commented Dec 3, 2019

RIOT does not work on latest macOS, bc the latter is 64bit only now - so no macOS right now

@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Dec 3, 2019

RIOT does not work on latest macOS, bc the latter is 64bit only now - so no macOS right now

@x3ro are you still working on a 64-bit native port? (:

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 3, 2019

tested ACK: Linux and FreeBSD

I guess FreeBSD implies OSX?

There is I think twenty years of divergence between FreeBSD and Darwin ^^, but okay :-)

@miri64 miri64 merged commit 2f74d9d into RIOT-OS:master Dec 3, 2019
@miri64 miri64 deleted the native/enh/reset-command branch December 3, 2019 09:56
@miri64 miri64 mentioned this pull request Dec 3, 2019
Closed
@miri64 miri64 mentioned this pull request Feb 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaspar030 kaspar030 Awaiting requested review from kaspar030

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

1 more reviewer

@smlng smlng smlng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.01

Development

Successfully merging this pull request may close these issues.

5 participants

@miri64 @fjmolinas @smlng @kaspar030 @cgundogan