* [PATCH] csky: Fix a size determination in gpr_get()
@ 2020-09-22 9:15 Zhenzhong Duan
2020-09-22 16:29 ` Al Viro
0 siblings, 1 reply; 11+ messages in thread
From: Zhenzhong Duan @ 2020-09-22 9:15 UTC (permalink / raw)
To: linux-kernel, linux-csky; +Cc: oleg, guoren, viro, Zhenzhong Duan
"*" is missed in size determination as we are passing register set
rather than a pointer.
Fixes: dcad7854fcce ("sky: switch to ->regset_get()")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>
---
arch/csky/kernel/ptrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/csky/kernel/ptrace.c b/arch/csky/kernel/ptrace.c
index d822144906ac..a4cf2e2ac15a 100644
--- a/arch/csky/kernel/ptrace.c
+++ b/arch/csky/kernel/ptrace.c
@@ -83,7 +83,7 @@ static int gpr_get(struct task_struct *target,
/* Abiv1 regs->tls is fake and we need sync here. */
regs->tls = task_thread_info(target)->tp_value;
- return membuf_write(&to, regs, sizeof(regs));
+ return membuf_write(&to, regs, sizeof(*regs));
}
static int gpr_set(struct task_struct *target,
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-22 9:15 [PATCH] csky: Fix a size determination in gpr_get() Zhenzhong Duan
@ 2020-09-22 16:29 ` Al Viro
2020-09-23 0:03 ` Guo Ren
2020-09-23 2:23 ` Zhenzhong Duan
0 siblings, 2 replies; 11+ messages in thread
From: Al Viro @ 2020-09-22 16:29 UTC (permalink / raw)
To: Zhenzhong Duan; +Cc: linux-kernel, linux-csky, oleg, guoren
On Tue, Sep 22, 2020 at 05:15:05PM +0800, Zhenzhong Duan wrote:
> "*" is missed in size determination as we are passing register set
> rather than a pointer.
Ack. I can push it to Linus today, unless you want it to go through
csky tree. Preferences?
> Fixes: dcad7854fcce ("sky: switch to ->regset_get()")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>
> ---
> arch/csky/kernel/ptrace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/csky/kernel/ptrace.c b/arch/csky/kernel/ptrace.c
> index d822144906ac..a4cf2e2ac15a 100644
> --- a/arch/csky/kernel/ptrace.c
> +++ b/arch/csky/kernel/ptrace.c
> @@ -83,7 +83,7 @@ static int gpr_get(struct task_struct *target,
> /* Abiv1 regs->tls is fake and we need sync here. */
> regs->tls = task_thread_info(target)->tp_value;
>
> - return membuf_write(&to, regs, sizeof(regs));
> + return membuf_write(&to, regs, sizeof(*regs));
> }
>
> static int gpr_set(struct task_struct *target,
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-22 16:29 ` Al Viro
@ 2020-09-23 0:03 ` Guo Ren
2020-09-23 0:23 ` Al Viro
2020-09-23 2:23 ` Zhenzhong Duan
1 sibling, 1 reply; 11+ messages in thread
From: Guo Ren @ 2020-09-23 0:03 UTC (permalink / raw)
To: Al Viro
Cc: Zhenzhong Duan, Linux Kernel Mailing List, linux-csky, Oleg Nesterov
Thx Duan,
Acked-by: Guo Ren <guoren@kernel.org>
Hi AI,
I found the broken commit still has a question:
> commit dcad7854fcce6a2d49b6a3ead5bbefeff047e559
> Author: Al Viro <viro@zeniv.linux.org.uk>
> Date: Tue Jun 16 15:28:29 2020 -0400
> csky: switch to ->regset_get()
> NB: WTF "- what the fuck :(" is fpregs_get() playing at???
The fpregs_get() is for REGSET_FPR regset used by ptrace (gdb) and all
fp regs are stored in threads' context.
So, WTF question for?
Best Regards
Guo Ren
On Wed, Sep 23, 2020 at 12:29 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Tue, Sep 22, 2020 at 05:15:05PM +0800, Zhenzhong Duan wrote:
> > "*" is missed in size determination as we are passing register set
> > rather than a pointer.
>
> Ack. I can push it to Linus today, unless you want it to go through
> csky tree. Preferences?
>
> > Fixes: dcad7854fcce ("sky: switch to ->regset_get()")
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@gmail.com>
> > ---
> > arch/csky/kernel/ptrace.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/csky/kernel/ptrace.c b/arch/csky/kernel/ptrace.c
> > index d822144906ac..a4cf2e2ac15a 100644
> > --- a/arch/csky/kernel/ptrace.c
> > +++ b/arch/csky/kernel/ptrace.c
> > @@ -83,7 +83,7 @@ static int gpr_get(struct task_struct *target,
> > /* Abiv1 regs->tls is fake and we need sync here. */
> > regs->tls = task_thread_info(target)->tp_value;
> >
> > - return membuf_write(&to, regs, sizeof(regs));
> > + return membuf_write(&to, regs, sizeof(*regs));
> > }
> >
> > static int gpr_set(struct task_struct *target,
> > --
> > 2.25.1
> >
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-23 0:03 ` Guo Ren
@ 2020-09-23 0:23 ` Al Viro
2020-09-23 2:37 ` Guo Ren
0 siblings, 1 reply; 11+ messages in thread
From: Al Viro @ 2020-09-23 0:23 UTC (permalink / raw)
To: Guo Ren
Cc: Zhenzhong Duan, Linux Kernel Mailing List, linux-csky, Oleg Nesterov
On Wed, Sep 23, 2020 at 08:03:20AM +0800, Guo Ren wrote:
> Thx Duan,
>
> Acked-by: Guo Ren <guoren@kernel.org>
>
> Hi AI,
>
> I found the broken commit still has a question:
>
> > commit dcad7854fcce6a2d49b6a3ead5bbefeff047e559
> > Author: Al Viro <viro@zeniv.linux.org.uk>
> > Date: Tue Jun 16 15:28:29 2020 -0400
>
> > csky: switch to ->regset_get()
>
> > NB: WTF "- what the fuck :(" is fpregs_get() playing at???
> The fpregs_get() is for REGSET_FPR regset used by ptrace (gdb) and all
> fp regs are stored in threads' context.
> So, WTF question for?
The part under
#if defined(CONFIG_CPU_HAS_FPUV2) && !defined(CONFIG_CPU_HAS_VDSP)
What's going on there? The mapping is really weird - assuming
you had v0..v31 in the first 32 elements of regs->vr[], you
end up with
v0 v1 v2 v3 v2 v3 v6 v7 v4 v5 v10 v11 v6 v7 v14 v15
v8 v9 v18 v19 v10 v11 v22 v23 v12 v13 v26 v27 v14 v15 v30 v31
in the beginning of the output. Assuming it is the intended
behaviour, it's probably worth some comments...
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-23 0:23 ` Al Viro
@ 2020-09-23 2:37 ` Guo Ren
2020-09-23 4:52 ` Al Viro
0 siblings, 1 reply; 11+ messages in thread
From: Guo Ren @ 2020-09-23 2:37 UTC (permalink / raw)
To: Al Viro
Cc: Zhenzhong Duan, Linux Kernel Mailing List, linux-csky, Oleg Nesterov
On Wed, Sep 23, 2020 at 8:23 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Wed, Sep 23, 2020 at 08:03:20AM +0800, Guo Ren wrote:
> > Thx Duan,
> >
> > Acked-by: Guo Ren <guoren@kernel.org>
> >
> > Hi AI,
> >
> > I found the broken commit still has a question:
> >
> > > commit dcad7854fcce6a2d49b6a3ead5bbefeff047e559
> > > Author: Al Viro <viro@zeniv.linux.org.uk>
> > > Date: Tue Jun 16 15:28:29 2020 -0400
> >
> > > csky: switch to ->regset_get()
> >
> > > NB: WTF "- what the fuck :(" is fpregs_get() playing at???
> > The fpregs_get() is for REGSET_FPR regset used by ptrace (gdb) and all
> > fp regs are stored in threads' context.
> > So, WTF question for?
>
> The part under
> #if defined(CONFIG_CPU_HAS_FPUV2) && !defined(CONFIG_CPU_HAS_VDSP)
>
> What's going on there? The mapping is really weird - assuming
> you had v0..v31 in the first 32 elements of regs->vr[], you
> end up with
>
> v0 v1 v2 v3 v2 v3 v6 v7 v4 v5 v10 v11 v6 v7 v14 v15
> v8 v9 v18 v19 v10 v11 v22 v23 v12 v13 v26 v27 v14 v15 v30 v31
>
> in the beginning of the output. Assuming it is the intended
> behaviour, it's probably worth some comments...
FPU & VDSP use the same regs. 32 FPU regs' width is 64b and 16 VDSP
regs' width is 128b.
vr[0], vr[1] = fp[0] & vr[0] vr[1], vr[2], vr[3] = vdsp reg[0]
...
vr[60], vr[61] = fp[15] & vr[60] vr[61], vr[62], vr[63] = vdsp reg[15]
vr[64], vr[65] = fp[16]
vr[66], vr[67] = fp[17]
...
vr[94], vr[95] = fp[31]
Yeah, this is confusing and I'll add a comment later.
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-23 2:37 ` Guo Ren
@ 2020-09-23 4:52 ` Al Viro
2020-09-25 6:33 ` Guo Ren
2020-12-23 2:31 ` Zhenzhong Duan
0 siblings, 2 replies; 11+ messages in thread
From: Al Viro @ 2020-09-23 4:52 UTC (permalink / raw)
To: Guo Ren
Cc: Zhenzhong Duan, Linux Kernel Mailing List, linux-csky, Oleg Nesterov
On Wed, Sep 23, 2020 at 10:37:31AM +0800, Guo Ren wrote:
> > What's going on there? The mapping is really weird - assuming
> > you had v0..v31 in the first 32 elements of regs->vr[], you
> > end up with
> >
> > v0 v1 v2 v3 v2 v3 v6 v7 v4 v5 v10 v11 v6 v7 v14 v15
> > v8 v9 v18 v19 v10 v11 v22 v23 v12 v13 v26 v27 v14 v15 v30 v31
> >
> > in the beginning of the output. Assuming it is the intended
> > behaviour, it's probably worth some comments...
> FPU & VDSP use the same regs. 32 FPU regs' width is 64b and 16 VDSP
> regs' width is 128b.
>
> vr[0], vr[1] = fp[0] & vr[0] vr[1], vr[2], vr[3] = vdsp reg[0]
> ...
> vr[60], vr[61] = fp[15] & vr[60] vr[61], vr[62], vr[63] = vdsp reg[15]
> vr[64], vr[65] = fp[16]
> vr[66], vr[67] = fp[17]
> ...
> vr[94], vr[95] = fp[31]
>
> Yeah, this is confusing and I'll add a comment later.
Umm... It would help if you described these 3 layouts:
1) kernel-side with VDSP
2) userland (identical to (1)?)
3) kernel-side without VDSP
Still confused...
PS: my apologies re commit message - I left a note to myself when doing
that series and then forgot about it ;-/
Anyway, which tree should it go through? In any case, that fix is
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
and I can take it through vfs.git or you guys can pick in csky tree;
up to you.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-23 4:52 ` Al Viro
@ 2020-09-25 6:33 ` Guo Ren
2020-12-23 2:31 ` Zhenzhong Duan
1 sibling, 0 replies; 11+ messages in thread
From: Guo Ren @ 2020-09-25 6:33 UTC (permalink / raw)
To: Al Viro
Cc: Zhenzhong Duan, Linux Kernel Mailing List, linux-csky, Oleg Nesterov
On Wed, Sep 23, 2020 at 12:52 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Wed, Sep 23, 2020 at 10:37:31AM +0800, Guo Ren wrote:
>
> > > What's going on there? The mapping is really weird - assuming
> > > you had v0..v31 in the first 32 elements of regs->vr[], you
> > > end up with
> > >
> > > v0 v1 v2 v3 v2 v3 v6 v7 v4 v5 v10 v11 v6 v7 v14 v15
> > > v8 v9 v18 v19 v10 v11 v22 v23 v12 v13 v26 v27 v14 v15 v30 v31
> > >
> > > in the beginning of the output. Assuming it is the intended
> > > behaviour, it's probably worth some comments...
> > FPU & VDSP use the same regs. 32 FPU regs' width is 64b and 16 VDSP
> > regs' width is 128b.
> >
> > vr[0], vr[1] = fp[0] & vr[0] vr[1], vr[2], vr[3] = vdsp reg[0]
> > ...
> > vr[60], vr[61] = fp[15] & vr[60] vr[61], vr[62], vr[63] = vdsp reg[15]
> > vr[64], vr[65] = fp[16]
> > vr[66], vr[67] = fp[17]
> > ...
> > vr[94], vr[95] = fp[31]
> >
> > Yeah, this is confusing and I'll add a comment later.
>
> Umm... It would help if you described these 3 layouts:
> 1) kernel-side with VDSP
With VDSP: we use vdsp ld/st instructions to access first 16
128bit-regs and use fpu ld/st instructions to access last 16
64bit-regs.
> 2) userland (identical to (1)?)
Identical to 1.
> 3) kernel-side without VDSP
Without VDSP: we use fpu ld/st instructions to access last 32 64bit-regs.
So, there are 96 32bit-vr[] for the struct. And with VDSP or not will
got different storage format.
With VDSP:
vr128[16] // contain first fp64[16]
fp64[16]; // second fp64[16]
Without VDSP:
fp64[32]
no-use for the reset vr[]
> Still confused...
>
> PS: my apologies re commit message - I left a note to myself when doing
> that series and then forgot about it ;-/
>
> Anyway, which tree should it go through? In any case, that fix is
Thx for your job, and pushing to linus with Zhenzhong Duan's advice.
> Acked-by: Al Viro <viro@zeniv.linux.org.uk>
> and I can take it through vfs.git or you guys can pick in csky tree;
> up to you.
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-23 4:52 ` Al Viro
2020-09-25 6:33 ` Guo Ren
@ 2020-12-23 2:31 ` Zhenzhong Duan
2020-12-23 14:57 ` Guo Ren
1 sibling, 1 reply; 11+ messages in thread
From: Zhenzhong Duan @ 2020-12-23 2:31 UTC (permalink / raw)
To: Al Viro; +Cc: Guo Ren, Linux Kernel Mailing List, linux-csky, Oleg Nesterov
On Wed, Sep 23, 2020 at 12:52 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Wed, Sep 23, 2020 at 10:37:31AM +0800, Guo Ren wrote:
>
> > > What's going on there? The mapping is really weird - assuming
> > > you had v0..v31 in the first 32 elements of regs->vr[], you
> > > end up with
> > >
> > > v0 v1 v2 v3 v2 v3 v6 v7 v4 v5 v10 v11 v6 v7 v14 v15
> > > v8 v9 v18 v19 v10 v11 v22 v23 v12 v13 v26 v27 v14 v15 v30 v31
> > >
> > > in the beginning of the output. Assuming it is the intended
> > > behaviour, it's probably worth some comments...
> > FPU & VDSP use the same regs. 32 FPU regs' width is 64b and 16 VDSP
> > regs' width is 128b.
> >
> > vr[0], vr[1] = fp[0] & vr[0] vr[1], vr[2], vr[3] = vdsp reg[0]
> > ...
> > vr[60], vr[61] = fp[15] & vr[60] vr[61], vr[62], vr[63] = vdsp reg[15]
> > vr[64], vr[65] = fp[16]
> > vr[66], vr[67] = fp[17]
> > ...
> > vr[94], vr[95] = fp[31]
> >
> > Yeah, this is confusing and I'll add a comment later.
>
> Umm... It would help if you described these 3 layouts:
> 1) kernel-side with VDSP
> 2) userland (identical to (1)?)
> 3) kernel-side without VDSP
> Still confused...
>
> PS: my apologies re commit message - I left a note to myself when doing
> that series and then forgot about it ;-/
>
> Anyway, which tree should it go through? In any case, that fix is
> Acked-by: Al Viro <viro@zeniv.linux.org.uk>
> and I can take it through vfs.git or you guys can pick in csky tree;
> up to you.
Hi Al, Guo
Seems this patch is still pending， could you help check it？Thanks
Zhenzhong
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-12-23 2:31 ` Zhenzhong Duan
@ 2020-12-23 14:57 ` Guo Ren
2020-12-24 2:21 ` Zhenzhong Duan
0 siblings, 1 reply; 11+ messages in thread
From: Guo Ren @ 2020-12-23 14:57 UTC (permalink / raw)
To: Zhenzhong Duan
Cc: Al Viro, Linux Kernel Mailing List, linux-csky, Oleg Nesterov
Hi Zhengzhong,
I'll take it, thx.
On Wed, Dec 23, 2020 at 10:31 AM Zhenzhong Duan
<zhenzhong.duan@gmail.com> wrote:
>
> On Wed, Sep 23, 2020 at 12:52 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Wed, Sep 23, 2020 at 10:37:31AM +0800, Guo Ren wrote:
> >
> > > > What's going on there? The mapping is really weird - assuming
> > > > you had v0..v31 in the first 32 elements of regs->vr[], you
> > > > end up with
> > > >
> > > > v0 v1 v2 v3 v2 v3 v6 v7 v4 v5 v10 v11 v6 v7 v14 v15
> > > > v8 v9 v18 v19 v10 v11 v22 v23 v12 v13 v26 v27 v14 v15 v30 v31
> > > >
> > > > in the beginning of the output. Assuming it is the intended
> > > > behaviour, it's probably worth some comments...
> > > FPU & VDSP use the same regs. 32 FPU regs' width is 64b and 16 VDSP
> > > regs' width is 128b.
> > >
> > > vr[0], vr[1] = fp[0] & vr[0] vr[1], vr[2], vr[3] = vdsp reg[0]
> > > ...
> > > vr[60], vr[61] = fp[15] & vr[60] vr[61], vr[62], vr[63] = vdsp reg[15]
> > > vr[64], vr[65] = fp[16]
> > > vr[66], vr[67] = fp[17]
> > > ...
> > > vr[94], vr[95] = fp[31]
> > >
> > > Yeah, this is confusing and I'll add a comment later.
> >
> > Umm... It would help if you described these 3 layouts:
> > 1) kernel-side with VDSP
> > 2) userland (identical to (1)?)
> > 3) kernel-side without VDSP
> > Still confused...
> >
> > PS: my apologies re commit message - I left a note to myself when doing
> > that series and then forgot about it ;-/
> >
> > Anyway, which tree should it go through? In any case, that fix is
> > Acked-by: Al Viro <viro@zeniv.linux.org.uk>
> > and I can take it through vfs.git or you guys can pick in csky tree;
> > up to you.
>
> Hi Al, Guo
>
> Seems this patch is still pending， could you help check it？Thanks
>
> Zhenzhong
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] csky: Fix a size determination in gpr_get()
2020-09-22 16:29 ` Al Viro
2020-09-23 0:03 ` Guo Ren
@ 2020-09-23 2:23 ` Zhenzhong Duan
1 sibling, 0 replies; 11+ messages in thread
From: Zhenzhong Duan @ 2020-09-23 2:23 UTC (permalink / raw)
To: Al Viro; +Cc: linux-kernel, linux-csky, oleg, guoren
On Wed, Sep 23, 2020 at 12:29 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Tue, Sep 22, 2020 at 05:15:05PM +0800, Zhenzhong Duan wrote:
> > "*" is missed in size determination as we are passing register set
> > rather than a pointer.
>
> Ack. I can push it to Linus today, unless you want it to go through
> csky tree. Preferences?
I prefer pushing to linus.
Regards
Zhenzhong
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-12-24 2:23 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22 9:15 [PATCH] csky: Fix a size determination in gpr_get() Zhenzhong Duan
2020-09-22 16:29 ` Al Viro
2020-09-23 0:03 ` Guo Ren
2020-09-23 0:23 ` Al Viro
2020-09-23 2:37 ` Guo Ren
2020-09-23 4:52 ` Al Viro
2020-09-25 6:33 ` Guo Ren
2020-12-23 2:31 ` Zhenzhong Duan
2020-12-23 14:57 ` Guo Ren
2020-12-24 2:21 ` Zhenzhong Duan
2020-09-23 2:23 ` Zhenzhong Duan
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).