On 2018-04-11 11:24, Ralf Ramsauer wrote:
> Hi,
> 
> On 04/11/2018 11:17 AM, f...@ozog.com wrote:
>> On 11.04.2018 10:27, Jan Kiszka wrote:
>>> On 2018-04-11 09:55, f...@ozog.com wrote:
>>>> In trying to get the tiny-demo send logs to jailhouse debug console I
>>>> found that:
>>>> - inmate lib handling of cmdline is broken
>>>> - inmate lib detection whether to use VMCALL (Intel) or VMMCALL (AMD)
>>>> instructions to do hypercalls is broken
>>>>
>>>> I found fixes but I'd like to discuss how to properly address the issues
>>>>
>>>> I -  cmdline
>>>>
>>>> I.1 - problem
>>>>
>>>> according to inmates/lib/x86/inmate.lds
>>>> cmdline is located at offset 0x100 in the running inmate
>>>> it contains one byte (a zero).
>>>> inmate code starts at 0x101
>>>
>>> Well, that's the location of the text section if there is nothing in the
>>> inmate that otherwise uses the cmdline section. But if your inmate is
>>> using the command line (lib/cmdline.c), it will usually reserve another
>>> 256 bytes (CMDLINE_BUFFER_SIZE). If something else provides a
>>> CMDLINE_BUFFER, that will will and may change that value.
>>>
>>>>
>>>> according to Documentation/debug-output.md
>>>> command line parameters are specified using -s <STRING>
>>>>
>>>> lastly, cell_shutdown_load() in tools/jailhouse.c considers "-s
>>>> <STRING>" as a second image to be loaded
>>>>
>>>> so the command
>>>>
>>>> jailhouse cell load --name tiny-demo inmates/demos/x86/tiny-demo.bin -s
>>>> "con-type=JAILHOUSE"
>>>
>>> ...is wrong. It's missing the "-a 0x100" on x86 (would be 0x1000 on ARM
>>> architectures). The documentation above is listing this in the examples
>>> section.
>>>
>>> Please suggest better wording of that document that could help to avoid
>>> such misunderstanding.
>>>
>>>>
>>>> has the following effect:
>>>>
>>>> load_image() @ driver/cell.c uses the first available memory_region and
>>>> copies inmates/demos/x86/tiny-demo.bin
>>>> to that location
>>>> then load_image() actually uses the same memory_region (same physical
>>>> start but ioremapped at a different location)
>>>> and overwrites the first bytes of inmates/demos/x86/tiny-demo.bin.
>>>>
>>>> Unexpectedly, the string "con-type=JAILHOUSE" happen to correspond to
>>>> executable code bytes and it does "something" on my system.
>>>>
>>>> I.2 - possible solution
>>>>
>>>> the simplest way to solve this is:
>>>> - reserve say the rest of the first page to boot code and command line:
>>>> a inmate.lds patch can do the job
>>>> - change jailhouse.c to load the fake image at offset 0x100 (updating
>>>> target_address) so that driver.c does not override the .boot section
>>>>
>>>> If this is acceptable for now, I can publish patches.
>>>
>>> This is a documentation issue, at most. See above.
>>
>> I now understand.
>>
>> My reading of
>>    cell load { ID | [--name] NAME } { IMAGE | { -s | --string } "STRING" }
>>              [-a | --address ADDRESS] ...
>>
>> was either wrong or have to be improved
>>
>> adding {} seems clearer to me:
>>    cell load { ID | [--name] NAME } { { IMAGE | { -s | --string }
>> "STRING" }
>>              [-a | --address ADDRESS] } ...
>>
>> The following example is misleading:
>>
>>     jailhouse cell load foocell inmate.bin -s "con-type=JAILHOUSE" -a 0x100
>>
>> it looks like -s and -a are parameters to inmate.bin
>>
>> The following better shows that we are loading two images,
>> the second one patching the first one at offset 0x100
>>
>>     jailhouse cell load foocell
>>         inmate.bin -a 0 \
>>         -s "con-type=JAILHOUSE" -a 0x100
>>
>> which can be abbreviated to :
>>
>>     jailhouse cell load foocell
>>         inmate.bin \
>>         -s "con-type=JAILHOUSE" -a 0x100
>>
>> The following sample illustrate the overall concepts of multiple images
>> loading
>>
>>     jailhouse cell load foocell \
>>         inmate.bin \
>>         -s "con-type=JAILHOUSE" -a 0x100 \
>>         sharedobject.so -a 0x1000000 \
>>         ramfs.bin -a 0x2000000
>>
>> If this is correct, I'll propose a set of Documentation patches
> This is correct. You could also explicitly mention that the address is
> defaulted to 0x0.
> 
> And that images or strings are loaded in the same order as specified, if
> regions overlap, the latter will overwrite the content.

Maybe time for jailhouse.8...? :)

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 jailhouse-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to