* [PATCH] arm64: errata: refer to config ARM64_ERRATUM_2645198 to make workaround work
@ 2022-12-15 9:48 Lukas Bulwahn
2022-12-15 10:57 ` Will Deacon
0 siblings, 1 reply; 5+ messages in thread
From: Lukas Bulwahn @ 2022-12-15 9:48 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Anshuman Khandual, linux-arm-kernel
Cc: kernel-janitors, linux-kernel, Lukas Bulwahn
Commit 44ecda71fd8a ("arm64: errata: Workaround possible Cortex-A715
[ESR|FAR]_ELx corruption") implements a workaround for arm64 erratum
2645198. The arm64 cpucaps is called WORKAROUND_2645198; the kernel build
configuration is called ARM64_ERRATUM_2645198.
In the functions huge_ptep_modify_prot_start() and
ptep_modify_prot_start(), the code accidently refers to the non-existing
config CONFIG_ARM64_WORKAROUND_2645198. Note that the config name uses
ERRATUM, not WORKAROUND. By this accidental misreference, this condition is
always false, the branch of the workaround is not reachable and the
workaround is effectively not implemented at all.
Refer to the intended config ARM64_ERRATUM_2645198 and make the intended
workaround effectively work.
Fixes: 44ecda71fd8a ("arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption")
Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
arch/arm64/mm/hugetlbpage.c | 2 +-
arch/arm64/mm/mmu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index cd8d96e1fa1a..95364e8bdc19 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -562,7 +562,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
{
- if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
+ if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
/*
* Break-before-make (BBM) is required for all user space mappings
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 12915f379c22..d77c9f56b7b4 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1633,7 +1633,7 @@ early_initcall(prevent_bootmem_remove_init);
pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
{
- if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
+ if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
/*
* Break-before-make (BBM) is required for all user space mappings
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] arm64: errata: refer to config ARM64_ERRATUM_2645198 to make workaround work
2022-12-15 9:48 [PATCH] arm64: errata: refer to config ARM64_ERRATUM_2645198 to make workaround work Lukas Bulwahn
@ 2022-12-15 10:57 ` Will Deacon
2022-12-15 11:29 ` Anshuman Khandual
0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2022-12-15 10:57 UTC (permalink / raw)
To: Lukas Bulwahn
Cc: Catalin Marinas, Anshuman Khandual, linux-arm-kernel,
kernel-janitors, linux-kernel
On Thu, Dec 15, 2022 at 10:48:11AM +0100, Lukas Bulwahn wrote:
> Commit 44ecda71fd8a ("arm64: errata: Workaround possible Cortex-A715
> [ESR|FAR]_ELx corruption") implements a workaround for arm64 erratum
> 2645198. The arm64 cpucaps is called WORKAROUND_2645198; the kernel build
> configuration is called ARM64_ERRATUM_2645198.
>
> In the functions huge_ptep_modify_prot_start() and
> ptep_modify_prot_start(), the code accidently refers to the non-existing
> config CONFIG_ARM64_WORKAROUND_2645198. Note that the config name uses
> ERRATUM, not WORKAROUND. By this accidental misreference, this condition is
> always false, the branch of the workaround is not reachable and the
> workaround is effectively not implemented at all.
>
> Refer to the intended config ARM64_ERRATUM_2645198 and make the intended
> workaround effectively work.
>
> Fixes: 44ecda71fd8a ("arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption")
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
> arch/arm64/mm/hugetlbpage.c | 2 +-
> arch/arm64/mm/mmu.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Thanks for the report!
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index cd8d96e1fa1a..95364e8bdc19 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -562,7 +562,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
>
> pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> {
> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
> /*
> * Break-before-make (BBM) is required for all user space mappings
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 12915f379c22..d77c9f56b7b4 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1633,7 +1633,7 @@ early_initcall(prevent_bootmem_remove_init);
>
> pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> {
> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
> /*
> * Break-before-make (BBM) is required for all user space mappings
Grr, this bug seems to exist in all three versions of the patch reviewed on
the list, so I can only draw the conclusion that this code has never been
tested. Consequently, I'm more inclined to _revert_ the change for now and
we can bring it back as a fix once somebody has checked that it actually
works properly.
Will
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] arm64: errata: refer to config ARM64_ERRATUM_2645198 to make workaround work
2022-12-15 10:57 ` Will Deacon
@ 2022-12-15 11:29 ` Anshuman Khandual
2022-12-15 19:44 ` Will Deacon
0 siblings, 1 reply; 5+ messages in thread
From: Anshuman Khandual @ 2022-12-15 11:29 UTC (permalink / raw)
To: Will Deacon, Lukas Bulwahn
Cc: Catalin Marinas, linux-arm-kernel, kernel-janitors, linux-kernel
On 12/15/22 16:27, Will Deacon wrote:
> On Thu, Dec 15, 2022 at 10:48:11AM +0100, Lukas Bulwahn wrote:
>> Commit 44ecda71fd8a ("arm64: errata: Workaround possible Cortex-A715
>> [ESR|FAR]_ELx corruption") implements a workaround for arm64 erratum
>> 2645198. The arm64 cpucaps is called WORKAROUND_2645198; the kernel build
>> configuration is called ARM64_ERRATUM_2645198.
>>
>> In the functions huge_ptep_modify_prot_start() and
>> ptep_modify_prot_start(), the code accidently refers to the non-existing
>> config CONFIG_ARM64_WORKAROUND_2645198. Note that the config name uses
>> ERRATUM, not WORKAROUND. By this accidental misreference, this condition is
>> always false, the branch of the workaround is not reachable and the
>> workaround is effectively not implemented at all.
>>
>> Refer to the intended config ARM64_ERRATUM_2645198 and make the intended
>> workaround effectively work.
>>
>> Fixes: 44ecda71fd8a ("arm64: errata: Workaround possible Cortex-A715 [ESR|FAR]_ELx corruption")
>> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
>> ---
>> arch/arm64/mm/hugetlbpage.c | 2 +-
>> arch/arm64/mm/mmu.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> Thanks for the report!
>
>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>> index cd8d96e1fa1a..95364e8bdc19 100644
>> --- a/arch/arm64/mm/hugetlbpage.c
>> +++ b/arch/arm64/mm/hugetlbpage.c
>> @@ -562,7 +562,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
>>
>> pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>> {
>> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
>> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
>> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
>> /*
>> * Break-before-make (BBM) is required for all user space mappings
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 12915f379c22..d77c9f56b7b4 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1633,7 +1633,7 @@ early_initcall(prevent_bootmem_remove_init);
>>
>> pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>> {
>> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
>> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
>> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
>> /*
>> * Break-before-make (BBM) is required for all user space mappings
>
> Grr, this bug seems to exist in all three versions of the patch reviewed on
> the list, so I can only draw the conclusion that this code has never been
Ohh, my bad, apologies. I did not have a real system with this erratum, although
had emulated and tested this workaround path via some other debug changes (which
might have just forced the first condition to always evaluate true).
> tested. Consequently, I'm more inclined to _revert_ the change for now and
> we can bring it back as a fix once somebody has checked that it actually
> works properly.
Please do not revert this change if possible.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] arm64: errata: refer to config ARM64_ERRATUM_2645198 to make workaround work
2022-12-15 11:29 ` Anshuman Khandual
@ 2022-12-15 19:44 ` Will Deacon
2022-12-15 23:31 ` Anshuman Khandual
0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2022-12-15 19:44 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Lukas Bulwahn, Catalin Marinas, linux-arm-kernel,
kernel-janitors, linux-kernel
On Thu, Dec 15, 2022 at 04:59:20PM +0530, Anshuman Khandual wrote:
> On 12/15/22 16:27, Will Deacon wrote:
> > On Thu, Dec 15, 2022 at 10:48:11AM +0100, Lukas Bulwahn wrote:
> >> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> >> index cd8d96e1fa1a..95364e8bdc19 100644
> >> --- a/arch/arm64/mm/hugetlbpage.c
> >> +++ b/arch/arm64/mm/hugetlbpage.c
> >> @@ -562,7 +562,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
> >>
> >> pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> >> {
> >> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> >> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
> >> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
> >> /*
> >> * Break-before-make (BBM) is required for all user space mappings
> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> >> index 12915f379c22..d77c9f56b7b4 100644
> >> --- a/arch/arm64/mm/mmu.c
> >> +++ b/arch/arm64/mm/mmu.c
> >> @@ -1633,7 +1633,7 @@ early_initcall(prevent_bootmem_remove_init);
> >>
> >> pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
> >> {
> >> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
> >> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
> >> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
> >> /*
> >> * Break-before-make (BBM) is required for all user space mappings
> >
> > Grr, this bug seems to exist in all three versions of the patch reviewed on
> > the list, so I can only draw the conclusion that this code has never been
>
> Ohh, my bad, apologies. I did not have a real system with this erratum, although
> had emulated and tested this workaround path via some other debug changes (which
> might have just forced the first condition to always evaluate true).
"might have"?
> > tested. Consequently, I'm more inclined to _revert_ the change for now and
> > we can bring it back as a fix once somebody has checked that it actually
> > works properly.
> Please do not revert this change if possible.
I've gone ahead with the revert anyway, just because it's the easy thing to
do and we can bring back a fixed version of the patch as a fix in the new
year. So please send a new version with this fix folded in after you've
tested that it doesn't cause regressions for systems without the erratum.
Cheers,
Will
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] arm64: errata: refer to config ARM64_ERRATUM_2645198 to make workaround work
2022-12-15 19:44 ` Will Deacon
@ 2022-12-15 23:31 ` Anshuman Khandual
0 siblings, 0 replies; 5+ messages in thread
From: Anshuman Khandual @ 2022-12-15 23:31 UTC (permalink / raw)
To: Will Deacon
Cc: Lukas Bulwahn, Catalin Marinas, linux-arm-kernel,
kernel-janitors, linux-kernel
On 12/16/22 01:14, Will Deacon wrote:
> On Thu, Dec 15, 2022 at 04:59:20PM +0530, Anshuman Khandual wrote:
>> On 12/15/22 16:27, Will Deacon wrote:
>>> On Thu, Dec 15, 2022 at 10:48:11AM +0100, Lukas Bulwahn wrote:
>>>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>>>> index cd8d96e1fa1a..95364e8bdc19 100644
>>>> --- a/arch/arm64/mm/hugetlbpage.c
>>>> +++ b/arch/arm64/mm/hugetlbpage.c
>>>> @@ -562,7 +562,7 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
>>>>
>>>> pte_t huge_ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>>>> {
>>>> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
>>>> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
>>>> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
>>>> /*
>>>> * Break-before-make (BBM) is required for all user space mappings
>>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>>> index 12915f379c22..d77c9f56b7b4 100644
>>>> --- a/arch/arm64/mm/mmu.c
>>>> +++ b/arch/arm64/mm/mmu.c
>>>> @@ -1633,7 +1633,7 @@ early_initcall(prevent_bootmem_remove_init);
>>>>
>>>> pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep)
>>>> {
>>>> - if (IS_ENABLED(CONFIG_ARM64_WORKAROUND_2645198) &&
>>>> + if (IS_ENABLED(CONFIG_ARM64_ERRATUM_2645198) &&
>>>> cpus_have_const_cap(ARM64_WORKAROUND_2645198)) {
>>>> /*
>>>> * Break-before-make (BBM) is required for all user space mappings
>>>
>>> Grr, this bug seems to exist in all three versions of the patch reviewed on
>>> the list, so I can only draw the conclusion that this code has never been
>>
>> Ohh, my bad, apologies. I did not have a real system with this erratum, although
>> had emulated and tested this workaround path via some other debug changes (which
>> might have just forced the first condition to always evaluate true).
>
> "might have"?
>
>>> tested. Consequently, I'm more inclined to _revert_ the change for now and
>>> we can bring it back as a fix once somebody has checked that it actually
>>> works properly.
>> Please do not revert this change if possible.
>
> I've gone ahead with the revert anyway, just because it's the easy thing to
> do and we can bring back a fixed version of the patch as a fix in the new
> year. So please send a new version with this fix folded in after you've
> tested that it doesn't cause regressions for systems without the erratum.
Sure, will resend. Again, apologies for this last minute merge window trouble.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-15 23:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-15 9:48 [PATCH] arm64: errata: refer to config ARM64_ERRATUM_2645198 to make workaround work Lukas Bulwahn
2022-12-15 10:57 ` Will Deacon
2022-12-15 11:29 ` Anshuman Khandual
2022-12-15 19:44 ` Will Deacon
2022-12-15 23:31 ` Anshuman Khandual
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®