arm64: Define Falkor v1 CPU

The Qualcomm Datacenter Technologies Falkor v1 CPU may allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being updated. When the erratum is triggered, page table entries using the new translation table base address (BADDR) will be allocated into the TLB using the old ASID. All circumstances leading to the incorrect ASID being cached in the TLB arise when software writes TTBRx_EL1[ASID] and TTBRx_EL1[BADDR], a memory operation is in the process of performing a translation using the specific TTBRx_EL1 being written, and the memory operation uses a translation table descriptor designated as non-global. EL2 and EL3 code changing the EL1&0 ASID is not subject to this erratum because hardware is prohibited from performing translations from an out-of-context translation regime.

Consider the following pseudo code.

write new BADDR and ASID values to TTBRx_EL1

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Quoted 26 lines · Christopher Covington

The Qualcomm Datacenter Technologies Falkor v1 CPU may allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being updated. When the erratum is triggered, page table entries using the new translation table base address (BADDR) will be allocated into the TLB using the old ASID. All circumstances leading to the incorrect ASID being cached in the TLB arise when software writes TTBRx_EL1[ASID] and TTBRx_EL1[BADDR], a memory operation is in the process of performing a translation using the specific TTBRx_EL1 being written, and the memory operation uses a translation table descriptor designated as non-global. EL2 and EL3 code changing the EL1&0 ASID is not subject to this erratum because hardware is prohibited from performing translations from an out-of-context translation regime.

Consider the following pseudo code.

write new BADDR and ASID values to TTBRx_EL1

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

Quoted 4 lines

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

I agree that there should be no explicit accesses to the VAs for these entries. So tasks should not see erroneous VAs, and we shouldn't see synchronous TLB conflict aborts.

Regardless, can this allow conflicting TLB entries to be allocated to the reserved ASID? e.g. if one task has a 4K mapping at a given VA, and another has a 2M mapping which covers that VA, can both be allocated into the TLBs under the reserved ASID?

Can that have any effect on asynchronous TLB lookups or page table walks, e.g. for speculated accesses?

Quoted 26 lines · Mark Rutland

Quoted 10 lines · Christopher Covington

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

Quoted 4 lines

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

I agree that there should be no explicit accesses to the VAs for these entries. So tasks should not see erroneous VAs, and we shouldn't see synchronous TLB conflict aborts.

Regardless, can this allow conflicting TLB entries to be allocated to the reserved ASID? e.g. if one task has a 4K mapping at a given VA, and another has a 2M mapping which covers that VA, can both be allocated into the TLBs under the reserved ASID?

Quoted 2 lines

Can that have any effect on asynchronous TLB lookups or page table walks, e.g. for speculated accesses?

A speculative access that inserts an entry into the TLB could possibly find the conflict but will not signal it. Does that answer your question?

Hi,

Quoted 27 lines · Christopher Covington

Quoted 26 lines · Mark Rutland

Quoted 10 lines · Christopher Covington

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

Quoted 4 lines

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

I agree that there should be no explicit accesses to the VAs for these entries. So tasks should not see erroneous VAs, and we shouldn't see synchronous TLB conflict aborts.

Regardless, can this allow conflicting TLB entries to be allocated to the reserved ASID? e.g. if one task has a 4K mapping at a given VA, and another has a 2M mapping which covers that VA, can both be allocated into the TLBs under the reserved ASID?

Quoted 6 lines

Quoted 2 lines

Can that have any effect on asynchronous TLB lookups or page table walks, e.g. for speculated accesses?

A speculative access that inserts an entry into the TLB could possibly find the conflict but will not signal it. Does that answer your question?

Yes!

The other case I was worried about was intermediate caching. I take it the values in TLBs are not used as part of subsequent page table walks?

If so, the above sounds fine to me.

Otherwise, we'll need additional TLB maintenance.

The Qualcomm Datacenter Technologies Falkor v1 CPU may allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being updated. When the erratum is triggered, page table entries using the new translation table base address (BADDR) will be allocated into the TLB using the old ASID. All circumstances leading to the incorrect ASID being cached in the TLB arise when software writes TTBRx_EL1[ASID] and TTBRx_EL1[BADDR], a memory operation is in the process of performing a translation using the specific TTBRx_EL1 being written, and the memory operation uses a translation table descriptor designated as non-global. EL2 and EL3 code changing the EL1&0 ASID is not subject to this erratum because hardware is prohibited from performing translations from an out-of-context translation regime.

