Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
On Wed, 2014-01-22 at 20:49 -0600, Wang Dongsheng-B40534 wrote: The whole point of calling enable_kernel_fp() in C code before suspending is to ensure that the FP state gets saved. If FP is used after that point it is a bug. If you're worried about such bugs, then clear MSR[FP] after calling enable_kernel_fp(), rather than adding redundant state saving. enable_kernel_fp() calling in MEM suspend flow. Hibernation is different with MEM suspend, and I'm not sure where will call this interface, so we need to ensure the integrity of the core saving. I don't think this code is *redundant*. I trust that the kernel can keep the FP related operations, that's why a judgment is here. :) For hibernation, save_processor_state() is called first, which does flush_fp_to_thread() which has a similar effect (though I wonder if it's being called on the correct task for non-SMP). Yes, thanks, I miss this code.:) But I still think we need to keep this judgment, because i provide an API. If you still insist on I can remove *FP*, but I don't want to do this..:) I insist. Redundant code wastes review and maintenance bandwidth, and is a potential source of bugs. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
On Mon, 2014-01-20 at 20:43 -0600, Wang Dongsheng-B40534 wrote: -Original Message- From: Wood Scott-B07421 Sent: Tuesday, January 21, 2014 9:06 AM To: Wang Dongsheng-B40534 Cc: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers On Mon, 2014-01-20 at 00:03 -0600, Wang Dongsheng-B40534 wrote: + /* + * Need to save float-point registers if MSR[FP] = 1. + */ + mfmsr r12 + andi. r12, r12, MSR_FP + beq 1f + do_sr_fpr_regs(save) C code should have already ensured that MSR[FP] is not 1 (and thus the FP context has been saved). Yes, right. But I mean if the FP still use in core save flow, we need to save it. In this process, i don't care what other code do, we need to focus on not losing valuable data. It is not allowed to use FP at that point. If MSR[FP] not active, that is FP not allowed to use. But here is a normal judgment, if MSR[FP] is active, this means that the floating point module is being used. I offer is a function of the interface, we don't know where is the function will be called. Just because we call this function in the context of uncertainty, we need this judgment to ensure that no data is lost. The whole point of calling enable_kernel_fp() in C code before suspending is to ensure that the FP state gets saved. If FP is used after that point it is a bug. If you're worried about such bugs, then clear MSR[FP] after calling enable_kernel_fp(), rather than adding redundant state saving. enable_kernel_fp() calling in MEM suspend flow. Hibernation is different with MEM suspend, and I'm not sure where will call this interface, so we need to ensure the integrity of the core saving. I don't think this code is *redundant*. I trust that the kernel can keep the FP related operations, that's why a judgment is here. :) For hibernation, save_processor_state() is called first, which does flush_fp_to_thread() which has a similar effect (though I wonder if it's being called on the correct task for non-SMP). -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
The whole point of calling enable_kernel_fp() in C code before suspending is to ensure that the FP state gets saved. If FP is used after that point it is a bug. If you're worried about such bugs, then clear MSR[FP] after calling enable_kernel_fp(), rather than adding redundant state saving. enable_kernel_fp() calling in MEM suspend flow. Hibernation is different with MEM suspend, and I'm not sure where will call this interface, so we need to ensure the integrity of the core saving. I don't think this code is *redundant*. I trust that the kernel can keep the FP related operations, that's why a judgment is here. :) For hibernation, save_processor_state() is called first, which does flush_fp_to_thread() which has a similar effect (though I wonder if it's being called on the correct task for non-SMP). Yes, thanks, I miss this code.:) But I still think we need to keep this judgment, because i provide an API. If you still insist on I can remove *FP*, but I don't want to do this..:) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
On Mon, 2014-01-20 at 00:03 -0600, Wang Dongsheng-B40534 wrote: + /* + * Need to save float-point registers if MSR[FP] = 1. + */ + mfmsr r12 + andi. r12, r12, MSR_FP + beq 1f + do_sr_fpr_regs(save) C code should have already ensured that MSR[FP] is not 1 (and thus the FP context has been saved). Yes, right. But I mean if the FP still use in core save flow, we need to save it. In this process, i don't care what other code do, we need to focus on not losing valuable data. It is not allowed to use FP at that point. If MSR[FP] not active, that is FP not allowed to use. But here is a normal judgment, if MSR[FP] is active, this means that the floating point module is being used. I offer is a function of the interface, we don't know where is the function will be called. Just because we call this function in the context of uncertainty, we need this judgment to ensure that no data is lost. The whole point of calling enable_kernel_fp() in C code before suspending is to ensure that the FP state gets saved. If FP is used after that point it is a bug. If you're worried about such bugs, then clear MSR[FP] after calling enable_kernel_fp(), rather than adding redundant state saving. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
-Original Message- From: Wood Scott-B07421 Sent: Tuesday, January 21, 2014 9:06 AM To: Wang Dongsheng-B40534 Cc: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers On Mon, 2014-01-20 at 00:03 -0600, Wang Dongsheng-B40534 wrote: + /* +* Need to save float-point registers if MSR[FP] = 1. +*/ + mfmsr r12 + andi. r12, r12, MSR_FP + beq 1f + do_sr_fpr_regs(save) C code should have already ensured that MSR[FP] is not 1 (and thus the FP context has been saved). Yes, right. But I mean if the FP still use in core save flow, we need to save it. In this process, i don't care what other code do, we need to focus on not losing valuable data. It is not allowed to use FP at that point. If MSR[FP] not active, that is FP not allowed to use. But here is a normal judgment, if MSR[FP] is active, this means that the floating point module is being used. I offer is a function of the interface, we don't know where is the function will be called. Just because we call this function in the context of uncertainty, we need this judgment to ensure that no data is lost. The whole point of calling enable_kernel_fp() in C code before suspending is to ensure that the FP state gets saved. If FP is used after that point it is a bug. If you're worried about such bugs, then clear MSR[FP] after calling enable_kernel_fp(), rather than adding redundant state saving. enable_kernel_fp() calling in MEM suspend flow. Hibernation is different with MEM suspend, and I'm not sure where will call this interface, so we need to ensure the integrity of the core saving. I don't think this code is *redundant*. I trust that the kernel can keep the FP related operations, that's why a judgment is here. :) Thanks, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
What is there that is specfic to a particular core type that can't be handled from C code? In the context of the calling, maybe not in C environment.(Deep sleep without C environment when calling those interfaces) Could you provide a concrete example? :) Deep sleep, the patches will comes out soon. + /* +* Need to save float-point registers if MSR[FP] = 1. +*/ + mfmsr r12 + andi. r12, r12, MSR_FP + beq 1f + do_sr_fpr_regs(save) C code should have already ensured that MSR[FP] is not 1 (and thus the FP context has been saved). Yes, right. But I mean if the FP still use in core save flow, we need to save it. In this process, i don't care what other code do, we need to focus on not losing valuable data. It is not allowed to use FP at that point. If MSR[FP] not active, that is FP not allowed to use. But here is a normal judgment, if MSR[FP] is active, this means that the floating point module is being used. I offer is a function of the interface, we don't know where is the function will be called. Just because we call this function in the context of uncertainty, we need this judgment to ensure that no data is lost. Thanks, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
On Tue, 2014-01-14 at 21:30 -0600, Wang Dongsheng-B40534 wrote: -Original Message- From: Wood Scott-B07421 Sent: Wednesday, January 15, 2014 7:51 AM To: Wang Dongsheng-B40534 Cc: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote: From: Wang Dongsheng dongsheng.w...@freescale.com Add fsl_cpu_state_save/fsl_cpu_state_restore functions, used for deep sleep and hibernation to save/restore core registers. We abstract out save/restore code for use in various modules, to make them don't need to maintain. Currently supported processors type are E6500, E5500, E500MC, E500v2 and E500v1. Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com What is there that is specfic to a particular core type that can't be handled from C code? In the context of the calling, maybe not in C environment.(Deep sleep without C environment when calling those interfaces) Could you provide a concrete example? + /* + * Need to save float-point registers if MSR[FP] = 1. + */ + mfmsr r12 + andi. r12, r12, MSR_FP + beq 1f + do_sr_fpr_regs(save) C code should have already ensured that MSR[FP] is not 1 (and thus the FP context has been saved). Yes, right. But I mean if the FP still use in core save flow, we need to save it. In this process, i don't care what other code do, we need to focus on not losing valuable data. It is not allowed to use FP at that point. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote: From: Wang Dongsheng dongsheng.w...@freescale.com Add fsl_cpu_state_save/fsl_cpu_state_restore functions, used for deep sleep and hibernation to save/restore core registers. We abstract out save/restore code for use in various modules, to make them don't need to maintain. Currently supported processors type are E6500, E5500, E500MC, E500v2 and E500v1. Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com What is there that is specfic to a particular core type that can't be handled from C code? + /* + * Need to save float-point registers if MSR[FP] = 1. + */ + mfmsr r12 + andi. r12, r12, MSR_FP + beq 1f + do_sr_fpr_regs(save) C code should have already ensured that MSR[FP] is not 1 (and thus the FP context has been saved). +/* + * r3 = the virtual address of buffer + * r4 = suspend type, 0-BASE_SAVE, 1-ALL_SAVE #define these magic numbers, and define what is meant by base save versus all save. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, January 15, 2014 7:51 AM To: Wang Dongsheng-B40534 Cc: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote: From: Wang Dongsheng dongsheng.w...@freescale.com Add fsl_cpu_state_save/fsl_cpu_state_restore functions, used for deep sleep and hibernation to save/restore core registers. We abstract out save/restore code for use in various modules, to make them don't need to maintain. Currently supported processors type are E6500, E5500, E500MC, E500v2 and E500v1. Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com What is there that is specfic to a particular core type that can't be handled from C code? In the context of the calling, maybe not in C environment.(Deep sleep without C environment when calling those interfaces) + /* +* Need to save float-point registers if MSR[FP] = 1. +*/ + mfmsr r12 + andi. r12, r12, MSR_FP + beq 1f + do_sr_fpr_regs(save) C code should have already ensured that MSR[FP] is not 1 (and thus the FP context has been saved). Yes, right. But I mean if the FP still use in core save flow, we need to save it. In this process, i don't care what other code do, we need to focus on not losing valuable data. +/* + * r3 = the virtual address of buffer + * r4 = suspend type, 0-BASE_SAVE, 1-ALL_SAVE #define these magic numbers, and define what is meant by base save versus all save. Ok, thanks. -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
