On 2018-05-07 15:35, Ralf Ramsauer wrote: > On 05/07/2018 03:25 PM, Jan Kiszka wrote: >> On 2018-05-07 14:12, Ralf Ramsauer wrote: >>> Don't jump directly to inmate_main from assembly, jump to c_entry. >>> >>> At the moment, c_entry does nothing else than jumping to inmate_main and >>> ending up in an endless loop if inmate_main returns. >>> >>> Later we can use c_entry to do platform independant setup/checks. >>> >>> Signed-off-by: Ralf Ramsauer <[email protected]> >>> --- >>> inmates/lib/arm-common/Makefile.lib | 2 +- >>> inmates/lib/arm/header.S | 2 +- >>> inmates/lib/arm64/header.S | 2 +- >>> inmates/lib/setup.c | 9 +++++++++ >>> inmates/lib/x86/Makefile | 2 +- >>> inmates/lib/x86/header.S | 2 +- >>> 6 files changed, 14 insertions(+), 5 deletions(-) >>> create mode 100644 inmates/lib/setup.c >>> >>> diff --git a/inmates/lib/arm-common/Makefile.lib >>> b/inmates/lib/arm-common/Makefile.lib >>> index fb3b6585..9edb2d9f 100644 >>> --- a/inmates/lib/arm-common/Makefile.lib >>> +++ b/inmates/lib/arm-common/Makefile.lib >>> @@ -36,7 +36,7 @@ >>> # THE POSSIBILITY OF SUCH DAMAGE. >>> # >>> >>> -objs-y := ../string.o ../cmdline.o >>> +objs-y := ../string.o ../cmdline.o ../setup.o >>> objs-y += printk.o gic.o timer.o >>> objs-y += uart-jailhouse.o uart-pl011.o uart-8250.o uart-8250-8.o >>> objs-y += uart-xuartps.o uart-mvebu.o uart-hscif.o uart-scifa.o uart-imx.o >>> diff --git a/inmates/lib/arm/header.S b/inmates/lib/arm/header.S >>> index d0a8f219..be1d23a8 100644 >>> --- a/inmates/lib/arm/header.S >>> +++ b/inmates/lib/arm/header.S >>> @@ -85,6 +85,6 @@ __reset_entry: >>> >>> 2: ldr sp, =stack_top >>> >>> - b inmate_main >>> + b c_entry >>> >>> .ltorg >>> diff --git a/inmates/lib/arm64/header.S b/inmates/lib/arm64/header.S >>> index e284aa5a..011aeb63 100644 >>> --- a/inmates/lib/arm64/header.S >>> +++ b/inmates/lib/arm64/header.S >>> @@ -57,7 +57,7 @@ __reset_entry: >>> >>> isb >>> >>> - b inmate_main >>> + b c_entry >>> >>> handle_irq: >>> bl vector_irq >>> diff --git a/inmates/lib/setup.c b/inmates/lib/setup.c >>> new file mode 100644 >>> index 00000000..ed35b6f8 >>> --- /dev/null >>> +++ b/inmates/lib/setup.c >>> @@ -0,0 +1,9 @@ >>> +#include <inmate.h> >>> + >>> +void __attribute__((noreturn)) c_entry(void); >>> + >>> +void __attribute__((noreturn)) c_entry(void) >>> +{ >>> + inmate_main(); >>> + spin_forever(); >> >> When spin_forever/halt used like that, we should ensure first that >> interrupts are disabled. > > Hmm, yes, they could be disabled. Though they shouldn't harm us. > > What about a architecture-dependent stop() or stop_machine() function > that first disables IRQs and then enters halt()? > > Do you also want to deinitialise the GIC? Shouldn't be necessary, though.
If we just define this as stop(), I see no point in doing it in c_entry anymore. Then there will be only one user, and that one can also just return to header and let it handle the stop. > >> >>> +} >>> diff --git a/inmates/lib/x86/Makefile b/inmates/lib/x86/Makefile >>> index de4d74b7..6561d0cf 100644 >>> --- a/inmates/lib/x86/Makefile >>> +++ b/inmates/lib/x86/Makefile >>> @@ -41,7 +41,7 @@ include $(INMATES_LIB)/Makefile.lib >>> always := lib.a lib32.a >>> >>> TARGETS := header.o hypercall.o ioapic.o printk.o smp.o >>> -TARGETS += ../pci.o ../string.o ../cmdline.o >>> +TARGETS += ../pci.o ../string.o ../cmdline.o ../setup.o >>> TARGETS_64_ONLY := int.o mem.o pci.o timing.o >>> >>> ccflags-y := -ffunction-sections >>> diff --git a/inmates/lib/x86/header.S b/inmates/lib/x86/header.S >>> index 95c7a5a4..1df4345b 100644 >>> --- a/inmates/lib/x86/header.S >>> +++ b/inmates/lib/x86/header.S >>> @@ -116,7 +116,7 @@ start64: >>> mov $bss_qwords,%rcx >>> rep stosq >>> >>> - mov $inmate_main,%rbx >>> + mov $c_entry,%rbx >>> >>> call_entry: >>> mov $stack_top,%rsp >>> >> >> x86 has assembly code after the call that also does cli; hlt. That can >> go when moving it to C. > > Ack. > >> >> And you forgot header-32.S on x86. > > Ups, sorry. > >> >> Might be worth checking at some point if there is more to convert, >> though we would then need arch_c_entry() as well. > > Yeah, I also thought of that. But at the moment there's nothing arch > dependent. And my uart patches won't introduce anything architecture > dependent as well. Maybe there will be something in the future, but it's > not a big issue to add an arch_c_entry() hook. > Right, was more a note to eventually check the existing logic again. Or thinking of it when adding more (like cache enabling). Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
