* [PATCH 0/3] Arm builds on Visual Studio
@ 2019-09-18 12:25 Baptiste Gerondeau
2019-09-18 12:25 ` [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format Baptiste Gerondeau
` (4 more replies)
0 siblings, 5 replies; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-18 12:25 UTC (permalink / raw)
To: devel
Cc: ard.biesheuvel, leif.lindholm, michael.d.kinney, liming.gao,
shenglei.zhang, Baptiste Gerondeau
We are currently making an effort to make ARM (and AARCH64 eventually)
builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
These 3 patches correspond to an effort to make the assembler work with
MSFT, which entails :
- Feeding MSFT the RVCT .asm files, since they share syntax
requirements.
- Fixing some instructions syntax in those .asm files, in order to make
them palatable for MSFT.
- Fixing some minor formatting issue in INF files, while we're at it.
This set enables the assembler, meanwhile the C also require changes,
which will come in a set later. This set makes the RVCT toolchain family
and profiles obsolete, unblocking :
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750
As mentioned in the above bug, dropping RVCT would entail orphanating
the .asm files that powered the RVCT build. Since Visual Studio uses the
same file syntax, those can be reused to power the VS build.
These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1)
Baptiste GERONDEAU (3):
ArmPkg/MdePkg : Unify INF files format
ARM/Assembler: Correct syntax from RVCT for MSFT
ARM/Assembler: Reuse RVCT assembler for MSFT build
ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30 +++++++++++++++++-------------
ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++--
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +-
ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +-
ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18 +++++++++---------
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 20 ++++++++++----------
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
21 files changed, 65 insertions(+), 59 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format
2019-09-18 12:25 [PATCH 0/3] Arm builds on Visual Studio Baptiste Gerondeau
@ 2019-09-18 12:25 ` 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
` (3 subsequent siblings)
4 siblings, 1 reply; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-18 12:25 UTC (permalink / raw)
To: devel
Cc: ard.biesheuvel, leif.lindholm, michael.d.kinney, liming.gao,
shenglei.zhang, Baptiste Gerondeau, Baptiste GERONDEAU
From: Baptiste GERONDEAU <bgerondeau@gmail.com>
Add a space between the '|' and the name of the toolchain to use,
as is the case in all other INF files.
Note that I did not touch the RVCT lines, since a following commit in
the set will address those.
Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
---
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +-
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
index f4fecbb4098a..33dddf1e2b97 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
@@ -22,7 +22,7 @@ [Sources.AARCH64]
[Sources.ARM]
Arm/ArmMmuLibCore.c
- Arm/ArmMmuLibV7Support.S |GCC
+ Arm/ArmMmuLibV7Support.S | GCC
Arm/ArmMmuLibV7Support.asm |RVCT
[Packages]
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
index e4e3d532e7b8..d38e1397eee1 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
@@ -79,11 +79,11 @@ [Defines.ARM, Defines.AARCH64]
LIBRARY_CLASS = BaseMemoryLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
[Sources.ARM]
- Arm/ScanMem.S |GCC
- Arm/SetMem.S |GCC
- Arm/CopyMem.S |GCC
- Arm/CompareMem.S |GCC
- Arm/CompareGuid.S |GCC
+ Arm/ScanMem.S | GCC
+ Arm/SetMem.S | GCC
+ Arm/CopyMem.S | GCC
+ Arm/CompareMem.S | GCC
+ Arm/CompareGuid.S | GCC
Arm/ScanMem.asm |RVCT
Arm/SetMem.asm |RVCT
--
2.23.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
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-18 12:25 ` Baptiste Gerondeau
2019-09-19 9:32 ` Ard Biesheuvel
2019-09-18 12:25 ` [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build Baptiste Gerondeau
` (2 subsequent siblings)
4 siblings, 1 reply; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-18 12:25 UTC (permalink / raw)
To: devel
Cc: ard.biesheuvel, leif.lindholm, michael.d.kinney, liming.gao,
shenglei.zhang, Baptiste Gerondeau, Baptiste GERONDEAU
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
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
blx R1
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
index 5a423df16bff..a46d70e41433 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
+++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
@@ -5,16 +5,16 @@
;
-AREA IoLibMmio, CODE, READONLY
+ AREA IoLibMmio, CODE, READONLY
-EXPORT MmioRead8Internal
-EXPORT MmioWrite8Internal
-EXPORT MmioRead16Internal
-EXPORT MmioWrite16Internal
-EXPORT MmioRead32Internal
-EXPORT MmioWrite32Internal
-EXPORT MmioRead64Internal
-EXPORT MmioWrite64Internal
+ EXPORT MmioRead8Internal
+ EXPORT MmioWrite8Internal
+ EXPORT MmioRead16Internal
+ EXPORT MmioWrite16Internal
+ EXPORT MmioRead32Internal
+ EXPORT MmioWrite32Internal
+ EXPORT MmioRead64Internal
+ EXPORT MmioWrite64Internal
;
; Reads an 8-bit MMIO register.
--
2.23.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build
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-18 12:25 ` [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT Baptiste Gerondeau
@ 2019-09-18 12:25 ` Baptiste Gerondeau
2019-09-19 9:38 ` Ard Biesheuvel
2019-09-18 16:43 ` [PATCH 0/3] Arm builds on Visual Studio Leif Lindholm
2019-09-19 6:19 ` Liming Gao
4 siblings, 1 reply; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-18 12:25 UTC (permalink / raw)
To: devel
Cc: ard.biesheuvel, leif.lindholm, michael.d.kinney, liming.gao,
shenglei.zhang, Baptiste Gerondeau, Baptiste GERONDEAU
From: Baptiste GERONDEAU <bgerondeau@gmail.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750"
Since RVCT shares the same assembler syntax as MSFT, use .asm files
and associate them with MSFT, which would be a first step to addressing
the above Bugzilla issue.
RVCT will also have to be erased from BaseTools/rest of the build
infrastructure, to fully address BZ#1750 ; this patch only addresses the
"code" in itself.
Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
---
ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +-
ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +-
ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +-
ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 10 +++++-----
MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
18 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
index 5e23c732bfab..4fccb938eb6d 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
@@ -22,7 +22,7 @@ [Sources]
[Sources.ARM]
GicV3/Arm/ArmGicV3.S | GCC
- GicV3/Arm/ArmGicV3.asm | RVCT
+ GicV3/Arm/ArmGicV3.asm | MSFT
[Sources.AARCH64]
GicV3/AArch64/ArmGicV3.S
diff --git a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
index fdb9c24d21bc..58b2ddbff858 100644
--- a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
+++ b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
@@ -33,7 +33,7 @@ [Sources.common]
[Sources.Arm]
Arm/ArmException.c
- Arm/ExceptionSupport.asm | RVCT
+ Arm/ExceptionSupport.asm | MSFT
Arm/ExceptionSupport.S | GCC
[Sources.AARCH64]
diff --git a/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf b/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
index ef1a43a27c45..a404ca2ccf82 100644
--- a/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
+++ b/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
@@ -28,7 +28,7 @@ [Sources.common]
[Sources.Arm]
Arm/ArmException.c
- Arm/ExceptionSupport.asm | RVCT
+ Arm/ExceptionSupport.asm | MSFT
Arm/ExceptionSupport.S | GCC
[Sources.AARCH64]
diff --git a/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
index 69f68f63d7a6..be8d8a228865 100644
--- a/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
+++ b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
@@ -15,7 +15,7 @@ [Defines]
LIBRARY_CLASS = ArmHvcLib
[Sources.ARM]
- Arm/ArmHvc.asm | RVCT
+ Arm/ArmHvc.asm | MSFT
Arm/ArmHvc.S | GCC
[Sources.AARCH64]
diff --git a/ArmPkg/Library/ArmLib/ArmBaseLib.inf b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
index 5e70990872f2..63e175623393 100644
--- a/ArmPkg/Library/ArmLib/ArmBaseLib.inf
+++ b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
@@ -30,10 +30,10 @@ [Sources.ARM]
Arm/ArmV7Support.S | GCC
Arm/ArmV7ArchTimerSupport.S | GCC
- Arm/ArmLibSupport.asm | RVCT
- Arm/ArmLibSupportV7.asm | RVCT
- Arm/ArmV7Support.asm | RVCT
- Arm/ArmV7ArchTimerSupport.asm | RVCT
+ Arm/ArmLibSupport.asm | MSFT
+ Arm/ArmLibSupportV7.asm | MSFT
+ Arm/ArmV7Support.asm | MSFT
+ Arm/ArmV7ArchTimerSupport.asm | MSFT
[Sources.AARCH64]
AArch64/AArch64Lib.h
diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
index 33dddf1e2b97..44366f02c6d9 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
@@ -23,7 +23,7 @@ [Sources.AARCH64]
[Sources.ARM]
Arm/ArmMmuLibCore.c
Arm/ArmMmuLibV7Support.S | GCC
- Arm/ArmMmuLibV7Support.asm |RVCT
+ Arm/ArmMmuLibV7Support.asm | MSFT
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
index 4f4b09f4528a..af8c0e53cc2b 100644
--- a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+++ b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
@@ -14,7 +14,7 @@ [Defines]
LIBRARY_CLASS = ArmSmcLib
[Sources.ARM]
- Arm/ArmSmc.asm | RVCT
+ Arm/ArmSmc.asm | MSFT
Arm/ArmSmc.S | GCC
[Sources.AARCH64]
diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
index fa19bf649131..f4c9e5510b9a 100644
--- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
+++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
@@ -21,7 +21,7 @@ [Sources.AARCH64]
[Sources.ARM]
Arm/Reset.S | GCC
- Arm/Reset.asm | RVCT
+ Arm/Reset.asm | MSFT
[Sources]
ArmSmcPsciResetSystemLib.c
diff --git a/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf b/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
index 744a29fbf723..6631e40df130 100644
--- a/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
+++ b/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
@@ -14,7 +14,7 @@ [Defines]
LIBRARY_CLASS = ArmSvcLib
[Sources.ARM]
- Arm/ArmSvc.asm | RVCT
+ Arm/ArmSvc.asm | MSFT
Arm/ArmSvc.S | GCC
[Sources.AARCH64]
diff --git a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
index e0d0028d8224..cc791a3a68fd 100644
--- a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
+++ b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
@@ -29,7 +29,7 @@ [Sources.common]
[Sources.Arm]
Arm/ArmPlatformHelper.S | GCC
- Arm/ArmPlatformHelper.asm | RVCT
+ Arm/ArmPlatformHelper.asm | MSFT
[Sources.AArch64]
AArch64/ArmPlatformHelper.S
diff --git a/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf b/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
index 76f809c80d9f..e88330c1c382 100644
--- a/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
+++ b/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
@@ -21,7 +21,7 @@ [Packages]
ArmPlatformPkg/ArmPlatformPkg.dec
[Sources.ARM]
- Arm/ArmPlatformStackLib.asm | RVCT
+ Arm/ArmPlatformStackLib.asm | MSFT
Arm/ArmPlatformStackLib.S | GCC
[Sources.AARCH64]
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
index f2ac45d171bc..b663ff749182 100644
--- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
+++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
@@ -21,11 +21,11 @@ [Sources.common]
[Sources.ARM]
Arm/ArchPrePeiCore.c
- Arm/PrePeiCoreEntryPoint.asm | RVCT
+ Arm/PrePeiCoreEntryPoint.asm | MSFT
Arm/PrePeiCoreEntryPoint.S | GCC
- Arm/SwitchStack.asm | RVCT
+ Arm/SwitchStack.asm | MSFT
Arm/SwitchStack.S | GCC
- Arm/Exception.asm | RVCT
+ Arm/Exception.asm | MSFT
Arm/Exception.S | GCC
[Sources.AARCH64]
diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
index 84c319c3679b..6d05ed096c4c 100644
--- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
@@ -21,11 +21,11 @@ [Sources.common]
[Sources.ARM]
Arm/ArchPrePeiCore.c
- Arm/PrePeiCoreEntryPoint.asm | RVCT
+ Arm/PrePeiCoreEntryPoint.asm | MSFT
Arm/PrePeiCoreEntryPoint.S | GCC
- Arm/SwitchStack.asm | RVCT
+ Arm/SwitchStack.asm | MSFT
Arm/SwitchStack.S | GCC
- Arm/Exception.asm | RVCT
+ Arm/Exception.asm | MSFT
Arm/Exception.S | GCC
[Sources.AARCH64]
diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
index 9c5da0d42a7b..fd2a35e59591 100644
--- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
@@ -22,7 +22,7 @@ [Sources]
[Sources.ARM]
Arm/ArchPrePi.c
Arm/ModuleEntryPoint.S | GCC
- Arm/ModuleEntryPoint.asm | RVCT
+ Arm/ModuleEntryPoint.asm | MSFT
[Sources.AArch64]
AArch64/ArchPrePi.c
diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
index ee9b05b25337..de3abadfeac6 100644
--- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
@@ -22,7 +22,7 @@ [Sources]
[Sources.ARM]
Arm/ArchPrePi.c
Arm/ModuleEntryPoint.S | GCC
- Arm/ModuleEntryPoint.asm | RVCT
+ Arm/ModuleEntryPoint.asm | MSFT
[Sources.AArch64]
AArch64/ArchPrePi.c
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
index ad68f841fb6b..62b46377116c 100644
--- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
@@ -31,7 +31,7 @@ [Sources]
[Sources.ARM]
IoLibArmVirt.c
Arm/ArmVirtMmio.S | GCC
- Arm/ArmVirtMmio.asm | RVCT
+ Arm/ArmVirtMmio.asm | MSFT
[Sources.AARCH64]
IoLibArmVirt.c
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
index d38e1397eee1..79ba2a2dfc39 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
@@ -85,11 +85,11 @@ [Sources.ARM]
Arm/CompareMem.S | GCC
Arm/CompareGuid.S | GCC
- Arm/ScanMem.asm |RVCT
- Arm/SetMem.asm |RVCT
- Arm/CopyMem.asm |RVCT
- Arm/CompareMem.asm |RVCT
- Arm/CompareGuid.asm |RVCT
+ Arm/ScanMem.asm | MSFT
+ Arm/SetMem.asm | MSFT
+ Arm/CopyMem.asm | MSFT
+ Arm/CompareMem.asm | MSFT
+ Arm/CompareGuid.asm | MSFT
[Sources.AARCH64]
AArch64/ScanMem.S
diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
index 446bc19b63eb..39c503a28a2c 100755
--- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
+++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
@@ -70,7 +70,7 @@ [Sources.EBC]
[Sources.ARM]
Synchronization.c
- Arm/Synchronization.asm | RVCT
+ Arm/Synchronization.asm | MSFT
Arm/Synchronization.S | GCC
[Sources.AARCH64]
--
2.23.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format
[not found] <cover.1568821123.git.baptiste.gerondeau@linaro.org>
@ 2019-09-18 16:05 ` Baptiste Gerondeau
2019-09-19 10:42 ` Baptiste Gerondeau
0 siblings, 1 reply; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-18 16:05 UTC (permalink / raw)
To: devel
Cc: ard.biesheuvel, leif.lindholm, michael.d.kinney, liming.gao,
shenglei.zhang, Baptiste Gerondeau
From: Baptiste GERONDEAU <baptiste.gerondeau@linaro.org>
Add a space between the '|' and the name of the toolchain to use,
as is the case in all other INF files.
Note that I did not touch the RVCT lines, since a following commit in
the set will address those.
Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
---
ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +-
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
index f4fecbb4098a..33dddf1e2b97 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
@@ -22,7 +22,7 @@ [Sources.AARCH64]
[Sources.ARM]
Arm/ArmMmuLibCore.c
- Arm/ArmMmuLibV7Support.S |GCC
+ Arm/ArmMmuLibV7Support.S | GCC
Arm/ArmMmuLibV7Support.asm |RVCT
[Packages]
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
index e4e3d532e7b8..d38e1397eee1 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
@@ -79,11 +79,11 @@ [Defines.ARM, Defines.AARCH64]
LIBRARY_CLASS = BaseMemoryLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
[Sources.ARM]
- Arm/ScanMem.S |GCC
- Arm/SetMem.S |GCC
- Arm/CopyMem.S |GCC
- Arm/CompareMem.S |GCC
- Arm/CompareGuid.S |GCC
+ Arm/ScanMem.S | GCC
+ Arm/SetMem.S | GCC
+ Arm/CopyMem.S | GCC
+ Arm/CompareMem.S | GCC
+ Arm/CompareGuid.S | GCC
Arm/ScanMem.asm |RVCT
Arm/SetMem.asm |RVCT
--
2.23.0
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 0/3] Arm builds on Visual Studio
2019-09-18 12:25 [PATCH 0/3] Arm builds on Visual Studio Baptiste Gerondeau
` (2 preceding siblings ...)
2019-09-18 12:25 ` [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build Baptiste Gerondeau
@ 2019-09-18 16:43 ` Leif Lindholm
2019-09-19 6:19 ` Liming Gao
4 siblings, 0 replies; 33+ messages in thread
From: Leif Lindholm @ 2019-09-18 16:43 UTC (permalink / raw)
To: Baptiste Gerondeau
Cc: devel, ard.biesheuvel, michael.d.kinney, liming.gao,
shenglei.zhang
Thanks Baptiste,
Ard: I would appreciate if you could sanity check the syntax
conversion in 2/3 - it looks correct to me.
>From my point of view, for the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
But we also need a nod from the MdePkg maintainers.
/
Leif
On Wed, Sep 18, 2019 at 06:05:21PM +0200, Baptiste Gerondeau wrote:
> EDIT: Resending the series since I mistakenly used the wrong email,
> sorry !
>
> We are currently making an effort to make ARM (and AARCH64 eventually)
> builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
>
> These 3 patches correspond to an effort to make the assembler work with
> MSFT, which entails :
> - Feeding MSFT the RVCT .asm files, since they share syntax
> requirements.
> - Fixing some instructions syntax in those .asm files, in order to make
> them palatable for MSFT.
> - Fixing some minor formatting issue in INF files, while we're at it.
>
> This set enables the assembler, meanwhile the C also require changes,
> which will come in a set later. This set makes the RVCT toolchain family
> and profiles obsolete, unblocking :
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750
>
> As mentioned in the above bug, dropping RVCT would entail orphanating
> the .asm files that powered the RVCT build. Since Visual Studio uses the
> same file syntax, those can be reused to power the VS build.
>
> These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1)
>
> Baptiste GERONDEAU (3):
> ArmPkg/MdePkg : Unify INF files format
> ARM/Assembler: Correct syntax from RVCT for MSFT
> ARM/Assembler: Reuse RVCT assembler for MSFT build
>
> ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30 +++++++++++++++++-------------
> ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
> ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++--
> ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +-
> ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +-
> ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18 +++++++++---------
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 20 ++++++++++----------
> MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> 21 files changed, 65 insertions(+), 59 deletions(-)
>
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/3] Arm builds on Visual Studio
2019-09-18 12:25 [PATCH 0/3] Arm builds on Visual Studio Baptiste Gerondeau
` (3 preceding siblings ...)
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
4 siblings, 1 reply; 33+ messages in thread
From: Liming Gao @ 2019-09-19 6:19 UTC (permalink / raw)
To: Baptiste Gerondeau, devel@edk2.groups.io
Cc: ard.biesheuvel@linaro.org, leif.lindholm@linaro.org,
Kinney, Michael D, Zhang, Shenglei
I add my comments.
>-----Original Message-----
>From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org]
>Sent: Thursday, September 19, 2019 12:05 AM
>To: devel@edk2.groups.io
>Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D
><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
>Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau
><baptiste.gerondeau@linaro.org>
>Subject: [PATCH 0/3] Arm builds on Visual Studio
>
>EDIT: Resending the series since I mistakenly used the wrong email,
>sorry !
>
>We are currently making an effort to make ARM (and AARCH64 eventually)
>builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
>
>These 3 patches correspond to an effort to make the assembler work with
>MSFT, which entails :
>- Feeding MSFT the RVCT .asm files, since they share syntax
> requirements.
Please separate the patch. Each patch is for each package, can't cross packages.
If so, the package maintainer can easy review the change.
>- Fixing some instructions syntax in those .asm files, in order to make
> them palatable for MSFT.
>- Fixing some minor formatting issue in INF files, while we're at it.
>
>This set enables the assembler, meanwhile the C also require changes,
>which will come in a set later. This set makes the RVCT toolchain family
>and profiles obsolete, unblocking :
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750
With this change, can we continue to work on BZ 1750?
>
>As mentioned in the above bug, dropping RVCT would entail orphanating
>the .asm files that powered the RVCT build. Since Visual Studio uses the
>same file syntax, those can be reused to power the VS build.
>
>These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1)
Do you mean you verify this change with new VS2019 tool chain?
Thanks
Liming
>
>Baptiste GERONDEAU (3):
> ArmPkg/MdePkg : Unify INF files format
> ARM/Assembler: Correct syntax from RVCT for MSFT
> ARM/Assembler: Reuse RVCT assembler for MSFT build
>
> ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30
>+++++++++++++++++-------------
> ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
> ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++--
> ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
>| 2 +-
> ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2
>+-
> ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18
>+++++++++---------
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf |
>20 ++++++++++----------
> MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> 21 files changed, 65 insertions(+), 59 deletions(-)
>
>--
>2.23.0
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format
2019-09-18 12:25 ` [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format Baptiste Gerondeau
@ 2019-09-19 9:29 ` Ard Biesheuvel
0 siblings, 0 replies; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 9:29 UTC (permalink / raw)
To: Baptiste Gerondeau
Cc: edk2-devel-groups-io, Leif Lindholm, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Wed, 18 Sep 2019 at 15:27, Baptiste Gerondeau
<baptiste.gerondeau@linaro.org> wrote:
>
> From: Baptiste GERONDEAU <bgerondeau@gmail.com>
>
> Add a space between the '|' and the name of the toolchain to use,
> as is the case in all other INF files.
Why?
> Note that I did not touch the RVCT lines, since a following commit in
> the set will address those.
>
> Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
> ---
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +-
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index f4fecbb4098a..33dddf1e2b97 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -22,7 +22,7 @@ [Sources.AARCH64]
>
> [Sources.ARM]
> Arm/ArmMmuLibCore.c
> - Arm/ArmMmuLibV7Support.S |GCC
> + Arm/ArmMmuLibV7Support.S | GCC
> Arm/ArmMmuLibV7Support.asm |RVCT
>
> [Packages]
> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> index e4e3d532e7b8..d38e1397eee1 100644
> --- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> @@ -79,11 +79,11 @@ [Defines.ARM, Defines.AARCH64]
> LIBRARY_CLASS = BaseMemoryLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
>
> [Sources.ARM]
> - Arm/ScanMem.S |GCC
> - Arm/SetMem.S |GCC
> - Arm/CopyMem.S |GCC
> - Arm/CompareMem.S |GCC
> - Arm/CompareGuid.S |GCC
> + Arm/ScanMem.S | GCC
> + Arm/SetMem.S | GCC
> + Arm/CopyMem.S | GCC
> + Arm/CompareMem.S | GCC
> + Arm/CompareGuid.S | GCC
>
> Arm/ScanMem.asm |RVCT
> Arm/SetMem.asm |RVCT
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
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
0 siblings, 1 reply; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 9:32 UTC (permalink / raw)
To: Baptiste Gerondeau
Cc: edk2-devel-groups-io, Leif Lindholm, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
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?
>
> 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?
> blx R1
>
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
> index 5a423df16bff..a46d70e41433 100644
> --- a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
> @@ -5,16 +5,16 @@
> ;
>
>
> -AREA IoLibMmio, CODE, READONLY
> + AREA IoLibMmio, CODE, READONLY
>
> -EXPORT MmioRead8Internal
> -EXPORT MmioWrite8Internal
> -EXPORT MmioRead16Internal
> -EXPORT MmioWrite16Internal
> -EXPORT MmioRead32Internal
> -EXPORT MmioWrite32Internal
> -EXPORT MmioRead64Internal
> -EXPORT MmioWrite64Internal
> + EXPORT MmioRead8Internal
> + EXPORT MmioWrite8Internal
> + EXPORT MmioRead16Internal
> + EXPORT MmioWrite16Internal
> + EXPORT MmioRead32Internal
> + EXPORT MmioWrite32Internal
> + EXPORT MmioRead64Internal
> + EXPORT MmioWrite64Internal
>
> ;
> ; Reads an 8-bit MMIO register.
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build
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
0 siblings, 1 reply; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 9:38 UTC (permalink / raw)
To: Baptiste Gerondeau
Cc: edk2-devel-groups-io, Leif Lindholm, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Wed, 18 Sep 2019 at 15:27, Baptiste Gerondeau
<baptiste.gerondeau@linaro.org> wrote:
>
> From: Baptiste GERONDEAU <bgerondeau@gmail.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750"
>
> Since RVCT shares the same assembler syntax as MSFT, use .asm files
> and associate them with MSFT, which would be a first step to addressing
> the above Bugzilla issue.
> RVCT will also have to be erased from BaseTools/rest of the build
> infrastructure, to fully address BZ#1750 ; this patch only addresses the
> "code" in itself.
>
> Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
The changes look fine to me, but please split them out per package as
Liming suggested.
> ---
> ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +-
> ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +-
> ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +-
> ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 10 +++++-----
> MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> 18 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> index 5e23c732bfab..4fccb938eb6d 100644
> --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> @@ -22,7 +22,7 @@ [Sources]
>
> [Sources.ARM]
> GicV3/Arm/ArmGicV3.S | GCC
> - GicV3/Arm/ArmGicV3.asm | RVCT
> + GicV3/Arm/ArmGicV3.asm | MSFT
>
> [Sources.AARCH64]
> GicV3/AArch64/ArmGicV3.S
> diff --git a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> index fdb9c24d21bc..58b2ddbff858 100644
> --- a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> +++ b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> @@ -33,7 +33,7 @@ [Sources.common]
>
> [Sources.Arm]
> Arm/ArmException.c
> - Arm/ExceptionSupport.asm | RVCT
> + Arm/ExceptionSupport.asm | MSFT
> Arm/ExceptionSupport.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf b/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
> index ef1a43a27c45..a404ca2ccf82 100644
> --- a/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
> +++ b/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
> @@ -28,7 +28,7 @@ [Sources.common]
>
> [Sources.Arm]
> Arm/ArmException.c
> - Arm/ExceptionSupport.asm | RVCT
> + Arm/ExceptionSupport.asm | MSFT
> Arm/ExceptionSupport.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> index 69f68f63d7a6..be8d8a228865 100644
> --- a/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> +++ b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> @@ -15,7 +15,7 @@ [Defines]
> LIBRARY_CLASS = ArmHvcLib
>
> [Sources.ARM]
> - Arm/ArmHvc.asm | RVCT
> + Arm/ArmHvc.asm | MSFT
> Arm/ArmHvc.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPkg/Library/ArmLib/ArmBaseLib.inf b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> index 5e70990872f2..63e175623393 100644
> --- a/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +++ b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> @@ -30,10 +30,10 @@ [Sources.ARM]
> Arm/ArmV7Support.S | GCC
> Arm/ArmV7ArchTimerSupport.S | GCC
>
> - Arm/ArmLibSupport.asm | RVCT
> - Arm/ArmLibSupportV7.asm | RVCT
> - Arm/ArmV7Support.asm | RVCT
> - Arm/ArmV7ArchTimerSupport.asm | RVCT
> + Arm/ArmLibSupport.asm | MSFT
> + Arm/ArmLibSupportV7.asm | MSFT
> + Arm/ArmV7Support.asm | MSFT
> + Arm/ArmV7ArchTimerSupport.asm | MSFT
>
> [Sources.AARCH64]
> AArch64/AArch64Lib.h
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index 33dddf1e2b97..44366f02c6d9 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -23,7 +23,7 @@ [Sources.AARCH64]
> [Sources.ARM]
> Arm/ArmMmuLibCore.c
> Arm/ArmMmuLibV7Support.S | GCC
> - Arm/ArmMmuLibV7Support.asm |RVCT
> + Arm/ArmMmuLibV7Support.asm | MSFT
>
> [Packages]
> ArmPkg/ArmPkg.dec
> diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> index 4f4b09f4528a..af8c0e53cc2b 100644
> --- a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> +++ b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> @@ -14,7 +14,7 @@ [Defines]
> LIBRARY_CLASS = ArmSmcLib
>
> [Sources.ARM]
> - Arm/ArmSmc.asm | RVCT
> + Arm/ArmSmc.asm | MSFT
> Arm/ArmSmc.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> index fa19bf649131..f4c9e5510b9a 100644
> --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> @@ -21,7 +21,7 @@ [Sources.AARCH64]
>
> [Sources.ARM]
> Arm/Reset.S | GCC
> - Arm/Reset.asm | RVCT
> + Arm/Reset.asm | MSFT
>
> [Sources]
> ArmSmcPsciResetSystemLib.c
> diff --git a/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf b/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> index 744a29fbf723..6631e40df130 100644
> --- a/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> +++ b/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> @@ -14,7 +14,7 @@ [Defines]
> LIBRARY_CLASS = ArmSvcLib
>
> [Sources.ARM]
> - Arm/ArmSvc.asm | RVCT
> + Arm/ArmSvc.asm | MSFT
> Arm/ArmSvc.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> index e0d0028d8224..cc791a3a68fd 100644
> --- a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> +++ b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> @@ -29,7 +29,7 @@ [Sources.common]
>
> [Sources.Arm]
> Arm/ArmPlatformHelper.S | GCC
> - Arm/ArmPlatformHelper.asm | RVCT
> + Arm/ArmPlatformHelper.asm | MSFT
>
> [Sources.AArch64]
> AArch64/ArmPlatformHelper.S
> diff --git a/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf b/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> index 76f809c80d9f..e88330c1c382 100644
> --- a/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> +++ b/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> @@ -21,7 +21,7 @@ [Packages]
> ArmPlatformPkg/ArmPlatformPkg.dec
>
> [Sources.ARM]
> - Arm/ArmPlatformStackLib.asm | RVCT
> + Arm/ArmPlatformStackLib.asm | MSFT
> Arm/ArmPlatformStackLib.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> index f2ac45d171bc..b663ff749182 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> @@ -21,11 +21,11 @@ [Sources.common]
>
> [Sources.ARM]
> Arm/ArchPrePeiCore.c
> - Arm/PrePeiCoreEntryPoint.asm | RVCT
> + Arm/PrePeiCoreEntryPoint.asm | MSFT
> Arm/PrePeiCoreEntryPoint.S | GCC
> - Arm/SwitchStack.asm | RVCT
> + Arm/SwitchStack.asm | MSFT
> Arm/SwitchStack.S | GCC
> - Arm/Exception.asm | RVCT
> + Arm/Exception.asm | MSFT
> Arm/Exception.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> index 84c319c3679b..6d05ed096c4c 100644
> --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> @@ -21,11 +21,11 @@ [Sources.common]
>
> [Sources.ARM]
> Arm/ArchPrePeiCore.c
> - Arm/PrePeiCoreEntryPoint.asm | RVCT
> + Arm/PrePeiCoreEntryPoint.asm | MSFT
> Arm/PrePeiCoreEntryPoint.S | GCC
> - Arm/SwitchStack.asm | RVCT
> + Arm/SwitchStack.asm | MSFT
> Arm/SwitchStack.S | GCC
> - Arm/Exception.asm | RVCT
> + Arm/Exception.asm | MSFT
> Arm/Exception.S | GCC
>
> [Sources.AARCH64]
> diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> index 9c5da0d42a7b..fd2a35e59591 100644
> --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> @@ -22,7 +22,7 @@ [Sources]
> [Sources.ARM]
> Arm/ArchPrePi.c
> Arm/ModuleEntryPoint.S | GCC
> - Arm/ModuleEntryPoint.asm | RVCT
> + Arm/ModuleEntryPoint.asm | MSFT
>
> [Sources.AArch64]
> AArch64/ArchPrePi.c
> diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> index ee9b05b25337..de3abadfeac6 100644
> --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> @@ -22,7 +22,7 @@ [Sources]
> [Sources.ARM]
> Arm/ArchPrePi.c
> Arm/ModuleEntryPoint.S | GCC
> - Arm/ModuleEntryPoint.asm | RVCT
> + Arm/ModuleEntryPoint.asm | MSFT
>
> [Sources.AArch64]
> AArch64/ArchPrePi.c
> diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
> index ad68f841fb6b..62b46377116c 100644
> --- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
> +++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
> @@ -31,7 +31,7 @@ [Sources]
> [Sources.ARM]
> IoLibArmVirt.c
> Arm/ArmVirtMmio.S | GCC
> - Arm/ArmVirtMmio.asm | RVCT
> + Arm/ArmVirtMmio.asm | MSFT
>
> [Sources.AARCH64]
> IoLibArmVirt.c
> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> index d38e1397eee1..79ba2a2dfc39 100644
> --- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> @@ -85,11 +85,11 @@ [Sources.ARM]
> Arm/CompareMem.S | GCC
> Arm/CompareGuid.S | GCC
>
> - Arm/ScanMem.asm |RVCT
> - Arm/SetMem.asm |RVCT
> - Arm/CopyMem.asm |RVCT
> - Arm/CompareMem.asm |RVCT
> - Arm/CompareGuid.asm |RVCT
> + Arm/ScanMem.asm | MSFT
> + Arm/SetMem.asm | MSFT
> + Arm/CopyMem.asm | MSFT
> + Arm/CompareMem.asm | MSFT
> + Arm/CompareGuid.asm | MSFT
>
> [Sources.AARCH64]
> AArch64/ScanMem.S
> diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> index 446bc19b63eb..39c503a28a2c 100755
> --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> @@ -70,7 +70,7 @@ [Sources.EBC]
>
> [Sources.ARM]
> Synchronization.c
> - Arm/Synchronization.asm | RVCT
> + Arm/Synchronization.asm | MSFT
> Arm/Synchronization.S | GCC
>
> [Sources.AARCH64]
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/3] Arm builds on Visual Studio
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
0 siblings, 2 replies; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 9:44 UTC (permalink / raw)
To: Gao, Liming
Cc: Baptiste Gerondeau, devel@edk2.groups.io,
ard.biesheuvel@linaro.org, Kinney, Michael D, Zhang, Shenglei
Hi Liming,
On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote:
> I add my comments.
>
> >-----Original Message-----
> >From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org]
> >Sent: Thursday, September 19, 2019 12:05 AM
> >To: devel@edk2.groups.io
> >Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D
> ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
> >Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau
> ><baptiste.gerondeau@linaro.org>
> >Subject: [PATCH 0/3] Arm builds on Visual Studio
> >
> >EDIT: Resending the series since I mistakenly used the wrong email,
> >sorry !
> >
> >We are currently making an effort to make ARM (and AARCH64 eventually)
> >builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
> >
> >These 3 patches correspond to an effort to make the assembler work with
> >MSFT, which entails :
> >- Feeding MSFT the RVCT .asm files, since they share syntax
> > requirements.
>
> Please separate the patch. Each patch is for each package, can't cross packages.
> If so, the package maintainer can easy review the change.
I agree with this as a general rule, but for this (hopefully never to
be repeated) operation, it makes sense to me to keep each change in
this set as one patch.
For the simple reason that the alternative leaves several unusable
commits in sequence in the repository. There is simply no way to
bisect through this change on a per-package basis.
This is after all a horrible horrible hack that lets us keep using the
.asm files provided for one toolchain family (RVCT) in a different
toolchain family (MSFT), without having to delete and re-add, losing
history in the process.
Would you be OK with an exception for this extremely unusual
situation?
> >- Fixing some instructions syntax in those .asm files, in order to make
> > them palatable for MSFT.
> >- Fixing some minor formatting issue in INF files, while we're at it.
> >
> >This set enables the assembler, meanwhile the C also require changes,
> >which will come in a set later. This set makes the RVCT toolchain family
> >and profiles obsolete, unblocking :
> >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750
>
> With this change, can we continue to work on BZ 1750?
Yes.
/
Leif
> >As mentioned in the above bug, dropping RVCT would entail orphanating
> >the .asm files that powered the RVCT build. Since Visual Studio uses the
> >same file syntax, those can be reused to power the VS build.
> >
> >These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1)
>
> Do you mean you verify this change with new VS2019 tool chain?
>
> Thanks
> Liming
> >
> >Baptiste GERONDEAU (3):
> > ArmPkg/MdePkg : Unify INF files format
> > ARM/Assembler: Correct syntax from RVCT for MSFT
> > ARM/Assembler: Reuse RVCT assembler for MSFT build
> >
> > ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> > ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30
> >+++++++++++++++++-------------
> > ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> > ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> > ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> > ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
> > ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++--
> > ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> >| 2 +-
> > ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> > ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> > ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2
> >+-
> > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> > ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> > ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> > MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18
> >+++++++++---------
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> > MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf |
> >20 ++++++++++----------
> > MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> > 21 files changed, 65 insertions(+), 59 deletions(-)
> >
> >--
> >2.23.0
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 9:32 ` Ard Biesheuvel
@ 2019-09-19 9:48 ` Leif Lindholm
2019-09-19 10:01 ` Ard Biesheuvel
0 siblings, 1 reply; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 9:48 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
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).
/
Leif
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build
2019-09-19 9:38 ` Ard Biesheuvel
@ 2019-09-19 9:52 ` Leif Lindholm
2019-09-19 9:59 ` Ard Biesheuvel
0 siblings, 1 reply; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 9:52 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, Sep 19, 2019 at 12:38:00PM +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>
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750"
> >
> > Since RVCT shares the same assembler syntax as MSFT, use .asm files
> > and associate them with MSFT, which would be a first step to addressing
> > the above Bugzilla issue.
> > RVCT will also have to be erased from BaseTools/rest of the build
> > infrastructure, to fully address BZ#1750 ; this patch only addresses the
> > "code" in itself.
> >
> > Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
>
> The changes look fine to me, but please split them out per package as
> Liming suggested.
Hmm, and I've just gone and contradicted that.
As I said in my reply to Liming, this is a very special situation, and
the net effect of splitting this patch up is that we end up with a
set of not-usefully-bisectable patches.
/
Leif
> > ---
> > ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> > ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> > ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> > ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> > ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +-
> > ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 2 +-
> > ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> > ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> > ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2 +-
> > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> > ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> > ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> > MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 10 +++++-----
> > MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> > 18 files changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> > index 5e23c732bfab..4fccb938eb6d 100644
> > --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> > +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> > @@ -22,7 +22,7 @@ [Sources]
> >
> > [Sources.ARM]
> > GicV3/Arm/ArmGicV3.S | GCC
> > - GicV3/Arm/ArmGicV3.asm | RVCT
> > + GicV3/Arm/ArmGicV3.asm | MSFT
> >
> > [Sources.AARCH64]
> > GicV3/AArch64/ArmGicV3.S
> > diff --git a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> > index fdb9c24d21bc..58b2ddbff858 100644
> > --- a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> > +++ b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
> > @@ -33,7 +33,7 @@ [Sources.common]
> >
> > [Sources.Arm]
> > Arm/ArmException.c
> > - Arm/ExceptionSupport.asm | RVCT
> > + Arm/ExceptionSupport.asm | MSFT
> > Arm/ExceptionSupport.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf b/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
> > index ef1a43a27c45..a404ca2ccf82 100644
> > --- a/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
> > +++ b/ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf
> > @@ -28,7 +28,7 @@ [Sources.common]
> >
> > [Sources.Arm]
> > Arm/ArmException.c
> > - Arm/ExceptionSupport.asm | RVCT
> > + Arm/ExceptionSupport.asm | MSFT
> > Arm/ExceptionSupport.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> > index 69f68f63d7a6..be8d8a228865 100644
> > --- a/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> > +++ b/ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> > @@ -15,7 +15,7 @@ [Defines]
> > LIBRARY_CLASS = ArmHvcLib
> >
> > [Sources.ARM]
> > - Arm/ArmHvc.asm | RVCT
> > + Arm/ArmHvc.asm | MSFT
> > Arm/ArmHvc.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPkg/Library/ArmLib/ArmBaseLib.inf b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > index 5e70990872f2..63e175623393 100644
> > --- a/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > +++ b/ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > @@ -30,10 +30,10 @@ [Sources.ARM]
> > Arm/ArmV7Support.S | GCC
> > Arm/ArmV7ArchTimerSupport.S | GCC
> >
> > - Arm/ArmLibSupport.asm | RVCT
> > - Arm/ArmLibSupportV7.asm | RVCT
> > - Arm/ArmV7Support.asm | RVCT
> > - Arm/ArmV7ArchTimerSupport.asm | RVCT
> > + Arm/ArmLibSupport.asm | MSFT
> > + Arm/ArmLibSupportV7.asm | MSFT
> > + Arm/ArmV7Support.asm | MSFT
> > + Arm/ArmV7ArchTimerSupport.asm | MSFT
> >
> > [Sources.AARCH64]
> > AArch64/AArch64Lib.h
> > diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > index 33dddf1e2b97..44366f02c6d9 100644
> > --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> > @@ -23,7 +23,7 @@ [Sources.AARCH64]
> > [Sources.ARM]
> > Arm/ArmMmuLibCore.c
> > Arm/ArmMmuLibV7Support.S | GCC
> > - Arm/ArmMmuLibV7Support.asm |RVCT
> > + Arm/ArmMmuLibV7Support.asm | MSFT
> >
> > [Packages]
> > ArmPkg/ArmPkg.dec
> > diff --git a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> > index 4f4b09f4528a..af8c0e53cc2b 100644
> > --- a/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> > +++ b/ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
> > @@ -14,7 +14,7 @@ [Defines]
> > LIBRARY_CLASS = ArmSmcLib
> >
> > [Sources.ARM]
> > - Arm/ArmSmc.asm | RVCT
> > + Arm/ArmSmc.asm | MSFT
> > Arm/ArmSmc.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> > index fa19bf649131..f4c9e5510b9a 100644
> > --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> > @@ -21,7 +21,7 @@ [Sources.AARCH64]
> >
> > [Sources.ARM]
> > Arm/Reset.S | GCC
> > - Arm/Reset.asm | RVCT
> > + Arm/Reset.asm | MSFT
> >
> > [Sources]
> > ArmSmcPsciResetSystemLib.c
> > diff --git a/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf b/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> > index 744a29fbf723..6631e40df130 100644
> > --- a/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> > +++ b/ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> > @@ -14,7 +14,7 @@ [Defines]
> > LIBRARY_CLASS = ArmSvcLib
> >
> > [Sources.ARM]
> > - Arm/ArmSvc.asm | RVCT
> > + Arm/ArmSvc.asm | MSFT
> > Arm/ArmSvc.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> > index e0d0028d8224..cc791a3a68fd 100644
> > --- a/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> > +++ b/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
> > @@ -29,7 +29,7 @@ [Sources.common]
> >
> > [Sources.Arm]
> > Arm/ArmPlatformHelper.S | GCC
> > - Arm/ArmPlatformHelper.asm | RVCT
> > + Arm/ArmPlatformHelper.asm | MSFT
> >
> > [Sources.AArch64]
> > AArch64/ArmPlatformHelper.S
> > diff --git a/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf b/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> > index 76f809c80d9f..e88330c1c382 100644
> > --- a/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> > +++ b/ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
> > @@ -21,7 +21,7 @@ [Packages]
> > ArmPlatformPkg/ArmPlatformPkg.dec
> >
> > [Sources.ARM]
> > - Arm/ArmPlatformStackLib.asm | RVCT
> > + Arm/ArmPlatformStackLib.asm | MSFT
> > Arm/ArmPlatformStackLib.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> > index f2ac45d171bc..b663ff749182 100644
> > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf
> > @@ -21,11 +21,11 @@ [Sources.common]
> >
> > [Sources.ARM]
> > Arm/ArchPrePeiCore.c
> > - Arm/PrePeiCoreEntryPoint.asm | RVCT
> > + Arm/PrePeiCoreEntryPoint.asm | MSFT
> > Arm/PrePeiCoreEntryPoint.S | GCC
> > - Arm/SwitchStack.asm | RVCT
> > + Arm/SwitchStack.asm | MSFT
> > Arm/SwitchStack.S | GCC
> > - Arm/Exception.asm | RVCT
> > + Arm/Exception.asm | MSFT
> > Arm/Exception.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> > index 84c319c3679b..6d05ed096c4c 100644
> > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> > @@ -21,11 +21,11 @@ [Sources.common]
> >
> > [Sources.ARM]
> > Arm/ArchPrePeiCore.c
> > - Arm/PrePeiCoreEntryPoint.asm | RVCT
> > + Arm/PrePeiCoreEntryPoint.asm | MSFT
> > Arm/PrePeiCoreEntryPoint.S | GCC
> > - Arm/SwitchStack.asm | RVCT
> > + Arm/SwitchStack.asm | MSFT
> > Arm/SwitchStack.S | GCC
> > - Arm/Exception.asm | RVCT
> > + Arm/Exception.asm | MSFT
> > Arm/Exception.S | GCC
> >
> > [Sources.AARCH64]
> > diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> > index 9c5da0d42a7b..fd2a35e59591 100644
> > --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
> > +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> > @@ -22,7 +22,7 @@ [Sources]
> > [Sources.ARM]
> > Arm/ArchPrePi.c
> > Arm/ModuleEntryPoint.S | GCC
> > - Arm/ModuleEntryPoint.asm | RVCT
> > + Arm/ModuleEntryPoint.asm | MSFT
> >
> > [Sources.AArch64]
> > AArch64/ArchPrePi.c
> > diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> > index ee9b05b25337..de3abadfeac6 100644
> > --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
> > +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> > @@ -22,7 +22,7 @@ [Sources]
> > [Sources.ARM]
> > Arm/ArchPrePi.c
> > Arm/ModuleEntryPoint.S | GCC
> > - Arm/ModuleEntryPoint.asm | RVCT
> > + Arm/ModuleEntryPoint.asm | MSFT
> >
> > [Sources.AArch64]
> > AArch64/ArchPrePi.c
> > diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
> > index ad68f841fb6b..62b46377116c 100644
> > --- a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
> > +++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
> > @@ -31,7 +31,7 @@ [Sources]
> > [Sources.ARM]
> > IoLibArmVirt.c
> > Arm/ArmVirtMmio.S | GCC
> > - Arm/ArmVirtMmio.asm | RVCT
> > + Arm/ArmVirtMmio.asm | MSFT
> >
> > [Sources.AARCH64]
> > IoLibArmVirt.c
> > diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> > index d38e1397eee1..79ba2a2dfc39 100644
> > --- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> > +++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> > @@ -85,11 +85,11 @@ [Sources.ARM]
> > Arm/CompareMem.S | GCC
> > Arm/CompareGuid.S | GCC
> >
> > - Arm/ScanMem.asm |RVCT
> > - Arm/SetMem.asm |RVCT
> > - Arm/CopyMem.asm |RVCT
> > - Arm/CompareMem.asm |RVCT
> > - Arm/CompareGuid.asm |RVCT
> > + Arm/ScanMem.asm | MSFT
> > + Arm/SetMem.asm | MSFT
> > + Arm/CopyMem.asm | MSFT
> > + Arm/CompareMem.asm | MSFT
> > + Arm/CompareGuid.asm | MSFT
> >
> > [Sources.AARCH64]
> > AArch64/ScanMem.S
> > diff --git a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > index 446bc19b63eb..39c503a28a2c 100755
> > --- a/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > +++ b/MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> > @@ -70,7 +70,7 @@ [Sources.EBC]
> >
> > [Sources.ARM]
> > Synchronization.c
> > - Arm/Synchronization.asm | RVCT
> > + Arm/Synchronization.asm | MSFT
> > Arm/Synchronization.S | GCC
> >
> > [Sources.AARCH64]
> > --
> > 2.23.0
> >
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build
2019-09-19 9:52 ` Leif Lindholm
@ 2019-09-19 9:59 ` Ard Biesheuvel
0 siblings, 0 replies; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 9:59 UTC (permalink / raw)
To: Leif Lindholm
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, 19 Sep 2019 at 12:52, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Sep 19, 2019 at 12:38:00PM +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>
> > >
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750"
> > >
> > > Since RVCT shares the same assembler syntax as MSFT, use .asm files
> > > and associate them with MSFT, which would be a first step to addressing
> > > the above Bugzilla issue.
> > > RVCT will also have to be erased from BaseTools/rest of the build
> > > infrastructure, to fully address BZ#1750 ; this patch only addresses the
> > > "code" in itself.
> > >
> > > Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
> >
> > The changes look fine to me, but please split them out per package as
> > Liming suggested.
>
> Hmm, and I've just gone and contradicted that.
> As I said in my reply to Liming, this is a very special situation, and
> the net effect of splitting this patch up is that we end up with a
> set of not-usefully-bisectable patches.
>
Fair enough. I won't get involved in that discussion, though.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 9:48 ` Leif Lindholm
@ 2019-09-19 10:01 ` Ard Biesheuvel
2019-09-19 10:09 ` Leif Lindholm
0 siblings, 1 reply; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 10:01 UTC (permalink / raw)
To: Leif Lindholm
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
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.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:01 ` Ard Biesheuvel
@ 2019-09-19 10:09 ` Leif Lindholm
2019-09-19 10:25 ` Ard Biesheuvel
0 siblings, 1 reply; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 10:09 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
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
/
Leif
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:09 ` Leif Lindholm
@ 2019-09-19 10:25 ` Ard Biesheuvel
2019-09-19 10:34 ` Baptiste Gerondeau
2019-09-19 10:37 ` Leif Lindholm
0 siblings, 2 replies; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 10:25 UTC (permalink / raw)
To: Leif Lindholm
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
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)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:25 ` Ard Biesheuvel
@ 2019-09-19 10:34 ` Baptiste Gerondeau
2019-09-19 10:37 ` Ard Biesheuvel
2019-09-19 10:37 ` Leif Lindholm
1 sibling, 1 reply; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-19 10:34 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Leif Lindholm, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]
>
>
>
> > > 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.
>
> 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
>
I can confirm that CODE 32 (or CODE32) does not work/does not allow the
barrel shifting to be assembled...
>
> > This one, possible - Baptiste?
> >
>
The unconditional str ? From what I understand, we drop the cmp and drop
the ldr/str of lr_svc and only keep those for lr_usr, right ?
>
>
> 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)
>
ACK ! Will mirror changes to asm on S files (on separate commit, right ?)
I'm only touching the files VS2019 as a problem with on the ARM build
anyways (this is what you meant by "really required" right ?)
--
Baptiste Gerondeau
Engineer - HPC SIG - LDCG - Linaro
#irc : BaptisteGer
[-- Attachment #2: Type: text/html, Size: 2133 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:25 ` Ard Biesheuvel
2019-09-19 10:34 ` Baptiste Gerondeau
@ 2019-09-19 10:37 ` Leif Lindholm
1 sibling, 0 replies; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 10:37 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, Sep 19, 2019 at 01:25:37PM +0300, Ard Biesheuvel wrote:
> On Thu, 19 Sep 2019 at 13:09, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > 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)
That _is_ a good point which I hadn't taken into consideration.
But that does mean we should force all .asm files to ARM, doesn't it?
Or do you mean that you're happy if the *disassembly* of the object
files to produce the same syntax?
/
Leif
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:34 ` Baptiste Gerondeau
@ 2019-09-19 10:37 ` Ard Biesheuvel
2019-09-19 10:47 ` Leif Lindholm
2019-09-19 11:07 ` Baptiste Gerondeau
0 siblings, 2 replies; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 10:37 UTC (permalink / raw)
To: Baptiste Gerondeau
Cc: Leif Lindholm, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, 19 Sep 2019 at 13:34, Baptiste Gerondeau
<baptiste.gerondeau@linaro.org> wrote:
>>
>>
>>
>> > > 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.
>
>
> > 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
> >
>
> I can confirm that CODE 32 (or CODE32) does not work/does not allow the barrel shifting to be assembled...
>
The doc suggests that something like
AREA CODE, ARM
should do the trick?
>>
>>
>> > This one, possible - Baptiste?
>> >
>
>
> The unconditional str ? From what I understand, we drop the cmp and drop the ldr/str of lr_svc and only keep those for lr_usr, right ?
>
>>
>>
>>
>> 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)
>
>
> ACK ! Will mirror changes to asm on S files (on separate commit, right ?)
> I'm only touching the files VS2019 as a problem with on the ARM build anyways (this is what you meant by "really required" right ?)
>
I mean that I'd prefer to assemble the .asm files in ARM mode,
especially since I am not convinced that the startup code we have is
guaranteed to switch into the right mode after the CPU comes out of
reset in ARM mode.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format
2019-09-18 16:05 ` [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format Baptiste Gerondeau
@ 2019-09-19 10:42 ` Baptiste Gerondeau
0 siblings, 0 replies; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-19 10:42 UTC (permalink / raw)
To: edk2-devel-groups-io
Cc: Ard Biesheuvel, Leif Lindholm, Kinney, Michael D, Gao, Liming,
Zhang, Shenglei
[-- Attachment #1: Type: text/plain, Size: 2575 bytes --]
Sorry, for replying on this thread, this is the correct one (messed up the
author's email, sorry again !)
Ard Biescheuvel asks : "Why ?"
The practical reason would be because it breaks an "grep -nr "|
${toolchain}" "
(although with some regex magic I guess it can be circumvented, but one
would need to know in advance that there are places where there aren't
spaces)
The second reason would be because it breaks the standard format used in
all other inf files.
But if you find this is useless, I'll drop this one !
On Wed, 18 Sep 2019 at 18:05, Baptiste Gerondeau <
baptiste.gerondeau@linaro.org> wrote:
> From: Baptiste GERONDEAU <baptiste.gerondeau@linaro.org>
>
> Add a space between the '|' and the name of the toolchain to use,
> as is the case in all other INF files.
> Note that I did not touch the RVCT lines, since a following commit in
> the set will address those.
>
> Signed-off-by: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>
> ---
> ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 2 +-
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index f4fecbb4098a..33dddf1e2b97 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -22,7 +22,7 @@ [Sources.AARCH64]
>
> [Sources.ARM]
> Arm/ArmMmuLibCore.c
> - Arm/ArmMmuLibV7Support.S |GCC
> + Arm/ArmMmuLibV7Support.S | GCC
> Arm/ArmMmuLibV7Support.asm |RVCT
>
> [Packages]
> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> index e4e3d532e7b8..d38e1397eee1 100644
> --- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
> @@ -79,11 +79,11 @@ [Defines.ARM, Defines.AARCH64]
> LIBRARY_CLASS = BaseMemoryLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER
> UEFI_DRIVER UEFI_APPLICATION
>
> [Sources.ARM]
> - Arm/ScanMem.S |GCC
> - Arm/SetMem.S |GCC
> - Arm/CopyMem.S |GCC
> - Arm/CompareMem.S |GCC
> - Arm/CompareGuid.S |GCC
> + Arm/ScanMem.S | GCC
> + Arm/SetMem.S | GCC
> + Arm/CopyMem.S | GCC
> + Arm/CompareMem.S | GCC
> + Arm/CompareGuid.S | GCC
>
> Arm/ScanMem.asm |RVCT
> Arm/SetMem.asm |RVCT
> --
> 2.23.0
>
>
--
Baptiste Gerondeau
Engineer - HPC SIG - LDCG - Linaro
#irc : BaptisteGer
[-- Attachment #2: Type: text/html, Size: 3469 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
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:07 ` Baptiste Gerondeau
1 sibling, 1 reply; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 10:47 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, Sep 19, 2019 at 01:37:40PM +0300, Ard Biesheuvel wrote:
> On Thu, 19 Sep 2019 at 13:34, Baptiste Gerondeau
> <baptiste.gerondeau@linaro.org> wrote:
> >> 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)
> >
> >
> > ACK ! Will mirror changes to asm on S files (on separate commit, right ?)
> > I'm only touching the files VS2019 as a problem with on the ARM
> > build anyways (this is what you meant by "really required" right
> > ?)
>
> I mean that I'd prefer to assemble the .asm files in ARM mode,
> especially since I am not convinced that the startup code we have is
> guaranteed to switch into the right mode after the CPU comes out of
> reset in ARM mode.
That could be resolved with a trivial branch at that point (or just
forcing ARM for the whole entry file) though.
But I'm OK with forcing ARM for all .asm files for now.
/
Leif
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:47 ` Leif Lindholm
@ 2019-09-19 10:53 ` Ard Biesheuvel
2019-09-19 11:25 ` Leif Lindholm
0 siblings, 1 reply; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 10:53 UTC (permalink / raw)
To: Leif Lindholm
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, 19 Sep 2019 at 13:47, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Sep 19, 2019 at 01:37:40PM +0300, Ard Biesheuvel wrote:
> > On Thu, 19 Sep 2019 at 13:34, Baptiste Gerondeau
> > <baptiste.gerondeau@linaro.org> wrote:
> > >> 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)
> > >
> > >
> > > ACK ! Will mirror changes to asm on S files (on separate commit, right ?)
> > > I'm only touching the files VS2019 as a problem with on the ARM
> > > build anyways (this is what you meant by "really required" right
> > > ?)
> >
> > I mean that I'd prefer to assemble the .asm files in ARM mode,
> > especially since I am not convinced that the startup code we have is
> > guaranteed to switch into the right mode after the CPU comes out of
> > reset in ARM mode.
>
> That could be resolved with a trivial branch at that point (or just
> forcing ARM for the whole entry file) though.
>
Of course.
The problem is that the first branch instruction is patched into the
FV files by the BaseTools, and so the startup code is entered in ARM
mode by default.
So that means we'll either have to
1) switch to ARM mode
2) emit one branch instruction
3) switch back to Thumb mode
4) fix up all the code so it assembles in Thumb mode
or
1) switch to ARM mode
> But I'm OK with forcing ARM for all .asm files for now.
>
Indeed.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:37 ` Ard Biesheuvel
2019-09-19 10:47 ` Leif Lindholm
@ 2019-09-19 11:07 ` Baptiste Gerondeau
1 sibling, 0 replies; 33+ messages in thread
From: Baptiste Gerondeau @ 2019-09-19 11:07 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Leif Lindholm, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
[-- Attachment #1: Type: text/plain, Size: 237 bytes --]
> The doc suggests that something like
>
> AREA CODE, ARM
>
> should do the trick?
>
I did try and this does not seem to do anything for the barrel shifting.
--
Baptiste Gerondeau
Engineer - HPC SIG - LDCG - Linaro
#irc : BaptisteGer
[-- Attachment #2: Type: text/html, Size: 622 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 10:53 ` Ard Biesheuvel
@ 2019-09-19 11:25 ` Leif Lindholm
2019-09-19 12:36 ` Ard Biesheuvel
0 siblings, 1 reply; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 11:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, Sep 19, 2019 at 01:53:35PM +0300, Ard Biesheuvel wrote:
> > > I mean that I'd prefer to assemble the .asm files in ARM mode,
> > > especially since I am not convinced that the startup code we have is
> > > guaranteed to switch into the right mode after the CPU comes out of
> > > reset in ARM mode.
> >
> > That could be resolved with a trivial branch at that point (or just
> > forcing ARM for the whole entry file) though.
> >
>
> Of course.
>
> The problem is that the first branch instruction is patched into the
> FV files by the BaseTools, and so the startup code is entered in ARM
> mode by default.
>
> So that means we'll either have to
> 1) switch to ARM mode
> 2) emit one branch instruction
> 3) switch back to Thumb mode
I was thinking more like tying down the entry function (or as I said,
the whole file) as ARM, then letting the toolchain decide for the bits
where we don't have instuction-set dependent ABIs.
> 4) fix up all the code so it assembles in Thumb mode
Which is what Baptiste has done in this set.
> 1) switch to ARM mode
In all 48 files (+3 in edk2-platforms), or just the ones where not
doing so triggers build errors? Currently.
I'm OK with restricting ourselves to just setting ARM in the
triggering files for simplicity, especially in order to streamline the
toolchain migration from RVCT to VS (and the subsequent purge of PVCT
support). I'm not seeing it as a solution.
/
Leif
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 11:25 ` Leif Lindholm
@ 2019-09-19 12:36 ` Ard Biesheuvel
2019-09-19 14:31 ` Leif Lindholm
0 siblings, 1 reply; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 12:36 UTC (permalink / raw)
To: Leif Lindholm
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, 19 Sep 2019 at 14:25, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Sep 19, 2019 at 01:53:35PM +0300, Ard Biesheuvel wrote:
> > > > I mean that I'd prefer to assemble the .asm files in ARM mode,
> > > > especially since I am not convinced that the startup code we have is
> > > > guaranteed to switch into the right mode after the CPU comes out of
> > > > reset in ARM mode.
> > >
> > > That could be resolved with a trivial branch at that point (or just
> > > forcing ARM for the whole entry file) though.
> > >
> >
> > Of course.
> >
> > The problem is that the first branch instruction is patched into the
> > FV files by the BaseTools, and so the startup code is entered in ARM
> > mode by default.
> >
> > So that means we'll either have to
> > 1) switch to ARM mode
> > 2) emit one branch instruction
> > 3) switch back to Thumb mode
>
> I was thinking more like tying down the entry function (or as I said,
> the whole file) as ARM, then letting the toolchain decide for the bits
> where we don't have instuction-set dependent ABIs.
>
> > 4) fix up all the code so it assembles in Thumb mode
>
> Which is what Baptiste has done in this set.
>
> > 1) switch to ARM mode
>
> In all 48 files (+3 in edk2-platforms), or just the ones where not
> doing so triggers build errors? Currently.
>
> I'm OK with restricting ourselves to just setting ARM in the
> triggering files for simplicity, especially in order to streamline the
> toolchain migration from RVCT to VS (and the subsequent purge of PVCT
> support). I'm not seeing it as a solution.
>
As long as we align the .asm files with the .S files, I am fine with
that. And since we focus on v7+ only these days, I'm fine with
changing existing ARM files to Thumb2.
What I don't want is a situation where the toolchain's default decides
how code is assembled, since that means you have to test all your code
changes twice.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 12:36 ` Ard Biesheuvel
@ 2019-09-19 14:31 ` Leif Lindholm
2019-09-19 14:44 ` Ard Biesheuvel
0 siblings, 1 reply; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 14:31 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, Sep 19, 2019 at 03:36:50PM +0300, Ard Biesheuvel wrote:
> On Thu, 19 Sep 2019 at 14:25, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > The problem is that the first branch instruction is patched into the
> > > FV files by the BaseTools, and so the startup code is entered in ARM
> > > mode by default.
> > >
> > > So that means we'll either have to
> > > 1) switch to ARM mode
> > > 2) emit one branch instruction
> > > 3) switch back to Thumb mode
> >
> > I was thinking more like tying down the entry function (or as I said,
> > the whole file) as ARM, then letting the toolchain decide for the bits
> > where we don't have instuction-set dependent ABIs.
> >
> > > 4) fix up all the code so it assembles in Thumb mode
> >
> > Which is what Baptiste has done in this set.
> >
> > > 1) switch to ARM mode
> >
> > In all 48 files (+3 in edk2-platforms), or just the ones where not
> > doing so triggers build errors? Currently.
> >
> > I'm OK with restricting ourselves to just setting ARM in the
> > triggering files for simplicity, especially in order to streamline the
> > toolchain migration from RVCT to VS (and the subsequent purge of PVCT
> > support). I'm not seeing it as a solution.
>
> As long as we align the .asm files with the .S files, I am fine with
> that. And since we focus on v7+ only these days, I'm fine with
> changing existing ARM files to Thumb2.
But I don't see us following this pattern today, for .S files.
Frankly, looking at it, I can't quite manage to untangle how we end up
generating A32 for .asm in the first place. Is it just the toolchain
default?
If we look for example at the post-build version of
Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.iii
, there is no .arm directive.
That gets assembled into
Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.obj
by the step
---
"arm-linux-gnueabihf-gcc" -march=armv7-a -c -x assembler -imacros
/work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/DEBUG/AutoGen.h
-mlittle-endian -o
/work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.obj
-I/work/git/edk2/ArmPkg/Library/ArmHvcLib/Arm
-I/work/git/edk2/ArmPkg/Library/ArmHvcLib
-I/work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/DEBUG
-I/work/git/edk2/MdePkg -I/work/git/edk2/MdePkg/Include
-I/work/git/edk2/MdePkg/Include/Arm -I/work/git/edk2/ArmPkg
-I/work/git/edk2/ArmPkg/Include /work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.iii
---
(Adding hilarity to the situation is the bit where we, when
preprocessing the file, explicitly pass -mthumb on the command line -
which is obviously ignored at that point.)
Fwiw, yes, it appears at least the Debian toolchain defaults to
assembling A32 if no .thumb directive is explicitly in place.
> What I don't want is a situation where the toolchain's default decides
> how code is assembled, since that means you have to test all your code
> changes twice.
And to me it looks like we avoid that problem today only by accident -
we are already relying on toolchain defaults for GCC.
Anyway, rewinding this about 27 steps ...
Are you OK with Baptiste going ahead and adding the ARM statement
*only* to the files requiring it to build[1] - and the one(s) requiring
it to operate properly (could you point it/them out please)?
[1]
Then the next question is whether he should also add it to the files
where that avoids the need for (some) refactoring of code, or whether
he should simply mirror the changes to .asm into .S?
The "we must tie down so that each .asm/.S file is only ever built to
a single instruction set" work is a substantial enough task, that I
hope we can undertake at a later date rather that rolling it into this
one.
/
Leif
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT
2019-09-19 14:31 ` Leif Lindholm
@ 2019-09-19 14:44 ` Ard Biesheuvel
0 siblings, 0 replies; 33+ messages in thread
From: Ard Biesheuvel @ 2019-09-19 14:44 UTC (permalink / raw)
To: Leif Lindholm
Cc: Baptiste Gerondeau, edk2-devel-groups-io, Kinney, Michael D,
Gao, Liming, Zhang, Shenglei, Baptiste GERONDEAU
On Thu, 19 Sep 2019 at 17:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Sep 19, 2019 at 03:36:50PM +0300, Ard Biesheuvel wrote:
> > On Thu, 19 Sep 2019 at 14:25, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > > The problem is that the first branch instruction is patched into the
> > > > FV files by the BaseTools, and so the startup code is entered in ARM
> > > > mode by default.
> > > >
> > > > So that means we'll either have to
> > > > 1) switch to ARM mode
> > > > 2) emit one branch instruction
> > > > 3) switch back to Thumb mode
> > >
> > > I was thinking more like tying down the entry function (or as I said,
> > > the whole file) as ARM, then letting the toolchain decide for the bits
> > > where we don't have instuction-set dependent ABIs.
> > >
> > > > 4) fix up all the code so it assembles in Thumb mode
> > >
> > > Which is what Baptiste has done in this set.
> > >
> > > > 1) switch to ARM mode
> > >
> > > In all 48 files (+3 in edk2-platforms), or just the ones where not
> > > doing so triggers build errors? Currently.
> > >
> > > I'm OK with restricting ourselves to just setting ARM in the
> > > triggering files for simplicity, especially in order to streamline the
> > > toolchain migration from RVCT to VS (and the subsequent purge of PVCT
> > > support). I'm not seeing it as a solution.
> >
> > As long as we align the .asm files with the .S files, I am fine with
> > that. And since we focus on v7+ only these days, I'm fine with
> > changing existing ARM files to Thumb2.
>
> But I don't see us following this pattern today, for .S files.
>
> Frankly, looking at it, I can't quite manage to untangle how we end up
> generating A32 for .asm in the first place. Is it just the toolchain
> default?
>
> If we look for example at the post-build version of
> Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.iii
> , there is no .arm directive.
>
> That gets assembled into
> Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.obj
> by the step
> ---
> "arm-linux-gnueabihf-gcc" -march=armv7-a -c -x assembler -imacros
> /work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/DEBUG/AutoGen.h
> -mlittle-endian -o
> /work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.obj
> -I/work/git/edk2/ArmPkg/Library/ArmHvcLib/Arm
> -I/work/git/edk2/ArmPkg/Library/ArmHvcLib
> -I/work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/DEBUG
> -I/work/git/edk2/MdePkg -I/work/git/edk2/MdePkg/Include
> -I/work/git/edk2/MdePkg/Include/Arm -I/work/git/edk2/ArmPkg
> -I/work/git/edk2/ArmPkg/Include /work/git/tianocore/Build/ArmVirtQemu-ARM/RELEASE_GCC5/ARM/ArmPkg/Library/ArmHvcLib/ArmHvcLib/OUTPUT/Arm/ArmHvc.iii
> ---
> (Adding hilarity to the situation is the bit where we, when
> preprocessing the file, explicitly pass -mthumb on the command line -
> which is obviously ignored at that point.)
>
> Fwiw, yes, it appears at least the Debian toolchain defaults to
> assembling A32 if no .thumb directive is explicitly in place.
>
Yes, that appears to be what happens. Since we use GCC for both
compiling and linker, but invoke the assembler directly (and don't
pass the C flags in this case), we end up with binutils using ARM by
default.
Note that the BaseMemoryLibOptDxe code uses thumb explicitly, but my
main concern is about the startup code we have in PrePi and PrePeiCore
etc
> > What I don't want is a situation where the toolchain's default decides
> > how code is assembled, since that means you have to test all your code
> > changes twice.
>
> And to me it looks like we avoid that problem today only by accident -
> we are already relying on toolchain defaults for GCC.
>
Yup
> Anyway, rewinding this about 27 steps ...
> Are you OK with Baptiste going ahead and adding the ARM statement
> *only* to the files requiring it to build[1] - and the one(s) requiring
> it to operate properly (could you point it/them out please)?
>
The correctness concern applies to files called ModuleEntryPoint.S,
but also to exception handling code that drops in and out of different
modes, and may manipulate exception return addresses etc. If we happen
to always compile that code to ARM today, I strongly suggest we keep
doing that for VS as well.
> [1]
> Then the next question is whether he should also add it to the files
> where that avoids the need for (some) refactoring of code, or whether
> he should simply mirror the changes to .asm into .S?
>
I think all of our 32-bit ARM .S files should have an explicit .arm
directive unless a .thumb directive is already in place.
> The "we must tie down so that each .asm/.S file is only ever built to
> a single instruction set" work is a substantial enough task, that I
> hope we can undertake at a later date rather that rolling it into this
> one.
>
See the above - I think it is fairly straight-forward.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 0/3] Arm builds on Visual Studio
2019-09-19 9:44 ` Leif Lindholm
@ 2019-09-19 14:53 ` Liming Gao
2019-09-19 19:24 ` [edk2-devel] " Laszlo Ersek
1 sibling, 0 replies; 33+ messages in thread
From: Liming Gao @ 2019-09-19 14:53 UTC (permalink / raw)
To: Leif Lindholm, Laszlo Ersek (lersek@redhat.com), afish@apple.com
Cc: Baptiste Gerondeau, devel@edk2.groups.io,
ard.biesheuvel@linaro.org, Kinney, Michael D, Zhang, Shenglei
Leif:
For this special case of the single patch to include the changes in cross packages, I include Laszlo, Fish and Mike for the discussion.
Thanks
Liming
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: Thursday, September 19, 2019 5:45 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: Baptiste Gerondeau <baptiste.gerondeau@linaro.org>; devel@edk2.groups.io; ard.biesheuvel@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>
> Subject: Re: [PATCH 0/3] Arm builds on Visual Studio
>
> Hi Liming,
>
> On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote:
> > I add my comments.
> >
> > >-----Original Message-----
> > >From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org]
> > >Sent: Thursday, September 19, 2019 12:05 AM
> > >To: devel@edk2.groups.io
> > >Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D
> > ><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
> > >Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau
> > ><baptiste.gerondeau@linaro.org>
> > >Subject: [PATCH 0/3] Arm builds on Visual Studio
> > >
> > >EDIT: Resending the series since I mistakenly used the wrong email,
> > >sorry !
> > >
> > >We are currently making an effort to make ARM (and AARCH64 eventually)
> > >builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
> > >
> > >These 3 patches correspond to an effort to make the assembler work with
> > >MSFT, which entails :
> > >- Feeding MSFT the RVCT .asm files, since they share syntax
> > > requirements.
> >
> > Please separate the patch. Each patch is for each package, can't cross packages.
> > If so, the package maintainer can easy review the change.
>
> I agree with this as a general rule, but for this (hopefully never to
> be repeated) operation, it makes sense to me to keep each change in
> this set as one patch.
>
> For the simple reason that the alternative leaves several unusable
> commits in sequence in the repository. There is simply no way to
> bisect through this change on a per-package basis.
>
> This is after all a horrible horrible hack that lets us keep using the
> .asm files provided for one toolchain family (RVCT) in a different
> toolchain family (MSFT), without having to delete and re-add, losing
> history in the process.
>
> Would you be OK with an exception for this extremely unusual
> situation?
>
> > >- Fixing some instructions syntax in those .asm files, in order to make
> > > them palatable for MSFT.
> > >- Fixing some minor formatting issue in INF files, while we're at it.
> > >
> > >This set enables the assembler, meanwhile the C also require changes,
> > >which will come in a set later. This set makes the RVCT toolchain family
> > >and profiles obsolete, unblocking :
> > >BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1750
> >
> > With this change, can we continue to work on BZ 1750?
>
> Yes.
>
> /
> Leif
>
> > >As mentioned in the above bug, dropping RVCT would entail orphanating
> > >the .asm files that powered the RVCT build. Since Visual Studio uses the
> > >same file syntax, those can be reused to power the VS build.
> > >
> > >These patches have been tested on VS2019 (v15.9.11) and VS2017 (v16.0.1)
> >
> > Do you mean you verify this change with new VS2019 tool chain?
> >
> > Thanks
> > Liming
> > >
> > >Baptiste GERONDEAU (3):
> > > ArmPkg/MdePkg : Unify INF files format
> > > ARM/Assembler: Correct syntax from RVCT for MSFT
> > > ARM/Assembler: Reuse RVCT assembler for MSFT build
> > >
> > > ArmPkg/Drivers/ArmGic/ArmGicLib.inf | 2 +-
> > > ArmPkg/Library/ArmExceptionLib/Arm/ExceptionSupport.asm | 30
> > >+++++++++++++++++-------------
> > > ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf | 2 +-
> > > ArmPkg/Library/ArmExceptionLib/ArmRelocateExceptionLib.inf | 2 +-
> > > ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf | 2 +-
> > > ArmPkg/Library/ArmLib/Arm/ArmV7Support.asm | 6 ++++--
> > > ArmPkg/Library/ArmLib/ArmBaseLib.inf | 8 ++++----
> > > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 4 ++--
> > > ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf | 2 +-
> > > ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
> > >| 2 +-
> > > ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf | 2 +-
> > > ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf | 2 +-
> > > ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf | 2
> > >+-
> > > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 6 +++---
> > > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 6 +++---
> > > ArmPlatformPkg/PrePi/PeiMPCore.inf | 2 +-
> > > ArmPlatformPkg/PrePi/PeiUniCore.inf | 2 +-
> > > MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 18
> > >+++++++++---------
> > > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 2 +-
> > > MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf |
> > >20 ++++++++++----------
> > > MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf | 2 +-
> > > 21 files changed, 65 insertions(+), 59 deletions(-)
> > >
> > >--
> > >2.23.0
> >
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio
2019-09-19 9:44 ` Leif Lindholm
2019-09-19 14:53 ` Liming Gao
@ 2019-09-19 19:24 ` Laszlo Ersek
2019-09-19 19:57 ` Andrew Fish
2019-09-19 20:27 ` Leif Lindholm
1 sibling, 2 replies; 33+ messages in thread
From: Laszlo Ersek @ 2019-09-19 19:24 UTC (permalink / raw)
To: devel, leif.lindholm, Gao, Liming
Cc: Baptiste Gerondeau, ard.biesheuvel@linaro.org, Kinney, Michael D,
Zhang, Shenglei
On 09/19/19 11:44, Leif Lindholm wrote:
> Hi Liming,
>
> On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote:
>> I add my comments.
>>
>>> -----Original Message-----
>>> From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org]
>>> Sent: Thursday, September 19, 2019 12:05 AM
>>> To: devel@edk2.groups.io
>>> Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
>>> Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau
>>> <baptiste.gerondeau@linaro.org>
>>> Subject: [PATCH 0/3] Arm builds on Visual Studio
>>>
>>> EDIT: Resending the series since I mistakenly used the wrong email,
>>> sorry !
>>>
>>> We are currently making an effort to make ARM (and AARCH64 eventually)
>>> builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
>>>
>>> These 3 patches correspond to an effort to make the assembler work with
>>> MSFT, which entails :
>>> - Feeding MSFT the RVCT .asm files, since they share syntax
>>> requirements.
>>
>> Please separate the patch. Each patch is for each package, can't cross packages.
>> If so, the package maintainer can easy review the change.
>
> I agree with this as a general rule, but for this (hopefully never to
> be repeated) operation, it makes sense to me to keep each change in
> this set as one patch.
>
> For the simple reason that the alternative leaves several unusable
> commits in sequence in the repository. There is simply no way to
> bisect through this change on a per-package basis.
>
> This is after all a horrible horrible hack that lets us keep using the
> .asm files provided for one toolchain family (RVCT) in a different
> toolchain family (MSFT), without having to delete and re-add, losing
> history in the process.
>
> Would you be OK with an exception for this extremely unusual
> situation?
(The question was posed to Liming, but I'm going to follow up here with
my own thoughts, after getting CC'd by Liming. Thanks for that BTW.)
Let's assume the changes are split up with a fine granularity. Under
that assumption, consider two cases:
(1) ARM builds on Visual Studio are broken until the last patch is
applied, but all other toolchains continue working fine throughout the
series.
versus
(2) ARM builds are broken on Visual Studio *and* on at least one other
-- preexistent -- toolchain, until the last patch in the series is applied.
Which case reflects reality?
If it's case (1), then I prefer the fine-grained patch series structure.
Nothing regresses with existent toolchains (so bisection works with
them), we just have to advertise the last patch in the series as the one
that enables Visual Studio.
If it's case (2), then I agree with the larger (multi-package) patch, as
a rare exception.
(We can also put it like this: if it's *possible* to write this series
such that it enables (1), then we should strive for that.)
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio
2019-09-19 19:24 ` [edk2-devel] " Laszlo Ersek
@ 2019-09-19 19:57 ` Andrew Fish
2019-09-19 20:27 ` Leif Lindholm
1 sibling, 0 replies; 33+ messages in thread
From: Andrew Fish @ 2019-09-19 19:57 UTC (permalink / raw)
To: devel, Laszlo Ersek
Cc: Leif Lindholm, Gao, Liming, Baptiste Gerondeau,
ard.biesheuvel@linaro.org, Mike Kinney, Zhang, Shenglei
[-- Attachment #1: Type: text/plain, Size: 3428 bytes --]
> On Sep 19, 2019, at 12:24 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 09/19/19 11:44, Leif Lindholm wrote:
>> Hi Liming,
>>
>> On Thu, Sep 19, 2019 at 06:19:42AM +0000, Gao, Liming wrote:
>>> I add my comments.
>>>
>>>> -----Original Message-----
>>>> From: Baptiste Gerondeau [mailto:baptiste.gerondeau@linaro.org]
>>>> Sent: Thursday, September 19, 2019 12:05 AM
>>>> To: devel@edk2.groups.io
>>>> Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
>>>> Shenglei <shenglei.zhang@intel.com>; Baptiste Gerondeau
>>>> <baptiste.gerondeau@linaro.org>
>>>> Subject: [PATCH 0/3] Arm builds on Visual Studio
>>>>
>>>> EDIT: Resending the series since I mistakenly used the wrong email,
>>>> sorry !
>>>>
>>>> We are currently making an effort to make ARM (and AARCH64 eventually)
>>>> builds using Microsoft's Visual Studio Compiler (aka MSVC/MSFT).
>>>>
>>>> These 3 patches correspond to an effort to make the assembler work with
>>>> MSFT, which entails :
>>>> - Feeding MSFT the RVCT .asm files, since they share syntax
>>>> requirements.
>>>
>>> Please separate the patch. Each patch is for each package, can't cross packages.
>>> If so, the package maintainer can easy review the change.
>>
>> I agree with this as a general rule, but for this (hopefully never to
>> be repeated) operation, it makes sense to me to keep each change in
>> this set as one patch.
>>
>> For the simple reason that the alternative leaves several unusable
>> commits in sequence in the repository. There is simply no way to
>> bisect through this change on a per-package basis.
>>
>> This is after all a horrible horrible hack that lets us keep using the
>> .asm files provided for one toolchain family (RVCT) in a different
>> toolchain family (MSFT), without having to delete and re-add, losing
>> history in the process.
>>
>> Would you be OK with an exception for this extremely unusual
>> situation?
>
> (The question was posed to Liming, but I'm going to follow up here with
> my own thoughts, after getting CC'd by Liming. Thanks for that BTW.)
>
> Let's assume the changes are split up with a fine granularity. Under
> that assumption, consider two cases:
>
> (1) ARM builds on Visual Studio are broken until the last patch is
> applied, but all other toolchains continue working fine throughout the
> series.
>
> versus
>
> (2) ARM builds are broken on Visual Studio *and* on at least one other
> -- preexistent -- toolchain, until the last patch in the series is applied.
>
>
> Which case reflects reality?
>
> If it's case (1), then I prefer the fine-grained patch series structure.
> Nothing regresses with existent toolchains (so bisection works with
> them), we just have to advertise the last patch in the series as the one
> that enables Visual Studio.
>
> If it's case (2), then I agree with the larger (multi-package) patch, as
> a rare exception.
>
> (We can also put it like this: if it's *possible* to write this series
> such that it enables (1), then we should strive for that.)
>
Laszlo,
I agree with your logic around (1) and (2). I'd also point out we can do the review using (1) and squash into a single commit if we need to take path (2).
Thanks,
Andrew Fish
> Thanks,
> Laszlo
>
>
[-- Attachment #2: Type: text/html, Size: 25570 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio
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
1 sibling, 1 reply; 33+ messages in thread
From: Leif Lindholm @ 2019-09-19 20:27 UTC (permalink / raw)
To: Laszlo Ersek
Cc: devel, Gao, Liming, Baptiste Gerondeau, ard.biesheuvel@linaro.org,
Kinney, Michael D, Zhang, Shenglei
On Thu, Sep 19, 2019 at 09:24:15PM +0200, Laszlo Ersek wrote:
> On 09/19/19 11:44, Leif Lindholm wrote:
> > I agree with this as a general rule, but for this (hopefully never to
> > be repeated) operation, it makes sense to me to keep each change in
> > this set as one patch.
> >
> > For the simple reason that the alternative leaves several unusable
> > commits in sequence in the repository. There is simply no way to
> > bisect through this change on a per-package basis.
> >
> > This is after all a horrible horrible hack that lets us keep using the
> > .asm files provided for one toolchain family (RVCT) in a different
> > toolchain family (MSFT), without having to delete and re-add, losing
> > history in the process.
> >
> > Would you be OK with an exception for this extremely unusual
> > situation?
>
> (The question was posed to Liming, but I'm going to follow up here with
> my own thoughts, after getting CC'd by Liming. Thanks for that BTW.)
Yes, I should have thought of that myself - Baptiste did exactly what
I asked him to.
> Let's assume the changes are split up with a fine granularity. Under
> that assumption, consider two cases:
>
> (1) ARM builds on Visual Studio are broken until the last patch is
> applied, but all other toolchains continue working fine throughout the
> series.
>
> versus
>
> (2) ARM builds are broken on Visual Studio *and* on at least one other
> -- preexistent -- toolchain, until the last patch in the series is applied.
>
>
> Which case reflects reality?
>
> If it's case (1), then I prefer the fine-grained patch series structure.
> Nothing regresses with existent toolchains (so bisection works with
> them), we just have to advertise the last patch in the series as the one
> that enables Visual Studio.
>
> If it's case (2), then I agree with the larger (multi-package) patch, as
> a rare exception.
>
> (We can also put it like this: if it's *possible* to write this series
> such that it enables (1), then we should strive for that.)
I agree with your logic.
It's just that I'm not even aware of anyone who has *tried* building
with RVCT for several years. So we leave the "probably not working"
toolchain potentially working for a few more commits, as a tradeoff
against enabling the new one in one go.
(Now, fair, there are other issues with the VS ARM port that as per
the cover letter will also need to be addressed before the port is
fully functional.)
/
Leif
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio
2019-09-19 20:27 ` Leif Lindholm
@ 2019-09-24 1:28 ` Liming Gao
0 siblings, 0 replies; 33+ messages in thread
From: Liming Gao @ 2019-09-24 1:28 UTC (permalink / raw)
To: devel@edk2.groups.io, leif.lindholm@linaro.org, Laszlo Ersek
Cc: Baptiste Gerondeau, ard.biesheuvel@linaro.org, Kinney, Michael D,
Zhang, Shenglei
Leif:
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Leif
>Lindholm
>Sent: Friday, September 20, 2019 4:27 AM
>To: Laszlo Ersek <lersek@redhat.com>
>Cc: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Baptiste
>Gerondeau <baptiste.gerondeau@linaro.org>; ard.biesheuvel@linaro.org;
>Kinney, Michael D <michael.d.kinney@intel.com>; Zhang, Shenglei
><shenglei.zhang@intel.com>
>Subject: Re: [edk2-devel] [PATCH 0/3] Arm builds on Visual Studio
>
>On Thu, Sep 19, 2019 at 09:24:15PM +0200, Laszlo Ersek wrote:
>> On 09/19/19 11:44, Leif Lindholm wrote:
>> > I agree with this as a general rule, but for this (hopefully never to
>> > be repeated) operation, it makes sense to me to keep each change in
>> > this set as one patch.
>> >
>> > For the simple reason that the alternative leaves several unusable
>> > commits in sequence in the repository. There is simply no way to
>> > bisect through this change on a per-package basis.
>> >
>> > This is after all a horrible horrible hack that lets us keep using the
>> > .asm files provided for one toolchain family (RVCT) in a different
>> > toolchain family (MSFT), without having to delete and re-add, losing
>> > history in the process.
>> >
>> > Would you be OK with an exception for this extremely unusual
>> > situation?
>>
>> (The question was posed to Liming, but I'm going to follow up here with
>> my own thoughts, after getting CC'd by Liming. Thanks for that BTW.)
>
>Yes, I should have thought of that myself - Baptiste did exactly what
>I asked him to.
>
>> Let's assume the changes are split up with a fine granularity. Under
>> that assumption, consider two cases:
>>
>> (1) ARM builds on Visual Studio are broken until the last patch is
>> applied, but all other toolchains continue working fine throughout the
>> series.
>>
>> versus
>>
>> (2) ARM builds are broken on Visual Studio *and* on at least one other
>> -- preexistent -- toolchain, until the last patch in the series is applied.
>>
>>
>> Which case reflects reality?
>>
>> If it's case (1), then I prefer the fine-grained patch series structure.
>> Nothing regresses with existent toolchains (so bisection works with
>> them), we just have to advertise the last patch in the series as the one
>> that enables Visual Studio.
>>
>> If it's case (2), then I agree with the larger (multi-package) patch, as
>> a rare exception.
>>
>> (We can also put it like this: if it's *possible* to write this series
>> such that it enables (1), then we should strive for that.)
>
>I agree with your logic.
>It's just that I'm not even aware of anyone who has *tried* building
>with RVCT for several years. So we leave the "probably not working"
>toolchain potentially working for a few more commits, as a tradeoff
>against enabling the new one in one go.
>
>(Now, fair, there are other issues with the VS ARM port that as per
>the cover letter will also need to be addressed before the port is
>fully functional.)
>
If RVCT doesn't work, this change will follow into case (1). Right?
Thanks
Liming
>/
> Leif
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2019-09-24 1:28 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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 1/3] ArmPkg/MdePkg : Unify INF files format Baptiste Gerondeau
2019-09-19 10:42 ` Baptiste Gerondeau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox