On 24.10.19 18:30, Claudio Imbrenda wrote:
On Thu, 24 Oct 2019 17:52:31 +0200
David Hildenbrand <david@xxxxxxxxxx> wrote:
On 24.10.19 15:40, Christian Borntraeger wrote:
On 24.10.19 15:27, David Hildenbrand wrote:
On 24.10.19 15:25, David Hildenbrand wrote:
On 24.10.19 13:40, Janosch Frank wrote:
From: Vasily Gorbik <gor@xxxxxxxxxxxxx>
Introduce KVM_S390_PROTECTED_VIRTUALIZATION_HOST kbuild option
for protected virtual machines hosting support code.
Add "prot_virt" command line option which controls if the kernel
protected VMs support is enabled at runtime.
Extend ultravisor info definitions and expose it via uv_info
struct filled in during startup.
Signed-off-by: Vasily Gorbik <gor@xxxxxxxxxxxxx>
---
.../admin-guide/kernel-parameters.txt | 5 ++
arch/s390/boot/Makefile | 2 +-
arch/s390/boot/uv.c | 20 +++++++-
arch/s390/include/asm/uv.h | 46
++++++++++++++++-- arch/s390/kernel/Makefile
| 1 + arch/s390/kernel/setup.c | 4 --
arch/s390/kernel/uv.c | 48
+++++++++++++++++++
arch/s390/kvm/Kconfig | 9 ++++ 8 files
changed, 126 insertions(+), 9 deletions(-) create mode 100644
arch/s390/kernel/uv.c
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt index
c7ac2f3ac99f..aa22e36b3105 100644 ---
a/Documentation/admin-guide/kernel-parameters.txt +++
b/Documentation/admin-guide/kernel-parameters.txt @@ -3693,6
+3693,11 @@ before loading.
See
Documentation/admin-guide/blockdev/ramdisk.rst.
+ prot_virt= [S390] enable hosting protected virtual
machines
+ isolated from the hypervisor (if hardware supports
+ that).
+ Format: <bool>
Isn't that a virt driver detail that should come in via KVM module
parameters? I don't see quite yet why this has to be a kernel
parameter (that can be changed at runtime).
I was confused by "runtime" in "which controls if the kernel
protected VMs support is enabled at runtime"
So this can't be changed at runtime. Can you clarify why kvm can't
initialize that when loaded and why we need a kernel parameter?
We have to do the opt-in very early for several reasons:
- we have to donate a potentially largish contiguous (in real)
range of memory to the ultravisor
If you'd be using CMA (or alloc_contig_pages() with less guarantees)
you could be making good use of the memory until you actually start
an encrypted guest.
no, the memory needs to be allocated before any other interaction with
the ultravisor is attempted, and the size depends on the size of the
I fail to see why you need interaction with the UV before you actually
start/create an encrypted guest (IOW why you can't defer uv_init()) -
but I am not past "[RFC 07/37] KVM: s390: protvirt: Secure memory is not
mergeable" yet and ...
_host_ memory. it can be a very substantial amount of memory, and thus
it's very likely to fail unless it's done very early at boot time.
... I guess you could still do that via CMA ... but it doesn't really
matter right now :) I understood the rational.
- The opt-in will also disable some features in the host that could
affect guest integrity (e.g. time sync via STP to avoid the host
messing with the guest time stepping). Linux is not happy when you
remove features at a later point in time
At least disabling STP shouldn't be a real issue if I'm not wrong
(maybe I am). But there seem to be more features.
(when I saw "prot_virt" it felt like somebody is using a big hammer
for small nails (e.g., compared to "stp=off").)
Can you guys add these details to the patch description?
yeah, probably a good idea
If a patch explains why it does something and not only what it does
usually makes me ask less stupid questions :)
--
Thanks,
David / dhildenb
