From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750964AbdE3U6L (ORCPT ); Tue, 30 May 2017 16:58:11 -0400 Received: from mail.kernel.org ([198.145.29.99]:42408 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750811AbdE3U6K (ORCPT ); Tue, 30 May 2017 16:58:10 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 63A1623A0C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Tue, 30 May 2017 15:58:08 -0500 From: Bjorn Helgaas To: Ashok Raj Cc: Jean-Phillipe Brucker , Bjorn Helgaas , Joerg Roedel , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, CQ Tang , David Woodhouse , iommu@lists.linux-foundation.org Subject: Re: [PATCH 2/2] PCI: Save properties required to handle FLR for replay purposes. Message-ID: <20170530205808.GA14896@bhelgaas-glaptop.roam.corp.google.com> References: <1496161549-167468-1-git-send-email-ashok.raj@intel.com> <1496161549-167468-3-git-send-email-ashok.raj@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1496161549-167468-3-git-send-email-ashok.raj@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 30, 2017 at 09:25:49AM -0700, Ashok Raj wrote: > From: CQ Tang > > Requires: https://patchwork.kernel.org/patch/9593891 > > > After a FLR, pci-states need to be restored. This patch saves PASID features > and PRI reqs cached. > > To: Bjorn Helgaas > To: Joerg Roedel > To: linux-pci@vger.kernel.org > To: linux-kernel@vger.kernel.org > Cc: Jean-Phillipe Brucker > Cc: David Woodhouse > Cc: iommu@lists.linux-foundation.org > > Signed-off-by: CQ Tang > Signed-off-by: Ashok Raj > --- > drivers/pci/ats.c | 65 +++++++++++++++++++++++++++++++++++++------------ > drivers/pci/pci.c | 3 +++ > include/linux/pci-ats.h | 10 ++++++++ > include/linux/pci.h | 6 +++++ > 4 files changed, 69 insertions(+), 15 deletions(-) > > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c > index 2126497..a769955 100644 > --- a/drivers/pci/ats.c > +++ b/drivers/pci/ats.c > @@ -160,17 +160,16 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs) > if (!pos) > return -EINVAL; > > - pci_read_config_word(pdev, pos + PCI_PRI_CTRL, &control); > pci_read_config_word(pdev, pos + PCI_PRI_STATUS, &status); > - if ((control & PCI_PRI_CTRL_ENABLE) || > - !(status & PCI_PRI_STATUS_STOPPED)) > + if (!(status & PCI_PRI_STATUS_STOPPED)) > return -EBUSY; > > pci_read_config_dword(pdev, pos + PCI_PRI_MAX_REQ, &max_requests); > reqs = min(max_requests, reqs); > + pdev->pri_reqs_alloc = reqs; > pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs); > > - control |= PCI_PRI_CTRL_ENABLE; > + control = PCI_PRI_CTRL_ENABLE; > pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); > > pdev->pri_enabled = 1; > @@ -206,6 +205,29 @@ void pci_disable_pri(struct pci_dev *pdev) > EXPORT_SYMBOL_GPL(pci_disable_pri); > > /** > + * pci_restore_pri_state - Restore PRI > + * @pdev: PCI device structure > + * > + */ > +void pci_restore_pri_state(struct pci_dev *pdev) > +{ > + u16 control = PCI_PRI_CTRL_ENABLE; > + u32 reqs = pdev->pri_reqs_alloc; > + int pos; > + > + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); > + if (!pos) > + return; > + > + if (!pdev->pri_enabled) > + return; I propose swapping the order of these tests, so that if PRI is not enabled, we don't have to search for the capability. Similarly for PASID below. I made these changes and re-indented these functions on my branch. No action required unless you object to these changes. > + > + pci_write_config_dword(pdev, pos + PCI_PRI_ALLOC_REQ, reqs); > + pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); > +} > +EXPORT_SYMBOL_GPL(pci_restore_pri_state); > + > +/** > * pci_reset_pri - Resets device's PRI state > * @pdev: PCI device structure > * > @@ -224,12 +246,7 @@ int pci_reset_pri(struct pci_dev *pdev) > if (!pos) > return -EINVAL; > > - pci_read_config_word(pdev, pos + PCI_PRI_CTRL, &control); > - if (control & PCI_PRI_CTRL_ENABLE) > - return -EBUSY; > - > - control |= PCI_PRI_CTRL_RESET; > - > + control = PCI_PRI_CTRL_RESET; > pci_write_config_word(pdev, pos + PCI_PRI_CTRL, control); > > return 0; > @@ -259,12 +276,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) > if (!pos) > return -EINVAL; > > - pci_read_config_word(pdev, pos + PCI_PASID_CTRL, &control); > pci_read_config_word(pdev, pos + PCI_PASID_CAP, &supported); > - > - if (control & PCI_PASID_CTRL_ENABLE) > - return -EINVAL; > - > supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV; > > /* User wants to enable anything unsupported? */ > @@ -272,6 +284,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) > return -EINVAL; > > control = PCI_PASID_CTRL_ENABLE | features; > + pdev->pasid_features = features; > > pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); > > @@ -305,6 +318,28 @@ void pci_disable_pasid(struct pci_dev *pdev) > EXPORT_SYMBOL_GPL(pci_disable_pasid); > > /** > + * pci_restore_pasid_state - Restore PASID capabilities. > + * @pdev: PCI device structure > + * > + */ > +void pci_restore_pasid_state(struct pci_dev *pdev) > +{ > + u16 control; > + int pos; > + > + pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID); > + if (!pos) > + return; > + > + if (!pdev->pasid_enabled) > + return; > + > + control = PCI_PASID_CTRL_ENABLE | pdev->pasid_features; > + pci_write_config_word(pdev, pos + PCI_PASID_CTRL, control); > +} > +EXPORT_SYMBOL_GPL(pci_restore_pasid_state); > + > +/** > * pci_pasid_features - Check which PASID features are supported > * @pdev: PCI device structure > * > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 7904d02..c9a6510 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -1171,6 +1172,8 @@ void pci_restore_state(struct pci_dev *dev) > > /* PCI Express register must be restored first */ > pci_restore_pcie_state(dev); > + pci_restore_pasid_state(dev); > + pci_restore_pri_state(dev); > pci_restore_ats_state(dev); > pci_restore_vc_state(dev); > > diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h > index 57e0b82..782fb8e 100644 > --- a/include/linux/pci-ats.h > +++ b/include/linux/pci-ats.h > @@ -7,6 +7,7 @@ > > int pci_enable_pri(struct pci_dev *pdev, u32 reqs); > void pci_disable_pri(struct pci_dev *pdev); > +void pci_restore_pri_state(struct pci_dev *pdev); > int pci_reset_pri(struct pci_dev *pdev); > > #else /* CONFIG_PCI_PRI */ > @@ -20,6 +21,10 @@ static inline void pci_disable_pri(struct pci_dev *pdev) > { > } > > +static inline void pci_restore_pri_state(struct pci_dev *pdev) > +{ > +} > + > static inline int pci_reset_pri(struct pci_dev *pdev) > { > return -ENODEV; > @@ -31,6 +36,7 @@ static inline int pci_reset_pri(struct pci_dev *pdev) > > int pci_enable_pasid(struct pci_dev *pdev, int features); > void pci_disable_pasid(struct pci_dev *pdev); > +void pci_restore_pasid_state(struct pci_dev *pdev); > int pci_pasid_features(struct pci_dev *pdev); > int pci_max_pasids(struct pci_dev *pdev); > > @@ -45,6 +51,10 @@ static inline void pci_disable_pasid(struct pci_dev *pdev) > { > } > > +static inline void pci_restore_pasid_state(struct pci_dev *pdev) > +{ > +} > + > static inline int pci_pasid_features(struct pci_dev *pdev) > { > return -EINVAL; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index bee980e..1ddb8e0 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -395,6 +395,12 @@ struct pci_dev { > u8 ats_stu; /* ATS Smallest Translation Unit */ > atomic_t ats_ref_cnt; /* number of VFs with ATS enabled */ > #endif > +#ifdef CONFIG_PCI_PRI > + u32 pri_reqs_alloc; /* Number of PRI requests allocated */ > +#endif > +#ifdef CONFIG_PCI_PASID > + u16 pasid_features; > +#endif > phys_addr_t rom; /* Physical address of ROM if it's not from the BAR */ > size_t romlen; /* Length of ROM if it's not from the BAR */ > char *driver_override; /* Driver name to force a match */ > -- > 2.7.4 >