* [PATCH v4] powerpc: Export thread_struct.used_vr/used_vsr to user space @ 2016-07-07 7:49 wei.guo.simon 2016-07-07 11:15 ` [v4] " Michael Ellerman 0 siblings, 1 reply; 10+ messages in thread From: wei.guo.simon @ 2016-07-07 7:49 UTC (permalink / raw) To: Michael Ellerman Cc: Simon Guo, Benjamin Herrenschmidt, Paul Mackerras, Kees Cook, Rashmica Gupta, linuxppc-dev, linux-kernel, Laurent Dufour From: Simon Guo <wei.guo.simon@gmail.com> These 2 fields track whether user process has used Altivec/VSX registers or not. They are used by kernel to setup signal frame on user stack correctly regarding vector part. CRIU(Checkpoint and Restore In User space) builds signal frame for restored process. It will need this export information to setup signal frame correctly. And CRIU will need to restore these 2 fields for the restored process. Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Kees Cook <keescook@chromium.org> Cc: Rashmica Gupta <rashmicy@gmail.com> Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org Cc: Laurent Dufour <ldufour@linux.vnet.ibm.com> Signed-off-by: Simon Guo <wei.guo.simon@gmail.com> Reviewed-by: Laurent Dufour <ldufour@linux.vnet.ibm.com> -- v3 -> v4: - Copying 64 bit data from/to user space using get_user/put_user is not supported on all architectures, and may result in the following build error on 32 bits build: arch/powerpc/kernel/built-in.o: In function `arch_ptrace': >> (.text+0xad4): undefined reference to `__get_user_bad' using copy_from_user/copy_to_user instead. v2 -> v3: - enlarge reg_usage from 32 to 64 bits - prefix ptrace API with PPC_ v1 -> v2: - minor change for coding style --- arch/powerpc/include/uapi/asm/ptrace.h | 11 ++++++++ arch/powerpc/kernel/ptrace.c | 48 ++++++++++++++++++++++++++++++++++ arch/powerpc/kernel/ptrace32.c | 2 ++ 3 files changed, 61 insertions(+) diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h index 8036b38..b357677 100644 --- a/arch/powerpc/include/uapi/asm/ptrace.h +++ b/arch/powerpc/include/uapi/asm/ptrace.h @@ -176,6 +176,17 @@ struct pt_regs { #define PTRACE_GETREGS64 0x16 #define PTRACE_SETREGS64 0x17 +/* + * Get or set some register used bit. + * The flags will be saved in a 64 bit data. + * Currently it is only used for VR/VSR usage. + */ +#define PPC_PTRACE_GET_REGS_USAGE 0x97 +#define PPC_PTRACE_SET_REGS_USAGE 0x96 + +#define PPC_PTRACE_REGS_USAGE_VR_BIT 0x01UL +#define PPC_PTRACE_REGS_USAGE_VSR_BIT 0x02UL + /* Calls to trace a 64bit program from a 32bit program */ #define PPC_PTRACE_PEEKTEXT_3264 0x95 #define PPC_PTRACE_PEEKDATA_3264 0x94 diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index a9aa2a5..d00d7c0 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3018,6 +3018,54 @@ long arch_ptrace(struct task_struct *child, long request, REGSET_SPE, 0, 35 * sizeof(u32), datavp); #endif + case PPC_PTRACE_GET_REGS_USAGE: + { + u64 *u64_datap = (u64 *)datavp; + u64 reg_usage = 0; + + if (addr != sizeof(u64)) + return -EINVAL; + +#ifdef CONFIG_ALTIVEC + if (child->thread.used_vr) + reg_usage |= PPC_PTRACE_REGS_USAGE_VR_BIT; +#endif +#ifdef CONFIG_VSX + if (child->thread.used_vsr) + reg_usage |= PPC_PTRACE_REGS_USAGE_VSR_BIT; +#endif + ret = copy_to_user(u64_datap, + ®_usage, + sizeof(reg_usage)) ? + -EFAULT : 0; + break; + } + + case PPC_PTRACE_SET_REGS_USAGE: + { + u64 *u64_datap = (u64 *)datavp; + u64 reg_usage = 0; + + if (addr != sizeof(u64)) + return -EINVAL; + + ret = copy_from_user(®_usage, + u64_datap, + sizeof(reg_usage)) ? + -EFAULT : 0; + + if (ret) + return ret; +#ifdef CONFIG_ALTIVEC + child->thread.used_vr = + !!(reg_usage & PPC_PTRACE_REGS_USAGE_VR_BIT); +#endif +#ifdef CONFIG_VSX + child->thread.used_vsr = + !!(reg_usage & PPC_PTRACE_REGS_USAGE_VSR_BIT); +#endif + break; + } default: ret = ptrace_request(child, request, addr, data); diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c index f52b7db..3aaa773 100644 --- a/arch/powerpc/kernel/ptrace32.c +++ b/arch/powerpc/kernel/ptrace32.c @@ -305,6 +305,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request, case PPC_PTRACE_GETHWDBGINFO: case PPC_PTRACE_SETHWDEBUG: case PPC_PTRACE_DELHWDEBUG: + case PPC_PTRACE_GET_REGS_USAGE: + case PPC_PTRACE_SET_REGS_USAGE: ret = arch_ptrace(child, request, addr, data); break; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space 2016-07-07 7:49 [PATCH v4] powerpc: Export thread_struct.used_vr/used_vsr to user space wei.guo.simon @ 2016-07-07 11:15 ` Michael Ellerman 2016-07-07 13:12 ` Laurent Dufour 0 siblings, 1 reply; 10+ messages in thread From: Michael Ellerman @ 2016-07-07 11:15 UTC (permalink / raw) To: Simon Guo Cc: Kees Cook, Simon Guo, Rashmica Gupta, linux-kernel, Paul Mackerras, Laurent Dufour, linuxppc-dev On Thu, 2016-07-07 at 07:49:36 UTC, Simon Guo wrote: > From: Simon Guo <wei.guo.simon@gmail.com> > > These 2 fields track whether user process has used Altivec/VSX > registers or not. They are used by kernel to setup signal frame > on user stack correctly regarding vector part. I still dislike this. It's just exporting internal kernel state, which I know is the point. And I'd still like to know why we're the only arch that needs to do this. I'm not saying I won't merge it, but I'd like to understand it better first. > CRIU(Checkpoint and Restore In User space) builds signal frame > for restored process. It will need this export information to > setup signal frame correctly. And CRIU will need to restore these > 2 fields for the restored process. I don't really know how CRIU works, but .. Does the kernel write a sigframe for the process that's being checkpointed? If so can't you infer the state of the bits based on what was written? Alternately, when restoring, can you setup the sigframe with the Altivec/VSX fields populated, and the kernel will then load them, regardless of whether they were actually used or not prior to the checkpoint? cheers ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space 2016-07-07 11:15 ` [v4] " Michael Ellerman @ 2016-07-07 13:12 ` Laurent Dufour 2016-07-07 13:21 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 10+ messages in thread From: Laurent Dufour @ 2016-07-07 13:12 UTC (permalink / raw) To: Michael Ellerman, Simon Guo Cc: Kees Cook, Rashmica Gupta, linux-kernel, Paul Mackerras, linuxppc-dev On 07/07/2016 13:15, Michael Ellerman wrote: > On Thu, 2016-07-07 at 07:49:36 UTC, Simon Guo wrote: >> From: Simon Guo <wei.guo.simon@gmail.com> >> >> These 2 fields track whether user process has used Altivec/VSX >> registers or not. They are used by kernel to setup signal frame >> on user stack correctly regarding vector part. > > I still dislike this. It's just exporting internal kernel state, which I know is > the point. > > And I'd still like to know why we're the only arch that needs to do this. > > I'm not saying I won't merge it, but I'd like to understand it better first. > >> CRIU(Checkpoint and Restore In User space) builds signal frame >> for restored process. It will need this export information to >> setup signal frame correctly. And CRIU will need to restore these >> 2 fields for the restored process. > > I don't really know how CRIU works, but .. > > Does the kernel write a sigframe for the process that's being checkpointed? If > so can't you infer the state of the bits based on what was written? Hi Michael, Basically, CRIU checkpoints the process register's state through the ptrace API, and it restores it through a signal frame at restart time. This is quite odd but that the way it works on all the CRIU's supported architectures. Obviously everything is done from/in user space, so the sigframe building too. Since we can't know from user space if the thread has used or not the Altivec/VSX registers, since we can't rely on the MSR bits, we always dump these registers. > Alternately, when restoring, can you setup the sigframe with the Altivec/VSX > fields populated, and the kernel will then load them, regardless of whether > they were actually used or not prior to the checkpoint? In the case of Altivec/VSX fields, we currently force the kernel to retrieve them from the signal frame by setting MSR_VEC/MSR_VSX so restore_sigcontext() will copy them to the kernel thread's state. However this doesn't touch to used_vsr and used_vr which may remain at 0. Most of the time this is fine, but in the case a thread which has really used those registers is catching a signal just after the restore and before it has touched to these registers again (and so set used_vsr/vr), these registers will not be pushed in the newly built signal frame since setup_sigcontext() check for used_vsr/vr before pushing the registers on the stack. This may be an issue in the case the thread wants to changed those registers (don't ask me why :)) in the stacked signal frame from the signal handler since they will not be there... Being able to get and set the used_vr and used_vsr thread's variables, fixes this issue. Cheers, Laurent. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space 2016-07-07 13:12 ` Laurent Dufour @ 2016-07-07 13:21 ` Benjamin Herrenschmidt 2016-07-07 13:29 ` Benjamin Herrenschmidt ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Benjamin Herrenschmidt @ 2016-07-07 13:21 UTC (permalink / raw) To: Laurent Dufour, Michael Ellerman, Simon Guo Cc: linux-kernel, linuxppc-dev, Paul Mackerras, Kees Cook, Rashmica Gupta On Thu, 2016-07-07 at 15:12 +0200, Laurent Dufour wrote: > > Basically, CRIU checkpoints the process register's state through the > ptrace API, and it restores it through a signal frame at restart time. > This is quite odd but that the way it works on all the CRIU's supported > architectures. > > Obviously everything is done from/in user space, so the sigframe > building too. > Since we can't know from user space if the thread has used or not the > Altivec/VSX registers, since we can't rely on the MSR bits, we always > dump these registers. Right, however is that an issue ? These days with glibc using V{M,S}X for things like memcpy I would think there is little to gain in trying to avoid dumping them. > > Alternately, when restoring, can you setup the sigframe with the Altivec/VSX > > fields populated, and the kernel will then load them, regardless of whether > > they were actually used or not prior to the checkpoint? > > In the case of Altivec/VSX fields, we currently force the kernel to > retrieve them from the signal frame by setting MSR_VEC/MSR_VSX so > restore_sigcontext() will copy them to the kernel thread's state. Yup, that's the way to go. > However this doesn't touch to used_vsr and used_vr which may remain at 0. That would be a kernel bug. > Most of the time this is fine, but in the case a thread which has really > used those registers is catching a signal just after the restore and > before it has touched to these registers again (and so set used_vsr/vr), > these registers will not be pushed in the newly built signal frame since > setup_sigcontext() check for used_vsr/vr before pushing the registers on > the stack. > This may be an issue in the case the thread wants to changed those > registers (don't ask me why :)) in the stacked signal frame from the > signal handler since they will not be there... > > Being able to get and set the used_vr and used_vsr thread's variables, > fixes this issue. I think the right fix is that if a restore_sigcontext() has the MSR bits set, it should set the corresponding used_* flag. Or is there a reason why that won't work ? Cheers, Ben. > Cheers, > Laurent. > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space 2016-07-07 13:21 ` Benjamin Herrenschmidt @ 2016-07-07 13:29 ` Benjamin Herrenschmidt [not found] ` <577f7a45.568f6b0a.bd0eb.4aa8SMTPIN_ADDED_BROKEN@mx.google.com> 2016-07-07 13:40 ` Laurent Dufour 2016-07-08 5:39 ` Simon Guo 2 siblings, 1 reply; 10+ messages in thread From: Benjamin Herrenschmidt @ 2016-07-07 13:29 UTC (permalink / raw) To: Laurent Dufour, Michael Ellerman, Simon Guo Cc: linux-kernel, linuxppc-dev, Paul Mackerras, Kees Cook, Rashmica Gupta On Thu, 2016-07-07 at 23:21 +1000, Benjamin Herrenschmidt wrote: > > I think the right fix is that if a restore_sigcontext() has the MSR > bits set, > it should set the corresponding used_* flag. Something like this: (totally untested) diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c index b6aa378..1bf074e 100644 --- a/arch/powerpc/kernel/signal_32.c +++ b/arch/powerpc/kernel/signal_32.c @@ -698,6 +698,7 @@ static long restore_user_regs(struct pt_regs *regs, if (__copy_from_user(¤t->thread.vr_state, &sr->mc_vregs, sizeof(sr->mc_vregs))) return 1; + current->thread.used_vr = true; } else if (current->thread.used_vr) memset(¤t->thread.vr_state, 0, ELF_NVRREG * sizeof(vector128)); @@ -724,6 +725,7 @@ static long restore_user_regs(struct pt_regs *regs, */ if (copy_vsx_from_user(current, &sr->mc_vsregs)) return 1; + current->thread.used_vsr = true; } else if (current->thread.used_vsr) for (i = 0; i < 32 ; i++) current->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = 0; @@ -743,6 +745,7 @@ static long restore_user_regs(struct pt_regs *regs, if (__copy_from_user(current->thread.evr, &sr->mc_vregs, ELF_NEVRREG * sizeof(u32))) return 1; + current->thread.used_spe = true; } else if (current->thread.used_spe) memset(current->thread.evr, 0, ELF_NEVRREG * sizeof(u32)); @@ -799,6 +802,7 @@ static long restore_tm_user_regs(struct pt_regs *regs, &tm_sr->mc_vregs, sizeof(sr->mc_vregs))) return 1; + current->thread.used_vr = true; } else if (current->thread.used_vr) { memset(¤t->thread.vr_state, 0, ELF_NVRREG * sizeof(vector128)); @@ -832,6 +836,7 @@ static long restore_tm_user_regs(struct pt_regs *regs, if (copy_vsx_from_user(current, &sr->mc_vsregs) || copy_transact_vsx_from_user(current, &tm_sr->mc_vsregs)) return 1; + current->thread.used_vsr = true; } else if (current->thread.used_vsr) for (i = 0; i < 32 ; i++) { current->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = 0; @@ -848,6 +853,7 @@ static long restore_tm_user_regs(struct pt_regs *regs, if (__copy_from_user(current->thread.evr, &sr->mc_vregs, ELF_NEVRREG * sizeof(u32))) return 1; + current->thread.used_spe = true; } else if (current->thread.used_spe) memset(current->thread.evr, 0, ELF_NEVRREG * sizeof(u32)); diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c index 2552079..8704269 100644 --- a/arch/powerpc/kernel/signal_64.c +++ b/arch/powerpc/kernel/signal_64.c @@ -363,9 +363,11 @@ static long restore_sigcontext(struct pt_regs *regs, sigset_t *set, int sig, if (v_regs && !access_ok(VERIFY_READ, v_regs, 34 * sizeof(vector128))) return -EFAULT; /* Copy 33 vec registers (vr0..31 and vscr) from the stack */ - if (v_regs != NULL && (msr & MSR_VEC) != 0) + if (v_regs != NULL && (msr & MSR_VEC) != 0) { err |= __copy_from_user(¤t->thread.vr_state, v_regs, 33 * sizeof(vector128)); + current->thread.used_vr = true; + } else if (current->thread.used_vr) memset(¤t->thread.vr_state, 0, 33 * sizeof(vector128)); /* Always get VRSAVE back */ @@ -385,9 +387,10 @@ static long restore_sigcontext(struct pt_regs *regs, sigset_t *set, int sig, * buffer for formatting, then into the taskstruct. */ v_regs += ELF_NVRREG; - if ((msr & MSR_VSX) != 0) + if ((msr & MSR_VSX) != 0) { err |= copy_vsx_from_user(current, v_regs); - else + current->thread.used_vsr = true; + } else for (i = 0; i < 32 ; i++) current->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = 0; #endif @@ -482,6 +485,7 @@ static long restore_tm_sigcontexts(struct pt_regs *regs, 33 * sizeof(vector128)); err |= __copy_from_user(¤t->thread.transact_vr, tm_v_regs, 33 * sizeof(vector128)); + current->thread.used_vr = true; } else if (current->thread.used_vr) { memset(¤t->thread.vr_state, 0, 33 * sizeof(vector128)); @@ -515,6 +519,7 @@ static long restore_tm_sigcontexts(struct pt_regs *regs, tm_v_regs += ELF_NVRREG; err |= copy_vsx_from_user(current, v_regs); err |= copy_transact_vsx_from_user(current, tm_v_regs); + current->thread.used_vsr = true; } else { for (i = 0; i < 32 ; i++) { current->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = 0; ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <577f7a45.568f6b0a.bd0eb.4aa8SMTPIN_ADDED_BROKEN@mx.google.com>]
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space [not found] ` <577f7a45.568f6b0a.bd0eb.4aa8SMTPIN_ADDED_BROKEN@mx.google.com> @ 2016-07-11 17:39 ` Simon Guo 2016-07-15 7:15 ` Simon Guo 1 sibling, 0 replies; 10+ messages in thread From: Simon Guo @ 2016-07-11 17:39 UTC (permalink / raw) To: Michael Ellerman Cc: Benjamin Herrenschmidt, Laurent Dufour, linux-kernel, linuxppc-dev, Paul Mackerras, Kees Cook, Rashmica Gupta, cyrilbur On Fri, Jul 08, 2016 at 08:02:42PM +1000, Michael Ellerman wrote: > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > > > On Thu, 2016-07-07 at 23:21 +1000, Benjamin Herrenschmidt wrote: > >> > >> I think the right fix is that if a restore_sigcontext() has the MSR > >> bits set, > >> it should set the corresponding used_* flag. > > > > Something like this: > > > > (totally untested) > > Simon/Laurent, can you guys test this and let me know if it works for > your usecase. Just notice this note. Yes I will test it on CRIU and update later. Thanks, - Simon ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space [not found] ` <577f7a45.568f6b0a.bd0eb.4aa8SMTPIN_ADDED_BROKEN@mx.google.com> 2016-07-11 17:39 ` Simon Guo @ 2016-07-15 7:15 ` Simon Guo [not found] ` <5790aa9c.05296b0a.adf58.5901SMTPIN_ADDED_BROKEN@mx.google.com> 1 sibling, 1 reply; 10+ messages in thread From: Simon Guo @ 2016-07-15 7:15 UTC (permalink / raw) To: Michael Ellerman Cc: Benjamin Herrenschmidt, Laurent Dufour, linux-kernel, linuxppc-dev, Paul Mackerras, Kees Cook, Rashmica Gupta, cyrilbur Michael, Ben, On Fri, Jul 08, 2016 at 08:02:42PM +1000, Michael Ellerman wrote: > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > > > On Thu, 2016-07-07 at 23:21 +1000, Benjamin Herrenschmidt wrote: > >> > >> I think the right fix is that if a restore_sigcontext() has the MSR > >> bits set, > >> it should set the corresponding used_* flag. > > > > Something like this: > > > > (totally untested) > > Simon/Laurent, can you guys test this and let me know if it works for > your usecase. > Sorry for the late reply. I tested Ben's patch and the fix works for the CRIU case mentioned before. Thanks, Simon ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <5790aa9c.05296b0a.adf58.5901SMTPIN_ADDED_BROKEN@mx.google.com>]
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space [not found] ` <5790aa9c.05296b0a.adf58.5901SMTPIN_ADDED_BROKEN@mx.google.com> @ 2016-07-25 8:52 ` Simon Guo 0 siblings, 0 replies; 10+ messages in thread From: Simon Guo @ 2016-07-25 8:52 UTC (permalink / raw) To: Michael Ellerman Cc: Benjamin Herrenschmidt, Laurent Dufour, linux-kernel, linuxppc-dev, Paul Mackerras, Kees Cook, Rashmica Gupta, cyrilbur On Thu, Jul 21, 2016 at 08:57:29PM +1000, Michael Ellerman wrote: > Can one of you send a properly formatted and signed-off patch. I will work on that. Thanks, Simon ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space 2016-07-07 13:21 ` Benjamin Herrenschmidt 2016-07-07 13:29 ` Benjamin Herrenschmidt @ 2016-07-07 13:40 ` Laurent Dufour 2016-07-08 5:39 ` Simon Guo 2 siblings, 0 replies; 10+ messages in thread From: Laurent Dufour @ 2016-07-07 13:40 UTC (permalink / raw) To: Benjamin Herrenschmidt, Michael Ellerman, Simon Guo Cc: linux-kernel, linuxppc-dev, Paul Mackerras, Kees Cook, Rashmica Gupta On 07/07/2016 15:21, Benjamin Herrenschmidt wrote: > On Thu, 2016-07-07 at 15:12 +0200, Laurent Dufour wrote: >> >> Basically, CRIU checkpoints the process register's state through the >> ptrace API, and it restores it through a signal frame at restart time. >> This is quite odd but that the way it works on all the CRIU's supported >> architectures. >> >> Obviously everything is done from/in user space, so the sigframe >> building too. >> Since we can't know from user space if the thread has used or not the >> Altivec/VSX registers, since we can't rely on the MSR bits, we always >> dump these registers. > > Right, however is that an issue ? These days with glibc using V{M,S}X > for things like memcpy I would think there is little to gain in trying > to avoid dumping them. > >>> Alternately, when restoring, can you setup the sigframe with the Altivec/VSX >>> fields populated, and the kernel will then load them, regardless of whether >>> they were actually used or not prior to the checkpoint? >> >> In the case of Altivec/VSX fields, we currently force the kernel to >> retrieve them from the signal frame by setting MSR_VEC/MSR_VSX so >> restore_sigcontext() will copy them to the kernel thread's state. > > Yup, that's the way to go. > >> However this doesn't touch to used_vsr and used_vr which may remain at 0. > > That would be a kernel bug. > >> Most of the time this is fine, but in the case a thread which has really >> used those registers is catching a signal just after the restore and >> before it has touched to these registers again (and so set used_vsr/vr), >> these registers will not be pushed in the newly built signal frame since >> setup_sigcontext() check for used_vsr/vr before pushing the registers on >> the stack. >> This may be an issue in the case the thread wants to changed those >> registers (don't ask me why :)) in the stacked signal frame from the >> signal handler since they will not be there... >> >> Being able to get and set the used_vr and used_vsr thread's variables, >> fixes this issue. > > I think the right fix is that if a restore_sigcontext() has the MSR bits set, > it should set the corresponding used_* flag. > > Or is there a reason why that won't work ? I got your point and I agree that most of the time now, the Altivec/VSX registers are used by libc. In that case is there still a need for the lazy Altivec/VSX registers dump in the signal frame ? I'm fine with your proposal, except that every restarted process will have the used_vr/used_vsx turned on after the restart since we can't check if these registers were used or not at checkpoint time. But that may be a minor point... Cheers, Laurent. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v4] powerpc: Export thread_struct.used_vr/used_vsr to user space 2016-07-07 13:21 ` Benjamin Herrenschmidt 2016-07-07 13:29 ` Benjamin Herrenschmidt 2016-07-07 13:40 ` Laurent Dufour @ 2016-07-08 5:39 ` Simon Guo 2 siblings, 0 replies; 10+ messages in thread From: Simon Guo @ 2016-07-08 5:39 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Laurent Dufour, Michael Ellerman, linux-kernel, linuxppc-dev, Paul Mackerras, Kees Cook, Rashmica Gupta On Thu, Jul 07, 2016 at 11:21:18PM +1000, Benjamin Herrenschmidt wrote: > I think the right fix is that if a restore_sigcontext() has the MSR bits set, > it should set the corresponding used_* flag. > > Or is there a reason why that won't work ? That sounds reaonable to me. I will prepare a patch based on that. Michael, Ben, Laurent, Thanks the discussion and proposal. - Simon ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-07-25 8:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-07 7:49 [PATCH v4] powerpc: Export thread_struct.used_vr/used_vsr to user space wei.guo.simon 2016-07-07 11:15 ` [v4] " Michael Ellerman 2016-07-07 13:12 ` Laurent Dufour 2016-07-07 13:21 ` Benjamin Herrenschmidt 2016-07-07 13:29 ` Benjamin Herrenschmidt [not found] ` <577f7a45.568f6b0a.bd0eb.4aa8SMTPIN_ADDED_BROKEN@mx.google.com> 2016-07-11 17:39 ` Simon Guo 2016-07-15 7:15 ` Simon Guo [not found] ` <5790aa9c.05296b0a.adf58.5901SMTPIN_ADDED_BROKEN@mx.google.com> 2016-07-25 8:52 ` Simon Guo 2016-07-07 13:40 ` Laurent Dufour 2016-07-08 5:39 ` Simon Guo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).