RE: [PATCH v2 15/15] Drivers: hv: Add modules to expose /dev/mshv to VMMs running on Hyper-V

[Saurabh Singh Sengar <ssengar@microsoft.com>]

> -----Original Message-----
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> Sent: Friday, August 18, 2023 3:32 AM
> To: linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org;
> x86@kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> arch@vger.kernel.org
> Cc: patches@lists.linux.dev; Michael Kelley (LINUX)
> <mikelley@microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> wei.liu@kernel.org; Haiyang Zhang <haiyangz@microsoft.com>; Dexuan Cui
> <decui@microsoft.com>; apais@linux.microsoft.com; Tianyu Lan
> <Tianyu.Lan@microsoft.com>; ssengar@linux.microsoft.com; MUKESH
> RATHOR <mukeshrathor@microsoft.com>; stanislav.kinsburskiy@gmail.com;
> jinankjain@linux.microsoft.com; vkuznets <vkuznets@redhat.com>;
> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de;
> dave.hansen@linux.intel.com; hpa@zytor.com; will@kernel.org;
> catalin.marinas@arm.com
> Subject: [PATCH v2 15/15] Drivers: hv: Add modules to expose /dev/mshv to
> VMMs running on Hyper-V
> 
> Add mshv, mshv_root, and mshv_vtl modules:
> 

<snip>

> +	ret = mshv_set_vp_registers(vp->index, vp->partition->id,
> +				    1, &dispatch_suspend);
> +	if (ret)
> +		pr_err("%s: failed to suspend partition %llu vp %u\n",
> +			__func__, vp->partition->id, vp->index);
> +
> +	return ret;
> +}
> +
> +static int
> +get_vp_signaled_count(struct mshv_vp *vp, u64 *count)
> +{
> +	int ret;
> +	struct hv_register_assoc root_signal_count = {
> +		.name = HV_REGISTER_VP_ROOT_SIGNAL_COUNT,
> +	};
> +
> +	ret = mshv_get_vp_registers(vp->index, vp->partition->id,
> +				    1, &root_signal_count);
> +
> +	if (ret) {
> +		pr_err("%s: failed to get root signal count for partition %llu vp
> %u",
> +			__func__, vp->partition->id, vp->index);
> +		*count = 0;

Have we missed a return here ?
Moreover, the return type of this function is never checked consider
checking it or change it to void.

<snip>

> +
> +/* Retrieve and stash the supported scheduler type */
> +static int __init mshv_retrieve_scheduler_type(void)
> +{
> +	struct hv_input_get_system_property *input;
> +	struct hv_output_get_system_property *output;
> +	unsigned long flags;
> +	u64 status;
> +
> +	local_irq_save(flags);
> +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> +
> +	memset(input, 0, sizeof(*input));
> +	memset(output, 0, sizeof(*output));
> +	input->property_id = HV_SYSTEM_PROPERTY_SCHEDULER_TYPE;
> +
> +	status = hv_do_hypercall(HVCALL_GET_SYSTEM_PROPERTY, input,
> output);
> +	if (!hv_result_success(status)) {
> +		local_irq_restore(flags);
> +		pr_err("%s: %s\n", __func__, hv_status_to_string(status));
> +		return hv_status_to_errno(status);
> +	}
> +
> +	hv_scheduler_type = output->scheduler_type;
> +	local_irq_restore(flags);
> +
> +	pr_info("mshv: hypervisor using %s\n",
> scheduler_type_to_string(hv_scheduler_type));

Nit: In this file we are using two styles of prints, few are appended with
"mshv:" and few with "__func__".  It's better to have a single style
for one module.