Re: [PATCH v2 2/5] hw/s390x/pv: Un-inline s390_pv_init()

[Richard Henderson]

On 12/17/22 07:24, Philippe Mathieu-Daudé wrote:
> @@ -251,7 +251,9 @@ struct S390PVGuestClass {
>   
>   int s390_pv_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>   {
> -    if (!object_dynamic_cast(OBJECT(cgs), TYPE_S390_PV_GUEST)) {
> +    assert(kvm_enabled());
> +
> +    if (!cgs || !object_dynamic_cast(OBJECT(cgs), TYPE_S390_PV_GUEST)) {
>           return 0;
>       }
>   
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 2e64ffab45..d9a96e315e 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -255,8 +255,10 @@ static void ccw_init(MachineState *machine)
>       /* init CPUs (incl. CPU model) early so s390_has_feature() works */
>       s390_init_cpus(machine);
>   
> -    /* Need CPU model to be determined before we can set up PV */
> -    s390_pv_init(machine->cgs, &error_fatal);
> +    if (kvm_enabled()) {
> +        /* Need CPU model to be determined before we can set up PV */
> +        s390_pv_kvm_init(machine->cgs, &error_fatal);
> +    }
>   
>       s390_flic_init();...
> -static inline int s390_pv_init(ConfidentialGuestSupport *cgs, Error **errp)
> -{
> -    if (!cgs) {
> -        return 0;
> -    }
> -    if (kvm_enabled()) {
> -        return s390_pv_kvm_init(cgs, errp);
> -    }
> -
> -    error_setg(errp, "Protected Virtualization requires KVM");
> -    return -1;
> -}

You've lost the error path above.  And it seems like we could just handle null cgs early. 
E.g.

    if (machine->cgs) {
        if (kvm_enabled()) {
            s390_pv_kvm_init(machine->cgs, &error_fatal);
        } else {
            error_report(...);
            exit(EXIT_FAILURE);
        }
    }

(since qabi/error.h says not to use error_setg(&error_fatal, ...)).


r~