Re: [PATCH v2 03/13] powerpc: Remove func_descr_t

Nicholas Piggin
2021-10-15T06:11:40Z

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

If you make a generic data type for architectures without function
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
char * and it's maybe not helpful to be so "clever".

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

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

Thanks,
Nick