| Message ID | 1445566385-15129-1-git-send-email-Peng.Fan@freescale.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Albert, Are you fine with this patch? I'd like the patch go through i.MX tree, since the other two patches needs this patch to work. Thanks, Peng. On Fri, Oct 23, 2015 at 10:13:03AM +0800, Peng Fan wrote: >The code such as PSCI in section named secure is bundled with >u-boot image, and when bootm, the code will be copied to their >runtime address same to compliation/linking address - >CONFIG_ARMV7_SECURE_BASE. > >When compile the PSCI code and link it into the u-boot image, >there will be relocation entries in .rel.dyn section for PSCI. >Actually, we do not needs these relocation entries. > >If still keep the relocation entries in .rel.dyn section, >r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid >address which may not support read/write for one SoC. >102 /* relative fix: increase location by offset */ >103 add r0, r0, r4 >104 ldr r1, [r0] >105 add r1, r1, r4 >106 str r1, [r0] > >So discard them to avoid touching the relocation entry in >arch/arm/lib/relocate.S. > >Signed-off-by: Peng Fan <Peng.Fan@freescale.com> >Cc: Tom Warren <twarren@nvidia.com> >Cc: York Sun <yorksun@freescale.com> >Cc: Hans De Goede <hdegoede@redhat.com> >Cc: Ian Campbell <ijc@hellion.org.uk> >Cc: Albert Aribaud <albert.u.boot@aribaud.net> >Cc: Tom Rini <trini@konsulko.com> >Cc: Jan Kiszka <jan.kiszka@siemens.com> >Cc: Stefano Babic <sbabic@denx.de> >--- > >Changes v4: > none > >V2 thread: http://lists.denx.de/pipermail/u-boot/2015-October/230755.html >Changes v3: > Move the relocation entries to the very begining and discard them. > >V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html >Changes V2: > Refine commit msg. > Discard the relocation entry section for secure text. > > arch/arm/cpu/u-boot.lds | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > >diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >index 03cd9f6..d48a905 100644 >--- a/arch/arm/cpu/u-boot.lds >+++ b/arch/arm/cpu/u-boot.lds >@@ -14,6 +14,23 @@ OUTPUT_ARCH(arm) > ENTRY(_start) > SECTIONS > { >+ /* >+ * Discard the relocation entries for secure text. >+ * The secure code is bundled with u-boot image, so there will >+ * be relocations entries for the secure code, since we use >+ * "-mword-relocations" to compile and "-pie" to link into the >+ * final image. We do not need the relocation entries for secure >+ * code, because secure code will not be relocated, it only needs >+ * to be copied from loading address to CONFIG_ARMV7_SECURE_BASE, >+ * which is the linking and running address for secure code. >+ * If keep the relocation entries in .rel.dyn section, >+ * "relocation offset + linking address" may locates into an >+ * address that is reserved by SoC, then will trigger data abort. >+ * >+ * The reason that move .rel._secure at the beginning, is to >+ * avoid hole in the final image. >+ */ >+ /DISCARD/ : { *(.rel._secure*) } > . = 0x00000000; > > . = ALIGN(4); >-- >1.8.4 > >
On Fri, Oct 30, 2015 at 04:23:49PM +0800, Peng Fan wrote: >Hi Albert, > >Are you fine with this patch? > >I'd like the patch go through i.MX tree, since the other two patches >needs this patch to work. Ping.. Thanks, Peng. > >Thanks, >Peng. > >On Fri, Oct 23, 2015 at 10:13:03AM +0800, Peng Fan wrote: >>The code such as PSCI in section named secure is bundled with >>u-boot image, and when bootm, the code will be copied to their >>runtime address same to compliation/linking address - >>CONFIG_ARMV7_SECURE_BASE. >> >>When compile the PSCI code and link it into the u-boot image, >>there will be relocation entries in .rel.dyn section for PSCI. >>Actually, we do not needs these relocation entries. >> >>If still keep the relocation entries in .rel.dyn section, >>r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid >>address which may not support read/write for one SoC. >>102 /* relative fix: increase location by offset */ >>103 add r0, r0, r4 >>104 ldr r1, [r0] >>105 add r1, r1, r4 >>106 str r1, [r0] >> >>So discard them to avoid touching the relocation entry in >>arch/arm/lib/relocate.S. >> >>Signed-off-by: Peng Fan <Peng.Fan@freescale.com> >>Cc: Tom Warren <twarren@nvidia.com> >>Cc: York Sun <yorksun@freescale.com> >>Cc: Hans De Goede <hdegoede@redhat.com> >>Cc: Ian Campbell <ijc@hellion.org.uk> >>Cc: Albert Aribaud <albert.u.boot@aribaud.net> >>Cc: Tom Rini <trini@konsulko.com> >>Cc: Jan Kiszka <jan.kiszka@siemens.com> >>Cc: Stefano Babic <sbabic@denx.de> >>--- >> >>Changes v4: >> none >> >>V2 thread: http://lists.denx.de/pipermail/u-boot/2015-October/230755.html >>Changes v3: >> Move the relocation entries to the very begining and discard them. >> >>V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html >>Changes V2: >> Refine commit msg. >> Discard the relocation entry section for secure text. >> >> arch/arm/cpu/u-boot.lds | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >>diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >>index 03cd9f6..d48a905 100644 >>--- a/arch/arm/cpu/u-boot.lds >>+++ b/arch/arm/cpu/u-boot.lds >>@@ -14,6 +14,23 @@ OUTPUT_ARCH(arm) >> ENTRY(_start) >> SECTIONS >> { >>+ /* >>+ * Discard the relocation entries for secure text. >>+ * The secure code is bundled with u-boot image, so there will >>+ * be relocations entries for the secure code, since we use >>+ * "-mword-relocations" to compile and "-pie" to link into the >>+ * final image. We do not need the relocation entries for secure >>+ * code, because secure code will not be relocated, it only needs >>+ * to be copied from loading address to CONFIG_ARMV7_SECURE_BASE, >>+ * which is the linking and running address for secure code. >>+ * If keep the relocation entries in .rel.dyn section, >>+ * "relocation offset + linking address" may locates into an >>+ * address that is reserved by SoC, then will trigger data abort. >>+ * >>+ * The reason that move .rel._secure at the beginning, is to >>+ * avoid hole in the final image. >>+ */ >>+ /DISCARD/ : { *(.rel._secure*) } >> . = 0x00000000; >> >> . = ALIGN(4); >>-- >>1.8.4 >> >> > >--
Hello Peng, On Fri, 23 Oct 2015 10:13:03 +0800, Peng Fan <Peng.Fan@freescale.com> wrote: > The code such as PSCI in section named secure is bundled with > u-boot image, and when bootm, the code will be copied to their > runtime address same to compliation/linking address - > CONFIG_ARMV7_SECURE_BASE. > > When compile the PSCI code and link it into the u-boot image, > there will be relocation entries in .rel.dyn section for PSCI. > Actually, we do not needs these relocation entries. > > If still keep the relocation entries in .rel.dyn section, > r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid > address which may not support read/write for one SoC. > 102 /* relative fix: increase location by offset */ > 103 add r0, r0, r4 > 104 ldr r1, [r0] > 105 add r1, r1, r4 > 106 str r1, [r0] > > So discard them to avoid touching the relocation entry in > arch/arm/lib/relocate.S. > > Signed-off-by: Peng Fan <Peng.Fan@freescale.com> > Cc: Tom Warren <twarren@nvidia.com> > Cc: York Sun <yorksun@freescale.com> > Cc: Hans De Goede <hdegoede@redhat.com> > Cc: Ian Campbell <ijc@hellion.org.uk> > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > Cc: Tom Rini <trini@konsulko.com> > Cc: Jan Kiszka <jan.kiszka@siemens.com> > Cc: Stefano Babic <sbabic@denx.de> > --- > > Changes v4: > none > > V2 thread: http://lists.denx.de/pipermail/u-boot/2015-October/230755.html > Changes v3: > Move the relocation entries to the very begining and discard them. > > V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html > Changes V2: > Refine commit msg. > Discard the relocation entry section for secure text. > > arch/arm/cpu/u-boot.lds | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > index 03cd9f6..d48a905 100644 > --- a/arch/arm/cpu/u-boot.lds > +++ b/arch/arm/cpu/u-boot.lds > @@ -14,6 +14,23 @@ OUTPUT_ARCH(arm) > ENTRY(_start) > SECTIONS > { > + /* > + * Discard the relocation entries for secure text. > + * The secure code is bundled with u-boot image, so there will > + * be relocations entries for the secure code, since we use > + * "-mword-relocations" to compile and "-pie" to link into the > + * final image. We do not need the relocation entries for secure > + * code, because secure code will not be relocated, it only needs > + * to be copied from loading address to CONFIG_ARMV7_SECURE_BASE, > + * which is the linking and running address for secure code. > + * If keep the relocation entries in .rel.dyn section, > + * "relocation offset + linking address" may locates into an > + * address that is reserved by SoC, then will trigger data abort. > + * > + * The reason that move .rel._secure at the beginning, is to > + * avoid hole in the final image. > + */ > + /DISCARD/ : { *(.rel._secure*) } > . = 0x00000000; > > . = ALIGN(4); > -- > 1.8.4 > > Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net> Amicalement,
Hi Stefano, On Tue, Nov 10, 2015 at 02:14:10PM +0100, Albert ARIBAUD wrote: >Hello Peng, > >On Fri, 23 Oct 2015 10:13:03 +0800, Peng Fan <Peng.Fan@freescale.com> >wrote: >> The code such as PSCI in section named secure is bundled with >> u-boot image, and when bootm, the code will be copied to their >> runtime address same to compliation/linking address - >> CONFIG_ARMV7_SECURE_BASE. >> >> When compile the PSCI code and link it into the u-boot image, >> there will be relocation entries in .rel.dyn section for PSCI. >> Actually, we do not needs these relocation entries. >> >> If still keep the relocation entries in .rel.dyn section, >> r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid >> address which may not support read/write for one SoC. >> 102 /* relative fix: increase location by offset */ >> 103 add r0, r0, r4 >> 104 ldr r1, [r0] >> 105 add r1, r1, r4 >> 106 str r1, [r0] >> >> So discard them to avoid touching the relocation entry in >> arch/arm/lib/relocate.S. >> >> Signed-off-by: Peng Fan <Peng.Fan@freescale.com> >> Cc: Tom Warren <twarren@nvidia.com> >> Cc: York Sun <yorksun@freescale.com> >> Cc: Hans De Goede <hdegoede@redhat.com> >> Cc: Ian Campbell <ijc@hellion.org.uk> >> Cc: Albert Aribaud <albert.u.boot@aribaud.net> >> Cc: Tom Rini <trini@konsulko.com> >> Cc: Jan Kiszka <jan.kiszka@siemens.com> >> Cc: Stefano Babic <sbabic@denx.de> >> --- >> >> Changes v4: >> none >> >> V2 thread: http://lists.denx.de/pipermail/u-boot/2015-October/230755.html >> Changes v3: >> Move the relocation entries to the very begining and discard them. >> >> V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html >> Changes V2: >> Refine commit msg. >> Discard the relocation entry section for secure text. >> >> arch/arm/cpu/u-boot.lds | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >> index 03cd9f6..d48a905 100644 >> --- a/arch/arm/cpu/u-boot.lds >> +++ b/arch/arm/cpu/u-boot.lds >> @@ -14,6 +14,23 @@ OUTPUT_ARCH(arm) >> ENTRY(_start) >> SECTIONS >> { >> + /* >> + * Discard the relocation entries for secure text. >> + * The secure code is bundled with u-boot image, so there will >> + * be relocations entries for the secure code, since we use >> + * "-mword-relocations" to compile and "-pie" to link into the >> + * final image. We do not need the relocation entries for secure >> + * code, because secure code will not be relocated, it only needs >> + * to be copied from loading address to CONFIG_ARMV7_SECURE_BASE, >> + * which is the linking and running address for secure code. >> + * If keep the relocation entries in .rel.dyn section, >> + * "relocation offset + linking address" may locates into an >> + * address that is reserved by SoC, then will trigger data abort. >> + * >> + * The reason that move .rel._secure at the beginning, is to >> + * avoid hole in the final image. >> + */ >> + /DISCARD/ : { *(.rel._secure*) } >> . = 0x00000000; >> >> . = ALIGN(4); >> -- >> 1.8.4 >> >> > >Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net> > Can you please take a look at the patch set and merge? Thanks, Peng. >Amicalement, >-- >Albert.
On 11/11/2015 02:14, Peng Fan wrote: > Hi Stefano, > > On Tue, Nov 10, 2015 at 02:14:10PM +0100, Albert ARIBAUD wrote: >> Hello Peng, >> >> On Fri, 23 Oct 2015 10:13:03 +0800, Peng Fan <Peng.Fan@freescale.com> >> wrote: >>> The code such as PSCI in section named secure is bundled with >>> u-boot image, and when bootm, the code will be copied to their >>> runtime address same to compliation/linking address - >>> CONFIG_ARMV7_SECURE_BASE. >>> >>> When compile the PSCI code and link it into the u-boot image, >>> there will be relocation entries in .rel.dyn section for PSCI. >>> Actually, we do not needs these relocation entries. >>> >>> If still keep the relocation entries in .rel.dyn section, >>> r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid >>> address which may not support read/write for one SoC. >>> 102 /* relative fix: increase location by offset */ >>> 103 add r0, r0, r4 >>> 104 ldr r1, [r0] >>> 105 add r1, r1, r4 >>> 106 str r1, [r0] >>> >>> So discard them to avoid touching the relocation entry in >>> arch/arm/lib/relocate.S. >>> >>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com> >>> Cc: Tom Warren <twarren@nvidia.com> >>> Cc: York Sun <yorksun@freescale.com> >>> Cc: Hans De Goede <hdegoede@redhat.com> >>> Cc: Ian Campbell <ijc@hellion.org.uk> >>> Cc: Albert Aribaud <albert.u.boot@aribaud.net> >>> Cc: Tom Rini <trini@konsulko.com> >>> Cc: Jan Kiszka <jan.kiszka@siemens.com> >>> Cc: Stefano Babic <sbabic@denx.de> >>> --- >>> >>> Changes v4: >>> none >>> >>> V2 thread: http://lists.denx.de/pipermail/u-boot/2015-October/230755.html >>> Changes v3: >>> Move the relocation entries to the very begining and discard them. >>> >>> V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html >>> Changes V2: >>> Refine commit msg. >>> Discard the relocation entry section for secure text. >>> >>> arch/arm/cpu/u-boot.lds | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >>> index 03cd9f6..d48a905 100644 >>> --- a/arch/arm/cpu/u-boot.lds >>> +++ b/arch/arm/cpu/u-boot.lds >>> @@ -14,6 +14,23 @@ OUTPUT_ARCH(arm) >>> ENTRY(_start) >>> SECTIONS >>> { >>> + /* >>> + * Discard the relocation entries for secure text. >>> + * The secure code is bundled with u-boot image, so there will >>> + * be relocations entries for the secure code, since we use >>> + * "-mword-relocations" to compile and "-pie" to link into the >>> + * final image. We do not need the relocation entries for secure >>> + * code, because secure code will not be relocated, it only needs >>> + * to be copied from loading address to CONFIG_ARMV7_SECURE_BASE, >>> + * which is the linking and running address for secure code. >>> + * If keep the relocation entries in .rel.dyn section, >>> + * "relocation offset + linking address" may locates into an >>> + * address that is reserved by SoC, then will trigger data abort. >>> + * >>> + * The reason that move .rel._secure at the beginning, is to >>> + * avoid hole in the final image. >>> + */ >>> + /DISCARD/ : { *(.rel._secure*) } >>> . = 0x00000000; >>> >>> . = ALIGN(4); >>> -- >>> 1.8.4 >>> >>> >> >> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net> >> > > Can you please take a look at the patch set and merge? I'll do Best regards, Stefano Babic
On 11/11/2015 18:05, Stefano Babic wrote: > On 11/11/2015 02:14, Peng Fan wrote: >> Hi Stefano, >> >> On Tue, Nov 10, 2015 at 02:14:10PM +0100, Albert ARIBAUD wrote: >>> Hello Peng, >>> >>> On Fri, 23 Oct 2015 10:13:03 +0800, Peng Fan <Peng.Fan@freescale.com> >>> wrote: >>>> The code such as PSCI in section named secure is bundled with >>>> u-boot image, and when bootm, the code will be copied to their >>>> runtime address same to compliation/linking address - >>>> CONFIG_ARMV7_SECURE_BASE. >>>> >>>> When compile the PSCI code and link it into the u-boot image, >>>> there will be relocation entries in .rel.dyn section for PSCI. >>>> Actually, we do not needs these relocation entries. >>>> >>>> If still keep the relocation entries in .rel.dyn section, >>>> r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid >>>> address which may not support read/write for one SoC. >>>> 102 /* relative fix: increase location by offset */ >>>> 103 add r0, r0, r4 >>>> 104 ldr r1, [r0] >>>> 105 add r1, r1, r4 >>>> 106 str r1, [r0] >>>> >>>> So discard them to avoid touching the relocation entry in >>>> arch/arm/lib/relocate.S. >>>> >>>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com> >>>> Cc: Tom Warren <twarren@nvidia.com> >>>> Cc: York Sun <yorksun@freescale.com> >>>> Cc: Hans De Goede <hdegoede@redhat.com> >>>> Cc: Ian Campbell <ijc@hellion.org.uk> >>>> Cc: Albert Aribaud <albert.u.boot@aribaud.net> >>>> Cc: Tom Rini <trini@konsulko.com> >>>> Cc: Jan Kiszka <jan.kiszka@siemens.com> >>>> Cc: Stefano Babic <sbabic@denx.de> >>>> --- >>>> >>>> Changes v4: >>>> none >>>> >>>> V2 thread: http://lists.denx.de/pipermail/u-boot/2015-October/230755.html >>>> Changes v3: >>>> Move the relocation entries to the very begining and discard them. >>>> >>>> V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html >>>> Changes V2: >>>> Refine commit msg. >>>> Discard the relocation entry section for secure text. >>>> >>>> arch/arm/cpu/u-boot.lds | 17 +++++++++++++++++ >>>> 1 file changed, 17 insertions(+) >>>> >>>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >>>> index 03cd9f6..d48a905 100644 >>>> --- a/arch/arm/cpu/u-boot.lds >>>> +++ b/arch/arm/cpu/u-boot.lds >>>> @@ -14,6 +14,23 @@ OUTPUT_ARCH(arm) >>>> ENTRY(_start) >>>> SECTIONS >>>> { >>>> + /* >>>> + * Discard the relocation entries for secure text. >>>> + * The secure code is bundled with u-boot image, so there will >>>> + * be relocations entries for the secure code, since we use >>>> + * "-mword-relocations" to compile and "-pie" to link into the >>>> + * final image. We do not need the relocation entries for secure >>>> + * code, because secure code will not be relocated, it only needs >>>> + * to be copied from loading address to CONFIG_ARMV7_SECURE_BASE, >>>> + * which is the linking and running address for secure code. >>>> + * If keep the relocation entries in .rel.dyn section, >>>> + * "relocation offset + linking address" may locates into an >>>> + * address that is reserved by SoC, then will trigger data abort. >>>> + * >>>> + * The reason that move .rel._secure at the beginning, is to >>>> + * avoid hole in the final image. >>>> + */ >>>> + /DISCARD/ : { *(.rel._secure*) } >>>> . = 0x00000000; >>>> >>>> . = ALIGN(4); >>>> -- >>>> 1.8.4 >>>> >>>> >>> >>> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net> >>> >> >> Can you please take a look at the patch set and merge? > Applied (whole series) to u-boot-imx, thanks ! Best regards, Stefano Babic
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 03cd9f6..d48a905 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -14,6 +14,23 @@ OUTPUT_ARCH(arm) ENTRY(_start) SECTIONS { + /* + * Discard the relocation entries for secure text. + * The secure code is bundled with u-boot image, so there will + * be relocations entries for the secure code, since we use + * "-mword-relocations" to compile and "-pie" to link into the + * final image. We do not need the relocation entries for secure + * code, because secure code will not be relocated, it only needs + * to be copied from loading address to CONFIG_ARMV7_SECURE_BASE, + * which is the linking and running address for secure code. + * If keep the relocation entries in .rel.dyn section, + * "relocation offset + linking address" may locates into an + * address that is reserved by SoC, then will trigger data abort. + * + * The reason that move .rel._secure at the beginning, is to + * avoid hole in the final image. + */ + /DISCARD/ : { *(.rel._secure*) } . = 0x00000000; . = ALIGN(4);
The code such as PSCI in section named secure is bundled with u-boot image, and when bootm, the code will be copied to their runtime address same to compliation/linking address - CONFIG_ARMV7_SECURE_BASE. When compile the PSCI code and link it into the u-boot image, there will be relocation entries in .rel.dyn section for PSCI. Actually, we do not needs these relocation entries. If still keep the relocation entries in .rel.dyn section, r0 at line 103 and 106 in arch/arm/lib/relocate.S may be an invalid address which may not support read/write for one SoC. 102 /* relative fix: increase location by offset */ 103 add r0, r0, r4 104 ldr r1, [r0] 105 add r1, r1, r4 106 str r1, [r0] So discard them to avoid touching the relocation entry in arch/arm/lib/relocate.S. Signed-off-by: Peng Fan <Peng.Fan@freescale.com> Cc: Tom Warren <twarren@nvidia.com> Cc: York Sun <yorksun@freescale.com> Cc: Hans De Goede <hdegoede@redhat.com> Cc: Ian Campbell <ijc@hellion.org.uk> Cc: Albert Aribaud <albert.u.boot@aribaud.net> Cc: Tom Rini <trini@konsulko.com> Cc: Jan Kiszka <jan.kiszka@siemens.com> Cc: Stefano Babic <sbabic@denx.de> --- Changes v4: none V2 thread: http://lists.denx.de/pipermail/u-boot/2015-October/230755.html Changes v3: Move the relocation entries to the very begining and discard them. V1 thread: http://lists.denx.de/pipermail/u-boot/2015-October/229426.html Changes V2: Refine commit msg. Discard the relocation entry section for secure text. arch/arm/cpu/u-boot.lds | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)