| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231024155649.GA1655811@bhelgaas>
Date: Tue, 24 Oct 2023 10:56:49 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Krzysztof Wilczyński <kw@...ux.com>,
Stephen Rothwell <sfr@...b.auug.org.au>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] PCI: dwc: Use FIELD_GET/PREP()
On Tue, Oct 24, 2023 at 02:03:36PM +0300, Ilpo Järvinen wrote:
> Convert open-coded variants of PCI field access into FIELD_GET/PREP()
> to make the code easier to understand.
>
> Add two missing defines into pci_regs.h. Logically, the Max No-Snoop
> Latency Register is a separate word sized register in the PCIe spec,
> but the pre-existing LTR defines in pci_regs.h with dword long values
> seem to consider the registers together (the same goes for the only
> user). Thus, follow the custom and make the new values also take both
> word long LTR registers as a joint dword register.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Replaced the previous "PCI: dwc: Use FIELD_GET/PREP()" patch on
pci/field-get, thanks!
Here's the diff from a1a3a781d0ba (the previous head of pci/field-get)
to the current head:
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 12d5ab2f5219..f2a515b639ab 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -126,6 +126,7 @@
#define APPL_LTR_MSG_1 0xC4
#define LTR_MSG_REQ BIT(15)
+#define LTR_NOSNOOP_MSG_REQ BIT(31)
#define APPL_LTR_MSG_2 0xC8
#define APPL_LTR_MSG_2_LTR_MSG_REQ_STATE BIT(3)
@@ -495,8 +496,12 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg)
ktime_t timeout;
/* 110us for both snoop and no-snoop */
- val = 110 | FIELD_PREP(PCI_LTR_SCALE_SHIFT, 2) | LTR_MSG_REQ;
- val |= FIELD_PREP(PCI_LTR_NOSNOOP_VALUE, val);
+ val = FIELD_PREP(PCI_LTR_VALUE_MASK, 110) |
+ FIELD_PREP(PCI_LTR_SCALE_MASK, 2) |
+ LTR_MSG_REQ |
+ FIELD_PREP(PCI_LTR_NOSNOOP_VALUE, 110) |
+ FIELD_PREP(PCI_LTR_NOSNOOP_SCALE, 2) |
+ LTR_NOSNOOP_MSG_REQ;
appl_writel(pcie, val, APPL_LTR_MSG_1);
/* Send LTR upstream */
> ---
>
> This patch REPLACES the previous patch which caused the build fail on
> arm and also failed to copy the entire value into nosnoop register
> like the original code did.
>
> v2:
> - Correct define name used in FIELD_PREP()
> - Correctly copy the contents of the full register, not just the
> Max Latency Value field.
> - Wrap also the 110 literal into FIELD_PREP()
>
> drivers/pci/controller/dwc/pcie-designware-ep.c | 7 ++++---
> drivers/pci/controller/dwc/pcie-tegra194.c | 10 +++++++---
> include/uapi/linux/pci_regs.h | 2 ++
> 3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index f9182f8d552f..20bef1436bfb 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -6,6 +6,7 @@
> * Author: Kishon Vijay Abraham I <kishon@...com>
> */
>
> +#include <linux/bitfield.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
>
> @@ -334,7 +335,7 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> if (!(val & PCI_MSI_FLAGS_ENABLE))
> return -EINVAL;
>
> - val = (val & PCI_MSI_FLAGS_QSIZE) >> 4;
> + val = FIELD_GET(PCI_MSI_FLAGS_QSIZE, val);
>
> return val;
> }
> @@ -357,7 +358,7 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS;
> val = dw_pcie_readw_dbi(pci, reg);
> val &= ~PCI_MSI_FLAGS_QMASK;
> - val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK;
> + val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, interrupts);
> dw_pcie_dbi_ro_wr_en(pci);
> dw_pcie_writew_dbi(pci, reg, val);
> dw_pcie_dbi_ro_wr_dis(pci);
> @@ -584,7 +585,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>
> reg = ep_func->msix_cap + func_offset + PCI_MSIX_TABLE;
> tbl_offset = dw_pcie_readl_dbi(pci, reg);
> - bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
> + bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
> tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>
> msix_tbl = ep->epf_bar[bir]->addr + tbl_offset;
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 248cd9347e8f..f2a515b639ab 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -126,7 +126,7 @@
>
> #define APPL_LTR_MSG_1 0xC4
> #define LTR_MSG_REQ BIT(15)
> -#define LTR_MST_NO_SNOOP_SHIFT 16
> +#define LTR_NOSNOOP_MSG_REQ BIT(31)
>
> #define APPL_LTR_MSG_2 0xC8
> #define APPL_LTR_MSG_2_LTR_MSG_REQ_STATE BIT(3)
> @@ -496,8 +496,12 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg)
> ktime_t timeout;
>
> /* 110us for both snoop and no-snoop */
> - val = 110 | (2 << PCI_LTR_SCALE_SHIFT) | LTR_MSG_REQ;
> - val |= (val << LTR_MST_NO_SNOOP_SHIFT);
> + val = FIELD_PREP(PCI_LTR_VALUE_MASK, 110) |
> + FIELD_PREP(PCI_LTR_SCALE_MASK, 2) |
> + LTR_MSG_REQ |
> + FIELD_PREP(PCI_LTR_NOSNOOP_VALUE, 110) |
> + FIELD_PREP(PCI_LTR_NOSNOOP_SCALE, 2) |
> + LTR_NOSNOOP_MSG_REQ;
> appl_writel(pcie, val, APPL_LTR_MSG_1);
>
> /* Send LTR upstream */
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e5f558d96493..495f0ae4ecd5 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -975,6 +975,8 @@
> #define PCI_LTR_VALUE_MASK 0x000003ff
> #define PCI_LTR_SCALE_MASK 0x00001c00
> #define PCI_LTR_SCALE_SHIFT 10
> +#define PCI_LTR_NOSNOOP_VALUE 0x03ff0000 /* Max No-Snoop Latency Value */
> +#define PCI_LTR_NOSNOOP_SCALE 0x1c000000 /* Scale for Max Value */
> #define PCI_EXT_CAP_LTR_SIZEOF 8
>
> /* Access Control Service */
> --
> 2.30.2
>
Powered by blists - more mailing lists