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. > > II - VMCALL > > II.1 - problem > hypercall_init() @ checks for X86_FEATURE_VMX to use either VMCALL or > VMMCALL. > but vcpu_handle_cpuid() @ hypervisor/arch/x86/vcpu.c always clears this > bit for non root cell. > Thus, hypercalls in non AMD system do not work at all in non-root cells > based on inmates demo code! Yeah, that's a regression of my commit 39dfc93aa472... > > II.2 - possible solution > > Linux as a guest uses a "synthetic" bit, X86_FEATURE_VMMCALL, > see > https://elixir.bootlin.com/linux/v4.16.1/source/arch/x86/include/asm/cpufeatures.h#L225 > > to detect proper alternative > #define KVM_HYPERCALL \ > ALTERNATIVE(".byte 0x0f,0x01,0xc1", ".byte 0x0f,0x01,0xd9", > X86_FEATURE_VMMCALL) > > I would suggest the same, adding a bit in cpuid leaf > (JAILHOUSE_CPUID_FEATURES). > That X86_FEATURE_VMMCALL is set by Linux itself. We should port that logic over: wwitch to VMMCALL if and only if an AMD CPU is detected (linux/arch/x86/kernel/cpu/amd.c), otherwise stick with Intel's VMCALL. Patch welcome! 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 jailhouse-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