Consider the following pseudo code.

write new BADDR and ASID values to TTBRx_EL1

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

Based on work by Shanker Donthineni <[email protected]>

Signed-off-by Christopher Covington <[email protected]>

---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 11 +++++++++++
 arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/mmu_context.h   |  8 +++++++-
 arch/arm64/kernel/cpu_errata.c         |  7 +++++++
 arch/arm64/mm/context.c                | 11 +++++++++++
 arch/arm64/mm/proc.S                   |  1 +
 8 files changed, 63 insertions(+), 2 deletions(-)

Quoted 26 lines · Christopher Covington

The Qualcomm Datacenter Technologies Falkor v1 CPU may allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being updated. When the erratum is triggered, page table entries using the new translation table base address (BADDR) will be allocated into the TLB using the old ASID. All circumstances leading to the incorrect ASID being cached in the TLB arise when software writes TTBRx_EL1[ASID] and TTBRx_EL1[BADDR], a memory operation is in the process of performing a translation using the specific TTBRx_EL1 being written, and the memory operation uses a translation table descriptor designated as non-global. EL2 and EL3 code changing the EL1&0 ASID is not subject to this erratum because hardware is prohibited from performing translations from an out-of-context translation regime.

Consider the following pseudo code.

write new BADDR and ASID values to TTBRx_EL1

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

Quoted 4 lines

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

Based on my understanding that entries allocated to the reserved ASID will not be used for subsequent page table walks (and so we don't have asynchronous behaviour to contend with), this sounds fine to me.

Thanks for taking the time to clarify the details on that.

Quoted 13 lines

Based on work by Shanker Donthineni <[email protected]>

Signed-off-by Christopher Covington <[email protected]>

---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 11 +++++++++++
 arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/mmu_context.h   |  8 +++++++-
 arch/arm64/kernel/cpu_errata.c         |  7 +++++++
 arch/arm64/mm/context.c                | 11 +++++++++++
 arch/arm64/mm/proc.S                   |  1 +
 8 files changed, 63 insertions(+), 2 deletions(-)

Don't we need to use pre_ttbr0_update_workaround in <asm/asm-uaccess.h> for CONFIG_ARM64_SW_TTBR0_PAN? We implicitly switch to the reserved ASID for the empty table in __uaccess_ttbr0_disable.

That also means we have to invalidate the reserved ASID so as to not accidentally hit while uaccess is disabled.

Quoted 56 lines · Mark Rutland

Quoted 26 lines · Christopher Covington

The Qualcomm Datacenter Technologies Falkor v1 CPU may allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being updated. When the erratum is triggered, page table entries using the new translation table base address (BADDR) will be allocated into the TLB using the old ASID. All circumstances leading to the incorrect ASID being cached in the TLB arise when software writes TTBRx_EL1[ASID] and TTBRx_EL1[BADDR], a memory operation is in the process of performing a translation using the specific TTBRx_EL1 being written, and the memory operation uses a translation table descriptor designated as non-global. EL2 and EL3 code changing the EL1&0 ASID is not subject to this erratum because hardware is prohibited from performing translations from an out-of-context translation regime.

Consider the following pseudo code.

write new BADDR and ASID values to TTBRx_EL1

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

Quoted 4 lines

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

Based on my understanding that entries allocated to the reserved ASID will not be used for subsequent page table walks (and so we don't have asynchronous behaviour to contend with), this sounds fine to me.

Thanks for taking the time to clarify the details on that.

Quoted 13 lines

Based on work by Shanker Donthineni <[email protected]>

Signed-off-by Christopher Covington <[email protected]>

---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 11 +++++++++++
 arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/mmu_context.h   |  8 +++++++-
 arch/arm64/kernel/cpu_errata.c         |  7 +++++++
 arch/arm64/mm/context.c                | 11 +++++++++++
 arch/arm64/mm/proc.S                   |  1 +
 8 files changed, 63 insertions(+), 2 deletions(-)

Don't we need to use pre_ttbr0_update_workaround in <asm/asm-uaccess.h> for CONFIG_ARM64_SW_TTBR0_PAN? We implicitly switch to the reserved ASID for the empty table in __uaccess_ttbr0_disable.

Quoted 2 lines

That also means we have to invalidate the reserved ASID so as to not accidentally hit while uaccess is disabled.

The CPU in question (Falkor v1) has hardware PAN support. Do we need to worry about including the workaround in the SW PAN code in that case?

Quoted 60 lines · Christopher Covington

Quoted 58 lines · Mark Rutland

Quoted 26 lines · Christopher Covington

The Qualcomm Datacenter Technologies Falkor v1 CPU may allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being updated. When the erratum is triggered, page table entries using the new translation table base address (BADDR) will be allocated into the TLB using the old ASID. All circumstances leading to the incorrect ASID being cached in the TLB arise when software writes TTBRx_EL1[ASID] and TTBRx_EL1[BADDR], a memory operation is in the process of performing a translation using the specific TTBRx_EL1 being written, and the memory operation uses a translation table descriptor designated as non-global. EL2 and EL3 code changing the EL1&0 ASID is not subject to this erratum because hardware is prohibited from performing translations from an out-of-context translation regime.

Consider the following pseudo code.

write new BADDR and ASID values to TTBRx_EL1

Replacing the above sequence with the one below will ensure that no TLB entries with an incorrect ASID are used by software.

write reserved value to TTBRx_EL1[ASID]
ISB
write new value to TTBRx_EL1[BADDR]
ISB
write new value to TTBRx_EL1[ASID]
ISB

Quoted 4 lines

When the above sequence is used, page table entries using the new BADDR value may still be incorrectly allocated into the TLB using the reserved ASID. Yet this will not reduce functionality, since TLB entries incorrectly tagged with the reserved ASID will never be hit by a later instruction.

Based on my understanding that entries allocated to the reserved ASID will not be used for subsequent page table walks (and so we don't have asynchronous behaviour to contend with), this sounds fine to me.

Thanks for taking the time to clarify the details on that.

Quoted 13 lines

Based on work by Shanker Donthineni <[email protected]>

Signed-off-by Christopher Covington <[email protected]>

---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 11 +++++++++++
 arch/arm64/include/asm/assembler.h     | 23 +++++++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/mmu_context.h   |  8 +++++++-
 arch/arm64/kernel/cpu_errata.c         |  7 +++++++
 arch/arm64/mm/context.c                | 11 +++++++++++
 arch/arm64/mm/proc.S                   |  1 +
 8 files changed, 63 insertions(+), 2 deletions(-)

Don't we need to use pre_ttbr0_update_workaround in <asm/asm-uaccess.h> for CONFIG_ARM64_SW_TTBR0_PAN? We implicitly switch to the reserved ASID for the empty table in __uaccess_ttbr0_disable.

That also means we have to invalidate the reserved ASID so as to not accidentally hit while uaccess is disabled.

Quoted 2 lines

The CPU in question (Falkor v1) has hardware PAN support. Do we need to worry about including the workaround in the SW PAN code in that case?

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Quoted 3 lines · Marc Zyngier

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Okay, I'll do my best to add support for the SW PAN case. I rebased and submitted v6 of the E1009 patch [1] so that it no longer depends on this patch landing first, if you all are inclined to pick it up while work on this E1003 patch continues.

1. https://patchwork.kernel.org/patch/9547923/

Quoted 9 lines · Christopher Covington

Quoted 3 lines · Marc Zyngier

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Okay, I'll do my best to add support for the SW PAN case. I rebased and submitted v6 of the E1009 patch [1] so that it no longer depends on this patch landing first, if you all are inclined to pick it up while work on this E1003 patch continues.

The alternative is not enabling SW_PAN (at runtime) if this errata is present, along with a warning stating that hardware-PAN should be enabled in kconfig instead. Not sure what distributions will make of that though.

Quoted 11 lines · Will Deacon

Quoted 9 lines · Christopher Covington

Quoted 3 lines · Marc Zyngier

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Okay, I'll do my best to add support for the SW PAN case. I rebased and submitted v6 of the E1009 patch [1] so that it no longer depends on this patch landing first, if you all are inclined to pick it up while work on this E1003 patch continues.

Quoted 4 lines

The alternative is not enabling SW_PAN (at runtime) if this errata is present, along with a warning stating that hardware-PAN should be enabled in kconfig instead. Not sure what distributions will make of that though.

The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled and in the absence of hardware PAN (or ARM64_PAN disabled), cpu_do_switch_mm is no longer called for user process switching, so the workaround is pretty much useless.

I'm ok with adding the Kconfig dependency below to QCOM_FALKOR_ERRATUM_1003:

	depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN

together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.

Quoted 17 lines · Catalin Marinas

Quoted 11 lines · Will Deacon

Quoted 9 lines · Christopher Covington

Quoted 3 lines · Marc Zyngier

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Okay, I'll do my best to add support for the SW PAN case. I rebased and submitted v6 of the E1009 patch [1] so that it no longer depends on this patch landing first, if you all are inclined to pick it up while work on this E1003 patch continues.

Quoted 4 lines

The alternative is not enabling SW_PAN (at runtime) if this errata is present, along with a warning stating that hardware-PAN should be enabled in kconfig instead. Not sure what distributions will make of that though.

Quoted 4 lines

The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled and in the absence of hardware PAN (or ARM64_PAN disabled), cpu_do_switch_mm is no longer called for user process switching, so the workaround is pretty much useless.

Oh, I see what you mean now.

Quoted 6 lines

I'm ok with adding the Kconfig dependency below to QCOM_FALKOR_ERRATUM_1003:

	depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN

together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.

That makes it look like hardware-PAN is the cause of the erratum.

Maybe
just select ARM64_PAN if the erratum workaround is selected, then runtime warning if we find that the h/w doesn't have PAN but does have the erratum (which should never fire)?

Quoted 25 lines · Will Deacon

Quoted 17 lines · Catalin Marinas

Quoted 11 lines · Will Deacon

Quoted 9 lines · Christopher Covington

Quoted 3 lines · Marc Zyngier

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Okay, I'll do my best to add support for the SW PAN case. I rebased and submitted v6 of the E1009 patch [1] so that it no longer depends on this patch landing first, if you all are inclined to pick it up while work on this E1003 patch continues.

Quoted 4 lines

The alternative is not enabling SW_PAN (at runtime) if this errata is present, along with a warning stating that hardware-PAN should be enabled in kconfig instead. Not sure what distributions will make of that though.

Quoted 4 lines

The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled and in the absence of hardware PAN (or ARM64_PAN disabled), cpu_do_switch_mm is no longer called for user process switching, so the workaround is pretty much useless.

Oh, I see what you mean now.

Quoted 8 lines

Quoted 6 lines

I'm ok with adding the Kconfig dependency below to QCOM_FALKOR_ERRATUM_1003:

	depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN

together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.

That makes it look like hardware-PAN is the cause of the erratum.

With the right Kconfig comment we could make this clearer.

Quoted 4 lines

Maybe
just select ARM64_PAN if the erratum workaround is selected, then runtime warning if we find that the h/w doesn't have PAN but does have the erratum (which should never fire)?

You still need this workaround even if you don't want any PAN (both sw and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not a dependency. It's more like if you do need a PAN, make sure you only use the hw one.

Quoted 28 lines · Will Deacon

Quoted 17 lines · Catalin Marinas

Quoted 11 lines · Will Deacon

Quoted 9 lines · Christopher Covington

Quoted 3 lines · Marc Zyngier

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Okay, I'll do my best to add support for the SW PAN case. I rebased and submitted v6 of the E1009 patch [1] so that it no longer depends on this patch landing first, if you all are inclined to pick it up while work on this E1003 patch continues.

Quoted 4 lines

The alternative is not enabling SW_PAN (at runtime) if this errata is present, along with a warning stating that hardware-PAN should be enabled in kconfig instead. Not sure what distributions will make of that though.

Quoted 4 lines

The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled and in the absence of hardware PAN (or ARM64_PAN disabled), cpu_do_switch_mm is no longer called for user process switching, so the workaround is pretty much useless.

Oh, I see what you mean now.

Quoted 3 lines

I'm ok with adding the Kconfig dependency below to QCOM_FALKOR_ERRATUM_1003:

Quoted 5 lines

Quoted 3 lines

	depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN

together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.

That makes it look like hardware-PAN is the cause of the erratum.

With the right Kconfig comment we could make this clearer.

Quoted 4 lines

Maybe
just select ARM64_PAN if the erratum workaround is selected, then runtime warning if we find that the h/w doesn't have PAN but does have the erratum (which should never fire)?

You still need this workaround even if you don't want any PAN (both sw and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not a dependency. It's more like if you do need a PAN, make sure you only use the hw one.

Quoted 33 lines · Catalin Marinas

Quoted 32 lines · Will Deacon

Quoted 17 lines · Catalin Marinas

Quoted 11 lines · Will Deacon

Quoted 9 lines · Christopher Covington

Quoted 3 lines · Marc Zyngier

Given that all ARMv8 CPUs can support SW_PAN, it is more likely to be enabled than the ARMv8.1 PAN. I'd vote for supporting the workaround in that case too, and hope that people do enable the HW version.

Okay, I'll do my best to add support for the SW PAN case. I rebased and submitted v6 of the E1009 patch [1] so that it no longer depends on this patch landing first, if you all are inclined to pick it up while work on this E1003 patch continues.

Quoted 4 lines

The alternative is not enabling SW_PAN (at runtime) if this errata is present, along with a warning stating that hardware-PAN should be enabled in kconfig instead. Not sure what distributions will make of that though.

Quoted 4 lines

The problem with this patch is that when ARM64_SW_TTBR0_PAN is enabled and in the absence of hardware PAN (or ARM64_PAN disabled), cpu_do_switch_mm is no longer called for user process switching, so the workaround is pretty much useless.

Oh, I see what you mean now.

Quoted 6 lines

I'm ok with adding the Kconfig dependency below to QCOM_FALKOR_ERRATUM_1003:

	depends on !ARM64_SW_TTBR0_PAN || ARM64_PAN

together with a run-time warning if ARM64_SW_TTBR0_PAN is being used.

Quoted 3 lines

Quoted 1 line

That makes it look like hardware-PAN is the cause of the erratum.

With the right Kconfig comment we could make this clearer.

It's not just a comment though, the kconfig option for the workaround will disappear from menuconfig as long as the dependencies aren't met. The dependency is really that SW_PAN depends on !ERRATUM_1003, but that doesn't work for the distributions.

Quoted 9 lines

Quoted 4 lines

Maybe
just select ARM64_PAN if the erratum workaround is selected, then runtime warning if we find that the h/w doesn't have PAN but does have the erratum (which should never fire)?

You still need this workaround even if you don't want any PAN (both sw and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not a dependency. It's more like if you do need a PAN, make sure you only use the hw one.

True, in the case that all PAN options are disabled we still want this to work. How about:

select ARM64_PAN if ARM64_SW_TTBR0_PAN

In fact, what's the reason for supporting SW_PAN and ARM64_PAN as a config combination? Why not just have "PAN" that enables them both and uses the hardware feature if it's there?

Quoted 12 lines · Will Deacon

Quoted 10 lines · Catalin Marinas

Quoted 4 lines · Will Deacon

Maybe
just select ARM64_PAN if the erratum workaround is selected, then runtime warning if we find that the h/w doesn't have PAN but does have the erratum (which should never fire)?

You still need this workaround even if you don't want any PAN (both sw and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not a dependency. It's more like if you do need a PAN, make sure you only use the hw one.

Quoted 4 lines

True, in the case that all PAN options are disabled we still want this to work. How about:

select ARM64_PAN if ARM64_SW_TTBR0_PAN

As I replied to myself, the above would work for me as well, so let's go for this.

Quoted 3 lines

In fact, what's the reason for supporting SW_PAN and ARM64_PAN as a config combination? Why not just have "PAN" that enables them both and uses the hardware feature if it's there?

Because SW PAN has a non-trivial performance hit. You would enable SW PAN only if you are paranoid about security. HW PAN, OTOH, is very cheap and I wouldn't want to miss enabling it in a single Image supporting ARMv8.0 and ARMv8.1 just because SW PAN is slow on ARMv8.0.

IOW, ARM64_PAN is default y while ARM64_SW_TTBR0_PAN is default n.

Quoted 30 lines · Catalin Marinas

Quoted 12 lines · Will Deacon

Quoted 10 lines · Catalin Marinas

Quoted 4 lines · Will Deacon

Maybe
just select ARM64_PAN if the erratum workaround is selected, then runtime warning if we find that the h/w doesn't have PAN but does have the erratum (which should never fire)?

You still need this workaround even if you don't want any PAN (both sw and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not a dependency. It's more like if you do need a PAN, make sure you only use the hw one.

Quoted 4 lines

True, in the case that all PAN options are disabled we still want this to work. How about:

select ARM64_PAN if ARM64_SW_TTBR0_PAN

As I replied to myself, the above would work for me as well, so let's go for this.

Quoted 3 lines

In fact, what's the reason for supporting SW_PAN and ARM64_PAN as a config combination? Why not just have "PAN" that enables them both and uses the hardware feature if it's there?

Because SW PAN has a non-trivial performance hit. You would enable SW PAN only if you are paranoid about security. HW PAN, OTOH, is very cheap and I wouldn't want to miss enabling it in a single Image supporting ARMv8.0 and ARMv8.1 just because SW PAN is slow on ARMv8.0.

Quoted 1 line

IOW, ARM64_PAN is default y while ARM64_SW_TTBR0_PAN is default n.

Ok, in that case, then how about another permutation: we make ARM64_SW_TTBR0_PAN depend on ARM64_PAN? Then when you select "PAN Support" you get a new menu option underneath it for the emulation? I think that solves the erratum case and the use-case above.

Quoted 31 lines · Will Deacon

Quoted 30 lines · Catalin Marinas

Quoted 12 lines · Will Deacon

Quoted 10 lines · Catalin Marinas

Quoted 4 lines · Will Deacon

Maybe
just select ARM64_PAN if the erratum workaround is selected, then runtime warning if we find that the h/w doesn't have PAN but does have the erratum (which should never fire)?

You still need this workaround even if you don't want any PAN (both sw and hw PAN disabled). I wouldn't want to select ARM64_PAN since it's not a dependency. It's more like if you do need a PAN, make sure you only use the hw one.

Quoted 4 lines

True, in the case that all PAN options are disabled we still want this to work. How about:

select ARM64_PAN if ARM64_SW_TTBR0_PAN

As I replied to myself, the above would work for me as well, so let's go for this.

Quoted 3 lines

In fact, what's the reason for supporting SW_PAN and ARM64_PAN as a config combination? Why not just have "PAN" that enables them both and uses the hardware feature if it's there?

Because SW PAN has a non-trivial performance hit. You would enable SW PAN only if you are paranoid about security. HW PAN, OTOH, is very cheap and I wouldn't want to miss enabling it in a single Image supporting ARMv8.0 and ARMv8.1 just because SW PAN is slow on ARMv8.0.

Quoted 6 lines

Quoted 1 line

IOW, ARM64_PAN is default y while ARM64_SW_TTBR0_PAN is default n.

Ok, in that case, then how about another permutation: we make ARM64_SW_TTBR0_PAN depend on ARM64_PAN? Then when you select "PAN Support" you get a new menu option underneath it for the emulation? I think that solves the erratum case and the use-case above.

The problem is that ARM64_PAN is an ARMv8.1 feature and currently grouped accordingly in Kconfig. ARM64_SW_TTBR0_PAN is complementary (and even not recommended on ARMv8.1). We can do this if we break the ARMv8.x feature grouping but just for this erratum, I don't think it's worth.

Refactor the KVM code to use the __tlbi macros, which will allow an errata workaround that repeats tlbi dsb sequences to only change one location. This is not intended to change the generated assembly and comparing before and after vmlinux objdump shows no functional changes.

Signed-off-by Christopher Covington <[email protected]>

Quoted 6 lines · Christopher Covington

Refactor the KVM code to use the __tlbi macros, which will allow an errata workaround that repeats tlbi dsb sequences to only change one location. This is not intended to change the generated assembly and comparing before and after vmlinux objdump shows no functional changes.

Signed-off-by Christopher Covington <[email protected]>

Acked-by Christoffer Dall <[email protected]>

Quoted 6 lines · Christoffer Dall

Quoted 5 lines · Christopher Covington

Refactor the KVM code to use the __tlbi macros, which will allow an errata workaround that repeats tlbi dsb sequences to only change one location. This is not intended to change the generated assembly and comparing before and after vmlinux objdump shows no functional changes.

Quoted 3 lines

Quoted 1 line

Signed-off-by Christopher Covington <[email protected]>

Acked-by Christoffer Dall <[email protected]>

Thanks, I'll queue this one via arm64.

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Quoted 9 lines · Will Deacon

Quoted 8 lines · Christoffer Dall

Quoted 6 lines · Christopher Covington

Refactor the KVM code to use the __tlbi macros, which will allow an errata workaround that repeats tlbi dsb sequences to only change one location. This is not intended to change the generated assembly and comparing before and after vmlinux objdump shows no functional changes.

Signed-off-by Christopher Covington <[email protected]>

Quoted 3 lines

Quoted 1 line

Acked-by Christoffer Dall <[email protected]>

Thanks, I'll queue this one via arm64.

I just noticed this patch but there's been a similar patch from Mark that I've been carrying as part of the KVM TLB monitoring series[0].

[0] http://www.mail-archive.com/[email protected]/msg09359.html

Quoted 18 lines · Punit Agrawal

Quoted 7 lines · Will Deacon

Quoted 6 lines · Christoffer Dall

Quoted 5 lines · Christopher Covington

Refactor the KVM code to use the __tlbi macros, which will allow an errata workaround that repeats tlbi dsb sequences to only change one location. This is not intended to change the generated assembly and comparing before and after vmlinux objdump shows no functional changes.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Christopher Covington <[email protected]>

Acked-by Christoffer Dall <[email protected]>

Thanks, I'll queue this one via arm64.

I just noticed this patch but there's been a similar patch from Mark that I've been carrying as part of the KVM TLB monitoring series[0].

Quoted 1 line

[0] http://www.mail-archive.com/[email protected]/msg09359.html

Well I've already queued the one from Christopher. It's weird that Mark's version appears to miss the local VMID case (but it does have the flush_icache_all I wanted :).

Anyway, I'm not reverting anything, so you'll need to rebase when this lot lands in mainline.

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Refactor the KVM code to use the __tlbi macros, which will allow an errata workaround that repeats tlbi dsb sequences to only change one location. This is not intended to change the generated assembly and comparing before and after vmlinux objdump shows no functional changes.

Signed-off-by Christopher Covington <[email protected]>

---
 arch/arm64/kvm/hyp/tlb.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

During a TLB invalidate sequence targeting the inner shareable domain, Falkor may prematurely complete the DSB before all loads and stores using the old translation are observed. Instruction fetches are not subject to the conditions of this erratum. If the original code sequence includes multiple TLB invalidate instructions followed by a single DSB, onle one of the TLB instructions needs to be repeated to work around this erratum. While the erratum only applies to cases in which the TLBI specifies the inner-shareable domain (*IS form of TLBI) and the DSB is ISH form or stronger (OSH, SYS), this changes applies the workaround overabundantly-- to local TLBI, DSB NSH sequences as well--for simplicity.

Based on work by Shanker Donthineni <[email protected]>

Signed-off-by Christopher Covington <[email protected]>

---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 10 ++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/tlbflush.h      | 18 +++++++++++++++---
 arch/arm64/kernel/cpu_errata.c         |  7 +++++++
 5 files changed, 35 insertions(+), 4 deletions(-)

arch/arm64/include/asm/tlbflush.h +3 -3

index deab52374119..fc434f421c7b 100644
@@ -36,9 +36,21 @@
  * not. The macros handles invoking the asm with or without the
  * register argument as appropriate.
  */
-#define __TLBI_0(op, arg)		asm ("tlbi " #op)
-#define __TLBI_1(op, arg)		asm ("tlbi " #op ", %0" : : "r" (arg))
-#define __TLBI_N(op, arg, n, ...)	__TLBI_##n(op, arg)
+#define __TLBI_0(op, arg) asm volatile ("tlbi " #op "\n"		       \
+			    ALTERNATIVE("nop\n		nop",		       \
+					"dsb ish\n	tlbi " #op,	       \