On Mon, 3 Feb 2020 23:03:42 +0100
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
> On 03.02.20 18:12, Cornelia Huck wrote:
> > 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?
>
> I am still in favour of removing this, but I did not get an "yes, lets do
> it" answer. Since removing is easier than re-adding its still in.
ok
>
> [...]
> >> + * Copyright IBM Corp. 2019
> >
> > Happy new year?
>
> yep :-)
> [..]
>
> >> +
> >> +#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()?
>
> Yes, this is information that is transferred from the decompressor
> to Linux.
> I think we discussed about this the last time as well?
I think I was confused about different things last time...
But that is probably a sign that this wants a comment :)
>
>
> >
> >> +#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).
>
> yes, a guest cannot be a host.
> >
> > 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?
>
> Right, you are either guest or host.
>
> > - 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.)
>
> Line too long and I hate breaking string over multiple lines.
> I can change if somebody comes up with a proper message that is not too long.
>
Fair enough; it's just that it's not very clear from the messages in
the log what happened. Maybe
"prot_virt: Running as protected virtualization guest."
"prot_virt: The ultravisor call facility is not available."
That at least links back to the kernel parameter.
[Aside: Would prot_virt_host be a better name? But that probably moves
us into bikeshed territory.]
