Re: [for-5.2 v4 10/10] s390: Recognize host-trust-limitation option

[Janosch Frank]

On 7/27/20 5:50 PM, Cornelia Huck wrote:
> On Fri, 24 Jul 2020 12:57:44 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
>> At least some s390 cpu models support "Protected Virtualization" (PV),
>> a mechanism to protect guests from eavesdropping by a compromised
>> hypervisor.
>>
>> This is similar in function to other mechanisms like AMD's SEV and
>> POWER's PEF, which are controlled bythe "host-trust-limitation"
>> machine option.  s390 is a slightly special case, because we already
>> supported PV, simply by using a CPU model with the required feature
>> (S390_FEAT_UNPACK).
>>
>> To integrate this with the option used by other platforms, we
>> implement the following compromise:
>>
>>  - When the host-trust-limitation option is set, s390 will recognize
>>    it, verify that the CPU can support PV (failing if not) and set
>>    virtio default options necessary for encrypted or protected guests,
>>    as on other platforms.  i.e. if host-trust-limitation is set, we
>>    will either create a guest capable of entering PV mode, or fail
>>    outright
>>
>>  - If host-trust-limitation is not set, guest's might still be able to
>>    enter PV mode, if the CPU has the right model.  This may be a
>>    little surprising, but shouldn't actually be harmful.
> 
> This could be workable, I guess. Would like a second opinion, though.
> 
>>
>> To start a guest supporting Protected Virtualization using the new
>> option use the command line arguments:
>>     -object s390-pv-guest,id=pv0 -machine host-trust-limitation=pv0
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  hw/s390x/pv.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 61 insertions(+)
>>
>> diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c
>> index ab3a2482aa..4bf3b345b6 100644
>> --- a/hw/s390x/pv.c
>> +++ b/hw/s390x/pv.c
>> @@ -14,8 +14,11 @@
>>  #include <linux/kvm.h>
>>  
>>  #include "cpu.h"
>> +#include "qapi/error.h"
>>  #include "qemu/error-report.h"
>>  #include "sysemu/kvm.h"
>> +#include "qom/object_interfaces.h"
>> +#include "exec/host-trust-limitation.h"
>>  #include "hw/s390x/ipl.h"
>>  #include "hw/s390x/pv.h"
>>  
>> @@ -111,3 +114,61 @@ void s390_pv_inject_reset_error(CPUState *cs)
>>      /* Report that we are unable to enter protected mode */
>>      env->regs[r1 + 1] = DIAG_308_RC_INVAL_FOR_PV;
>>  }
>> +
>> +#define TYPE_S390_PV_GUEST "s390-pv-guest"
>> +#define S390_PV_GUEST(obj)                              \
>> +    OBJECT_CHECK(S390PVGuestState, (obj), TYPE_S390_PV_GUEST)
>> +
>> +typedef struct S390PVGuestState S390PVGuestState;
>> +
>> +/**
>> + * S390PVGuestState:
>> + *
>> + * The S390PVGuestState object is basically a dummy used to tell the
>> + * host trust limitation system to use s390's PV mechanism.  guest.
>> + *
>> + * # $QEMU \
>> + *         -object s390-pv-guest,id=pv0 \
>> + *         -machine ...,host-trust-limitation=pv0
>> + */
>> +struct S390PVGuestState {
>> +    Object parent_obj;
>> +};
>> +
>> +static int s390_pv_kvm_init(HostTrustLimitation *gmpo, Error **errp)
>> +{
>> +    if (!s390_has_feat(S390_FEAT_UNPACK)) {
>> +        error_setg(errp,
>> +                   "CPU model does not support Protected Virtualization");
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
> 
> So here's where I'm confused: If I follow the code correctly, the
> ->kvm_init callback is invoked before kvm_arch_init() is called. The
> kvm_arch_init() implementation for s390x checks whether
> KVM_CAP_S390_PROTECTED is available, which is a pre-req for
> S390_FEAT_UNPACK. Am I missing something? Can someone with access to PV
> hardware check whether this works as intended?

Doesn't look good:

./s390x-run s390x/stsi.img -object s390-pv-guest,id=pv0 -machine
host-trust-limitation=pv0
/usr/local/bin/qemu-system-s390x -nodefaults -nographic -machine
s390-ccw-virtio,accel=kvm -chardev stdio,id=con0 -device
sclpconsole,chardev=con0 -kernel s390x/stsi.img -object
s390-pv-guest,id=pv0 -machine host-trust-limitation=pv0 # -initrd
/tmp/tmp.uacr85fJnw
qemu-system-s390x: CPU model does not support Protected Virtualization
qemu-system-s390x: failed to initialize kvm: Operation not permitted


Without the htl it's happy.

> 
>> +
>> +static void s390_pv_guest_class_init(ObjectClass *oc, void *data)
>> +{
>> +    HostTrustLimitationClass *gmpc = HOST_TRUST_LIMITATION_CLASS(oc);
>> +
>> +    gmpc->kvm_init = s390_pv_kvm_init;
>> +}
>> +
>> +static const TypeInfo s390_pv_guest_info = {
>> +    .parent = TYPE_OBJECT,
>> +    .name = TYPE_S390_PV_GUEST,
>> +    .instance_size = sizeof(S390PVGuestState),
>> +    .class_init = s390_pv_guest_class_init,
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { TYPE_HOST_TRUST_LIMITATION },
>> +        { TYPE_USER_CREATABLE },
>> +        { }
>> +    }
>> +};
>> +
>> +static void
>> +s390_pv_register_types(void)
>> +{
>> +    type_register_static(&s390_pv_guest_info);
>> +}
>> +
>> +type_init(s390_pv_register_types);
>

signature.asc
Description: OpenPGP digital signature