On Mon, Jan 27, 2025 at 09:02:22PM +0000, Michael Kelley wrote:
From: Hamza Mahfooz hamzamahfooz@linux.microsoft.com Sent: Monday, January 27, 2025 10:10 AM
We should select PCI_HYPERV here, otherwise it's possible for devices to not show up as expected, at least not in an orderly manner.
The commit message needs more precision: What does "not show up" mean, and what does "not in an orderly manner" mean? And "it's possible" is vague -- can you be more specific about the conditions? Also, avoid the use of personal pronouns like "we".
But the commit message notwithstanding, I don't think this is change that should be made. CONFIG_PCI_HYPERV refers to the VMBus device driver for handling vPCI (a.k.a PCI pass-thru) devices. It's perfectly possible and normal for a VM on Hyper-V to not have any such devices, in which case the driver isn't needed and should not be forced to be included. (See Documentation/virt/hyperv/vpci.rst for more on vPCI devices.)
Ya, we ran into an issue where CONFIG_NVME_CORE=y and CONFIG_PCI_HYPERV=m caused the passed-through SSDs not to show up (i.e. they aren't visible to userspace). I guess it's cause PCI_HYPERV has to load in before the nvme stuff for that workload. So, I thought it was reasonable to select PCI_HYPERV here to prevent someone else from shooting themselves in the foot. Though, I guess it really it on the distro guys to get that right.
There are other VMBus device drivers: storvsc, netvsc, the Hyper-V frame buffer driver, the "util" drivers for shutdown, KVP, etc., and more. These each have their own CONFIG_* entry, and current practice doesn't select them when CONFIG_HYPERV is set. I don't see a reason that the vPCI driver should be handled differently.
Also, different distro vendors take different approaches as to whether these drivers are built as modules, or as built-in to their kernel images. I'm not sure what the Kconfig tool does when a SELECT statement identifies a tri-state setting. Since CONFIG_HYPERV is tri-state, does the target of the SELECT get the same tri-state value as CONFIG_HYPERV? Again, that may not be what distro vendors want. They may choose to have some of the VMBus drivers built-in and others built as modules. Distro vendors (and anyone doing a custom kernel build) should be allowed to make their choices just like for any other drivers.
If you've come across a situation these considerations don't apply or are problematic, provide more details. That's what a good commit message should do -- be convincing as to *why* the change should be made! :-)
Michael
Cc: stable@vger.kernel.org Cc: Wei Liu wei.liu@kernel.org Signed-off-by: Hamza Mahfooz hamzamahfooz@linux.microsoft.com
drivers/hv/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig index 862c47b191af..6ee75b3f0fa6 100644 --- a/drivers/hv/Kconfig +++ b/drivers/hv/Kconfig @@ -9,6 +9,7 @@ config HYPERV select PARAVIRT select X86_HV_CALLBACK_VECTOR if X86 select OF_EARLY_FLATTREE if OF
- select PCI_HYPERV if PCI help Select this option to run Linux as a Hyper-V client operating system.
-- 2.47.1