Re: [PATCH V1 2/2] Drivers: hv: Fix bugs and issues around repeat hypercall

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] Drivers: hv: unused HV_REP_COUNT_MAX macro and dead return in
  mshyperv.h
--

commit 9765fd900ad842b6359a24f592cd624ffdc92be5
Author: Mukesh R <mrathor@linux.microsoft.com>

Drivers: hv: Fix bugs and issues around repeat hypercall

This commit updates repeat hypercall handling to account for partial
completions on failure, splits repeat hypercall helpers between hardware
confidential VMs and regular VMs, and renames HV_STATUS_TIME_OUT to
HV_STATUS_TIMEOUT.

> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 396bcb3db0bcc..87c784b9daa69 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -28,6 +28,9 @@
>  
>  #define VTPM_BASE_ADDRESS 0xfed40000
>  
> +#define HV_REP_COUNT_MAX	\
> +	(HV_HYPERCALL_REP_COMP_MASK >> HV_HYPERCALL_REP_COMP_OFFSET)
> +
>  enum hv_partition_type {

[ ... ]

> @@ -135,45 +143,63 @@ static inline unsigned int hv_repcomp(u64 status)
>  	/* Bits [43:32] of status have 'Reps completed' data. */
>  	return (status & HV_HYPERCALL_REP_COMP_MASK) >>
>  			 HV_HYPERCALL_REP_COMP_OFFSET;
>  }
>  
> -/*
> - * Rep hypercalls. Callers of this functions are supposed to ensure that
> - * rep_count, varhead_size, and rep_start comply with Hyper-V hypercall
> - * definition.
> - */
> -static inline u64 hv_do_rep_hypercall_ex(u16 code, u16 rep_count,
> -					 u16 varhead_size, u16 rep_start,
> -					 void *input, void *output)
> +static inline u64 hv_do_rep_hypercall_reg(u16 code, u16 rep_count,
> +					  u16 varhead_size, u16 rep_start,
> +					  void *input, void *output)
>  {
>  	u64 control = code;
> -	u64 status;
> -	u16 rep_comp;
>  
>  	control |= (u64)varhead_size << HV_HYPERCALL_VARHEAD_OFFSET;
>  	control |= (u64)rep_count << HV_HYPERCALL_REP_COMP_OFFSET;
>  	control |= (u64)rep_start << HV_HYPERCALL_REP_START_OFFSET;
>  
> -	do {
> +	return hv_do_hypercall(control, input, output);
> +}
> +
> +static inline u64 hv_do_rep_hypercall_cvm(u16 code, u16 rep_count,
> +					  u16 varhead_size, u16 rep_start,
> +					  void *input, void *output)
> +{
> +	u64 status, control = code;
> +	u16 rep_comp = 0;
> +
> +	control |= (u64)varhead_size << HV_HYPERCALL_VARHEAD_OFFSET;
> +	control |= (u64)rep_count << HV_HYPERCALL_REP_COMP_OFFSET;
> +	control |= (u64)rep_start << HV_HYPERCALL_REP_START_OFFSET;
> +
> +	for (;;) {
>  		status = hv_do_hypercall(control, input, output);
> -		if (!hv_result_success(status))
> +
> +		if (!hv_result_timeout(status))
>  			return status;
>  
>  		rep_comp = hv_repcomp(status);
> -
>  		control &= ~HV_HYPERCALL_REP_START_MASK;
>  		control |= (u64)rep_comp << HV_HYPERCALL_REP_START_OFFSET;
>  
>  		touch_nmi_watchdog();
> -	} while (rep_comp < rep_count);
> +	}
>  
>  	return status;
>  }

[Severity: Low]
Is the trailing return status; in hv_do_rep_hypercall_cvm() needed now
that the loop is for (;;) with no break statement?  Also, HV_REP_COUNT_MAX
added earlier in this file has no users in the tree.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114025313.1257042-1-mrathor@linux.microsoft.com?part=2