In order to be able to disable lse_atomic even if cpu
support it, most likely because of memory controller
cannot deal with the lse atomic instructions, use a
new idreg override to deal with it.
Signed-off-by: Maria Yu <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 2 ++
arch/arm64/include/asm/cpufeature.h | 1 +
arch/arm64/kernel/cpufeature.c | 4 +++-
arch/arm64/kernel/idreg-override.c | 11 +++++++++++
4 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 85fb0fa5d091..6ad754549f1d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -433,6 +433,8 @@
arm64.nomops [ARM64] Unconditionally disable Memory Copy and Memory
Set instructions support
+ arm64.nolse_atomic [ARM64] Unconditionally disable LSE Atomic support
+
ataflop= [HW,M68k]
atarimouse= [HW,MOUSE] Atari Mouse
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 96e50227f940..9d56dea1fe62 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -916,6 +916,7 @@ extern struct arm64_ftr_override id_aa64pfr0_override;
extern struct arm64_ftr_override id_aa64pfr1_override;
extern struct arm64_ftr_override id_aa64zfr0_override;
extern struct arm64_ftr_override id_aa64smfr0_override;
+extern struct arm64_ftr_override id_aa64isar0_override;
extern struct arm64_ftr_override id_aa64isar1_override;
extern struct arm64_ftr_override id_aa64isar2_override;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f9d456fe132d..9bd766880807 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -669,6 +669,7 @@ struct arm64_ftr_override __ro_after_init id_aa64pfr0_override;
struct arm64_ftr_override __ro_after_init id_aa64pfr1_override;
struct arm64_ftr_override __ro_after_init id_aa64zfr0_override;
struct arm64_ftr_override __ro_after_init id_aa64smfr0_override;
+struct arm64_ftr_override __ro_after_init id_aa64isar0_override;
struct arm64_ftr_override __ro_after_init id_aa64isar1_override;
struct arm64_ftr_override __ro_after_init id_aa64isar2_override;
@@ -721,7 +722,8 @@ static const struct __ftr_reg_entry {
ARM64_FTR_REG(SYS_ID_AA64DFR1_EL1, ftr_raz),
/* Op1 = 0, CRn = 0, CRm = 6 */
- ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0),
+ ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0,
+ &id_aa64isar0_override),
ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1,
&id_aa64isar1_override),
ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2,
diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
index 2fe2491b692c..af41ab4f3d94 100644
--- a/arch/arm64/kernel/idreg-override.c
+++ b/arch/arm64/kernel/idreg-override.c
@@ -105,6 +105,15 @@ static const struct ftr_set_desc pfr1 __initconst = {
},
};
+static const struct ftr_set_desc isar0 __initconst = {
+ .name = "id_aa64isar0",
+ .override = &id_aa64isar0_override,
+ .fields = {
+ FIELD("atomic", ID_AA64ISAR0_EL1_ATOMIC_SHIFT, NULL),
+ {}
+ },
+};
+
static const struct ftr_set_desc isar1 __initconst = {
.name = "id_aa64isar1",
.override = &id_aa64isar1_override,
@@ -163,6 +172,7 @@ static const struct ftr_set_desc * const regs[] __initconst = {
&mmfr1,
&pfr0,
&pfr1,
+ &isar0,
&isar1,
&isar2,
&smfr0,
@@ -185,6 +195,7 @@ static const struct {
{ "arm64.nomops", "id_aa64isar2.mops=0" },
{ "arm64.nomte", "id_aa64pfr1.mte=0" },
{ "nokaslr", "arm64_sw.nokaslr=1" },
+ { "arm64.nolse_atomic", "id_aa64isar0.atomic=0" },
};
static int __init parse_nokaslr(char *unused)
--
2.17.1
On 7/10/2023 2:07 PM, Randy Dunlap wrote:
> Hi--
>
> On 7/9/23 22:59, Maria Yu wrote:
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 85fb0fa5d091..6ad754549f1d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -433,6 +433,8 @@
>> arm64.nomops [ARM64] Unconditionally disable Memory Copy and Memory
>> Set instructions support
>>
>> + arm64.nolse_atomic [ARM64] Unconditionally disable LSE Atomic support
>> +
>
> These entries should remain in alphabetical order, so arm64.nolse_atomic
> should be just after arm64.nobti.
Thanks for the quic feedback, will fix this in next patchset.
>
> Yes, these are not quite in the correct order. :(
>
> Thanks.
>
>> ataflop= [HW,M68k]
>>
>> atarimouse= [HW,MOUSE] Atari Mouse
>
--
Thx and BRs,
Aiqun(Maria) Yu
Hi--
On 7/9/23 22:59, Maria Yu wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 85fb0fa5d091..6ad754549f1d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -433,6 +433,8 @@
> arm64.nomops [ARM64] Unconditionally disable Memory Copy and Memory
> Set instructions support
>
> + arm64.nolse_atomic [ARM64] Unconditionally disable LSE Atomic support
> +
These entries should remain in alphabetical order, so arm64.nolse_atomic
should be just after arm64.nobti.
Yes, these are not quite in the correct order. :(
Thanks.
> ataflop= [HW,M68k]
>
> atarimouse= [HW,MOUSE] Atari Mouse
--
~Randy
On Mon, 10 Jul 2023 06:59:55 +0100,
Maria Yu <[email protected]> wrote:
>
> In order to be able to disable lse_atomic even if cpu
> support it, most likely because of memory controller
> cannot deal with the lse atomic instructions, use a
> new idreg override to deal with it.
In general, the idreg overrides are *not* there to paper over HW bugs.
They are there to force the kernel to use or disable a feature for
performance reason or to guide the *enabling* of a feature, but not
because the HW is broken.
The broken status of a HW platform must also be documented so that we
know what to expect when we look at, for example, a bad case of memory
corruption (something I'd expect to see on a system that only
partially implements atomic memory operations).
>
> Signed-off-by: Maria Yu <[email protected]>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 2 ++
> arch/arm64/include/asm/cpufeature.h | 1 +
> arch/arm64/kernel/cpufeature.c | 4 +++-
> arch/arm64/kernel/idreg-override.c | 11 +++++++++++
> 4 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 85fb0fa5d091..6ad754549f1d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -433,6 +433,8 @@
> arm64.nomops [ARM64] Unconditionally disable Memory Copy and Memory
> Set instructions support
>
> + arm64.nolse_atomic [ARM64] Unconditionally disable LSE Atomic support
> +
'nolse', or 'noatomic' should be enough. In general, the suffix should
be either derived from the FEAT_* name or the idreg field name.
> ataflop= [HW,M68k]
>
> atarimouse= [HW,MOUSE] Atari Mouse
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 96e50227f940..9d56dea1fe62 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -916,6 +916,7 @@ extern struct arm64_ftr_override id_aa64pfr0_override;
> extern struct arm64_ftr_override id_aa64pfr1_override;
> extern struct arm64_ftr_override id_aa64zfr0_override;
> extern struct arm64_ftr_override id_aa64smfr0_override;
> +extern struct arm64_ftr_override id_aa64isar0_override;
> extern struct arm64_ftr_override id_aa64isar1_override;
> extern struct arm64_ftr_override id_aa64isar2_override;
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index f9d456fe132d..9bd766880807 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -669,6 +669,7 @@ struct arm64_ftr_override __ro_after_init id_aa64pfr0_override;
> struct arm64_ftr_override __ro_after_init id_aa64pfr1_override;
> struct arm64_ftr_override __ro_after_init id_aa64zfr0_override;
> struct arm64_ftr_override __ro_after_init id_aa64smfr0_override;
> +struct arm64_ftr_override __ro_after_init id_aa64isar0_override;
> struct arm64_ftr_override __ro_after_init id_aa64isar1_override;
> struct arm64_ftr_override __ro_after_init id_aa64isar2_override;
>
> @@ -721,7 +722,8 @@ static const struct __ftr_reg_entry {
> ARM64_FTR_REG(SYS_ID_AA64DFR1_EL1, ftr_raz),
>
> /* Op1 = 0, CRn = 0, CRm = 6 */
> - ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0),
> + ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0,
> + &id_aa64isar0_override),
> ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1,
> &id_aa64isar1_override),
> ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2,
> diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
> index 2fe2491b692c..af41ab4f3d94 100644
> --- a/arch/arm64/kernel/idreg-override.c
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -105,6 +105,15 @@ static const struct ftr_set_desc pfr1 __initconst = {
> },
> };
>
> +static const struct ftr_set_desc isar0 __initconst = {
> + .name = "id_aa64isar0",
> + .override = &id_aa64isar0_override,
> + .fields = {
> + FIELD("atomic", ID_AA64ISAR0_EL1_ATOMIC_SHIFT, NULL),
> + {}
> + },
> +};
> +
> static const struct ftr_set_desc isar1 __initconst = {
> .name = "id_aa64isar1",
> .override = &id_aa64isar1_override,
> @@ -163,6 +172,7 @@ static const struct ftr_set_desc * const regs[] __initconst = {
> &mmfr1,
> &pfr0,
> &pfr1,
> + &isar0,
> &isar1,
> &isar2,
> &smfr0,
> @@ -185,6 +195,7 @@ static const struct {
> { "arm64.nomops", "id_aa64isar2.mops=0" },
> { "arm64.nomte", "id_aa64pfr1.mte=0" },
> { "nokaslr", "arm64_sw.nokaslr=1" },
> + { "arm64.nolse_atomic", "id_aa64isar0.atomic=0" },
And what of 32bit?
M.
--
Without deviation from the norm, progress is not possible.
On 7/10/2023 3:27 PM, Marc Zyngier wrote:
> On Mon, 10 Jul 2023 06:59:55 +0100,
> Maria Yu <[email protected]> wrote:
>>
>> In order to be able to disable lse_atomic even if cpu
>> support it, most likely because of memory controller
>> cannot deal with the lse atomic instructions, use a
>> new idreg override to deal with it.
>
> In general, the idreg overrides are *not* there to paper over HW bugs.
> They are there to force the kernel to use or disable a feature for
> performance reason or to guide the *enabling* of a feature, but not
> because the HW is broken.
>
> The broken status of a HW platform must also be documented so that we
> know what to expect when we look at, for example, a bad case of memory
> corruption (something I'd expect to see on a system that only
> partially implements atomic memory operations).
>
good idea. A noc error would be happened if the lse atomic instruction
happened during a memory controller doesn't support lse atomic instructions.
I can put the information in next patchset comment message. Pls feel
free to let know if there is other place to have this kind of
information with.
>>
>> Signed-off-by: Maria Yu <[email protected]>
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 2 ++
>> arch/arm64/include/asm/cpufeature.h | 1 +
>> arch/arm64/kernel/cpufeature.c | 4 +++-
>> arch/arm64/kernel/idreg-override.c | 11 +++++++++++
>> 4 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 85fb0fa5d091..6ad754549f1d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -433,6 +433,8 @@
>> arm64.nomops [ARM64] Unconditionally disable Memory Copy and Memory
>> Set instructions support
>>
>> + arm64.nolse_atomic [ARM64] Unconditionally disable LSE Atomic support
>> +
>
> 'nolse', or 'noatomic' should be enough. In general, the suffix should
> be either derived from the FEAT_* name or the idreg field name.
noatomic can be used in next patchset.
ID_AA64ISAR0_EL1_ATOMIC_SHIFT
>
>> ataflop= [HW,M68k]
>>
>> atarimouse= [HW,MOUSE] Atari Mouse
>> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
>> index 96e50227f940..9d56dea1fe62 100644
>> --- a/arch/arm64/include/asm/cpufeature.h
>> +++ b/arch/arm64/include/asm/cpufeature.h
>> @@ -916,6 +916,7 @@ extern struct arm64_ftr_override id_aa64pfr0_override;
>> extern struct arm64_ftr_override id_aa64pfr1_override;
>> extern struct arm64_ftr_override id_aa64zfr0_override;
>> extern struct arm64_ftr_override id_aa64smfr0_override;
>> +extern struct arm64_ftr_override id_aa64isar0_override;
>> extern struct arm64_ftr_override id_aa64isar1_override;
>> extern struct arm64_ftr_override id_aa64isar2_override;
>>
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index f9d456fe132d..9bd766880807 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -669,6 +669,7 @@ struct arm64_ftr_override __ro_after_init id_aa64pfr0_override;
>> struct arm64_ftr_override __ro_after_init id_aa64pfr1_override;
>> struct arm64_ftr_override __ro_after_init id_aa64zfr0_override;
>> struct arm64_ftr_override __ro_after_init id_aa64smfr0_override;
>> +struct arm64_ftr_override __ro_after_init id_aa64isar0_override;
>> struct arm64_ftr_override __ro_after_init id_aa64isar1_override;
>> struct arm64_ftr_override __ro_after_init id_aa64isar2_override;
>>
>> @@ -721,7 +722,8 @@ static const struct __ftr_reg_entry {
>> ARM64_FTR_REG(SYS_ID_AA64DFR1_EL1, ftr_raz),
>>
>> /* Op1 = 0, CRn = 0, CRm = 6 */
>> - ARM64_FTR_REG(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0),
>> + ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR0_EL1, ftr_id_aa64isar0,
>> + &id_aa64isar0_override),
>> ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR1_EL1, ftr_id_aa64isar1,
>> &id_aa64isar1_override),
>> ARM64_FTR_REG_OVERRIDE(SYS_ID_AA64ISAR2_EL1, ftr_id_aa64isar2,
>> diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
>> index 2fe2491b692c..af41ab4f3d94 100644
>> --- a/arch/arm64/kernel/idreg-override.c
>> +++ b/arch/arm64/kernel/idreg-override.c
>> @@ -105,6 +105,15 @@ static const struct ftr_set_desc pfr1 __initconst = {
>> },
>> };
>>
>> +static const struct ftr_set_desc isar0 __initconst = {
>> + .name = "id_aa64isar0",
>> + .override = &id_aa64isar0_override,
>> + .fields = {
>> + FIELD("atomic", ID_AA64ISAR0_EL1_ATOMIC_SHIFT, NULL),
>> + {}
>> + },
>> +};
>> +
>> static const struct ftr_set_desc isar1 __initconst = {
>> .name = "id_aa64isar1",
>> .override = &id_aa64isar1_override,
>> @@ -163,6 +172,7 @@ static const struct ftr_set_desc * const regs[] __initconst = {
>> &mmfr1,
>> &pfr0,
>> &pfr1,
>> + &isar0,
>> &isar1,
>> &isar2,
>> &smfr0,
>> @@ -185,6 +195,7 @@ static const struct {
>> { "arm64.nomops", "id_aa64isar2.mops=0" },
>> { "arm64.nomte", "id_aa64pfr1.mte=0" },
>> { "nokaslr", "arm64_sw.nokaslr=1" },
>> + { "arm64.nolse_atomic", "id_aa64isar0.atomic=0" },
>
> And what of 32bit?
>
> M.
>
--
Thx and BRs,
Aiqun(Maria) Yu
On Mon, 10 Jul 2023 09:19:54 +0100,
"Aiqun(Maria) Yu" <[email protected]> wrote:
>
> On 7/10/2023 3:27 PM, Marc Zyngier wrote:
> > On Mon, 10 Jul 2023 06:59:55 +0100,
> > Maria Yu <[email protected]> wrote:
> >>
> >> In order to be able to disable lse_atomic even if cpu
> >> support it, most likely because of memory controller
> >> cannot deal with the lse atomic instructions, use a
> >> new idreg override to deal with it.
> >
> > In general, the idreg overrides are *not* there to paper over HW bugs.
> > They are there to force the kernel to use or disable a feature for
> > performance reason or to guide the *enabling* of a feature, but not
> > because the HW is broken.
> >
> > The broken status of a HW platform must also be documented so that we
> > know what to expect when we look at, for example, a bad case of memory
> > corruption (something I'd expect to see on a system that only
> > partially implements atomic memory operations).
> >
>
> good idea. A noc error would be happened if the lse atomic instruction
> happened during a memory controller doesn't support lse atomic
> instructions.
> I can put the information in next patchset comment message. Pls feel
> free to let know if there is other place to have this kind of
> information with.
For a start, Documentation/arch/arm64/silicon-errata.rst should
contain an entry for the actual erratum, and a description of the
symptoms of the issue (you're mentioning a "noc error": how is that
reported to the CPU?).
The workaround should also be detected at runtime -- we cannot rely on
the user to provide a command-line argument to disable an essential
feature that anyone has taken for granted for most of a decade...
[...]
> >> @@ -185,6 +195,7 @@ static const struct {
> >> { "arm64.nomops", "id_aa64isar2.mops=0" },
> >> { "arm64.nomte", "id_aa64pfr1.mte=0" },
> >> { "nokaslr", "arm64_sw.nokaslr=1" },
> >> + { "arm64.nolse_atomic", "id_aa64isar0.atomic=0" },
> >
> > And what of 32bit?
This particular question still stands, as it is likely to affect VMs.
M.
--
Without deviation from the norm, progress is not possible.
On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
> In order to be able to disable lse_atomic even if cpu
> support it, most likely because of memory controller
> cannot deal with the lse atomic instructions, use a
> new idreg override to deal with it.
This should not be a problem for cacheable memory though, right?
Given that Linux does not issue atomic operations to non-cacheable mappings,
I'm struggling to see why there's a problem here.
Please can you explain the problem that you are trying to solve?
Will
On 7/10/2023 5:31 PM, Marc Zyngier wrote:
> On Mon, 10 Jul 2023 09:19:54 +0100,
> "Aiqun(Maria) Yu" <[email protected]> wrote:
>>
>> On 7/10/2023 3:27 PM, Marc Zyngier wrote:
>>> On Mon, 10 Jul 2023 06:59:55 +0100,
>>> Maria Yu <[email protected]> wrote:
>>>>
>>>> In order to be able to disable lse_atomic even if cpu
>>>> support it, most likely because of memory controller
>>>> cannot deal with the lse atomic instructions, use a
>>>> new idreg override to deal with it.
>>>
>>> In general, the idreg overrides are *not* there to paper over HW bugs.
>>> They are there to force the kernel to use or disable a feature for
>>> performance reason or to guide the *enabling* of a feature, but not
>>> because the HW is broken.
>>>
>>> The broken status of a HW platform must also be documented so that we
>>> know what to expect when we look at, for example, a bad case of memory
>>> corruption (something I'd expect to see on a system that only
>>> partially implements atomic memory operations).
>>>
>>
>> good idea. A noc error would be happened if the lse atomic instruction
>> happened during a memory controller doesn't support lse atomic
>> instructions.
>> I can put the information in next patchset comment message. Pls feel
>> free to let know if there is other place to have this kind of
>> information with.
>
> For a start, Documentation/arch/arm64/silicon-errata.rst should
> contain an entry for the actual erratum, and a description of the
> symptoms of the issue (you're mentioning a "noc error": how is that
> reported to the CPU?).
This is not a cpu's errata as my understanding. It is the DDR subsystem
which don't have the LSE atomic feature supported.
>
> The workaround should also be detected at runtime -- we cannot rely on
> the user to provide a command-line argument to disable an essential
> feature that anyone has taken for granted for most of a decade...
We are also seeking help from DDR Subsystem POC to see whether it is
possible to detect the LSE atomic feature support or not at runtime.
In my opinion, LSE atomic is a system level feature instead of a cpu
only feature. So currently solution we is that even if cpu support lse
atomic, but it still needed to be disabled if the cpu working with a lse
atomic not support by current system's DDR subsystem.
>
> [...]
>
>>>> @@ -185,6 +195,7 @@ static const struct {
>>>> { "arm64.nomops", "id_aa64isar2.mops=0" },
>>>> { "arm64.nomte", "id_aa64pfr1.mte=0" },
>>>> { "nokaslr", "arm64_sw.nokaslr=1" },
>>>> + { "arm64.nolse_atomic", "id_aa64isar0.atomic=0" },
>>>
>>> And what of 32bit?
>
> This particular question still stands, as it is likely to affect VMs.
>
> M.
>
--
Thx and BRs,
Aiqun(Maria) Yu
On 7/10/2023 5:37 PM, Will Deacon wrote:
> On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
>> In order to be able to disable lse_atomic even if cpu
>> support it, most likely because of memory controller
>> cannot deal with the lse atomic instructions, use a
>> new idreg override to deal with it.
>
> This should not be a problem for cacheable memory though, right?
>
> Given that Linux does not issue atomic operations to non-cacheable mappings,
> I'm struggling to see why there's a problem here.
The lse atomic operation can be issued on non-cacheable mappings as
well. Even if it is cached data, with different CPUECTLR_EL1 setting, it
can also do far lse atomic operations.
>
> Please can you explain the problem that you are trying to solve?
In our current case, it is a 100% reproducible issue that happened for
uncached data, the cpu which support LSE atomic, but the system's DDR
subsystem is not support this and caused a NOC error and thus
synchronous external abort happened.
>
> Will
--
Thx and BRs,
Aiqun(Maria) Yu
On Tue, 11 Jul 2023 04:30:44 +0100,
"Aiqun(Maria) Yu" <[email protected]> wrote:
>
> On 7/10/2023 5:31 PM, Marc Zyngier wrote:
> > On Mon, 10 Jul 2023 09:19:54 +0100,
> > "Aiqun(Maria) Yu" <[email protected]> wrote:
> >>
> >> On 7/10/2023 3:27 PM, Marc Zyngier wrote:
> >>> On Mon, 10 Jul 2023 06:59:55 +0100,
> >>> Maria Yu <[email protected]> wrote:
> >>>>
> >>>> In order to be able to disable lse_atomic even if cpu
> >>>> support it, most likely because of memory controller
> >>>> cannot deal with the lse atomic instructions, use a
> >>>> new idreg override to deal with it.
> >>>
> >>> In general, the idreg overrides are *not* there to paper over HW bugs.
> >>> They are there to force the kernel to use or disable a feature for
> >>> performance reason or to guide the *enabling* of a feature, but not
> >>> because the HW is broken.
> >>>
> >>> The broken status of a HW platform must also be documented so that we
> >>> know what to expect when we look at, for example, a bad case of memory
> >>> corruption (something I'd expect to see on a system that only
> >>> partially implements atomic memory operations).
> >>>
> >>
> >> good idea. A noc error would be happened if the lse atomic instruction
> >> happened during a memory controller doesn't support lse atomic
> >> instructions.
> >> I can put the information in next patchset comment message. Pls feel
> >> free to let know if there is other place to have this kind of
> >> information with.
> >
> > For a start, Documentation/arch/arm64/silicon-errata.rst should
> > contain an entry for the actual erratum, and a description of the
> > symptoms of the issue (you're mentioning a "noc error": how is that
> > reported to the CPU?).
>
> This is not a cpu's errata as my understanding. It is the DDR
> subsystem which don't have the LSE atomic feature supported.
CPU or not doesn't matter. We also track system errata.
> >
> > The workaround should also be detected at runtime -- we cannot rely on
> > the user to provide a command-line argument to disable an essential
> > feature that anyone has taken for granted for most of a decade...
>
> We are also seeking help from DDR Subsystem POC to see whether it is
> possible to detect the LSE atomic feature support or not at runtime.
Keying it off a DT compatible (or something similar) would work.
> In my opinion, LSE atomic is a system level feature instead of a cpu
> only feature. So currently solution we is that even if cpu support lse
> atomic, but it still needed to be disabled if the cpu working with a
> lse atomic not support by current system's DDR subsystem.
In the absence of a detection mechanism for anything past the CPU,
this is a moot point. At this stage, this is a bit like saying
"writing to memory is a system thing, not only a CPU feature".
And this also breaks KVM if these CPUs don't have FWB, as a guest can
always map a piece of memory as non-cacheable, and trigger the issue
you describe in your reply to Will, even if you hide the atomics on
the host.
M.
--
Without deviation from the norm, progress is not possible.
On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
> On 7/10/2023 5:37 PM, Will Deacon wrote:
> > On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
> > > In order to be able to disable lse_atomic even if cpu
> > > support it, most likely because of memory controller
> > > cannot deal with the lse atomic instructions, use a
> > > new idreg override to deal with it.
> >
> > This should not be a problem for cacheable memory though, right?
> >
> > Given that Linux does not issue atomic operations to non-cacheable mappings,
> > I'm struggling to see why there's a problem here.
>
> The lse atomic operation can be issued on non-cacheable mappings as well.
> Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
> do far lse atomic operations.
Please can you point me to the place in the kernel sources where this
happens? The architecture doesn't guarantee that atomics to non-cacheable
mappings will work, see "B2.2.6 Possible implementation restrictions on
using atomic instructions". Linux, therefore, doesn't issue atomics
to non-cacheable memory.
> > Please can you explain the problem that you are trying to solve?
>
> In our current case, it is a 100% reproducible issue that happened for
> uncached data, the cpu which support LSE atomic, but the system's DDR
> subsystem is not support this and caused a NOC error and thus synchronous
> external abort happened.
So? The Arm ARM allows this behaviour and Linux shouldn't run into it.
Will
On 7/11/2023 2:57 PM, Marc Zyngier wrote:
> On Tue, 11 Jul 2023 04:30:44 +0100,
> "Aiqun(Maria) Yu" <[email protected]> wrote:
>>
>> On 7/10/2023 5:31 PM, Marc Zyngier wrote:
>>> On Mon, 10 Jul 2023 09:19:54 +0100,
>>> "Aiqun(Maria) Yu" <[email protected]> wrote:
>>>>
>>>> On 7/10/2023 3:27 PM, Marc Zyngier wrote:
>>>>> On Mon, 10 Jul 2023 06:59:55 +0100,
>>>>> Maria Yu <[email protected]> wrote:
>>>>>>
>>>>>> In order to be able to disable lse_atomic even if cpu
>>>>>> support it, most likely because of memory controller
>>>>>> cannot deal with the lse atomic instructions, use a
>>>>>> new idreg override to deal with it.
>>>>>
>>>>> In general, the idreg overrides are *not* there to paper over HW bugs.
>>>>> They are there to force the kernel to use or disable a feature for
>>>>> performance reason or to guide the *enabling* of a feature, but not
>>>>> because the HW is broken.
>>>>>
>>>>> The broken status of a HW platform must also be documented so that we
>>>>> know what to expect when we look at, for example, a bad case of memory
>>>>> corruption (something I'd expect to see on a system that only
>>>>> partially implements atomic memory operations).
>>>>>
>>>>
>>>> good idea. A noc error would be happened if the lse atomic instruction
>>>> happened during a memory controller doesn't support lse atomic
>>>> instructions.
>>>> I can put the information in next patchset comment message. Pls feel
>>>> free to let know if there is other place to have this kind of
>>>> information with.
>>>
>>> For a start, Documentation/arch/arm64/silicon-errata.rst should
>>> contain an entry for the actual erratum, and a description of the
>>> symptoms of the issue (you're mentioning a "noc error": how is that
>>> reported to the CPU?).
>>
>> This is not a cpu's errata as my understanding. It is the DDR
>> subsystem which don't have the LSE atomic feature supported.
>
> CPU or not doesn't matter. We also track system errata.
Thank you for clarify on this.
If I am correct understanding, are you suggesting system errata with DT
seperate compatible (or similar) to runtime disable this feature instead
of idreg override with arm64.nolse options?
While it is better to finally affect the host arm64_ftr_regs value since
it can also derived to kvm sys reg as well.
>
>>>
>>> The workaround should also be detected at runtime -- we cannot rely on
>>> the user to provide a command-line argument to disable an essential
>>> feature that anyone has taken for granted for most of a decade...
>>
>> We are also seeking help from DDR Subsystem POC to see whether it is
>> possible to detect the LSE atomic feature support or not at runtime.
>
> Keying it off a DT compatible (or something similar) would work.
>
>> In my opinion, LSE atomic is a system level feature instead of a cpu
>> only feature. So currently solution we is that even if cpu support lse
>> atomic, but it still needed to be disabled if the cpu working with a
>> lse atomic not support by current system's DDR subsystem.
>
> In the absence of a detection mechanism for anything past the CPU,
> this is a moot point. At this stage, this is a bit like saying
> "writing to memory is a system thing, not only a CPU feature".
>
> And this also breaks KVM if these CPUs don't have FWB, as a guest can
> always map a piece of memory as non-cacheable, and trigger the issue
> you describe in your reply to Will, even if you hide the atomics on
> the host.
>
For the KVM part, per my understanding, as long as the current feature
id being overriden, the KVM system also get the current vcpu without the
lse atomic feature enabled.
KVM vcpu will read the sys reg from host arm64_ftr_regs which is already
been controled by the idreg_overrides.
check reference from:
https://elixir.bootlin.com/linux/v6.5-rc1/source/arch/arm64/kernel/cpufeature.c#L680
https://elixir.bootlin.com/linux/v6.5-rc1/source/arch/arm64/kvm/sys_regs.c#L1360
> M.
>
--
Thx and BRs,
Aiqun(Maria) Yu
On Tue, Jul 11, 2023 at 06:15:49PM +0800, Aiqun(Maria) Yu wrote:
> On 7/11/2023 4:22 PM, Will Deacon wrote:
> > On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
> > > On 7/10/2023 5:37 PM, Will Deacon wrote:
> > > > On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
> > > > > In order to be able to disable lse_atomic even if cpu
> > > > > support it, most likely because of memory controller
> > > > > cannot deal with the lse atomic instructions, use a
> > > > > new idreg override to deal with it.
> > > >
> > > > This should not be a problem for cacheable memory though, right?
> > > >
> > > > Given that Linux does not issue atomic operations to non-cacheable mappings,
> > > > I'm struggling to see why there's a problem here.
> > >
> > > The lse atomic operation can be issued on non-cacheable mappings as well.
> > > Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
> > > do far lse atomic operations.
> >
> > Please can you point me to the place in the kernel sources where this
> > happens? The architecture doesn't guarantee that atomics to non-cacheable
> > mappings will work, see "B2.2.6 Possible implementation restrictions on
> > using atomic instructions". Linux, therefore, doesn't issue atomics
> > to non-cacheable memory.
>
> We encounter the issue on third party kernel modules
Which kernel modules?
Those modules are clearly broken; as Will has already said, the architecture
says doing atomics to non-cacheable memory can result in external aborts, and
that's exaclty the behaviour that you're reporting as a problem. This is
working *as designed*.
Note that the same is true for LDXR+STXR; so just hiding LSE doesn't make
sense: if the code falls back to LDXR+STXR it still suffers from the exact same
problem.
Regardless, hiding bugs in out-of-tree code is not a justification for changing
the upstream kernel.
> and third party apps instead of linux kernel itself.
Which apps?
Why are those apps using non-cacheable memory?
Why are those apps trying to perform atomics to non-cacheable memory?
> This is a tradeoff of performance and stability. Per my understanding,
> options can be used to enable the lse_atomic to have the most performance
> cared system, and disable the lse_atomic by stability cared most system.
I think that's a misrepresentation of this patch.
This patch disables a feature to *hide* bugs in out-of-tree kernel modules and
userspace software. It's not about making the system more stable, it's about
making broken code appear to work.
The LSE atomics aren't just about performance. They're significantly fairer
than LDXR+STXR in many practical situations, and contribute to the stability of
the system.
Thanks,
Mark.
> > > > Please can you explain the problem that you are trying to solve?
> > >
> > > In our current case, it is a 100% reproducible issue that happened for
> > > uncached data, the cpu which support LSE atomic, but the system's DDR
> > > subsystem is not support this and caused a NOC error and thus synchronous
> > > external abort happened.
> >
> > So? The Arm ARM allows this behaviour and Linux shouldn't run into it.
> >
> > Will
>
> --
> Thx and BRs,
> Aiqun(Maria) Yu
>
On 7/11/2023 4:22 PM, Will Deacon wrote:
> On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
>> On 7/10/2023 5:37 PM, Will Deacon wrote:
>>> On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
>>>> In order to be able to disable lse_atomic even if cpu
>>>> support it, most likely because of memory controller
>>>> cannot deal with the lse atomic instructions, use a
>>>> new idreg override to deal with it.
>>>
>>> This should not be a problem for cacheable memory though, right?
>>>
>>> Given that Linux does not issue atomic operations to non-cacheable mappings,
>>> I'm struggling to see why there's a problem here.
>>
>> The lse atomic operation can be issued on non-cacheable mappings as well.
>> Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
>> do far lse atomic operations.
>
> Please can you point me to the place in the kernel sources where this
> happens? The architecture doesn't guarantee that atomics to non-cacheable
> mappings will work, see "B2.2.6 Possible implementation restrictions on
> using atomic instructions". Linux, therefore, doesn't issue atomics
> to non-cacheable memory.
We encounter the issue on third party kernel modules and third party
apps instead of linux kernel itself.
This is a tradeoff of performance and stability. Per my understanding,
options can be used to enable the lse_atomic to have the most
performance cared system, and disable the lse_atomic by stability cared
most system.
>
>>> Please can you explain the problem that you are trying to solve?
>>
>> In our current case, it is a 100% reproducible issue that happened for
>> uncached data, the cpu which support LSE atomic, but the system's DDR
>> subsystem is not support this and caused a NOC error and thus synchronous
>> external abort happened.
>
> So? The Arm ARM allows this behaviour and Linux shouldn't run into it.
>
> Will
--
Thx and BRs,
Aiqun(Maria) Yu
On Tue, 11 Jul 2023 11:12:48 +0100,
"Aiqun(Maria) Yu" <[email protected]> wrote:
>
> For the KVM part, per my understanding, as long as the current feature
> id being overriden, the KVM system also get the current vcpu without
> the lse atomic feature enabled.
> KVM vcpu will read the sys reg from host arm64_ftr_regs which is
> already been controled by the idreg_overrides.
You're completely missing the point.
The guest is free to map memory as non-cacheable *and* to use LSE
atomics even if the idregs pretend this is not available. At which
point the HW throws a fit and the system is dead. Is that acceptable?
Of course not.
So there are two aspects to your problem:
- for Linux, there is nothing to do: the kernel will correctly behave,
and as long as you don't expose non-cacheable memory to userspace.
Out of tree drivers are none of our concern here.
- for guests, it looks like the HW doesn't provide the basic
requirements for virtualisation, and you should always disable KVM
on this HW (or even better, enter the kernel at EL1).
In both cases, nothing to do in the kernel, which is good news.
M.
--
Without deviation from the norm, progress is not possible.
On Tue, Jul 11, 2023 at 06:15:49PM +0800, Aiqun(Maria) Yu wrote:
> On 7/11/2023 4:22 PM, Will Deacon wrote:
> > On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
> > > On 7/10/2023 5:37 PM, Will Deacon wrote:
> > > > On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
> > > > > In order to be able to disable lse_atomic even if cpu
> > > > > support it, most likely because of memory controller
> > > > > cannot deal with the lse atomic instructions, use a
> > > > > new idreg override to deal with it.
> > > >
> > > > This should not be a problem for cacheable memory though, right?
> > > >
> > > > Given that Linux does not issue atomic operations to non-cacheable mappings,
> > > > I'm struggling to see why there's a problem here.
> > >
> > > The lse atomic operation can be issued on non-cacheable mappings as well.
> > > Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
> > > do far lse atomic operations.
> >
> > Please can you point me to the place in the kernel sources where this
> > happens? The architecture doesn't guarantee that atomics to non-cacheable
> > mappings will work, see "B2.2.6 Possible implementation restrictions on
> > using atomic instructions". Linux, therefore, doesn't issue atomics
> > to non-cacheable memory.
>
> We encounter the issue on third party kernel modules and third party apps
> instead of linux kernel itself.
Great, so there's nothing to do in the kernel then!
The third party code needs to be modified not to use atomic instructions
with non-cacheable mappings. No need to involve us with that.
> This is a tradeoff of performance and stability. Per my understanding,
> options can be used to enable the lse_atomic to have the most performance
> cared system, and disable the lse_atomic by stability cared most system.
Where do livelock and starvation fit in with "stability"? Disabling LSE
atomics for things like qspinlock and the scheduler just because of some
badly written third-party code isn't much of a tradeoff.
Will
On 7/11/2023 6:38 PM, Marc Zyngier wrote:
> On Tue, 11 Jul 2023 11:12:48 +0100,
> "Aiqun(Maria) Yu" <[email protected]> wrote:
>>
>> For the KVM part, per my understanding, as long as the current feature
>> id being overriden, the KVM system also get the current vcpu without
>> the lse atomic feature enabled.
>> KVM vcpu will read the sys reg from host arm64_ftr_regs which is
>> already been controled by the idreg_overrides.
>
> You're completely missing the point.
>
> The guest is free to map memory as non-cacheable *and* to use LSE
> atomics even if the idregs pretend this is not available. At which
The guest also can have the current linux kernel mechanism of LSE ATOMIC
way.
+----------------------------+
| |
| Read the cpu feature IDs |
+----------------------------+
v
+----------------------------+ +-------------------+
| | Y | Use lse atomic ins|
| if lse atomic supported | -- | |
+----------------------------+ +-------------------+
v N
+----------------------------+
| Use r/stxr + atomic ins |
| |
+----------------------------+
Just like other KVM vcpu cpu features, lse atomic can be a feature
inherit from the pysical cpu features for the KVM vcpus.
> point the HW throws a fit and the system is dead. Is that acceptable?
> Of course not.
>
The current patchset is try to have the ability to *kind of free* to not
make system dead. Since currently linux kernel already have the runtime
patching of lse atomic ops, we are trying to have user have option to
re-use the switch of system_uses_lse_atomics().
#define __lse_ll_sc_body(op, ...) \
({ \
system_uses_lse_atomics() ? \
__lse_##op(__VA_ARGS__) : \
__ll_sc_##op(__VA_ARGS__); \
})
> So there are two aspects to your problem:
>
> - for Linux, there is nothing to do: the kernel will correctly behave,
> and as long as you don't expose non-cacheable memory to userspace.
> Out of tree drivers are none of our concern here.
>
For Linux kernel, we have provide the In-line patching at runtime, and
all third party kernel modules are built with those in-line patching as
well.
if we can have an option, the current system can still run those third
party kernel modules without system crash.
/* In-line patching at runtime */
#define ARM64_LSE_ATOMIC_INSN(llsc, lse) \
ALTERNATIVE(llsc, __LSE_PREAMBLE lse, ARM64_HAS_LSE_ATOMICS)
static __always_inline bool system_uses_lse_atomics(void)
{
return alternative_has_cap_likely(ARM64_HAS_LSE_ATOMICS);
}
> - for guests, it looks like the HW doesn't provide the basic
> requirements for virtualisation, and you should always disable KVM
> on this HW (or even better, enter the kernel at EL1).
>
I can see that KVM can still be supported even if current phisical cpu
don't have emulated vcpu features. We can only disable the current vcpu
feature which exposed to KVM guest.
> In both cases, nothing to do in the kernel, which is good news.
>
> M.
>
--
Thx and BRs,
Aiqun(Maria) Yu
On 7/11/2023 6:25 PM, Will Deacon wrote:
> On Tue, Jul 11, 2023 at 06:15:49PM +0800, Aiqun(Maria) Yu wrote:
>> On 7/11/2023 4:22 PM, Will Deacon wrote:
>>> On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
>>>> On 7/10/2023 5:37 PM, Will Deacon wrote:
>>>>> On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
>>>>>> In order to be able to disable lse_atomic even if cpu
>>>>>> support it, most likely because of memory controller
>>>>>> cannot deal with the lse atomic instructions, use a
>>>>>> new idreg override to deal with it.
>>>>>
>>>>> This should not be a problem for cacheable memory though, right?
>>>>>
>>>>> Given that Linux does not issue atomic operations to non-cacheable mappings,
>>>>> I'm struggling to see why there's a problem here.
>>>>
>>>> The lse atomic operation can be issued on non-cacheable mappings as well.
>>>> Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
>>>> do far lse atomic operations.
>>>
>>> Please can you point me to the place in the kernel sources where this
>>> happens? The architecture doesn't guarantee that atomics to non-cacheable
>>> mappings will work, see "B2.2.6 Possible implementation restrictions on
>>> using atomic instructions". Linux, therefore, doesn't issue atomics
>>> to non-cacheable memory.
>>
>> We encounter the issue on third party kernel modules and third party apps
>> instead of linux kernel itself.
>
> Great, so there's nothing to do in the kernel then!
>
> The third party code needs to be modified not to use atomic instructions
> with non-cacheable mappings. No need to involve us with that.
>
>> This is a tradeoff of performance and stability. Per my understanding,
>> options can be used to enable the lse_atomic to have the most performance
>> cared system, and disable the lse_atomic by stability cared most system.
>
> Where do livelock and starvation fit in with "stability"? Disabling LSE
> atomics for things like qspinlock and the scheduler just because of some
> badly written third-party code isn't much of a tradeoff.
We also have requirement to have cpus/system fully support lse atomic
and cpus/system not fully support lse atomic with a generic kernel image.
Same kernel module wanted to be used by lse atomic fully support cpu and
not fully support cpu/system as well.
That's why we want to have a runtime option here.
>
> Will
--
Thx and BRs,
Aiqun(Maria) Yu
On Wed, 12 Jul 2023 03:47:55 +0100,
"Aiqun(Maria) Yu" <[email protected]> wrote:
>
> On 7/11/2023 6:38 PM, Marc Zyngier wrote:
> > On Tue, 11 Jul 2023 11:12:48 +0100,
> > "Aiqun(Maria) Yu" <[email protected]> wrote:
> >>
> >> For the KVM part, per my understanding, as long as the current feature
> >> id being overriden, the KVM system also get the current vcpu without
> >> the lse atomic feature enabled.
> >> KVM vcpu will read the sys reg from host arm64_ftr_regs which is
> >> already been controled by the idreg_overrides.
> >
> > You're completely missing the point.
> >
> > The guest is free to map memory as non-cacheable *and* to use LSE
> > atomics even if the idregs pretend this is not available. At which
> The guest also can have the current linux kernel mechanism of LSE
> ATOMIC way.
[snip useless diagrams]
Yes, the guest can do the right thing. The guest, a totally
unprivileged piece of SW, can also ignore the idregs and take the
whole machine down because your HW is broken.
> Just like other KVM vcpu cpu features, lse atomic can be a feature
> inherit from the pysical cpu features for the KVM vcpus.
See above. Your reasoning applies to a well behaved guest, which is
the *wrong* way to reason about these things.
M.
--
Without deviation from the norm, progress is not possible.
On Wed, Jul 12, 2023 at 11:09:10AM +0800, Aiqun(Maria) Yu wrote:
> On 7/11/2023 6:25 PM, Will Deacon wrote:
> > On Tue, Jul 11, 2023 at 06:15:49PM +0800, Aiqun(Maria) Yu wrote:
> > > On 7/11/2023 4:22 PM, Will Deacon wrote:
> > > > On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
> > > > > On 7/10/2023 5:37 PM, Will Deacon wrote:
> > > > > > On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
> > > > > > > In order to be able to disable lse_atomic even if cpu
> > > > > > > support it, most likely because of memory controller
> > > > > > > cannot deal with the lse atomic instructions, use a
> > > > > > > new idreg override to deal with it.
> > > > > >
> > > > > > This should not be a problem for cacheable memory though, right?
> > > > > >
> > > > > > Given that Linux does not issue atomic operations to non-cacheable mappings,
> > > > > > I'm struggling to see why there's a problem here.
> > > > >
> > > > > The lse atomic operation can be issued on non-cacheable mappings as well.
> > > > > Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
> > > > > do far lse atomic operations.
> > > >
> > > > Please can you point me to the place in the kernel sources where this
> > > > happens? The architecture doesn't guarantee that atomics to non-cacheable
> > > > mappings will work, see "B2.2.6 Possible implementation restrictions on
> > > > using atomic instructions". Linux, therefore, doesn't issue atomics
> > > > to non-cacheable memory.
> > >
> > > We encounter the issue on third party kernel modules and third party apps
> > > instead of linux kernel itself.
> >
> > Great, so there's nothing to do in the kernel then!
> >
> > The third party code needs to be modified not to use atomic instructions
> > with non-cacheable mappings. No need to involve us with that.
>
> > > This is a tradeoff of performance and stability. Per my understanding,
> > > options can be used to enable the lse_atomic to have the most performance
> > > cared system, and disable the lse_atomic by stability cared most system.
> >
> > Where do livelock and starvation fit in with "stability"? Disabling LSE
> > atomics for things like qspinlock and the scheduler just because of some
> > badly written third-party code isn't much of a tradeoff.
> We also have requirement to have cpus/system fully support lse atomic and
> cpus/system not fully support lse atomic with a generic kernel image.
Who *specifically* has this requirement (i.e. what does 'we' mean here)? The
upstream kernel does not require that atomics work on non-cacheable memory, and
saying "The company I work for want this" doesn't change that.
AFAICT the system here is architecturally compliant, and what you're relying
upon something that the architecture doesn't guarantee, and Linux doesn't
guarantee.
> Same kernel module wanted to be used by lse atomic fully support cpu and not
> fully support cpu/system as well.
Which kernel modules *specifically* need to do atomics to non-cacheable memory?
> That's why we want to have a runtime option here.
As per other replies, a runtime option doesn't solve the issue you have
described, and it will adversely affect the system in other ways (e.g. the
livelock and starvation issues will mentioned, which we have seen with
LDXR+STXR atomics).
Thanks,
Mark.
On 7/12/2023 3:29 PM, Marc Zyngier wrote:
> On Wed, 12 Jul 2023 03:47:55 +0100,
> "Aiqun(Maria) Yu" <[email protected]> wrote:
>>
>> On 7/11/2023 6:38 PM, Marc Zyngier wrote:
>>> On Tue, 11 Jul 2023 11:12:48 +0100,
>>> "Aiqun(Maria) Yu" <[email protected]> wrote:
>>>>
>>>> For the KVM part, per my understanding, as long as the current feature
>>>> id being overriden, the KVM system also get the current vcpu without
>>>> the lse atomic feature enabled.
>>>> KVM vcpu will read the sys reg from host arm64_ftr_regs which is
>>>> already been controled by the idreg_overrides.
>>>
>>> You're completely missing the point.
>>>
>>> The guest is free to map memory as non-cacheable *and* to use LSE
>>> atomics even if the idregs pretend this is not available. At which
>> The guest also can have the current linux kernel mechanism of LSE
>> ATOMIC way.
>
> [snip useless diagrams]
>
> Yes, the guest can do the right thing. The guest, a totally
> unprivileged piece of SW, can also ignore the idregs and take the
> whole machine down because your HW is broken.
>
if the guest ignore the idregs, it is not supported by the current Linux
KVM id reg emulation as well. The similar rule is applied to other cpu
feature as well.
So it can be an expected machine down because of this.
We want to support/utilize the current HW with current inline runtime
patching for lse atomic ops.
>> Just like other KVM vcpu cpu features, lse atomic can be a feature
>> inherit from the pysical cpu features for the KVM vcpus.
>
> See above. Your reasoning applies to a well behaved guest, which is
> the *wrong* way to reason about these things.
The feature supported is not always that *freely* even for current cpu
features as well.
Our current target is that the software can utilize the HW as best as
software can.
The current HW can be possible with Generic common Image with other cpu
which support lse atomic. So the Image can have inline runtime patching
for lse atomic operations. And from software side it can have option to
support this.
For example, for current newer memory controller the far lse atomic
operations is supported, and the atomic operation is not limited to
non-cached memory mapping as well.
Also the lse atomic instead of FWB performance in specific scenarios can
be different with current hardware design as well.
we are trying to do possible improvement with HW design change instead
of ruin it.
Feel free to comment if it is not same understanding.
>
> M.
>
--
Thx and BRs,
Aiqun(Maria) Yu
On 7/12/2023 3:36 PM, Mark Rutland wrote:
> On Wed, Jul 12, 2023 at 11:09:10AM +0800, Aiqun(Maria) Yu wrote:
>> On 7/11/2023 6:25 PM, Will Deacon wrote:
>>> On Tue, Jul 11, 2023 at 06:15:49PM +0800, Aiqun(Maria) Yu wrote:
>>>> On 7/11/2023 4:22 PM, Will Deacon wrote:
>>>>> On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
>>>>>> On 7/10/2023 5:37 PM, Will Deacon wrote:
>>>>>>> On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
>>>>>>>> In order to be able to disable lse_atomic even if cpu
>>>>>>>> support it, most likely because of memory controller
>>>>>>>> cannot deal with the lse atomic instructions, use a
>>>>>>>> new idreg override to deal with it.
>>>>>>>
>>>>>>> This should not be a problem for cacheable memory though, right?
>>>>>>>
>>>>>>> Given that Linux does not issue atomic operations to non-cacheable mappings,
>>>>>>> I'm struggling to see why there's a problem here.
>>>>>>
>>>>>> The lse atomic operation can be issued on non-cacheable mappings as well.
>>>>>> Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
>>>>>> do far lse atomic operations.
>>>>>
>>>>> Please can you point me to the place in the kernel sources where this
>>>>> happens? The architecture doesn't guarantee that atomics to non-cacheable
>>>>> mappings will work, see "B2.2.6 Possible implementation restrictions on
>>>>> using atomic instructions". Linux, therefore, doesn't issue atomics
>>>>> to non-cacheable memory.
>>>>
>>>> We encounter the issue on third party kernel modules and third party apps
>>>> instead of linux kernel itself.
>>>
>>> Great, so there's nothing to do in the kernel then!
>>>
>>> The third party code needs to be modified not to use atomic instructions
>>> with non-cacheable mappings. No need to involve us with that.
>>
>>>> This is a tradeoff of performance and stability. Per my understanding,
>>>> options can be used to enable the lse_atomic to have the most performance
>>>> cared system, and disable the lse_atomic by stability cared most system.
>>>
>>> Where do livelock and starvation fit in with "stability"? Disabling LSE
>>> atomics for things like qspinlock and the scheduler just because of some
>>> badly written third-party code isn't much of a tradeoff.
>
>> We also have requirement to have cpus/system fully support lse atomic and
>> cpus/system not fully support lse atomic with a generic kernel image.
>
> Who *specifically* has this requirement (i.e. what does 'we' mean here)? The
I can use other word to describe the requirement instead of "we".
There is requirements like android google gki. It request different cpu
arch system to use same generic kernel Image.
> upstream kernel does not require that atomics work on non-cacheable memory, and
The same issue the system can be down of lse atomic not supported for
cachable memory when there need far atomic.
> saying "The company I work for want this" doesn't change that.
>
> AFAICT the system here is architecturally compliant, and what you're relying
> upon something that the architecture doesn't guarantee, and Linux doesn't
> guarantee.
It is not also only our company's problem:
To support the atomic instructions added in the Armv8.1 architecture,
CHI-B provides Atomic Transactions. while Atomic Transactions support is
also *optional* from CHI-B.
So far atomic cannot fully supported by ARMv8.1 cpu + CHI-B system as well.
from:
https://developer.arm.com/documentation/102407/0100/Atomic-operations?lang=en
So only cpu support atomic cannot garantee the system support lse atomic
>
>> Same kernel module wanted to be used by lse atomic fully support cpu and not
>> fully support cpu/system as well.
>
> Which kernel modules *specifically* need to do atomics to non-cacheable memory?
The driver want to always do far atomic(no speculatively) and allow a
read-modify-write non-interruptible sequence in a single instruction.
>
>> That's why we want to have a runtime option here.
>
> As per other replies, a runtime option doesn't solve the issue you have
> described, and it will adversely affect the system in other ways (e.g. the
> livelock and starvation issues will mentioned, which we have seen with
> LDXR+STXR atomics).
I myself also have encounter issues from livelock because of LDXR+STXR
atomics unfairness before. More likely happened when different
performance cpu. So myself also glad to using atomics instead of
exclusive access.
So if there is a way to fully utilize the atomic instructions for
current hardware, and also support the far atomic, that can be much
better solution than currently disable the feature.
>
> Thanks,
> Mark.
Pls feel free to comments. It would lead to a reasonable and usable
solution from our discussions.
--
Thx and BRs,
Aiqun(Maria) Yu
On Thu, Jul 13, 2023 at 10:24:24AM +0800, Aiqun(Maria) Yu wrote:
> On 7/12/2023 3:36 PM, Mark Rutland wrote:
> > On Wed, Jul 12, 2023 at 11:09:10AM +0800, Aiqun(Maria) Yu wrote:
> > > On 7/11/2023 6:25 PM, Will Deacon wrote:
> > > > On Tue, Jul 11, 2023 at 06:15:49PM +0800, Aiqun(Maria) Yu wrote:
> > > > > On 7/11/2023 4:22 PM, Will Deacon wrote:
> > > > > > On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
> > > > > > > On 7/10/2023 5:37 PM, Will Deacon wrote:
> > > > > > > > On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
> > > > > > > > > In order to be able to disable lse_atomic even if cpu
> > > > > > > > > support it, most likely because of memory controller
> > > > > > > > > cannot deal with the lse atomic instructions, use a
> > > > > > > > > new idreg override to deal with it.
> > > > > > > >
> > > > > > > > This should not be a problem for cacheable memory though, right?
> > > > > > > >
> > > > > > > > Given that Linux does not issue atomic operations to non-cacheable mappings,
> > > > > > > > I'm struggling to see why there's a problem here.
> > > > > > >
> > > > > > > The lse atomic operation can be issued on non-cacheable mappings as well.
> > > > > > > Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
> > > > > > > do far lse atomic operations.
> > > > > >
> > > > > > Please can you point me to the place in the kernel sources where this
> > > > > > happens? The architecture doesn't guarantee that atomics to non-cacheable
> > > > > > mappings will work, see "B2.2.6 Possible implementation restrictions on
> > > > > > using atomic instructions". Linux, therefore, doesn't issue atomics
> > > > > > to non-cacheable memory.
> > > > >
> > > > > We encounter the issue on third party kernel modules and third party apps
> > > > > instead of linux kernel itself.
> > > >
> > > > Great, so there's nothing to do in the kernel then!
> > > >
> > > > The third party code needs to be modified not to use atomic instructions
> > > > with non-cacheable mappings. No need to involve us with that.
> > >
> > > > > This is a tradeoff of performance and stability. Per my understanding,
> > > > > options can be used to enable the lse_atomic to have the most performance
> > > > > cared system, and disable the lse_atomic by stability cared most system.
> > > >
> > > > Where do livelock and starvation fit in with "stability"? Disabling LSE
> > > > atomics for things like qspinlock and the scheduler just because of some
> > > > badly written third-party code isn't much of a tradeoff.
> >
> > > We also have requirement to have cpus/system fully support lse atomic and
> > > cpus/system not fully support lse atomic with a generic kernel image.
> >
> > Who *specifically* has this requirement (i.e. what does 'we' mean here)? The
>
> I can use other word to describe the requirement instead of "we".
>
> There is requirements like android google gki. It request different cpu arch
> system to use same generic kernel Image.
GKI requires the system to use the generic kernel image; GKI does not require
supporting atomics to non-cacheable mappings.
What I am asking is: who has the requirement to perform atomics to
non-cacheable mappings?
> > upstream kernel does not require that atomics work on non-cacheable memory, and
>
> The same issue the system can be down of lse atomic not supported for
> cachable memory when there need far atomic.
Are you saying that LSE atomics to *cacheable* mappings do not work on your
system?
Specifically, when using a Normal Inner-Shareable Inner-Writeback
Outer-Writeback mapping, do the LSE atomics work or not work?
> > saying "The company I work for want this" doesn't change that.
> >
> > AFAICT the system here is architecturally compliant, and what you're relying
> > upon something that the architecture doesn't guarantee, and Linux doesn't
> > guarantee.
>
> It is not also only our company's problem:
> To support the atomic instructions added in the Armv8.1 architecture, CHI-B
> provides Atomic Transactions. while Atomic Transactions support is also
> *optional* from CHI-B.
>
> So far atomic cannot fully supported by ARMv8.1 cpu + CHI-B system as well.
>
> from:
> https://developer.arm.com/documentation/102407/0100/Atomic-operations?lang=en
>
> So only cpu support atomic cannot garantee the system support lse atomic
> >
> > > Same kernel module wanted to be used by lse atomic fully support cpu and not
> > > fully support cpu/system as well.
> >
> > Which kernel modules *specifically* need to do atomics to non-cacheable memory?
> The driver want to always do far atomic(no speculatively) and allow a
> read-modify-write non-interruptible sequence in a single instruction.
That doesn't answer my question (you haven't told me what "the driver" is).
That doesn't explain why you need to use non-cachable memory for this.
Thanks,
Mark.
On 7/13/2023 7:20 PM, Mark Rutland wrote:
> On Thu, Jul 13, 2023 at 10:24:24AM +0800, Aiqun(Maria) Yu wrote:
>> On 7/12/2023 3:36 PM, Mark Rutland wrote:
>>> On Wed, Jul 12, 2023 at 11:09:10AM +0800, Aiqun(Maria) Yu wrote:
>>>> On 7/11/2023 6:25 PM, Will Deacon wrote:
>>>>> On Tue, Jul 11, 2023 at 06:15:49PM +0800, Aiqun(Maria) Yu wrote:
>>>>>> On 7/11/2023 4:22 PM, Will Deacon wrote:
>>>>>>> On Tue, Jul 11, 2023 at 12:02:22PM +0800, Aiqun(Maria) Yu wrote:
>>>>>>>> On 7/10/2023 5:37 PM, Will Deacon wrote:
>>>>>>>>> On Mon, Jul 10, 2023 at 01:59:55PM +0800, Maria Yu wrote:
>>>>>>>>>> In order to be able to disable lse_atomic even if cpu
>>>>>>>>>> support it, most likely because of memory controller
>>>>>>>>>> cannot deal with the lse atomic instructions, use a
>>>>>>>>>> new idreg override to deal with it.
>>>>>>>>>
>>>>>>>>> This should not be a problem for cacheable memory though, right?
>>>>>>>>>
>>>>>>>>> Given that Linux does not issue atomic operations to non-cacheable mappings,
>>>>>>>>> I'm struggling to see why there's a problem here.
>>>>>>>>
>>>>>>>> The lse atomic operation can be issued on non-cacheable mappings as well.
>>>>>>>> Even if it is cached data, with different CPUECTLR_EL1 setting, it can also
>>>>>>>> do far lse atomic operations.
>>>>>>>
>>>>>>> Please can you point me to the place in the kernel sources where this
>>>>>>> happens? The architecture doesn't guarantee that atomics to non-cacheable
>>>>>>> mappings will work, see "B2.2.6 Possible implementation restrictions on
>>>>>>> using atomic instructions". Linux, therefore, doesn't issue atomics
>>>>>>> to non-cacheable memory.
>>>>>>
>>>>>> We encounter the issue on third party kernel modules and third party apps
>>>>>> instead of linux kernel itself.
>>>>>
>>>>> Great, so there's nothing to do in the kernel then!
>>>>>
>>>>> The third party code needs to be modified not to use atomic instructions
>>>>> with non-cacheable mappings. No need to involve us with that.
>>>>
>>>>>> This is a tradeoff of performance and stability. Per my understanding,
>>>>>> options can be used to enable the lse_atomic to have the most performance
>>>>>> cared system, and disable the lse_atomic by stability cared most system.
>>>>>
>>>>> Where do livelock and starvation fit in with "stability"? Disabling LSE
>>>>> atomics for things like qspinlock and the scheduler just because of some
>>>>> badly written third-party code isn't much of a tradeoff.
>>>
>>>> We also have requirement to have cpus/system fully support lse atomic and
>>>> cpus/system not fully support lse atomic with a generic kernel image.
>>>
>>> Who *specifically* has this requirement (i.e. what does 'we' mean here)? The
>>
>> I can use other word to describe the requirement instead of "we".
>>
>> There is requirements like android google gki. It request different cpu arch
>> system to use same generic kernel Image.
>
> GKI requires the system to use the generic kernel image; GKI does not require
> supporting atomics to non-cacheable mappings.
GKI does not have to require atomics to non-cacheable mappings.
GKI requires LSE ATOMIC feature to be enabled by default.
And GKI requires runtime disable the current cpu lse atomic feature.
It was an old soc, We received stability issues and finally completely
disable lse atomic for the product when it is non-gki kernel.
>
> What I am asking is: who has the requirement to perform atomics to
> non-cacheable mappings?
>
>>> upstream kernel does not require that atomics work on non-cacheable memory, and
>>
>> The same issue the system can be down of lse atomic not supported for
>> cachable memory when there need far atomic.
>
> Are you saying that LSE atomics to *cacheable* mappings do not work on your
> system?
>
> Specifically, when using a Normal Inner-Shareable Inner-Writeback
> Outer-Writeback mapping, do the LSE atomics work or not work?
*cacheable* mapping have the LSE atomic is not working if far atomic is
performed.
>
>>> saying "The company I work for want this" doesn't change that.
>>>
>>> AFAICT the system here is architecturally compliant, and what you're relying
>>> upon something that the architecture doesn't guarantee, and Linux doesn't
>>> guarantee.
>>
>> It is not also only our company's problem:
>> To support the atomic instructions added in the Armv8.1 architecture, CHI-B
>> provides Atomic Transactions. while Atomic Transactions support is also
>> *optional* from CHI-B.
>>
>> So far atomic cannot fully supported by ARMv8.1 cpu + CHI-B system as well.
>>
>> from:
>> https://developer.arm.com/documentation/102407/0100/Atomic-operations?lang=en
>>
>> So only cpu support atomic cannot garantee the system support lse atomic
>>>
>>>> Same kernel module wanted to be used by lse atomic fully support cpu and not
>>>> fully support cpu/system as well.
>>>
>>> Which kernel modules *specifically* need to do atomics to non-cacheable memory?
>> The driver want to always do far atomic(no speculatively) and allow a
>> read-modify-write non-interruptible sequence in a single instruction.
>
> That doesn't answer my question (you haven't told me what "the driver" is).
The customers' third part drivers. Do you want to have the driver's
name? Or source code? The driver works well on current far atomic
supported systems.
Is it a reasonable action like this from your point of view like this?
The driver want to always do far atomic(no speculatively) and allow a
read-modify-write non-interruptible sequence in a single instruction?
There is also an example in below link that far atomic usage is allowed
and sometimes performs good than near atomic:
https://developer.arm.com/documentation/102407/0100/Atomic-operations?lang=en
>
> That doesn't explain why you need to use non-cachable memory for this.
I want to correct that not "I need to", it is the end user/third party
driver want to do far atomics.
And my intention here is to give the options to let end user decide they
can disable the lse atomic from their end.
With disablement, the benefit is that they can keep the same code(kind
of GKI from customer end) for far atomic supported/not supported systems.
>
> Thanks,
> Mark.
--
Thx and BRs,
Aiqun(Maria) Yu
On Thu, Jul 13, 2023 at 10:08:34PM +0800, Aiqun(Maria) Yu wrote:
> On 7/13/2023 7:20 PM, Mark Rutland wrote:
> > Are you saying that LSE atomics to *cacheable* mappings do not work on your
> > system?
> >
> > Specifically, when using a Normal Inner-Shareable Inner-Writeback
> > Outer-Writeback mapping, do the LSE atomics work or not work?
> *cacheable* mapping have the LSE atomic is not working if far atomic is
> performed.
Thanks for confirming; the fact that this doesn't work on *cacheable* memory is
definitely a major issue. I think everyone is confused here because of the
earlier mention of non-cachable accesses (which don't matter).
I know that some CPU implementations have EL3 control bits to force LSE atomics
to be performed near (e.g. in Cortex-A55, the CPUECTLR.ATOM control bits),
which would avoid the issue while still allowing the LSE atomics to be used.
If those can be configured in EL3 firmware, that would be a preferable
workaround.
Can you say which CPUs are integrated in this system? and/or can you check if
such control bits exist?
Thanks,
Mark.
On 7/14/2023 3:08 AM, Mark Rutland wrote:
> On Thu, Jul 13, 2023 at 10:08:34PM +0800, Aiqun(Maria) Yu wrote:
>> On 7/13/2023 7:20 PM, Mark Rutland wrote:
>>> Are you saying that LSE atomics to *cacheable* mappings do not work on your
>>> system?
>>>
>>> Specifically, when using a Normal Inner-Shareable Inner-Writeback
>>> Outer-Writeback mapping, do the LSE atomics work or not work?
>> *cacheable* mapping have the LSE atomic is not working if far atomic is
>> performed.
>
> Thanks for confirming; the fact that this doesn't work on *cacheable* memory is
> definitely a major issue. I think everyone is confused here because of the
> earlier mention of non-cachable accesses (which don't matter).
>
Maybe I can have the information collected in a summary to see if that
helps.
> I know that some CPU implementations have EL3 control bits to force LSE atomics
> to be performed near (e.g. in Cortex-A55, the CPUECTLR.ATOM control bits),
> which would avoid the issue while still allowing the LSE atomics to be used.
>
> If those can be configured in EL3 firmware, that would be a preferable
> workaround.
>
> Can you say which CPUs are integrated in this system? and/or can you check if
> such control bits exist?
We have CPUECTLR_EL1.ATOM bit can force LSE atomics to be perform near.
CPUECTLR_EL1 is also an option to EL1 kernel drivers to be configuarable.
Try to a detailed summarise of the whole discussions, anyone can ignore
some part if you are already know.
* Part 1: Solution for this issue.
While we still want to have options to let third party and end users
can have options:
1.Disable lse atomic cap.
2.*Disallow* far atomic by "CPUECTLR_EL1.atom force near atomic" and
non-cachable mappling for lse atomic only.
* Part 2: Why we need the solution
1. There is also some case far atomic is better performance than near
atomic. end user may still can still try to do allow far atomic.
while this driver is also use kerenl LSE ATOMIC macro, so it can be
running on cpu don't support lse atomic and cpu support lse atomic already.
while current system, cpu have feature register said lse atomic is
supported, but memory controller is not supported is currently not yet
supported.
2. cpu feature of lse atomic capbility can be controled via options for
the same image.
Can have GKI(generic kernel Image) + same third party drivers Images
support multi systems.
-- *New system* fully support lse atomic
-- *Intermidiate support system* which only have cpu support lse atomic,
but have memory control/bus don't support lse atomic.* (mainly issue are
discussed in this thread.)
-- *old system* have cpu don't have this cpu feature at all.
3. better for debugging purpose, it would be easier for verify if it is
this feature related or not.
4. *Disallow* from the developer side is not easy to control, expecially
when they have the same code working on *old system* or *new system*,
but failed on current *Intermidiate support system*.
>
> Thanks,
> Mark.
>
Thx for discussion in details.
--
Thx and BRs,
Aiqun(Maria) Yu
On Fri, Jul 14, 2023 at 09:56:27AM +0800, Aiqun(Maria) Yu wrote:
> On 7/14/2023 3:08 AM, Mark Rutland wrote:
> > On Thu, Jul 13, 2023 at 10:08:34PM +0800, Aiqun(Maria) Yu wrote:
> > > On 7/13/2023 7:20 PM, Mark Rutland wrote:
> > > > Are you saying that LSE atomics to *cacheable* mappings do not work on your
> > > > system?
> > > >
> > > > Specifically, when using a Normal Inner-Shareable Inner-Writeback
> > > > Outer-Writeback mapping, do the LSE atomics work or not work?
> > > *cacheable* mapping have the LSE atomic is not working if far atomic is
> > > performed.
> >
> > Thanks for confirming; the fact that this doesn't work on *cacheable* memory is
> > definitely a major issue. I think everyone is confused here because of the
> > earlier mention of non-cachable accesses (which don't matter).
> >
> Maybe I can have the information collected in a summary to see if that
> helps.
> > I know that some CPU implementations have EL3 control bits to force LSE atomics
> > to be performed near (e.g. in Cortex-A55, the CPUECTLR.ATOM control bits),
> > which would avoid the issue while still allowing the LSE atomics to be used.
> >
> > If those can be configured in EL3 firmware, that would be a preferable
> > workaround.
> >
> > Can you say which CPUs are integrated in this system? and/or can you check if
> > such control bits exist?
>
> We have CPUECTLR_EL1.ATOM bit can force LSE atomics to be perform near.
> CPUECTLR_EL1 is also an option to EL1 kernel drivers to be configuarable.
>
> Try to a detailed summarise of the whole discussions, anyone can ignore some
> part if you are already know.
>
> * Part 1: Solution for this issue.
> While we still want to have options to let third party and end users can
> have options:
> 1.Disable lse atomic cap.
> 2.*Disallow* far atomic by "CPUECTLR_EL1.atom force near atomic" and
> non-cachable mappling for lse atomic only.
Sorry, but this still isn't making sense to me. Which CPUs do you have on
this SoC?
My understanding of the CPUs from ARM is that LSE atomics are not supposed
to be sent to blocks that don't support them. That doesn't mean you have to
do everything near, however -- you can still execute them at e.g. L2.
For example, the Cortex-X1 TRM states:
| Atomic instructions to cacheable memory can be performed as either
| near atomics or far atomics, depending on where the cache line
| containing the data resides.
|
| When an instruction hits in the L1 data cache in a unique state, then
| it is performed as a near atomic in the L1 memory system. If the atomic
| operation misses in the L1 cache, or the line is shared with another
| core, then the atomic is sent as a far atomic on the core CHI interface.
|
| If the operation misses everywhere within the cluster, and the
| interconnect supports far atomics, then the atomic is passed on to the
| interconnect to perform the operation.
|
| When the operation hits anywhere inside the cluster, or when an
| interconnect does not support atomics, the L3 memory system performs
| the atomic operation. If the line is not already there, it allocates
| the line into the L3 cache. This depends on whether the DSU is configured
| with an L3 cache.
So something doesn't add up.
> * Part 2: Why we need the solution
> 1. There is also some case far atomic is better performance than near
> atomic. end user may still can still try to do allow far atomic.
> while this driver is also use kerenl LSE ATOMIC macro, so it can be running
> on cpu don't support lse atomic and cpu support lse atomic already.
> while current system, cpu have feature register said lse atomic is
> supported, but memory controller is not supported is currently not yet
> supported.
I think you're forgetting the fact that these instructions can be executed
by userspace, so the kernel option is completely bogus. If you're saying
that cacheable atomics can cause external aborts, then I can write an app
which will crash your device even if you've set this command line option.
Will
On 7/14/2023 4:23 PM, Will Deacon wrote:
> On Fri, Jul 14, 2023 at 09:56:27AM +0800, Aiqun(Maria) Yu wrote:
>> On 7/14/2023 3:08 AM, Mark Rutland wrote:
>>> On Thu, Jul 13, 2023 at 10:08:34PM +0800, Aiqun(Maria) Yu wrote:
>>>> On 7/13/2023 7:20 PM, Mark Rutland wrote:
>>>>> Are you saying that LSE atomics to *cacheable* mappings do not work on your
>>>>> system?
>>>>>
>>>>> Specifically, when using a Normal Inner-Shareable Inner-Writeback
>>>>> Outer-Writeback mapping, do the LSE atomics work or not work?
>>>> *cacheable* mapping have the LSE atomic is not working if far atomic is
>>>> performed.
>>>
>>> Thanks for confirming; the fact that this doesn't work on *cacheable* memory is
>>> definitely a major issue. I think everyone is confused here because of the
>>> earlier mention of non-cachable accesses (which don't matter).
>>>
>> Maybe I can have the information collected in a summary to see if that
>> helps.
>>> I know that some CPU implementations have EL3 control bits to force LSE atomics
>>> to be performed near (e.g. in Cortex-A55, the CPUECTLR.ATOM control bits),
>>> which would avoid the issue while still allowing the LSE atomics to be used.
>>>
>>> If those can be configured in EL3 firmware, that would be a preferable
>>> workaround.
>>>
>>> Can you say which CPUs are integrated in this system? and/or can you check if
>>> such control bits exist?
>>
>> We have CPUECTLR_EL1.ATOM bit can force LSE atomics to be perform near.
>> CPUECTLR_EL1 is also an option to EL1 kernel drivers to be configuarable.
>>
>> Try to a detailed summarise of the whole discussions, anyone can ignore some
>> part if you are already know.
>>
>> * Part 1: Solution for this issue.
>> While we still want to have options to let third party and end users can
>> have options:
>> 1.Disable lse atomic cap.
>> 2.*Disallow* far atomic by "CPUECTLR_EL1.atom force near atomic" and
>> non-cachable mappling for lse atomic only.
>
> Sorry, but this still isn't making sense to me. Which CPUs do you have on
> this SoC?
cpu is cortex A78/A55.
>
> My understanding of the CPUs from ARM is that LSE atomics are not supposed
> to be sent to blocks that don't support them. That doesn't mean you have to
> do everything near, however -- you can still execute them at e.g. L2.
>
> For example, the Cortex-X1 TRM states:
>
> | Atomic instructions to cacheable memory can be performed as either
> | near atomics or far atomics, depending on where the cache line
> | containing the data resides.
> |
> | When an instruction hits in the L1 data cache in a unique state, then
> | it is performed as a near atomic in the L1 memory system. If the atomic
> | operation misses in the L1 cache, or the line is shared with another
> | core, then the atomic is sent as a far atomic on the core CHI interface.
lse atomic is optional to CHI-B for example, some system may have cpu
feature register have lse atomic feature, but the far atomic is not
accpeted by CHI side. It will be simiar issue that we do.
> |
> | If the operation misses everywhere within the cluster, and the
> | interconnect supports far atomics, then the atomic is passed on to the
> | interconnect to perform the operation.
> |
> | When the operation hits anywhere inside the cluster, or when an
> | interconnect does not support atomics, the L3 memory system performs
> | the atomic operation. If the line is not already there, it allocates
> | the line into the L3 cache. This depends on whether the DSU is configured
> | with an L3 cache.
>
> So something doesn't add up.
>
>> * Part 2: Why we need the solution
>> 1. There is also some case far atomic is better performance than near
>> atomic. end user may still can still try to do allow far atomic.
>> while this driver is also use kerenl LSE ATOMIC macro, so it can be running
>> on cpu don't support lse atomic and cpu support lse atomic already.
>> while current system, cpu have feature register said lse atomic is
>> supported, but memory controller is not supported is currently not yet
>> supported.
>
> I think you're forgetting the fact that these instructions can be executed
> by userspace, so the kernel option is completely bogus. If you're saying
> that cacheable atomics can cause external aborts, then I can write an app
> which will crash your device even if you've set this command line option.
>
For apps like userspace also needed to check the system capbility as far
as I know.
> Will
--
Thx and BRs,
Aiqun(Maria) Yu
On Fri, Jul 14, 2023 at 06:12:02PM +0800, Aiqun(Maria) Yu wrote:
> On 7/14/2023 4:23 PM, Will Deacon wrote:
> > On Fri, Jul 14, 2023 at 09:56:27AM +0800, Aiqun(Maria) Yu wrote:
> > > Try to a detailed summarise of the whole discussions, anyone can ignore some
> > > part if you are already know.
> > >
> > > * Part 1: Solution for this issue.
> > > While we still want to have options to let third party and end users can
> > > have options:
> > > 1.Disable lse atomic cap.
> > > 2.*Disallow* far atomic by "CPUECTLR_EL1.atom force near atomic" and
> > > non-cachable mappling for lse atomic only.
> >
> > Sorry, but this still isn't making sense to me. Which CPUs do you have on
> > this SoC?
> cpu is cortex A78/A55.
> >
> > My understanding of the CPUs from ARM is that LSE atomics are not supposed
> > to be sent to blocks that don't support them. That doesn't mean you have to
> > do everything near, however -- you can still execute them at e.g. L2.
> >
> > For example, the Cortex-X1 TRM states:
> >
> > | Atomic instructions to cacheable memory can be performed as either
> > | near atomics or far atomics, depending on where the cache line
> > | containing the data resides.
> > |
> > | When an instruction hits in the L1 data cache in a unique state, then
> > | it is performed as a near atomic in the L1 memory system. If the atomic
> > | operation misses in the L1 cache, or the line is shared with another
> > | core, then the atomic is sent as a far atomic on the core CHI interface.
> lse atomic is optional to CHI-B for example, some system may have cpu
> feature register have lse atomic feature, but the far atomic is not accpeted
> by CHI side. It will be simiar issue that we do.
Again, that should not be a problem. Looking at the A55 TRM, it explicitly
says that atomics will be done in the L3 if the interconnect does not
support them. The A78 TRM doesn't talk about this at all, so I defer to
Mark (or anybody else from Arm) on how that works, but one might assume
that it does something similar to the other Arm cores.
> > > * Part 2: Why we need the solution
> > > 1. There is also some case far atomic is better performance than near
> > > atomic. end user may still can still try to do allow far atomic.
> > > while this driver is also use kerenl LSE ATOMIC macro, so it can be running
> > > on cpu don't support lse atomic and cpu support lse atomic already.
> > > while current system, cpu have feature register said lse atomic is
> > > supported, but memory controller is not supported is currently not yet
> > > supported.
> >
> > I think you're forgetting the fact that these instructions can be executed
> > by userspace, so the kernel option is completely bogus. If you're saying
> > that cacheable atomics can cause external aborts, then I can write an app
> > which will crash your device even if you've set this command line option.
> >
> For apps like userspace also needed to check the system capbility as far as
That's not something you can enforce, so a malicious application can easily
crash your system.
Will
On 7/14/2023 8:09 PM, Will Deacon wrote:
> On Fri, Jul 14, 2023 at 06:12:02PM +0800, Aiqun(Maria) Yu wrote:
>> On 7/14/2023 4:23 PM, Will Deacon wrote:
>>> On Fri, Jul 14, 2023 at 09:56:27AM +0800, Aiqun(Maria) Yu wrote:
>>>> Try to a detailed summarise of the whole discussions, anyone can ignore some
>>>> part if you are already know.
>>>>
>>>> * Part 1: Solution for this issue.
>>>> While we still want to have options to let third party and end users can
>>>> have options:
>>>> 1.Disable lse atomic cap.
>>>> 2.*Disallow* far atomic by "CPUECTLR_EL1.atom force near atomic" and
>>>> non-cachable mappling for lse atomic only.
>>>
>>> Sorry, but this still isn't making sense to me. Which CPUs do you have on
>>> this SoC?
>> cpu is cortex A78/A55.
>>>
>>> My understanding of the CPUs from ARM is that LSE atomics are not supposed
>>> to be sent to blocks that don't support them. That doesn't mean you have to
>>> do everything near, however -- you can still execute them at e.g. L2.
>>>
>>> For example, the Cortex-X1 TRM states:
>>>
>>> | Atomic instructions to cacheable memory can be performed as either
>>> | near atomics or far atomics, depending on where the cache line
>>> | containing the data resides.
>>> |
>>> | When an instruction hits in the L1 data cache in a unique state, then
>>> | it is performed as a near atomic in the L1 memory system. If the atomic
>>> | operation misses in the L1 cache, or the line is shared with another
>>> | core, then the atomic is sent as a far atomic on the core CHI interface.
>> lse atomic is optional to CHI-B for example, some system may have cpu
>> feature register have lse atomic feature, but the far atomic is not accpeted
>> by CHI side. It will be simiar issue that we do.
>
> Again, that should not be a problem. Looking at the A55 TRM, it explicitly
> says that atomics will be done in the L3 if the interconnect does not
> support them. The A78 TRM doesn't talk about this at all, so I defer to
We will check internally to see why it is not happened in current system
which have issue.
> Mark (or anybody else from Arm) on how that works, but one might assume
> that it does something similar to the other Arm cores.
I checked other Arm cores like A720 TRM. It seems the similar statement:
If the operation hits anywhere inside the cluster, or if an interconnect
does not support atomics, then the L3 memory system performs the atomic
operation. If the line is not already there, it allocates the line into
the L3 cache.
>
>>>> * Part 2: Why we need the solution
>>>> 1. There is also some case far atomic is better performance than near
>>>> atomic. end user may still can still try to do allow far atomic.
>>>> while this driver is also use kerenl LSE ATOMIC macro, so it can be running
>>>> on cpu don't support lse atomic and cpu support lse atomic already.
>>>> while current system, cpu have feature register said lse atomic is
>>>> supported, but memory controller is not supported is currently not yet
>>>> supported.
>>>
>>> I think you're forgetting the fact that these instructions can be executed
>>> by userspace, so the kernel option is completely bogus. If you're saying
>>> that cacheable atomics can cause external aborts, then I can write an app
>>> which will crash your device even if you've set this command line option.
>>>
>> For apps like userspace also needed to check the system capbility as far as
>
> That's not something you can enforce, so a malicious application can easily
> crash your system.
we provide the capability for sotfware to do the runtime compatible
check. we surely cannot enforce that. If the software is align the rule,
it can also funcational fine on the "intermediate support system".
By the way:
ALso the A720 TRM doc mentioned the uncachable memory case:
The Cortex-A720 core supports atomics to Device or Non-cacheable memory,
however this relies on the interconnect also supporting atomics. If such
an atomic instruction is executed when the interconnect does not support
them, then it results in an abort.
So it is not a single one company's scenario as well, shall we have an
option to handle this scenario?
Why the non-cacheable memory is forbiden? Does it only due to limitation
of hardware?
>
> Will
--
Thx and BRs,
Aiqun(Maria) Yu