From: Mark Salter <msalter@redhat.com>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: linux-arch <linux-arch@vger.kernel.org>,
Aurelien Jacquiot <a-jacquiot@ti.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Guenter Roeck <linux@roeck-us.net>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: c6x linker issue on linux-next-20160808 + some linker table work
Date: Thu, 11 Aug 2016 13:14:34 -0400 [thread overview]
Message-ID: <1470935674.3551.118.camel@redhat.com> (raw)
In-Reply-To: <20160811155926.GX3296@wotan.suse.de>
On Thu, 2016-08-11 at 17:59 +0200, Luis R. Rodriguez wrote:
> On Thu, Aug 11, 2016 at 07:32:42AM -0400, Mark Salter wrote:
> >
> > On Thu, 2016-08-11 at 07:56 +0200, Luis R. Rodriguez wrote:
> > >
> > > On Wed, Aug 10, 2016 at 07:04:09PM -0400, Mark Salter wrote:
> > > >
> > > >
> > > > On Wed, 2016-08-10 at 23:30 +0200, Luis R. Rodriguez wrote:
> > > > >
> > > > > OK thanks I've found a clean solution minimal solution to this as follows. This now
> > > > > builds fine. Is this a fine work around for now ?
> > > > Almost. You also need:
> > > >
> > > > diff --git a/include/linux/tables.h b/include/linux/tables.h
> > > > index a39ab03..3fa8d4d 100644
> > > > --- a/include/linux/tables.h
> > > > +++ b/include/linux/tables.h
> > > > @@ -325,7 +325,7 @@
> > > > __attribute__((used, \
> > > > weak, \
> > > > __aligned__(LINUX_SECTION_ALIGNMENT(name)),\
> > > > - section(SECTION_TBL(SECTION_RODATA, \
> > > > + section(SECTION_TBL(SECTION_TBL_RO, \
> > > > name, level))))
> > > >
> > > > /**
> > > >
> > > > Otherwise, start and end RO table markers end up in different sections.
> > > I thought that was not needed as weak attributes already force it to go to
> > > .const ? Anyway I've added this as well. Thanks!
> > The section attribute forced both variables into .rodata but the weak
> > attribute prevented accesses from using the SB-relative reloc. The
> > non-weak variable is the one that led to the link error.
> I ask as set_section_tbl_type() was not patched for instance, so firmware/Makefile
> still uses SECTION_RODATA, and it compiles and links fine. Should that also be
> using then SECTION_TBL_RO ? Or do we only need this for the C constructors ?
>
> Luis
Yuck. You need SECTION_TBL_RO and s/.rodata/.const/ in that Makefile.
C6X doesn't support any of the devices with firmware, so I just added:
fw-shipped-y += ti_3410.fw
to firmware/Makefile for testing.
Leaving in .rodata and SECTION_RODATA, I got:
% readelf --syms vmlinux | grep -e _fw_ -e builtin_fw
8445: e01d4000 0 NOTYPE LOCAL DEFAULT 7 _fw_ti_3410_fw_bin
8446: e01d75c5 0 NOTYPE LOCAL DEFAULT 7 _fw_end
8447: e020a7cc 0 NOTYPE LOCAL DEFAULT 7 _fw_ti_3410_fw_name
11063: e023d688 0 OBJECT GLOBAL DEFAULT 13 builtin_fw__end
15867: e023d688 0 OBJECT WEAK DEFAULT 13 builtin_fw
From the above addresses, the _fw symbols are in .rodata and the builtin_fw symbols
are in .const.
Changing the Makefile to use .rodata and SECTION_TBL_RO, I see:
8445: e0239688 0 NOTYPE LOCAL DEFAULT 13 _fw_ti_3410_fw_bin
8446: e023cc4d 0 NOTYPE LOCAL DEFAULT 13 _fw_end
8447: e023cc50 0 NOTYPE LOCAL DEFAULT 13 _fw_ti_3410_fw_name
11063: e0239688 0 OBJECT GLOBAL DEFAULT 13 builtin_fw__end
15867: e0239688 0 OBJECT WEAK DEFAULT 13 builtin_fw
which has everything in .const as it should be. But still builtin_fw and
builtin_fw__end are at same address which seems wrong.
WARNING: multiple messages have this Message-ID (diff)
From: Mark Salter <msalter@redhat.com>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: linux-arch <linux-arch@vger.kernel.org>,
Aurelien Jacquiot <a-jacquiot@ti.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Guenter Roeck <linux@roeck-us.net>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: c6x linker issue on linux-next-20160808 + some linker table work
Date: Thu, 11 Aug 2016 13:14:34 -0400 [thread overview]
Message-ID: <1470935674.3551.118.camel@redhat.com> (raw)
Message-ID: <20160811171434.y8jQKXnWsS-7JFogMfTNQ0EpTAKsv6CD3DBEQQLat8M@z> (raw)
In-Reply-To: <20160811155926.GX3296@wotan.suse.de>
On Thu, 2016-08-11 at 17:59 +0200, Luis R. Rodriguez wrote:
> On Thu, Aug 11, 2016 at 07:32:42AM -0400, Mark Salter wrote:
> >
> > On Thu, 2016-08-11 at 07:56 +0200, Luis R. Rodriguez wrote:
> > >
> > > On Wed, Aug 10, 2016 at 07:04:09PM -0400, Mark Salter wrote:
> > > >
> > > >
> > > > On Wed, 2016-08-10 at 23:30 +0200, Luis R. Rodriguez wrote:
> > > > >
> > > > > OK thanks I've found a clean solution minimal solution to this as follows. This now
> > > > > builds fine. Is this a fine work around for now ?
> > > > Almost. You also need:
> > > >
> > > > diff --git a/include/linux/tables.h b/include/linux/tables.h
> > > > index a39ab03..3fa8d4d 100644
> > > > --- a/include/linux/tables.h
> > > > +++ b/include/linux/tables.h
> > > > @@ -325,7 +325,7 @@
> > > > __attribute__((used, \
> > > > weak, \
> > > > __aligned__(LINUX_SECTION_ALIGNMENT(name)),\
> > > > - section(SECTION_TBL(SECTION_RODATA, \
> > > > + section(SECTION_TBL(SECTION_TBL_RO, \
> > > > name, level))))
> > > >
> > > > /**
> > > >
> > > > Otherwise, start and end RO table markers end up in different sections.
> > > I thought that was not needed as weak attributes already force it to go to
> > > .const ? Anyway I've added this as well. Thanks!
> > The section attribute forced both variables into .rodata but the weak
> > attribute prevented accesses from using the SB-relative reloc. The
> > non-weak variable is the one that led to the link error.
> I ask as set_section_tbl_type() was not patched for instance, so firmware/Makefile
> still uses SECTION_RODATA, and it compiles and links fine. Should that also be
> using then SECTION_TBL_RO ? Or do we only need this for the C constructors ?
>
> Luis
Yuck. You need SECTION_TBL_RO and s/.rodata/.const/ in that Makefile.
C6X doesn't support any of the devices with firmware, so I just added:
fw-shipped-y += ti_3410.fw
to firmware/Makefile for testing.
Leaving in .rodata and SECTION_RODATA, I got:
% readelf --syms vmlinux | grep -e _fw_ -e builtin_fw
8445: e01d4000 0 NOTYPE LOCAL DEFAULT 7 _fw_ti_3410_fw_bin
8446: e01d75c5 0 NOTYPE LOCAL DEFAULT 7 _fw_end
8447: e020a7cc 0 NOTYPE LOCAL DEFAULT 7 _fw_ti_3410_fw_name
11063: e023d688 0 OBJECT GLOBAL DEFAULT 13 builtin_fw__end
15867: e023d688 0 OBJECT WEAK DEFAULT 13 builtin_fw
WARNING: multiple messages have this Message-ID (diff)
From: Mark Salter <msalter@redhat.com>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: linux-arch <linux-arch@vger.kernel.org>,
Aurelien Jacquiot <a-jacquiot@ti.com>,
Josh Poimboeuf <jpoimboe@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Guenter Roeck <linux@roeck-us.net>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: c6x linker issue on linux-next-20160808 + some linker table work
Date: Thu, 11 Aug 2016 13:14:34 -0400 [thread overview]
Message-ID: <1470935674.3551.118.camel@redhat.com> (raw)
In-Reply-To: <20160811155926.GX3296@wotan.suse.de>
On Thu, 2016-08-11 at 17:59 +0200, Luis R. Rodriguez wrote:
> On Thu, Aug 11, 2016 at 07:32:42AM -0400, Mark Salter wrote:
> >
> > On Thu, 2016-08-11 at 07:56 +0200, Luis R. Rodriguez wrote:
> > >
> > > On Wed, Aug 10, 2016 at 07:04:09PM -0400, Mark Salter wrote:
> > > >
> > > >
> > > > On Wed, 2016-08-10 at 23:30 +0200, Luis R. Rodriguez wrote:
> > > > >
> > > > > OK thanks I've found a clean solution minimal solution to this as follows. This now
> > > > > builds fine. Is this a fine work around for now ?
> > > > Almost. You also need:
> > > >
> > > > diff --git a/include/linux/tables.h b/include/linux/tables.h
> > > > index a39ab03..3fa8d4d 100644
> > > > --- a/include/linux/tables.h
> > > > +++ b/include/linux/tables.h
> > > > @@ -325,7 +325,7 @@
> > > > __attribute__((used, \
> > > > weak, \
> > > > __aligned__(LINUX_SECTION_ALIGNMENT(name)),\
> > > > - section(SECTION_TBL(SECTION_RODATA, \
> > > > + section(SECTION_TBL(SECTION_TBL_RO, \
> > > > name, level))))
> > > >
> > > > /**
> > > >
> > > > Otherwise, start and end RO table markers end up in different sections.
> > > I thought that was not needed as weak attributes already force it to go to
> > > .const ? Anyway I've added this as well. Thanks!
> > The section attribute forced both variables into .rodata but the weak
> > attribute prevented accesses from using the SB-relative reloc. The
> > non-weak variable is the one that led to the link error.
> I ask as set_section_tbl_type() was not patched for instance, so firmware/Makefile
> still uses SECTION_RODATA, and it compiles and links fine. Should that also be
> using then SECTION_TBL_RO ? Or do we only need this for the C constructors ?
>
> Luis
Yuck. You need SECTION_TBL_RO and s/.rodata/.const/ in that Makefile.
C6X doesn't support any of the devices with firmware, so I just added:
fw-shipped-y += ti_3410.fw
to firmware/Makefile for testing.
Leaving in .rodata and SECTION_RODATA, I got:
% readelf --syms vmlinux | grep -e _fw_ -e builtin_fw
8445: e01d4000 0 NOTYPE LOCAL DEFAULT 7 _fw_ti_3410_fw_bin
8446: e01d75c5 0 NOTYPE LOCAL DEFAULT 7 _fw_end
8447: e020a7cc 0 NOTYPE LOCAL DEFAULT 7 _fw_ti_3410_fw_name
11063: e023d688 0 OBJECT GLOBAL DEFAULT 13 builtin_fw__end
15867: e023d688 0 OBJECT WEAK DEFAULT 13 builtin_fw
>From the above addresses, the _fw symbols are in .rodata and the builtin_fw symbols
are in .const.
Changing the Makefile to use .rodata and SECTION_TBL_RO, I see:
8445: e0239688 0 NOTYPE LOCAL DEFAULT 13 _fw_ti_3410_fw_bin
8446: e023cc4d 0 NOTYPE LOCAL DEFAULT 13 _fw_end
8447: e023cc50 0 NOTYPE LOCAL DEFAULT 13 _fw_ti_3410_fw_name
11063: e0239688 0 OBJECT GLOBAL DEFAULT 13 builtin_fw__end
15867: e0239688 0 OBJECT WEAK DEFAULT 13 builtin_fw
which has everything in .const as it should be. But still builtin_fw and
builtin_fw__end are at same address which seems wrong.
next prev parent reply other threads:[~2016-08-11 17:14 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-09 8:11 c6x linker issue on linux-next-20160808 + some linker table work Luis R. Rodriguez
2016-08-09 13:37 ` Guenter Roeck
2016-08-09 17:04 ` Mark Salter
2016-08-09 18:40 ` Luis R. Rodriguez
2016-08-09 18:45 ` Mark Salter
2016-08-10 1:50 ` Mark Salter
[not found] ` <CAB=NE6XbxvpdsXecPLbh9krsKRvwKcwEzcve4XorpPetU3Xk6Q@mail.gmail.com>
2016-08-10 3:04 ` Mark Salter
2016-08-10 21:30 ` Luis R. Rodriguez
2016-08-10 23:04 ` Mark Salter
2016-08-11 5:56 ` Luis R. Rodriguez
2016-08-11 11:32 ` Mark Salter
2016-08-11 15:59 ` Luis R. Rodriguez
2016-08-11 17:14 ` Mark Salter [this message]
2016-08-11 17:14 ` Mark Salter
2016-08-11 17:14 ` Mark Salter
2016-08-11 17:56 ` Luis R. Rodriguez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1470935674.3551.118.camel@redhat.com \
--to=msalter@redhat.com \
--cc=a-jacquiot@ti.com \
--cc=hpa@zytor.com \
--cc=jpoimboe@redhat.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mcgrof@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.