Date   

Re: [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format

Baptiste Gerondeau
 

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@...> wrote:
From: Baptiste GERONDEAU <baptiste.gerondeau@...>

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@...>
---
 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


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Ard Biesheuvel
 

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.


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Leif Lindholm
 

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


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Baptiste Gerondeau
 



> > 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


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Ard Biesheuvel
 

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)


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Leif Lindholm
 

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


Re: [PATCH 0/9] Various line ending and encoding fixes

Ard Biesheuvel
 

On Thu, 19 Sep 2019 at 01:43, Leif Lindholm <leif.lindholm@linaro.org> wrote:

I have started looking into doing the CRLF->native conversion for EDK2,
and as part of my initial scan, I found a bunch of trivial issues that
would be easier to just fix beforehand.

Leif Lindholm (9):
BaseTools: add missing newlines at end of files
EmbeddedPkg: add missing newline at end of TemplateResetSystemLib.inf
NetworkPkg: add missing newline at end of file
EmbeddedPkg: delete outdated FdtLib README.txt
BaseTools: fix line endings in SetupGit.py Conf files
DynamicTablesPkg: fix .dsc line ending
ArmPkg: ArmScmiDxe - convert .h to UTF-8 from 8859-x
BaseTools: correct line endings for ConvertFce Python script
EmbeddedPkg: convert Lauterbach README.txt to UTF-8
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

ArmPkg/Drivers/ArmScmiDxe/ArmScmiPerformanceProtocolPrivate.h | 2 +-
BaseTools/Conf/diff.order | 2 +-
BaseTools/Conf/gitattributes | 2 +-
BaseTools/Scripts/ConvertFceToStructurePcd.py | 10 +++++-----
BaseTools/Source/Python/AutoGen/DataPipe.py | 2 +-
BaseTools/Source/Python/Common/DataType.py | 2 +-
BaseTools/Source/Python/Common/GlobalData.py | 2 +-
DynamicTablesPkg/DynamicTablesPkg.dsc | 2 +-
EmbeddedPkg/Library/FdtLib/README.txt | 1 -
EmbeddedPkg/Library/TemplateResetSystemLib/TemplateResetSystemLib.inf | 2 +-
EmbeddedPkg/Scripts/LauterbachT32/README.txt | 4 ++--
NetworkPkg/WifiConnectionManagerDxe/WifiConnectionManagerDxeStrings.uni | 2 +-
12 files changed, 16 insertions(+), 17 deletions(-)
delete mode 100644 EmbeddedPkg/Library/FdtLib/README.txt

Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
--
2.20.1


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Ard Biesheuvel
 

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.


