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:   Fri, 15 Oct 2021 16:11:32 +1000
From:   Nicholas Piggin <npiggin@gmail.com>
Subject: Re: [PATCH v2 03/13] powerpc: Remove func_descr_t
To:     Andrew Morton <akpm@linux-foundation.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Helge Deller <deller@gmx.de>, Daniel Axtens <dja@axtens.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
        Kees Cook <keescook@chromium.org>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Paul Mackerras <paulus@samba.org>
Cc:     linux-arch@vger.kernel.org, linux-ia64@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
References: <cover.1634190022.git.christophe.leroy@csgroup.eu>
        <16eef6afbf7322d0c07760ebf827b8f9f50f7c6e.1634190022.git.christophe.leroy@csgroup.eu>
        <874k9j45fm.fsf@dja-thinkpad.axtens.net>
        <b02d5211-2f00-b303-766b-d612c1bd4402@csgroup.eu>
In-Reply-To: <b02d5211-2f00-b303-766b-d612c1bd4402@csgroup.eu>
MIME-Version: 1.0
Message-Id: <1634277766.29y8aqzatr.astroid@bobo.none>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Excerpts from Christophe Leroy's message of October 15, 2021 3:19 pm:
>=20
>=20
> Le 15/10/2021 =C3=A0 00:17, Daniel Axtens a =C3=A9crit=C2=A0:
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>=20
>>> 'func_descr_t' is redundant with 'struct ppc64_opd_entry'
>>=20
>> 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.)
>=20
> An architecture not using function descriptors won't do much with=20
> ((func_desc_t *)x)->addr. This is just done to allow building stuff=20
> regardless.
>=20
> I prefer something like
>=20
> 	if (have_function_descriptors())
> 		addr =3D (func_desc_t *)ptr)->addr;
> 	else
> 		addr =3D ptr;

If you make a generic data type for architectures without function=20
descriptors as such

typedef struct func_desc {
    char addr[0];
} func_desc_t;

Then you can do that with no if. The downside is your addr has to be=20
char * and it's maybe not helpful to be so "clever".

>>   - why pick ppc64_opd_entry over func_descr_t?
>=20
> Good question. At the begining it was because it was in UAPI headers,=20
> and also because it was the one used in our=20
> dereference_function_descriptor().
>=20
> But at the end maybe that's not the more logical choice. I need to look=20
> a bit more.

I would prefer the func_descr_t (with 'toc' and 'env') if you're going=20
to change it.

Thanks,
Nick