Re: [PATCH] i386/sev: Nitpick at the error message's output

["Alex Bennée" <alex.bennee@linaro.org>]

Hyman Huang <yong.huang@smartx.com> writes:

> The incorrect error message was produced as a result of
> the return number being disregarded on the sev_kvm_init
> failure path.
>
> For instance, when a user's failure to launch a SEV guest
> is caused by an incorrect IOCTL, the following message is
> reported:
>
> kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0
> kvm: failed to initialize kvm: Operation not permitted
>
> While the error message's accurate output should be:
>
> kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0
> kvm: failed to initialize kvm: Inappropriate ioctl for device
>
> Fix this by returning the return number directly on the
> failure path.
>
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  target/i386/sev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 9a71246682..4a69ca457c 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -1019,7 +1019,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>  err:
>      sev_guest = NULL;
>      ram_block_discard_disable(false);
> -    return -1;
> +    return ret;

I don't think this will be correct in all cases because there are other
legs (e.g. if (host_cbitpos != sev->cbitpos)) where ret may be the
successful setting of ram_block_discard_disable(true).

You might want to explore if the function can be re-written with
explicit return's and utilise autofree to do the clean-up of dynamic
objects.

I think this entails setting sev_guest at the end of the function just
before the return 0.

I'm not sure if there is a clean way to handle
ram_block_discard_disable(false); cleanly for all the failure legs
though. Maybe someone with more familiarity with the code has some ideas?


>  }
>  
>  int

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro