On 05/07/2018 03:42 PM, Jan Kiszka wrote: > 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.
We need at least a arch_disable_irqs(), if you do want to disable IRQs before halt(). I'd like to stick to the C-version as halt() gets more users and we keep assembly low. Ralf > >> >>> >>>> +} >>>> 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 > -- 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.
