Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 14 Feb 2025 15:33:44 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 649a910e34b0 - main - bhyve: Avoid holding /dev/pci open unnecessarily
Message-ID:  <202502141533.51EFXinw007896@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=649a910e34b0314647d16a94f8af6de0f4cfd4b5

commit 649a910e34b0314647d16a94f8af6de0f4cfd4b5
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-02-14 15:25:08 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-02-14 15:25:08 +0000

    bhyve: Avoid holding /dev/pci open unnecessarily
    
    Some device models, LPC in particular, will call pci_host_read_config()
    when probing for devices.  Currently this results in pcifd_init()
    opening /dev/pci, and thus bhyve holds the fd open even when it's not
    needed.
    
    Modify pci_host_{read,write}_config() to open /dev/pci independent of
    the global pcifd.  This means that these routines can only be used
    during VM initialization, as capsicum will prevent further opens
    afterward.  Introduce internal wrappers which use the global pcifd,
    intended for the passthru code.
    
    Reviewed by:    jhb
    MFC after:      3 weeks
    Fixes:          563fd2240e13 ("bhyve: export funcs for read/write pci config")
    Differential Revision:  https://reviews.freebsd.org/D48908
---
 usr.sbin/bhyve/pci_passthru.c | 119 +++++++++++++++++++++++++++++-------------
 1 file changed, 83 insertions(+), 36 deletions(-)

diff --git a/usr.sbin/bhyve/pci_passthru.c b/usr.sbin/bhyve/pci_passthru.c
index b8ac782bd7be..61983010192a 100644
--- a/usr.sbin/bhyve/pci_passthru.c
+++ b/usr.sbin/bhyve/pci_passthru.c
@@ -120,13 +120,24 @@ msi_caplen(int msgctrl)
 }
 
 static int
-pcifd_init(void)
+pcifd_open(void)
 {
-	pcifd = open(_PATH_DEVPCI, O_RDWR, 0);
-	if (pcifd < 0) {
+	int fd;
+
+	fd = open(_PATH_DEVPCI, O_RDWR, 0);
+	if (fd < 0) {
 		warn("failed to open %s", _PATH_DEVPCI);
-		return (1);
+		return (-1);
 	}
+	return (fd);
+}
+
+static int
+pcifd_init(void)
+{
+	pcifd = pcifd_open();
+	if (pcifd < 0)
+		return (1);
 
 #ifndef WITHOUT_CAPSICUM
 	cap_rights_t pcifd_rights;
@@ -143,43 +154,75 @@ pcifd_init(void)
 	return (0);
 }
 
-uint32_t
-pci_host_read_config(const struct pcisel *sel, long reg, int width)
+static uint32_t
+host_read_config(int fd, const struct pcisel *sel, long reg, int width)
 {
 	struct pci_io pi;
 
-	if (pcifd < 0 && pcifd_init()) {
-		return (0);
-	}
-
 	bzero(&pi, sizeof(pi));
 	pi.pi_sel = *sel;
 	pi.pi_reg = reg;
 	pi.pi_width = width;
 
-	if (ioctl(pcifd, PCIOCREAD, &pi) < 0)
-		return (0);				/* XXX */
+	if (ioctl(fd, PCIOCREAD, &pi) < 0)
+		return (0);			/* XXX */
 	else
 		return (pi.pi_data);
 }
 
-void
-pci_host_write_config(const struct pcisel *sel, long reg, int width,
+static uint32_t
+passthru_read_config(const struct pcisel *sel, long reg, int width)
+{
+	return (host_read_config(pcifd, sel, reg, width));
+}
+
+uint32_t
+pci_host_read_config(const struct pcisel *sel, long reg, int width)
+{
+	uint32_t ret;
+	int fd;
+
+	fd = pcifd_open();
+	if (fd < 0)
+		return (0);
+	ret = host_read_config(fd, sel, reg, width);
+	(void)close(fd);
+	return (ret);
+}
+
+static void
+host_write_config(int fd, const struct pcisel *sel, long reg, int width,
     uint32_t data)
 {
 	struct pci_io pi;
 
-	if (pcifd < 0 && pcifd_init()) {
-		return;
-	}
-
 	bzero(&pi, sizeof(pi));
 	pi.pi_sel = *sel;
 	pi.pi_reg = reg;
 	pi.pi_width = width;
 	pi.pi_data = data;
 
-	(void)ioctl(pcifd, PCIOCWRITE, &pi);		/* XXX */
+	(void)ioctl(fd, PCIOCWRITE, &pi);		/* XXX */
+}
+
+static void
+passthru_write_config(const struct pcisel *sel, long reg, int width,
+    uint32_t data)
+{
+	host_write_config(pcifd, sel, reg, width, data);
+}
+
+void
+pci_host_write_config(const struct pcisel *sel, long reg, int width,
+    uint32_t data)
+{
+	int fd;
+
+	fd = pcifd_open();
+	if (fd < 0)
+		return;
+	host_write_config(fd, sel, reg, width, data);
+	(void)close(fd);
 }
 
 #ifdef LEGACY_SUPPORT
@@ -224,24 +267,24 @@ cfginitmsi(struct passthru_softc *sc)
 	 * Parse the capabilities and cache the location of the MSI
 	 * and MSI-X capabilities.
 	 */
-	sts = pci_host_read_config(&sel, PCIR_STATUS, 2);
+	sts = passthru_read_config(&sel, PCIR_STATUS, 2);
 	if (sts & PCIM_STATUS_CAPPRESENT) {
-		ptr = pci_host_read_config(&sel, PCIR_CAP_PTR, 1);
+		ptr = passthru_read_config(&sel, PCIR_CAP_PTR, 1);
 		while (ptr != 0 && ptr != 0xff) {
-			cap = pci_host_read_config(&sel, ptr + PCICAP_ID, 1);
+			cap = passthru_read_config(&sel, ptr + PCICAP_ID, 1);
 			if (cap == PCIY_MSI) {
 				/*
 				 * Copy the MSI capability into the config
 				 * space of the emulated pci device
 				 */
 				sc->psc_msi.capoff = ptr;
-				sc->psc_msi.msgctrl = pci_host_read_config(&sel,
-				    ptr + 2, 2);
+				sc->psc_msi.msgctrl =
+				    passthru_read_config(&sel, ptr + 2, 2);
 				sc->psc_msi.emulated = 0;
 				caplen = msi_caplen(sc->psc_msi.msgctrl);
 				capptr = ptr;
 				while (caplen > 0) {
-					u32 = pci_host_read_config(&sel, capptr,
+					u32 = passthru_read_config(&sel, capptr,
 					    4);
 					pci_set_cfgdata32(pi, capptr, u32);
 					caplen -= 4;
@@ -256,7 +299,7 @@ cfginitmsi(struct passthru_softc *sc)
 				msixcap_ptr = (char *)&msixcap;
 				capptr = ptr;
 				while (caplen > 0) {
-					u32 = pci_host_read_config(&sel, capptr,
+					u32 = passthru_read_config(&sel, capptr,
 					    4);
 					memcpy(msixcap_ptr, &u32, 4);
 					pci_set_cfgdata32(pi, capptr, u32);
@@ -265,7 +308,7 @@ cfginitmsi(struct passthru_softc *sc)
 					msixcap_ptr += 4;
 				}
 			}
-			ptr = pci_host_read_config(&sel, ptr + PCICAP_NEXTPTR,
+			ptr = passthru_read_config(&sel, ptr + PCICAP_NEXTPTR,
 			    1);
 		}
 	}
@@ -301,7 +344,7 @@ cfginitmsi(struct passthru_softc *sc)
 	 */
 	if ((sts & PCIM_STATUS_CAPPRESENT) != 0 && sc->psc_msi.capoff == 0) {
 		int origptr, msiptr;
-		origptr = pci_host_read_config(&sel, PCIR_CAP_PTR, 1);
+		origptr = passthru_read_config(&sel, PCIR_CAP_PTR, 1);
 		msiptr = passthru_add_msicap(pi, 1, origptr);
 		sc->psc_msi.capoff = msiptr;
 		sc->psc_msi.msgctrl = pci_get_cfgdata16(pi, msiptr + 2);
@@ -535,6 +578,8 @@ cfginitbar(struct passthru_softc *sc)
 	 * Initialize BAR registers
 	 */
 	for (i = 0; i <= PCI_BARMAX; i++) {
+		uint8_t lobits;
+
 		bzero(&bar, sizeof(bar));
 		bar.pbi_sel = sc->psc_sel;
 		bar.pbi_reg = PCIR_BAR(i);
@@ -580,8 +625,8 @@ cfginitbar(struct passthru_softc *sc)
 			return (-1);
 
 		/* Use same lobits as physical bar */
-		uint8_t lobits = pci_host_read_config(&sc->psc_sel, PCIR_BAR(i),
-		    0x01);
+		lobits = (uint8_t)passthru_read_config(&sc->psc_sel,
+		    PCIR_BAR(i), 0x01);
 		if (bartype == PCIBAR_MEM32 || bartype == PCIBAR_MEM64) {
 			lobits &= ~PCIM_BAR_MEM_BASE;
 		} else {
@@ -628,7 +673,7 @@ cfginit(struct pci_devinst *pi, int bus, int slot, int func)
 	intpin = pci_get_cfgdata8(pi, PCIR_INTPIN);
 	for (int i = 0; i <= PCIR_MAXLAT; i += 4) {
 		pci_set_cfgdata32(pi, i,
-		    pci_host_read_config(&sc->psc_sel, i, 4));
+		    passthru_read_config(&sc->psc_sel, i, 4));
 	}
 	pci_set_cfgdata16(pi, PCIR_COMMAND, cmd);
 	pci_set_cfgdata8(pi, PCIR_INTLINE, intline);
@@ -987,15 +1032,17 @@ passthru_cfgread_default(struct passthru_softc *sc,
 	 * device's config space.
 	 */
 	if (coff == PCIR_COMMAND) {
+		uint32_t st;
+
 		if (bytes <= 2)
 			return (-1);
-		*rv = pci_host_read_config(&sc->psc_sel, PCIR_STATUS, 2) << 16 |
-		    pci_get_cfgdata16(pi, PCIR_COMMAND);
+		st = passthru_read_config(&sc->psc_sel, PCIR_STATUS, 2);
+		*rv = (st << 16) | pci_get_cfgdata16(pi, PCIR_COMMAND);
 		return (0);
 	}
 
 	/* Everything else just read from the device's config space */
-	*rv = pci_host_read_config(&sc->psc_sel, coff, bytes);
+	*rv = passthru_read_config(&sc->psc_sel, coff, bytes);
 
 	return (0);
 }
@@ -1078,7 +1125,7 @@ passthru_cfgwrite_default(struct passthru_softc *sc, struct pci_devinst *pi,
 			return (-1);
 
 		/* Update the physical status register. */
-		pci_host_write_config(&sc->psc_sel, PCIR_STATUS, val >> 16, 2);
+		passthru_write_config(&sc->psc_sel, PCIR_STATUS, val >> 16, 2);
 
 		/* Update the virtual command register. */
 		cmd_old = pci_get_cfgdata16(pi, PCIR_COMMAND);
@@ -1087,7 +1134,7 @@ passthru_cfgwrite_default(struct passthru_softc *sc, struct pci_devinst *pi,
 		return (0);
 	}
 
-	pci_host_write_config(&sc->psc_sel, coff, bytes, val);
+	passthru_write_config(&sc->psc_sel, coff, bytes, val);
 
 	return (0);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502141533.51EFXinw007896>