diff mbox series

[v2,7/9] powerpc/ps3: Add check for otheros image size

Message ID 4e8defeb49d62dd9d435e5ea3ddc5668e56fa496.1589049250.git.geoff@infradead.org (mailing list archive)
State Rejected, archived
Headers show
Series [v2,1/9] powerpc/head_check: Automatic verbosity | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (1bc92fe3175eb26ff37e580c0383d7a9abe06835)
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 22 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Geoff Levand May 9, 2020, 6:58 p.m. UTC
The ps3's otheros flash loader has a size limit of 16 MiB for the
uncompressed image.  If that limit will be reached output the
flash image file as 'otheros-too-big.bld'.

Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 arch/powerpc/boot/wrapper | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Michael Ellerman May 15, 2020, 2:02 a.m. UTC | #1
Hi Geoff,

Geoff Levand <geoff@infradead.org> writes:
> The ps3's otheros flash loader has a size limit of 16 MiB for the
> uncompressed image.  If that limit will be reached output the
> flash image file as 'otheros-too-big.bld'.
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
>  arch/powerpc/boot/wrapper | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
> index 35ace40d9fc2..ab1e3ddc79f3 100755
> --- a/arch/powerpc/boot/wrapper
> +++ b/arch/powerpc/boot/wrapper
> @@ -571,7 +571,20 @@ ps3)
>          count=$overlay_size bs=1
>  
>      odir="$(dirname "$ofile.bin")"
> -    rm -f "$odir/otheros.bld"
> -    gzip -n --force -9 --stdout "$ofile.bin" > "$odir/otheros.bld"
> +
> +    # The ps3's flash loader has a size limit of 16 MiB for the uncompressed
> +    # image.  If a compressed image that exceeded this limit is written to
> +    # flash the loader will decompress that image until the 16 MiB limit is
> +    # reached, then enter the system reset vector of the partially decompressed
> +    # image.  No warning is issued.
> +    rm -f "$odir"/{otheros,otheros-too-big}.bld
> +    size=$(${CROSS}nm --no-sort --radix=d "$ofile" | egrep ' _end$' | cut -d' ' -f1)
> +    bld="otheros.bld"
> +    if [ $size -gt $((0x1000000)) ]; then
> +        bld="otheros-too-big.bld"
> +        echo "  INFO: Uncompressed kernel is too large to program into PS3 flash memory;" \

This now appears on all my ppc64_defconfig builds, which I don't really
like.

That does highlight the fact that ppc64_defconfig including
CONFIG_PPC_PS3 is not really helpful for people actually wanting to run
the kernel on a PS3.

So I wonder if we should drop CONFIG_PPC_PS3 from ppc64_defconfig, in
which case I'd be happy to keep the INFO message because it should only
appear on ps3 specific builds.

The other option would be to drop the message, or only print it when
we're doing a verbose build.

Thoughts?

cheers
Geoff Levand May 16, 2020, 4:03 p.m. UTC | #2
Hi Michael,

On 5/14/20 7:02 PM, Michael Ellerman wrote:
> Geoff Levand <geoff@infradead.org> writes:
...
>> +    # The ps3's flash loader has a size limit of 16 MiB for the uncompressed
>> +    # image.  If a compressed image that exceeded this limit is written to
>> +    # flash the loader will decompress that image until the 16 MiB limit is
>> +    # reached, then enter the system reset vector of the partially decompressed
>> +    # image.  No warning is issued.
>> +    rm -f "$odir"/{otheros,otheros-too-big}.bld
>> +    size=$(${CROSS}nm --no-sort --radix=d "$ofile" | egrep ' _end$' | cut -d' ' -f1)
>> +    bld="otheros.bld"
>> +    if [ $size -gt $((0x1000000)) ]; then
>> +        bld="otheros-too-big.bld"
>> +        echo "  INFO: Uncompressed kernel is too large to program into PS3 flash memory;" \
> 
> This now appears on all my ppc64_defconfig builds, which I don't really
> like.

No, neither do I.  I didn't think of that case.

> That does highlight the fact that ppc64_defconfig including
> CONFIG_PPC_PS3 is not really helpful for people actually wanting to run
> the kernel on a PS3.

No, this is just for the bootloader image (.bld) that can be
programed into flash memory.  This is what is used to create,
for example, a petitboot bootloader image.

Normal usage is for the bootloader in flash to load a vmlinux
image from disk or network, in which case running a ppc64_defconfig
image would be fine.

> So I wonder if we should drop CONFIG_PPC_PS3 from ppc64_defconfig, in
> which case I'd be happy to keep the INFO message because it should only
> appear on ps3 specific builds.

I'd like to keep CONFIG_PPC_PS3 set in ppc64_defconfig.  I feel it
useful to get some build testing of the PS3 platform code.

> The other option would be to drop the message, or only print it when
> we're doing a verbose build.

Building a boatloader image to program into flash memory is
something only very advanced users would be doing.  I don't
think they would need this message.  They would see the file
name and understand the situation.  I'll post a v3 patch that
removes the message.

-Geoff
Michael Ellerman May 18, 2020, 6:31 a.m. UTC | #3
Geoff Levand <geoff@infradead.org> writes:
> Hi Michael,
>
> On 5/14/20 7:02 PM, Michael Ellerman wrote:
>> Geoff Levand <geoff@infradead.org> writes:
> ...
>>> +    # The ps3's flash loader has a size limit of 16 MiB for the uncompressed
>>> +    # image.  If a compressed image that exceeded this limit is written to
>>> +    # flash the loader will decompress that image until the 16 MiB limit is
>>> +    # reached, then enter the system reset vector of the partially decompressed
>>> +    # image.  No warning is issued.
>>> +    rm -f "$odir"/{otheros,otheros-too-big}.bld
>>> +    size=$(${CROSS}nm --no-sort --radix=d "$ofile" | egrep ' _end$' | cut -d' ' -f1)
>>> +    bld="otheros.bld"
>>> +    if [ $size -gt $((0x1000000)) ]; then
>>> +        bld="otheros-too-big.bld"
>>> +        echo "  INFO: Uncompressed kernel is too large to program into PS3 flash memory;" \
>> 
>> This now appears on all my ppc64_defconfig builds, which I don't really
>> like.
>
> No, neither do I.  I didn't think of that case.
>
>> That does highlight the fact that ppc64_defconfig including
>> CONFIG_PPC_PS3 is not really helpful for people actually wanting to run
>> the kernel on a PS3.
>
> No, this is just for the bootloader image (.bld) that can be
> programed into flash memory.  This is what is used to create,
> for example, a petitboot bootloader image.
>
> Normal usage is for the bootloader in flash to load a vmlinux
> image from disk or network, in which case running a ppc64_defconfig
> image would be fine.

Ah yep, that rings a bell.

>> So I wonder if we should drop CONFIG_PPC_PS3 from ppc64_defconfig, in
>> which case I'd be happy to keep the INFO message because it should only
>> appear on ps3 specific builds.
>
> I'd like to keep CONFIG_PPC_PS3 set in ppc64_defconfig.  I feel it
> useful to get some build testing of the PS3 platform code.
>
>> The other option would be to drop the message, or only print it when
>> we're doing a verbose build.
>
> Building a boatloader image to program into flash memory is
> something only very advanced users would be doing.  I don't
> think they would need this message.  They would see the file
> name and understand the situation.  I'll post a v3 patch that
> removes the message.

Great, thanks.

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index 35ace40d9fc2..ab1e3ddc79f3 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -571,7 +571,20 @@  ps3)
         count=$overlay_size bs=1
 
     odir="$(dirname "$ofile.bin")"
-    rm -f "$odir/otheros.bld"
-    gzip -n --force -9 --stdout "$ofile.bin" > "$odir/otheros.bld"
+
+    # The ps3's flash loader has a size limit of 16 MiB for the uncompressed
+    # image.  If a compressed image that exceeded this limit is written to
+    # flash the loader will decompress that image until the 16 MiB limit is
+    # reached, then enter the system reset vector of the partially decompressed
+    # image.  No warning is issued.
+    rm -f "$odir"/{otheros,otheros-too-big}.bld
+    size=$(${CROSS}nm --no-sort --radix=d "$ofile" | egrep ' _end$' | cut -d' ' -f1)
+    bld="otheros.bld"
+    if [ $size -gt $((0x1000000)) ]; then
+        bld="otheros-too-big.bld"
+        echo "  INFO: Uncompressed kernel is too large to program into PS3 flash memory;" \
+        "size=0x$(printf "%x\n" $size), limit=0x1000000."
+    fi
+    gzip -n --force -9 --stdout "$ofile.bin" > "$odir/$bld"
     ;;
 esac