|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xen:arm: Populate arm64 image header
On 09/01/2018 02:01 PM, Amit Tomer wrote: > Hello, > >> Yes, and in fact it seems one can work around this by cleverly >> constructing the load addresses, > > But we are really dealing a corner case here. No matter where > we load the image, it would be relocated to 0x80000( since dram > starts at 0x0000...) and unfortunately first 16MiB is reserved for > ROM Firmware. > >>>> This unwanted situation can be fixed by updating image_size field >>>> along side kernel flags so that image wouldn't relocate from initial >>>> load address. >>> >>> I think the first step is to fix your U-boot and rethink where you load >>> your binaries. >> >> I think U-Boot perfectly complies with the kernel document. Xen not so >> much. The kernel image format was deliberately updated to become more >> flexible with certain memory layout situations as we have here. >> There is for instance a problem if there is something precious at 512KB >> into DRAM (secure memory owned by firmware), as regardless of the load >> addresses the user chooses U-Boot will (rightfully!) revert to the >> original "512KB into DRAM" address to keep compatibility with older >> kernels - and it believes Xen is such a one because of the ancient >> header format. >> >> But ... >> >>> Regarding the patch in itself, I think this is a good addition as it >>> allow Xen to be loaded in more places. But please rewrite the commit >>> message accordingly, this is an update to a new version. >> >> I totally agree with that, the commit message should be reworded to >> stress that we want to comply with a newer version of the kernel image >> header (which is around for four years by now!), and just mention that >> it fixes problems with non-ancient U-Boots on certain platforms as an >> additional reason. >> >>>> [1]:https://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/lib/image.c;h=699bf44e702f7a7084997406203fd7d2aaaf87fa;hb=HEAD#l50 >>>> >>>> >>>> These changes are derived from kernel v4.18 files >>>> >>>> Signed-off-by: Amit Singh Tomar <amittomer25@xxxxxxxxx> >>>> --- >>>> xen/arch/arm/arm64/head.S | 5 ++- >>>> xen/arch/arm/arm64/lib/assembler.h | 11 +++++ >>>> xen/arch/arm/xen.lds.S | 3 ++ >>>> xen/include/asm-arm/arm64/linux_header_vars.h | 62 >>>> +++++++++++++++++++++++++++ >>>> 4 files changed, 79 insertions(+), 2 deletions(-) >>>> create mode 100644 xen/include/asm-arm/arm64/linux_header_vars.h >>>> >>>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >>>> index d63734f..ce72c95 100644 >>>> --- a/xen/arch/arm/arm64/head.S >>>> +++ b/xen/arch/arm/arm64/head.S >>>> @@ -25,6 +25,7 @@ >>>> #include <asm/early_printk.h> >>>> #include <efi/efierr.h> >>>> #include <asm/arm64/efibind.h> >>>> +#include "lib/assembler.h" >>>> #define PT_PT 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 >>>> P=1 */ >>>> #define PT_MEM 0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 >>>> P=1 */ >>>> @@ -120,8 +121,8 @@ efi_head: >>>> add x13, x18, #0x16 >>>> b real_start /* branch to kernel start */ >>>> .quad 0 /* Image load offset from start >>>> of RAM */ >>>> - .quad 0 /* reserved */ >>>> - .quad 0 /* reserved */ >>>> + le64sym _kernel_size_le /* Effective size of kernel >>>> image, little-endian */ >>>> + le64sym _kernel_flags_le /* Informative flags, >>>> little-endian */ >>> >>> All the dance for to convert to little endian is not necessary on Xen. >>> Please rework your series to avoid such code, this would also reduce >>> quite significantly the series. >> >> Indeed! > > How about this change? The diff below doesn't make sense. Please just send a new version without *any* endianess code and with the changed commit message - focusing on the overdue image format update as the main rationale and just mentioning the platform as an example. Cheers, Andre. > > index ce72c95..ec29e01 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -121,8 +121,8 @@ efi_head: > add x13, x18, #0x16 > b real_start /* branch to kernel start */ > .quad 0 /* Image load offset from start of RAM > */ > - le64sym _kernel_size_le /* Effective size of kernel > image, little-endian */ > - le64sym _kernel_flags_le /* Informative flags, little-endian */ > + .quad _end - start /* Effective size of kernel image, > little-endian */ > + .quad __HEAD_FLAGS /* Informative flags, little-endian */ > .quad 0 /* reserved */ > .quad 0 /* reserved */ > .quad 0 /* reserved */ > diff --git a/xen/arch/arm/arm64/lib/assembler.h > b/xen/arch/arm/arm64/lib/assembler.h > index c0ef758..05861b8 100644 > --- a/xen/arch/arm/arm64/lib/assembler.h > +++ b/xen/arch/arm/arm64/lib/assembler.h > @@ -9,15 +9,11 @@ > #define CPU_BE(x...) > #define CPU_LE(x...) x > > - /* > - * Emit a 64-bit absolute little endian symbol reference in a way that > - * ensures that it will be resolved at build time, even when building a > - * PIE binary. This requires cooperation from the linker script, which > - * must emit the lo32/hi32 halves individually. > - */ > - .macro le64sym, sym > - .long \sym\()_lo32 > - .long \sym\()_hi32 > - .endm > +#define __HEAD_FLAG_PAGE_SIZE 1 /* 4K hard-coded */ > + > +#define __HEAD_FLAG_PHYS_BASE 1 > + > +#define __HEAD_FLAGS ((__HEAD_FLAG_PAGE_SIZE << 1) | \ > > Thanks > -Amit > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |