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 1D8CDC4332F for ; Thu, 14 Oct 2021 22:17:40 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 92A81610D1 for ; Thu, 14 Oct 2021 22:17:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 92A81610D1 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=axtens.net Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 02E406B006C; Thu, 14 Oct 2021 18:17:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F1F456B0071; Thu, 14 Oct 2021 18:17:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E0F8C900002; Thu, 14 Oct 2021 18:17:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0193.hostedemail.com [216.40.44.193]) by kanga.kvack.org (Postfix) with ESMTP id D3F836B006C for ; Thu, 14 Oct 2021 18:17:38 -0400 (EDT) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 9E5D82D4D8 for ; Thu, 14 Oct 2021 22:17:38 +0000 (UTC) X-FDA: 78696455796.20.A890C5C Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) by imf02.hostedemail.com (Postfix) with ESMTP id BA3897001A08 for ; Thu, 14 Oct 2021 22:17:36 +0000 (UTC) Received: by mail-pj1-f47.google.com with SMTP id d13-20020a17090ad3cd00b0019e746f7bd4so7991530pjw.0 for ; Thu, 14 Oct 2021 15:17:37 -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=/gn1naYin2rxewmNR+UQ+4Ua2Az6pxvVd0XA2L6nJ+U=; b=ONfSismG4d04naWppWaw5fFBIKSKKMAy0gVJiXZrFConvnU5E8d2Ayd3VDh0kKxPlM KfYheVy3VZqFxW7C3SyDj3kHRI0/KpnkFFKB2eHymwNvTlk77B8NEvbREKsCxf/hUfZR 66TL6htytJE5V6/HVu+C2OZbv0Cu9cqLYCK78= 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=/gn1naYin2rxewmNR+UQ+4Ua2Az6pxvVd0XA2L6nJ+U=; b=PfOgWh0ovTVZLLP1jiQqdiPOjCsA0rjwIYLicuSY5UoNsBgcSKc1VEtP2X3gAMsWH5 yqRfz6hcH0JgtG7oXCgkImnwRgIrnhgxHYqivlRILY5XRAzVjAbJ1CEC415M8t/kTEIN Yxnd3fmUFiTTZHU7xlAUHfZbO5QWz5sUlsJKDEs3Wgffi1fBodh15YUuEUkqu4oK9Dq6 dEKsg6Y4VX7uKl0H94CCP5abPEXD6lcU2wXMHqXuRw3q0P3lxwTYYUAhl76bogE+nY+1 i6pupsdvJaRmbsFedgkSnbpyiaC3aj2AFDCFb5ctcuSwgMiYnP+9Uu6d9wlPWVnIW/po +4nA== X-Gm-Message-State: AOAM53223+yz0yP/UFX3AIZz0CSfb8j5O2ZCim0EOZ/s0Ld1EaFcaiJ9 c++jo9C7yXNzEZZq9VaPszicTQ== X-Google-Smtp-Source: ABdhPJzab2EpHK7Q/T8QXwpqraVUPmAWt0eL0woPg5T6pMmI9UAwBAQkaSq8CsyFo20UObxGkQ6Z9w== X-Received: by 2002:a17:90b:224e:: with SMTP id hk14mr9005831pjb.224.1634249856727; Thu, 14 Oct 2021 15:17:36 -0700 (PDT) Received: from localhost ([2001:4479:e300:600:4901:2fb9:ed97:3a3e]) by smtp.gmail.com with ESMTPSA id x27sm3586622pfo.90.2021.10.14.15.17.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Oct 2021 15:17:36 -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 03/13] powerpc: Remove func_descr_t In-Reply-To: <16eef6afbf7322d0c07760ebf827b8f9f50f7c6e.1634190022.git.christophe.leroy@csgroup.eu> References: <16eef6afbf7322d0c07760ebf827b8f9f50f7c6e.1634190022.git.christophe.leroy@csgroup.eu> Date: Fri, 15 Oct 2021 09:17:33 +1100 Message-ID: <874k9j45fm.fsf@dja-thinkpad.axtens.net> MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Queue-Id: BA3897001A08 X-Stat-Signature: 5e1suuhdoncih1ffif4ehyts9pqpukap Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=axtens.net header.s=google header.b=ONfSismG; dmarc=none; spf=pass (imf02.hostedemail.com: domain of dja@axtens.net designates 209.85.216.47 as permitted sender) smtp.mailfrom=dja@axtens.net X-Rspamd-Server: rspam02 X-HE-Tag: 1634249856-601685 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Christophe Leroy writes: > 'func_descr_t' is redundant with 'struct ppc64_opd_entry' So, if I understand the overall direction of the series, you're consolidating powerpc around one single type for function descriptors, and then you're creating a generic typedef so that generic code can always do ((func_desc_t)x)->addr to get the address of a function out of a function descriptor regardless of arch. (And regardless of whether the arch uses function descriptors or not.) So: - why pick ppc64_opd_entry over func_descr_t? - Why not make our struct just called func_desc_t - why have a ppc64_opd_entry type or a func_descr_t typedef? - Should this patch wait until after you've made the generic func_desc_t change and move directly to that new interface? (rather than move from func_descr_t -> ppc64_opd_entry -> ...) Or is there a particular reason arch specific code should use an arch-specific struct or named type? > Remove it. > > Signed-off-by: Christophe Leroy > --- > arch/powerpc/include/asm/code-patching.h | 2 +- > arch/powerpc/include/asm/types.h | 6 ------ > arch/powerpc/kernel/signal_64.c | 8 ++++---- > 3 files changed, 5 insertions(+), 11 deletions(-) > > diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h > index 4ba834599c4d..f3445188d319 100644 > --- a/arch/powerpc/include/asm/code-patching.h > +++ b/arch/powerpc/include/asm/code-patching.h > @@ -110,7 +110,7 @@ static inline unsigned long ppc_function_entry(void *func) > * function's descriptor. The first entry in the descriptor is the > * address of the function text. > */ > - return ((func_descr_t *)func)->entry; > + return ((struct ppc64_opd_entry *)func)->addr; > #else > return (unsigned long)func; > #endif > diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h > index f1630c553efe..97da77bc48c9 100644 > --- a/arch/powerpc/include/asm/types.h > +++ b/arch/powerpc/include/asm/types.h > @@ -23,12 +23,6 @@ > > typedef __vector128 vector128; > > -typedef struct { > - unsigned long entry; > - unsigned long toc; > - unsigned long env; > -} func_descr_t; I was a little concerned about going from a 3-element struct to a 2-element struct (as ppc64_opd_entry doesn't have an element for env) - but we don't seem to take the sizeof this anywhere, nor do we use env anywhere, nor do we do funky macro stuff with it in the signal handling code that might implictly use the 3rd element, so I guess this will work. Still, func_descr_t seems to describe the underlying ABI better than ppc64_opd_entry... > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_POWERPC_TYPES_H */ > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c > index 1831bba0582e..63ddbe7b108c 100644 > --- a/arch/powerpc/kernel/signal_64.c > +++ b/arch/powerpc/kernel/signal_64.c > @@ -933,11 +933,11 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set, > * descriptor is the entry address of signal and the second > * entry is the TOC value we need to use. > */ > - func_descr_t __user *funct_desc_ptr = > - (func_descr_t __user *) ksig->ka.sa.sa_handler; > + struct ppc64_opd_entry __user *funct_desc_ptr = > + (struct ppc64_opd_entry __user *)ksig->ka.sa.sa_handler; > > - err |= get_user(regs->ctr, &funct_desc_ptr->entry); > - err |= get_user(regs->gpr[2], &funct_desc_ptr->toc); > + err |= get_user(regs->ctr, &funct_desc_ptr->addr); > + err |= get_user(regs->gpr[2], &funct_desc_ptr->r2); Likewise, r2 seems like a worse name than toc. I guess we could clean that up another time though. Kind regards, Daniel > } > > /* enter the signal handler in native-endian mode */ > -- > 2.31.1