| 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: <CA+-6iNxKavvTthcpWeMTu5FfeqZhvMbALK1WwdRqQhivHJFWTQ@mail.gmail.com>
Date: Thu, 9 Nov 2023 17:06:15 -0500
From: Jim Quinlan <james.quinlan@...adcom.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: linux-pci@...r.kernel.org,
Nicolas Saenz Julienne <nsaenz@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Cyril Brulebois <kibi@...ian.org>,
Phil Elwell <phil@...pberrypi.com>,
bcm-kernel-feedback-list@...adcom.com,
Jim Quinlan <jim2101024@...il.com>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Rob Herring <robh@...nel.org>,
"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE"
<linux-rpi-kernel@...ts.infradead.org>,
"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE"
<linux-arm-kernel@...ts.infradead.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 2/3] PCI: brcmstb: Configure HW CLKREQ# mode
appropriate for downstream device
On Thu, Nov 9, 2023 at 4:27 PM Bjorn Helgaas <helgaas@...nel.org> wrote:
>
> On Thu, Nov 09, 2023 at 02:13:53PM -0500, Jim Quinlan wrote:
> > The Broadcom STB/CM PCIe HW core, which is also used in RPi SOCs, must be
> > deliberately set by the PCIe RC HW into one of three mutually exclusive
> > modes:
> >
> > "safe" -- No CLKREQ# expected or required, refclk is always provided. This
> > mode should work for all devices but is not be capable of any refclk
> > power savings.
> >
> > "no-l1ss" -- CLKREQ# is expected to be driven by the downstream device for
> > CPM and ASPM L0s and L1. Provides Clock Power Management, L0s, and L1,
> > but cannot provide L1 substate (L1SS) power savings. If the downstream
> > device connected to the RC is L1SS capable AND the OS enables L1SS, all
> > PCIe traffic may abruptly halt, potentially hanging the system.
> >
> > "default" -- Bidirectional CLKREQ# between the RC and downstream device.
> > Provides ASPM L0s, L1, and L1SS, but not compliant to provide Clock
> > Power Management; specifically, may not be able to meet the Tclron max
> > timing of 400ns as specified in "Dynamic Clock Control", section
> > 3.2.5.2.2 of the PCIe spec. This situation is atypical and should
> > happen only with older devices.
>
> The PCIe base spec r6.0 has no section 3.2.5.2.2. Looks like this
> could be:
>
> PCIe Mini CEM r2.1, sec 3.2.5.2.2 (December, 2016), or
> PCIe CEM r5.1, sec 2.8.2 (August, 2023)
>
> I don't know the relationship between the "Mini CEM" and the "CEM"
> specs, but CEM r5.1 seems to have the same text as the Mini CEM r2.1
> about Dynamic Clock Control.
>
> We're hampered by the lack of clear subscripts here, but the text in
> both capitalizes the "CRL" part, e.g., "T_CLRon".
>
> > Previously, this driver always set the mode to "no-l1ss", as almost all
> > STB/CM boards operate in this mode. But now there is interest in
> > activating L1SS power savings from STB/CM customers, which requires "aspm"
> > mode. In addition, a bug was filed for RPi4 CM platform because most
> > devices did not work in "no-l1ss" mode.
> >
> > Note that the mode is specified by the DT property "brcm,clkreq-mode". If
> > this property is omitted, then "default" mode is chosen.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217276
> >
> > Signed-off-by: Jim Quinlan <james.quinlan@...adcom.com>
> > ---
> > drivers/pci/controller/pcie-brcmstb.c | 65 ++++++++++++++++++++++-----
> > 1 file changed, 55 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index f9dd6622fe10..f45c5d0168d3 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -121,9 +121,12 @@
> >
> > #define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204
> > #define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
> > +#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_L1SS_ENABLE_MASK 0x200000
> > #define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x08000000
> > #define PCIE_BMIPS_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x00800000
> > -
> > +#define PCIE_CLKREQ_MASK \
> > + (PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK | \
> > + PCIE_MISC_HARD_PCIE_HARD_DEBUG_L1SS_ENABLE_MASK)
> >
> > #define PCIE_INTR2_CPU_BASE 0x4300
> > #define PCIE_MSI_INTR2_BASE 0x4500
> > @@ -1028,13 +1031,61 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> > return 0;
> > }
> >
> > +static void brcm_config_clkreq(struct brcm_pcie *pcie)
> > +{
> > + static const char err_msg[] = "invalid 'brcm,clkreq-mode' DT string\n";
> > + const char *mode = "default";
> > + u32 clkreq_cntl;
> > + int ret;
> > +
> > + ret = of_property_read_string(pcie->np, "brcm,clkreq-mode", &mode);
> > + if (ret && ret != -EINVAL) {
> > + dev_err(pcie->dev, err_msg);
> > + mode = "safe";
> > + }
> > +
> > + /* Start out assuming safe mode (both mode bits cleared) */
> > + clkreq_cntl = readl(pcie->base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> > + clkreq_cntl &= ~PCIE_CLKREQ_MASK;
> > +
> > + if (strcmp(mode, "no-l1ss") == 0) {
> > + /*
> > + * "no-l1ss" -- Provides Clock Power Management, L0s, and
> > + * L1, but cannot provide L1 substate (L1SS) power
> > + * savings. If the downstream device connected to the RC is
> > + * L1SS capable AND the OS enables L1SS, all PCIe traffic
> > + * may abruptly halt, potentially hanging the system.
> > + */
> > + clkreq_cntl |= PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK;
>
> Does this somehow change the features advertised by the Root Port,
> e.g., does it hide the L1 PM Substates Capability completely, or at
> least clear the L1 PM Substates Supported bit?
>
> It it doesn't, the PCI core may enable L1SS and cause this hang.
> Every feature advertised in config space is expected to work.
Agree, I'll put this back in and also reconcile your other comments.
BTW, besides the RPi4, I haven't been able to find a Linux platform
where I can do
echo $POLICY > /sys/module/pcie_aspm/parameters/policy
It seems that the FW/ACPI typically locks this down. I did see a
comment somewhere that
said that the reason it was locked down is because too many devices
cannot handle it.
FWIW.
Regards,
Jim Quinlan
Broadcom STB/CM
>
> > + } else if (strcmp(mode, "default") == 0) {
> > + /*
> > + * "default" -- Provides L0s, L1, and L1SS, but not
> > + * compliant to provide Clock Power Management;
> > + * specifically, may not be able to meet the Tclron max
> > + * timing of 400ns as specified in "Dynamic Clock Control",
> > + * section 3.2.5.2.2 of the PCIe spec. This situation is
> > + * atypical and should happen only with older devices.
> > + */
> > + clkreq_cntl |= PCIE_MISC_HARD_PCIE_HARD_DEBUG_L1SS_ENABLE_MASK;
> > + } else {
> > + /*
> > + * "safe" -- No power savings; refclk is driven by RC
> > + * unconditionally.
> > + */
> > + if (strcmp(mode, "safe") != 0)
> > + dev_err(pcie->dev, err_msg);
> > + mode = "safe";
> > + }
> > + writel(clkreq_cntl, pcie->base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> > + dev_info(pcie->dev, "clkreq-mode set to %s\n", mode);
> > +}
> > +
> > static int brcm_pcie_start_link(struct brcm_pcie *pcie)
> > {
> > struct device *dev = pcie->dev;
> > void __iomem *base = pcie->base;
> > u16 nlw, cls, lnksta;
> > bool ssc_good = false;
> > - u32 tmp;
> > int ret, i;
> >
> > /* Unassert the fundamental reset */
> > @@ -1059,6 +1110,8 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
> > return -ENODEV;
> > }
> >
> > + brcm_config_clkreq(pcie);
> > +
> > if (pcie->gen)
> > brcm_pcie_set_gen(pcie, pcie->gen);
> >
> > @@ -1077,14 +1130,6 @@ static int brcm_pcie_start_link(struct brcm_pcie *pcie)
> > pci_speed_string(pcie_link_speed[cls]), nlw,
> > ssc_good ? "(SSC)" : "(!SSC)");
> >
> > - /*
> > - * Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1
> > - * is enabled => setting the CLKREQ_DEBUG_ENABLE field to 1.
> > - */
> > - tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> > - tmp |= PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK;
> > - writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> > -
> > return 0;
> > }
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4210 bytes)
Powered by blists - more mailing lists