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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <278a45c0-bd49-b8a2-63ae-80ad851bab9b@arm.com>
Date:   Wed, 11 Jan 2017 18:22:08 +0000
From:   Marc Zyngier <marc.zyngier@....com>
To:     Catalin Marinas <catalin.marinas@....com>,
        Christopher Covington <cov@...eaurora.org>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Christoffer Dall <christoffer.dall@...aro.org>,
        Will Deacon <will.deacon@....com>, kvm@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org, shankerd@...eaurora.org,
        timur@...eaurora.org, Jonathan Corbet <corbet@....net>,
        linux-doc@...r.kernel.org, Jon Masters <jcm@...hat.com>,
        Mark Langsdorf <mlangsdo@...hat.com>,
        Mark Salter <msalter@...hat.com>
Subject: Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

On 11/01/17 18:06, Catalin Marinas wrote:
> Some minor comments below, nothing fundamental (as long as you say the
> new sequence doesn't have the speculative TLB load problem I mentioned
> on a previous version).
> 
> On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 405da11..7151aed 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -42,24 +42,25 @@ file acts as a registry of software workarounds in the Linux Kernel and
>>  will be updated when new workarounds are committed and backported to
>>  stable kernels.
>>  
>> -| Implementor    | Component       | Erratum ID      | Kconfig                 |
>> -+----------------+-----------------+-----------------+-------------------------+
>> -| ARM            | Cortex-A53      | #826319         | ARM64_ERRATUM_826319    |
>> -| ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319    |
>> -| ARM            | Cortex-A53      | #824069         | ARM64_ERRATUM_824069    |
>> -| ARM            | Cortex-A53      | #819472         | ARM64_ERRATUM_819472    |
>> -| ARM            | Cortex-A53      | #845719         | ARM64_ERRATUM_845719    |
>> -| ARM            | Cortex-A53      | #843419         | ARM64_ERRATUM_843419    |
>> -| ARM            | Cortex-A57      | #832075         | ARM64_ERRATUM_832075    |
>> -| ARM            | Cortex-A57      | #852523         | N/A                     |
>> -| ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220    |
>> -| ARM            | Cortex-A72      | #853709         | N/A                     |
>> -| ARM            | MMU-500         | #841119,#826419 | N/A                     |
>> -|                |                 |                 |                         |
>> -| Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375    |
>> -| Cavium         | ThunderX ITS    | #23144          | CAVIUM_ERRATUM_23144    |
>> -| Cavium         | ThunderX GICv3  | #23154          | CAVIUM_ERRATUM_23154    |
>> -| Cavium         | ThunderX Core   | #27456          | CAVIUM_ERRATUM_27456    |
>> -| Cavium         | ThunderX SMMUv2 | #27704          | N/A		       |
>> -|                |                 |                 |                         |
>> -| Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
>> +| Implementor   | Component       | Erratum ID      | Kconfig                  |
>> ++---------------+-----------------+-----------------+--------------------------+
>> +| ARM           | Cortex-A53      | #826319         | ARM64_ERRATUM_826319     |
>> +| ARM           | Cortex-A53      | #827319         | ARM64_ERRATUM_827319     |
>> +| ARM           | Cortex-A53      | #824069         | ARM64_ERRATUM_824069     |
>> +| ARM           | Cortex-A53      | #819472         | ARM64_ERRATUM_819472     |
>> +| ARM           | Cortex-A53      | #845719         | ARM64_ERRATUM_845719     |
>> +| ARM           | Cortex-A53      | #843419         | ARM64_ERRATUM_843419     |
>> +| ARM           | Cortex-A57      | #832075         | ARM64_ERRATUM_832075     |
>> +| ARM           | Cortex-A57      | #852523         | N/A                      |
>> +| ARM           | Cortex-A57      | #834220         | ARM64_ERRATUM_834220     |
>> +| ARM           | Cortex-A72      | #853709         | N/A                      |
>> +| ARM           | MMU-500         | #841119,#826419 | N/A                      |
>> +|               |                 |                 |                          |
>> +| Cavium        | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375     |
>> +| Cavium        | ThunderX ITS    | #23144          | CAVIUM_ERRATUM_23144     |
>> +| Cavium        | ThunderX GICv3  | #23154          | CAVIUM_ERRATUM_23154     |
>> +| Cavium        | ThunderX Core   | #27456          | CAVIUM_ERRATUM_27456     |
>> +| Cavium        | ThunderX SMMUv2 | #27704          | N/A                      |
>> +|               |                 |                 |                          |
>> +| Freescale/NXP | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585      |
>> +| Qualcomm      | Falkor v1       | E1003           | QCOM_FALKOR_ERRATUM_1003 |
> 
> Please don't change the "Implementor" column width, there is no point
> and it makes the patch harder to read (i.e. this hunk should only have
> one line).
> 
>> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
>> index 4c63cb1..5a0a82a 100644
>> --- a/arch/arm64/mm/context.c
>> +++ b/arch/arm64/mm/context.c
>> @@ -87,6 +87,11 @@ static void flush_context(unsigned int cpu)
>>  	/* Update the list of reserved ASIDs and the ASID bitmap. */
>>  	bitmap_clear(asid_map, 0, NUM_USER_ASIDS);
>>  
>> +	/* Reserve ASID for Falkor erratum 1003 */
>> +	if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
>> +	    cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
>> +		__set_bit(FALKOR_RESERVED_ASID, asid_map);
>> +
>>  	/*
>>  	 * Ensure the generation bump is observed before we xchg the
>>  	 * active_asids.
>> @@ -244,6 +249,11 @@ static int asids_init(void)
>>  		panic("Failed to allocate bitmap for %lu ASIDs\n",
>>  		      NUM_USER_ASIDS);
>>  
>> +	/* Reserve ASID for Falkor erratum 1003 */
>> +	if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
>> +	    cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
>> +		__set_bit(FALKOR_RESERVED_ASID, asid_map);
>> +
>>  	pr_info("ASID allocator initialised with %lu entries\n", NUM_USER_ASIDS);
>>  	return 0;
>>  }
> 
> You could as well write a small static function in this file and call it
> twice.
> 
>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 32682be..9ee46df 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -23,6 +23,7 @@
>>  #include <asm/assembler.h>
>>  #include <asm/asm-offsets.h>
>>  #include <asm/hwcap.h>
>> +#include <asm/mmu_context.h>
>>  #include <asm/pgtable.h>
>>  #include <asm/pgtable-hwdef.h>
>>  #include <asm/cpufeature.h>
>> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
>>  ENTRY(cpu_do_switch_mm)
>>  	mmid	x1, x1				// get mm->context.id
>>  	bfi	x0, x1, #48, #16		// set the ASID
>> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
>> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003
>> +	mrs     x2, ttbr0_el1
>> +	mov     x3, #FALKOR_RESERVED_ASID
>> +	bfi     x2, x3, #48, #16                // reserved ASID + old BADDR
>> +	msr     ttbr0_el1, x2
>> +	isb
>> +	bfi     x2, x0, #0, #48                 // reserved ASID + new BADDR
>> +	msr     ttbr0_el1, x2
>> +	isb
>> +alternative_else_nop_endif
>> +#endif
>>  	msr	ttbr0_el1, x0			// set TTBR0
>>  	isb
>>  	post_ttbr0_update_workaround
> 
> Please move the above hunk to a pre_ttbr0_update_workaround macro for
> consistency with post_ttbr0_update_workaround.

In which case (and also for consistency), should we add that pre_ttbr0
macro to entry.S, just before __uaccess_ttbr0_enable? It may not be
needed in the SW pan case, but it is probably worth entertaining the
idea that there may be something to do there...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