From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9203C433FE for ; Thu, 14 Oct 2021 21:45:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AEC5C61090 for ; Thu, 14 Oct 2021 21:45:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234124AbhJNVr3 (ORCPT ); Thu, 14 Oct 2021 17:47:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54726 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232933AbhJNVr2 (ORCPT ); Thu, 14 Oct 2021 17:47:28 -0400 Received: from mail-pg1-x52a.google.com (mail-pg1-x52a.google.com [IPv6:2607:f8b0:4864:20::52a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7BA22C061755 for ; Thu, 14 Oct 2021 14:45:23 -0700 (PDT) Received: by mail-pg1-x52a.google.com with SMTP id e65so4331412pgc.5 for ; Thu, 14 Oct 2021 14:45:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:in-reply-to:references:date:message-id :mime-version; bh=4GrPc7FGyCnCD/UjuDxp/izjM7wkRrx7pBgnvNh8AfA=; b=XqUQLpCs2S/h/ceVWU2JHhlb+0n6xx80AR84zM1fbTXyJpTTdz7naNXuSISe0OmVPh NjZ/D/TD0ZH2RQIVG7s4CPqlOO1tq69oTkrFSh1I9LXUEVLqPBgSqyhAGps/jJj4ZA4P vglf7c8/7+vfhlwsZ4SeyOpGpkDyS0m7q2pnE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=4GrPc7FGyCnCD/UjuDxp/izjM7wkRrx7pBgnvNh8AfA=; b=M6aI26wm0qh9DCvA4x0OXwni8jIEqIUxnsgNf26ABluBofPL/gO+icgaWeTIuPKHDd VTUNWbGoIIAX400/IHPwxnaAKhB3G23DM9W0CycyLVEu+JiqV0wCURTZWlSruLxpgQc8 6nik9Ek8KhYJH3WNeeANzVRtjvJ6P2vB57DwezcgjWRysMocfVyQB+8VfeHR2B3U/Rdd K/nKFZNEIH+O74Gjjd50imVNhd/PpbW7cnUT/UCpiFyfErA3GlrJ45sBY4MdTDf/Y7+P /H/B072DI39IQ4mMZVI8tFkyOgJ6sxGINCBAgiX1jfRFRAQOcQ06AxE7jYPpaa5ARlnH iZLA== X-Gm-Message-State: AOAM532hFJxpN6l+HMmWekA1WUU9nhEfk2meRG6Htq/AygMlxm99qdNy 03onFPlMT5AGZ+1XPvyDCN51UA== X-Google-Smtp-Source: ABdhPJxS8/zOn3bSDjYQg+GZqWOsdaDA+H+My597hv01n1JtKvTgaMjIuZ//mrufBDPiNL/OIH5zxA== X-Received: by 2002:a63:dc42:: with SMTP id f2mr6272407pgj.152.1634247922841; Thu, 14 Oct 2021 14:45:22 -0700 (PDT) Received: from localhost ([2001:4479:e300:600:4901:2fb9:ed97:3a3e]) by smtp.gmail.com with ESMTPSA id s62sm3366448pgc.5.2021.10.14.14.45.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Oct 2021 14:45:22 -0700 (PDT) From: Daniel Axtens To: Christophe Leroy , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Andrew Morton , "James E.J. Bottomley" , Helge Deller , Arnd Bergmann , Kees Cook , Greg Kroah-Hartman Cc: linux-arch@vger.kernel.org, linux-ia64@vger.kernel.org, linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry' In-Reply-To: <49f59a8bf2c4d95cfaa03bd3dd3c1569822ad6ba.1634190022.git.christophe.leroy@csgroup.eu> References: <49f59a8bf2c4d95cfaa03bd3dd3c1569822ad6ba.1634190022.git.christophe.leroy@csgroup.eu> Date: Fri, 15 Oct 2021 08:45:19 +1100 Message-ID: <877def46xc.fsf@dja-thinkpad.axtens.net> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org Christophe Leroy writes: > There are three architectures with function descriptors, try to > have common names for the address they contain in order to > refactor some functions into generic functions later. > > powerpc has 'funcaddr' > ia64 has 'ip' > parisc has 'addr' > > Vote for 'addr' and update 'struct ppc64_opd_entry' accordingly. I would have picked 'funcaddr', but at least 'addr' is better than 'ip'! And I agree that consistency, and then making things generic is worthwhile. I grepped the latest powerpc/next for uses of 'funcaddr'. There were 5, your patch changes all 5. The series passes build tests and this patch has no checkpatch or other style concerns. On that basis: Reviewed-by: Daniel Axtens Kind regards, Daniel > Reviewed-by: Kees Cook > Signed-off-by: Christophe Leroy > --- > arch/powerpc/include/asm/elf.h | 2 +- > arch/powerpc/include/asm/sections.h | 2 +- > arch/powerpc/kernel/module_64.c | 6 +++--- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h > index a4406714c060..bb0f278f9ed4 100644 > --- a/arch/powerpc/include/asm/elf.h > +++ b/arch/powerpc/include/asm/elf.h > @@ -178,7 +178,7 @@ void relocate(unsigned long final_address); > > /* There's actually a third entry here, but it's unused */ > struct ppc64_opd_entry { > - unsigned long funcaddr; > + unsigned long addr; > unsigned long r2; > }; > > diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h > index 6e4af4492a14..32e7035863ac 100644 > --- a/arch/powerpc/include/asm/sections.h > +++ b/arch/powerpc/include/asm/sections.h > @@ -77,7 +77,7 @@ static inline void *dereference_function_descriptor(void *ptr) > struct ppc64_opd_entry *desc = ptr; > void *p; > > - if (!get_kernel_nofault(p, (void *)&desc->funcaddr)) > + if (!get_kernel_nofault(p, (void *)&desc->addr)) > ptr = p; > return ptr; > } > diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c > index 6baa676e7cb6..82908c9be627 100644 > --- a/arch/powerpc/kernel/module_64.c > +++ b/arch/powerpc/kernel/module_64.c > @@ -72,11 +72,11 @@ static func_desc_t func_desc(unsigned long addr) > } > static unsigned long func_addr(unsigned long addr) > { > - return func_desc(addr).funcaddr; > + return func_desc(addr).addr; > } > static unsigned long stub_func_addr(func_desc_t func) > { > - return func.funcaddr; > + return func.addr; > } > static unsigned int local_entry_offset(const Elf64_Sym *sym) > { > @@ -187,7 +187,7 @@ static int relacmp(const void *_x, const void *_y) > static unsigned long get_stubs_size(const Elf64_Ehdr *hdr, > const Elf64_Shdr *sechdrs) > { > - /* One extra reloc so it's always 0-funcaddr terminated */ > + /* One extra reloc so it's always 0-addr terminated */ > unsigned long relocs = 1; > unsigned i; > > -- > 2.31.1 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Axtens Date: Thu, 14 Oct 2021 21:45:19 +0000 Subject: Re: [PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry' Message-Id: <877def46xc.fsf@dja-thinkpad.axtens.net> List-Id: References: <49f59a8bf2c4d95cfaa03bd3dd3c1569822ad6ba.1634190022.git.christophe.leroy@csgroup.eu> In-Reply-To: <49f59a8bf2c4d95cfaa03bd3dd3c1569822ad6ba.1634190022.git.christophe.leroy@csgroup.eu> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Christophe Leroy , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Andrew Morton , "James E.J. Bottomley" , Helge Deller , Arnd Bergmann , Kees Cook , Greg Kroah-Hartman Cc: linux-arch@vger.kernel.org, linux-ia64@vger.kernel.org, linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org Christophe Leroy writes: > There are three architectures with function descriptors, try to > have common names for the address they contain in order to > refactor some functions into generic functions later. > > powerpc has 'funcaddr' > ia64 has 'ip' > parisc has 'addr' > > Vote for 'addr' and update 'struct ppc64_opd_entry' accordingly. I would have picked 'funcaddr', but at least 'addr' is better than 'ip'! And I agree that consistency, and then making things generic is worthwhile. I grepped the latest powerpc/next for uses of 'funcaddr'. There were 5, your patch changes all 5. The series passes build tests and this patch has no checkpatch or other style concerns. On that basis: Reviewed-by: Daniel Axtens Kind regards, Daniel > Reviewed-by: Kees Cook > Signed-off-by: Christophe Leroy > --- > arch/powerpc/include/asm/elf.h | 2 +- > arch/powerpc/include/asm/sections.h | 2 +- > arch/powerpc/kernel/module_64.c | 6 +++--- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h > index a4406714c060..bb0f278f9ed4 100644 > --- a/arch/powerpc/include/asm/elf.h > +++ b/arch/powerpc/include/asm/elf.h > @@ -178,7 +178,7 @@ void relocate(unsigned long final_address); > > /* There's actually a third entry here, but it's unused */ > struct ppc64_opd_entry { > - unsigned long funcaddr; > + unsigned long addr; > unsigned long r2; > }; > > diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h > index 6e4af4492a14..32e7035863ac 100644 > --- a/arch/powerpc/include/asm/sections.h > +++ b/arch/powerpc/include/asm/sections.h > @@ -77,7 +77,7 @@ static inline void *dereference_function_descriptor(void *ptr) > struct ppc64_opd_entry *desc = ptr; > void *p; > > - if (!get_kernel_nofault(p, (void *)&desc->funcaddr)) > + if (!get_kernel_nofault(p, (void *)&desc->addr)) > ptr = p; > return ptr; > } > diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c > index 6baa676e7cb6..82908c9be627 100644 > --- a/arch/powerpc/kernel/module_64.c > +++ b/arch/powerpc/kernel/module_64.c > @@ -72,11 +72,11 @@ static func_desc_t func_desc(unsigned long addr) > } > static unsigned long func_addr(unsigned long addr) > { > - return func_desc(addr).funcaddr; > + return func_desc(addr).addr; > } > static unsigned long stub_func_addr(func_desc_t func) > { > - return func.funcaddr; > + return func.addr; > } > static unsigned int local_entry_offset(const Elf64_Sym *sym) > { > @@ -187,7 +187,7 @@ static int relacmp(const void *_x, const void *_y) > static unsigned long get_stubs_size(const Elf64_Ehdr *hdr, > const Elf64_Shdr *sechdrs) > { > - /* One extra reloc so it's always 0-funcaddr terminated */ > + /* One extra reloc so it's always 0-addr terminated */ > unsigned long relocs = 1; > unsigned i; > > -- > 2.31.1