This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCHv3] powerpc: ABI change - add HWCAP/HWCAP2/platform info to TCB
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Carlos Eduardo Seo <cseo at linux dot vnet dot ibm dot com>
- Cc: munroesj at linux dot vnet dot ibm dot com, Peter Bergner <bergner at vnet dot ibm dot com>, Roland McGrath <roland at hack dot frob dot com>, Joseph Myers <joseph at codesourcery dot com>, GNU C Library <libc-alpha at sourceware dot org>, Tulio Machado <tuliom at linux dot vnet dot ibm dot com>, "Steven J. Munroe" <sjmunroe at us dot ibm dot com>
- Date: Fri, 9 Oct 2015 16:14:11 -0400
- Subject: Re: [PATCHv3] powerpc: ABI change - add HWCAP/HWCAP2/platform info to TCB
- Authentication-results: sourceware.org; auth=none
- References: <4BE991CB-F601-4D63-A416-5991FE870FC4 at linux dot vnet dot ibm dot com> <alpine dot DEB dot 2 dot 10 dot 1509232114070 dot 10585 at digraph dot polyomino dot org dot uk> <B650304C-6207-46AC-B6A8-FA29959B305D at linux dot vnet dot ibm dot com> <alpine dot DEB dot 2 dot 10 dot 1509232141250 dot 10585 at digraph dot polyomino dot org dot uk> <1443046986 dot 23503 dot 68 dot camel at otta> <alpine dot DEB dot 2 dot 10 dot 1509232234500 dot 10585 at digraph dot polyomino dot org dot uk> <1443050731 dot 23503 dot 78 dot camel at otta> <alpine dot DEB dot 2 dot 10 dot 1509232340470 dot 10585 at digraph dot polyomino dot org dot uk> <1443057751 dot 13186 dot 6 dot camel at otta> <alpine dot DEB dot 2 dot 10 dot 1509240126140 dot 10585 at digraph dot polyomino dot org dot uk> <1443058522 dot 13186 dot 9 dot camel at otta> <1443654009 dot 4885 dot 4 dot camel at oc7878010663> <B2EB15A6-FEB5-4DBA-8719-0BAA5B65EF5E at linux dot vnet dot ibm dot com> <5615CD5D dot 1080409 at redhat dot com> <3C6704AA-1037-487A-BE68-BFACCB6A15C4 at linux dot vnet dot ibm dot com>
On 10/07/2015 11:03 PM, Carlos Eduardo Seo wrote:
>
> Thanks for the review. Answers below:
Thank you again for you patience in the review process.
>> On Oct 7, 2015, at 10:56 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> (a) Reference to GLIBC_2.23 in ppc.h functions?
>
> A dummy call should be harmless at that point. What do you think? If
> you have a better idea, please let me know.
You don't need this if, as Steven and Peter suggest, you initialize
everything before CTORs are run.
Therefore you should be cutting this code and verifying your hooks
run before CTORs.
>>
>> (b) Definition of _dl_powerpc_platforms in test program.
>>
>> This is an indication that something is wrong. No user code should ever
>> need to define a _dl_* data symbol. Everything should just work for -static
>> and -shared, otherwise you have something wrong and still need to fix it.
>>
>> Why is this definition needed?
>
> In order to test this new feature, I get the data I need from the
> auxiliary vector via getauxval(). For AT_PLATFORM, we do not store
> the platform string in the TCB, but the platform number (which is
> generated by _dl_string_platform() in the __parse* function). So, in
> order to get this number from getauxval(), I need to call
> _dl_string_platform() again, and that depends on
> _dl_powerpc_platform.
>
> Do you have a better idea that doesn’t end up duplicating more code?
> This is my least favorite part of this patch, to be honest. So any
> suggestion to get rid of that is welcome.
The function _dl_string_platform should always work without
needing to define anything by hand.
I see a few options, but the best is likely to have the test case
use #include <dl-procinfo.c> to get the definitions.
The best example is that ldconfig.c includes it also and uses the
dlprocinfo.h APIs, so that's how to solve this.
e.g.
#define PROCINFO_CLASS static
#include <dl-procinfo.c>
>> (c) Avoid custom dl-support.c
>>
>> Are you sure you can't avoid this? Can we add AT_PLATFORM directly to the
>> case statement used in _dl_aux_init?
>
> I suppose we could. I’ll see if that doesn’t break anything.
Thanks. That simplifies future power maintenance and fixes every arch.
>>
>> (c) libc-start.c handling of AT_*
>>
>> This seems like your double processing these entries here and in _dl_aux_init?
>> Could you please verify this?
>
> OK. If that works, I’ll also spin a patch to cleanup AT_DCACHEBSIZE later.
Thanks.
>>
>> (d) Nit-pics about FAIL/PASS
>>
>> See POSIX compliant testing notes from DejaGNU.
Thanks again.
Cheers,
Carlos.