Re: [edk2-devel][PATCH] UefiCpuPkg: Support FFS3 GUID in SearchForBfvBase.asm

2022-03-10 Thread Kuo, Ted
Hi Ray,

Please find my inline comments with [Ted].

Thanks,
Ted

-Original Message-
From: Ni, Ray  
Sent: Thursday, March 10, 2022 12:15 PM
To: Kuo, Ted ; devel@edk2.groups.io
Cc: De, Debkumar ; Han, Harry ; 
West, Catharine 
Subject: RE: [edk2-devel][PATCH] UefiCpuPkg: Support FFS3 GUID in 
SearchForBfvBase.asm

3 comments starting with "[Ray]".

 ;#define EFI_FIRMWARE_FILE_SYSTEM2_GUID \  ;  { 0x8c8ce578, 0x8a3d, 0x4f1c, { 
0x99, 0x35, 0x89, 0x61, 0x85, 0xc3, 0x2d, 0xd3 } } -%define FFS_GUID_DWORD0 
0x8c8ce578 -%define FFS_GUID_DWORD1 0x4f1c8a3d -%define FFS_GUID_DWORD2 
0x61893599 -%define FFS_GUID_DWORD3 0xd32dc385
+%define FFS2_GUID_DWORD0 0x8c8ce578
+%define FFS2_GUID_DWORD1 0x4f1c8a3d
+%define FFS2_GUID_DWORD2 0x61893599
+%define FFS2_GUID_DWORD3 0xd32dc385
+
+;#define EFI_FIRMWARE_FILE_SYSTEM3_GUID \ ;  { 0x8c8ce578, 0x3dcb, 
+0x4dca, { 0xbd, 0x6f, 0x1e, 0x96, 0x89, 0xe7, 0x34, 0x9a } } %define 
+FFS3_GUID_DWORD0 0x5473c07a %define FFS3_GUID_DWORD1 0x4dca3dcb %define 
+FFS3_GUID_DWORD2 0x961e6fbd %define FFS3_GUID_DWORD3 0x9a34e789

[Ray] 1. I am not sure if it's the best representation of GUID in NASM? I am 
not the NASM expert. If there is no recommendation, I am ok with this.
[Ted] Somehow the diff doesn't reflect the actual representation in my editor. 
I'll check and update the patch.
 
 BITS32
 
@@ -25,6 +32,7 @@ BITS32
 Flat32SearchForBfvBase:
 
 xor eax, eax
+mov ecx, 2 ; 2: FFS3 GUID, 1: FFS2 GUID, 0: Not Found

[Ray] 2. Can you map ECX 2 to FFS2 GUID, ECX 3 to FFS3 GUID?
[Ted] Yes, will update in next patch.

 searchingForBfvHeaderLoop:
 ;
 ; We check for a firmware volume at every 4KB address in the top 16MB @@ 
-32,20 +40,37 @@ searchingForBfvHeaderLoop:
 ;
 sub eax, 0x1000
 cmp eax, 0xff00
-jb  searchedForBfvHeaderButNotFound
+jb  searchingForBfvWithOtherFfsGuid
+cmp ecx, 2
+jne searchingForFfs2Guid
 
 ;
-; Check FFS GUID
+; Check FFS3 GUID
 ;
-cmp dword [eax + 0x10], FFS_GUID_DWORD0
+cmp dword [eax + 0x10], FFS3_GUID_DWORD0
 jne searchingForBfvHeaderLoop
-cmp dword [eax + 0x14], FFS_GUID_DWORD1
+cmp dword [eax + 0x14], FFS3_GUID_DWORD1
 jne searchingForBfvHeaderLoop
-cmp dword [eax + 0x18], FFS_GUID_DWORD2
+cmp dword [eax + 0x18], FFS3_GUID_DWORD2
 jne searchingForBfvHeaderLoop
-cmp dword [eax + 0x1c], FFS_GUID_DWORD3
+cmp dword [eax + 0x1c], FFS3_GUID_DWORD3
 jne searchingForBfvHeaderLoop
+jmp checkingFvLength
 
+searchingForFfs2Guid:
+;
+; Check FFS2 GUID
+;
+cmp dword [eax + 0x10], FFS2_GUID_DWORD0
+jne searchingForBfvHeaderLoop
+cmp dword [eax + 0x14], FFS2_GUID_DWORD1
+jne searchingForBfvHeaderLoop
+cmp dword [eax + 0x18], FFS2_GUID_DWORD2
+jne searchingForBfvHeaderLoop
+cmp dword [eax + 0x1c], FFS2_GUID_DWORD3
+jne searchingForBfvHeaderLoop
+
+checkingFvLength:

[Ray] 3. Why is this label added?
[Ted] You meant searchingForBfvWithOtherFfsGuid? This is used to restart a new 
search with a new target guid before reaching the end of the supported ffs guid 
list.

 ;
 ; Check FV Length
 ;
@@ -57,6 +82,12 @@ searchingForBfvHeaderLoop:
 
 jmp searchedForBfvHeaderAndItWasFound
 
+searchingForBfvWithOtherFfsGuid:
+xor eax, eax
+dec ecx
+cmp ecx, 0
+jne searchingForBfvHeaderLoop
+
 searchedForBfvHeaderButNotFound:
 ;
 ; Hang if the SEC entry point was not found
--
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87405): https://edk2.groups.io/g/devel/message/87405
Mute This Topic: https://groups.io/mt/89613429/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel][PATCH] UefiCpuPkg: Support FFS3 GUID in SearchForBfvBase.asm

2022-03-09 Thread Ni, Ray
3 comments starting with "[Ray]".

 ;#define EFI_FIRMWARE_FILE_SYSTEM2_GUID \
 ;  { 0x8c8ce578, 0x8a3d, 0x4f1c, { 0x99, 0x35, 0x89, 0x61, 0x85, 0xc3, 0x2d, 
0xd3 } }
-%define FFS_GUID_DWORD0 0x8c8ce578
-%define FFS_GUID_DWORD1 0x4f1c8a3d
-%define FFS_GUID_DWORD2 0x61893599
-%define FFS_GUID_DWORD3 0xd32dc385
+%define FFS2_GUID_DWORD0 0x8c8ce578
+%define FFS2_GUID_DWORD1 0x4f1c8a3d
+%define FFS2_GUID_DWORD2 0x61893599
+%define FFS2_GUID_DWORD3 0xd32dc385
+
+;#define EFI_FIRMWARE_FILE_SYSTEM3_GUID \
+;  { 0x8c8ce578, 0x3dcb, 0x4dca, { 0xbd, 0x6f, 0x1e, 0x96, 0x89, 0xe7, 0x34, 
0x9a } }
+%define FFS3_GUID_DWORD0 0x5473c07a
+%define FFS3_GUID_DWORD1 0x4dca3dcb
+%define FFS3_GUID_DWORD2 0x961e6fbd
+%define FFS3_GUID_DWORD3 0x9a34e789

[Ray] 1. I am not sure if it's the best representation of GUID in NASM? I am 
not the NASM expert. If there is no recommendation, I am ok with this.

 
 BITS32
 
@@ -25,6 +32,7 @@ BITS32
 Flat32SearchForBfvBase:
 
 xor eax, eax
+mov ecx, 2 ; 2: FFS3 GUID, 1: FFS2 GUID, 0: Not Found

[Ray] 2. Can you map ECX 2 to FFS2 GUID, ECX 3 to FFS3 GUID?

 searchingForBfvHeaderLoop:
 ;
 ; We check for a firmware volume at every 4KB address in the top 16MB
@@ -32,20 +40,37 @@ searchingForBfvHeaderLoop:
 ;
 sub eax, 0x1000
 cmp eax, 0xff00
-jb  searchedForBfvHeaderButNotFound
+jb  searchingForBfvWithOtherFfsGuid
+cmp ecx, 2
+jne searchingForFfs2Guid
 
 ;
-; Check FFS GUID
+; Check FFS3 GUID
 ;
-cmp dword [eax + 0x10], FFS_GUID_DWORD0
+cmp dword [eax + 0x10], FFS3_GUID_DWORD0
 jne searchingForBfvHeaderLoop
-cmp dword [eax + 0x14], FFS_GUID_DWORD1
+cmp dword [eax + 0x14], FFS3_GUID_DWORD1
 jne searchingForBfvHeaderLoop
-cmp dword [eax + 0x18], FFS_GUID_DWORD2
+cmp dword [eax + 0x18], FFS3_GUID_DWORD2
 jne searchingForBfvHeaderLoop
-cmp dword [eax + 0x1c], FFS_GUID_DWORD3
+cmp dword [eax + 0x1c], FFS3_GUID_DWORD3
 jne searchingForBfvHeaderLoop
+jmp checkingFvLength
 
+searchingForFfs2Guid:
+;
+; Check FFS2 GUID
+;
+cmp dword [eax + 0x10], FFS2_GUID_DWORD0
+jne searchingForBfvHeaderLoop
+cmp dword [eax + 0x14], FFS2_GUID_DWORD1
+jne searchingForBfvHeaderLoop
+cmp dword [eax + 0x18], FFS2_GUID_DWORD2
+jne searchingForBfvHeaderLoop
+cmp dword [eax + 0x1c], FFS2_GUID_DWORD3
+jne searchingForBfvHeaderLoop
+
+checkingFvLength:

[Ray] 3. Why is this label added?

 ;
 ; Check FV Length
 ;
@@ -57,6 +82,12 @@ searchingForBfvHeaderLoop:
 
 jmp searchedForBfvHeaderAndItWasFound
 
+searchingForBfvWithOtherFfsGuid:
+xor eax, eax
+dec ecx
+cmp ecx, 0
+jne searchingForBfvHeaderLoop
+
 searchedForBfvHeaderButNotFound:
 ;
 ; Hang if the SEC entry point was not found
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87395): https://edk2.groups.io/g/devel/message/87395
Mute This Topic: https://groups.io/mt/89613429/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel][PATCH] UefiCpuPkg: Support FFS3 GUID in SearchForBfvBase.asm

2022-03-07 Thread Kuo, Ted
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3862
The new algorithm searches BFV address with FFS3 GUID first.
If not found, it will search BFV address with FFS2 GUID.

Cc: Ray Ni 
Cc: Debkumar De 
Cc: Harry Han 
Cc: Catharine West 
Signed-off-by: Ted Kuo 
---
 .../Vtf0/Bin/IA32/ResetVector.ia32.port80.raw  | Bin 484 -> 548 bytes
 .../ResetVector/Vtf0/Bin/IA32/ResetVector.ia32.raw | Bin 468 -> 516 bytes
 .../Vtf0/Bin/IA32/ResetVector.ia32.serial.raw  | Bin 868 -> 932 bytes
 .../Bin/X64/PageTable1G/ResetVector.x64.port80.raw | Bin 12292 -> 12292 bytes
 .../Vtf0/Bin/X64/PageTable1G/ResetVector.x64.raw   | Bin 12292 -> 12292 bytes
 .../Bin/X64/PageTable1G/ResetVector.x64.serial.raw | Bin 12292 -> 12292 bytes
 .../Bin/X64/PageTable2M/ResetVector.x64.port80.raw | Bin 28676 -> 28676 bytes
 .../Vtf0/Bin/X64/PageTable2M/ResetVector.x64.raw   | Bin 28676 -> 28676 bytes
 .../Bin/X64/PageTable2M/ResetVector.x64.serial.raw | Bin 28676 -> 28676 bytes
 .../ResetVector/Vtf0/Ia32/SearchForBfvBase.asm |  51 +
 10 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Bin/IA32/ResetVector.ia32.port80.raw 
