Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Andrew, could you please add this patch to -mm. As we discussed here, it should be OK for us. Thanks, - Bryan Wu On Fri, 2007-09-21 at 11:32 +0800, David McCullough wrote: > > Jivin Robin Getz lays it down ... > > On Thu 20 Sep 2007 11:03, David McCullough pondered: > > > I would say that (a) is definately not the case. I am sure the BF > guys > > > will say they have been banging us on the head with changes for a > long > > > time and getting no where as we considered the changes to severe > or out > > > of line. > > > > I don't think we have been "banging heads" with you (unless that is > your > > feeling?) - how about "working together, but diverting to satisfy > different > > needs" :) > > No head banging feelings here, but I would understand if you guys > felt > that way occasionally ;-) I obviously forgot the happy face on that > statement. It was meant as a good thing. > > > I think that we have had more issues in the uClinux-dist (userspace > and build > > environment), but for kernel code, we have moved from some > non-standard > > (stupid) things we were doing early on to what we have today - which > is as > > common/standard with other archs as we can be. > > > > Although this is slightly off topic - on the uClinux distribution > side - most > > of our changes are based on requirements/desires from being able to > support > > fdpic elf and flat formats, and to attempt to make things easier for > end > > users/us to use/maintain. Where we do make changes - we always send > the patch > > upstream and have the conversation with you (not everyone else does > this), > > and some/most times rework things so they are more acceptable to > you. We > > don't always come to an agreement - but we always have the > discussion, and > > are willing to move if we can make things better that still meets > both our > > needs/desires. > > > > > This particular patch was trivial in comparison to others I've > seen, > > > > That is what we thought. > > > > > it fixed all the existing arches (not something that is always > done) and > > > seemed a reasonable start to finally get the BF guys up and > running. > > > Still, happy to make it better of course ;-) > > > > As always - we are more than happy to explore/review alternative > patches if > > people want to write/sumbit them. > > Cheers, > Davidm > > -- > David McCullough, [EMAIL PROTECTED], Ph:+61 > 734352815 > Secure Computing - SnapGear http://www.uCdot.org > http://www.cyberguard.com > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Andrew, could you please add this patch to -mm. As we discussed here, it should be OK for us. Thanks, - Bryan Wu On Fri, 2007-09-21 at 11:32 +0800, David McCullough wrote: Jivin Robin Getz lays it down ... On Thu 20 Sep 2007 11:03, David McCullough pondered: I would say that (a) is definately not the case. I am sure the BF guys will say they have been banging us on the head with changes for a long time and getting no where as we considered the changes to severe or out of line. I don't think we have been banging heads with you (unless that is your feeling?) - how about working together, but diverting to satisfy different needs :) No head banging feelings here, but I would understand if you guys felt that way occasionally ;-) I obviously forgot the happy face on that statement. It was meant as a good thing. I think that we have had more issues in the uClinux-dist (userspace and build environment), but for kernel code, we have moved from some non-standard (stupid) things we were doing early on to what we have today - which is as common/standard with other archs as we can be. Although this is slightly off topic - on the uClinux distribution side - most of our changes are based on requirements/desires from being able to support fdpic elf and flat formats, and to attempt to make things easier for end users/us to use/maintain. Where we do make changes - we always send the patch upstream and have the conversation with you (not everyone else does this), and some/most times rework things so they are more acceptable to you. We don't always come to an agreement - but we always have the discussion, and are willing to move if we can make things better that still meets both our needs/desires. This particular patch was trivial in comparison to others I've seen, That is what we thought. it fixed all the existing arches (not something that is always done) and seemed a reasonable start to finally get the BF guys up and running. Still, happy to make it better of course ;-) As always - we are more than happy to explore/review alternative patches if people want to write/sumbit them. Cheers, Davidm -- David McCullough, [EMAIL PROTECTED], Ph:+61 734352815 Secure Computing - SnapGear http://www.uCdot.org http://www.cyberguard.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Jivin Robin Getz lays it down ... > On Thu 20 Sep 2007 11:03, David McCullough pondered: > > I would say that (a) is definately not the case. I am sure the BF guys > > will say they have been banging us on the head with changes for a long > > time and getting no where as we considered the changes to severe or out > > of line. > > I don't think we have been "banging heads" with you (unless that is your > feeling?) - how about "working together, but diverting to satisfy different > needs" :) No head banging feelings here, but I would understand if you guys felt that way occasionally ;-) I obviously forgot the happy face on that statement. It was meant as a good thing. > I think that we have had more issues in the uClinux-dist (userspace and build > environment), but for kernel code, we have moved from some non-standard > (stupid) things we were doing early on to what we have today - which is as > common/standard with other archs as we can be. > > Although this is slightly off topic - on the uClinux distribution side - most > of our changes are based on requirements/desires from being able to support > fdpic elf and flat formats, and to attempt to make things easier for end > users/us to use/maintain. Where we do make changes - we always send the patch > upstream and have the conversation with you (not everyone else does this), > and some/most times rework things so they are more acceptable to you. We > don't always come to an agreement - but we always have the discussion, and > are willing to move if we can make things better that still meets both our > needs/desires. > > > This particular patch was trivial in comparison to others I've seen, > > That is what we thought. > > > it fixed all the existing arches (not something that is always done) and > > seemed a reasonable start to finally get the BF guys up and running. > > Still, happy to make it better of course ;-) > > As always - we are more than happy to explore/review alternative patches if > people want to write/sumbit them. Cheers, Davidm -- David McCullough, [EMAIL PROTECTED], Ph:+61 734352815 Secure Computing - SnapGear http://www.uCdot.org http://www.cyberguard.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu 20 Sep 2007 11:03, David McCullough pondered: > I would say that (a) is definately not the case. I am sure the BF guys > will say they have been banging us on the head with changes for a long > time and getting no where as we considered the changes to severe or out > of line. I don't think we have been "banging heads" with you (unless that is your feeling?) - how about "working together, but diverting to satisfy different needs" :) I think that we have had more issues in the uClinux-dist (userspace and build environment), but for kernel code, we have moved from some non-standard (stupid) things we were doing early on to what we have today - which is as common/standard with other archs as we can be. Although this is slightly off topic - on the uClinux distribution side - most of our changes are based on requirements/desires from being able to support fdpic elf and flat formats, and to attempt to make things easier for end users/us to use/maintain. Where we do make changes - we always send the patch upstream and have the conversation with you (not everyone else does this), and some/most times rework things so they are more acceptable to you. We don't always come to an agreement - but we always have the discussion, and are willing to move if we can make things better that still meets both our needs/desires. > This particular patch was trivial in comparison to others I've seen, That is what we thought. > it fixed all the existing arches (not something that is always done) and > seemed a reasonable start to finally get the BF guys up and running. > Still, happy to make it better of course ;-) As always - we are more than happy to explore/review alternative patches if people want to write/sumbit them. -Robin - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Jivin Paul Mundt lays it down ... ... > > The other maintainers who have spoken up didn't seem to think this was > > ugly, or an abuse. I'm surprised to hear language like that when > > discussing a patch that adds an if statement, a local variable and one > > parameter in a function call. > > > Yes, well, the scope of the changes is really rather irrelevant, it's the > technical basis behind it that should be the concern. My suggestion was > to lose the local variable, get rid of this "persistent" API, and plug in > something like flat_validate_relval() or something equally silly where > each architecture has the option to grab the relval and set up whatever > sort of state it wants to out-of-line. > > This is making API changes where it's convenient for your platform to use > this value, and there's no reason to change the API here at all. The > if/continue block are not an issue, it is the whole concept of persistent > data in this case that has no need to be applied uniformally across all > other architectures. > > Why should all architectures have to change their APIs (not just adding > new things mind you, also changing existing definitions) to accomodate > something that can trivially be kept in the blackfin code? Fair comment. I hadn't considered that it could be even cleaner. If it's easily doable then I agree with your concerns. > I wager the other maintainers either a) don't care because it doesn't > effect them, or b) have resigned binfmt_flat to its fate. Neither option > is particularly appealing in terms of getting that code tidied. That sort > of indifference is largely why binfmt_flat is as much of a mess as it is > today. Your patch is certainly not the cause of this, the architecture > callbacks there are just a mess to begin with, I would prefer that we try > to keep new additions flexible without blindly hardcoding more usage > assumptions all over the place and having to paper over them again later. I would say that (a) is definately not the case. I am sure the BF guys will say they have been banging us on the head with changes for a long time and getting no where as we considered the changes to severe or out of line. This particular patch was trivial in comparison to others I've seen, it fixed all the existing arches (not something that is always done) and seemed a reasonable start to finally get the BF guys up and running. Still, happy to make it better of course ;-) As for (b), I think it will be around for a while. It's not as flexible as fdpic, but it's still a tiny, simple format and not everyone has an fdpic implementation yet :-) Cheers, Davidm -- David McCullough, [EMAIL PROTECTED], Ph:+61 734352815 Secure Computing - SnapGear http://www.uCdot.org http://www.cyberguard.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Paul Mundt wrote: This is making API changes where it's convenient for your platform to use this value, and there's no reason to change the API here at all. Your proposed addition of flat_validate_relval is an API change, and very similar in nature to what I've done. A local variable here is the most natural way to store this information. What do you suggest we use, a global? A member in the task struct? Why should all architectures have to change their APIs (not just adding new things mind you, also changing existing definitions) to accomodate something that can trivially be kept in the blackfin code? I don't see how there's a burden given that we've provided patches, and most maintainers have already said their fine with it. It seems to me that it's a natural and common thing for many architectures to be updated when new things are added to core code. Bernd -- This footer brought to you by insane German lawmakers. Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368 Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, Sep 20, 2007 at 02:04:26PM +0200, Bernd Schmidt wrote: > Paul Mundt wrote: > >I find it a bit disconcerting that blackfin already depends on this > >in-tree without there being any earlier discussion on making these > >changes. > > Parts of the initial submission were picked up (the include/asm > directory), other's weren't. Little we can do about that. > What you can do about that is to order the patches, so one doesn't get applied ahead of the other. People have successfully managed to submit patches with dependencies in the past, you can look through the archives for examples. > >Since all the architecture is interested in is the relval that has > >associated "persistent" data encoded in it, why don't we just have a stub > >to give the architecture a chance to validate the relval before the > >flat_get_relocate_addr() and move this stuff there instead? ie, blackfin > >takes this out-of-line and manages its persistent value there. > > What is flat_set_persistent other than a stub to validate the relval? > I'm not at all sure what you're proposing or how it would be different. > My apologies for not making it clearer. I did not get a chance to hack together a patch today. Hopefully the below will clear up whatever confusion there was. > >load_flat_file() is ugly enough without dumping more architecture > >callback abuses in it. > > The other maintainers who have spoken up didn't seem to think this was > ugly, or an abuse. I'm surprised to hear language like that when > discussing a patch that adds an if statement, a local variable and one > parameter in a function call. > Yes, well, the scope of the changes is really rather irrelevant, it's the technical basis behind it that should be the concern. My suggestion was to lose the local variable, get rid of this "persistent" API, and plug in something like flat_validate_relval() or something equally silly where each architecture has the option to grab the relval and set up whatever sort of state it wants to out-of-line. This is making API changes where it's convenient for your platform to use this value, and there's no reason to change the API here at all. The if/continue block are not an issue, it is the whole concept of persistent data in this case that has no need to be applied uniformally across all other architectures. Why should all architectures have to change their APIs (not just adding new things mind you, also changing existing definitions) to accomodate something that can trivially be kept in the blackfin code? I wager the other maintainers either a) don't care because it doesn't effect them, or b) have resigned binfmt_flat to its fate. Neither option is particularly appealing in terms of getting that code tidied. That sort of indifference is largely why binfmt_flat is as much of a mess as it is today. Your patch is certainly not the cause of this, the architecture callbacks there are just a mess to begin with, I would prefer that we try to keep new additions flexible without blindly hardcoding more usage assumptions all over the place and having to paper over them again later. I can hack up some patches tomorrow if you're still unsure of what I'm proposing. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Paul Mundt wrote:
I find it a bit disconcerting that blackfin already depends on this
in-tree without there being any earlier discussion on making these
changes.
Parts of the initial submission were picked up (the include/asm
directory), other's weren't. Little we can do about that.
*/
if (rev > OLD_FLAT_VERSION) {
+ unsigned long persistent = 0;
for (i=0; i < relocs; i++) {
unsigned long addr, relval;
@@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
relocated (of course, the address has to be
relocated first). */
relval = ntohl(reloc[i]);
+ if (flat_set_persistent (relval, ))
+ continue;
addr = flat_get_relocate_addr(relval);
rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
if (rp == (unsigned long *)RELOC_FAILED) {
I don't much care for this API. It's shuffling around a temporary
variable for the architecture code that's set for certain relocations
that are otherwise unhandled.
Since all the architecture is interested in is the relval that has
associated "persistent" data encoded in it, why don't we just have a stub
to give the architecture a chance to validate the relval before the
flat_get_relocate_addr() and move this stuff there instead? ie, blackfin
takes this out-of-line and manages its persistent value there.
What is flat_set_persistent other than a stub to validate the relval?
I'm not at all sure what you're proposing or how it would be different.
load_flat_file() is ugly enough without dumping more architecture
callback abuses in it.
The other maintainers who have spoken up didn't seem to think this was
ugly, or an abuse. I'm surprised to hear language like that when
discussing a patch that adds an if statement, a local variable and one
parameter in a function call.
Bernd
--
This footer brought to you by insane German lawmakers.
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
From: David McCullough <[EMAIL PROTECTED]> Date: Thu, 20 Sep 2007 11:55:25 +1000 > > Jivin Robin Getz lays it down ... > > On Tue 18 Sep 2007 04:09, Bryan Wu pondered: > > > From: Bernd Schmidt <[EMAIL PROTECTED]> > > > > > > This just adds minimum support for the Blackfin relocations, > > > since we don't have enough space in each reloc. The idea > > > is to store a value with one relocation so that subsequent ones can > > > access it. > > > > > > Signed-off-by: Bernd Schmidt <[EMAIL PROTECTED]> > > > Signed-off-by: Bryan Wu <[EMAIL PROTECTED]> > > > Cc: David McCullough <[EMAIL PROTECTED]> > > > Cc: Greg Ungerer <[EMAIL PROTECTED]> > > > > Adding the other appropriate maintainers. for h8, m32r, sh and v850. > > Looks fine to me, obviously impacts the existing arches very little. > Can't see why it shouldn't get included, > > Cheers, > Davidm Looks fine to me, too. Acked-by: Hirokazu Takata <[EMAIL PROTECTED]> -- Hirokazu Takata <[EMAIL PROTECTED]> Linux/M32R Project: http://www.linux-m32r.org/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Robin Getz <[EMAIL PROTECTED]> writes: > BTW - does anyone know Miles Bader's current email address? There isn't one > listed in MAINTAINERS - and he is still listed for the NEC V850? [EMAIL PROTECTED] -Miles -- Everywhere is walking distance if you have the time. -- Steven Wright - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, 2007-09-20 at 14:34 +0800, Bryan Wu wrote: > On Thu, 2007-09-20 at 02:08 -0400, Robin Getz wrote: > > On Wed 19 Sep 2007 23:54, Paul Mundt pondered: > > > On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: > > > > On 9/19/07, Paul Mundt <[EMAIL PROTECTED]> wrote: > > > > > On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: > > > > > > Jivin Robin Getz lays it down ... > > > > > > > On Tue 18 Sep 2007 04:09, Bryan Wu pondered: > > > > > > > > This just adds minimum support for the Blackfin relocations, > > > > > > > > since we don't have enough space in each reloc. The idea > > > > > > > > is to store a value with one relocation so that subsequent > > > > > > > > ones can access it. > > > > > > > > > > > > > > Adding the other appropriate maintainers. for h8, m32r, sh and > > > > > > > v850. > > > > > > > > > > > > Looks fine to me, obviously impacts the existing arches very > > > > > > little. Can't see why it shouldn't get included, > > > > > > > > > > I find it a bit disconcerting that blackfin already depends on this > > > > > in-tree without there being any earlier discussion on making these > > > > > changes. > > > > > > > > not really ... this patch was posted before but was lost in the > > > > shuffle ... and i'm not quite sure what you mean by "depends on" ... > > > > if you want to use the FLAT file format on a Blackfin processor, then > > > > this patch is needed, but that isnt the only file format that works on > > > > the Blackfin port as we also have FDPIC ELF > > > > > > > What I mean by "depends on" is that for what you are attempting to > > > patch, your architecture has an in-tree dependency on something that > > > hasn't > > > been discussed. > > > > "not been discussed" because it was sent to lkml - > > http://lkml.org/lkml/2006/9/22/60 > > - and it got missed/left on the floor during the arch/blackfin inclusion > > (which was huge), not because of any deliberate malicious intent on our > > part > > to mislead or try to get this in now by doing an end around as you are > > implying. > > > > Yes, as Robin pointed out, this patch was sent to LKML at least 3 times > including Bernd's email. This is the 4th round. > http://lkml.org/lkml/2007/5/29/24 > http://lkml.org/lkml/2007/5/28/63 > > I don't wanna to resend it again and again to annoy the receiver and > LKML. But IMO, technically this patch looks fine to binfmt_flat and > other architectures. And the in-tree Blackfin port will fail to be > compiled with this patch if the BINFMT_FLAT is enabled. > Oops, typo. Correct one: it will fail to be compiled without this patch. > > Our mistake for not poking everyone/resending things sooner/before > > arch/blackfin was included. Bryan will try to make sure that doesn't happen > > again (right Bryan?) - like he is now, by poking/resending things, and > > making > > sure that the appropriate maintainers (like you, if we are changing things > > you maintain) are included. (which is what I think the problem was). > > > > If we can focus on the technical merits of things, rather than how we got > > here - which I agree sucks - was our mistake - we are sorry - we will try > > to > > make sure that it doesn't happen again - I think it would be time/effort > > better spent. > > > > Yes, I will try my best to avoid this happen again. But on the other > hand, several reasons made this mess happen: > a) not very easy found the maintainer information for several domain, > for example this patch should invite binfmt_flat maintainers, arch > maintainers (Thanks Robin to invite them), blackfin port maintainers and > toolchain maintainers. > b) even the maintainers got this patch email, they don't have time to > review or reply. So the patch was ignored by LKML and the sender, sorry > -:))). > c) There is a rule that do __NOT__ send the same patch again and again, > I don't wanna to break this rule. But if there is no feedback about the > patch, we have no choice but resent it or just ignore it. > > I know it is general problem in the LKML patch review. > > Thanks > -Bryan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, 2007-09-20 at 02:08 -0400, Robin Getz wrote: > On Wed 19 Sep 2007 23:54, Paul Mundt pondered: > > On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: > > > On 9/19/07, Paul Mundt <[EMAIL PROTECTED]> wrote: > > > > On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: > > > > > Jivin Robin Getz lays it down ... > > > > > > On Tue 18 Sep 2007 04:09, Bryan Wu pondered: > > > > > > > This just adds minimum support for the Blackfin relocations, > > > > > > > since we don't have enough space in each reloc. The idea > > > > > > > is to store a value with one relocation so that subsequent > > > > > > > ones can access it. > > > > > > > > > > > > Adding the other appropriate maintainers. for h8, m32r, sh and > > > > > > v850. > > > > > > > > > > Looks fine to me, obviously impacts the existing arches very > > > > > little. Can't see why it shouldn't get included, > > > > > > > > I find it a bit disconcerting that blackfin already depends on this > > > > in-tree without there being any earlier discussion on making these > > > > changes. > > > > > > not really ... this patch was posted before but was lost in the > > > shuffle ... and i'm not quite sure what you mean by "depends on" ... > > > if you want to use the FLAT file format on a Blackfin processor, then > > > this patch is needed, but that isnt the only file format that works on > > > the Blackfin port as we also have FDPIC ELF > > > > > What I mean by "depends on" is that for what you are attempting to > > patch, your architecture has an in-tree dependency on something that hasn't > > been discussed. > > "not been discussed" because it was sent to lkml - > http://lkml.org/lkml/2006/9/22/60 > - and it got missed/left on the floor during the arch/blackfin inclusion > (which was huge), not because of any deliberate malicious intent on our part > to mislead or try to get this in now by doing an end around as you are > implying. > Yes, as Robin pointed out, this patch was sent to LKML at least 3 times including Bernd's email. This is the 4th round. http://lkml.org/lkml/2007/5/29/24 http://lkml.org/lkml/2007/5/28/63 I don't wanna to resend it again and again to annoy the receiver and LKML. But IMO, technically this patch looks fine to binfmt_flat and other architectures. And the in-tree Blackfin port will fail to be compiled with this patch if the BINFMT_FLAT is enabled. > Our mistake for not poking everyone/resending things sooner/before > arch/blackfin was included. Bryan will try to make sure that doesn't happen > again (right Bryan?) - like he is now, by poking/resending things, and making > sure that the appropriate maintainers (like you, if we are changing things > you maintain) are included. (which is what I think the problem was). > > If we can focus on the technical merits of things, rather than how we got > here - which I agree sucks - was our mistake - we are sorry - we will try to > make sure that it doesn't happen again - I think it would be time/effort > better spent. > Yes, I will try my best to avoid this happen again. But on the other hand, several reasons made this mess happen: a) not very easy found the maintainer information for several domain, for example this patch should invite binfmt_flat maintainers, arch maintainers (Thanks Robin to invite them), blackfin port maintainers and toolchain maintainers. b) even the maintainers got this patch email, they don't have time to review or reply. So the patch was ignored by LKML and the sender, sorry -:))). c) There is a rule that do __NOT__ send the same patch again and again, I don't wanna to break this rule. But if there is no feedback about the patch, we have no choice but resent it or just ignore it. I know it is general problem in the LKML patch review. Thanks -Bryan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Wed 19 Sep 2007 23:54, Paul Mundt pondered: > On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: > > On 9/19/07, Paul Mundt <[EMAIL PROTECTED]> wrote: > > > On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: > > > > Jivin Robin Getz lays it down ... > > > > > On Tue 18 Sep 2007 04:09, Bryan Wu pondered: > > > > > > This just adds minimum support for the Blackfin relocations, > > > > > > since we don't have enough space in each reloc. The idea > > > > > > is to store a value with one relocation so that subsequent > > > > > > ones can access it. > > > > > > > > > > Adding the other appropriate maintainers. for h8, m32r, sh and > > > > > v850. > > > > > > > > Looks fine to me, obviously impacts the existing arches very > > > > little. Can't see why it shouldn't get included, > > > > > > I find it a bit disconcerting that blackfin already depends on this > > > in-tree without there being any earlier discussion on making these > > > changes. > > > > not really ... this patch was posted before but was lost in the > > shuffle ... and i'm not quite sure what you mean by "depends on" ... > > if you want to use the FLAT file format on a Blackfin processor, then > > this patch is needed, but that isnt the only file format that works on > > the Blackfin port as we also have FDPIC ELF > > > What I mean by "depends on" is that for what you are attempting to > patch, your architecture has an in-tree dependency on something that hasn't > been discussed. "not been discussed" because it was sent to lkml - http://lkml.org/lkml/2006/9/22/60 - and it got missed/left on the floor during the arch/blackfin inclusion (which was huge), not because of any deliberate malicious intent on our part to mislead or try to get this in now by doing an end around as you are implying. Our mistake for not poking everyone/resending things sooner/before arch/blackfin was included. Bryan will try to make sure that doesn't happen again (right Bryan?) - like he is now, by poking/resending things, and making sure that the appropriate maintainers (like you, if we are changing things you maintain) are included. (which is what I think the problem was). If we can focus on the technical merits of things, rather than how we got here - which I agree sucks - was our mistake - we are sorry - we will try to make sure that it doesn't happen again - I think it would be time/effort better spent. -Robin BTW - does anyone know Miles Bader's current email address? There isn't one listed in MAINTAINERS - and he is still listed for the NEC V850? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Wed 19 Sep 2007 23:54, Paul Mundt pondered: On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: On 9/19/07, Paul Mundt [EMAIL PROTECTED] wrote: On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: Jivin Robin Getz lays it down ... On Tue 18 Sep 2007 04:09, Bryan Wu pondered: This just adds minimum support for the Blackfin relocations, since we don't have enough space in each reloc. The idea is to store a value with one relocation so that subsequent ones can access it. Adding the other appropriate maintainers. for h8, m32r, sh and v850. Looks fine to me, obviously impacts the existing arches very little. Can't see why it shouldn't get included, I find it a bit disconcerting that blackfin already depends on this in-tree without there being any earlier discussion on making these changes. not really ... this patch was posted before but was lost in the shuffle ... and i'm not quite sure what you mean by depends on ... if you want to use the FLAT file format on a Blackfin processor, then this patch is needed, but that isnt the only file format that works on the Blackfin port as we also have FDPIC ELF What I mean by depends on is that for what you are attempting to patch, your architecture has an in-tree dependency on something that hasn't been discussed. not been discussed because it was sent to lkml - http://lkml.org/lkml/2006/9/22/60 - and it got missed/left on the floor during the arch/blackfin inclusion (which was huge), not because of any deliberate malicious intent on our part to mislead or try to get this in now by doing an end around as you are implying. Our mistake for not poking everyone/resending things sooner/before arch/blackfin was included. Bryan will try to make sure that doesn't happen again (right Bryan?) - like he is now, by poking/resending things, and making sure that the appropriate maintainers (like you, if we are changing things you maintain) are included. (which is what I think the problem was). If we can focus on the technical merits of things, rather than how we got here - which I agree sucks - was our mistake - we are sorry - we will try to make sure that it doesn't happen again - I think it would be time/effort better spent. -Robin BTW - does anyone know Miles Bader's current email address? There isn't one listed in MAINTAINERS - and he is still listed for the NEC V850? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, 2007-09-20 at 02:08 -0400, Robin Getz wrote: On Wed 19 Sep 2007 23:54, Paul Mundt pondered: On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: On 9/19/07, Paul Mundt [EMAIL PROTECTED] wrote: On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: Jivin Robin Getz lays it down ... On Tue 18 Sep 2007 04:09, Bryan Wu pondered: This just adds minimum support for the Blackfin relocations, since we don't have enough space in each reloc. The idea is to store a value with one relocation so that subsequent ones can access it. Adding the other appropriate maintainers. for h8, m32r, sh and v850. Looks fine to me, obviously impacts the existing arches very little. Can't see why it shouldn't get included, I find it a bit disconcerting that blackfin already depends on this in-tree without there being any earlier discussion on making these changes. not really ... this patch was posted before but was lost in the shuffle ... and i'm not quite sure what you mean by depends on ... if you want to use the FLAT file format on a Blackfin processor, then this patch is needed, but that isnt the only file format that works on the Blackfin port as we also have FDPIC ELF What I mean by depends on is that for what you are attempting to patch, your architecture has an in-tree dependency on something that hasn't been discussed. not been discussed because it was sent to lkml - http://lkml.org/lkml/2006/9/22/60 - and it got missed/left on the floor during the arch/blackfin inclusion (which was huge), not because of any deliberate malicious intent on our part to mislead or try to get this in now by doing an end around as you are implying. Yes, as Robin pointed out, this patch was sent to LKML at least 3 times including Bernd's email. This is the 4th round. http://lkml.org/lkml/2007/5/29/24 http://lkml.org/lkml/2007/5/28/63 I don't wanna to resend it again and again to annoy the receiver and LKML. But IMO, technically this patch looks fine to binfmt_flat and other architectures. And the in-tree Blackfin port will fail to be compiled with this patch if the BINFMT_FLAT is enabled. Our mistake for not poking everyone/resending things sooner/before arch/blackfin was included. Bryan will try to make sure that doesn't happen again (right Bryan?) - like he is now, by poking/resending things, and making sure that the appropriate maintainers (like you, if we are changing things you maintain) are included. (which is what I think the problem was). If we can focus on the technical merits of things, rather than how we got here - which I agree sucks - was our mistake - we are sorry - we will try to make sure that it doesn't happen again - I think it would be time/effort better spent. Yes, I will try my best to avoid this happen again. But on the other hand, several reasons made this mess happen: a) not very easy found the maintainer information for several domain, for example this patch should invite binfmt_flat maintainers, arch maintainers (Thanks Robin to invite them), blackfin port maintainers and toolchain maintainers. b) even the maintainers got this patch email, they don't have time to review or reply. So the patch was ignored by LKML and the sender, sorry -:))). c) There is a rule that do __NOT__ send the same patch again and again, I don't wanna to break this rule. But if there is no feedback about the patch, we have no choice but resent it or just ignore it. I know it is general problem in the LKML patch review. Thanks -Bryan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, 2007-09-20 at 14:34 +0800, Bryan Wu wrote: On Thu, 2007-09-20 at 02:08 -0400, Robin Getz wrote: On Wed 19 Sep 2007 23:54, Paul Mundt pondered: On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: On 9/19/07, Paul Mundt [EMAIL PROTECTED] wrote: On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: Jivin Robin Getz lays it down ... On Tue 18 Sep 2007 04:09, Bryan Wu pondered: This just adds minimum support for the Blackfin relocations, since we don't have enough space in each reloc. The idea is to store a value with one relocation so that subsequent ones can access it. Adding the other appropriate maintainers. for h8, m32r, sh and v850. Looks fine to me, obviously impacts the existing arches very little. Can't see why it shouldn't get included, I find it a bit disconcerting that blackfin already depends on this in-tree without there being any earlier discussion on making these changes. not really ... this patch was posted before but was lost in the shuffle ... and i'm not quite sure what you mean by depends on ... if you want to use the FLAT file format on a Blackfin processor, then this patch is needed, but that isnt the only file format that works on the Blackfin port as we also have FDPIC ELF What I mean by depends on is that for what you are attempting to patch, your architecture has an in-tree dependency on something that hasn't been discussed. not been discussed because it was sent to lkml - http://lkml.org/lkml/2006/9/22/60 - and it got missed/left on the floor during the arch/blackfin inclusion (which was huge), not because of any deliberate malicious intent on our part to mislead or try to get this in now by doing an end around as you are implying. Yes, as Robin pointed out, this patch was sent to LKML at least 3 times including Bernd's email. This is the 4th round. http://lkml.org/lkml/2007/5/29/24 http://lkml.org/lkml/2007/5/28/63 I don't wanna to resend it again and again to annoy the receiver and LKML. But IMO, technically this patch looks fine to binfmt_flat and other architectures. And the in-tree Blackfin port will fail to be compiled with this patch if the BINFMT_FLAT is enabled. Oops, typo. Correct one: it will fail to be compiled without this patch. Our mistake for not poking everyone/resending things sooner/before arch/blackfin was included. Bryan will try to make sure that doesn't happen again (right Bryan?) - like he is now, by poking/resending things, and making sure that the appropriate maintainers (like you, if we are changing things you maintain) are included. (which is what I think the problem was). If we can focus on the technical merits of things, rather than how we got here - which I agree sucks - was our mistake - we are sorry - we will try to make sure that it doesn't happen again - I think it would be time/effort better spent. Yes, I will try my best to avoid this happen again. But on the other hand, several reasons made this mess happen: a) not very easy found the maintainer information for several domain, for example this patch should invite binfmt_flat maintainers, arch maintainers (Thanks Robin to invite them), blackfin port maintainers and toolchain maintainers. b) even the maintainers got this patch email, they don't have time to review or reply. So the patch was ignored by LKML and the sender, sorry -:))). c) There is a rule that do __NOT__ send the same patch again and again, I don't wanna to break this rule. But if there is no feedback about the patch, we have no choice but resent it or just ignore it. I know it is general problem in the LKML patch review. Thanks -Bryan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Robin Getz [EMAIL PROTECTED] writes: BTW - does anyone know Miles Bader's current email address? There isn't one listed in MAINTAINERS - and he is still listed for the NEC V850? [EMAIL PROTECTED] -Miles -- Everywhere is walking distance if you have the time. -- Steven Wright - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
From: David McCullough [EMAIL PROTECTED] Date: Thu, 20 Sep 2007 11:55:25 +1000 Jivin Robin Getz lays it down ... On Tue 18 Sep 2007 04:09, Bryan Wu pondered: From: Bernd Schmidt [EMAIL PROTECTED] This just adds minimum support for the Blackfin relocations, since we don't have enough space in each reloc. The idea is to store a value with one relocation so that subsequent ones can access it. Signed-off-by: Bernd Schmidt [EMAIL PROTECTED] Signed-off-by: Bryan Wu [EMAIL PROTECTED] Cc: David McCullough [EMAIL PROTECTED] Cc: Greg Ungerer [EMAIL PROTECTED] Adding the other appropriate maintainers. for h8, m32r, sh and v850. Looks fine to me, obviously impacts the existing arches very little. Can't see why it shouldn't get included, Cheers, Davidm Looks fine to me, too. Acked-by: Hirokazu Takata [EMAIL PROTECTED] -- Hirokazu Takata [EMAIL PROTECTED] Linux/M32R Project: http://www.linux-m32r.org/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Paul Mundt wrote:
I find it a bit disconcerting that blackfin already depends on this
in-tree without there being any earlier discussion on making these
changes.
Parts of the initial submission were picked up (the include/asm
directory), other's weren't. Little we can do about that.
*/
if (rev OLD_FLAT_VERSION) {
+ unsigned long persistent = 0;
for (i=0; i relocs; i++) {
unsigned long addr, relval;
@@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
relocated (of course, the address has to be
relocated first). */
relval = ntohl(reloc[i]);
+ if (flat_set_persistent (relval, persistent))
+ continue;
addr = flat_get_relocate_addr(relval);
rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
if (rp == (unsigned long *)RELOC_FAILED) {
I don't much care for this API. It's shuffling around a temporary
variable for the architecture code that's set for certain relocations
that are otherwise unhandled.
Since all the architecture is interested in is the relval that has
associated persistent data encoded in it, why don't we just have a stub
to give the architecture a chance to validate the relval before the
flat_get_relocate_addr() and move this stuff there instead? ie, blackfin
takes this out-of-line and manages its persistent value there.
What is flat_set_persistent other than a stub to validate the relval?
I'm not at all sure what you're proposing or how it would be different.
load_flat_file() is ugly enough without dumping more architecture
callback abuses in it.
The other maintainers who have spoken up didn't seem to think this was
ugly, or an abuse. I'm surprised to hear language like that when
discussing a patch that adds an if statement, a local variable and one
parameter in a function call.
Bernd
--
This footer brought to you by insane German lawmakers.
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, Sep 20, 2007 at 02:04:26PM +0200, Bernd Schmidt wrote: Paul Mundt wrote: I find it a bit disconcerting that blackfin already depends on this in-tree without there being any earlier discussion on making these changes. Parts of the initial submission were picked up (the include/asm directory), other's weren't. Little we can do about that. What you can do about that is to order the patches, so one doesn't get applied ahead of the other. People have successfully managed to submit patches with dependencies in the past, you can look through the archives for examples. Since all the architecture is interested in is the relval that has associated persistent data encoded in it, why don't we just have a stub to give the architecture a chance to validate the relval before the flat_get_relocate_addr() and move this stuff there instead? ie, blackfin takes this out-of-line and manages its persistent value there. What is flat_set_persistent other than a stub to validate the relval? I'm not at all sure what you're proposing or how it would be different. My apologies for not making it clearer. I did not get a chance to hack together a patch today. Hopefully the below will clear up whatever confusion there was. load_flat_file() is ugly enough without dumping more architecture callback abuses in it. The other maintainers who have spoken up didn't seem to think this was ugly, or an abuse. I'm surprised to hear language like that when discussing a patch that adds an if statement, a local variable and one parameter in a function call. Yes, well, the scope of the changes is really rather irrelevant, it's the technical basis behind it that should be the concern. My suggestion was to lose the local variable, get rid of this persistent API, and plug in something like flat_validate_relval() or something equally silly where each architecture has the option to grab the relval and set up whatever sort of state it wants to out-of-line. This is making API changes where it's convenient for your platform to use this value, and there's no reason to change the API here at all. The if/continue block are not an issue, it is the whole concept of persistent data in this case that has no need to be applied uniformally across all other architectures. Why should all architectures have to change their APIs (not just adding new things mind you, also changing existing definitions) to accomodate something that can trivially be kept in the blackfin code? I wager the other maintainers either a) don't care because it doesn't effect them, or b) have resigned binfmt_flat to its fate. Neither option is particularly appealing in terms of getting that code tidied. That sort of indifference is largely why binfmt_flat is as much of a mess as it is today. Your patch is certainly not the cause of this, the architecture callbacks there are just a mess to begin with, I would prefer that we try to keep new additions flexible without blindly hardcoding more usage assumptions all over the place and having to paper over them again later. I can hack up some patches tomorrow if you're still unsure of what I'm proposing. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Paul Mundt wrote: This is making API changes where it's convenient for your platform to use this value, and there's no reason to change the API here at all. Your proposed addition of flat_validate_relval is an API change, and very similar in nature to what I've done. A local variable here is the most natural way to store this information. What do you suggest we use, a global? A member in the task struct? Why should all architectures have to change their APIs (not just adding new things mind you, also changing existing definitions) to accomodate something that can trivially be kept in the blackfin code? I don't see how there's a burden given that we've provided patches, and most maintainers have already said their fine with it. It seems to me that it's a natural and common thing for many architectures to be updated when new things are added to core code. Bernd -- This footer brought to you by insane German lawmakers. Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368 Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Jivin Paul Mundt lays it down ... ... The other maintainers who have spoken up didn't seem to think this was ugly, or an abuse. I'm surprised to hear language like that when discussing a patch that adds an if statement, a local variable and one parameter in a function call. Yes, well, the scope of the changes is really rather irrelevant, it's the technical basis behind it that should be the concern. My suggestion was to lose the local variable, get rid of this persistent API, and plug in something like flat_validate_relval() or something equally silly where each architecture has the option to grab the relval and set up whatever sort of state it wants to out-of-line. This is making API changes where it's convenient for your platform to use this value, and there's no reason to change the API here at all. The if/continue block are not an issue, it is the whole concept of persistent data in this case that has no need to be applied uniformally across all other architectures. Why should all architectures have to change their APIs (not just adding new things mind you, also changing existing definitions) to accomodate something that can trivially be kept in the blackfin code? Fair comment. I hadn't considered that it could be even cleaner. If it's easily doable then I agree with your concerns. I wager the other maintainers either a) don't care because it doesn't effect them, or b) have resigned binfmt_flat to its fate. Neither option is particularly appealing in terms of getting that code tidied. That sort of indifference is largely why binfmt_flat is as much of a mess as it is today. Your patch is certainly not the cause of this, the architecture callbacks there are just a mess to begin with, I would prefer that we try to keep new additions flexible without blindly hardcoding more usage assumptions all over the place and having to paper over them again later. I would say that (a) is definately not the case. I am sure the BF guys will say they have been banging us on the head with changes for a long time and getting no where as we considered the changes to severe or out of line. This particular patch was trivial in comparison to others I've seen, it fixed all the existing arches (not something that is always done) and seemed a reasonable start to finally get the BF guys up and running. Still, happy to make it better of course ;-) As for (b), I think it will be around for a while. It's not as flexible as fdpic, but it's still a tiny, simple format and not everyone has an fdpic implementation yet :-) Cheers, Davidm -- David McCullough, [EMAIL PROTECTED], Ph:+61 734352815 Secure Computing - SnapGear http://www.uCdot.org http://www.cyberguard.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu 20 Sep 2007 11:03, David McCullough pondered: I would say that (a) is definately not the case. I am sure the BF guys will say they have been banging us on the head with changes for a long time and getting no where as we considered the changes to severe or out of line. I don't think we have been banging heads with you (unless that is your feeling?) - how about working together, but diverting to satisfy different needs :) I think that we have had more issues in the uClinux-dist (userspace and build environment), but for kernel code, we have moved from some non-standard (stupid) things we were doing early on to what we have today - which is as common/standard with other archs as we can be. Although this is slightly off topic - on the uClinux distribution side - most of our changes are based on requirements/desires from being able to support fdpic elf and flat formats, and to attempt to make things easier for end users/us to use/maintain. Where we do make changes - we always send the patch upstream and have the conversation with you (not everyone else does this), and some/most times rework things so they are more acceptable to you. We don't always come to an agreement - but we always have the discussion, and are willing to move if we can make things better that still meets both our needs/desires. This particular patch was trivial in comparison to others I've seen, That is what we thought. it fixed all the existing arches (not something that is always done) and seemed a reasonable start to finally get the BF guys up and running. Still, happy to make it better of course ;-) As always - we are more than happy to explore/review alternative patches if people want to write/sumbit them. -Robin - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Jivin Robin Getz lays it down ... On Thu 20 Sep 2007 11:03, David McCullough pondered: I would say that (a) is definately not the case. I am sure the BF guys will say they have been banging us on the head with changes for a long time and getting no where as we considered the changes to severe or out of line. I don't think we have been banging heads with you (unless that is your feeling?) - how about working together, but diverting to satisfy different needs :) No head banging feelings here, but I would understand if you guys felt that way occasionally ;-) I obviously forgot the happy face on that statement. It was meant as a good thing. I think that we have had more issues in the uClinux-dist (userspace and build environment), but for kernel code, we have moved from some non-standard (stupid) things we were doing early on to what we have today - which is as common/standard with other archs as we can be. Although this is slightly off topic - on the uClinux distribution side - most of our changes are based on requirements/desires from being able to support fdpic elf and flat formats, and to attempt to make things easier for end users/us to use/maintain. Where we do make changes - we always send the patch upstream and have the conversation with you (not everyone else does this), and some/most times rework things so they are more acceptable to you. We don't always come to an agreement - but we always have the discussion, and are willing to move if we can make things better that still meets both our needs/desires. This particular patch was trivial in comparison to others I've seen, That is what we thought. it fixed all the existing arches (not something that is always done) and seemed a reasonable start to finally get the BF guys up and running. Still, happy to make it better of course ;-) As always - we are more than happy to explore/review alternative patches if people want to write/sumbit them. Cheers, Davidm -- David McCullough, [EMAIL PROTECTED], Ph:+61 734352815 Secure Computing - SnapGear http://www.uCdot.org http://www.cyberguard.com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: > On 9/19/07, Paul Mundt <[EMAIL PROTECTED]> wrote: > > On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: > > > Jivin Robin Getz lays it down ... > > > > On Tue 18 Sep 2007 04:09, Bryan Wu pondered: > > > > > This just adds minimum support for the Blackfin relocations, > > > > > since we don't have enough space in each reloc. The idea > > > > > is to store a value with one relocation so that subsequent ones can > > > > > access it. > > > > > > > > Adding the other appropriate maintainers. for h8, m32r, sh and v850. > > > > > > Looks fine to me, obviously impacts the existing arches very little. > > > Can't see why it shouldn't get included, > > > > I find it a bit disconcerting that blackfin already depends on this > > in-tree without there being any earlier discussion on making these > > changes. > > not really ... this patch was posted before but was lost in the > shuffle ... and i'm not quite sure what you mean by "depends on" ... > if you want to use the FLAT file format on a Blackfin processor, then > this patch is needed, but that isnt the only file format that works on > the Blackfin port as we also have FDPIC ELF > What I mean by "depends on" is that for what you are attempting to patch, your architecture has an in-tree dependency on something that hasn't been discussed. It's more than a bit backwards to push the follow-up bits before even getting an Ack on the changes you want to make in the common code. The fact you have other options is great, so at least you haven't shafted all of your git kernel users, but that's not the point. You should not be pushing things in to the kernel that have dependencies on API changes that may or may not be merged, especially when you're breaking the entire kernel for your platform (and the ability to bisect for those users) in the process. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On 9/19/07, Paul Mundt <[EMAIL PROTECTED]> wrote: > On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: > > Jivin Robin Getz lays it down ... > > > On Tue 18 Sep 2007 04:09, Bryan Wu pondered: > > > > This just adds minimum support for the Blackfin relocations, > > > > since we don't have enough space in each reloc. The idea > > > > is to store a value with one relocation so that subsequent ones can > > > > access it. > > > > > > Adding the other appropriate maintainers. for h8, m32r, sh and v850. > > > > Looks fine to me, obviously impacts the existing arches very little. > > Can't see why it shouldn't get included, > > I find it a bit disconcerting that blackfin already depends on this > in-tree without there being any earlier discussion on making these > changes. not really ... this patch was posted before but was lost in the shuffle ... and i'm not quite sure what you mean by "depends on" ... if you want to use the FLAT file format on a Blackfin processor, then this patch is needed, but that isnt the only file format that works on the Blackfin port as we also have FDPIC ELF i havent used FLAT files on a Blackfin board in quite a long time actually ... -mike - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote:
> Jivin Robin Getz lays it down ...
> > On Tue 18 Sep 2007 04:09, Bryan Wu pondered:
> > > This just adds minimum support for the Blackfin relocations,
> > > since we don't have enough space in each reloc. The idea
> > > is to store a value with one relocation so that subsequent ones can
> > > access it.
> >
> > Adding the other appropriate maintainers. for h8, m32r, sh and v850.
>
> Looks fine to me, obviously impacts the existing arches very little.
> Can't see why it shouldn't get included,
>
I find it a bit disconcerting that blackfin already depends on this
in-tree without there being any earlier discussion on making these
changes.
> > >*/
> > > if (rev > OLD_FLAT_VERSION) {
> > > + unsigned long persistent = 0;
> > > for (i=0; i < relocs; i++) {
> > > unsigned long addr, relval;
> > >
> > > @@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
> > > relocated (of course, the address has to be
> > > relocated first). */
> > > relval = ntohl(reloc[i]);
> > > + if (flat_set_persistent (relval, ))
> > > + continue;
> > > addr = flat_get_relocate_addr(relval);
> > > rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
> > > if (rp == (unsigned long *)RELOC_FAILED) {
I don't much care for this API. It's shuffling around a temporary
variable for the architecture code that's set for certain relocations
that are otherwise unhandled.
Since all the architecture is interested in is the relval that has
associated "persistent" data encoded in it, why don't we just have a stub
to give the architecture a chance to validate the relval before the
flat_get_relocate_addr() and move this stuff there instead? ie, blackfin
takes this out-of-line and manages its persistent value there.
> > > @@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
> > > }
> > >
> > > /* Get the pointer's value. */
> > > - addr = flat_get_addr_from_rp(rp, relval, flags);
> > > + addr = flat_get_addr_from_rp(rp, relval, flags,
> > > );
There's no reason for this to be a pointer. The current in-tree blackfin
code doesn't change this value, and if you have patches that are doing
that, this is even more reason to bury this kind of silliness in your
architecture code.
load_flat_file() is ugly enough without dumping more architecture
callback abuses in it.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Wed 19 Sep 2007 21:55, David McCullough pondered: > Jivin Robin Getz lays it down ... > > On Tue 18 Sep 2007 04:09, Bryan Wu pondered: > > > From: Bernd Schmidt <[EMAIL PROTECTED]> > > > > > > This just adds minimum support for the Blackfin relocations, > > > since we don't have enough space in each reloc. The idea > > > is to store a value with one relocation so that subsequent ones can > > > access it. > > > > > > Signed-off-by: Bernd Schmidt <[EMAIL PROTECTED]> > > > Signed-off-by: Bryan Wu <[EMAIL PROTECTED]> > > > Cc: David McCullough <[EMAIL PROTECTED]> > > > Cc: Greg Ungerer <[EMAIL PROTECTED]> > > > > Adding the other appropriate maintainers. for h8, m32r, sh and v850. > > Looks fine to me, obviously impacts the existing arches very little. > Can't see why it shouldn't get included, Is that Australian for Acked-by:? :) -Robin - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Jivin Robin Getz lays it down ...
> On Tue 18 Sep 2007 04:09, Bryan Wu pondered:
> > From: Bernd Schmidt <[EMAIL PROTECTED]>
> >
> > This just adds minimum support for the Blackfin relocations,
> > since we don't have enough space in each reloc. The idea
> > is to store a value with one relocation so that subsequent ones can
> > access it.
> >
> > Signed-off-by: Bernd Schmidt <[EMAIL PROTECTED]>
> > Signed-off-by: Bryan Wu <[EMAIL PROTECTED]>
> > Cc: David McCullough <[EMAIL PROTECTED]>
> > Cc: Greg Ungerer <[EMAIL PROTECTED]>
>
> Adding the other appropriate maintainers. for h8, m32r, sh and v850.
Looks fine to me, obviously impacts the existing arches very little.
Can't see why it shouldn't get included,
Cheers,
Davidm
> > ---
> > fs/binfmt_flat.c |5 -
> > include/asm-h8300/flat.h |3 ++-
> > include/asm-m32r/flat.h |3 ++-
> > include/asm-m68knommu/flat.h |3 ++-
> > include/asm-sh/flat.h|3 ++-
> > include/asm-v850/flat.h |4 +++-
> > 6 files changed, 15 insertions(+), 6 deletions(-)
> > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> > index 7b0265d..13b58f8 100644
> > --- a/fs/binfmt_flat.c
> > +++ b/fs/binfmt_flat.c
> > @@ -742,6 +742,7 @@ static int load_flat_file(struct linux_binprm * bprm,
> > * __start to address 4 so that is okay).
> > */
> > if (rev > OLD_FLAT_VERSION) {
> > + unsigned long persistent = 0;
> > for (i=0; i < relocs; i++) {
> > unsigned long addr, relval;
> >
> > @@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
> >relocated (of course, the address has to be
> >relocated first). */
> > relval = ntohl(reloc[i]);
> > + if (flat_set_persistent (relval, ))
> > + continue;
> > addr = flat_get_relocate_addr(relval);
> > rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
> > if (rp == (unsigned long *)RELOC_FAILED) {
> > @@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
> > }
> >
> > /* Get the pointer's value. */
> > - addr = flat_get_addr_from_rp(rp, relval, flags);
> > + addr = flat_get_addr_from_rp(rp, relval, flags,
> > );
> > if (addr != 0) {
> > /*
> > * Do the relocation. PIC relocs in the data
> > section are
> > diff --git a/include/asm-h8300/flat.h b/include/asm-h8300/flat.h
> > index c20eee7..2a87350 100644
> > --- a/include/asm-h8300/flat.h
> > +++ b/include/asm-h8300/flat.h
> > @@ -9,6 +9,7 @@
> > #defineflat_argvp_envp_on_stack() 1
> > #defineflat_old_ram_flag(flags)1
> > #defineflat_reloc_valid(reloc, size) ((reloc) <= (size))
> > +#defineflat_set_persistent(relval, p) 0
> >
> > /*
> > * on the H8 a couple of the relocations have an instruction in the
> > @@ -18,7 +19,7 @@
> > */
> >
> > #defineflat_get_relocate_addr(rel) (rel)
> > -#define flat_get_addr_from_rp(rp, relval, flags) \
> > +#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
> > (get_unaligned(rp) & ((flags & FLAT_FLAG_GOTPIC) ? 0x:
> > 0x00ff))
> > #define flat_put_addr_at_rp(rp, addr, rel) \
> > put_unaligned (((*(char *)(rp)) << 24) | ((addr) & 0x00ff), rp)
> > diff --git a/include/asm-m32r/flat.h b/include/asm-m32r/flat.h
> > index 1b285f6..d851cf0 100644
> > --- a/include/asm-m32r/flat.h
> > +++ b/include/asm-m32r/flat.h
> > @@ -15,9 +15,10 @@
> > #defineflat_stack_align(sp)(*sp += (*sp & 3 ? (4 - (*sp &
> > 3)): 0))
> > #defineflat_argvp_envp_on_stack() 0
> > #defineflat_old_ram_flag(flags)(flags)
> > +#defineflat_set_persistent(relval, p) 0
> > #defineflat_reloc_valid(reloc, size) \
> > (((reloc) - textlen_for_m32r_lo16_data) <= (size))
> > -#define flat_get_addr_from_rp(rp, relval, flags) \
> > +#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
> > m32r_flat_get_addr_from_rp(rp, relval, (text_len) )
> >
> > #define flat_put_addr_at_rp(rp, addr, relval) \
> > diff --git a/include/asm-m68knommu/flat.h b/include/asm-m68knommu/flat.h
> > index 2d836ed..814b517 100644
> > --- a/include/asm-m68knommu/flat.h
> > +++ b/include/asm-m68knommu/flat.h
> > @@ -9,8 +9,9 @@
> > #defineflat_argvp_envp_on_stack() 1
> > #defineflat_old_ram_flag(flags)(flags)
> > #defineflat_reloc_valid(reloc, size) ((reloc) <= (size))
> > -#defineflat_get_addr_from_rp(rp, relval, flags) get_unaligned(rp)
> > +#defineflat_get_addr_from_rp(rp, relval, flags, p) get_unaligned(rp)
> > #defineflat_put_addr_at_rp(rp, val,
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Tue 18 Sep 2007 04:09, Bryan Wu pondered:
> From: Bernd Schmidt <[EMAIL PROTECTED]>
>
> This just adds minimum support for the Blackfin relocations,
> since we don't have enough space in each reloc. The idea
> is to store a value with one relocation so that subsequent ones can
> access it.
>
> Signed-off-by: Bernd Schmidt <[EMAIL PROTECTED]>
> Signed-off-by: Bryan Wu <[EMAIL PROTECTED]>
> Cc: David McCullough <[EMAIL PROTECTED]>
> Cc: Greg Ungerer <[EMAIL PROTECTED]>
Adding the other appropriate maintainers. for h8, m32r, sh and v850.
> ---
> fs/binfmt_flat.c |5 -
> include/asm-h8300/flat.h |3 ++-
> include/asm-m32r/flat.h |3 ++-
> include/asm-m68knommu/flat.h |3 ++-
> include/asm-sh/flat.h|3 ++-
> include/asm-v850/flat.h |4 +++-
> 6 files changed, 15 insertions(+), 6 deletions(-)
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 7b0265d..13b58f8 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -742,6 +742,7 @@ static int load_flat_file(struct linux_binprm * bprm,
>* __start to address 4 so that is okay).
>*/
> if (rev > OLD_FLAT_VERSION) {
> + unsigned long persistent = 0;
> for (i=0; i < relocs; i++) {
> unsigned long addr, relval;
>
> @@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
> relocated (of course, the address has to be
> relocated first). */
> relval = ntohl(reloc[i]);
> + if (flat_set_persistent (relval, ))
> + continue;
> addr = flat_get_relocate_addr(relval);
> rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
> if (rp == (unsigned long *)RELOC_FAILED) {
> @@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
> }
>
> /* Get the pointer's value. */
> - addr = flat_get_addr_from_rp(rp, relval, flags);
> + addr = flat_get_addr_from_rp(rp, relval, flags,
> );
> if (addr != 0) {
> /*
>* Do the relocation. PIC relocs in the data
> section are
> diff --git a/include/asm-h8300/flat.h b/include/asm-h8300/flat.h
> index c20eee7..2a87350 100644
> --- a/include/asm-h8300/flat.h
> +++ b/include/asm-h8300/flat.h
> @@ -9,6 +9,7 @@
> #define flat_argvp_envp_on_stack() 1
> #define flat_old_ram_flag(flags)1
> #define flat_reloc_valid(reloc, size) ((reloc) <= (size))
> +#define flat_set_persistent(relval, p) 0
>
> /*
> * on the H8 a couple of the relocations have an instruction in the
> @@ -18,7 +19,7 @@
> */
>
> #define flat_get_relocate_addr(rel) (rel)
> -#define flat_get_addr_from_rp(rp, relval, flags) \
> +#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
> (get_unaligned(rp) & ((flags & FLAT_FLAG_GOTPIC) ? 0x:
> 0x00ff))
> #define flat_put_addr_at_rp(rp, addr, rel) \
> put_unaligned (((*(char *)(rp)) << 24) | ((addr) & 0x00ff), rp)
> diff --git a/include/asm-m32r/flat.h b/include/asm-m32r/flat.h
> index 1b285f6..d851cf0 100644
> --- a/include/asm-m32r/flat.h
> +++ b/include/asm-m32r/flat.h
> @@ -15,9 +15,10 @@
> #define flat_stack_align(sp)(*sp += (*sp & 3 ? (4 - (*sp &
> 3)): 0))
> #define flat_argvp_envp_on_stack() 0
> #define flat_old_ram_flag(flags)(flags)
> +#define flat_set_persistent(relval, p) 0
> #define flat_reloc_valid(reloc, size) \
> (((reloc) - textlen_for_m32r_lo16_data) <= (size))
> -#define flat_get_addr_from_rp(rp, relval, flags) \
> +#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
> m32r_flat_get_addr_from_rp(rp, relval, (text_len) )
>
> #define flat_put_addr_at_rp(rp, addr, relval) \
> diff --git a/include/asm-m68knommu/flat.h b/include/asm-m68knommu/flat.h
> index 2d836ed..814b517 100644
> --- a/include/asm-m68knommu/flat.h
> +++ b/include/asm-m68knommu/flat.h
> @@ -9,8 +9,9 @@
> #define flat_argvp_envp_on_stack() 1
> #define flat_old_ram_flag(flags)(flags)
> #define flat_reloc_valid(reloc, size) ((reloc) <= (size))
> -#define flat_get_addr_from_rp(rp, relval, flags) get_unaligned(rp)
> +#define flat_get_addr_from_rp(rp, relval, flags, p) get_unaligned(rp)
> #define flat_put_addr_at_rp(rp, val, relval) put_unaligned(val,rp)
> #define flat_get_relocate_addr(rel) (rel)
> +#define flat_set_persistent(relval, p) 0
>
> #endif /* __M68KNOMMU_FLAT_H__ */
> diff --git a/include/asm-sh/flat.h b/include/asm-sh/flat.h
> index 0d5cc04..dc4f595 100644
> ---
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Tue 18 Sep 2007 04:09, Bryan Wu pondered:
From: Bernd Schmidt [EMAIL PROTECTED]
This just adds minimum support for the Blackfin relocations,
since we don't have enough space in each reloc. The idea
is to store a value with one relocation so that subsequent ones can
access it.
Signed-off-by: Bernd Schmidt [EMAIL PROTECTED]
Signed-off-by: Bryan Wu [EMAIL PROTECTED]
Cc: David McCullough [EMAIL PROTECTED]
Cc: Greg Ungerer [EMAIL PROTECTED]
Adding the other appropriate maintainers. for h8, m32r, sh and v850.
---
fs/binfmt_flat.c |5 -
include/asm-h8300/flat.h |3 ++-
include/asm-m32r/flat.h |3 ++-
include/asm-m68knommu/flat.h |3 ++-
include/asm-sh/flat.h|3 ++-
include/asm-v850/flat.h |4 +++-
6 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 7b0265d..13b58f8 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -742,6 +742,7 @@ static int load_flat_file(struct linux_binprm * bprm,
* __start to address 4 so that is okay).
*/
if (rev OLD_FLAT_VERSION) {
+ unsigned long persistent = 0;
for (i=0; i relocs; i++) {
unsigned long addr, relval;
@@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
relocated (of course, the address has to be
relocated first). */
relval = ntohl(reloc[i]);
+ if (flat_set_persistent (relval, persistent))
+ continue;
addr = flat_get_relocate_addr(relval);
rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
if (rp == (unsigned long *)RELOC_FAILED) {
@@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
}
/* Get the pointer's value. */
- addr = flat_get_addr_from_rp(rp, relval, flags);
+ addr = flat_get_addr_from_rp(rp, relval, flags,
persistent);
if (addr != 0) {
/*
* Do the relocation. PIC relocs in the data
section are
diff --git a/include/asm-h8300/flat.h b/include/asm-h8300/flat.h
index c20eee7..2a87350 100644
--- a/include/asm-h8300/flat.h
+++ b/include/asm-h8300/flat.h
@@ -9,6 +9,7 @@
#define flat_argvp_envp_on_stack() 1
#define flat_old_ram_flag(flags)1
#define flat_reloc_valid(reloc, size) ((reloc) = (size))
+#define flat_set_persistent(relval, p) 0
/*
* on the H8 a couple of the relocations have an instruction in the
@@ -18,7 +19,7 @@
*/
#define flat_get_relocate_addr(rel) (rel)
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
(get_unaligned(rp) ((flags FLAT_FLAG_GOTPIC) ? 0x:
0x00ff))
#define flat_put_addr_at_rp(rp, addr, rel) \
put_unaligned (((*(char *)(rp)) 24) | ((addr) 0x00ff), rp)
diff --git a/include/asm-m32r/flat.h b/include/asm-m32r/flat.h
index 1b285f6..d851cf0 100644
--- a/include/asm-m32r/flat.h
+++ b/include/asm-m32r/flat.h
@@ -15,9 +15,10 @@
#define flat_stack_align(sp)(*sp += (*sp 3 ? (4 - (*sp
3)): 0))
#define flat_argvp_envp_on_stack() 0
#define flat_old_ram_flag(flags)(flags)
+#define flat_set_persistent(relval, p) 0
#define flat_reloc_valid(reloc, size) \
(((reloc) - textlen_for_m32r_lo16_data) = (size))
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
m32r_flat_get_addr_from_rp(rp, relval, (text_len) )
#define flat_put_addr_at_rp(rp, addr, relval) \
diff --git a/include/asm-m68knommu/flat.h b/include/asm-m68knommu/flat.h
index 2d836ed..814b517 100644
--- a/include/asm-m68knommu/flat.h
+++ b/include/asm-m68knommu/flat.h
@@ -9,8 +9,9 @@
#define flat_argvp_envp_on_stack() 1
#define flat_old_ram_flag(flags)(flags)
#define flat_reloc_valid(reloc, size) ((reloc) = (size))
-#define flat_get_addr_from_rp(rp, relval, flags) get_unaligned(rp)
+#define flat_get_addr_from_rp(rp, relval, flags, p) get_unaligned(rp)
#define flat_put_addr_at_rp(rp, val, relval) put_unaligned(val,rp)
#define flat_get_relocate_addr(rel) (rel)
+#define flat_set_persistent(relval, p) 0
#endif /* __M68KNOMMU_FLAT_H__ */
diff --git a/include/asm-sh/flat.h b/include/asm-sh/flat.h
index 0d5cc04..dc4f595 100644
--- a/include/asm-sh/flat.h
+++ b/include/asm-sh/flat.h
@@ -16,8 +16,9 @@
#define
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
Jivin Robin Getz lays it down ...
On Tue 18 Sep 2007 04:09, Bryan Wu pondered:
From: Bernd Schmidt [EMAIL PROTECTED]
This just adds minimum support for the Blackfin relocations,
since we don't have enough space in each reloc. The idea
is to store a value with one relocation so that subsequent ones can
access it.
Signed-off-by: Bernd Schmidt [EMAIL PROTECTED]
Signed-off-by: Bryan Wu [EMAIL PROTECTED]
Cc: David McCullough [EMAIL PROTECTED]
Cc: Greg Ungerer [EMAIL PROTECTED]
Adding the other appropriate maintainers. for h8, m32r, sh and v850.
Looks fine to me, obviously impacts the existing arches very little.
Can't see why it shouldn't get included,
Cheers,
Davidm
---
fs/binfmt_flat.c |5 -
include/asm-h8300/flat.h |3 ++-
include/asm-m32r/flat.h |3 ++-
include/asm-m68knommu/flat.h |3 ++-
include/asm-sh/flat.h|3 ++-
include/asm-v850/flat.h |4 +++-
6 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 7b0265d..13b58f8 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -742,6 +742,7 @@ static int load_flat_file(struct linux_binprm * bprm,
* __start to address 4 so that is okay).
*/
if (rev OLD_FLAT_VERSION) {
+ unsigned long persistent = 0;
for (i=0; i relocs; i++) {
unsigned long addr, relval;
@@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
relocated (of course, the address has to be
relocated first). */
relval = ntohl(reloc[i]);
+ if (flat_set_persistent (relval, persistent))
+ continue;
addr = flat_get_relocate_addr(relval);
rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
if (rp == (unsigned long *)RELOC_FAILED) {
@@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
}
/* Get the pointer's value. */
- addr = flat_get_addr_from_rp(rp, relval, flags);
+ addr = flat_get_addr_from_rp(rp, relval, flags,
persistent);
if (addr != 0) {
/*
* Do the relocation. PIC relocs in the data
section are
diff --git a/include/asm-h8300/flat.h b/include/asm-h8300/flat.h
index c20eee7..2a87350 100644
--- a/include/asm-h8300/flat.h
+++ b/include/asm-h8300/flat.h
@@ -9,6 +9,7 @@
#defineflat_argvp_envp_on_stack() 1
#defineflat_old_ram_flag(flags)1
#defineflat_reloc_valid(reloc, size) ((reloc) = (size))
+#defineflat_set_persistent(relval, p) 0
/*
* on the H8 a couple of the relocations have an instruction in the
@@ -18,7 +19,7 @@
*/
#defineflat_get_relocate_addr(rel) (rel)
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
(get_unaligned(rp) ((flags FLAT_FLAG_GOTPIC) ? 0x:
0x00ff))
#define flat_put_addr_at_rp(rp, addr, rel) \
put_unaligned (((*(char *)(rp)) 24) | ((addr) 0x00ff), rp)
diff --git a/include/asm-m32r/flat.h b/include/asm-m32r/flat.h
index 1b285f6..d851cf0 100644
--- a/include/asm-m32r/flat.h
+++ b/include/asm-m32r/flat.h
@@ -15,9 +15,10 @@
#defineflat_stack_align(sp)(*sp += (*sp 3 ? (4 - (*sp
3)): 0))
#defineflat_argvp_envp_on_stack() 0
#defineflat_old_ram_flag(flags)(flags)
+#defineflat_set_persistent(relval, p) 0
#defineflat_reloc_valid(reloc, size) \
(((reloc) - textlen_for_m32r_lo16_data) = (size))
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
m32r_flat_get_addr_from_rp(rp, relval, (text_len) )
#define flat_put_addr_at_rp(rp, addr, relval) \
diff --git a/include/asm-m68knommu/flat.h b/include/asm-m68knommu/flat.h
index 2d836ed..814b517 100644
--- a/include/asm-m68knommu/flat.h
+++ b/include/asm-m68knommu/flat.h
@@ -9,8 +9,9 @@
#defineflat_argvp_envp_on_stack() 1
#defineflat_old_ram_flag(flags)(flags)
#defineflat_reloc_valid(reloc, size) ((reloc) = (size))
-#defineflat_get_addr_from_rp(rp, relval, flags) get_unaligned(rp)
+#defineflat_get_addr_from_rp(rp, relval, flags, p) get_unaligned(rp)
#defineflat_put_addr_at_rp(rp, val, relval) put_unaligned(val,rp)
#defineflat_get_relocate_addr(rel) (rel)
+#defineflat_set_persistent(relval, p) 0
#endif /* __M68KNOMMU_FLAT_H__ */
diff --git
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Wed 19 Sep 2007 21:55, David McCullough pondered: Jivin Robin Getz lays it down ... On Tue 18 Sep 2007 04:09, Bryan Wu pondered: From: Bernd Schmidt [EMAIL PROTECTED] This just adds minimum support for the Blackfin relocations, since we don't have enough space in each reloc. The idea is to store a value with one relocation so that subsequent ones can access it. Signed-off-by: Bernd Schmidt [EMAIL PROTECTED] Signed-off-by: Bryan Wu [EMAIL PROTECTED] Cc: David McCullough [EMAIL PROTECTED] Cc: Greg Ungerer [EMAIL PROTECTED] Adding the other appropriate maintainers. for h8, m32r, sh and v850. Looks fine to me, obviously impacts the existing arches very little. Can't see why it shouldn't get included, Is that Australian for Acked-by:? :) -Robin - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote:
Jivin Robin Getz lays it down ...
On Tue 18 Sep 2007 04:09, Bryan Wu pondered:
This just adds minimum support for the Blackfin relocations,
since we don't have enough space in each reloc. The idea
is to store a value with one relocation so that subsequent ones can
access it.
Adding the other appropriate maintainers. for h8, m32r, sh and v850.
Looks fine to me, obviously impacts the existing arches very little.
Can't see why it shouldn't get included,
I find it a bit disconcerting that blackfin already depends on this
in-tree without there being any earlier discussion on making these
changes.
*/
if (rev OLD_FLAT_VERSION) {
+ unsigned long persistent = 0;
for (i=0; i relocs; i++) {
unsigned long addr, relval;
@@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
relocated (of course, the address has to be
relocated first). */
relval = ntohl(reloc[i]);
+ if (flat_set_persistent (relval, persistent))
+ continue;
addr = flat_get_relocate_addr(relval);
rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
if (rp == (unsigned long *)RELOC_FAILED) {
I don't much care for this API. It's shuffling around a temporary
variable for the architecture code that's set for certain relocations
that are otherwise unhandled.
Since all the architecture is interested in is the relval that has
associated persistent data encoded in it, why don't we just have a stub
to give the architecture a chance to validate the relval before the
flat_get_relocate_addr() and move this stuff there instead? ie, blackfin
takes this out-of-line and manages its persistent value there.
@@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
}
/* Get the pointer's value. */
- addr = flat_get_addr_from_rp(rp, relval, flags);
+ addr = flat_get_addr_from_rp(rp, relval, flags,
persistent);
There's no reason for this to be a pointer. The current in-tree blackfin
code doesn't change this value, and if you have patches that are doing
that, this is even more reason to bury this kind of silliness in your
architecture code.
load_flat_file() is ugly enough without dumping more architecture
callback abuses in it.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On 9/19/07, Paul Mundt [EMAIL PROTECTED] wrote: On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: Jivin Robin Getz lays it down ... On Tue 18 Sep 2007 04:09, Bryan Wu pondered: This just adds minimum support for the Blackfin relocations, since we don't have enough space in each reloc. The idea is to store a value with one relocation so that subsequent ones can access it. Adding the other appropriate maintainers. for h8, m32r, sh and v850. Looks fine to me, obviously impacts the existing arches very little. Can't see why it shouldn't get included, I find it a bit disconcerting that blackfin already depends on this in-tree without there being any earlier discussion on making these changes. not really ... this patch was posted before but was lost in the shuffle ... and i'm not quite sure what you mean by depends on ... if you want to use the FLAT file format on a Blackfin processor, then this patch is needed, but that isnt the only file format that works on the Blackfin port as we also have FDPIC ELF i havent used FLAT files on a Blackfin board in quite a long time actually ... -mike - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_flat: minimum support for theBlackfin relocations
On Wed, Sep 19, 2007 at 11:42:53PM -0400, Mike Frysinger wrote: On 9/19/07, Paul Mundt [EMAIL PROTECTED] wrote: On Thu, Sep 20, 2007 at 11:55:25AM +1000, David McCullough wrote: Jivin Robin Getz lays it down ... On Tue 18 Sep 2007 04:09, Bryan Wu pondered: This just adds minimum support for the Blackfin relocations, since we don't have enough space in each reloc. The idea is to store a value with one relocation so that subsequent ones can access it. Adding the other appropriate maintainers. for h8, m32r, sh and v850. Looks fine to me, obviously impacts the existing arches very little. Can't see why it shouldn't get included, I find it a bit disconcerting that blackfin already depends on this in-tree without there being any earlier discussion on making these changes. not really ... this patch was posted before but was lost in the shuffle ... and i'm not quite sure what you mean by depends on ... if you want to use the FLAT file format on a Blackfin processor, then this patch is needed, but that isnt the only file format that works on the Blackfin port as we also have FDPIC ELF What I mean by depends on is that for what you are attempting to patch, your architecture has an in-tree dependency on something that hasn't been discussed. It's more than a bit backwards to push the follow-up bits before even getting an Ack on the changes you want to make in the common code. The fact you have other options is great, so at least you haven't shafted all of your git kernel users, but that's not the point. You should not be pushing things in to the kernel that have dependencies on API changes that may or may not be merged, especially when you're breaking the entire kernel for your platform (and the ability to bisect for those users) in the process. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
