On Mon, 3 Feb 2020 08:19:22 -0500
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
> From: Vasily Gorbik <gor@xxxxxxxxxxxxx>
>
> Introduce KVM_S390_PROTECTED_VIRTUALIZATION_HOST kbuild option for
> protected virtual machines hosting support code.
Hm... I seem to remember that you wanted to drop this config option and
always build the code, in order to reduce complexity. Have you
reconsidered this?
>
> Add "prot_virt" command line option which controls if the kernel
> protected VMs support is enabled at early boot time. This has to be
> done early, because it needs large amounts of memory and will disable
> some features like STP time sync for the lpar.
>
> 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 | 19 ++++++++
> 8 files changed, 136 insertions(+), 9 deletions(-)
> create mode 100644 arch/s390/kernel/uv.c
(...)
> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> new file mode 100644
> index 000000000000..35ce89695509
> --- /dev/null
> +++ b/arch/s390/kernel/uv.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Common Ultravisor functions and initialization
> + *
> + * Copyright IBM Corp. 2019
Happy new year?
> + */
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/sizes.h>
> +#include <linux/bitmap.h>
> +#include <linux/memblock.h>
> +#include <asm/facility.h>
> +#include <asm/sections.h>
> +#include <asm/uv.h>
> +
> +#ifdef CONFIG_PROTECTED_VIRTUALIZATION_GUEST
> +int __bootdata_preserved(prot_virt_guest);
Confused. You have this and uv_info below both in this file and in
boot/uv.c. Is there some magic happening in __bootdata_preserved()?
> +#endif
> +
> +#ifdef CONFIG_KVM_S390_PROTECTED_VIRTUALIZATION_HOST
> +int prot_virt_host;
> +EXPORT_SYMBOL(prot_virt_host);
> +struct uv_info __bootdata_preserved(uv_info);
> +EXPORT_SYMBOL(uv_info);
> +
> +static int __init prot_virt_setup(char *val)
> +{
> + bool enabled;
> + int rc;
> +
> + rc = kstrtobool(val, &enabled);
> + if (!rc && enabled)
> + prot_virt_host = 1;
> +
> + if (is_prot_virt_guest() && prot_virt_host) {
> + prot_virt_host = 0;
> + pr_info("Running as protected virtualization guest.");
Trying to disentangle that a bit in my mind...
If we don't have facility 158, is_prot_virt_guest() will return 0. If
protected host support has been requested, we'll print a message below
(and turn it off).
If the hardware provides the facilities for running as a protected virt
guest, we turn off protected virt host support if requested and print a
messages that we're a guest.
Two questions:
- Can the hardware ever provide both host and guest interfaces at the
same time? I guess not; maybe add a comment?
- Do we also want to print a message that we're running as a guest if
the user didn't enable host support? If not, maybe prefix the message
with "Cannot enable support for protected virtualization host:" or
so? (Maybe also a good idea for the message below.)
> + }
> +
> + if (prot_virt_host && !test_facility(158)) {
> + prot_virt_host = 0;
> + pr_info("The ultravisor call facility is not available.");
> + }
> +
> + return rc;
> +}
> +early_param("prot_virt", prot_virt_setup);
> +#endif
(...)
