From: Bjorn Helgaas <helgaas@kernel.org>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
Jesse Brandeburg <jesse.brandeburg@intel.com>,
intel-wired-lan@lists.osuosl.org,
Tony Nguyen <anthony.l.nguyen@intel.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
Oliver O'Halloran <oohall@gmail.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Ard Biesheuvel <ardb@kernel.org>,
Borislav Petkov <bp@alien8.de>,
linux-edac@vger.kernel.org, linux-efi@vger.kernel.org,
Tony Luck <tony.luck@intel.com>
Subject: Re: [PATCH 3/4] PCI: Add TLP Prefix reading into pcie_read_tlp_log()
Date: Fri, 22 Mar 2024 14:30:11 -0500 [thread overview]
Message-ID: <20240322193011.GA701027@bhelgaas> (raw)
In-Reply-To: <20240206135717.8565-4-ilpo.jarvinen@linux.intel.com>
On Tue, Feb 06, 2024 at 03:57:16PM +0200, Ilpo Järvinen wrote:
> pcie_read_tlp_log() handles only 4 TLP Header Log DWORDs but TLP Prefix
> Log (PCIe r6.1 secs 7.8.4.12 & 7.9.14.13) may also be present.
s/TLP Header Log/Header Log/ to match spec terminology (also below)
> Generalize pcie_read_tlp_log() and struct pcie_tlp_log to handle also
> TLP Prefix Log. The layout of relevant registers in AER and DPC
> Capability is not identical but the offsets of TLP Header Log and TLP
> Prefix Log vary so the callers must pass the offsets to
> pcie_read_tlp_log().
s/is not identical but/is identical, but/ ?
The spec is a little obtuse about Header Log Size.
> Convert eetlp_prefix_path into integer called eetlp_prefix_max and
> make is available also when CONFIG_PCI_PASID is not configured to
> be able to determine the number of E-E Prefixes.
I think this eetlp_prefix_path piece is right, but would be nice in a
separate patch since it's a little bit different piece to review.
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -11336,7 +11336,9 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
> if (!pos)
> goto skip_bad_vf_detection;
>
> - ret = pcie_read_tlp_log(pdev, pos + PCI_ERR_HEADER_LOG, &tlp_log);
> + ret = pcie_read_tlp_log(pdev, pos + PCI_ERR_HEADER_LOG,
> + pos + PCI_ERR_PREFIX_LOG,
> + aer_tlp_log_len(pdev), &tlp_log);
> if (ret < 0) {
> ixgbe_check_cfg_remove(hw, pdev);
> goto skip_bad_vf_detection;
We applied the patch to export pcie_read_tlp_log(), but I'm having
second thoughts about it. I don't think drivers really have any
business here, and I'd rather not expose either pcie_read_tlp_log() or
aer_tlp_log_len().
This part of ixgbe_io_error_detected() was added by 83c61fa97a7d
("ixgbe: Add protection from VF invalid target DMA"), and to me it
looks like debug code that probably doesn't need to be there as long
as the PCI core does the appropriate logging.
Bjorn
next prev parent reply other threads:[~2024-03-22 19:30 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-06 13:57 [PATCH 0/4] PCI: Consolidate TLP Log reading and printing Ilpo Järvinen
2024-02-06 13:57 ` [PATCH 1/4] PCI/AER: Cleanup register variable Ilpo Järvinen
2024-02-06 13:57 ` [PATCH 2/4] PCI: Generalize TLP Header Log reading Ilpo Järvinen
2024-03-14 17:16 ` Bjorn Helgaas
2024-02-06 13:57 ` [PATCH 3/4] PCI: Add TLP Prefix reading into pcie_read_tlp_log() Ilpo Järvinen
2024-03-22 19:30 ` Bjorn Helgaas [this message]
2024-02-06 13:57 ` [PATCH 4/4] PCI: Create helper to print TLP Header and Prefix Log Ilpo Järvinen
2024-02-07 11:50 ` [PATCH 0/4] PCI: Consolidate TLP Log reading and printing Ilpo Järvinen
2024-03-08 21:31 ` Bjorn Helgaas
2024-03-11 11:34 ` Ilpo Järvinen
2024-03-22 14:16 ` Ilpo Järvinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240322193011.GA701027@bhelgaas \
--to=helgaas@kernel.org \
--cc=anthony.l.nguyen@intel.com \
--cc=ardb@kernel.org \
--cc=bhelgaas@google.com \
--cc=bp@alien8.de \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jesse.brandeburg@intel.com \
--cc=kuba@kernel.org \
--cc=linux-edac@vger.kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mahesh@linux.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=oohall@gmail.com \
--cc=pabeni@redhat.com \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).