RE: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in VMBus init message

[Saurabh Singh Sengar]

> -----Original Message-----
> From: Tianyu Lan <ltykernel@xxxxxxxxx>
> Sent: Thursday, August 10, 2023 9:53 PM
> To: Wei Liu <wei.liu@xxxxxxxxxx>; Saurabh Singh Sengar
> <ssengar@xxxxxxxxxxxxx>
> Cc: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang
> <haiyangz@xxxxxxxxxxxxx>; Dexuan Cui <decui@xxxxxxxxxxxxx>;
> tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; bp@xxxxxxxxx;
> dave.hansen@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx; hpa@xxxxxxxxx;
> daniel.lezcano@xxxxxxxxxx; arnd@xxxxxxxx; Michael Kelley (LINUX)
> <mikelley@xxxxxxxxxxxxx>; Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>; linux-
> arch@xxxxxxxxxxxxxxx; linux-hyperv@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; vkuznets <vkuznets@xxxxxxxxxx>
> Subject: Re: [EXTERNAL] [PATCH V2 2/9] x86/hyperv: Set Virtual Trust Level in
> VMBus init message
> 
> On 8/7/2023 12:48 PM, Wei Liu wrote:
> > On Fri, Jul 07, 2023 at 09:07:54AM +0000, Saurabh Singh Sengar wrote:
> >>
> >>
> >>> +
> >>> +	ret = hv_do_hypercall(control, input, output);
> >>> +	if (hv_result_success(ret))
> >>> +		vtl = output->as64.low & HV_X64_VTL_MASK;
> >>> +	else
> >>> +		pr_err("Hyper-V: failed to get VTL! %lld", ret);
> >>
> >> In case of error this function will return vtl=0, which can be the valid value
> of vtl.
> >> I suggest we initialize vtl with -1 so and then check for its return.
> >>
> >> This could be a good utility function which can be used for any
> >> Hyper-V VTL system, so think of making it global ?
> >>
> >
> > Tianyu -- your thought on this?
> 
> In current user cases, the guest only runs in VTL0 and Hyper-V may return VTL
> error in some cases but kernel still may run with 0 as VTL.
> 
> I just sent out v5 and set VTL to 0 by default if fail to get VTL from Hyper-V and
> give out a warning log. The get_vtl() is only called on enlightened SEV-SNP
> guest. If there is new case that needs handle the error from Hyper-V when
> call VTL hvcall, we may add the logic later.

I understand your requirement.
IMO it's cleaner if function returns errors, rather than relying solely on error prints.
While caller of this function can choose to ignore errors, the function must not overlook them.
something like this should work for your requirement:

ret = get_vtl()
ms_hyperv.vtl = ret < 0 ? 0 : ret;

This way desired functionality will be intact, and function also will be self-contained.