From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 42EAF190664; Mon, 27 Jan 2025 21:43:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=13.77.154.182 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738014184; cv=none; b=NWwRH6MMKxeXu5P6WzaNe+o/XeEJXmKLpNYeXzrnjT5Mkx98QcwmnxyskOGE5zZSOo0UIkZfdLhaOGlBSUnVvrzPHj2iovthOlI6/ZrCBJmshrBdEhDF/SdghJy8xH9KdmPasQo4e8QUs0gag6TQeUjqXfPHbB3q/35Vzt8LkMo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738014184; c=relaxed/simple; bh=amWnXKJDfu9x7z8AEwJSfk3fbiQeT7cAvuR5bp9HWTc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=eajmI1vKzMPoWGhKkhXJ42lY2rpawB4wqXb3l7ruwIgOpRkfizUuuWS/ejawNwkXAL7U3NDOsGQsBJKe/V/7GEFR10VJ8iRgM3oXhjNAHG8Xme5IrqNJ8IwWbhubVGM3cBNWNRXVX3K9CR0zD6LhudoLpl0mKOGllws4ZE/Sb9I= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.microsoft.com; spf=pass smtp.mailfrom=linux.microsoft.com; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b=R4CbrTEs; arc=none smtp.client-ip=13.77.154.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.microsoft.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.microsoft.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="R4CbrTEs" Received: from hm-sls2 (bras-base-toroon4332w-grc-51-184-146-177-43.dsl.bell.ca [184.146.177.43]) by linux.microsoft.com (Postfix) with ESMTPSA id 38EF0203716A; Mon, 27 Jan 2025 13:43:02 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 38EF0203716A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1738014182; bh=sDyFIa9eqolzc81S1g1JJRzaiBj3izmmQxt0+YYah2c=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=R4CbrTEs3YCAokeulW4Lh9X9v/nwjnxkI5U6CpFnK7+9EwBUlBMF+05Mi232qZu+w OVXcUu1xeANxKBlDfo9cE3oF0uAGeEzIovjeyGrS1nZKL9SE6dxdEI5D2wkGu2oBuy hC/rwIbjHCI376IJDfLPQ6cZjUQjym6fweosUrro= Date: Mon, 27 Jan 2025 16:42:56 -0500 From: Hamza Mahfooz To: Michael Kelley Cc: "linux-hyperv@vger.kernel.org" , "stable@vger.kernel.org" , Wei Liu , "K. Y. Srinivasan" , Haiyang Zhang , Dexuan Cui , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] drivers/hv: select PCI_HYPERV if PCI is enabled Message-ID: References: <20250127180947.123174-1-hamzamahfooz@linux.microsoft.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Jan 27, 2025 at 09:02:22PM +0000, Michael Kelley wrote: > From: Hamza Mahfooz 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 > > Signed-off-by: Hamza Mahfooz > > --- > > 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 > > >