Re: [PATCH] Drivers: hv: Introduce mshv_vtl driver

[Roman Kisel <romank@linux.microsoft.com>]

On 5/7/2025 8:00 PM, Saurabh Singh Sengar wrote:
>>
>> On 5/7/2025 4:21 AM, Naman Jain wrote:
>>>
>>>
>>
>> [...]
>>
>>> <snip>
>>>
>>>>> +        return -EINVAL;
>>>>> +    if (copy_from_user(payload, (void __user *)message.payload_ptr,
>>>>> +               message.payload_size))
>>>>> +        return -EFAULT;
>>>>> +
>>>>> +    return hv_post_message((union
>>>>
>>>> This function definition is in separate file which can be build as
>>>> independent module, this will cause problem while linking . Try
>>>> building with CONFIG_HYPERV=m and check.
>>>>
>>>> - Saurabh
>>>
>>> Thanks for reviewing Saurabh. As CONFIG_HYPERV can be set to 'm'
>>> and CONFIG_MSHV_VTL depends on it, changing CONFIG_MSHV_VTL to
>>> tristate and a few tweaks in Makefile will fix this issue. This will
>>> ensure that mshv_vtl is also built as a module when hyperv is built as a
>> module.
>>>
>>> I'll take care of this in next version.
>>
>> Let me ask for a clarification. How would the system boot if CONFIG_HYPERV
>> is set to m? The arch parts are going to be still compiled-in, correct?
>> Otherwise I don't see how that would initialize.
>>
>> I am thinking who would load Hyper-V modules on the system that requires
>> Hyper-V here. It is understandable that distro's build Hyper-V as a module.
>> That way, they don't have to load anything when there is no Hyper-V. Here, it
>> is Hyper-V in and out, what do we need to fix?
> 
> We need to fix compilation - for everyone.

You seem to know for whom it is broken, would be great to share this
knowledge. When CONFIG_MSHV_VTL is set to "m", OpenHCL will break down
without additional work. So why do we need to be able to build that
as a module, to let someone build the firmware that doesn't work?

So far the request comes off as absurd to me.

> 
> - Saurabh
> 
>>
>>>
>>> here is the diff for reference:
>>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig index
>>> 57dcfcb69b88..c7f21b483377 100644
>>> --- a/drivers/hv/Kconfig
>>> +++ b/drivers/hv/Kconfig
>>> @@ -73,7 +73,7 @@ config MSHV_ROOT
>>>             If unsure, say N.
>>>
>>>    config MSHV_VTL
>>> -       bool "Microsoft Hyper-V VTL driver"
>>> +       tristate "Microsoft Hyper-V VTL driver"
>>>           depends on HYPERV && X86_64
>>>           depends on TRANSPARENT_HUGEPAGE
>>>           depends on OF
>>> diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile index
>>> 5e785dae08cc..c53a0df746b7 100644
>>> --- a/drivers/hv/Makefile
>>> +++ b/drivers/hv/Makefile
>>> @@ -15,9 +15,11 @@ hv_vmbus-$(CONFIG_HYPERV_TESTING)    +=
>>> hv_debugfs.o
>>>    hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_utils_transport.o
>>>    mshv_root-y := mshv_root_main.o mshv_synic.o mshv_eventfd.o
>>> mshv_irq.o \
>>>                  mshv_root_hv_call.o mshv_portid_table.o
>>> +mshv_vtl-y := mshv_vtl_main.o
>>>
>>>    # Code that must be built-in
>>>    obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o -obj-$(subst
>>> m,y,$(CONFIG_MSHV_ROOT)) += hv_proc.o mshv_common.o
>>> -
>>> -mshv_vtl-y := mshv_vtl_main.o mshv_common.o
>>> +obj-$(subst m,y,$(CONFIG_MSHV_ROOT)) += hv_proc.o ifneq
>>> +($(CONFIG_MSHV_ROOT) $(CONFIG_MSHV_VTL),)
>>> +    obj-y += mshv_common.o
>>> +endif
>>>
>>> Regards,
>>> Naman
>>
>> --
>> Thank you,
>> Roman
> 

-- 
Thank you,
Roman