Re: [RFC 02/37] s390/protvirt: introduce host side setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux