[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <68344e40-45aa-41d1-9df2-26f12db9e109@linux.microsoft.com>
Date: Fri, 25 Aug 2023 11:26:02 -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/23/2023 12:40 AM, Saurabh Singh Sengar wrote:
>
>
>> -----Original Message-----
>> From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
>> Sent: Wednesday, August 23, 2023 1:49 AM
>> To: Saurabh Singh Sengar <ssengar@...rosoft.com>; 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: Re: [PATCH v2 15/15] Drivers: hv: Add modules to expose /dev/mshv
>> to VMMs running on Hyper-V
>>
>> On 8/19/2023 10:19 PM, Saurabh Singh Sengar wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
>>>> Sent: Saturday, August 19, 2023 12:30 AM
>>>> To: Saurabh Singh Sengar <ssengar@...rosoft.com>; 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: Re: [PATCH v2 15/15] Drivers: hv: Add modules to expose
>>>> /dev/mshv to VMMs running on Hyper-V
>>>>
>>>> On 8/18/2023 6:08 AM, Saurabh Singh Sengar wrote:
>>>>>> +
>>>>>> +config MSHV_VTL
>>>>>> + tristate "Microsoft Hyper-V VTL driver"
>>>>>> + depends on MSHV
>>>>>> + select HYPERV_VTL_MODE
>>>>>> + select TRANSPARENT_HUGEPAGE
>>>>>
>>>>> TRANSPARENT_HUGEPAGE can be avoided for now.
>>>>>
>>>>
>>>> I will remove it in the next version. Thanks.
>>>>>> +
>>>>>> +#define HV_GET_REGISTER_BATCH_SIZE \
>>>>>> + (HV_HYP_PAGE_SIZE / sizeof(union hv_register_value))
>>>>>> +#define HV_SET_REGISTER_BATCH_SIZE \
>>>>>> + ((HV_HYP_PAGE_SIZE - sizeof(struct hv_input_set_vp_registers)) \
>>>>>> + / sizeof(struct hv_register_assoc))
>>>>>> +
>>>>>> +int hv_call_get_vp_registers(
>>>>>> + u32 vp_index,
>>>>>> + u64 partition_id,
>>>>>> + u16 count,
>>>>>> + union hv_input_vtl input_vtl,
>>>>>> + struct hv_register_assoc *registers) {
>>>>>> + struct hv_input_get_vp_registers *input_page;
>>>>>> + union hv_register_value *output_page;
>>>>>> + u16 completed = 0;
>>>>>> + unsigned long remaining = count;
>>>>>> + int rep_count, i;
>>>>>> + u64 status;
>>>>>> + unsigned long flags;
>>>>>> +
>>>>>> + local_irq_save(flags);
>>>>>> +
>>>>>> + input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
>>>>>> + output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
>>>>>> +
>>>>>> + input_page->partition_id = partition_id;
>>>>>> + input_page->vp_index = vp_index;
>>>>>> + input_page->input_vtl.as_uint8 = input_vtl.as_uint8;
>>>>>> + input_page->rsvd_z8 = 0;
>>>>>> + input_page->rsvd_z16 = 0;
>>>>>> +
>>>>>> + while (remaining) {
>>>>>> + rep_count = min(remaining, HV_GET_REGISTER_BATCH_SIZE);
>>>>>> + for (i = 0; i < rep_count; ++i)
>>>>>> + input_page->names[i] = registers[i].name;
>>>>>> +
>>>>>> + status = hv_do_rep_hypercall(HVCALL_GET_VP_REGISTERS,
>>>>>> rep_count,
>>>>>> + 0, input_page, output_page);
>>>>>
>>>>> Is there any possibility that count value is passed 0 by mistake ?
>>>>> In that case status will remain uninitialized.
>>>>>
>>>>
>>>> These lines ensure rep_count is never 0 here:
>>>>
>>>> while (remaining) {
>>>> rep_count = min(remaining, HV_GET_REGISTER_BATCH_SIZE);
>>>>
>>>> Remaining can't be 0 or the loop would exit, and
>>>> HV_GET_REGISTER_BATCH_SIZE is not 0, or we would never get any
>> registers.
>>>
>>> There is a parameter in this function "count". I was checking if there
>>> is any possibility that is passed as 0 by mistake ? this will lead to
>>> "remaining" value as 0 and loop will never execute. Which results using
>> "status" uninitialized later in the function.
>>>
>>>
>>
>> Ah ok, thanks! I will change it to return immediately in case count is 0.
>
> Or you can initialize status with appropriate error value, either way is fine.
> Please consider fixing the same issue in hv_call_set_vp_registers as well.
>
Thanks again - noted.
Powered by blists - more mailing lists