Re: [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build

Ard Biesheuvel
 

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.


Re: [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build

Leif Lindholm
 

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


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Leif Lindholm
 

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


Re: [PATCH 0/3] Arm builds on Visual Studio

Leif Lindholm
 

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


Re: [PATCH 3/3] ARM/Assembler: Reuse RVCT assembler for MSFT build

Ard Biesheuvel
 

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


Re: [PATCH 2/3] ARM/Assembler: Correct syntax from RVCT for MSFT

Ard Biesheuvel
 

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


Re: [PATCH 1/3] ArmPkg/MdePkg : Unify INF files format

Ard Biesheuvel
 

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


Re: [edk2-platforms PATCH v5] MinPlatformPkg/TestPointCheckLib: Add check for pointers

Chiu, Chasel
 

Please update commit message as the latest change does not return EFI_NOT_FOUND in DxeCheckBootVariable.c.
With above updated, Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@intel.com>
Sent: Thursday, September 19, 2019 4:08 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
<chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2-platforms PATCH v5] MinPlatformPkg/TestPointCheckLib: Add
check for pointers

In DxeCheckBootVariable.c, add check for BootOrder and Variable that
return EFI_NOT_FOUND when they are NULL.
In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL when
allocating memory to what it points to.

Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---

v2: Update copyright

v3: Fix the coding style.

v4: Update the logic in DxeCheckBootVariable.c.
We directly return when BootOrder/Variable == NULL, in previous versions.

v5: Update the format of copyright and the coding style.

.../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 6 +++---
.../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 8 +++++---
2 files changed, 8 insertions(+), 6 deletions(-)

diff --git
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB
ootVariable.c
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB
ootVariable.c
index 85bd5b3d..5437cf6a 100644
---
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckB
ootVariable.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckBootVariable.c
@@ -1,6 +1,6 @@
/** @file

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -130,7 +130,7 @@ TestPointCheckLoadOptionVariable (
for (ListIndex = 0; ListIndex <
sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]);
ListIndex++) {
UnicodeSPrint (BootOrderName, sizeof(BootOrderName), L"%sOrder",
mLoadOptionVariableList[ListIndex]);
Status = GetVariable2 (BootOrderName, &gEfiGlobalVariableGuid,
(VOID **)&BootOrder, &OrderSize);
- if (EFI_ERROR(Status)) {
+ if (EFI_ERROR(Status) || (BootOrder == NULL)) {
continue;
}
for (Index = 0; Index < OrderSize/sizeof(CHAR16); Index++) { @@ -222,7
+222,7 @@ TestPointCheckKeyOptionVariable (
for (Index = 0; ; Index++) {
UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x",
mKeyOptionVariableList[ListIndex], Index);
Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid,
&Variable, &Size);
- if (!EFI_ERROR(Status)) {
+ if (!EFI_ERROR(Status) && (Variable != NULL)) {
DumpKeyOption (KeyOptionName, Variable, Size);
} else {
break;
diff --git
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG
cd.c
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG
cd.c
index 82709d44..066121c7 100644
---
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckG
cd.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckGcd.c
@@ -1,6 +1,6 @@
/** @file

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -241,11 +241,13 @@ TestPointDumpGcd (
}
}
if (GcdMemoryMap != NULL) {
- *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ if (GcdIoMap != NULL) {
+ *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ }
*GcdIoMapNumberOfDescriptors = NumberOfDescriptors;
}
}
-
+
if (DumpPrint) {
DEBUG ((DEBUG_INFO, "==== TestPointDumpGcd - Exit\n"));
}
--
2.18.0.windows.1


Re: [edk2-platforms Patch 0/2] Convert UNI files from UTF-16 to UTF-8

Ard Biesheuvel
 

On Thu, 19 Sep 2019 at 03:23, Michael D Kinney
<michael.d.kinney@intel.com> wrote:

https://bugzilla.tianocore.org/show_bug.cgi?id=2199

Convert UNI files in Hisilicon and Marvel packages from UTF-16 to UTF-8.

There are no file content changes in these patches.

The patch files show these as binary files due to the UTF-16 encoding. Future
updates to these files in UTF-8 format will show the changes as text files.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Michael D Kinney (2):
Hisilicon: Convert UNI files from UTF-16 to UTF-8
Marvel: Convert UNI files from UTF-16 to UTF-8

.../BoardFeature2PHi1610Strings.uni | Bin 3538 -> 1780 bytes
.../Type09/MiscSystemSlotDesignation.uni | Bin 826 -> 412 bytes
.../Applications/EepromCmd/EepromCmd.uni | Bin 3558 -> 1778 bytes
.../Applications/FirmwareUpdate/FUpdate.uni | Bin 3500 -> 1749 bytes
.../Applications/SpiTool/SpiFlashCmd.uni | Bin 3958 -> 1978 bytes
5 files changed, 0 insertions(+), 0 deletions(-)

--
2.21.0.windows.1




Re: [PATCH] Edk2: Add Zhichao as the maintainer for shellbin relase

Leif Lindholm
 

I have no objection to this addition, but I do have two questions - and
really I realise this is a follow-on for 81a8a52a6bb21 ("ShellBinPkg:
Remove ShellBinPkg"):

1) Since ShellBinPkg no longer exists, what is the intent of listing
this in the Maintainers.txt file? Generally, this is so that people
know who to Cc: on patches - but there will not be any patches here.
2) If Zhichao is now responsible for this, should Jaben and Ray still
be listed?

Best Regards,

Leif

On Thu, Sep 19, 2019 at 05:58:16AM +0000, Gao, Liming wrote:
Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Gao, Zhichao
Sent: Thursday, September 19, 2019 1:46 PM
To: devel@edk2.groups.io
Cc: Andrew Fish <afish@apple.com>; Laszlo Ersek <lersek@redhat.com>; Leif
Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D
<michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel
<ard.biesheuvel@linaro.org>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] Edk2: Add Zhichao as the maintainer for shellbin relase

Zhichao is responsible for the IA32 and X64 ARCH shell binary generation.

Cc: Andrew Fish <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
Maintainers.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index dcf81c737a..a0121123ec 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -426,6 +426,7 @@ Maintainers for stable Shell binaries generation
when need to publish Shell binaries with edk2 release:
M: Jaben Carsey <jaben.carsey@intel.com> (Ia32/X64)
M: Ray Ni <ray.ni@intel.com> (Ia32/X64)
+M: Zhichao Gao <zhichao.gao@intel.com> (Ia32/X64)
M: Leif Lindholm <leif.lindholm@linaro.org> (ARM/AArch64)
M: Ard Biesheuvel <ard.biesheuvel@linaro.org> (ARM/AArch64)

--
2.21.0.windows.1


Re: [PATCH V2] BaseTools:Fix the issue that build report failed

Bob Feng
 

Reviewed-by: Bob Feng <bob.c.feng@intel.com>

-----Original Message-----
From: Fan, ZhijuX
Sent: Thursday, September 19, 2019 5:04 PM
To: devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>; Feng, Bob C <bob.c.feng@intel.com>
Subject: [PATCH V2] BaseTools:Fix the issue that build report failed

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2201

An error occurs using special VpdPcd that is not used in the Inf file

In dsc:
[PcdsDynamicExVpd.common.DEFAULT]
gBoardModuleTokenSpaceGuid.test1|*|{CODE({
{0x0} // terminator
})}

In dec:
[PcdsDynamicEx]
# Vpd GPIO table
gBoardModuleTokenSpaceGuid.test1|{0}|GPIO_INIT_CONFIG[]|0x50000018 {
<HeaderFiles>
Library/GpioLib.h
<Packages>
MdePkg/MdePkg.dec
}
ValueError: invalid literal for int() with base 0: '*'

This Patch is going to fix issue

Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
---
Old title:Change the way that get some VpdPcd information

BaseTools/Source/Python/build/BuildReport.py | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/build/BuildReport.py b/BaseTools/Source/Python/build/BuildReport.py
index 6b26f1c3b0..880459d367 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1419,10 +1419,19 @@ class PcdReport(object):
FileWrite(File, '%*s' % (self.MaxLen + 4, SkuInfo.VpdOffset))
VPDPcdItem = (Pcd.TokenSpaceGuidCName + '.' + PcdTokenCName, SkuIdName, SkuInfo.VpdOffset, Pcd.MaxDatumSize, SkuInfo.DefaultValue)
if VPDPcdItem not in VPDPcdList:
- VPDPcdList.append(VPDPcdItem)
+ PcdGuidList = self.UnusedPcds.get(Pcd.TokenSpaceGuidCName)
+ if PcdGuidList:
+ PcdList = PcdGuidList.get(Pcd.Type)
+ if not PcdList:
+ VPDPcdList.append(VPDPcdItem)
+ for VpdPcd in PcdList:
+ if PcdTokenCName == VpdPcd.TokenCName:
+ break
+ else:
+ VPDPcdList.append(VPDPcdItem)
if IsStructure:
FiledOverrideFlag = False
- OverrideValues = Pcd.SkuOverrideValues[Sku]
+ OverrideValues = Pcd.SkuOverrideValues.get(Sku)
if OverrideValues:
Keys = list(OverrideValues.keys())
OverrideFieldStruct = self.OverrideFieldValue(Pcd, OverrideValues[Keys[0]])
--
2.14.1.windows.1


[PATCH V2] BaseTools:Fix the issue that build report failed

Fan, ZhijuX
 

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2201

An error occurs using special VpdPcd that is not used in the Inf file

In dsc:
[PcdsDynamicExVpd.common.DEFAULT]
gBoardModuleTokenSpaceGuid.test1|*|{CODE({
{0x0} // terminator
})}

In dec:
[PcdsDynamicEx]
# Vpd GPIO table
gBoardModuleTokenSpaceGuid.test1|{0}|GPIO_INIT_CONFIG[]|0x50000018 {
<HeaderFiles>
Library/GpioLib.h
<Packages>
MdePkg/MdePkg.dec
}
ValueError: invalid literal for int() with base 0: '*'

This Patch is going to fix issue

Cc: Liming Gao <liming.gao@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Signed-off-by: Zhiju.Fan <zhijux.fan@intel.com>
---
Old title:Change the way that get some VpdPcd information

BaseTools/Source/Python/build/BuildReport.py | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/build/BuildReport.py b/BaseTools/Source/Python/build/BuildReport.py
index 6b26f1c3b0..880459d367 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1419,10 +1419,19 @@ class PcdReport(object):
FileWrite(File, '%*s' % (self.MaxLen + 4, SkuInfo.VpdOffset))
VPDPcdItem = (Pcd.TokenSpaceGuidCName + '.' + PcdTokenCName, SkuIdName, SkuInfo.VpdOffset, Pcd.MaxDatumSize, SkuInfo.DefaultValue)
if VPDPcdItem not in VPDPcdList:
- VPDPcdList.append(VPDPcdItem)
+ PcdGuidList = self.UnusedPcds.get(Pcd.TokenSpaceGuidCName)
+ if PcdGuidList:
+ PcdList = PcdGuidList.get(Pcd.Type)
+ if not PcdList:
+ VPDPcdList.append(VPDPcdItem)
+ for VpdPcd in PcdList:
+ if PcdTokenCName == VpdPcd.TokenCName:
+ break
+ else:
+ VPDPcdList.append(VPDPcdItem)
if IsStructure:
FiledOverrideFlag = False
- OverrideValues = Pcd.SkuOverrideValues[Sku]
+ OverrideValues = Pcd.SkuOverrideValues.get(Sku)
if OverrideValues:
Keys = list(OverrideValues.keys())
OverrideFieldStruct = self.OverrideFieldValue(Pcd, OverrideValues[Keys[0]])
--
2.14.1.windows.1