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>