On Thu, 19 Sep 2019 at 13:09, Leif Lindholm <leif.lindh...@linaro.org> wrote:
>
> On Thu, Sep 19, 2019 at 01:01:04PM +0300, Ard Biesheuvel wrote:
> > On Thu, 19 Sep 2019 at 12:48, Leif Lindholm <leif.lindh...@linaro.org>
> > wrote:
> > >
> > > On Thu, Sep 19, 2019 at 12:32:56PM +0300, Ard Biesheuvel wrote:
> > > > On Wed, 18 Sep 2019 at 15:27, Baptiste Gerondeau
> > > > <baptiste.gerond...@linaro.org> wrote:
> > > > >
> > > > > From: Baptiste GERONDEAU <bgerond...@gmail.com>
> > > > >
> > > > > RVCT and MSFT's ARM assembler share the same file syntax, but some
> > > > > instructions use pre-UAL syntax that is not picked up
> > > > > by MSFT's ARM assembler, this commit translates those instructions
> > > > > into MSFT-buildable ones (subset of UAL/THUMB).
> > > > >
> > > > > Signed-off-by: Baptiste Gerondeau <baptiste.gerond...@linaro.org>
> > > > > ---
> > > > > ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30
> > > > > +++++++++++++++++-------------
> > > > > ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
> > > > > MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18
> > > > > +++++++++---------
> > > > > 3 files changed, 30 insertions(+), 24 deletions(-)
> > > > >
> > > > > diff --git a/ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm
> > > > > b/ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm
> > > > > index aa0229d2e85f..880246bd6206 100644
> > > > > --- a/ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm
> > > > > +++ b/ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm
> > > > > @@ -90,7 +90,7 @@ Fiq
> > > > > ResetEntry
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > ; We are already in SVC mode
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > >
> > > > > @@ -102,7 +102,7 @@ UndefinedInstructionEntry
> > > > > sub LR, LR, #4 ; Only -2 for Thumb, adjust in
> > > > > CommonExceptionEntry
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > cps #0x13 ; Switch to SVC for common
> > > > > stack
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > >
> > > > > @@ -113,7 +113,7 @@ UndefinedInstructionEntry
> > > > > SoftwareInterruptEntry
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > ; We are already in SVC mode
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > >
> > > > > @@ -125,7 +125,7 @@ PrefetchAbortEntry
> > > > > sub LR,LR,#4
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > cps #0x13 ; Switch to SVC for common
> > > > > stack
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > >
> > > > > @@ -137,7 +137,7 @@ DataAbortEntry
> > > > > sub LR,LR,#8
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > cps #0x13 ; Switch to SVC for common
> > > > > stack
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > >
> > > > > @@ -148,7 +148,7 @@ DataAbortEntry
> > > > > ReservedExceptionEntry
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > cps #0x13 ; Switch to SVC for common
> > > > > stack
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > >
> > > > > @@ -160,7 +160,7 @@ IrqEntry
> > > > > sub LR,LR,#4
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > cps #0x13 ; Switch to SVC for common
> > > > > stack
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > >
> > > > > @@ -172,7 +172,7 @@ FiqEntry
> > > > > sub LR,LR,#4
> > > > > srsfd #0x13! ; Store return state on SVC
> > > > > stack
> > > > > cps #0x13 ; Switch to SVC for common
> > > > > stack
> > > > > - stmfd SP!,{LR} ; Store the link register for
> > > > > the current mode
> > > > > + push {LR} ; Store the link register for
> > > > > the current mode
> > > > > sub SP,SP,#0x20 ; Save space for SP, LR, PC,
> > > > > IFAR - CPSR
> > > > > stmfd SP!,{R0-R12} ; Store the register state
> > > > > ; Since we have already switch
> > > > > to SVC R8_fiq - R12_fiq
> > > > > @@ -213,9 +213,11 @@ AsmCommonExceptionEntry
> > > > > and R3, R1, #0x1f ; Check CPSR to see if User or
> > > > > System Mode
> > > > > cmp R3, #0x1f ; if ((CPSR == 0x10) || (CPSR ==
> > > > > 0x1f))
> > > > > cmpne R3, #0x10 ;
> > > > > - stmeqed R2, {lr}^ ; save unbanked lr
> > > > > + mrseq R8, lr_usr ; save unbanked lr to R8
> > > > > + streq R2, [R8] ; make R2 point to R8
> > > > > ; else
> > > > > - stmneed R2, {lr} ; save SVC lr
> > > > > + mrsne R8, lr_svc ; save SVC lr to R8
> > > > > + strne R2, [R8] ; make R2 point to R8
> > > > >
> > > >
> > > > Can you make this str unconditional, and drop the former?
> > >
> > > Yeah, that would be an improvement.
> > >
> > > > >
> > > > > ldr R5, [SP, #0x58] ; PC is the LR pushed by srsfd
> > > > > @@ -280,15 +282,17 @@ CommonCExceptionHandler (
> > > > > and R1, R1, #0x1f ; Check to see if User or System
> > > > > Mode
> > > > > cmp R1, #0x1f ; if ((CPSR == 0x10) || (CPSR ==
> > > > > 0x1f))
> > > > > cmpne R1, #0x10 ;
> > > > > - ldmeqed R2, {lr}^ ; restore unbanked lr
> > > > > + ldreq R8, [R2] ; load sys/usr lr from R2
> > > > > pointer
> > > > > + msreq lr_usr, R8 ; restore unbanked lr
> > > > > ; else
> > > > > - ldmneed R3, {lr} ; restore SVC lr, via ldmfd
> > > > > SP!, {LR}
> > > > > + ldrne R8, [R3] ; load SVC lr from R3 pointer
> > > > > + msrne lr_svc, R8 ; restore SVC lr, via ldmfd
> > > > > SP!, {LR}
> > > > >
> > > > > ldmfd SP!,{R0-R12} ; Restore general purpose
> > > > > registers
> > > > > ; Exception handler can not
> > > > > change SP
> > > > >
> > > > > add SP,SP,#0x20 ; Clear out the remaining stack
> > > > > space
> > > > > - ldmfd SP!,{LR} ; restore the link register for
> > > > > this context
> > > > > + pop {LR} ; restore the link register for
> > > > > this context
> > > > > rfefd SP! ; return from exception via
> > > > > srsfd stack slot
> > > > >
> > > > > END
> > > > > diff --git a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm
> > > > > b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm
> > > > > index 3146c2b52181..724306399e6c 100644
> > > > > --- a/ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm
> > > > > +++ b/ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm
> > > > > @@ -200,8 +200,10 @@ Loop2
> > > > > mov R9, R4 ; R9 working copy of the max way
> > > > > size (right aligned)
> > > > >
> > > > > Loop3
> > > > > - orr R0, R10, R9, LSL R5 ; factor in the way number and cache
> > > > > number into R11
> > > > > - orr R0, R0, R7, LSL R2 ; factor in the index number
> > > > > + lsl R8, R9, R5
> > > > > + orr R0, R10, R8 ; factor in the way number and cache
> > > > > number
> > > > > + lsl R8, R7, R2
> > > > > + orr R0, R0, R8 ; factor in the index number
> > > > >
> > > >
> > > > What's wrong with this code?
> > >
> > > Inline barrel shifter is only available in A32, not T32 (and VS seems
> > > to love T32).
> >
> > So is this simply the default of the compiler? I'd prefer it if we
> > could add a 'CODE 32' directive instead, that way, we may not need any
> > of the other changes to begin with.
>
> Some of them really weren't supported regardless (and the indentation
> change of directives was required).
>
> This one, possible - Baptiste?
>
> Oh, and the CODE 32 directive is not supported - it's ARM or THUMB :)
> https://docs.microsoft.com/en-us/cpp/assembler/arm/arm-assembler-directives?view=vs-2019
>
OK.
In any case, I'd strongly prefer it if the .S and .asm files produced
identical object code, so please apply the same changes to the sibling
.S files as well, please (but only the ones that are really required
when building it in ARM mode)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47588): https://edk2.groups.io/g/devel/message/47588
Mute This Topic: https://groups.io/mt/34187299/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
