From nobody Tue Apr 29 18:30:35 2025 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Zn8593SkMz5vN8V; Tue, 29 Apr 2025 18:30:37 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Zn8573t5xz4HV1; Tue, 29 Apr 2025 18:30:35 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1745951435; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=8kVExsAydjMEbSDipmXbm+bXpAjELANQEr04kgKv7Ws=; b=MyiLA1xeV5PyhGwfjFbgN94DG8WBDVF1vX73hLAwRiThOQ6n2JBr+ClfANGjoK+Pa5NTc+ LIsSrmsR9vWcoAL8vDzg6sNIatUwk+aOhHXWDdPz6x+tUgzQDlsmiQ5/IQjRPk92fFv9sh K6zsEmSYW7s4TgzDytgv5RHiEFs6kSzl7quq+Y2f8dBsM479/4+NUNfXzESduYhWRKcm4f Ue+uJpugb2cU9nPRbFsK3sqZ5BjoWoy4CWD6PMoMXO3WaKzUVAIcBMXp7oiGoLdktApEMu Y7d46pbvvwb94QWnzfxxBuDrmDKdeFK6mAoGx2cqFTSW42HYeF/FrQboqw1S8g== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1745951435; a=rsa-sha256; cv=none; b=nTaTuG/2G9d7xJjifn6b53hxf3r79M89eSRT6pUxJ+FRZ8vNg79RVeU5DmUANQDt1pvqCf mcd6F5RVMo8L7uXleSbR1IngjVyd2mN+5fgqaYmtRU7Oo8ZYFqBToL3R1LgczNuWSVWUsa FYDhPMSTJgzYBO9HLZxOhNUDeY8XUrAfBDfa+E2135iZkT41A6jr5MuCq56MJHFXT4x+3J DauOPYpUZjIqG4D/f3TUEy4iVif2YmqND5qhpsP+stcZwwUQkHXWLp/lAOOHsXInFnLCk2 rzEKcZmln34meNGNlFA/PXp833GduAt5gwmHLyFYlne7x1wzowkXxxkg+7R2hQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1745951435; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=8kVExsAydjMEbSDipmXbm+bXpAjELANQEr04kgKv7Ws=; b=BmFLoA7WodxZ6d7HGfnXpAuYSul/Qbgq7J85xNpz1+thTIKLhJLnvPkxxzLuQZTCwAb2xi 2KHtBHPhDq0GW26/rb/dWL0hVcWqURMTECjD/FtY/ibXw5yI9NyFmlGkLS59pZTqrk3+Kp cQPrTVHFhl4xfSZ2ADQac5KiJlsJy6FMS5c6zVGwzahAZWWqJ5t4/LtxBL0W4bv6gmGJc8 OXqH1G7zs19VhRqVjBxkLmsmzqYJOofMUcYJakfAyd56tkzfnU/W9gaR79aURanYuDeoGX Z8gk/1+7UPMsRdCxD4ZpZ7IFkKv84mB7/e0NIyMJm6s+YqFKSX4FESpef5F+Ww== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Zn8573JbTzXQP; Tue, 29 Apr 2025 18:30:35 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 53TIUZaE022484; Tue, 29 Apr 2025 18:30:35 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 53TIUZer022481; Tue, 29 Apr 2025 18:30:35 GMT (envelope-from git) Date: Tue, 29 Apr 2025 18:30:35 GMT Message-Id: <202504291830.53TIUZer022481@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: John Baldwin Subject: git: 0a515a8d36ad - stable/14 - pci: Don't cache the count of MSI/MSI-X messages before allocation List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jhb X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 0a515a8d36ad41e21c4af00f9d97a32ae9fa61e0 Auto-Submitted: auto-generated The branch stable/14 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=0a515a8d36ad41e21c4af00f9d97a32ae9fa61e0 commit 0a515a8d36ad41e21c4af00f9d97a32ae9fa61e0 Author: John Baldwin AuthorDate: 2025-02-11 14:11:48 +0000 Commit: John Baldwin CommitDate: 2025-04-29 14:24:32 +0000 pci: Don't cache the count of MSI/MSI-X messages before allocation A device can in theory change the read-only fields in the MSI/MSI-X control registers that indicate the maximum number of supported registers in response to changing other device registers. For example, certain Intel networking VFs change the number of messages as a result of changes in the PCI_IOV_ADD_VF callback. To support this, always read the current value of the relevant control register in the *_count and *_alloc methods. Once messages have been allocated, the control register value remains cached. Reported by: Krzysztof Galazka Reviewed by: Krzysztof Galazka , erj Differential Revision: https://reviews.freebsd.org/D48890 (cherry picked from commit 346020138a0fd20085ebc285f090df38d7d18527) --- sys/dev/iavf/iavf_lib.c | 25 ---------------- sys/dev/iavf/iavf_lib.h | 1 - sys/dev/iavf/if_iavf_iflib.c | 3 -- sys/dev/pci/pci.c | 70 ++++++++++++++++++++++++++------------------ sys/dev/pci/pcireg.h | 3 ++ sys/dev/pci/pcivar.h | 2 -- 6 files changed, 45 insertions(+), 59 deletions(-) diff --git a/sys/dev/iavf/iavf_lib.c b/sys/dev/iavf/iavf_lib.c index 3116ce0501c2..433d31904ea4 100644 --- a/sys/dev/iavf/iavf_lib.c +++ b/sys/dev/iavf/iavf_lib.c @@ -1463,31 +1463,6 @@ iavf_mark_del_vlan_filter(struct iavf_sc *sc, u16 vtag) return (i); } -/** - * iavf_update_msix_devinfo - Fix MSIX values for pci_msix_count() - * @dev: pointer to kernel device - * - * Fix cached MSI-X control register information. This is a workaround - * for an issue where VFs spawned in non-passthrough mode on FreeBSD - * will have their PCI information cached before the PF driver - * finishes updating their PCI information. - * - * @pre Must be called before pci_msix_count() - */ -void -iavf_update_msix_devinfo(device_t dev) -{ - struct pci_devinfo *dinfo; - u32 msix_ctrl; - u8 msix_location; - - dinfo = (struct pci_devinfo *)device_get_ivars(dev); - msix_location = dinfo->cfg.msix.msix_location; - msix_ctrl = pci_read_config(dev, msix_location + PCIR_MSIX_CTRL, 2); - dinfo->cfg.msix.msix_ctrl = msix_ctrl; - dinfo->cfg.msix.msix_msgnum = (msix_ctrl & PCIM_MSIXCTRL_TABLE_SIZE) + 1; -} - /** * iavf_disable_queues_with_retries - Send PF multiple DISABLE_QUEUES messages * @sc: device softc diff --git a/sys/dev/iavf/iavf_lib.h b/sys/dev/iavf/iavf_lib.h index f3ccd9f0c52f..2f874b2e4410 100644 --- a/sys/dev/iavf/iavf_lib.h +++ b/sys/dev/iavf/iavf_lib.h @@ -474,7 +474,6 @@ struct iavf_mac_filter * u64 iavf_baudrate_from_link_speed(struct iavf_sc *sc); void iavf_add_vlan_filter(struct iavf_sc *sc, u16 vtag); int iavf_mark_del_vlan_filter(struct iavf_sc *sc, u16 vtag); -void iavf_update_msix_devinfo(device_t dev); void iavf_disable_queues_with_retries(struct iavf_sc *); int iavf_sysctl_current_speed(SYSCTL_HANDLER_ARGS); diff --git a/sys/dev/iavf/if_iavf_iflib.c b/sys/dev/iavf/if_iavf_iflib.c index d460df6e0d25..e4dd3b1e59a4 100644 --- a/sys/dev/iavf/if_iavf_iflib.c +++ b/sys/dev/iavf/if_iavf_iflib.c @@ -379,9 +379,6 @@ iavf_if_attach_pre(if_ctx_t ctx) scctx->isc_capabilities = scctx->isc_capenable = IAVF_CAPS; scctx->isc_tx_csum_flags = CSUM_OFFLOAD; - /* Update OS cache of MSIX control register values */ - iavf_update_msix_devinfo(dev); - return (0); err_vc_tq: diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c index 07a9534f6b91..7107fbe4884b 100644 --- a/sys/dev/pci/pci.c +++ b/sys/dev/pci/pci.c @@ -937,14 +937,10 @@ pci_read_cap(device_t pcib, pcicfgregs *cfg) case PCIY_MSI: /* PCI MSI */ cfg->msi.msi_location = ptr; cfg->msi.msi_ctrl = REG(ptr + PCIR_MSI_CTRL, 2); - cfg->msi.msi_msgnum = 1 << ((cfg->msi.msi_ctrl & - PCIM_MSICTRL_MMC_MASK)>>1); break; case PCIY_MSIX: /* PCI MSI-X */ cfg->msix.msix_location = ptr; cfg->msix.msix_ctrl = REG(ptr + PCIR_MSIX_CTRL, 2); - cfg->msix.msix_msgnum = (cfg->msix.msix_ctrl & - PCIM_MSIXCTRL_TABLE_SIZE) + 1; val = REG(ptr + PCIR_MSIX_TABLE, 4); cfg->msix.msix_table_bar = PCIR_BAR(val & PCIM_MSIX_BIR_MASK); @@ -1719,7 +1715,7 @@ pci_mask_msix(device_t dev, u_int index) struct pcicfg_msix *msix = &dinfo->cfg.msix; uint32_t offset, val; - KASSERT(msix->msix_msgnum > index, ("bogus index")); + KASSERT(PCI_MSIX_MSGNUM(msix->msix_ctrl) > index, ("bogus index")); offset = msix->msix_table_offset + index * 16 + 12; val = bus_read_4(msix->msix_table_res, offset); val |= PCIM_MSIX_VCTRL_MASK; @@ -1738,7 +1734,7 @@ pci_unmask_msix(device_t dev, u_int index) struct pcicfg_msix *msix = &dinfo->cfg.msix; uint32_t offset, val; - KASSERT(msix->msix_table_len > index, ("bogus index")); + KASSERT(PCI_MSIX_MSGNUM(msix->msix_ctrl) > index, ("bogus index")); offset = msix->msix_table_offset + index * 16 + 12; val = bus_read_4(msix->msix_table_res, offset); val &= ~PCIM_MSIX_VCTRL_MASK; @@ -1775,11 +1771,13 @@ pci_resume_msix(device_t dev) struct pcicfg_msix *msix = &dinfo->cfg.msix; struct msix_table_entry *mte; struct msix_vector *mv; - u_int i; + u_int i, msgnum; if (msix->msix_alloc > 0) { + msgnum = PCI_MSIX_MSGNUM(msix->msix_ctrl); + /* First, mask all vectors. */ - for (i = 0; i < msix->msix_msgnum; i++) + for (i = 0; i < msgnum; i++) pci_mask_msix(dev, i); /* Second, program any messages with at least one handler. */ @@ -1810,6 +1808,7 @@ pci_alloc_msix_method(device_t dev, device_t child, int *count) struct resource_list_entry *rle; u_int actual, i, max; int error, irq; + uint16_t ctrl, msgnum; /* Don't let count == 0 get us into trouble. */ if (*count < 1) @@ -1848,11 +1847,14 @@ pci_alloc_msix_method(device_t dev, device_t child, int *count) } cfg->msix.msix_pba_res = rle->res; + ctrl = pci_read_config(child, cfg->msix.msix_location + PCIR_MSIX_CTRL, + 2); + msgnum = PCI_MSIX_MSGNUM(ctrl); if (bootverbose) device_printf(child, "attempting to allocate %d MSI-X vectors (%d supported)\n", - *count, cfg->msix.msix_msgnum); - max = min(*count, cfg->msix.msix_msgnum); + *count, msgnum); + max = min(*count, msgnum); for (i = 0; i < max; i++) { /* Allocate a message. */ error = PCIB_ALLOC_MSIX(device_get_parent(dev), child, &irq); @@ -1912,7 +1914,7 @@ pci_alloc_msix_method(device_t dev, device_t child, int *count) } /* Mask all vectors. */ - for (i = 0; i < cfg->msix.msix_msgnum; i++) + for (i = 0; i < msgnum; i++) pci_mask_msix(child, i); /* Allocate and initialize vector data and virtual table. */ @@ -1927,9 +1929,10 @@ pci_alloc_msix_method(device_t dev, device_t child, int *count) } /* Update control register to enable MSI-X. */ - cfg->msix.msix_ctrl |= PCIM_MSIXCTRL_MSIX_ENABLE; + ctrl |= PCIM_MSIXCTRL_MSIX_ENABLE; pci_write_config(child, cfg->msix.msix_location + PCIR_MSIX_CTRL, - cfg->msix.msix_ctrl, 2); + ctrl, 2); + cfg->msix.msix_ctrl = ctrl; /* Update counts of alloc'd messages. */ cfg->msix.msix_alloc = actual; @@ -1992,7 +1995,7 @@ pci_remap_msix_method(device_t dev, device_t child, int count, * table can't be bigger than the actual MSI-X table in the * device. */ - if (count < 1 || count > msix->msix_msgnum) + if (count < 1 || count > PCI_MSIX_MSGNUM(msix->msix_ctrl)) return (EINVAL); /* Sanity check the vectors. */ @@ -2158,9 +2161,13 @@ pci_msix_count_method(device_t dev, device_t child) { struct pci_devinfo *dinfo = device_get_ivars(child); struct pcicfg_msix *msix = &dinfo->cfg.msix; + uint16_t ctrl; - if (pci_do_msix && msix->msix_location != 0) - return (msix->msix_msgnum); + if (pci_do_msix && msix->msix_location != 0) { + ctrl = pci_read_config(child, msix->msix_location + + PCIR_MSI_CTRL, 2); + return (PCI_MSIX_MSGNUM(ctrl)); + } return (0); } @@ -2595,7 +2602,7 @@ pci_alloc_msi_method(device_t dev, device_t child, int *count) struct resource_list_entry *rle; u_int actual, i; int error, irqs[32]; - uint16_t ctrl; + uint16_t ctrl, msgnum; /* Don't let count == 0 get us into trouble. */ if (*count < 1) @@ -2618,13 +2625,15 @@ pci_alloc_msi_method(device_t dev, device_t child, int *count) if (cfg->msi.msi_location == 0 || !pci_do_msi) return (ENODEV); + ctrl = pci_read_config(child, cfg->msi.msi_location + PCIR_MSI_CTRL, 2); + msgnum = PCI_MSI_MSGNUM(ctrl); if (bootverbose) device_printf(child, - "attempting to allocate %d MSI vectors (%d supported)\n", - *count, cfg->msi.msi_msgnum); + "attempting to allocate %d MSI vectors (%u supported)\n", + *count, msgnum); /* Don't ask for more than the device supports. */ - actual = min(*count, cfg->msi.msi_msgnum); + actual = min(*count, msgnum); /* Don't ask for more than 32 messages. */ actual = min(actual, 32); @@ -2693,7 +2702,6 @@ pci_alloc_msi_method(device_t dev, device_t child, int *count) } /* Update control register with actual count. */ - ctrl = cfg->msi.msi_ctrl; ctrl &= ~PCIM_MSICTRL_MME_MASK; ctrl |= (ffs(actual) - 1) << 4; cfg->msi.msi_ctrl = ctrl; @@ -2767,9 +2775,13 @@ pci_msi_count_method(device_t dev, device_t child) { struct pci_devinfo *dinfo = device_get_ivars(child); struct pcicfg_msi *msi = &dinfo->cfg.msi; + uint16_t ctrl; - if (pci_do_msi && msi->msi_location != 0) - return (msi->msi_msgnum); + if (pci_do_msi && msi->msi_location != 0) { + ctrl = pci_read_config(child, msi->msi_location + PCIR_MSI_CTRL, + 2); + return (PCI_MSI_MSGNUM(ctrl)); + } return (0); } @@ -3023,19 +3035,21 @@ pci_print_verbose(struct pci_devinfo *dinfo) status & PCIM_PSTAT_DMASK); } if (cfg->msi.msi_location) { - int ctrl; + uint16_t ctrl, msgnum; ctrl = cfg->msi.msi_ctrl; + msgnum = PCI_MSI_MSGNUM(ctrl); printf("\tMSI supports %d message%s%s%s\n", - cfg->msi.msi_msgnum, - (cfg->msi.msi_msgnum == 1) ? "" : "s", + msgnum, (msgnum == 1) ? "" : "s", (ctrl & PCIM_MSICTRL_64BIT) ? ", 64 bit" : "", (ctrl & PCIM_MSICTRL_VECTOR) ? ", vector masks":""); } if (cfg->msix.msix_location) { + uint16_t msgnum; + + msgnum = PCI_MSIX_MSGNUM(cfg->msix.msix_ctrl); printf("\tMSI-X supports %d message%s ", - cfg->msix.msix_msgnum, - (cfg->msix.msix_msgnum == 1) ? "" : "s"); + msgnum, (msgnum == 1) ? "" : "s"); if (cfg->msix.msix_table_bar == cfg->msix.msix_pba_bar) printf("in map 0x%x\n", cfg->msix.msix_table_bar); diff --git a/sys/dev/pci/pcireg.h b/sys/dev/pci/pcireg.h index 623deb8b4505..f6aaf30611e4 100644 --- a/sys/dev/pci/pcireg.h +++ b/sys/dev/pci/pcireg.h @@ -616,6 +616,8 @@ #define PCIM_MSICTRL_MMC_16 0x0008 #define PCIM_MSICTRL_MMC_32 0x000A #define PCIM_MSICTRL_MSI_ENABLE 0x0001 +#define PCI_MSI_MSGNUM(ctrl) \ + (1 << (((ctrl) & PCIM_MSICTRL_MMC_MASK) >> 1)) #define PCIR_MSI_ADDR 0x4 #define PCIR_MSI_ADDR_HIGH 0x8 #define PCIR_MSI_DATA 0x8 @@ -965,6 +967,7 @@ #define PCIM_MSIXCTRL_MSIX_ENABLE 0x8000 #define PCIM_MSIXCTRL_FUNCTION_MASK 0x4000 #define PCIM_MSIXCTRL_TABLE_SIZE 0x07FF +#define PCI_MSIX_MSGNUM(ctrl) (((ctrl) & PCIM_MSIXCTRL_TABLE_SIZE) + 1) #define PCIR_MSIX_TABLE 0x4 #define PCIR_MSIX_PBA 0x8 #define PCIM_MSIX_BIR_MASK 0x7 diff --git a/sys/dev/pci/pcivar.h b/sys/dev/pci/pcivar.h index 37d7daff37f7..888159d59c1a 100644 --- a/sys/dev/pci/pcivar.h +++ b/sys/dev/pci/pcivar.h @@ -90,7 +90,6 @@ struct pcicfg_vpd { struct pcicfg_msi { uint16_t msi_ctrl; /* Message Control */ uint8_t msi_location; /* Offset of MSI capability registers. */ - uint8_t msi_msgnum; /* Number of messages */ int msi_alloc; /* Number of allocated messages. */ uint64_t msi_addr; /* Contents of address register. */ uint16_t msi_data; /* Contents of data register. */ @@ -111,7 +110,6 @@ struct msix_table_entry { struct pcicfg_msix { uint16_t msix_ctrl; /* Message Control */ - uint16_t msix_msgnum; /* Number of messages */ uint8_t msix_location; /* Offset of MSI-X capability registers. */ uint8_t msix_table_bar; /* BAR containing vector table. */ uint8_t msix_pba_bar; /* BAR containing PBA. */