linux-kernel - RE: [PATCH] drivers/hv: select PCI

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] [day] [month] [year] [list]

Message-ID:
 <SN6PR02MB4157D37501FAC6288334B62ED4EF2@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Tue, 28 Jan 2025 04:45:32 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Wei Liu <wei.liu@...nel.org>, Hamza Mahfooz
	<hamzamahfooz@...ux.microsoft.com>
CC: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>, "K. Y. Srinivasan"
	<kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, Dexuan Cui
	<decui@...rosoft.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] drivers/hv: select PCI_HYPERV if PCI is enabled

From: Wei Liu <wei.liu@...nel.org> Sent: Monday, January 27, 2025 8:10 PM
> 
> On Mon, Jan 27, 2025 at 04:42:56PM -0500, Hamza Mahfooz wrote:
> > On Mon, Jan 27, 2025 at 09:02:22PM +0000, Michael Kelley wrote:
> > > From: Hamza Mahfooz <hamzamahfooz@...ux.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.
> 
> Does inserting the PCI_HYPERV module trigger a (re)scanning of the
> (v)PCI bus? If so, the passed-through NVMe devices should show up just
> fine, I suppose.

vPCI devices are made available to a Hyper-V VM as VMBus offers of
class "vPCI". For such an offer, the Linux device subsystem tries to find
a matching VMBus driver. If the vPCI driver isn't available, the offer just
stays in the VMBus code waiting for a driver.  If the vPCI driver later shows
up, the Linux device subsystem does the match, and VMBus device
probing proceeds. The hv_pci_probe() function in the vPCI driver is called,
and all the normal steps are taken so that the vPCI device appears and is
functional in the VM. The details of "the normal steps" are in the
documentation in Documentation/virt/hyperv/vpci.rst. See Sections
"Device Presentation" and "PCI Device Setup". A key point is that each
vPCI device is modelled in Linux to be a separate PCI domain with its
own host bridge and root PCI bus.

So the outcome is as you describe and would expect, though the
mechanism is not a scan of the virtual PCI bus. 

Michael

> 
> I agree with Michael that we should not select PCI_HYPERV by default. In
> some environments, it is not needed at all.
> 
> Thanks,
> Wei.