P9 support in binutils

Bug #1655181 reported by bugproxy on 2017-01-09
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
binutils (Ubuntu)
High
Matthias Klose
Xenial
Undecided
Matthias Klose

Bug Description

[SRU Justification]
Hardware enablement of new P9 architecture on 16.04 LTS

[Test case]
Verify that the binutils testsuite still passes. This requires manual review of the build logs.

[Regression potential]
One of the patches changes the behavior around handling of undefined weak symbols on ppc64el as a whole. This behavior change could cause regressions on code built for existing ppc64el targets. This is unlikely because nothing should be relying on the current behavior, and this is an upstream patch driven by the architecture owner.

One of the patches changes the set of assembly mnemonics available for POWER9 opcodes. This should have no impact because nothing should be relying on the not-yet-finalized P9 support in xenial.

The upstream binutils (2.27 and trunk) is almost complete for POWER9, but unfortunately the changes expected to land in DD2 hardware have not yet been set in stone. With that said, the remaining changes are fairly small and will only affect user space; there are no kernel dependencies on the unresolved architecture changes.

== Comment: #3 - Peter E. Bergner <email address hidden> - 2016-11-01 10:43:16 ==
Complete POWER9 support is upstream and exists in trunk and the binutils 2.27 and 2.26 release branches.

== Comment: #4 - Breno Henrique Leitao <email address hidden> - 2016-11-21 12:45:53 ==
(In reply to comment #3)
> Complete POWER9 support is upstream and exists in trunk and the binutils
> 2.27 and 2.26 release branches.

Binutils version 2.27 is already in Ubuntu 17.04. Is there anything else required?

== Comment: #5 - William J. Schmidt <email address hidden> - 2016-11-21 13:32:50 ==
(In reply to comment #4)
> (In reply to comment #3)
> > Complete POWER9 support is upstream and exists in trunk and the binutils
> > 2.27 and 2.26 release branches.
>
> Binutils version 2.27 is already in Ubuntu 17.04. Is there anything else
> required?

Well, as long as they pick up the backported fixes, nothing else is required. That's what this feature is there to ensure.

== Comment: #6 - Breno Henrique Leitao <email address hidden> - 2016-11-24 06:52:51 ==
Hi,

> Well, as long as they pick up the backported fixes, nothing else is
> required. That's what this feature is there to ensure.

Do you know, at this time, which are the fixes that will be requested to be backported?

== Comment: #7 - William J. Schmidt <email address hidden> - 2016-11-28 09:17:01 ==
(In reply to comment #6)
> Hi,
>
> > Well, as long as they pick up the backported fixes, nothing else is
> > required. That's what this feature is there to ensure.
>
> Do you know, at this time, which are the fixes that will be requested to be
> backported?

Peter, can you please respond to Breno's question?

== Comment: #8 - Peter E. Bergner <email address hidden> - 2016-11-29 12:08:11 ==
CC'ing Alan in case he has an extra input.

Looking through the binutils-2_27-branch log, I see the following commits we would want picked up:

commit 799b679496c98eb1f31625b00bb5db67a6f608d7
Author: Peter Bergner <email address hidden>
Date: Fri Sep 16 16:17:46 2016 -0500

    Backport lastest POWER9 support to match final ISA 3.0 documentation.

    opcodes/
            Apply from master.
            2016-09-14 Peter Bergner <email address hidden>

            * ppc-opc.c (powerpc_opcodes) <slbiag>: New mnemonic.
            <addex., brd, brh, brw, lwzmx, nandxor, rldixor, setbool,
            xor3>: Delete mnemonics.
            <cp_abort>: Rename mnemonic from ...
            <cpabort>: ...to this.
            <setb>: Change to a X form instruction.
            <sync>: Change to 1 operand form.
            <copy>: Delete mnemonic.
            <copy_first>: Rename mnemonic from ...
            <copy>: ...to this.
            <paste, paste.>: Delete mnemonics.
            <paste_last>: Rename mnemonic from ...
            <paste.>: ...to this.

    gas/
            Apply from master.
            2016-09-14 Peter Bergner <email address hidden>

            * testsuite/gas/ppc/power9.d <slbiag, cpabort> New tests.
            <addex., brd, brh, brw, lwzmx, nandxor, rldixor, setbool,
            xor3, cp_abort, copy_first, paste, paste_last, sync>: Remove tests.
            <copy, paste.>: Update tests.
            * testsuite/gas/ppc/power9.s: Likewise.

commit c6a7c521c14e0cc188ccc0388e0d5d21c2042c94
Author: Alan Modra <email address hidden>
Date: Thu Sep 1 14:56:52 2016 +0930

    Don't treat .opd section specially when ELFv2

    Fixes a gdb segfault if a section named .opd is found in ELFv2 binaries.

            * elf64-ppc.c (synthetic_opd): New static var.
            (compare_symbols): Don't treat symbols in .opd specially for ELFv2.
            (ppc64_elf_get_synthetic_symtab): Likewise. Comment.

commit 2a0b8eb7a7974ff7605cb3ba5dffa5abef286ffa
Author: Alan Modra <email address hidden>
Date: Tue Aug 30 20:57:32 2016 +0930

    ppc apuinfo for spe parsed incorrectly

    apuinfo saying SPE resulted in mach = bfd_mach_ppc_vle due to a
    missing break.

            PR 20531
            * elf32-ppc.c (_bfd_elf_ppc_set_arch): Add missing "break".

commit 7f27ccfcd5b86a6517a5c01d1cc29e87ac39c13c
Author: Alan Modra <email address hidden>
Date: Fri Aug 19 11:06:41 2016 +0930

    PR 20472, PowerPC64 ifunc confusion

    This patch fixes quite a lot of confusion in allocate_dynrelocs over
    ifuncs. Function descriptors make ELFv1 quite different to ELFv2.

            PR 20472
            * elf64-ppc.c (ppc64_elf_before_check_relocs): Tweak abiversion test.
            (readonly_dynrelocs): Comment fix.
            (global_entry_stub): New function.
            (ppc64_elf_adjust_dynamic_symbol): Tweak abiversion test. Match
            ELFv2 code deciding on dynamic relocs vs. global entry stubs to
            that in size_global_entry_stubs, handling ifunc too. Delete dead
            weak sym code.
            (allocate_dynrelocs): Ensure dyn_relocs field is cleared when no
            dyn_relocs are needed. Correct handling of ifunc dyn_relocs.
            Tidy ELIMINATE_COPY_RELOCS code, only setting dynindx for
            undefweak syms. Expand and correct comments.
            (size_global_entry_stubs): Ensure symbol is defined.
            (ppc64_elf_relocate_section): Match condition under which
            dyn_relocs are emitted to that in allocate_dynrelocs.

commit e4aa8a9f60398eacd04398bcc51d7be5f93ed4eb
Author: Alan Modra <email address hidden>
Date: Thu Aug 11 12:30:52 2016 +0930

    PowerPC64 ELFv1 undefined weak functions

    Undefined weak functions, like __gmon_start__, were not being made
    dynamic or emitting plt call code. While the behaviour of undefined
    weak symbols is not defined in the ELF standard, the intention on
    powerpc64 was to make it possible to link without a definition of such
    symbols and at run time behave the same as if a definition was found
    at link time in a shared library.

            * elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Don't exit with
            non_got_ref true in any case where we could have generated dynbss
            copies but decide not to do so.

bugproxy (bugproxy) on 2017-01-09
tags: added: architecture-ppc64le bugnameltc-145030 severity-high targetmilestone-inin1704
Changed in ubuntu:
assignee: nobody → Taco Screen team (taco-screen-team)
affects: ubuntu → binutils (Ubuntu)
Manoj Iyer (manjo) wrote :

Steve, could you please take a look at this request?

Changed in binutils (Ubuntu):
assignee: Taco Screen team (taco-screen-team) → Steve Langasek (vorlon)
importance: Undecided → High

------- Comment From <email address hidden> 2017-01-23 14:49 EDT-------
(In reply to comment #14)
> Steve, could you please take a look at this request?

Steve and I do not understand what you are asking here. Can you be specific about what you want from us? I thought this bug was fairly clear about what we want, as I specified what upstream commits were needed.

Matthias Klose (doko) on 2017-01-24
Changed in binutils (Ubuntu):
assignee: Steve Langasek (vorlon) → Matthias Klose (doko)
status: New → Fix Released
Changed in binutils (Ubuntu Xenial):
assignee: nobody → Matthias Klose (doko)
Matthias Klose (doko) wrote :

799b679496c98eb1f31625b00bb5db67a6f608d7 already is on the 2.26 branch.
e4aa8a9f60398eacd04398bcc51d7be5f93ed4eb applies cleanly
c6a7c521c14e0cc188ccc0388e0d5d21c2042c94 applies cleanly
7f27ccfcd5b86a6517a5c01d1cc29e87ac39c13c doesn't apply cleanly
2a0b8eb7a7974ff7605cb3ba5dffa5abef286ffa looks like something not applicable for 2.26?

bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2017-01-29 22:25 EDT-------
I think both e4aa8a9 ("PowerPC64 ELFv1 undefined weak functions") and 7f27ccfc ("PR 20472, PowerPC64 ifunc confusion") should be left off 2.26 based binutils.

While the undefined weak fix is desirable, it broke ifunc and the followup fix for ifunc dynamic relocation accounting confusion requires other infrastructure changes that are not on 2.26. The backports necessary are large enough that I think it would be better to simply require 2.27 if we really want undefined weak to be fixed.

Breno Leitão (breno-leitao) wrote :

Sorry for the confusing here. The backporting word was used to backport the patches to 17.04, since Canonical upgraded to a newer binutils, we are good here.

Hello bugproxy, or anyone else affected,

Accepted binutils into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/binutils/2.26.1-1ubuntu1~16.04.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

description: updated
Changed in binutils (Ubuntu Xenial):
status: New → Fix Committed
tags: added: verification-needed
Matthias Klose (doko) wrote :

the testsuite passes and doesn't show any regressions.

tags: added: verification-done
removed: verification-needed
tags: added: verification-done-xenial
removed: verification-done
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers