| 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: <86y0ybsd0d.wl-maz@kernel.org>
Date: Wed, 12 Feb 2025 09:55:46 +0000
From: Marc Zyngier <maz@...nel.org>
To: Alyssa Rosenzweig <alyssa@...enzweig.io>
Cc: Hector Martin <marcan@...can.st>, Sven Peter <sven@...npeter.dev>, Bjorn
Helgaas <bhelgaas@...gle.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>, Manivannan
Sadhasivam <manivannan.sadhasivam@...aro.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Mark Kettenis <kettenis@...nbsd.org>, Stan Skowronek
<stan@...ellium.com>, asahi@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-pci@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/7] PCI: apple: Add T602x PCIe support
On Tue, 11 Feb 2025 19:54:32 +0000,
Alyssa Rosenzweig <alyssa@...enzweig.io> wrote:
>
> From: Hector Martin <marcan@...can.st>
>
> This version of the hardware moved around a bunch of registers, so we
> drop the old compatible for these and introduce register offset
> structures to handle the differences.
>
> Signed-off-by: Hector Martin <marcan@...can.st>
> Signed-off-by: Alyssa Rosenzweig <alyssa@...enzweig.io>
> ---
> drivers/pci/controller/pcie-apple.c | 125 ++++++++++++++++++++++++++++++------
> 1 file changed, 105 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> index 7f4839fb0a5b15a9ca87337f53c14a1ce08301fc..7c598334427cb56ca066890ac61143ae1d3ed744 100644
> --- a/drivers/pci/controller/pcie-apple.c
> +++ b/drivers/pci/controller/pcie-apple.c
> @@ -26,6 +26,7 @@
> #include <linux/list.h>
> #include <linux/module.h>
> #include <linux/msi.h>
> +#include <linux/of_device.h>
> #include <linux/of_irq.h>
> #include <linux/pci-ecam.h>
>
> @@ -104,7 +105,7 @@
> #define PORT_REFCLK_CGDIS BIT(8)
> #define PORT_PERST 0x00814
> #define PORT_PERST_OFF BIT(0)
> -#define PORT_RID2SID(i16) (0x00828 + 4 * (i16))
> +#define PORT_RID2SID 0x00828
> #define PORT_RID2SID_VALID BIT(31)
> #define PORT_RID2SID_SID_SHIFT 16
> #define PORT_RID2SID_BUS_SHIFT 8
> @@ -122,7 +123,7 @@
> #define PORT_TUNSTAT_PERST_ACK_PEND BIT(1)
> #define PORT_PREFMEM_ENABLE 0x00994
>
> -#define MAX_RID2SID 64
> +#define MAX_RID2SID 512
>
> /*
> * The doorbell address is set to 0xfffff000, which by convention
> @@ -133,6 +134,57 @@
> */
> #define DOORBELL_ADDR CONFIG_PCIE_APPLE_MSI_DOORBELL_ADDR
>
> +struct reg_info {
> + u32 phy_lane_ctl;
> + u32 port_msiaddr;
> + u32 port_msiaddr_hi;
> + u32 port_refclk;
> + u32 port_perst;
> + u32 port_rid2sid;
> + u32 port_msimap;
> + u32 max_rid2sid;
> + u32 max_msimap;
> +};
> +
> +const struct reg_info t8103_hw = {
> + .phy_lane_ctl = PHY_LANE_CTL,
> + .port_msiaddr = PORT_MSIADDR,
> + .port_msiaddr_hi = 0,
> + .port_refclk = PORT_REFCLK,
> + .port_perst = PORT_PERST,
> + .port_rid2sid = PORT_RID2SID,
> + .port_msimap = 0,
> + .max_rid2sid = 64,
> + .max_msimap = 0,
> +};
> +
> +#define PORT_T602X_MSIADDR 0x016c
> +#define PORT_T602X_MSIADDR_HI 0x0170
> +#define PORT_T602X_PERST 0x082c
> +#define PORT_T602X_RID2SID 0x3000
> +#define PORT_T602X_MSIMAP 0x3800
> +
> +#define PORT_MSIMAP_ENABLE BIT(31)
> +#define PORT_MSIMAP_TARGET GENMASK(7, 0)
> +
> +const struct reg_info t602x_hw = {
> + .phy_lane_ctl = 0,
> + .port_msiaddr = PORT_T602X_MSIADDR,
> + .port_msiaddr_hi = PORT_T602X_MSIADDR_HI,
> + .port_refclk = 0,
> + .port_perst = PORT_T602X_PERST,
> + .port_rid2sid = PORT_T602X_RID2SID,
> + .port_msimap = PORT_T602X_MSIMAP,
> + .max_rid2sid = 512, /* 16 on t602x, guess for autodetect on future HW */
> + .max_msimap = 512, /* 96 on t602x, guess for autodetect on future HW */
What is max_msimap for? It is never used.
> +};
> +
> +static const struct of_device_id apple_pcie_of_match_hw[] = {
> + { .compatible = "apple,t6020-pcie", .data = &t602x_hw },
> + { .compatible = "apple,pcie", .data = &t8103_hw },
> + { }
> +};
> +
> struct apple_pcie {
> struct mutex lock;
> struct device *dev;
> @@ -143,6 +195,7 @@ struct apple_pcie {
> struct completion event;
> struct irq_fwspec fwspec;
> u32 nvecs;
> + const struct reg_info *hw;
> };
>
> struct apple_pcie_port {
> @@ -266,14 +319,14 @@ static void apple_port_irq_mask(struct irq_data *data)
> {
> struct apple_pcie_port *port = irq_data_get_irq_chip_data(data);
>
> - writel_relaxed(BIT(data->hwirq), port->base + PORT_INTMSKSET);
> + rmw_set(BIT(data->hwirq), port->base + PORT_INTMSK);
> }
>
> static void apple_port_irq_unmask(struct irq_data *data)
> {
> struct apple_pcie_port *port = irq_data_get_irq_chip_data(data);
>
> - writel_relaxed(BIT(data->hwirq), port->base + PORT_INTMSKCLR);
> + rmw_clear(BIT(data->hwirq), port->base + PORT_INTMSK);
> }
>
> static bool hwirq_is_intx(unsigned int hwirq)
> @@ -377,6 +430,7 @@ static void apple_port_irq_handler(struct irq_desc *desc)
> static int apple_pcie_port_setup_irq(struct apple_pcie_port *port)
> {
> struct fwnode_handle *fwnode = &port->np->fwnode;
> + struct apple_pcie *pcie = port->pcie;
> unsigned int irq;
>
> /* FIXME: consider moving each interrupt under each port */
> @@ -392,19 +446,35 @@ static int apple_pcie_port_setup_irq(struct apple_pcie_port *port)
> return -ENOMEM;
>
> /* Disable all interrupts */
> - writel_relaxed(~0, port->base + PORT_INTMSKSET);
> + writel_relaxed(~0, port->base + PORT_INTMSK);
> writel_relaxed(~0, port->base + PORT_INTSTAT);
> + writel_relaxed(~0, port->base + PORT_LINKCMDSTS);
Can you elaborate on this change?
>
> irq_set_chained_handler_and_data(irq, apple_port_irq_handler, port);
>
> /* Configure MSI base address */
> BUILD_BUG_ON(upper_32_bits(DOORBELL_ADDR));
> - writel_relaxed(lower_32_bits(DOORBELL_ADDR), port->base + PORT_MSIADDR);
> + writel_relaxed(lower_32_bits(DOORBELL_ADDR),
> + port->base + pcie->hw->port_msiaddr);
> + if (pcie->hw->port_msiaddr_hi)
> + writel_relaxed(0, port->base + pcie->hw->port_msiaddr_hi);
>
> /* Enable MSIs, shared between all ports */
> - writel_relaxed(0, port->base + PORT_MSIBASE);
> - writel_relaxed((ilog2(port->pcie->nvecs) << PORT_MSICFG_L2MSINUM_SHIFT) |
> - PORT_MSICFG_EN, port->base + PORT_MSICFG);
> + if (pcie->hw->port_msimap) {
> + int i;
> +
> + for (i = 0; i < pcie->nvecs; i++) {
> + writel_relaxed(FIELD_PREP(PORT_MSIMAP_TARGET, i) |
> + PORT_MSIMAP_ENABLE,
> + port->base + pcie->hw->port_msimap + 4 * i);
> + }
> +
> + writel_relaxed(PORT_MSICFG_EN, port->base + PORT_MSICFG);
> + } else {
> + writel_relaxed(0, port->base + PORT_MSIBASE);
> + writel_relaxed((ilog2(pcie->nvecs) << PORT_MSICFG_L2MSINUM_SHIFT) |
> + PORT_MSICFG_EN, port->base + PORT_MSICFG);
> + }
>
> return 0;
> }
> @@ -472,7 +542,9 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie,
> u32 stat;
> int res;
>
> - rmw_set(PHY_LANE_CTL_CFGACC, port->phy + PHY_LANE_CTL);
> + if (pcie->hw->phy_lane_ctl)
> + rmw_set(PHY_LANE_CTL_CFGACC, port->phy + pcie->hw->phy_lane_ctl);
> +
> rmw_set(PHY_LANE_CFG_REFCLK0REQ, port->phy + PHY_LANE_CFG);
>
> res = readl_relaxed_poll_timeout(port->phy + PHY_LANE_CFG,
> @@ -489,10 +561,13 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie,
> if (res < 0)
> return res;
>
> - rmw_clear(PHY_LANE_CTL_CFGACC, port->phy + PHY_LANE_CTL);
> + if (pcie->hw->phy_lane_ctl)
> + rmw_clear(PHY_LANE_CTL_CFGACC, port->phy + pcie->hw->phy_lane_ctl);
>
> rmw_set(PHY_LANE_CFG_REFCLKEN, port->phy + PHY_LANE_CFG);
> - rmw_set(PORT_REFCLK_EN, port->base + PORT_REFCLK);
> +
> + if (pcie->hw->port_refclk)
> + rmw_set(PORT_REFCLK_EN, port->base + pcie->hw->port_refclk);
>
> return 0;
> }
> @@ -500,9 +575,9 @@ static int apple_pcie_setup_refclk(struct apple_pcie *pcie,
> static u32 apple_pcie_rid2sid_write(struct apple_pcie_port *port,
> int idx, u32 val)
> {
> - writel_relaxed(val, port->base + PORT_RID2SID(idx));
> + writel_relaxed(val, port->base + port->pcie->hw->port_rid2sid + 4 * idx);
> /* Read back to ensure completion of the write */
> - return readl_relaxed(port->base + PORT_RID2SID(idx));
> + return readl_relaxed(port->base + port->pcie->hw->port_rid2sid + 4 * idx);
> }
>
> static int apple_pcie_setup_port(struct apple_pcie *pcie,
> @@ -563,7 +638,7 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
> usleep_range(100, 200);
>
> /* Deassert PERST# */
> - rmw_set(PORT_PERST_OFF, port->base + PORT_PERST);
> + rmw_set(PORT_PERST_OFF, port->base + pcie->hw->port_perst);
> gpiod_set_value_cansleep(reset, 0);
>
> /* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */
> @@ -576,15 +651,12 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
> return ret;
> }
>
> - rmw_clear(PORT_REFCLK_CGDIS, port->base + PORT_REFCLK);
> - rmw_clear(PORT_APPCLK_CGDIS, port->base + PORT_APPCLK);
> -
> ret = apple_pcie_port_setup_irq(port);
> if (ret)
> return ret;
>
> /* Reset all RID/SID mappings, and check for RAZ/WI registers */
> - for (i = 0; i < MAX_RID2SID; i++) {
> + for (i = 0; i < pcie->hw->max_rid2sid; i++) {
> if (apple_pcie_rid2sid_write(port, i, 0xbad1d) != 0xbad1d)
> break;
> apple_pcie_rid2sid_write(port, i, 0);
> @@ -608,6 +680,12 @@ static int apple_pcie_setup_port(struct apple_pcie *pcie,
> if (!wait_for_completion_timeout(&pcie->event, HZ / 10))
> dev_warn(pcie->dev, "%pOF link didn't come up\n", np);
>
> + if (pcie->hw->port_refclk)
> + rmw_clear(PORT_REFCLK_CGDIS, port->base + PORT_REFCLK);
Shouldn't this be using the actual value for port_refclk?
> + else
> + rmw_set(PHY_LANE_CFG_REFCLKCGEN, port->phy + PHY_LANE_CFG);
> + rmw_clear(PORT_APPCLK_CGDIS, port->base + PORT_APPCLK);
> +
Can you elaborate on this particular change?
I always assumed this was some clock-gating that needed to occur
*before* the link training was started. This is now taking place after
training, and the commit message doesn't say anything about it.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists