From: "Ard Biesheuvel" <ard.biesheuvel@linaro.org>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>,
edk2-devel-groups-io <devel@edk2.groups.io>,
"Kinney, Michael D" <michael.d.kinney@intel.com>,
"Gao, Liming" <liming.gao@intel.com>,
"Zhang, Shenglei" <shenglei.zhang@intel.com>,
Baptiste GERONDEAU <bgerondeau@gmail.com>
Subject: Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
Date: Thu, 19 Sep 2019 13:25:37 +0300 [thread overview]
Message-ID: <CAKv+Gu9AWVtFL3f-gc3Bxoyka3-1NqgkWeK6Du+4oh1Evxg8ug@mail.gmail.com> (raw)
In-Reply-To: <20190919100921.GR28454@bivouac.eciton.net>
On Thu, 19 Sep 2019 at 13:09, Leif Lindholm <leif.lindholm@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.lindholm@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.gerondeau@linaro.org> wrote:
> > > > >
> > > > > From: Baptiste GERONDEAU <bgerondeau@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.gerondeau@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)
next prev parent reply other threads:[~2019-09-19 10:26 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-18 12:25 [PATCH 0/3] Arm builds on Visual Studio Baptiste Gerondeau
2019-09-18 12:25 ` [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format Baptiste Gerondeau
2019-09-19 9:29 ` Ard Biesheuvel
2019-09-18 12:25 ` [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT Baptiste Gerondeau
2019-09-19 9:32 ` Ard Biesheuvel
2019-09-19 9:48 ` Leif Lindholm
2019-09-19 10:01 ` Ard Biesheuvel
2019-09-19 10:09 ` Leif Lindholm
2019-09-19 10:25 ` Ard Biesheuvel [this message]
2019-09-19 10:34 ` Baptiste Gerondeau
2019-09-19 10:37 ` Ard Biesheuvel
2019-09-19 10:47 ` Leif Lindholm
2019-09-19 10:53 ` Ard Biesheuvel
2019-09-19 11:25 ` Leif Lindholm
2019-09-19 12:36 ` Ard Biesheuvel
2019-09-19 14:31 ` Leif Lindholm
2019-09-19 14:44 ` Ard Biesheuvel
2019-09-19 11:07 ` Baptiste Gerondeau
2019-09-19 10:37 ` Leif Lindholm
2019-09-18 12:25 ` [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build Baptiste Gerondeau
2019-09-19 9:38 ` Ard Biesheuvel
2019-09-19 9:52 ` Leif Lindholm
2019-09-19 9:59 ` Ard Biesheuvel
2019-09-18 16:43 ` [PATCH 0/3] Arm builds on Visual Studio Leif Lindholm
2019-09-19 6:19 ` Liming Gao
2019-09-19 9:44 ` Leif Lindholm
2019-09-19 14:53 ` Liming Gao
2019-09-19 19:24 ` [edk2-devel] " Laszlo Ersek
2019-09-19 19:57 ` Andrew Fish
2019-09-19 20:27 ` Leif Lindholm
2019-09-24 1:28 ` Liming Gao
[not found] <cover.1568821123.git.baptiste.gerondeau@linaro.org>
2019-09-18 16:05 ` [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT Baptiste Gerondeau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu9AWVtFL3f-gc3Bxoyka3-1NqgkWeK6Du+4oh1Evxg8ug@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox