Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: /tllqZmaNSf0pwVngbbkvrWa4jPFHORmq6vu9xyHDGcSsHwMl/qi4RNeuiLjTRfDbsjhW8OpW5
 XpDoR0jI099Yp0G6vt/om2Pojc8aDZfLbMoob8LgnthK26Rb0cSfYsTVQ4Ern0hBZIR6KSwrm9
 kPXw0Spmdy4JWwlpp9wKLil6PbUJBOBwtSzvJ1Q9EHvDltd2hSqGxudITIM9Pzo9fB2E6u2jC9
 P5rBXhyk54OnjHb69E6cbn3W3ylV+tI3USE+3ZbUWFWzpalrEMNZiY6+7xUABrZfkQ0JFAQwrq
 eUE=
IronPort-SDR: tmI21nJS/lX63fAYjytIkd1PWsChQ6KHyBIxA5KxqlfhiOGJCJCNheH3zB+KLrZxhRCjNircE2
 7NeegtjBktaQ==
IronPort-SDR: VUewrqLbnzqHz+1znrMauq+iKaQ9F6wif5BzAqHWU2V2ZT7K1lVDvG0MTfXji6pud1QgJHJ3zk
 LoEnDGt9JhZA==
WDCIronportException: Internal
Date:   Wed, 5 Aug 2020 11:25:11 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@wdc.com>
To:     Al Viro <viro@zeniv.linux.org.uk>
cc:     Palmer Dabbelt <palmer@dabbelt.com>,
        linux-riscv@lists.infradead.org,
        Paul Walmsley <paul.walmsley@sifive.com>,
        aou@eecs.berkeley.edu, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org
Subject: Re: [PATCH 1/2] riscv: ptrace: Use the correct API for `fcsr'
 access
In-Reply-To: <20200805024807.GM1236603@ZenIV.linux.org.uk>
Message-ID: <alpine.LFD.2.21.2008051117180.24175@redsun52.ssa.fujisawa.hgst.com>
References: <20200805020745.GL1236603@ZenIV.linux.org.uk> <mhng-cd1ff2e9-7d34-4d56-8d79-b2d02a239290@palmerdabbelt-glaptop1> <20200805024807.GM1236603@ZenIV.linux.org.uk>
User-Agent: Alpine 2.21 (LFD 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 5 Aug 2020, Al Viro wrote:

> > I'm not sure I understand what you're saying, but given that branch replaces
> > all of this I guess it's best to just do nothing on our end here?
> 
> It doesn't replace ->put() (for now); it _does_ replace ->get() and AFAICS the
> replacement is much saner:
> 
> static int riscv_fpr_get(struct task_struct *target,
>                          const struct user_regset *regset,
>                          struct membuf to)
> {
> 	struct __riscv_d_ext_state *fstate = &target->thread.fstate;
> 
> 	membuf_write(&to, fstate, offsetof(struct __riscv_d_ext_state, fcsr));
> 	membuf_store(&to, fstate->fcsr);
> 	return membuf_zero(&to, 4);     // explicitly pad
> }

 I'm glad to see the old interface go, it was cumbersome.

> user_regset_copyout() calling conventions are atrocious and so are those of
> regset ->get().  The best thing to do with both is to take them out of their
> misery and be done with that.  Do you see any problems with riscv gdbserver
> on current linux-next?  If not, I'd rather see that "API" simply go away...
> If there are problems, I would very much prefer fixes on top of what's done
> in that branch.

 I can push linux-next through regression-testing with RISC-V gdbserver 
and/or native GDB if that would help.  This is also used with core dumps, 
but honestly I don't know what state RISC-V support is in in the BFD/GDB's 
core dump interpreter, as people tend to forget about the core dump 
feature nowadays.

  Maciej