Re: CVS commit: src/sys/conf
On Nov 13, 10:28pm, Masao Uebayashi wrote: } On Thu, Nov 13, 2014 at 10:21 PM, Christos Zoulas wrote: } > On Nov 13, 10:15pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote: } > } > | I'm not sure about your goal, but anyway it should be proposed } > | and discussed proper lists before commit. } } I have almost no questions, nothing to discuess except details. It has nothing to do with whether or not you have questions. Others have questions. It is customary, and in many cases, required to discuss things before doing massive rototills that affect important parts of the system. Also, when multiple people object to something you don't get to just ignore the objections. They need to be addressed properly. Not to mention that if you were to explain what you are doing, what you're plans and goals are, some of the objections might go away. }-- End of excerpt from Masao Uebayashi
Re: CVS commit: src/sys/arch/atari/stand/installboot
On Nov 14, 3:00am, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
-- Subject: Re: CVS commit: src/sys/arch/atari/stand/installboot
| "Test it (or call for testers) before commit"
| because installboot could be fatal on install floppy and bootstrap.
|
| > but memcpy is the only way.
|
| - cast via (void *)
That does not work.
| - union {uint16_t w[256]; struct bootblock bbp;}
That works...
| - be16enc(9)
I don't see how that helps...
christos
Re: CVS commit: src/sys/arch/atari/stand/installboot
christos@ wrote:
> On Nov 14, 2:43am, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
> -- Subject: Re: CVS commit: src/sys/arch/atari/stand/installboot
>
> | christos@ wrote:
> |
> | > Module Name: src
> | > Committed By: christos
> | > Date: Thu Nov 13 17:19:29 UTC 2014
> | >
> | > Modified Files:
> | > src/sys/arch/atari/stand/installboot: installboot.c
> | >
> | > Log Message:
> | > fix strict aliasing violations
> |
> | > - *((u_int16_t *)bb->bb_xxboot + 255) = 0;
> | > - *((u_int16_t *)bb->bb_xxboot + 255) = 0x1234 - abcksum(bb->bb_xxboot);
> | > + sum = 0;
> | > + memcpy(bb->bb_xxboot + 255, &sum, sizeof(sum));
> | > + sum = 0x1234 - abcksum(bb->bb_xxboot);
> | > + memcpy(bb->bb_xxboot + 255, &sum, sizeof(sum));
> |
> | I doubt your "bb->bb_xxboot + 255" is the same place
> | as the original "(u_int16_t *)bb->bb_xxboot + 255"
> | (the cksum word looks at the end of 512 byte sector).
> |
> | memcpy(9) looks also awful for readers because it hides endianness..
>
> Let me fix it,
"Test it (or call for testers) before commit"
because installboot could be fatal on install floppy and bootstrap.
> but memcpy is the only way.
- cast via (void *)
- union {uint16_t w[256]; struct bootblock bbp;}
- be16enc(9)
etc?
---
Izumi Tsutsui
Re: CVS commit: src/sys/arch/atari/stand/installboot
On Nov 14, 2:43am, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote: -- Subject: Re: CVS commit: src/sys/arch/atari/stand/installboot | christos@ wrote: | | > Module Name:src | > Committed By: christos | > Date: Thu Nov 13 17:19:29 UTC 2014 | > | > Modified Files: | > src/sys/arch/atari/stand/installboot: installboot.c | > | > Log Message: | > fix strict aliasing violations | | > - *((u_int16_t *)bb->bb_xxboot + 255) = 0; | > - *((u_int16_t *)bb->bb_xxboot + 255) = 0x1234 - abcksum(bb->bb_xxboot); | > + sum = 0; | > + memcpy(bb->bb_xxboot + 255, &sum, sizeof(sum)); | > + sum = 0x1234 - abcksum(bb->bb_xxboot); | > + memcpy(bb->bb_xxboot + 255, &sum, sizeof(sum)); | | I doubt your "bb->bb_xxboot + 255" is the same place | as the original "(u_int16_t *)bb->bb_xxboot + 255" | (the cksum word looks at the end of 512 byte sector). | | memcpy(9) looks also awful for readers because it hides endianness.. Let me fix it, but memcpy is the only way. christos
Re: CVS commit: src/sys/arch/atari/stand/installboot
christos@ wrote: > Module Name: src > Committed By: christos > Date: Thu Nov 13 17:19:29 UTC 2014 > > Modified Files: > src/sys/arch/atari/stand/installboot: installboot.c > > Log Message: > fix strict aliasing violations > - *((u_int16_t *)bb->bb_xxboot + 255) = 0; > - *((u_int16_t *)bb->bb_xxboot + 255) = 0x1234 - abcksum(bb->bb_xxboot); > + sum = 0; > + memcpy(bb->bb_xxboot + 255, &sum, sizeof(sum)); > + sum = 0x1234 - abcksum(bb->bb_xxboot); > + memcpy(bb->bb_xxboot + 255, &sum, sizeof(sum)); I doubt your "bb->bb_xxboot + 255" is the same place as the original "(u_int16_t *)bb->bb_xxboot + 255" (the cksum word looks at the end of 512 byte sector). memcpy(9) looks also awful for readers because it hides endianness.. --- Izumi Tsutsui
Re: CVS commit: src/sys/conf
On Fri, Nov 14, 2014 at 1:14 AM, Christos Zoulas wrote: > In article > , > Masao Uebayashi wrote: >> >>I can't answer everything soon but one reason coming to mind: >> >>I want to hide all "link_set_*" sections except "link_set_modules" in >>*.kmod. AFAIK linker script doesn't have a syntax to do ``merge >>everything except this pattern''. Thus I need to know all link-set >>sections explicitly. > > Ok, but can't you generate a script dynamically that handles that? I think it is possible.
Re: CVS commit: src/sys/conf
In article , Masao Uebayashi wrote: > >I can't answer everything soon but one reason coming to mind: > >I want to hide all "link_set_*" sections except "link_set_modules" in >*.kmod. AFAIK linker script doesn't have a syntax to do ``merge >everything except this pattern''. Thus I need to know all link-set >sections explicitly. Ok, but can't you generate a script dynamically that handles that? christos
Re: CVS commit: src/sys/conf
On Fri, Nov 14, 2014 at 12:43 AM, Christos Zoulas wrote: > In article , > Christos Zoulas wrote: >>In article >>, >> >>Can you please explain what you are trying to do and what are your goals? >>I am not sure if I agree that the list of link_sets is "well known". If >>you ask anyone to enumerate them they will probably resort to nm the >>kernel... > > Ok, I see the plan in config now, but I'd still prefer that the link > sets were discovered rather than hard-coded like it has been so far. > What do others think? I can't answer everything soon but one reason coming to mind: I want to hide all "link_set_*" sections except "link_set_modules" in *.kmod. AFAIK linker script doesn't have a syntax to do ``merge everything except this pattern''. Thus I need to know all link-set sections explicitly.
Re: CVS commit: src/sys/conf
In article , Christos Zoulas wrote: >In article >, > >Can you please explain what you are trying to do and what are your goals? >I am not sure if I agree that the list of link_sets is "well known". If >you ask anyone to enumerate them they will probably resort to nm the >kernel... Ok, I see the plan in config now, but I'd still prefer that the link sets were discovered rather than hard-coded like it has been so far. What do others think? christos
Re: CVS commit: src/sys/conf
In article , Masao Uebayashi wrote: >On Thu, Nov 13, 2014 at 11:54 PM, Christos Zoulas wrote: >> Any solution that involves hard-coding the currently used linksets by >> name is no solution for me. Please come up with a complete solution, >> and present it for discussion. Yes, the current script is a hack, but >> it is a hack we've been using for a while successfully. > >I would not call it "hard-coding"; link-sets are well-known. All the >used link-sets are known before building. > >I'd detect unknown link-set sections and make that build fail. This >is more strict. > >I'd also provide a way to declare extra link-set for 3rd party modules. Can you please explain what you are trying to do and what are your goals? I am not sure if I agree that the list of link_sets is "well known". If you ask anyone to enumerate them they will probably resort to nm the kernel... christos
Re: CVS commit: src/sys/conf
On Thu, Nov 13, 2014 at 11:54 PM, Christos Zoulas wrote: > Any solution that involves hard-coding the currently used linksets by > name is no solution for me. Please come up with a complete solution, > and present it for discussion. Yes, the current script is a hack, but > it is a hack we've been using for a while successfully. I would not call it "hard-coding"; link-sets are well-known. All the used link-sets are known before building. I'd detect unknown link-set sections and make that build fail. This is more strict. I'd also provide a way to declare extra link-set for 3rd party modules.
Re: CVS commit: src/sys/conf
On Nov 13, 10:49pm, uebay...@gmail.com (Masao Uebayashi) wrote:
-- Subject: Re: CVS commit: src/sys/conf
| On Thu, Nov 13, 2014 at 10:43 PM, Christos Zoulas wrote:
| > On Nov 13, 10:28pm, uebay...@gmail.com (Masao Uebayashi) wrote:
| > -- Subject: Re: CVS commit: src/sys/conf
| >
| > | On Thu, Nov 13, 2014 at 10:21 PM, Christos Zoulas
wrote:
| > | > On Nov 13, 10:15pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
| > | > -- Subject: Re: CVS commit: src/sys/conf
| > | >
| > | > | I'm not sure about your goal, but anyway it should be proposed
| > | > | and discussed proper lists before commit.
| > |
| > | I have almost no questions, nothing to discuess except details.
| > |
| > | > | In the perfect world, both cats and shark should have native
bootloaders
| > | > | that can load native ELF directly and the a.out hack seems a special
case.
| > | >
| > | > The script is also used to produce the __{start,stop}_link_set.* symbols
| > | > for modules now...
| > |
| > | I know.
| > |
| > | And as I said repeatedly, it moves link_set_* sections at wrong places...
| >
| > It does not matter currently. It is really simple to move them anywhere
| > when it makes a difference.
|
| It does not matter for you, it matters for me.
Any solution that involves hard-coding the currently used linksets by
name is no solution for me. Please come up with a complete solution,
and present it for discussion. Yes, the current script is a hack, but
it is a hack we've been using for a while successfully.
christos
Re: CVS commit: src/sys/conf
uebayasi@ wrote: > I have almost no questions, nothing to discuess except details. The review is held not for you but other developers and users. http://www.netbsd.org/developers/commit-guidelines.html >> "Obvious" fixes can be committed without any prior discussion or review. >> (The definition of "obvious" in the GCC Project is: "could not possibly >> cause anyone to object." We adopt this definition here) >> >> All other (i. e. "non-obvious") fixes should have a review. It looks there are so many blames for your recent changes. --- Izumi Tsutsui
Re: CVS commit: src/sys/conf
On Thu, Nov 13, 2014 at 10:43 PM, Christos Zoulas wrote:
> On Nov 13, 10:28pm, uebay...@gmail.com (Masao Uebayashi) wrote:
> -- Subject: Re: CVS commit: src/sys/conf
>
> | On Thu, Nov 13, 2014 at 10:21 PM, Christos Zoulas
> wrote:
> | > On Nov 13, 10:15pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
> | > -- Subject: Re: CVS commit: src/sys/conf
> | >
> | > | I'm not sure about your goal, but anyway it should be proposed
> | > | and discussed proper lists before commit.
> |
> | I have almost no questions, nothing to discuess except details.
> |
> | > | In the perfect world, both cats and shark should have native bootloaders
> | > | that can load native ELF directly and the a.out hack seems a special
> case.
> | >
> | > The script is also used to produce the __{start,stop}_link_set.* symbols
> | > for modules now...
> |
> | I know.
> |
> | And as I said repeatedly, it moves link_set_* sections at wrong places...
>
> It does not matter currently. It is really simple to move them anywhere
> when it makes a difference.
It does not matter for you, it matters for me.
Re: CVS commit: src/sys/conf
On Nov 13, 10:28pm, uebay...@gmail.com (Masao Uebayashi) wrote:
-- Subject: Re: CVS commit: src/sys/conf
| On Thu, Nov 13, 2014 at 10:21 PM, Christos Zoulas wrote:
| > On Nov 13, 10:15pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
| > -- Subject: Re: CVS commit: src/sys/conf
| >
| > | I'm not sure about your goal, but anyway it should be proposed
| > | and discussed proper lists before commit.
|
| I have almost no questions, nothing to discuess except details.
|
| > | In the perfect world, both cats and shark should have native bootloaders
| > | that can load native ELF directly and the a.out hack seems a special case.
| >
| > The script is also used to produce the __{start,stop}_link_set.* symbols
| > for modules now...
|
| I know.
|
| And as I said repeatedly, it moves link_set_* sections at wrong places...
It does not matter currently. It is really simple to move them anywhere
when it makes a difference.
christos
Re: CVS commit: src/sys/conf
On Thu, Nov 13, 2014 at 10:21 PM, Christos Zoulas wrote:
> On Nov 13, 10:15pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
> -- Subject: Re: CVS commit: src/sys/conf
>
> | I'm not sure about your goal, but anyway it should be proposed
> | and discussed proper lists before commit.
I have almost no questions, nothing to discuess except details.
> | In the perfect world, both cats and shark should have native bootloaders
> | that can load native ELF directly and the a.out hack seems a special case.
>
> The script is also used to produce the __{start,stop}_link_set.* symbols
> for modules now...
I know.
And as I said repeatedly, it moves link_set_* sections at wrong places...
Re: CVS commit: src/sys/conf
On Nov 13, 10:15pm, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote:
-- Subject: Re: CVS commit: src/sys/conf
| I'm not sure about your goal, but anyway it should be proposed
| and discussed proper lists before commit.
|
| In the perfect world, both cats and shark should have native bootloaders
| that can load native ELF directly and the a.out hack seems a special case.
The script is also used to produce the __{start,stop}_link_set.* symbols
for modules now...
christos
Re: CVS commit: src/sys/conf
uebayasi@ wrote:
> > Modified Files:
> > src/sys/arch/cats/conf: Makefile.cats.inc
> > src/sys/arch/shark/conf: Makefile.shark.inc
> > src/sys/conf: Makefile.kern.inc
> > Added Files:
> > src/sys/arch/arm/conf: kern.ldscript.head kern.ldscript.tail
> > mkldscript.sh
> >
> > Log Message:
> > work around a binutils bug where converting ELF kernels to a.out with
> > objcopy
> > produces corrupted binaries when the link_set_* sections extend into another
> > page after the end of the .text section by using a generated an ldscript
> > that
> > puts all the link_set_* data into the .text section in the first place.
> > ---
> >
> > You can see how they can be valid in the Makefiles and
> > kern.ldscript.{head,tail} files.
> >
> > Anyway, it looks required for a.out only but no one will fix
> > a.out features..
>
> IIUC the requirement for mkldscript.sh users is to merge link_set_*
> sections. I'll merge link_set_* into .rodata on all ports. Then this
> hack can go away.
I'm not sure about your goal, but anyway it should be proposed
and discussed proper lists before commit.
In the perfect world, both cats and shark should have native bootloaders
that can load native ELF directly and the a.out hack seems a special case.
---
Izumi Tsutsui
Re: CVS commit: src/sys/conf
On Wed, Nov 12, 2014 at 11:04 PM, Izumi Tsutsui wrote:
> christos@ wrote:
>
>> I grepped -R and used nxr but found nothing.
>
> Hmm.
> http://nxr.netbsd.org/search?q=mkldscript.sh&project=src
> shows all files..
>
>> >I wonder if you need a different script (or proper wrapper)
>> >for modules..
>>
>> Well, it did not have a valid syntax also when I tried it for some
>> reason (ld complained). It needed:
>>
>> link_set_foo : { *(link_set_foo) }
>>
>> instead of:
>>
>> *(link_set_foo)
>
> The initial commit message says:
> http://mail-index.netbsd.org/source-changes/2004/09/13/msg152610.html
> ---
> Modified Files:
> src/sys/arch/cats/conf: Makefile.cats.inc
> src/sys/arch/shark/conf: Makefile.shark.inc
> src/sys/conf: Makefile.kern.inc
> Added Files:
> src/sys/arch/arm/conf: kern.ldscript.head kern.ldscript.tail
> mkldscript.sh
>
> Log Message:
> work around a binutils bug where converting ELF kernels to a.out with objcopy
> produces corrupted binaries when the link_set_* sections extend into another
> page after the end of the .text section by using a generated an ldscript that
> puts all the link_set_* data into the .text section in the first place.
> ---
>
> You can see how they can be valid in the Makefiles and
> kern.ldscript.{head,tail} files.
>
> Anyway, it looks required for a.out only but no one will fix
> a.out features..
IIUC the requirement for mkldscript.sh users is to merge link_set_*
sections. I'll merge link_set_* into .rodata on all ports. Then this
hack can go away.
(I also found that arm constructor section (.init_array) can't be
converted to a.out too.)
