2022-07-06 11:42:58

by Qi Hu

[permalink] [raw]
Subject: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu

From: Qi Hu <[email protected]>

The `vcsr` is not used anymore. Remove this member from `loongarch_fpu`.

From 3A5000(LoongArch), `vcsr` is removed in hardware. FP and LSX/LASX
both use `fcsr` as their csr.

Particularly, fcsr from $r16 to $r31 are reserved for LSX/LASX, an
using the registers in this area will cause SXD/ASXD if LSX/LASX is
not enabled.

Signed-off-by: Qi Hu <[email protected]>
---
V3:
- Modify commit message to conform to the format.
V2:
- Add more details in the commit message.
---
arch/loongarch/include/asm/fpregdef.h | 1 -
arch/loongarch/include/asm/processor.h | 2 --
arch/loongarch/kernel/asm-offsets.c | 1 -
arch/loongarch/kernel/fpu.S | 10 ----------
4 files changed, 14 deletions(-)

diff --git a/arch/loongarch/include/asm/fpregdef.h b/arch/loongarch/include/asm/fpregdef.h
index adb16e4b43b0..b6be527831dd 100644
--- a/arch/loongarch/include/asm/fpregdef.h
+++ b/arch/loongarch/include/asm/fpregdef.h
@@ -48,6 +48,5 @@
#define fcsr1 $r1
#define fcsr2 $r2
#define fcsr3 $r3
-#define vcsr16 $r16

#endif /* _ASM_FPREGDEF_H */
diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
index 1d63c934b289..57ec45aa078e 100644
--- a/arch/loongarch/include/asm/processor.h
+++ b/arch/loongarch/include/asm/processor.h
@@ -80,7 +80,6 @@ BUILD_FPR_ACCESS(64)

struct loongarch_fpu {
unsigned int fcsr;
- unsigned int vcsr;
uint64_t fcc; /* 8x8 */
union fpureg fpr[NUM_FPU_REGS];
};
@@ -161,7 +160,6 @@ struct thread_struct {
*/ \
.fpu = { \
.fcsr = 0, \
- .vcsr = 0, \
.fcc = 0, \
.fpr = {{{0,},},}, \
}, \
diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c
index bfb65eb2844f..20cd9e16a95a 100644
--- a/arch/loongarch/kernel/asm-offsets.c
+++ b/arch/loongarch/kernel/asm-offsets.c
@@ -166,7 +166,6 @@ void output_thread_fpu_defines(void)

OFFSET(THREAD_FCSR, loongarch_fpu, fcsr);
OFFSET(THREAD_FCC, loongarch_fpu, fcc);
- OFFSET(THREAD_VCSR, loongarch_fpu, vcsr);
BLANK();
}

diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
index 75c6ce0682a2..a631a7137667 100644
--- a/arch/loongarch/kernel/fpu.S
+++ b/arch/loongarch/kernel/fpu.S
@@ -146,16 +146,6 @@
movgr2fcsr fcsr0, \tmp0
.endm

- .macro sc_save_vcsr base, tmp0
- movfcsr2gr \tmp0, vcsr16
- EX st.w \tmp0, \base, 0
- .endm
-
- .macro sc_restore_vcsr base, tmp0
- EX ld.w \tmp0, \base, 0
- movgr2fcsr vcsr16, \tmp0
- .endm
-
/*
* Save a thread's fp context.
*/
--
2.36.1


2022-07-07 06:58:33

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu

Hi, all,

I have rewritten the commit message. If no problem, this patch will be
queued for loongarch-fixes.
--
The `vcsr` only exists in the old hardware design, it isn't used in any
shipped hardware from Loongson-3A5000 on. FPU and LSX/LASX both use the
`fcsr` as their control and status registers now. For example, the RM
control bit in fcsr0 is shared by FPU, LSX and LASX.

Particularly, fcsr16 to fcsr31 are reserved for LSX/LASX now, access to
these registers has no visible effect if LSX/LASX is enabled, and will
cause SXD/ASXD exceptions if LSX/LASX is not enabled.

So, mentions of vcsr are obsolete in the first place, let's remove them.
--
Huacai

