linux-kernel - Re: [PATCH] Drivers: hv: Introduce mshv

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: <aBztdK82ZQQvnWsh@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>
Date: Thu, 8 May 2025 17:44:20 +0000
From: Wei Liu <wei.liu@...nel.org>
To: Roman Kisel <romank@...ux.microsoft.com>
Cc: Saurabh Singh Sengar <ssengar@...rosoft.com>,
	Naman Jain <namjain@...ux.microsoft.com>,
	KY Srinivasan <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
	Anirudh Rayabharam <anrayabh@...ux.microsoft.com>,
	Saurabh Sengar <ssengar@...ux.microsoft.com>,
	Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>,
	Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: Re: [PATCH] Drivers: hv: Introduce mshv_vtl driver

On Thu, May 08, 2025 at 08:44:14AM -0700, Roman Kisel wrote:
> 
> 
> 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.
> 

I don't think this is an absurd request.

While obviously Microsoft will only build the code as builtin, there are
bots that do randconfig build tests but never run the resulting binary.

Thanks,
Wei.

> > 
> > - 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
>