On Tue, Feb 09, 2021 at 03:45:23PM +0000, Michael Kelley wrote:

As noted in a previous email, we don't have a consistent
pattern for checking Hyper-V hypercall status.  Existing code and
recent new code uses a number of variants.  The variants work, but
a consistent pattern would improve the readability of the code, and
be more conformant to what the Hyper-V TLFS says about hypercall
status.  In particular, the 64 bit hypercall status contains fields that
the TLFS says should be ignored -- evidently they are not guaranteed
to be zero (though I've never seen otherwise).

I'd propose the following helper functions to go in
asm-generic/mshyperv.h.  The function names are relatively short
for readability:

static inline u64 hv_result(u64 status)
{
	return status & HV_HYPERCALL_RESULT_MASK;
}

static inline bool hv_result_success(u64 status)
{
	return hv_result(status) == HV_STATUS_SUCCESS;
}

static inline unsigned int hv_repcomp(u64 status)
{
	return (status & HV_HYPERCALL_REP_COMP_MASK) >>
			HV_HYPERCALL_REP_COMP_OFFSET;
}

The hv_do_hypercall() function (and its 'rep' and 'fast' variants) should
always assign the result to a u64 local variable, which is the return
type of the functions.  Then the above functions can act on that local
variable.  Here are some examples:

	u64		status;
	unsigned int	completed;

	status = hv_do_hypercall(<some args>);
	if (!hv_result_success(status)) {
		<handle error case>
	}

	status = hv_do_rep_hypercall(<some args>);
	if (hv_result(status) == HV_STATUS_INSUFFICIENT_MEMORY) {
		<deposit more memory pages>
		goto retry;
	} else if (!hv_result_success(status)) {
		<handle error case>
	}
	completed = hv_repcomp(status);


Thoughts?

I think this is definitely an improvement over open-coding everywhere.

Wei.