Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-24 Thread Christopher Covington
Hi Catalin,

On 01/11/2017 01:06 PM, 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).

This workaround is documented as providing functional correctness for
both explicit and speculative memory accesses.

>> 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 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
>>  ENTRY(cpu_do_switch_mm)
>>  mmidx1, 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.

How should I deal with inputs to the macro?

A) Use no input parameters and hard code x0 usage
B) Use a macro input parameter for the new TTBR value (x0)
C) Something else

Thanks,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-24 Thread Christopher Covington
On 01/12/2017 11:12 AM, Mark Rutland wrote:
> On Thu, Jan 12, 2017 at 03:45:48PM +, Catalin Marinas wrote:
>> On Wed, Jan 11, 2017 at 06:40:52PM +, Mark Rutland wrote:
> 
>>> Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0().
>>
>> This may be fine if my assumptions about this erratum are correct. In
>> the cpu_set_reserved_ttbr0() case we set TTBR0_EL1 to a table without
>> any entries, so no new entries could be tagged with the old ASID.
> 
> For some reason, I was under the impression that the issue was old table
> entries being allocated to the new ASID. Looking over the series again,
> it's not clear to me precisely which cases can occur.
> 
> It would be good to see that clarified.

I'll add more general background the commit message in the next revision
and I've also asked directly about this. The answer is that page table
entries using the new translation table base address will be allocated
into the TLB using the old ASID. If employing the workaround, it's
possible for page table entries using the new translation table base
to be allocated into the TLB using the reserved ASID.

Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-16 Thread Christopher Covington
Hi Mark,

On 01/11/2017 01:45 PM, Mark Rutland wrote:
> On Wed, Jan 11, 2017 at 12:40:42PM -0600, Timur Tabi wrote:
>> On 01/11/2017 12:37 PM, Mark Rutland wrote:
>>> The name, as it is, is perfectly descriptive.
>>>
>>> Let's not sacrifice legibility over a non-issue.
>>
>> I don't want to kick a dead horse or anything, but changing it to
>> QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems
>> without sacrificing anything.
> 
> The CPU is called "Falkor", not "FLKR", and we're not coming up with an
> ACPI table name...
> 
> The ARM Ltd. erratum numbers are global to all parts, so we don't
> include the part name. Is the 1003 erratum number specific to Falkor?
>
> If it's global, you could use QCOM_ERRATUM_1003 instead.

E1003 is specific to Falkor, and hopefully just its first major revision.
Qualcomm Technology's first/previous generation ARMv8 custom
microarchitecture used errata numbers below 1000. I am not aware of
global coordination in the numbering, unfortunately.

> Otherwise, QCOM_FALKOR_ERRATUM_1003 is preferable.

Thanks,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-12 Thread Mark Rutland
On Thu, Jan 12, 2017 at 03:45:48PM +, Catalin Marinas wrote:
> On Wed, Jan 11, 2017 at 06:40:52PM +, Mark Rutland wrote:

> > Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0().
> 
> This may be fine if my assumptions about this erratum are correct. In
> the cpu_set_reserved_ttbr0() case we set TTBR0_EL1 to a table without
> any entries, so no new entries could be tagged with the old ASID.

For some reason, I was under the impression that the issue was old table
entries being allocated to the new ASID. Looking over the series again,
it's not clear to me precisely which cases can occur.

It would be good to see that clarified.

Thanks,
Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-12 Thread Will Deacon
On Thu, Jan 12, 2017 at 03:55:58PM +, Catalin Marinas wrote:
> On Wed, Jan 11, 2017 at 06:22:08PM +, Marc Zyngier wrote:
> > On 11/01/17 18:06, Catalin Marinas wrote:
> > > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> > >> 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 
> > >>  #include 
> > >>  #include 
> > >> +#include 
> > >>  #include 
> > >>  #include 
> > >>  #include 
> > >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
> > >>  ENTRY(cpu_do_switch_mm)
> > >>  mmidx1, 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...
> 
> It may actually be needed in entry.S as well. With SW PAN, we move the
> context switching from cpu_do_switch_mm to the kernel_exit macro when
> returning to user. In this case we are switching from the reserved ASID
> 0 and reserved TTBR0_EL1 (pointing to a zeroed page) to the user's
> TTBR0_EL1 and ASID. If the ASID switch isn't taken into account, we may
> end up with new TLB entries being tagged with the reserved ASID. Apart
> from a potential loss of protection with TTBR0 PAN, is there anything
> else that could go wrong? Maybe a TLB conflict if we mix TLBs from
> multiple address spaces tagged with the same reserved ASID.
> 
> If the above is an issue, we would need to patch
> __uaccess_ttbr0_enable() as well, though I'm more inclined to make this
> erratum not selectable when TTBR0 PAN is enabled.

I don't think that's a reasonable approach. By all means change the
default, but we need to support kernel images with both of these kconfig
options enabled.

Will
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-12 Thread Catalin Marinas
On Wed, Jan 11, 2017 at 06:22:08PM +, Marc Zyngier wrote:
> On 11/01/17 18:06, Catalin Marinas wrote:
> > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> >> 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 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
> >>  ENTRY(cpu_do_switch_mm)
> >>mmidx1, 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...