b/UefiCpuPkg/ResetVector/Vtf0/Bin/IA32/ResetVector.ia32.port80.raw
index 
79b23c047bdc6e552d77d5c9e9aeae21ff04d91d..ccb5fb9d28d295dabdf41113e959d4a04823e917
 100644
GIT binary patch
delta 227
zcmaFDyo9BG0uUG;*vZ7ez@W<@z`$S)9RYyQbps@7N`P<5a2DnL~gPkVYw4**r!cFFjcZU?HGY1)}sx(=wS^|0>c
z(xuH6Dh#FbI}bBQD3s24Eoykcv-u}OY2Sto!p|CZ><5C*qp$yU9^J6F

delta 158
zcmV;P0Ac^61mpu+kdTm~BVV5C8x@00030ax;N=5P0Q`jCJdQcoaQ~98Y!Pfp{31
zHHl$$*@1W*g~Kh=b;8$wu(9TV=}(c8
zUjg)y@_Q48|1M?^0T*VtBLDzE4S>dJ0U!uIkRJe$|NsA!umLp*>x1c7{{R2!
Mtds8n6_W!3A^UepbpQYW

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Bin/IA32/ResetVector.ia32.raw 
b/UefiCpuPkg/ResetVector/Vtf0/Bin/IA32/ResetVector.ia32.raw
index 
ce7faa502b858e99908bcdb397b776258205e1d5..45ef6ccbcb61368711b3d1e90fd9f0633a8424dd
 100644
GIT binary patch
delta 225
zcmcb@+`>{n0SF8a>||nKV9;d{U|_HXGX59EHUDHPRcoves5(#_Qu?g1LgcjVDc{n&
zjTI7m^W~3nW7tMv416`-n$r#(HT2Y{+C)y36^7FJorjqt6iR2j7BoEI+5D5Cv~S0L?*04scOHHHuk+|j`-wr}jBh5cDmQQY
zub0NpD4w=MnSsH9U+F;FtK9$p{|hkeaA0F#;8%K(wu6JA??`Yyb-n;_3H
Z0jOYd0;3s6_e+a^|Np<-GkG4P833l9W|#l~

delta 153
zcmZo+xxySY0SF8a=rRZ}FxWCMF#InvZmbZfc-qrb`ns_~#I{Suzw}XKg~UwL
zjTJJjhjlNPUTCgRVJJP-d6+puq4e-;>)Kp!4YKf1O8Px=-v1XZ$kpQMqv4f4ww*
zM)9;A$_xw+{7MJXUZqaXVU!VYn;_3H;s5{tAge)O@^>vc|Nj4fxn=S>Ml%58
CZbm==

diff --git a/UefiCpuPkg/ResetVector/Vtf0/Bin/IA32/ResetVector.ia32.serial.raw 
b/UefiCpuPkg/ResetVector/Vtf0/Bin/IA32/ResetVector.ia32.serial.raw
index 
6503a988abdac06f9aa88f0a65f2525e12233b0a..16143454c0b6df93468bcde510289e43b343d8fd
 100644
GIT binary patch
delta 214
zcmaFDwuF6xY`rV9;enk@3=9mq3<3-cwm`=JqPXUtOr>g#6#`WUibG1DHCBk6wms!r
zdbhDcVsF0Ow9+e$6*8UAO=gvzeysvjRq?c^r}O|=df9ZChs+p#piKXj+s#*{0
zUM^kQT%p2HI=}NUbA&?ajMssN2Rxg9GL-hE?fT0+F{OjiW8`AFs>J_`LCD8
z@Lz#iWfnVuB+AHPBT}&|oJ`>~_Cj9^Ze*zRte#c}Yu;9;Yrq|zIvi^U${?Gsa
LFE>v%Vm1N**C}5Y

delta 146
zcmV;D0B!%I2jm8j9YX~JF~BVV5C8x@00030ax;N=5P0Q`jCJdQcoaQ~98Y!Pfp{31
zHHl$$*@1W*g~Kh=brcl3=iGhJZmqCDm!4}B)UliB;lc`j#u|lBgKygUvv)3
zNbJp*n^t{?AX75Kkxp|qp$yU9^J6Bj4$8^oS9{Mya%q3`zYTOak(_!-61
zb|^D2IPfbSNPD$!a;u^OyQ4b81EBWFTNEw%UN^ja@b~}!mpT7{gvsPTiV}rVygbAi%(2%fP_!zsR_;LZISlPfzLV#tISJE*byQM~xK{Gfg`aOK
z$h02Tysxk81Z^i=0z<_Lw-!>@TaY`>8ANp?ZUi48fjh|6GZHF=g
zg9E?PfwWg`llLkr@O!Brcl3=iGhJZmqCDm!4}B)UliB;lc`j#u|lBgKygUvv)3
zNbJp*n^tq;e#OWC
z|0Nh+`|M`_}ygvDBH^Ya%+q?ID)Jx-M6i?fs%)sEluXG^o)w0RCiZc9u
z>I@fv+9p83MyJRzkL7q|Noc8|A2(?|NsAY0P%kh#bX|dM?DmecyvDa|7^m9
eje;9^6<$hx0&4}@$q*Le#xP;ROJ;}$oBQj$Rc

delta 239
zcmZokXi3<>rr@K?Ai%(2%fP_!zsR_;LZISlPfzLV#tISJE*byQM~xK{Gfg`aOK
z$h02Tysxk81Z^i=0z<_Lw-!>>7a><5C*qp$yU9(`Fqxl19OQD^cag(*P}AOHWC
zV0bwdXz(3;+M~E8Tc~^4D}G_Hw|&%0<7X64+o8{
z27aXnX*)O=`fl_c=zGxjqVK|MS9OL7K>5iR6m9ulx4bm?_y7OP8W3SKSw=~Mal%Hy
e54;L5Z+)Bq0zms1!b02_CQNvF7$O1|o$orgJI)

diff --git 
a/UefiCpuPkg/ResetVector/Vtf0/Bin/X64/PageTable1G/ResetVector.x64.serial.raw 
b/UefiCpuPkg/ResetVector/Vtf0/Bin/X64/PageTable1G/ResetVector.x64.serial.raw
index 
d9b051ff06d1d30b0dfd585e05e75c98ebd88b03..45e8a5f25e082462e79ef894749fc02a8f5e112c
 100644
GIT binary patch
delta 259
zcmZokXi3;0t5Cm}+3>(lCI$uuT?PRL23sKGe^FfXPo`3}#tMO|1H~bw)R4PTQXH
zExp@VA+a}KZd#tNCv=O(jCPrp_Hs;YR}(^Gl?sLHlW#=mqsP}NM+`W
zbuX7LZLUyZD4pMVm^nhBbjIsI!vmhpKN(8<(suo2o}8l4!FXWuTZLPUuO^>RoWeM5
za;%aYqwwSrau&;S2V#)%f$8
z>GijlZ2w=H|M~y_rS)GRQMY-&@>gz!m#;og00E$%7{Wr_7$!`3c^M)C7M=hAJ^6Hf

delta 209
zcmZokXi3;0tKi(rY;
zmfmcvkZC=vd%5<{+>8Z}c%n=HuhhKY6o}tjgcw(}u;w{Ecla!_~#!a58nO}3=aHC2hv`#Otw~z;SW}4DER#U{{$$QJWtt1G~&-|rq|zI
x{`>bb{m=jZFOB~KiL%Y_mA`UNR4`C@x$84nG0>R|VIgh|6DGV|3=x5t0RVO0UcCSS

diff --git 
a/UefiCpuPkg/ResetVector/Vtf0/Bin/X64/PageTable2M/ResetVector.x64.port80.raw 
b/UefiCpuPkg/ResetVector/Vtf0/Bin/X64/PageTable2M/ResetVector.x64.port80.raw
index 
0e53a574fab74db6973d7ea41a6a495266a4d0ae..4acf69533a7b60ad0902e9b3dc320e32baa79d23
 100644
GIT binary patch
delta 291