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;
Date:   Sat, 12 Sep 2020 17:40:41 +0900
From:   Stafford Horne <shorne@gmail.com>
To:     Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Jonas Bonn <jonas@southpole.se>,
        Albert Ou <aou@eecs.berkeley.edu>,
        linux-riscv@lists.infradead.org,
        Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
        openrisc@lists.librecores.org, Palmer Dabbelt <palmer@dabbelt.com>,
        Greentime Hu <green.hu@gmail.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v3 3/3] openrisc: Fix issue with get_user for 64-bit
 values
Message-ID: <20200912084041.GQ3562056@lianli.shorne-pla.net>
References: <20200910233940.2132107-1-shorne@gmail.com>
 <20200910233940.2132107-4-shorne@gmail.com>
 <20200911205526.65llro2gnh7zlsu4@ltop.local>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200911205526.65llro2gnh7zlsu4@ltop.local>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Sep 11, 2020 at 10:55:26PM +0200, Luc Van Oostenryck wrote:
> On Fri, Sep 11, 2020 at 08:39:40AM +0900, Stafford Horne wrote:
> > A build failure was raised by kbuild with the following error.
> > 
> >     drivers/android/binder.c: Assembler messages:
> >     drivers/android/binder.c:3861: Error: unrecognized keyword/register name `l.lwz ?ap,4(r24)'
> >     drivers/android/binder.c:3866: Error: unrecognized keyword/register name `l.addi ?ap,r0,0'
> > 
> > The issue is with 64-bit get_user() calls on openrisc.  I traced this to
> > a problem where in the internally in the get_user macros there is a cast
> > to long __gu_val this causes GCC to think the get_user call is 32-bit.
> > This binder code is really long and GCC allocates register r30, which
> > triggers the issue. The 64-bit get_user asm tries to get the 64-bit pair
> > register, which for r30 overflows the general register names and returns
> > the dummy register ?ap.
> > 
> > The fix here is to move the temporary variables into the asm macros.  We
> > use a 32-bit __gu_tmp for 32-bit and smaller macro and a 64-bit tmp in
> > the 64-bit macro.  The cast in the 64-bit macro has a trick of casting
> > through __typeof__((x)-(x)) which avoids the below warning.  This was
> > barrowed from riscv.
> > 
> >     arch/openrisc/include/asm/uaccess.h:240:8: warning: cast to pointer from integer of different size
> > 
> > I tested this is a small unit test to check reading between 64-bit and
> > 32-bit pointers to 64-bit and 32-bit values in all combinations.  Also I
> > ran make C=1 to confirm no new sparse warnings came up.  It all looks
> > clean to me.
> 
> It looks correct to me too now at C & assembly level.
> Feel free to add my:
> Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Thanks a lot.

-Stafford