Re: [PATCH 05/19] configs: arm: add comm_region to all inmates

2018-05-02 Thread ff 
> From: JAN KISZKA <jan.kis...@web.de>
> Date: 1 May 2018 at 18:55
> Subject: Re: [PATCH 05/19] configs: arm: add comm_region to all inmates
> To: Ralf Ramsauer <ralf.ramsa...@oth-regensburg.de>, 
> jailhouse-dev@googlegroups.com
> 
> On 2018-05-01 10:54, Ralf Ramsauer wrote:
>> On 04/27/2018 08:27 PM, Jan Kiszka wrote:
>>> On 2018-04-27 11:36, Ralf Ramsauer wrote:
>>>> On x86, we already use the communication region to pass certain platform
>>>> specific information to guests. So let's do this on all ARM
>>>> architectures as well. This patch is a preparation for that.
>>>> 
>>>> Let's use guest paddr 0x8000 (2GiB) for the comm region, which
>>>> doesn't seem to collide with any existing (MMIO) region of our current
>>>> inmates.
>>> 
>>> So you really checked them all? Also validated that there is no
>>> collision with any of the virtual mmcfg region we define (that's in the
>>> system config then)?
>> 
>> I manually checked them, including the mmcfg region. But I think it's
>> worth crosschecking them.
>> 
>> Of course, 0x8000 overlaps with physical RAM on most ARM platforms,
>> but that's not an issue for the communication region in inmates.
>> 
>> The point of this address is that on most platforms, MMIO regions are
>> spread somewhere below 0x8000, where it's on the one hand hard to
>> find a common free region, and on the other it's likely to get a new
>> board where that address might be in use.
> 
> Hmm, may work for us - provided we never try to steal RAM for inmates
> from that address onward... OK, will double-check the targets.
> 
> 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 [1].

As per "Principles of ARM(R) Memory Maps White Paper", Document number:
ARM DEN 0001C, 

http://infocenter.arm.com/help/topic/com.arm.doc.den0001c/DEN0001C_principles_of_arm_memory_maps.pdf


I would say the selected location will never point to MMIO. 

-FF 

Links:
--
[1] https://groups.google.com/d/optout

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

Re: PCI devices on Arm

2018-04-26 Thread ff 

I just see how much I need to understand beyond what I asked...

How should it work from a 10,000ft view if we lived in a perfect world?

PCI controllerS are terminated on the main Linux. Jaihouse hypervisor 
take over and reexposes virtual ones to root-cell who then re-probes the 
whole space?
As IOMMUs are also attached to the PCI domains, I assume the same 
transfer must be done for IOMMUs?
Some PCI devices are "platform" devices (i.e. not really behind a PCI 
bridge but controlled through PCI) and pugable devices (those really are 
PCI devices) so the IOMMU config may not be exactly the same for those 
two types of devices.


shouldn't we have a

struct pci_domain {
__u64 pci_mmconfig_base;
__u32 pci_mmconfig_size;
__u8 pci_mmconfig_start_bus;
__u8 pci_mmconfig_end_bus;
struct jailhouse_iommu* iommu;
} __attribute__((packed));

and

struct jailhouse_system {
char signature[6];
__u16 revision;
struct jailhouse_memory hypervisor_memory;
struct jailhouse_debug_console debug_console;
struct {
struct pci_domain pci_domains[MAX_PCI_DOMAINS];
__u8 pci_is_virtual;


Cheers

FF




On 26.04.2018 18:09, Jan Kiszka wrote:

On 2018-04-26 17:53, f...@ozog.com wrote:
On Cavium ThunderX there are PCI devices and multiple PCI domains, 
each

with a set of buses and MMIO areas.
DeviceTree can be used to identify all those.

I see a flag pci_is_virtual is true on Arm.
But I am not sure I fully understand the implications.

In theory, we should add multi-domain support to allow proper device
probing here correct ?



Look at
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-April/573481.html
e.g.

Will likely become

PCI: Enable PCI_DOMAINS along with generic PCI host controller

This controller is often instantiated by hypervisors, and they may add
multiple of them or add them in addition to a physical host controller
like the Jailhouse hypervisor is doing. Therefore allow for multiple
domains so that we can handle them all.

Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
---
 drivers/pci/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 0d0177ce436c..3d25b35bb5ab 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -68,6 +68,7 @@ config PCI_HOST_GENERIC
depends on (ARM || ARM64) && OF
select PCI_HOST_COMMON
select IRQ_DOMAIN
+   select PCI_DOMAINS
help
  Say Y here if you want to support a simple generic PCI host
  controller, such as the one emulated by kvmtool.

We do use interception of a physical host controller as well, but only
on the Seattle.

The whole series
(http://lists.infradead.org/pipermail/linux-arm-kernel/2018-April/573492.html)
might be of interest for you. For studies, or maybe you can even 
support

reviews.

Oh, and I'm still looking for someone to help me getting
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/555415.html
in. :)

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.

PCI devices on Arm

2018-04-26 Thread ff 
On Cavium ThunderX there are PCI devices and multiple PCI domains, each 
with a set of buses and MMIO areas.

DeviceTree can be used to identify all those.

I see a flag pci_is_virtual is true on Arm.
But I am not sure I fully understand the implications.

In theory, we should add multi-domain support to allow proper device 
probing here correct ?


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

Re: [RFC]: warn on jailhouse console use while missing the flag

2018-04-24 Thread ff 

On 24.04.2018 16:13, Jan Kiszka wrote:

On 2018-04-24 15:39, f...@ozog.com wrote:

Hi,

Using the jailhouse console from an inmate requires the cell config to
have the JAILHOUSE_CELL_DEBUG_CONSOLE flag set. If it does not, all
output is silently dropped.

Adding a jailhouse console message stating that an inmate has been
blocked accessing the console is a nice to have from a developer
standpoint but not that useful in production.

Is it worth issuing this notification?


Doable, it's just a bit of "warning: you have debugging disabled".


If so, is it a good idea to have a "DEBUG" version of jailhouse to
activate it?
It looks like there will be a need for warn_once flags in the cell
definition as we just want to warn once, not for every character sent
via the hypercall.


Well... we could attach it to CONFIG_TRACE_ERROR:

diff --git a/hypervisor/control.c b/hypervisor/control.c
index 858ffc745..5cf96c951 100644
--- a/hypervisor/control.c
+++ b/hypervisor/control.c
@@ -876,7 +876,7 @@ long hypercall(unsigned long code, unsigned long
arg1, unsigned long arg2)
case JAILHOUSE_HC_DEBUG_CONSOLE_PUTC:
if (!(cpu_data->cell->config->flags &
  JAILHOUSE_CELL_DEBUG_CONSOLE))
-   return -EPERM;
+   return trace_error(-EPERM);
printk("%c", (char)arg1);
return 0;
default:

That won't be warn_once, but it's really simple.
Yes. Actually this is good enough as it helps "keep it simple". I am not 
fan of adding infrastructure for warn_once until it serves multiple 
purposes...

I assume you already provided the patch ;-)



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.

[RFC]: warn on jailhouse console use while missing the flag

2018-04-24 Thread ff 

Hi,

Using the jailhouse console from an inmate requires the cell config to 
have the JAILHOUSE_CELL_DEBUG_CONSOLE flag set. If it does not, all 
output is silently dropped.


Adding a jailhouse console message stating that an inmate has been 
blocked accessing the console is a nice to have from a developer 
standpoint but not that useful in production.


Is it worth issuing this notification?
If so, is it a good idea to have a "DEBUG" version of jailhouse to 
activate it?
It looks like there will be a need for warn_once flags in the cell 
definition as we just want to warn once, not for every character sent 
via the hypercall.


-FF

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

[PATCH] [MAN]: first cut at man pages

2018-04-16 Thread ff 
Document jailhouse in general, jailhouse enable and parts of jailhouse 
cell

user can enter
man jailhouse cell
or
man jailhouse-cell

No install procedure is yet provided

Signed-off-by: Francois-Frederic Ozog 
---
 man/man8/jailhouse-cell.8   | 129 


