Re: [PATCH 1/1] Drivers: hv: vmbus: Fail VMBus module init for non-nested root partition

[Easwar Hariharan <easwar.hariharan@linux.microsoft.com>]

On 8/4/2026 21:13, Michael Kelley wrote:
> From: Easwar Hariharan <easwar.hariharan@linux.microsoft.com> Sent: Tuesday, August 4, 2026 3:49 PM
>>
>> On 8/4/2026 15:47, Easwar Hariharan wrote:
>>> On 8/4/2026 15:17, Michael Kelley wrote:
>>>> From: Easwar Hariharan <easwar.hariharan@linux.microsoft.com> Sent: Tuesday, August 4, 2026 12:40 PM
>>>>>
>>>>> On 8/4/2026 12:05, Michael Kelley wrote:
>>>>>> The VMBus module should not be loaded when Linux is running directly
>>>>>> in the root partition and root is not nested in another VM. Current
>>>>>> code checks this condition and skips VMBus module initialization, which
>>>>>> works. But it returns 0 as the result, so Linux thinks the module has
>>>>>> successfully loaded. Later, if the module were to be unloaded, the
>>>>>> VMBus module unload code tries to clean up things that were never
>>>>>> initialized, resulting in memory faults and a panic.
>>>>>>
>>>>>> Fix this by having VMBus module initialization return -ENODEV for this
>>>>>> case. The module is then not loaded, and the unload path can never run.
>>>>>>
>>>>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>>>>> Closes: https://lore.kernel.org/linux-
>>>>> hyperv/20260721154943.A09BD1F00A3D@smtp.kernel.org/
>>>>>> Fixes: 7e279d78664aa ("Drivers: hv: vmbus: skip VMBus initialization if Linux is root")
>>>>>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
>>>>>> ---
>>>>>>  drivers/hv/vmbus_drv.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>>>>>> index e19ec73b0187..849d7e1a7320 100644
>>>>>> --- a/drivers/hv/vmbus_drv.c
>>>>>> +++ b/drivers/hv/vmbus_drv.c
>>>>>> @@ -2976,7 +2976,7 @@ static int __init hv_acpi_init(void)
>>>>>>  		return -ENODEV;
>>>>>>
>>>>>>  	if (hv_root_partition() && !hv_nested)
>>>>>> -		return 0;
>>>>>> +		return -ENODEV;
>>>>>>
>>>>>>  	/*
>>>>>>  	 * Get ACPI resources first.
>>>>>
>>>>> This seems straightforward:
>>>>
>>>> Alas, it's not so straightforward, as Sashiko pointed out.  I knew that
>>>> the mshv module has a dependency on the vmbus module, but had
>>>> forgotten. There's a reason for the dependency as described in the
>>>> commit message for 840b740a35bf.
>>>>
>>>> There's another easy way to fix the VMBus module unload problem.
>>>> I'll send a v2. :-)
>>>>
>>>> Michael
>>>>
>>> I may be missing something, but mshv_root is used in 3 cases:
>>>
>>> 1) Baremetal root partition, which requires no VMBus
>>>
>>> 2) L1VH aka Direct Virtualization, which is conditioned on hv_l1vh_partition(), which is
>> not the check here in the vmbus driver
>>>
>>> 3) For the OpenHCL paravisor, where I honestly don't know what the dependency
>> chain looks like, but based on a quick glance
>>> at https://github.com/microsoft/OHCL-Linux-Kernel/ and a cursory grep, doesn't
>> seem to rely on hv_root_partition() but does rely
>>> on VMbus.
>>>
>>> I feel like Sashiko's review falls into item 1, but then again, there may just be a
>> mismatch
>>> between reality and my mental model, or my mental model may becorrect, but the
>> code doesn't match it.
>>>
>>> Thanks,
>>> Easwar (he/him)
>>
>>
>> Well, number 4 is as a parent on a nested hypervisor, but !hv_nested takes care of that.
>>
> 
> For your #1, indeed the root partition does not require actual VMBus
> functionality. But it *does* require that the VMBus module be loaded
> if CONFIG_HYPERV_VMBUS=y. In that case, the only function the root
> requires for #1 is the function hv_vmbus_exists(), and it requires that
> to distinguish between your #1 and your #4 when it sets up the SynIC.
> 
> If you want to build an image that only works for #1, then build with
> CONFIG_HYPERV_VMBUS=n and there's no issue. But if you want an
> image that works for #1 or #4, build with CONFIG_HYPERV_VMBUS=y
> or =m, and the VMBus module must loaded so that hv_vmbus_exists()
> gives the right answer (false for #1, true for #4). Furthermore, the
> mshv_root dependency on the VMBus module ensures that the
> VMBus module is always loaded before the mshv_root module so
> that hv_vmbus_exists() will give an up-to-date answer when
> mshv_root asks.
> 
> The way the SynIC is managed should be refactored to better
> handle #4, as Jork Loeser has proposed. Hopefully such a refactoring
> would make the mshv_root->vmbus dependency go away. But until
> such a refactoring is done, the dependency is what we have.
> 
> I agree that your #2 and #3 are not relevant here.
> 
> Michael

Makes sense, I did not think of the distro user that'd probably like 1 image that serves all use
cases. I'll look at your v2.

Thanks,
Easwar (he/him)