On 11/4/19 3:26 PM, Cornelia Huck wrote:
> On Fri, 1 Nov 2019 09:53:12 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 24.10.19 13:40, Janosch Frank wrote:
>>> From: Vasily Gorbik <gor@linux.ibm.com>
>>>
>>> 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@linux.ibm.com>
>>> ---
>>>  .../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/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig
>>> index d3db3d7ed077..652b36f0efca 100644
>>> --- a/arch/s390/kvm/Kconfig
>>> +++ b/arch/s390/kvm/Kconfig
>>> @@ -55,6 +55,15 @@ config KVM_S390_UCONTROL
>>>
>>>  	  If unsure, say N.
>>>
>>> +config KVM_S390_PROTECTED_VIRTUALIZATION_HOST
>>> +	bool "Protected guests execution support"
>>> +	depends on KVM
>>> +	---help---
>>> +	  Support hosting protected virtual machines isolated from the
>>> +	  hypervisor.
>>> +
>>> +	  If unsure, say Y.
>>> +
>>>  # OK, it's a little counter-intuitive to do this, but it puts it neatly under
>>>  # the virtualization menu.
>>>  source "drivers/vhost/Kconfig"
>>>   
>>
>> As we have the prot_virt kernel paramter there is a way to fence this during runtime
>> Not sure if we really need a build time fence. We could get rid of
>> CONFIG_KVM_S390_PROTECTED_VIRTUALIZATION_HOST and just use CONFIG_KVM instead,
>> assuming that in the long run all distros will enable that anyway. 
> 
> I still need to read through the rest of this patch set to have an
> informed opinion on that, which will probably take some more time.
> 
>> If other reviewers prefer to keep that extra option what about the following to the
>> help section:
>>
>> ----
>> Support hosting protected virtual machines in KVM. The state of these machines like
>> memory content or register content is protected from the host or host administrators.
>>
>> Enabling this option will enable extra code that talks to a new firmware instance
> 
> "...that allows the host kernel to talk..." ?

"allows a Linux hypervisor to talk..." ?

> 
>> called ultravisor that will take care of protecting the guest while also enabling
>> KVM to run this guest.
>>
>> This feature must be enable by the kernel command line option prot_virt.
> 
> s/enable by/enabled via/
> 
>>
>> 	  If unsure, say Y.
> 
> Looks better. I'm continuing to read the rest of this series before I
> say more, though :)
>