From owner-dev-commits-src-all@freebsd.org Thu Apr 8 17:36:19 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C07D85B9E18; Thu, 8 Apr 2021 17:36:19 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (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 "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FGT2W57P0z3pY6; Thu, 8 Apr 2021 17:36:19 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from John-Baldwins-MacBook-Pro.local (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 2B5F6567A; Thu, 8 Apr 2021 17:36:19 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Subject: Re: git: 5a898b2b78ce - main - Set PCIe device's Max_Payload_Size to match PCIe root's. To: Alexander Motin , src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202104051440.135EeeTZ095177@gitrepo.freebsd.org> From: John Baldwin Message-ID: <4091ea80-5269-9c78-9fe6-6bba2ed85fbd@FreeBSD.org> Date: Thu, 8 Apr 2021 10:36:12 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <202104051440.135EeeTZ095177@gitrepo.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Apr 2021 17:36:19 -0000 On 4/5/21 7:40 AM, Alexander Motin wrote: > The branch main has been updated by mav: > > URL: https://cgit.FreeBSD.org/src/commit/?id=5a898b2b78ce04d608bbaaa0813424b11f921ae7 > > commit 5a898b2b78ce04d608bbaaa0813424b11f921ae7 > Author: Alexander Motin > AuthorDate: 2021-04-05 14:34:40 +0000 > Commit: Alexander Motin > CommitDate: 2021-04-05 14:34:40 +0000 > > Set PCIe device's Max_Payload_Size to match PCIe root's. > > Usually on boot the MPS is already configured by BIOS. But we've > found that on hot-plug it is not true at least for our Supermicro > X11 boards. As result, mismatch between root's configuration of > 256 bytes and device's default of 128 bytes cause problems for some > devices, while others seem to work fine. > > MFC after: 1 month > Sponsored by: iXsystems, Inc. > --- > sys/dev/pci/pci.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c > index ab19d13fc13a..ef138e926b6f 100644 > --- a/sys/dev/pci/pci.c > +++ b/sys/dev/pci/pci.c > @@ -4267,6 +4267,45 @@ pci_create_iov_child_method(device_t bus, device_t pf, uint16_t rid, > } > #endif > > +/* > + * For PCIe device set Max_Payload_Size to match PCIe root's. > + */ > +static void > +pcie_setup_mps(device_t dev) > +{ > + struct pci_devinfo *dinfo = device_get_ivars(dev); > + device_t root; > + uint16_t rmps, mmps, mps; > + > + if (dinfo->cfg.pcie.pcie_location == 0) > + return; > + root = pci_find_pcie_root_port(dev); > + if (root == NULL) > + return; > + /* Check whether the MPS is already configured. */ We tend to add blank lines before comments. > + rmps = pcie_read_config(root, PCIER_DEVICE_CTL, 2) & > + PCIEM_CTL_MAX_PAYLOAD; > + mps = pcie_read_config(dev, PCIER_DEVICE_CTL, 2) & > + PCIEM_CTL_MAX_PAYLOAD; > + if (mps == rmps) > + return; > + /* Check whether the device is capable of the root's MPS. */ > + mmps = (pcie_read_config(dev, PCIER_DEVICE_CAP, 2) & > + PCIEM_CAP_MAX_PAYLOAD) << 5; > + if (rmps > mmps) { > + /* > + * The device is unable to handle root's MPS. Limit root. > + * XXX: We should traverse through all the tree, applying > + * it to all the devices. > + */ Hmmm, limiting the root seems very dubious here. Do you really need this? If not, I'd put it behind a tunable sysctl that defaults to off. Ideally what you'd do here is use an and of the two masks and select one of those bits to choose the value rather than assuming the root can do the device's value. -- John Baldwin