| 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
| ||
|
Date: Tue, 7 Feb 2017 22:05:09 -0600
From: Timur Tabi <timur@...eaurora.org>
To: Christopher Covington <cov@...eaurora.org>,
Jonathan Corbet <corbet@....net>,
Marc Zyngier <marc.zyngier@....com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>, linux-doc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Mark Rutland <mark.rutland@....com>,
linux-kernel@...r.kernel.org, shankerd@...eaurora.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>,
Russell King <linux@...linux.org.uk>,
linux-serial@...r.kernel.org
Cc: Mark Langsdorf <mlangsdo@...hat.com>,
Mark Salter <msalter@...hat.com>, Jon Masters <jcm@...hat.com>,
Neil Leeder <nleeder@...eaurora.org>
Subject: Re: [PATCH 1/2] tty: pl011: Work around QDF2400 E44 stuck BUSY bit
Christopher Covington wrote:
> The Qualcomm Datacenter Technologies QDF2400 family of SoCs contains a
> custom (non-PrimeCell) implementation of the SBSA UART. Occasionally the
> BUSY bit in the Flag Register gets stuck as 1, erratum 44 for both 2432v1
> and 2400v1 SoCs. Checking that the Transmit FIFO Empty (TXFE) bit is 0,
> instead of checking that the BUSY bit is 1, works around the issue. To
> facilitate this substitution when UART AMBA Port (UAP) data is available,
> introduce vendor-specific inversion of Feature Register bits. To keep the
> change small, this patch only works around the full console case, where UAP
> data is available, and does not handle the erratum for earlycon, as the UAP
> data is not available then.
>
> Signed-off-by: Christopher Covington <cov@...eaurora.org>
> Acked-by: Russell King <rmk+kernel@...linux.org.uk>
> ---
> Changes between the previous RFC [1] and this PATCH:
> * don't use arch/arm64/kernel/cpu_errata.c at Will's request
> * separate out earlycon case to separate patch
> * rework earlycon case to not depend on UAP as suggested by Timur
>
> Because they need a newly-defined MIDR values, the patches are currently
> based on:
> https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
>
> I'm not confident that I know the best route for these two patches. Should
> I request Catalin and Will to take them via arm64 as the essential MIDR
> additions are their purview?
>
> Thanks Russell for the ack. Compared to the RFC, I've made minor changes to
> what is now patch 1/2 and am making an educated guess that the ack sticks
> (but if not please let me know). Patch 2/2 is significantly revised from
> the RFC so I've not included the ack on it.
>
> 1. https://patchwork.codeaurora.org/patch/163281/
> ---
> Documentation/arm64/silicon-errata.txt | 2 ++
> arch/arm64/include/asm/cputype.h | 2 ++
> drivers/tty/serial/Kconfig | 12 ++++++++++
> drivers/tty/serial/amba-pl011.c | 40 +++++++++++++++++++++++++++++++---
> 4 files changed, 53 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 50da8391e9dd..0993ebb3e86b 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -65,3 +65,5 @@ stable kernels.
> | Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
> | Qualcomm Tech. | Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003|
> | Qualcomm Tech. | Falkor v1 | E1009 | QCOM_FALKOR_ERRATUM_1009|
> +| Qualcomm Tech. | QDF2432v1 UART | SoC E44 | QCOM_QDF2400_ERRATUM_44 |
> +| Qualcomm Tech. | QDF2400v1 UART | SoC E44 | QCOM_QDF2400_ERRATUM_44 |
> diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
> index fc502713ab37..cb399c7fe6ec 100644
> --- a/arch/arm64/include/asm/cputype.h
> +++ b/arch/arm64/include/asm/cputype.h
> @@ -88,12 +88,14 @@
>
> #define BRCM_CPU_PART_VULCAN 0x516
>
> +#define QCOM_CPU_PART_KRYO_V1 0x281
> #define QCOM_CPU_PART_FALKOR_V1 0x800
>
> #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
> #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
> #define MIDR_THUNDERX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
> #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
> +#define MIDR_QCOM_KRYO_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_V1)
> #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1)
>
> #ifndef __ASSEMBLY__
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index e9cf5b67f1b7..4cde8f48a540 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -73,6 +73,18 @@ config SERIAL_AMBA_PL011_CONSOLE
> your boot loader (lilo or loadlin) about how to pass options to the
> kernel at boot time.)
>
> +config QCOM_QDF2400_ERRATUM_44
> + bool "Work around QDF2400 SoC E44 stuck BUSY bit"
> + depends on SERIAL_AMBA_PL011_CONSOLE=y
> + default y
> + help
> + The BUSY bit in the Flag Register of the UART on the QDF2432v1 and
> + QDF2400v1 SoCs may get stuck as 1, resulting in a hung serial console.
> + Say Y here to work around the issue by checking TXFE == 0 instead of
> + BUSY == 1 on affected systems.
> +
> + If unsure, say Y.
> +
> config SERIAL_EARLYCON_ARM_SEMIHOST
> bool "Early console using ARM semihosting"
> depends on ARM64 || ARM
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index d4171d71a258..41e51901d6ef 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -97,6 +97,7 @@ struct vendor_data {
> unsigned int fr_dsr;
> unsigned int fr_cts;
> unsigned int fr_ri;
> + unsigned int inv_fr;
> bool access_32b;
> bool oversampling;
> bool dma_threshold;
> @@ -141,6 +142,25 @@ static struct vendor_data vendor_sbsa = {
> .fixed_options = true,
> };
>
> +#ifdef CONFIG_QCOM_QDF2400_ERRATUM_44
> +static struct vendor_data vendor_qdt_qdf2400_e44 = {
> + .reg_offset = pl011_std_offsets,
> + .fr_busy = UART011_FR_TXFE,
> + .fr_dsr = UART01x_FR_DSR,
> + .fr_cts = UART01x_FR_CTS,
> + .fr_ri = UART011_FR_RI,
> + .inv_fr = UART011_FR_TXFE,
> + .access_32b = true,
> + .oversampling = false,
> + .dma_threshold = false,
> + .cts_event_workaround = false,
> + .always_enabled = true,
> + .fixed_options = true,
> +};
> +#else
> +#define vendor_qdt_qdf2400_e44 vendor_sbsa
> +#endif
Instead of the #else, just put the #ifdef inside qdf2400_e44(). That way, the
function always returns False if CONFIG_QCOM_QDF2400_ERRATUM_44 is not defined.
Also, don't you need to add a definition of .inv_fr in vendor_sbsa and the other
vendor_xxx structures?
> +
> static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
> [REG_DR] = UART01x_DR,
> [REG_ST_DMAWM] = ST_UART011_DMAWM,
> @@ -1518,7 +1538,7 @@ static unsigned int pl011_tx_empty(struct uart_port *port)
> {
> struct uart_amba_port *uap =
> container_of(port, struct uart_amba_port, port);
> - unsigned int status = pl011_read(uap, REG_FR);
> + unsigned int status = pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr;
> return status & (uap->vendor->fr_busy | UART01x_FR_TXFF) ?
> 0 : TIOCSER_TEMT;
> }
> @@ -2218,7 +2238,8 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
> * Finally, wait for transmitter to become empty
> * and restore the TCR
> */
> - while (pl011_read(uap, REG_FR) & uap->vendor->fr_busy)
> + while ((pl011_read(uap, REG_FR) ^ uap->vendor->inv_fr)
> + & uap->vendor->fr_busy)
I really think the XOR logic needs to be documented wherever it's used. It's
just too confusing.
> cpu_relax();
> if (!uap->vendor->always_enabled)
> pl011_write(old_cr, uap, REG_CR);
> @@ -2383,6 +2404,13 @@ static struct console amba_console = {
>
> #define AMBA_CONSOLE (&amba_console)
>
> +static bool qdf2400_e44(void) {
Call it needs_qdf2400_e44(void) ?
> + u32 cpu_var_model = read_cpuid_id() & ~MIDR_REVISION_MASK;
> +
> + return (cpu_var_model == MIDR_QCOM_KRYO_V1 ||
> + cpu_var_model == MIDR_QCOM_FALKOR_V1);
> +}
> +
> static void pl011_putc(struct uart_port *port, int c)
> {
> while (readl(port->membase + UART01x_FR) & UART01x_FR_TXFF)
> @@ -2645,12 +2673,18 @@ static int sbsa_uart_probe(struct platform_device *pdev)
> uap->port.irq = ret;
>
> uap->reg_offset = vendor_sbsa.reg_offset;
> - uap->vendor = &vendor_sbsa;
> uap->fifosize = 32;
> uap->port.iotype = vendor_sbsa.access_32b ? UPIO_MEM32 : UPIO_MEM;
> uap->port.ops = &sbsa_uart_pops;
> uap->fixed_baud = baudrate;
>
> + if (qdf2400_e44()) {
> + uap->vendor = &vendor_qdt_qdf2400_e44;
> + dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
> + } else {
> + uap->vendor = &vendor_sbsa;
> + }
> +
> snprintf(uap->type, sizeof(uap->type), "SBSA");
>
> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
Powered by blists - more mailing lists