| Message ID | 20190722213958.5761-19-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Rework head.S to make it more compliant with the Arm Arm | expand |
On Mon, 22 Jul 2019, Julien Grall wrote: > Arm64 provides instructions to load a PC-relative address, but with some > limitations: > - adr is enable to cope with +/-1MB > - adrp is enale to cope with +/-4GB but relative to a 4KB page > address > > Because of that, the code requires to use 2 instructions to load any Xen > symbol. To make the code more obvious, introducing a new macro adr_l is > introduced. > > The new macro is used to replace a couple of open-coded use in > efi_xen_start. > > The macro is copied from Linux 5.2-rc4. I was going to ask why the strange name "adr_l", now I know why :-) I'd suggest to name it more clearly to maybe "adr_relative"? In any case: Acked-by: Stefano Stabellini <sstabellini@kernel.org> > Signed-off-by: Julien Grall <julien.grall@arm.coM> Typo in your address > --- > Changes in v2: > - Patch added > --- > xen/arch/arm/arm64/head.S | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 9afd89d447..2287f3ce48 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -111,6 +111,18 @@ > > #endif /* !CONFIG_EARLY_PRINTK */ > > +/* > + * Pseudo-op for PC relative adr <reg>, <symbol> where <symbol> is > + * within the range +/- 4GB of the PC. > + * > + * @dst: destination register (64 bit wide) > + * @sym: name of the symbol > + */ > +.macro adr_l, dst, sym > + adrp \dst, \sym > + add \dst, \dst, :lo12:\sym > +.endm > + > /* Load the physical address of a symbol into xb */ > .macro load_paddr xb, sym > ldr \xb, =\sym > @@ -886,11 +898,9 @@ ENTRY(efi_xen_start) > * Flush dcache covering current runtime addresses > * of xen text/data. Then flush all of icache. > */ > - adrp x1, _start > - add x1, x1, #:lo12:_start > + adr_l x1, _start > mov x0, x1 > - adrp x2, _end > - add x2, x2, #:lo12:_end > + adr_l x2, _end > sub x1, x2, x1 > > bl __flush_dcache_area > -- > 2.11.0 >
Hi, On 7/30/19 7:24 PM, Stefano Stabellini wrote: > On Mon, 22 Jul 2019, Julien Grall wrote: >> Arm64 provides instructions to load a PC-relative address, but with some >> limitations: >> - adr is enable to cope with +/-1MB >> - adrp is enale to cope with +/-4GB but relative to a 4KB page >> address >> >> Because of that, the code requires to use 2 instructions to load any Xen >> symbol. To make the code more obvious, introducing a new macro adr_l is >> introduced. >> >> The new macro is used to replace a couple of open-coded use in >> efi_xen_start. >> >> The macro is copied from Linux 5.2-rc4. > > I was going to ask why the strange name "adr_l", now I know why :-) I think this stands for "load". > > I'd suggest to name it more clearly to maybe "adr_relative"? This is a bit weird to have one full word and the other one shorten. The current solution has the advantage to be short and therefore looks like an instruction (and so keep everything correctly aligned). So I would prefer to keep the function as is. > In any case: > > Acked-by: Stefano Stabellini <sstabellini@kernel.org> Thank you. > >> Signed-off-by: Julien Grall <julien.grall@arm.coM> > > Typo in your address > > >> --- >> Changes in v2: >> - Patch added >> --- >> xen/arch/arm/arm64/head.S | 18 ++++++++++++++---- >> 1 file changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >> index 9afd89d447..2287f3ce48 100644 >> --- a/xen/arch/arm/arm64/head.S >> +++ b/xen/arch/arm/arm64/head.S >> @@ -111,6 +111,18 @@ >> >> #endif /* !CONFIG_EARLY_PRINTK */ >> >> +/* >> + * Pseudo-op for PC relative adr <reg>, <symbol> where <symbol> is >> + * within the range +/- 4GB of the PC. >> + * >> + * @dst: destination register (64 bit wide) >> + * @sym: name of the symbol >> + */ >> +.macro adr_l, dst, sym >> + adrp \dst, \sym >> + add \dst, \dst, :lo12:\sym >> +.endm >> + >> /* Load the physical address of a symbol into xb */ >> .macro load_paddr xb, sym >> ldr \xb, =\sym >> @@ -886,11 +898,9 @@ ENTRY(efi_xen_start) >> * Flush dcache covering current runtime addresses >> * of xen text/data. Then flush all of icache. >> */ >> - adrp x1, _start >> - add x1, x1, #:lo12:_start >> + adr_l x1, _start >> mov x0, x1 >> - adrp x2, _end >> - add x2, x2, #:lo12:_end >> + adr_l x2, _end >> sub x1, x2, x1 >> >> bl __flush_dcache_area >> -- >> 2.11.0 >>
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 9afd89d447..2287f3ce48 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -111,6 +111,18 @@ #endif /* !CONFIG_EARLY_PRINTK */ +/* + * Pseudo-op for PC relative adr <reg>, <symbol> where <symbol> is + * within the range +/- 4GB of the PC. + * + * @dst: destination register (64 bit wide) + * @sym: name of the symbol + */ +.macro adr_l, dst, sym + adrp \dst, \sym + add \dst, \dst, :lo12:\sym +.endm + /* Load the physical address of a symbol into xb */ .macro load_paddr xb, sym ldr \xb, =\sym @@ -886,11 +898,9 @@ ENTRY(efi_xen_start) * Flush dcache covering current runtime addresses * of xen text/data. Then flush all of icache. */ - adrp x1, _start - add x1, x1, #:lo12:_start + adr_l x1, _start mov x0, x1 - adrp x2, _end - add x2, x2, #:lo12:_end + adr_l x2, _end sub x1, x2, x1 bl __flush_dcache_area
Arm64 provides instructions to load a PC-relative address, but with some limitations: - adr is enable to cope with +/-1MB - adrp is enale to cope with +/-4GB but relative to a 4KB page address Because of that, the code requires to use 2 instructions to load any Xen symbol. To make the code more obvious, introducing a new macro adr_l is introduced. The new macro is used to replace a couple of open-coded use in efi_xen_start. The macro is copied from Linux 5.2-rc4. Signed-off-by: Julien Grall <julien.grall@arm.coM> --- Changes in v2: - Patch added --- xen/arch/arm/arm64/head.S | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)