On Wed, Jul 6, 2022 at 7:29 PM huqi <[email protected]> wrote:
>
> From: Qi Hu <[email protected]>
>
> The `vcsr` is not used anymore. Remove this member from `loongarch_fpu`.
>
> From 3A5000(LoongArch), `vcsr` is removed in hardware. FP and LSX/LASX
> both use `fcsr` as their csr.
>
> Particularly, fcsr from $r16 to $r31 are reserved for LSX/LASX, an
> using the registers in this area will cause SXD/ASXD if LSX/LASX is
> not enabled.
>
> Signed-off-by: Qi Hu <[email protected]>
> ---
> V3:
> - Modify commit message to conform to the format.
> V2:
> - Add more details in the commit message.
> ---
> arch/loongarch/include/asm/fpregdef.h | 1 -
> arch/loongarch/include/asm/processor.h | 2 --
> arch/loongarch/kernel/asm-offsets.c | 1 -
> arch/loongarch/kernel/fpu.S | 10 ----------
> 4 files changed, 14 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/fpregdef.h b/arch/loongarch/include/asm/fpregdef.h
> index adb16e4b43b0..b6be527831dd 100644
> --- a/arch/loongarch/include/asm/fpregdef.h
> +++ b/arch/loongarch/include/asm/fpregdef.h
> @@ -48,6 +48,5 @@
> #define fcsr1 $r1
> #define fcsr2 $r2
> #define fcsr3 $r3
> -#define vcsr16 $r16
>
> #endif /* _ASM_FPREGDEF_H */
> diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
> index 1d63c934b289..57ec45aa078e 100644
> --- a/arch/loongarch/include/asm/processor.h
> +++ b/arch/loongarch/include/asm/processor.h
> @@ -80,7 +80,6 @@ BUILD_FPR_ACCESS(64)
>
> struct loongarch_fpu {
> unsigned int fcsr;
> - unsigned int vcsr;
> uint64_t fcc; /* 8x8 */
> union fpureg fpr[NUM_FPU_REGS];
> };
> @@ -161,7 +160,6 @@ struct thread_struct {
> */ \
> .fpu = { \
> .fcsr = 0, \
> - .vcsr = 0, \
> .fcc = 0, \
> .fpr = {{{0,},},}, \
> }, \
> diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c
> index bfb65eb2844f..20cd9e16a95a 100644
> --- a/arch/loongarch/kernel/asm-offsets.c
> +++ b/arch/loongarch/kernel/asm-offsets.c
> @@ -166,7 +166,6 @@ void output_thread_fpu_defines(void)
>
> OFFSET(THREAD_FCSR, loongarch_fpu, fcsr);
> OFFSET(THREAD_FCC, loongarch_fpu, fcc);
> - OFFSET(THREAD_VCSR, loongarch_fpu, vcsr);
> BLANK();
> }
>
> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
> index 75c6ce0682a2..a631a7137667 100644
> --- a/arch/loongarch/kernel/fpu.S
> +++ b/arch/loongarch/kernel/fpu.S
> @@ -146,16 +146,6 @@
> movgr2fcsr fcsr0, \tmp0
> .endm
>
> - .macro sc_save_vcsr base, tmp0
> - movfcsr2gr \tmp0, vcsr16
> - EX st.w \tmp0, \base, 0
> - .endm
> -
> - .macro sc_restore_vcsr base, tmp0
> - EX ld.w \tmp0, \base, 0
> - movgr2fcsr vcsr16, \tmp0
> - .endm
> -
> /*
> * Save a thread's fp context.
> */
> --
> 2.36.1
>
>

2022-07-07 07:31:31

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu

On 2022/7/7 14:45, Huacai Chen wrote:
> Hi, all,
>
> I have rewritten the commit message. If no problem, this patch will be
> queued for loongarch-fixes.
> --
> The `vcsr` only exists in the old hardware design, it isn't used in any
> shipped hardware from Loongson-3A5000 on. FPU and LSX/LASX both use the
> `fcsr` as their control and status registers now. For example, the RM
> control bit in fcsr0 is shared by FPU, LSX and LASX.

FPU is a function unit while LSX/LASX are ISA extensions, they don't
belong to the same category.

"Scalar FP and LSX/LASX instructions all use ...", and "... is shared by
FP, LSX and LASX instructions." sound better.

> Particularly, fcsr16 to fcsr31 are reserved for LSX/LASX now, access to
> these registers has no visible effect if LSX/LASX is enabled, and will
> cause SXD/ASXD exceptions if LSX/LASX is not enabled.
>
> So, mentions of vcsr are obsolete in the first place, let's remove them.

