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 _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 can see that using the memblock allocator early is easier. But you > waste "largish ... range of memory" even if you never run VMs. this is inevitable > > Maybe something to work on in the future. honestly unlikely. which is why protected virtualization needs to be enabled explicitly > > > - 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