It may actually be needed in entry.S as well. With SW PAN, we move the
context switching from cpu_do_switch_mm to the kernel_exit macro when
returning to user. In this case we are switching from the reserved ASID
0 and reserved TTBR0_EL1 (pointing to a zeroed page) to the user's
TTBR0_EL1 and ASID. If the ASID switch isn't taken into account, we may
end up with new TLB entries being tagged with the reserved ASID. Apart
from a potential loss of protection with TTBR0 PAN, is there anything
else that could go wrong? Maybe a TLB conflict if we mix TLBs from
multiple address spaces tagged with the same reserved ASID.

If the above is an issue, we would need to patch
__uaccess_ttbr0_enable() as well, though I'm more inclined to make this
erratum not selectable when TTBR0 PAN is enabled.

-- 
Catalin
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-12 Thread Catalin Marinas
On Wed, Jan 11, 2017 at 06:40:52PM +, Mark Rutland wrote:
> On Wed, Jan 11, 2017 at 06:22:08PM +, Marc Zyngier wrote:
> > On 11/01/17 18:06, Catalin Marinas wrote:
> > > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> > >> 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 
> > >>  #include 
> > >>  #include 
> > >> +#include 
> > >>  #include 
> > >>  #include 
> > >>  #include 
> > >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
> > >>  ENTRY(cpu_do_switch_mm)
> > >>  mmidx1, 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...
> 
> Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0().

This may be fine if my assumptions about this erratum are correct. In
the cpu_set_reserved_ttbr0() case we set TTBR0_EL1 to a table without
any entries, so no new entries could be tagged with the old ASID.

-- 
Catalin
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-12 Thread Catalin Marinas
On Wed, Jan 11, 2017 at 06:37:39PM +, Mark Rutland wrote:
> On Wed, Jan 11, 2017 at 12:35:55PM -0600, Timur Tabi wrote:
> > On 01/11/2017 12:33 PM, Mark Rutland wrote:
> > >It'll need to affect all lines since the kconfig column needs to expand
> > >by at least one character to fit QCOM_FALKOR_ERRATUM_1003.
> > 
> > Or we can make the macro shorter.
> 
> The name, as it is, is perfectly descriptive.
> 
> Let's not sacrifice legibility over a non-issue.

I agree, I didn't realise that the we expand the last column already.
It's a non-issue indeed.

-- 
Catalin
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-11 Thread Marc Zyngier
[finally, some proper bikeshedding]

On 11/01/17 18:40, Timur Tabi wrote:
> On 01/11/2017 12:37 PM, Mark Rutland wrote:
>> The name, as it is, is perfectly descriptive.
>>
>> Let's not sacrifice legibility over a non-issue.
> 
> I don't want to kick a dead horse or anything, but changing it to 
> QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems without 
> sacrificing anything.

Other than not being able to grep for the core name in the source tree,
how do you suggest we pronounce FLKR? Because so far, it rolls off the
tongue in an interesting way...

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-11 Thread Mark Rutland
On Wed, Jan 11, 2017 at 12:40:42PM -0600, Timur Tabi wrote:
> On 01/11/2017 12:37 PM, Mark Rutland wrote:
> >The name, as it is, is perfectly descriptive.
> >
> >Let's not sacrifice legibility over a non-issue.
> 
> I don't want to kick a dead horse or anything, but changing it to
> QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems
> without sacrificing anything.

The CPU is called "Falkor", not "FLKR", and we're not coming up with an
ACPI table name...

The ARM Ltd. erratum numbers are global to all parts, so we don't
include the part name. Is the 1003 erratum number specific to Falkor?

If it's global, you could use QCOM_ERRATUM_1003 instead.

Otherwise, QCOM_FALKOR_ERRATUM_1003 is preferable.

Thanks,
Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-11 Thread Mark Rutland
On Wed, Jan 11, 2017 at 06:22:08PM +, Marc Zyngier wrote:
> On 11/01/17 18:06, Catalin Marinas wrote:
> > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> >> 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 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
> >>  ENTRY(cpu_do_switch_mm)
> >>mmidx1, 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...

Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0().

Thanks,
Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-11 Thread Timur Tabi

On 01/11/2017 12:37 PM, Mark Rutland wrote:

The name, as it is, is perfectly descriptive.

Let's not sacrifice legibility over a non-issue.


I don't want to kick a dead horse or anything, but changing it to 
QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems without 
sacrificing anything.


--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-11 Thread Mark Rutland
On Wed, Jan 11, 2017 at 12:35:55PM -0600, Timur Tabi wrote:
> On 01/11/2017 12:33 PM, Mark Rutland wrote:
> >It'll need to affect all lines since the kconfig column needs to expand
> >by at least one character to fit QCOM_FALKOR_ERRATUM_1003.
> 
> Or we can make the macro shorter.

The name, as it is, is perfectly descriptive.

Let's not sacrifice legibility over a non-issue.

Thanks,
Mark.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-11 Thread Marc Zyngier
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. */
>>  

Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003

2017-01-11 Thread Catalin Marinas
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) &&
>