Works for me and the wording is objective enough.

> --
> Huacai
>
> On Wed, Jul 6, 2022 at 7:29 PM huqi <[email protected]> wrote:
>> From: Qi Hu <[email protected]>
>>
>> The `vcsr` is not used anymore. Remove this member from `loongarch_fpu`.
>>
>> From 3A5000(LoongArch), `vcsr` is removed in hardware. FP and LSX/LASX
>> both use `fcsr` as their csr.
>>
>> Particularly, fcsr from $r16 to $r31 are reserved for LSX/LASX, an
>> using the registers in this area will cause SXD/ASXD if LSX/LASX is
>> not enabled.
>>
>> Signed-off-by: Qi Hu <[email protected]>
>> ---
>> V3:
>> - Modify commit message to conform to the format.
>> V2:
>> - Add more details in the commit message.
>> ---
>> arch/loongarch/include/asm/fpregdef.h | 1 -
>> arch/loongarch/include/asm/processor.h | 2 --
>> arch/loongarch/kernel/asm-offsets.c | 1 -
>> arch/loongarch/kernel/fpu.S | 10 ----------
>> 4 files changed, 14 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/fpregdef.h b/arch/loongarch/include/asm/fpregdef.h
>> index adb16e4b43b0..b6be527831dd 100644
>> --- a/arch/loongarch/include/asm/fpregdef.h
>> +++ b/arch/loongarch/include/asm/fpregdef.h
>> @@ -48,6 +48,5 @@
>> #define fcsr1 $r1
>> #define fcsr2 $r2
>> #define fcsr3 $r3
>> -#define vcsr16 $r16
>>
>> #endif /* _ASM_FPREGDEF_H */
>> diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
>> index 1d63c934b289..57ec45aa078e 100644
>> --- a/arch/loongarch/include/asm/processor.h
>> +++ b/arch/loongarch/include/asm/processor.h
>> @@ -80,7 +80,6 @@ BUILD_FPR_ACCESS(64)
>>
>> struct loongarch_fpu {
>> unsigned int fcsr;
>> - unsigned int vcsr;
>> uint64_t fcc; /* 8x8 */
>> union fpureg fpr[NUM_FPU_REGS];
>> };
>> @@ -161,7 +160,6 @@ struct thread_struct {
>> */ \
>> .fpu = { \
>> .fcsr = 0, \
>> - .vcsr = 0, \
>> .fcc = 0, \
>> .fpr = {{{0,},},}, \
>> }, \
>> diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c
>> index bfb65eb2844f..20cd9e16a95a 100644
>> --- a/arch/loongarch/kernel/asm-offsets.c
>> +++ b/arch/loongarch/kernel/asm-offsets.c
>> @@ -166,7 +166,6 @@ void output_thread_fpu_defines(void)
>>
>> OFFSET(THREAD_FCSR, loongarch_fpu, fcsr);
>> OFFSET(THREAD_FCC, loongarch_fpu, fcc);
>> - OFFSET(THREAD_VCSR, loongarch_fpu, vcsr);
>> BLANK();
>> }
>>
>> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
>> index 75c6ce0682a2..a631a7137667 100644
>> --- a/arch/loongarch/kernel/fpu.S
>> +++ b/arch/loongarch/kernel/fpu.S
>> @@ -146,16 +146,6 @@
>> movgr2fcsr fcsr0, \tmp0
>> .endm
>>
>> - .macro sc_save_vcsr base, tmp0
>> - movfcsr2gr \tmp0, vcsr16
>> - EX st.w \tmp0, \base, 0
>> - .endm
>> -
>> - .macro sc_restore_vcsr base, tmp0
>> - EX ld.w \tmp0, \base, 0
>> - movgr2fcsr vcsr16, \tmp0
>> - .endm
>> -
>> /*
>> * Save a thread's fp context.
>> */
>> --
>> 2.36.1
>>
>>
Apart from the syntactical nit:

Reviewed-by: WANG Xuerui <[email protected]>

2022-07-07 07:45:01

by Qi Hu

[permalink] [raw]
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu


On 2022/7/7 14:45, Huacai Chen wrote:
> Hi, all,
>
> I have rewritten the commit message. If no problem, this patch will be
> queued for loongarch-fixes.
> --
> The `vcsr` only exists in the old hardware design, it isn't used in any
> shipped hardware from Loongson-3A5000 on. FPU and LSX/LASX both use the
> `fcsr` as their control and status registers now. For example, the RM
> control bit in fcsr0 is shared by FPU, LSX and LASX.
>
> Particularly, fcsr16 to fcsr31 are reserved for LSX/LASX now, access to
> these registers has no visible effect if LSX/LASX is enabled, and will
> cause SXD/ASXD exceptions if LSX/LASX is not enabled.
>
> So, mentions of vcsr are obsolete in the first place, let's remove them.

Or simply say this?

Remove VCSR from "loongarch_fpu" which is used for debugging.
> --
> Huacai
>
> On Wed, Jul 6, 2022 at 7:29 PM huqi <[email protected]> wrote:
>> From: Qi Hu <[email protected]>
>>
>> The `vcsr` is not used anymore. Remove this member from `loongarch_fpu`.
>>
>> From 3A5000(LoongArch), `vcsr` is removed in hardware. FP and LSX/LASX
>> both use `fcsr` as their csr.
>>
>> Particularly, fcsr from $r16 to $r31 are reserved for LSX/LASX, an
>> using the registers in this area will cause SXD/ASXD if LSX/LASX is
>> not enabled.
>>
>> Signed-off-by: Qi Hu <[email protected]>
>> ---
>> V3:
>> - Modify commit message to conform to the format.
>> V2:
>> - Add more details in the commit message.
>> ---
>> arch/loongarch/include/asm/fpregdef.h | 1 -
>> arch/loongarch/include/asm/processor.h | 2 --
>> arch/loongarch/kernel/asm-offsets.c | 1 -
>> arch/loongarch/kernel/fpu.S | 10 ----------
>> 4 files changed, 14 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/fpregdef.h b/arch/loongarch/include/asm/fpregdef.h
>> index adb16e4b43b0..b6be527831dd 100644
>> --- a/arch/loongarch/include/asm/fpregdef.h
>> +++ b/arch/loongarch/include/asm/fpregdef.h
>> @@ -48,6 +48,5 @@
>> #define fcsr1 $r1
>> #define fcsr2 $r2
>> #define fcsr3 $r3
>> -#define vcsr16 $r16
>>
>> #endif /* _ASM_FPREGDEF_H */
>> diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
>> index 1d63c934b289..57ec45aa078e 100644
>> --- a/arch/loongarch/include/asm/processor.h
>> +++ b/arch/loongarch/include/asm/processor.h
>> @@ -80,7 +80,6 @@ BUILD_FPR_ACCESS(64)
>>
>> struct loongarch_fpu {
>> unsigned int fcsr;
>> - unsigned int vcsr;
>> uint64_t fcc; /* 8x8 */
>> union fpureg fpr[NUM_FPU_REGS];
>> };
>> @@ -161,7 +160,6 @@ struct thread_struct {
>> */ \
>> .fpu = { \
>> .fcsr = 0, \
>> - .vcsr = 0, \
>> .fcc = 0, \
>> .fpr = {{{0,},},}, \
>> }, \
>> diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c
>> index bfb65eb2844f..20cd9e16a95a 100644
>> --- a/arch/loongarch/kernel/asm-offsets.c
>> +++ b/arch/loongarch/kernel/asm-offsets.c
>> @@ -166,7 +166,6 @@ void output_thread_fpu_defines(void)
>>
>> OFFSET(THREAD_FCSR, loongarch_fpu, fcsr);
>> OFFSET(THREAD_FCC, loongarch_fpu, fcc);
>> - OFFSET(THREAD_VCSR, loongarch_fpu, vcsr);
>> BLANK();
>> }
>>
>> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
>> index 75c6ce0682a2..a631a7137667 100644
>> --- a/arch/loongarch/kernel/fpu.S
>> +++ b/arch/loongarch/kernel/fpu.S
>> @@ -146,16 +146,6 @@
>> movgr2fcsr fcsr0, \tmp0
>> .endm
>>
>> - .macro sc_save_vcsr base, tmp0
>> - movfcsr2gr \tmp0, vcsr16
>> - EX st.w \tmp0, \base, 0
>> - .endm
>> -
>> - .macro sc_restore_vcsr base, tmp0
>> - EX ld.w \tmp0, \base, 0
>> - movgr2fcsr vcsr16, \tmp0
>> - .endm
>> -
>> /*
>> * Save a thread's fp context.
>> */
>> --
>> 2.36.1
>>
>>

2022-07-07 08:05:38

by Xi Ruoyao

[permalink] [raw]
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu

On Thu, 2022-07-07 at 15:35 +0800, Qi Hu wrote:

> Remove VCSR from "loongarch_fpu" which is used for debugging.

"was" :). And it's better to at least add something like "before 3A5000
retailed".
>

--
Xi Ruoyao <[email protected]>
School of Aerospace Science and Technology, Xidian University

2022-07-07 08:47:06

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu



在2022年7月7日七月 上午8:35,Qi Hu写道:
> On 2022/7/7 14:45, Huacai Chen wrote:
>> Hi, all,
>>
>> I have rewritten the commit message. If no problem, this patch will be
>> queued for loongarch-fixes.
>> --
>> The `vcsr` only exists in the old hardware design, it isn't used in any
>> shipped hardware from Loongson-3A5000 on. FPU and LSX/LASX both use the
>> `fcsr` as their control and status registers now. For example, the RM
>> control bit in fcsr0 is shared by FPU, LSX and LASX.
>>
>> Particularly, fcsr16 to fcsr31 are reserved for LSX/LASX now, access to
>> these registers has no visible effect if LSX/LASX is enabled, and will
>> cause SXD/ASXD exceptions if LSX/LASX is not enabled.
>>
>> So, mentions of vcsr are obsolete in the first place, let's remove them.
>
> Or simply say this?
>
> Remove VCSR from "loongarch_fpu" which is used for debugging.

Then you it will be a cross talk vulnerability if you simply ignore it in kernel.

Processes can create a covert channel for surreptitious data exchange based on VCSR like m1racles[1] for apple m1.

Thanks
[1]: https://m1racles.com/

>> --
>> Huacai
>>
>> On Wed, Jul 6, 2022 at 7:29 PM huqi <[email protected]> wrote:
>>> From: Qi Hu <[email protected]>
>>>
>>> The `vcsr` is not used anymore. Remove this member from `loongarch_fpu`.
>>>
>>> From 3A5000(LoongArch), `vcsr` is removed in hardware. FP and LSX/LASX
>>> both use `fcsr` as their csr.
>>>
>>> Particularly, fcsr from $r16 to $r31 are reserved for LSX/LASX, an
>>> using the registers in this area will cause SXD/ASXD if LSX/LASX is
>>> not enabled.
>>>
>>> Signed-off-by: Qi Hu <[email protected]>
>>> ---
>>> V3:
>>> - Modify commit message to conform to the format.
>>> V2:
>>> - Add more details in the commit message.
>>> ---
>>> arch/loongarch/include/asm/fpregdef.h | 1 -
>>> arch/loongarch/include/asm/processor.h | 2 --
>>> arch/loongarch/kernel/asm-offsets.c | 1 -
>>> arch/loongarch/kernel/fpu.S | 10 ----------
>>> 4 files changed, 14 deletions(-)
>>>
>>> diff --git a/arch/loongarch/include/asm/fpregdef.h b/arch/loongarch/include/asm/fpregdef.h
>>> index adb16e4b43b0..b6be527831dd 100644
>>> --- a/arch/loongarch/include/asm/fpregdef.h
>>> +++ b/arch/loongarch/include/asm/fpregdef.h
>>> @@ -48,6 +48,5 @@
>>> #define fcsr1 $r1
>>> #define fcsr2 $r2
>>> #define fcsr3 $r3
>>> -#define vcsr16 $r16
>>>
>>> #endif /* _ASM_FPREGDEF_H */
>>> diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
>>> index 1d63c934b289..57ec45aa078e 100644
>>> --- a/arch/loongarch/include/asm/processor.h
>>> +++ b/arch/loongarch/include/asm/processor.h
>>> @@ -80,7 +80,6 @@ BUILD_FPR_ACCESS(64)
>>>
>>> struct loongarch_fpu {
>>> unsigned int fcsr;
>>> - unsigned int vcsr;
>>> uint64_t fcc; /* 8x8 */
>>> union fpureg fpr[NUM_FPU_REGS];
>>> };
>>> @@ -161,7 +160,6 @@ struct thread_struct {
>>> */ \
>>> .fpu = { \
>>> .fcsr = 0, \
>>> - .vcsr = 0, \
>>> .fcc = 0, \
>>> .fpr = {{{0,},},}, \
>>> }, \
>>> diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c
>>> index bfb65eb2844f..20cd9e16a95a 100644
>>> --- a/arch/loongarch/kernel/asm-offsets.c
>>> +++ b/arch/loongarch/kernel/asm-offsets.c
>>> @@ -166,7 +166,6 @@ void output_thread_fpu_defines(void)
>>>
>>> OFFSET(THREAD_FCSR, loongarch_fpu, fcsr);
>>> OFFSET(THREAD_FCC, loongarch_fpu, fcc);
>>> - OFFSET(THREAD_VCSR, loongarch_fpu, vcsr);
>>> BLANK();
>>> }
>>>
>>> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
>>> index 75c6ce0682a2..a631a7137667 100644
>>> --- a/arch/loongarch/kernel/fpu.S
>>> +++ b/arch/loongarch/kernel/fpu.S
>>> @@ -146,16 +146,6 @@
>>> movgr2fcsr fcsr0, \tmp0
>>> .endm
>>>
>>> - .macro sc_save_vcsr base, tmp0
>>> - movfcsr2gr \tmp0, vcsr16
>>> - EX st.w \tmp0, \base, 0
>>> - .endm
>>> -
>>> - .macro sc_restore_vcsr base, tmp0
>>> - EX ld.w \tmp0, \base, 0
>>> - movgr2fcsr vcsr16, \tmp0
>>> - .endm
>>> -
>>> /*
>>> * Save a thread's fp context.
>>> */
>>> --
>>> 2.36.1
>>>
>>>

--
- Jiaxun

2022-07-07 09:46:23

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu


On 2022/7/7 16:30, Jiaxun Yang wrote:
>
> 在2022年7月7日七月 上午8:35,Qi Hu写道:
>> On 2022/7/7 14:45, Huacai Chen wrote:
>>> Hi, all,
>>>
>>> I have rewritten the commit message. If no problem, this patch will be
>>> queued for loongarch-fixes.
>>> --
>>> The `vcsr` only exists in the old hardware design, it isn't used in any
>>> shipped hardware from Loongson-3A5000 on. FPU and LSX/LASX both use the
>>> `fcsr` as their control and status registers now. For example, the RM
>>> control bit in fcsr0 is shared by FPU, LSX and LASX.
>>>
>>> Particularly, fcsr16 to fcsr31 are reserved for LSX/LASX now, access to
>>> these registers has no visible effect if LSX/LASX is enabled, and will
>>> cause SXD/ASXD exceptions if LSX/LASX is not enabled.
>>>
>>> So, mentions of vcsr are obsolete in the first place, let's remove them.
>> Or simply say this?
>>
>> Remove VCSR from "loongarch_fpu" which is used for debugging.
> Then you it will be a cross talk vulnerability if you simply ignore it in kernel.
>
> Processes can create a covert channel for surreptitious data exchange based on VCSR like m1racles[1] for apple m1.

Hmm this seems to not be the case; according to Ruoyao's experiment
results, writes to FCSR16 are ignored and reads return all-zeros, hence
no risk of creating a covert channel by this cleanup.

However, the experiment result is counter-intuitive to me after all; are
you aware that perhaps *some* 3A5000's in the wild actually have
functional VCSR? If so, the state obviously needs isolation
(saving/restoring simply becoming no-ops on later steppings), but
otherwise this cleanup should be correct.

>
> Thanks
> [1]: https://m1racles.com/
>
>>> --
>>> Huacai
>>>
>>> On Wed, Jul 6, 2022 at 7:29 PM huqi <[email protected]> wrote:
>>>> From: Qi Hu <[email protected]>
>>>>
>>>> The `vcsr` is not used anymore. Remove this member from `loongarch_fpu`.
>>>>
>>>> From 3A5000(LoongArch), `vcsr` is removed in hardware. FP and LSX/LASX
>>>> both use `fcsr` as their csr.
>>>>
>>>> Particularly, fcsr from $r16 to $r31 are reserved for LSX/LASX, an
>>>> using the registers in this area will cause SXD/ASXD if LSX/LASX is
>>>> not enabled.
>>>>
>>>> Signed-off-by: Qi Hu <[email protected]>
>>>> ---
>>>> V3:
>>>> - Modify commit message to conform to the format.
>>>> V2:
>>>> - Add more details in the commit message.
>>>> ---
>>>> arch/loongarch/include/asm/fpregdef.h | 1 -
>>>> arch/loongarch/include/asm/processor.h | 2 --
>>>> arch/loongarch/kernel/asm-offsets.c | 1 -
>>>> arch/loongarch/kernel/fpu.S | 10 ----------
>>>> 4 files changed, 14 deletions(-)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/fpregdef.h b/arch/loongarch/include/asm/fpregdef.h
>>>> index adb16e4b43b0..b6be527831dd 100644
>>>> --- a/arch/loongarch/include/asm/fpregdef.h
>>>> +++ b/arch/loongarch/include/asm/fpregdef.h
>>>> @@ -48,6 +48,5 @@
>>>> #define fcsr1 $r1
>>>> #define fcsr2 $r2
>>>> #define fcsr3 $r3
>>>> -#define vcsr16 $r16
>>>>
>>>> #endif /* _ASM_FPREGDEF_H */
>>>> diff --git a/arch/loongarch/include/asm/processor.h b/arch/loongarch/include/asm/processor.h
>>>> index 1d63c934b289..57ec45aa078e 100644
>>>> --- a/arch/loongarch/include/asm/processor.h
>>>> +++ b/arch/loongarch/include/asm/processor.h
>>>> @@ -80,7 +80,6 @@ BUILD_FPR_ACCESS(64)
>>>>
>>>> struct loongarch_fpu {
>>>> unsigned int fcsr;
>>>> - unsigned int vcsr;
>>>> uint64_t fcc; /* 8x8 */
>>>> union fpureg fpr[NUM_FPU_REGS];
>>>> };
>>>> @@ -161,7 +160,6 @@ struct thread_struct {
>>>> */ \
>>>> .fpu = { \
>>>> .fcsr = 0, \
>>>> - .vcsr = 0, \
>>>> .fcc = 0, \
>>>> .fpr = {{{0,},},}, \
>>>> }, \
>>>> diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c
>>>> index bfb65eb2844f..20cd9e16a95a 100644
>>>> --- a/arch/loongarch/kernel/asm-offsets.c
>>>> +++ b/arch/loongarch/kernel/asm-offsets.c
>>>> @@ -166,7 +166,6 @@ void output_thread_fpu_defines(void)
>>>>
>>>> OFFSET(THREAD_FCSR, loongarch_fpu, fcsr);
>>>> OFFSET(THREAD_FCC, loongarch_fpu, fcc);
>>>> - OFFSET(THREAD_VCSR, loongarch_fpu, vcsr);
>>>> BLANK();
>>>> }
>>>>
>>>> diff --git a/arch/loongarch/kernel/fpu.S b/arch/loongarch/kernel/fpu.S
>>>> index 75c6ce0682a2..a631a7137667 100644
>>>> --- a/arch/loongarch/kernel/fpu.S
>>>> +++ b/arch/loongarch/kernel/fpu.S
>>>> @@ -146,16 +146,6 @@
>>>> movgr2fcsr fcsr0, \tmp0
>>>> .endm
>>>>
>>>> - .macro sc_save_vcsr base, tmp0
>>>> - movfcsr2gr \tmp0, vcsr16
>>>> - EX st.w \tmp0, \base, 0
>>>> - .endm
>>>> -
>>>> - .macro sc_restore_vcsr base, tmp0
>>>> - EX ld.w \tmp0, \base, 0
>>>> - movgr2fcsr vcsr16, \tmp0
>>>> - .endm
>>>> -
>>>> /*
>>>> * Save a thread's fp context.
>>>> */
>>>> --
>>>> 2.36.1
>>>>
>>>>

2022-07-07 10:29:16

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH v3] LoongArch: Remove vcsr in loongarch_fpu



在 2022/7/7 9:56, WANG Xuerui 写道:
>
> However, the experiment result is counter-intuitive to me after all;
> are you aware that perhaps *some* 3A5000's in the wild actually have
> functional VCSR? If so, the state obviously needs isolation
> (saving/restoring simply becoming no-ops on later steppings), but
> otherwise this cleanup should be correct.
^ Yep, my concern was 3A5000 with VCSR enabled can be vulnerable.

Thanks
- Jiaxun