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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c53fa288-a16d-4f23-83de-84d6ff20823c@linux.microsoft.com>
Date:   Tue, 22 Aug 2023 14:00:10 -0700
From:   Nuno Das Neves <nunodasneves@...ux.microsoft.com>
To:     Saurabh Singh Sengar <ssengar@...rosoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>
Cc:     "patches@...ts.linux.dev" <patches@...ts.linux.dev>,
        "Michael Kelley (LINUX)" <mikelley@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Dexuan Cui <decui@...rosoft.com>,
        "apais@...ux.microsoft.com" <apais@...ux.microsoft.com>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        "ssengar@...ux.microsoft.com" <ssengar@...ux.microsoft.com>,
        MUKESH RATHOR <mukeshrathor@...rosoft.com>,
        "stanislav.kinsburskiy@...il.com" <stanislav.kinsburskiy@...il.com>,
        "jinankjain@...ux.microsoft.com" <jinankjain@...ux.microsoft.com>,
        vkuznets <vkuznets@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "will@...nel.org" <will@...nel.org>,
        "catalin.marinas@....com" <catalin.marinas@....com>
Subject: Re: [PATCH v2 15/15] Drivers: hv: Add modules to expose /dev/mshv to
 VMMs running on Hyper-V

On 8/21/2023 11:18 AM, Saurabh Singh Sengar wrote:
> 
> 
>> -----Original Message-----
>> From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
>> Sent: Friday, August 18, 2023 3:32 AM
>> To: linux-hyperv@...r.kernel.org; linux-kernel@...r.kernel.org;
>> x86@...nel.org; linux-arm-kernel@...ts.infradead.org; linux-
>> arch@...r.kernel.org
>> Cc: patches@...ts.linux.dev; Michael Kelley (LINUX)
>> <mikelley@...rosoft.com>; KY Srinivasan <kys@...rosoft.com>;
>> wei.liu@...nel.org; Haiyang Zhang <haiyangz@...rosoft.com>; Dexuan Cui
>> <decui@...rosoft.com>; apais@...ux.microsoft.com; Tianyu Lan
>> <Tianyu.Lan@...rosoft.com>; ssengar@...ux.microsoft.com; MUKESH
>> RATHOR <mukeshrathor@...rosoft.com>; stanislav.kinsburskiy@...il.com;
>> jinankjain@...ux.microsoft.com; vkuznets <vkuznets@...hat.com>;
>> tglx@...utronix.de; mingo@...hat.com; bp@...en8.de;
>> dave.hansen@...ux.intel.com; hpa@...or.com; will@...nel.org;
>> catalin.marinas@....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.
> 

Thanks, we do need to return here.

This function is called on a cleanup path (deleting the guest VM), so if it fails
something is wrong. Instead of returning void, I think we should check the return
value with WARN_ON(), and abort the cleanup if it failed.

>> +
>> +/* 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.

Thanks, I'll switch them all to __func__