| Message ID | 20190913192433.4316-2-palmer@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Documentation: riscv: Image cleanups | expand |
On Fri, 13 Sep 2019, Palmer Dabbelt wrote: > The documentation for our bootloader claims that it can be made > compatible with arm64, but that's not true. There are some differences > between our format and arm64: > > * We've used the first 32 bits of their 64-bit "res2" as a version number, > which we're currently setting to non-zero. During the review of this patch, I assumed -- maybe incorrectly -- that ARM64 didn't require the reserved fields to be zero. In retrospect, this was not a good assumption to make. It would have been better for me to get explicit agreement from the ARM64 folks. > * We're using their "res4" field as our magic number. > * We're treating their magic number as our "res3" field, This looks like the key issue. Let's rename the 32-bit RISC-V res3 field as magic2, and just use it. Then over time we should be able to deprecate the original RISC-V 64-bit magic field. > which nominally contains a flag for big endian systems already. This > can't get set, so we should just drop it -- it's also not described > what the flag means. This one I'm not seeing. In both headers, the flags field is in the same place: towards the beginning of the headers, before the reserved fields: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/include/asm/image.h#n56 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/include/asm/image.h#n49 The endianness bit in that field is defined the same way: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.rst#n106 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/riscv/boot-image-header.txt#n46 Please let me know if I've misunderstood your point. > > This patch removes the claim that our header can be made compatible with > arm64. > > Signed-off-by: Palmer Dabbelt <palmer@sifive.com> > --- > Documentation/riscv/boot-image-header.txt | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/Documentation/riscv/boot-image-header.txt b/Documentation/riscv/boot-image-header.txt > index 1b73fea23b39..77e8e505bc41 100644 > --- a/Documentation/riscv/boot-image-header.txt > +++ b/Documentation/riscv/boot-image-header.txt > @@ -21,9 +21,8 @@ The following 64-byte header is present in decompressed Linux kernel image. > u32 res3; /* Reserved for additional RISC-V specific header */ > u32 res4; /* Reserved for PE COFF offset */ > > -This header format is compliant with PE/COFF header and largely inspired from > -ARM64 header. Thus, both ARM64 & RISC-V header can be combined into one common > -header in future. > +This header format is compliant with PE/COFF header and largely inspired by, > +but not compatible with, the ARM64 header. > > Notes: > - This header can also be reused to support EFI stub for RISC-V in future. EFI > -- > 2.21.0 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv > - Paul
On Fri, 2019-09-13 at 18:13 -0700, Paul Walmsley wrote: > On Fri, 13 Sep 2019, Palmer Dabbelt wrote: > > > The documentation for our bootloader claims that it can be made > > compatible with arm64, but that's not true. There are some > > differences > > between our format and arm64: > > > > * We've used the first 32 bits of their 64-bit "res2" as a version > > number, > > which we're currently setting to non-zero. > > During the review of this patch, I assumed -- maybe incorrectly -- > that > ARM64 didn't require the reserved fields to be zero. In retrospect, > this > was not a good assumption to make. It would have been better for me > to > get explicit agreement from the ARM64 folks. > Any reserved field is set to zero. The idea was to modify the reserve field for ARM to have a flag version if we ever unify both ARM64 & RISC-V headers. > > * We're using their "res4" field as our magic number. > > * We're treating their magic number as our "res3" field, > > This looks like the key issue. Let's rename the 32-bit RISC-V res3 > field > as magic2, and just use it. Then over time we should be able to > deprecate > the original RISC-V 64-bit magic field. > Originally, "RISCV" magic value was a 64-bit value. That's why I kept in res4 field. Again, my plan was to have a arch specific ifdef only for magic values if we ever unify both ARM64 & RISC-V headers. We never got the ack for unification of boot header proposal from ARM maintainers. Thus, these things never got implemented. > > which nominally contains a flag for big endian systems > > already. This > > can't get set, so we should just drop it -- it's also not > > described > > what the flag means. > > This one I'm not seeing. In both headers, the flags field is in the > same place: towards the beginning of the headers, before the > reserved > fields: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/include/asm/image.h#n56 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/include/asm/image.h#n49 > > The endianness bit in that field is defined the same way: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.rst#n106 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/riscv/boot-image-header.txt#n46 > > Please let me know if I've misunderstood your point. > I agree with Paul. Regards, Atish > > This patch removes the claim that our header can be made compatible > > with > > arm64. > > > > Signed-off-by: Palmer Dabbelt <palmer@sifive.com> > > --- > > Documentation/riscv/boot-image-header.txt | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/riscv/boot-image-header.txt > > b/Documentation/riscv/boot-image-header.txt > > index 1b73fea23b39..77e8e505bc41 100644 > > --- a/Documentation/riscv/boot-image-header.txt > > +++ b/Documentation/riscv/boot-image-header.txt > > @@ -21,9 +21,8 @@ The following 64-byte header is present in > > decompressed Linux kernel image. > > u32 res3; /* Reserved for additional RISC-V > > specific header */ > > u32 res4; /* Reserved for PE COFF offset */ > > > > -This header format is compliant with PE/COFF header and largely > > inspired from > > -ARM64 header. Thus, both ARM64 & RISC-V header can be combined > > into one common > > -header in future. > > +This header format is compliant with PE/COFF header and largely > > inspired by, > > +but not compatible with, the ARM64 header. > > > > Notes: > > - This header can also be reused to support EFI stub for RISC-V in > > future. EFI > > -- > > 2.21.0 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > - Paul
diff --git a/Documentation/riscv/boot-image-header.txt b/Documentation/riscv/boot-image-header.txt index 1b73fea23b39..77e8e505bc41 100644 --- a/Documentation/riscv/boot-image-header.txt +++ b/Documentation/riscv/boot-image-header.txt @@ -21,9 +21,8 @@ The following 64-byte header is present in decompressed Linux kernel image. u32 res3; /* Reserved for additional RISC-V specific header */ u32 res4; /* Reserved for PE COFF offset */ -This header format is compliant with PE/COFF header and largely inspired from -ARM64 header. Thus, both ARM64 & RISC-V header can be combined into one common -header in future. +This header format is compliant with PE/COFF header and largely inspired by, +but not compatible with, the ARM64 header. Notes: - This header can also be reused to support EFI stub for RISC-V in future. EFI
The documentation for our bootloader claims that it can be made compatible with arm64, but that's not true. There are some differences between our format and arm64: * We've used the first 32 bits of their 64-bit "res2" as a version number, which we're currently setting to non-zero. * We're using their "res4" field as our magic number. * We're treating their magic number as our "res3" field, which nominally contains a flag for big endian systems already. This can't get set, so we should just drop it -- it's also not described what the flag means. This patch removes the claim that our header can be made compatible with arm64. Signed-off-by: Palmer Dabbelt <palmer@sifive.com> --- Documentation/riscv/boot-image-header.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)