Re: [PATCH v15 1/9] PCI: Cache PCIe FLR capability

From: Raphael Norwitz <raphael.norwitz@nutanix.com>
To: Amey Narkhede <ameynarkhede03@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	Raphael Norwitz <raphael.norwitz@nutanix.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kw@linux.com" <kw@linux.com>,
	Shanker Donthineni <sdonthineni@nvidia.com>,
	Sinan Kaya <okaya@kernel.org>, Len Brown <lenb@kernel.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH v15 1/9] PCI: Cache PCIe FLR capability
Date: Mon, 9 Aug 2021 11:13:54 +0000	[thread overview]
Message-ID: <20210809111349.GA867@raphael-debian-dev> (raw)
In-Reply-To: <20210805162917.3989-2-ameynarkhede03@gmail.com>

On Thu, Aug 05, 2021 at 09:59:09PM +0530, Amey Narkhede wrote:
> Add a new member called devcap in struct pci_dev for caching the device
> capabilities to avoid reading PCI_EXP_DEVCAP multiple times.
> 
> Refactor pcie_has_flr() to use cached device capabilities.
> 
> Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

> ---
>  drivers/pci/pci.c   | 6 ++----
>  drivers/pci/probe.c | 5 +++--
>  include/linux/pci.h | 1 +
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 452351025a09..1fafd05caa41 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -31,6 +31,7 @@
>  #include <linux/vmalloc.h>
>  #include <asm/dma.h>
>  #include <linux/aer.h>
> +#include <linux/bitfield.h>
>  #include "pci.h"
>  
>  DEFINE_MUTEX(pci_slot_mutex);
> @@ -4620,13 +4621,10 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
>   */
>  bool pcie_has_flr(struct pci_dev *dev)
>  {
> -	u32 cap;
> -
>  	if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
>  		return false;
>  
> -	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
> -	return cap & PCI_EXP_DEVCAP_FLR;
> +	return FIELD_GET(PCI_EXP_DEVCAP_FLR, dev->devcap) == 1;
>  }
>  EXPORT_SYMBOL_GPL(pcie_has_flr);
>  
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 3a62d09b8869..df3f9db6e151 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -19,6 +19,7 @@
>  #include <linux/hypervisor.h>
>  #include <linux/irqdomain.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/bitfield.h>
>  #include "pci.h"
>  
>  #define CARDBUS_LATENCY_TIMER	176	/* secondary latency timer */
> @@ -1497,8 +1498,8 @@ void set_pcie_port_type(struct pci_dev *pdev)
>  	pdev->pcie_cap = pos;
>  	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &reg16);
>  	pdev->pcie_flags_reg = reg16;
> -	pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
> -	pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
> +	pci_read_config_dword(pdev, pos + PCI_EXP_DEVCAP, &pdev->devcap);
> +	pdev->pcie_mpss = FIELD_GET(PCI_EXP_DEVCAP_PAYLOAD, pdev->devcap);
>  
>  	parent = pci_upstream_bridge(pdev);
>  	if (!parent)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c20211e59a57..697b1f085c7b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -333,6 +333,7 @@ struct pci_dev {
>  	struct rcec_ea	*rcec_ea;	/* RCEC cached endpoint association */
>  	struct pci_dev  *rcec;          /* Associated RCEC device */
>  #endif
> +	u32		devcap;		/* PCIe device capabilities */
>  	u8		pcie_cap;	/* PCIe capability offset */
>  	u8		msi_cap;	/* MSI capability offset */
>  	u8		msix_cap;	/* MSI-X capability offset */
> -- 
> 2.32.0
> 
>