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.

> 
>> +}
>> 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.

  Ralf

> 
> 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.

Reply via email to