 man/man8/jailhouse-enable.8 |  63 ++
 man/man8/jailhouse.8|  93 
 3 files changed, 285 insertions(+)
 create mode 100644 man/man8/jailhouse-cell.8
 create mode 100644 man/man8/jailhouse-enable.8
 create mode 100644 man/man8/jailhouse.8

diff --git a/man/man8/jailhouse-cell.8 b/man/man8/jailhouse-cell.8
new file mode 100644
index ..954fa495
--- /dev/null
+++ b/man/man8/jailhouse-cell.8
@@ -0,0 +1,129 @@
+'\" t
+.\" Title: jailhouse
+.\"Author: [see the "Authors" section]
+.\"  Date: 14/04/2018
+.\"Manual: Jailhouse Manual
+.\"Source: Git 0.8
+.\"  Language: English
+.\"
+.TH "JAILHOUSE-CELL" "8" "14/04/2018" "Jailhouse 0\&.8\&.0" "Jailhouse 
Manual"

+.\" -
+.\" * Define some portability stuff
+.\" -
+.\" ~
+.\" http://bugs.debian.org/507673
+.\" http://lists.gnu.org/archive/html/groff/2009-02/msg00013.html
+.\" ~
+.ie \n(.g .ds Aq \(aq
+.el   .ds Aq '
+.\" -
+.\" * set default formatting
+.\" -
+.\" disable hyphenation
+.nh
+.\" disable justification (adjust text to left margin only)
+.ad l
+.\" -
+.\" * MAIN CONTENT STARTS HERE *
+.\" -
+.SH "NAME"
+jailhouse-cell \- controlling cells
+.SH "SYNOPSIS"
+.sp
+.nf
+\fIjailhouse\fR cell [collect | create | destroy | linux | load | 
shutdown | start | stats] []

+.fi
+.sp
+.SH "DESCRIPTION"
+.sp
+.PP
+\fBjailhouse cell load\fR { ID | [--name] NAME }  {  
} ...

+.RS 4
+.sp
+Where  is { IMAGE | { -s | --string } "STRING" } [-a 
| --address ADDRESS]}

+.RE
+.RS 4
+.sp
+Valid forms are:
+.sp
+# loads inamte\&.bin (offset 0 assumed)
+jailhouse cell load foocell inmate\&.bin
+.sp
+# same as above with explicit location
+jailhouse cell load foocell inmate\&.bin -a 0
+.sp
+# load three binary objects (in order)
+jailhouse cell load foocell \\
+inmate\&.bin \\
+sharedobject\&.so -a 0x100 \\
+ramfs\&.bin -a 0x200
+.RE
+.RS 4
+.sp
+The first example assumes "-a 0"\&.
+.sp
+The last example, loads in the order specified, three binary objects,
+the first one at offset 0, the second one at 0x100\&.
+Should inmate.bin be larger than 0x100, the upper part will be 
overridden

+by sharedobject\&.so\&.
+.sp
+Whatever load order, execution starts in the cell at offset 0\&.
+.sp
+This multi-image loading capability can be used to patch images and
+pass parameters to the image. The following explains how parameters are 
passed

+with the inmate library\&.
+.sp
+The inmate library assumes a command line string to be located at a 
fixed

+location that is processor specific:
+.RE
+.RS 4
+- On x86 this is offset 0x100 (see inmates/lib/x86/inmate\&.lds)
+.RE
+.RS 4
+- On arm64, this is offset 0x1000 (see 
inmates/lib/arm64/inmate\&.lds\&.S)

+.RE
+.RS 4
+.sp
+The command line string capacity is fixed (256 bytes by defaylt) by 
CMDLINE_BUFFER_SIZE

+in inmates/lib/cmdline\&.c\&.
+.sp
+Here is an example to pass  parameters stored in the file
+commandline.txt to the last example on an x86 system:
+.sp
+OFFSET=0x100
+jailhouse cell load foocell \\
+inmate\&.bin \\
+commanline\&.txt -a $OFFSET \\
+sharedobject\&.so -a 0x100 \\
+ramfs\&.bin -a 0x200
+.sp
+This command patches inmate.bin at offset 0x100 that happens to be 
char* cmdline for

+inmates that uses Jailhouse inmates library\&.
+.sp
+Note: on an arm64 we would set OFFSET=0x1000
+.sp
+To be more practical and avoid using a text file, there is an 
image-as-string

+option:
+.sp
+OFFSET=0x100
+jailhouse cell load foocell \\
+inmate\&.bin \\
+-s "" -a $OFFSET \\
+sharedobject\&.so -a 0x100 \\
+ramfs\&.bin -a 0x200
+.sp
+The string in the -s need to be less than 255 characters long 
(CMDLINE_BUFFER_SIZE - terminating \\0)

+otherwise it will silently overwrite existing code\&.
+
+.RE
+
+.SH "SEE ALSO"
+jailhouse(8) jailhouse-enable(8) jailhouse.ko(8)
+.SH "AUTHORS"
+.sp
+Jailhouse was started by Jan Kiszka\&. Contributions have come from the 
Jailhouse mailing list 
<\m[blue]\fBjailhouse\-dev@googlegroups\&.com\fR\m[]\&\s-2\u\d\s+2>\&.

+.sp
+If you have a clone of

[PATCH] inmates: assume VMCALL for hypercalls, detect AMD to change

2018-04-11 Thread ff 

inmates cannot use X86_FEATURE_VMX from regular cpuid
as vcpu maks the bit explicitely on non-root cells.

use cpuid leaf 0 to detect AuthenticAMD and change to VMMCALL

Signed-off-by: Francois-Frederic Ozog 
---
 inmates/lib/x86/hypercall.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/inmates/lib/x86/hypercall.c b/inmates/lib/x86/hypercall.c
index ac99f0c3..2d7a96e1 100644
--- a/inmates/lib/x86/hypercall.c
+++ b/inmates/lib/x86/hypercall.c
@@ -40,19 +40,19 @@

 #define X86_FEATURE_VMX(1 << 5)

-bool jailhouse_use_vmcall;
+bool jailhouse_use_vmcall = true;

 void hypercall_init(void)
 {
-   u32 eax = 1, ecx = 0;
+   u32 eax = 0, ebx = 0, ecx = 0, edx = 0;

asm volatile(
"cpuid"
: "=c" (ecx)
-   : "a" (eax), "c" (ecx)
-   : "rbx", "rdx", "memory"
+   : "a" (eax), "b" (ebx), "c" (ecx), "d" (edx)
+   : "memory"
);

-   if (ecx & X86_FEATURE_VMX)
-   jailhouse_use_vmcall = true;
+   if (ebx == 0x68747541 && ecx == 0x444d4163 && edx == 0x69746E65)
+   jailhouse_use_vmcall = false;
 }
--
2.11.0

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

Re: [PATCH] core: ensure proper detection on VMX vs SVM from inmates

2018-04-11 Thread ff 

On 11.04.2018 14:18, Jan Kiszka wrote:

On 2018-04-11 11:35, f...@ozog.com wrote:

inmates cannot use X86_FEATURE_VMX from regular cpuid
as vcpu maks the bit explicitely on non-root cells.

create the equivalent bit (same register, same position)
in the JAILHOUSE_CPUID_FEATURES cpuid leaf

Signed-off-by: Francois-Frederic Ozog 
---
 hypervisor/arch/x86/vcpu.c | 13 +
 include/arch/x86/asm/jailhouse_hypercall.h |  5 +
 inmates/lib/x86/hypercall.c    |  2 +-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 21da0592..2bfbff94 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -331,6 +331,17 @@ bool vcpu_handle_msr_write(void)
    return true;
 }

+static bool is_vmx(void)
+{
+   u32 eax, ebx, ecx, edx;
+
+   eax=1;
+   ebx = ecx = edx =0;
+   cpuid(, , , );
+
+   return ecx & X86_FEATURE_VMX;
+}
+
 void vcpu_handle_cpuid(void)
 {
    static const char signature[12] = "Jailhouse";
@@ -350,6 +361,8 @@ void vcpu_handle_cpuid(void)
    guest_regs->rax = 0;
    guest_regs->rbx = 0;
    guest_regs->rcx = 0;
+   if (is_vmx())
+   guest_regs->rcx |= X86_FEATURE_VMX;
    guest_regs->rdx = 0;
    break;
    default:
diff --git a/include/arch/x86/asm/jailhouse_hypercall.h
b/include/arch/x86/asm/jailhouse_hypercall.h
index 3a52599f..052b9255 100644
--- a/include/arch/x86/asm/jailhouse_hypercall.h
+++ b/include/arch/x86/asm/jailhouse_hypercall.h
@@ -68,6 +68,11 @@
 #define JAILHOUSE_CPUID_SIGNATURE  0x4000
 #define JAILHOUSE_CPUID_FEATURES   0x4001

+/*
+ * ECX
+ * bit 5: 1=VMX
+*/
+#define JAILHOUSE_CPUID_FEATURE_VMX    (1ULL << 0)
 /**
  * @defgroup Hypercalls Hypercall Subsystem
  *
diff --git a/inmates/lib/x86/hypercall.c b/inmates/lib/x86/hypercall.c
index ac99f0c3..4a356390 100644
--- a/inmates/lib/x86/hypercall.c
+++ b/inmates/lib/x86/hypercall.c
@@ -44,7 +44,7 @@ bool jailhouse_use_vmcall;

 void hypercall_init(void)
 {
-   u32 eax = 1, ecx = 0;
+   u32 eax = JAILHOUSE_CPUID_FEATURES, ecx = 0;

    asm volatile(
    "cpuid"
--
2.11.0



That's neither what I meant nor how Linux (as guest) works. Just probe
for an AMD CPU and set jailhouse_use_vmcall to true if it's not found.

Jan

I am proposing another patch then rather than using this mail thread.

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

RFC: text for cell loading documentation

2018-04-11 Thread ff 

I am proposing to add a Documentation/cell-loading.md file.
Rather than proposing the .md directly, I prefer to propose raw text 
discussion,
then work on the cosmetics and links from other parts of the 
Documentation.


-FF

Jailhouse loading of images is pretty flexible but can be disconcerting.

The syntax is:

jailhouse cell load \
{ ID | [--name] NAME } \
{ { IMAGE | { -s | --string } "STRING" } [-a | --address ADDRESS] } ...

Valid forms are:

# loads inamte.bin (offset 0 assumed)
jailhouse cell load foocell inmate.bin

# same as above with explicit location
jailhouse cell load foocell inmate.bin -a 0

# load three binary objects (in order)
jailhouse cell load foocell \
inmate.bin \
sharedobject.so -a 0x100 \
ramfs.bin -a 0x200

The first example assumes "-a 0".

The last example, loads in the order specified, three binary objects,
the first one at offset 0, the second one at 0x100.
Should inmate.bin be larger than 0x100, the upper part will be 
overridden

by sharedobject.so.

Whatever load order, execution starts in the cell at offset 0.


This multi-image loading capability can be used to patch images and
pass parameters to the image. The following explains how parameters are 
passed

with the inmate library.

The inmate library assumes a command line string to be located at a 
fixed

location that is processor specific:
- On x86 this is offset 0x100 (see inmates/lib/x86/inmate.lds)
- On arm64, this is offset 0x1000 (see inmates/lib/arm64/inmate.lds.S)

The command line string capacity is fixed (256 bytes by defaylt) by 
CMDLINE_BUFFER_SIZE

in inmates/lib/cmdline.c.

Here is an example to pass  parameters stored in the file
commandline.txt to the last example on an x86 system:

OFFSET=0x100
jailhouse cell load foocell \
inmate.bin \
commanline.txt -a $OFFSET \
sharedobject.so -a 0x100 \
ramfs.bin -a 0x200

This command patches inmate.bin at offset 0x100 that happens to be char* 
cmdline for

inmates that uses Jailhouse inmates library.

Note: on an arm64 we would set OFFSET=0x1000

To be more practical and avoid using a text file, there is an 
image-as-string

option:

OFFSET=0x100
jailhouse cell load foocell \
inmate.bin \
-s "" -a $OFFSET \
sharedobject.so -a 0x100 \
ramfs.bin -a 0x200

The string in the -s need to be less than 255 characters long 
(CMDLINE_BUFFER_SIZE - terminatind \0)

otherwise it will silently overwrite existing code.

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

[PATCH] core: ensure proper detection on VMX vs SVM from inmates

2018-04-11 Thread ff 

inmates cannot use X86_FEATURE_VMX from regular cpuid
as vcpu maks the bit explicitely on non-root cells.

create the equivalent bit (same register, same position)
in the JAILHOUSE_CPUID_FEATURES cpuid leaf

Signed-off-by: Francois-Frederic Ozog 
---
 hypervisor/arch/x86/vcpu.c | 13 +
 include/arch/x86/asm/jailhouse_hypercall.h |  5 +
 inmates/lib/x86/hypercall.c|  2 +-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/hypervisor/arch/x86/vcpu.c b/hypervisor/arch/x86/vcpu.c
index 21da0592..2bfbff94 100644
--- a/hypervisor/arch/x86/vcpu.c
+++ b/hypervisor/arch/x86/vcpu.c
@@ -331,6 +331,17 @@ bool vcpu_handle_msr_write(void)
return true;
 }

+static bool is_vmx(void)
+{
+   u32 eax, ebx, ecx, edx;
+
+   eax=1;
+   ebx = ecx = edx =0;
+   cpuid(, , , );
+
+   return ecx & X86_FEATURE_VMX;
+}
+
 void vcpu_handle_cpuid(void)
 {
static const char signature[12] = "Jailhouse";
@@ -350,6 +361,8 @@ void vcpu_handle_cpuid(void)
guest_regs->rax = 0;
guest_regs->rbx = 0;
guest_regs->rcx = 0;
+   if (is_vmx())
+   guest_regs->rcx |= X86_FEATURE_VMX;
guest_regs->rdx = 0;
break;
default:
diff --git a/include/arch/x86/asm/jailhouse_hypercall.h 
b/include/arch/x86/asm/jailhouse_hypercall.h

index 3a52599f..052b9255 100644
--- a/include/arch/x86/asm/jailhouse_hypercall.h
+++ b/include/arch/x86/asm/jailhouse_hypercall.h
@@ -68,6 +68,11 @@
 #define JAILHOUSE_CPUID_SIGNATURE  0x4000
 #define JAILHOUSE_CPUID_FEATURES   0x4001

+/*
+ * ECX
+ * bit 5: 1=VMX
+*/
+#define JAILHOUSE_CPUID_FEATURE_VMX(1ULL << 0)
 /**
  * @defgroup Hypercalls Hypercall Subsystem
  *
diff --git a/inmates/lib/x86/hypercall.c b/inmates/lib/x86/hypercall.c
index ac99f0c3..4a356390 100644
--- a/inmates/lib/x86/hypercall.c
+++ b/inmates/lib/x86/hypercall.c
@@ -44,7 +44,7 @@ bool jailhouse_use_vmcall;

 void hypercall_init(void)
 {
-   u32 eax = 1, ecx = 0;
+   u32 eax = JAILHOUSE_CPUID_FEATURES, ecx = 0;

asm volatile(
"cpuid"
--
2.11.0

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

Re: [RFC]: fixing cmdline and hypercall instruction detection

2018-04-11 Thread ff 

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 

lastly, cell_shutdown_load() in tools/jailhouse.c considers "-s
" 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 0x100 \
ramfs.bin -a 0x200

If this is correct, I'll propose a set of Documentation patches





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


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

[RFC]: fixing cmdline and hypercall instruction detection

2018-04-11 Thread ff 
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

according to Documentation/debug-output.md
command line parameters are specified using -s 

lastly, cell_shutdown_load() in tools/jailhouse.c considers "-s 
" 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"


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.

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!


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




-FF

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

[PATCH 2/2] tools: proper reporting of mandatory and optional VMXON features

2018-04-10 Thread ff 
on a VMX capable processor, firmware has the capability to block use of 
it.

WHen it is the case, bits of IA32_FEATURE_CONTROL are cleared.
At a minimum, a jailhouse system need to be able to use VMX outside SMX.

Signed-off-by: Francois-Frederic Ozog 
---
 tools/jailhouse-hardware-check | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/jailhouse-hardware-check 
b/tools/jailhouse-hardware-check

index 67d3b078..2e3b818f 100755
--- a/tools/jailhouse-hardware-check
+++ b/tools/jailhouse-hardware-check
@@ -182,8 +182,8 @@ if cpu_vendor == 'GenuineIntel':
 check_feature('VT-x (VMX)', 'vmx' in cpu_features)

 feature = msr.read(MSR.IA32_FEATURE_CONTROL)
-check_feature('  VMX without TXT',
-  (feature & (1 << 0)) == 0 or feature & (1 << 2))
+check_feature('  VMX outside SMX', feature & (1 << 2))
+check_feature('  VMX inside SMX', feature & (1 << 1), True)
 check_feature('  IA32_TRUE_*_CLTS',
   msr.read(MSR.IA32_VMX_BASIC) & (1 << 55))

--
2.11.0

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

[PATCH 1/2] driver: ensure jailhouse is not enabled when VT-X is disabled by firmware

2018-04-10 Thread ff 

Whithout the check,
jailhouse enable configs/x86/sysconfig.cell
results in a GP and a reboot

do not allow enable if firmware has disabled VT-X on Intel VMX

Signed-off-by: Francois-Frederic Ozog 
---
 driver/main.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/driver/main.c b/driver/main.c
index ee585848..723a9576 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -40,6 +40,10 @@
 #ifdef CONFIG_ARM
 #include 
 #endif
+#ifdef CONFIG_X86
+#include 
+#include 
+#endif

 #include "cell.h"
 #include "jailhouse.h"
@@ -392,6 +396,18 @@ static int jailhouse_cmd_enable(struct 
jailhouse_system __user *arg)

goto error_put_module;
}
 #endif
+#ifdef CONFIG_X86
+   if (boot_cpu_has(X86_FEATURE_VMX)) {
+   u64 features;
+
+   rdmsrl(MSR_IA32_FEATURE_CONTROL, features);
+   if ((features & 
FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX) == 0) {
+   pr_err("jailhouse: vt-x disabled by 
Firmware/BIOS\n");

+   err = -ENODEV;
+   goto error_put_module;
+   }
+   }
+#endif

/* Load hypervisor image */
err = request_firmware(, fw_name, jailhouse_dev);
--
2.11.0

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