Date   

Re: [PATCH v2 04/13] MdePkg/Register/Amd: define GHCB macro for Register GPA structure

Erdem Aktas
 

I verified that the values align with the GHCB spec publication:
#56421 Revision: 2.00

Just one question: is there any reason why GHCB_* defines are decimal
while the SVM_EXIT_* are all in hexadecimal? Does EDK2 have any
preference?

Reviewed-by: Erdem Aktas <erdemaktas@...>

-Erdem

On Wed, May 12, 2021 at 4:46 PM Brijesh Singh <brijesh.singh@...> wrote:

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

An SEV-SNP guest is required to perform the GHCB GPA registration. See
the GHCB specification for further details.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Reviewed-by: Laszlo Ersek <lersek@...>
Reviewed-by: Liming Gao <gaoliming@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index cdb8f588ccf8..542e4cdf4782 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -53,6 +53,11 @@ typedef union {
UINT64 Features:52;
} GhcbHypervisorFeatures;

+ struct {
+ UINT64 Function:12;
+ UINT64 GuestFrameNumber:52;
+ } GhcbGpaRegister;
+
VOID *Ghcb;

UINT64 GhcbPhysicalAddress;
@@ -62,6 +67,8 @@ typedef union {
#define GHCB_INFO_SEV_INFO_GET 2
#define GHCB_INFO_CPUID_REQUEST 4
#define GHCB_INFO_CPUID_RESPONSE 5
+#define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST 18
+#define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE 19
#define GHCB_HYPERVISOR_FEATURES_REQUEST 128
#define GHCB_HYPERVISOR_FEATURES_RESPONSE 129
#define GHCB_INFO_TERMINATE_REQUEST 256
--
2.17.1


Re: [PATCH v2 03/13] MdePkg/Register/Amd: define GHCB macros for hypervisor feature detection

Erdem Aktas
 

I verified that the values align with the GHCB spec publication:
#56421 Revision: 2.00

Reviewed-by: Erdem Aktas <erdemaktas@...>

-Erdem

On Wed, May 12, 2021 at 4:46 PM Brijesh Singh <brijesh.singh@...> wrote:

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

Version 2 of GHCB introduces advertisement of features that are supported
by the hypervisor. See the GHCB spec section 2.2 for an additional details.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Reviewed-by: Laszlo Ersek <lersek@...>
Reviewed-by: Liming Gao <gaoliming@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Fam17Msr.h | 7 +++++++
MdePkg/Include/Register/Amd/Ghcb.h | 8 ++++++++
2 files changed, 15 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
index 7368ce7af02a..cdb8f588ccf8 100644
--- a/MdePkg/Include/Register/Amd/Fam17Msr.h
+++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
@@ -48,6 +48,11 @@ typedef union {
UINT32 Reserved2:32;
} GhcbTerminate;

+ struct {
+ UINT64 Function:12;
+ UINT64 Features:52;
+ } GhcbHypervisorFeatures;
+
VOID *Ghcb;

UINT64 GhcbPhysicalAddress;
@@ -57,6 +62,8 @@ typedef union {
#define GHCB_INFO_SEV_INFO_GET 2
#define GHCB_INFO_CPUID_REQUEST 4
#define GHCB_INFO_CPUID_RESPONSE 5
+#define GHCB_HYPERVISOR_FEATURES_REQUEST 128
+#define GHCB_HYPERVISOR_FEATURES_RESPONSE 129
#define GHCB_INFO_TERMINATE_REQUEST 256

#define GHCB_TERMINATE_GHCB 0
diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 712dc8e769c0..ec232ebd3807 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -54,6 +54,7 @@
#define SVM_EXIT_NMI_COMPLETE 0x80000003ULL
#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
+#define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL
#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL

//
@@ -154,4 +155,11 @@ typedef union {
#define GHCB_EVENT_INJECTION_TYPE_EXCEPTION 3
#define GHCB_EVENT_INJECTION_TYPE_SOFT_INT 4

+//
+// Hypervisor features
+//
+#define GHCB_HV_FEATURES_SNP BIT0
+#define GHCB_HV_FEATURES_SNP_AP_CREATE (GHCB_HV_FEATURES_SNP | BIT1)
+#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION (GHCB_HV_FEATURES_SNP_AP_CREATE | BIT2)
+#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER (GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION | BIT3)
#endif
--
2.17.1


Re: [PATCH v1 2/3] MdeModulePkg/PciBusDxe: Fix possible uninitialized use

Sergei Dmitrouk <sergei@...>
 

If the function gets invalid value for the `ResizableBarOp` parameter
and asserts are disabled, `Bit` can be used uninitialized.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Sergei Dmitrouk <sergei@...>
---

Notes:
v2:
- simplify if-statement to avoid unused branches

MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 6bba28367165..4caac56f1dcd 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -1778,10 +1778,9 @@ PciProgramResizableBar (

if (ResizableBarOp == PciResizableBarMax) {
Bit = HighBitSet64(Capabilities);
- } else if (ResizableBarOp == PciResizableBarMin) {
+ } else {
+ ASSERT (ResizableBarOp == PciResizableBarMin);
Bit = LowBitSet64(Capabilities);
- } else {
- ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp == PciResizableBarMin));
}

ASSERT (Bit >= 0);
--
2.17.6


Re: [edk2-non-osi] [PATCH] ElkhartlakeSiliconBinPkg: Add EHL microcode

Chaganty, Rangasai V
 

Couple of comments:
1. Please fix the file description of MicrocodeUpdates.inf and make sure it reflects the contents of the file.
2. Since this is a new file, start with year 2021.

-Sai

-----Original Message-----
From: Lim, Jin Jhu <jin.jhu.lim@...>
Sent: Thursday, May 06, 2021 2:11 AM
To: devel@edk2.groups.io
Cc: Lim, Jin Jhu <jin.jhu.lim@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Chaganty, Rangasai V <rangasai.v.chaganty@...>
Subject: [edk2-devel] [edk2-non-osi] [PATCH] ElkhartlakeSiliconBinPkg: Add EHL microcode

Added "production" version of microcode

Signed-off-by: jinjhuli <jin.jhu.lim@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Sai Chaganty <rangasai.v.chaganty@...>
---
.../Microcode/MicrocodeUpdates.inf | 18 ++++++++++++++++++
.../Microcode/m0190661_00000011.mcb | Bin 0 -> 19456 bytes
2 files changed, 18 insertions(+)
create mode 100644 Silicon/Intel/ElkhartlakeSiliconBinPkg/Microcode/MicrocodeUpdates.inf
create mode 100644 Silicon/Intel/ElkhartlakeSiliconBinPkg/Microcode/m0190661_00000011.mcb

diff --git a/Silicon/Intel/ElkhartlakeSiliconBinPkg/Microcode/MicrocodeUpdates.inf b/Silicon/Intel/ElkhartlakeSiliconBinPkg/Microcode/MicrocodeUpdates.inf
new file mode 100644
index 0000000..ac95123
--- /dev/null
+++ b/Silicon/Intel/ElkhartlakeSiliconBinPkg/Microcode/MicrocodeUpdates.
+++ inf
@@ -0,0 +1,18 @@
+### @file
+# Component information file for AcpiPlatform module # # Copyright (c)
+2019 - 2020, Intel Corporation. All rights reserved.<BR> # #
+SPDX-License-Identifier: BSD-2-Clause-Patent # ###
+
+[defines]
+ INF_VERSION = 0x00010017
+ BASE_NAME = MicrocodeUpdates
+ FILE_GUID = 9F7348C8-0CB1-4667-9319-122CACFB0B22
+ VERSION_STRING = 1.0
+ MODULE_TYPE = USER_DEFINED
+
+[Sources]
+ m0190661_00000011.mcb
diff --git a/Silicon/Intel/ElkhartlakeSiliconBinPkg/Microcode/m0190661_00000011.mcb b/Silicon/Intel/ElkhartlakeSiliconBinPkg/Microcode/m0190661_00000011.mcb
new file mode 100644
index 0000000000000000000000000000000000000000..8140fcc082340983a439fec96732faa7678ef6e1
GIT binary patch
literal 19456
zcmaI7W2`Vt4=uWF+qP}nwr$(CZQHiZXWO=I?|t6y{=CUKD_Na3Y5Qx^%%m+K0002u
ze<UOT0Wt)G1i0iRk^%hB{vYS1{}VF*@&AYY|7<`2;QucU_&<FB2><{Dh>+la=l|mV
zm*@Y5|M~piaAZ(pX}#1Ws#Yu3N}hVq%a){NwTULgqb4Qyt}7N8U!7sZ+eO2)R6(XK
zQkdJ#h-tPNTe1GL)^5W^3LQP#Sl~>_&bNKQrOGB$G#n%G*3mCV4h<!xMwNF=Kh%Y5
z&{#%koiF7>vZ?wB6jvyhiYzz(09`Ll@Q1#p61@5R5WS)AWpp~MuD<2Rv!HrEZ^N`B
zLgFued#5W$7_aC)J#iALag~JNRr1<-i@|@t97w%8W37Wiev&!TZ0-K<QlYSa9#3?4
zPsIKFee9j2re!;PTsLQ9h?R<OsG*NpzlOswcL_eVmqM}{ADq$CR)ml4=?3{%G2mZd
z%s|gFZ_Qyr4m;aMO;Mf<VTheBP`Rc(Z;U{poucw8SWb2F?xEFq#wHyu6x6k29hl4~
ztWPJlUaWG1`&%g(Ln(7&&0}3ub;BdLkkN+0-nX*xAey9w3m|0G`EB#_>~*yb4xg?(
zAU1cijFR@nxmCYC6Ofq6pP71vkp)rxnN=j|MOcLL0A@0)Jrky4I|TVXp(|(enc7zi
z4YqdB5u7M#!JQI9OR`WskHWlnxtzY>*I2BehOkY<U>qB2G?Na9Xsl=}9ED}NOe%5y
zac2ZraA^C(0UhC8H<cCxXDYhdnbz~Ay3_|dZGgz>!Qi6|4mGrm_C(dE7Aq?r?w#}m
zVh1rY_@2a_V9iHc1dn^i!NFRcm+T;I&6V_n_m%D~HPOEg26cy{Fd^K3H$ZHzKtxgz
zop!Dvi4wl`8g~Q?4PzJy$^|@GCzh_IIDZ-4UxhgQ5j6cl(G7_$nKjH=T*2>pJE=?Q
zgZaRZ(OY7!A}^`c<{j7_7sDl0<C*V+QbKl#-b!GYMd$hx4ZbVW2?x~cTL<3}(+aC)
zyHeBD7(w(!TRe&{q#(>N^tZiX$ep+zmo<)U)!9|_I8{Q!1eLA|xyr-(EDfs?3Z7Tb
zdf1?$Vv@Zfd@3^T?|Q39f^|6+#tQ}~=*t#jz<1&;(UxNKxbN6VYcx<19FU{|x--fK
zf}?WJQgzb-nvLBt2N>d-A{qB)5}}AVJ7&r0Tv9G_$Pj!D^dc%NmE{A-;JKd!W<M+%
zIkb}YT68IQFk$n`xOr|&SDnk|kXCw*dtZbcxT6F70={GN{3YCOb%cIv7$7Q?vrXR{
zH9-7TiBpXcPK)`{^@2D>HH4ywN#3QH2zlcfuRjdS?z`_PW#BqW#-gPwmF7%qPUBi)
zltH*1wY*B#$FBzJ)wjWCT&cimTwyqI8IS2tjc0m6-4^z!)vXM2gIx2N)qOwu5m+XE
z7I8>Wa61#OfN1uE_rBw5k`WR{KvLN@mPr{ej_!#qQ#uZt13N6Y_eiOwNuQdTN-}J_
zc)#S}HDM`$zx9U6#`vgBTlzvCq@YTBv9>m@B|7kp+Snh@aQ4ATe9PEWT9GXxYYt{t
zL7@nMrYD=f8hR^2GxARqy%3nyWz|?Cy@kLfo)Nh%A{i*8ehS=!^$~u*goha8XteJ*
z)dkwd3*hDZORg`;bFmurpOq;K?c?|id=HgxL&00Fe%}(#zBWTD(ZMxS3vHQGHWpfo
zQje5p1Vq+ZoBp1th<XF9Cnvi|t>;;?PCP9o%2lboFATuEt`rq46?CDimQ-lYNJIF@
z=fj7`x~(riNvsN^g;IEqb0Z29E7d=b9h8ipVkJ-(iU|xN7F7$BcgVTyE}~|sUH-vV
zepVN*AsUh~huh)#gaS>}B8yivmnuc^f5Nz5n9&Sd&|9vlE&1t)Pg`y6))Wij%Ty(O
zKsJv!p*#c=-W0!FO&#t9l<cwMUUlYhjH}3aC}5XKQYp`83f*%K>-gY-`~G~=Mr9Vj
z%50TiFV19vaT+sKBlr18JnvBYyZ&&uMxvk`v|AuZ2Oj!VBDQncCJShLN;qdTmfP+E
z2?pI3A*>m~#xFKb%->Ed84`Novy!LN(+Bpx%|khxo2ouTqUQl8=Enlu+1JSzEYO$h
zj@mXDj6}JGwXWDNXYTfire;zNw<*uCqE(KwV+6T)woolF+oXBlGtmkxj>}U*UO~iN
z`?^|M``<;4Z|-;B!Ws%=M9^`mDm>Y_$%@vsWVxf5uAnDNbn%YPJ-55$zJ`5Wj|?~W
zxp+2N1HOiKjm^n4@p@rw+%M;?ZlMVcm9%MY%?h}xleLA`q}&%9W}0SQYAq3eD`5Kq
zDC>i#{KBIH5VYz<*~-%mIn`QKQ9RKnV5t=v2T_~EdU0}T`$OWushpOPY-=&H$hN-V
z^0Hdl0=u#bAeWMDqwZXpZu*(~4mx}MwS@N1gC)tyW;;zLw<ixISfJYnFZ>CWg>U8i
zl&L4*{Ts|J7<N~{D*v$7rdL@A$_QFuaiuVtJ4bHMUEVnZ7%%*0_WFvJCQf}%<4)Qt
z6c|(B2WVo=;b}S2^f@woK8kbZ`oSQf%0J9^i_hEaJNxBaFa;F4O3&Md;15Web|}*M
zKVh`T<|{Kq5r_9y2-|yza=D1~YHvc8a9f6;TZ`e1S<k<(WOA==0IBI&o^bwY2Jc~@
zVA3GF8uE0aGoEIvl6+80G^Z7k*J<1oxdM0?ui2m!VHrsoRS}e++tdlzn!&jg#z@4p
z#L07_d4q|pm16X;LTg=cR*j{Bg=N_x@`Y6LFu}osko|_nU-)Ho#gUC`R?{T-X&HQ}
zgvbp|-`K|PvO{hScULwF=bM*Inf!spMq6Iq2_B@jrTNVVr+-7FTVToy*99`}HZWS@
zRoLyg{$i&s{puz{p)3dp7*+U~Vfx-8>+D%_9NMkzM^NEY-7fUPrk|kroUrHF-a){C
z6Va<0nNte@VFNHEIONerMYdQ+mUmf&4;4q|bK2|-0bsVd_U9m)>D#AK5sCBp<T=Fm
z+>}3*xF?tj3|22AP<m#Cl7gpNnd?w_Tt#b^_N7H4gatRxFlsj9#$HnfsPuBVBhiAI
zjynW_5AVq0(Ng3EwDq3G^_6J>3BqlfWMJz}R>-^6pbUyr3o1#DfA;f!?*W;OxbELk
z+Vz1Ag-E@k<PpS(dQ~sG$~%&m%WDY4PO7hua2Q{oXK_&0>kPNn{gPW;JyU;D)AV!S
zaWUY?l_K(#=R~?I%wV44H3z(&TKxmaQrVhV4^w~g7pY-7Q>bi@s4|)RK^`0zP;^=O
zA_E``j!nuXX5RGV%d!NRi-CEvWJWELn`897r{>UX*;kGwt(elmk8Kb!rgi{F2AkdP
ztriihc0$BNI^Zau=GY6Ea$1FYhnVc(1T9x!TOa|d4SV}{D5;81hp~E>W;e?3S82t+
z^}^D>JaqPf-38NQc-Ee<W$hkJNm`kSjs#{bAX6qIi5AQ2)hS~W+v|orgmn3ggn$m)
zyqc+R9igc{m;%iFIlmFzyT=Wt!RQp1?0`LMMinCWQiU=8<|NQ)s$%#IJByzP0H}<(
zcjRq+*oP+KcbOnaDWFg(D_C|t0<ayR^>c@Qz#>AO%h$f=Uw2WJQWeX_w676m<x3B<
zlCYO-GoM@5r6(r>*sHA=GlQj^jYrzgZyIL0Zwzm_F@e;?vwau{KL<&=yEmJR0*HTF
z@rX4ZZ7C2w%#_22r=cQ|Gr4=u#?4(Z&|+SfAn#v%ukfx9%FxwMC_Y3W{DB0I*Yj#C
z66)ZgMSq`cp`Q?+Zp4V!x+OZ?`$HFHoDUNF0}$9HIPpS(CvfNh=f~T;^`p9<@AZ7S
z@I>AQDDTx`ns0<SE&_M9a^cIPb>Qe@qjQa8#@FNJ78742vRVQAoVCI^0!+&(YFp}v
z7LP2oL>!JN^m775#-QB>I8w&L7&J?n!%qs#{$-5xnb*np8hp^#ArY}ddWOST5F`P+
zGB({O=t|HKuVoRn)g+h1T0igc%snSR*5u4gk5gaUye{$*l0i@eZn^FXY1Pg?a;vMV
zmZl63Ad43E2|Y10T=~X>yfquc*Y?{=-++_V-w+Me=I)AGZ{MELH{2Hu6OO>ZjA$-b
z=@p!Xy-4&(c1~XG4Gx94z2HSOaVb@)`AT&Q%s<6OEg0X|d9bbtogxaqzh5%U>tMKi
zaKkXI5hv?Kk5NQiF?3O0_{$5c{6+8758w3BV?FyyjD)j4Z{Q@+n3HiiP8&LU9;Y2A
zW=oO+KdES=;kGmhySA&On406Hgu1^U<%71q!C-dKyfJ9Oej;_KWG@ejE3&BsA0>mR
zJ>o#!QbnL_mb5(2C9b!Ahwp(S@^lY=I@b0fokByjRX!@I->#VnFvTORpEg1emQvC<
zu~^|E()yWtJD;xZ#4#a9zjA|Wa!BAMjx55Q6H-Z{e`msk3Fsji*|N1ubS_Jr;+9u#
zdAVWkJ~kHN=pzJdu?0e_V7yv-+tG85Be%VSUwPgc7a^%ELUbNcPz_x`NXfPtPS>+h
z!kx<0JsHsLd`ZCe>8Y*%=j3JLLX*Otrf!_FgCI+H)<Nd2+w+12tqehN@sZ%B4<iWa
z%EEucOAe<I535-{aPw0$xS6{xoAO*Siu!!?aH5qr_i~h}gGU1K0ypJh>Q4$0(@&&1
z_ImAYuio|hl!Dnmt&9AwTnN)PA@)jrTvmf?sHuVtM4pkEcC*pv0Y<G^l4d!vJAtrX
z5KZ<zA?Ttk@OK)szj<G@3x{}B>dQSi6sjtS8>E4crB}dzT@xSy%+_`onzPuRcq7d?
z@2$Rn{$QB%c2*_xN)9`WlvkQ$Do`#yS#~<uSbcovhM!@7n?Fi-k2pLS$iN2sQ5F_w
zA`El3tEoJB{2}XyYH7)YiX&F3^-{L79I>uSw&g=K!@}JJK~09uj`kX-Za?vahu~~b
z_=nJv7PjRNmA{)>o4WmpIua=tt%Tr?q&#)6Y+JnwC%TtiA`Y_<b=v2}wTKg#jQ>K8
zwkVD`zX?gFCASk5$VC?rq06Ym0i~dUq{lD%BYy8u6ov35OF-(0H4^v5BlNn}*Jr<g
z8OhP^PsP};?KY=<Sa#=AqwrA1U|?)po}=oS5uSXrSr*l=H+DwjOqymHW%_7rz@1&J
zfbE5jV*R^LDN%9J-^l4juv2An#P=_dkKfFxb?SU3^b^)OC}7agCRpRyhTic;XwA^j
zgC`G+u0jJl$OT;jnxlIJULfDulwRVby*pn|hp`5n2t1d=7@Ne*mWaDRyEWTNVm4g+
z!Z*AXa=@8F4+q=g0B5><(&b)<`;Y(35Yn%tZvtF)LbX;NaR%5p9}NLu=o2cF<tG1^
zP$9vV0InHb?0|d-3MHB=Oq@1hdNH`w&jQ>f`6FkqI+fhW60+`VHQlMP5ez0qv*J5g
zPekH<dDZaRka{efQD^42ye(nOo!U7nH(huSzalTJ)d1!D4{q<M3+=*6VY%Jm`A%#8
zO@EMkSPrntH>R&+8C6|!imVuwCo{9T0z>fN+;yDfhXBw(^4@c}tZaxFh|c*wQd!!g
ztm<cGa!Eb7*d$#{N=>Xd=>YB|y2XcsDSz+EZpcU~xF&M#5bOX5d<`U5wk6{Xhe5ac
zZYx4q0m)9JhkBfGxI5|T1oFJ3ajU%i``;o?XKO+I!e5@82wll*+hD{2v!PVx^v5V*
zUZQE)!SC^O*!B9rPlEMIF!#BK-Pdy&sCzw4cBx{9{aVpmN*!XOn2g*4e(5hVQ*|}R
za6CXiB;Jw=3y-v8<kcZBg)UbZgeZEd@4ZH_Su*l3wVBRLFArJpNTp#{9v431YJbWW
zB{-4AfCM1NSJ5IZeq+<1(<atz0%3;Se7Lw<QYq@tsliQqzM*!;C#`v@nh@tweO9sd
z>pS^X6o{MeHlEPqLk*{wF*d%_KKzR+QXYk6E9A8Wp$?6saH>h+^w0DaILZ5fQ95TY
zaLRV%rli7UA}+V!JX?)Y0UgW?B1&ppsIDL;`}p5AgPB1>akF7cx%%j|2jH2!v9!na
zJk1yy7?zIjTfB^jRalz6i?E7l520VbI2P{>yu7&#NO5GibLC8|{21>B0iWdwrmw%=
zIMCz3Y)SlDTPu+_a9>U{s*i<zjgOY31lineqAIFzXuEH2?@V1$22pxbQfY#vph)Fb
z@JyV6^9d*Mxfl!t;OIX@^6TiW^kT$iZvL~41YvEjkZ;|P?WfPZ#PMQj2I_Z>m@s5>
z1cfl09YGr0>iFVU*6rWch_izkCF#{yZ4^5MfJ2DHK-Gy7?ak0V*rlg-E!sVm;Un4+
z&rq>F@z580#GB0KMI+E<>ZZCoWz4CoYM&`9FprG+bwzas@1S$rw+nIwPH#7J($9}%
zQd^g2#SGG=n%KXRyu^GgIA!Uhy%gY;L{Z@^FB_zWWf)uV@<;A=O8h=X*n{x}L^x1D
zbYs4h-3eut5(XP^WG1E)cYxxZCB2nm>=FGPn6uGlc`^uFq$khzhiRrQ*uE0|TtCV3
zN}IQH!KQBh$4n#kTC8xBO8By0(F(cl`HajRAFV@*u`e7nY6vCC60#QouHn+V;{JH1
zG%pLU!XEihEtC`JN=7R;K!$R>Lt1$a;%aF?GPW6F-cW{{PneOfu!ELBV4tx|%}{x;
z#e))n78Xr+dJAw37W`SZ6SaR!Rtvj^fpuCy85|*aPfPO!o{`B6f*pJ%h?3oVB=io}
z9CSy6Z2ax%Xt-$=VAu-DV%Z4KW?gGlXslWD^^I8CMCER*B+&`xzVfM*dr6`2Iuxri
znfr?wQrV@QRbZO)g<ugg313FCl{^P=gxNH{m8US1AQoU$Dm4T~u73+);A}y`Md7;b
zx#<=n<0s0d95{DnClGg&G@5-R4HedD4BXm?0<%Wgh>Nqcbc2xA+J3tF&QOO_6AY~(
zzdx+`BJ|zs;)_hn(ru#{>2ekYbm|c`URAYGSPzqhkfnxfxiltmkd#h4tN6yNPxfol
zlZ5V)?*K#vv<p_3A@3U;J)oT2oD=i2P8da$2i>8ttpre@(9sMkO1r_kG)OCzqOxlw
zVXP#4;OyO6{0d7aq0?LX#aDF`w`i498lcZc6@IFUYuz@Q4XoH;VkLm_q><1y1(P)E
zOnkGp{YW~TUE>l6Nt>p+VVg3Jo0=UmBb!2^#r?KRuk@9iAHY^5vwN1x)MN2jY}Q8r
z>=Y-AYAFz2l#43Q_7XfCXT5exrc$5r(Mwh&)_kT#)HRr7OUf85uW@vORJC{G6={?|
zG?{P#sF?B3{gHvuhQN%BkJk}AixgOf#HzMgM5Ry;vvcqJdGc>?x#q&2M)_gDWbl##
zXrHIy9N(jq=!(?(y3_@7mC9r}*rLNUC!A#{D_Zgkg>3iHGaH{~F*RDkG(2&oABHZ`
zeL;e;1|MjWMF3_bEtY5A%k{%l)oo3SgSabqcBU{B*Lp-1EH#z=)}|s$z4qR2grk|e
z%{ToQwlz;s`83!@KKXR}Y|Z_{&MtN~Gu03!2`P?w2)M8lu9ys3IO)G`b-@McClY0k
zL1L8>GhdW8{y~rGVvP&r^rpRCWI3E4ZcG!Ez_X&0GS$Jqki&WDhdt$uVQ5dej5y3=
zu5nbPoy6Vdnb2$3ZX;s7*T!B%QV1DP@0N(&keClOv)#(wG9N*~K3cQZ$Wz`c)BZoa
ze4}a0a=|vMP1R0ASb21*VggXoj^c%cY97rtMf@;}nBGg5Vd$j%a>ItJIP4;2mq;Gs
zqnDP@GH8MotrpHVl=~dIYyed0Wj6;6xg*F1V6tsa;T-*qdwRhy>XD`>cJL1vyMPKG
z3f61@uaZP=d2b_5cw1%Q!PsI~z1BuSL6Mj9Am7yMq|1?0K_gYwS}I!rVXkFt6-)~?
z2wW=i2s_@meu6?n*|R9%Bqa@t2G`cAX_g5fs%pLW<QeD<U{p=mfS3=z8hEu*prV6#
z&arB@BMeM3K^Qv<K_R|+JAM-}YX_BIw=PyLM{3I*Kw-bQM03VRP_P?Q{9UJxQubf9
zpAh`Zzs5DI8dm!u!88A08O0sn(PIJ~XR&TJhboq^AC~-7j;#7q;6l|M<hi~+!!*r{
z2-kZLglO+%ngC}nyEUv+Nzkat9Mqh{%l)L|qQLM?Nj-c-ZjiuW(MGc&bB^4wbZU9l
zWOviDw-8%>5h8vGtvVe&`>P&2b`?;ET#?JObj-&<Ek6SbGN?kRAqPZ>P_0OV`YA^#
z-L_aiwmtA!6N|wHzQg7{S^a-}l13_R;&>%+_{6p+;c~dDw|Dxh^lnsa)_Lm>4T|Ra
zD3I;^ObAZrT*6X0=>sbp&#XZF#iH#3*Up#;5IwZu0RyY#MZBEE!CK)bcgidgYv<uR
zJN`8`p`1)ct2ag&gE2|sxc=c+DNLNwa5MZ`K9Y|EDaQK>$JnEo-pO154k)um`ayFY
z!j{TR^etl}CZC}kB8lsR>(y^3ql#@P+IcW3I1Vzz9~jcSB?4s)45_f{RFK*~!GTE7
zx*pjenZ&tghDL<?46<o7aLVGde*?&{E6c^d6u!1YsM_I-$?75g)erxZ!Dy<2Dvw6J
zo?_~OOm=z=Q^DKbhb1o>tQRHrpJn4o(K=)J2}8B*xZ`NS%#XK+7je6RSqs`dCC|a4
z?0I_tVv$dY@4S*2wY1r(lLtJB{#145OR#V!*Nlzo`$j=7YVa9?an@`-DZb5v5~GrL
z^dT|mBF@iUV*C+HeIK*zF>vAU=FzFkNz>1oXEIjT2<PKy*_u6W0$68}q{S6$$nL})
zveUh1sv?$PkA6c0vI~=wggk@?zwHAuCOubfDIRO5>mSmSoCFKwJa7BRZ%Hk3;T&Kk
z-?BWyTX(5MVNo%O@)kH+oUpDBQFG@{?7C2NDk7Lz=VZ3DrhafHwUv$)ay2*>AN@R%
zvpI84)MqPHKSf%~;jv%{0M~;#uTl%@6Qg_pXg96xbcH>VNX(8KG`bX6?LlsWkPuiL
ziv$o*pNut&E|SmM0%)zUS;qzEQgZenr$8W|;RA0V`XbJ~bkoI8RaSe7Up{ow4fNN=
z;x_@|$mtv>dA>79N4eV6<shlioDG!du=y=xV^%|K%M}-BmOCGrfNnRJ9^{cLPhJDN
z==5XS(7YSpW4n%$9CRCMOTD;@o!m-wVc7$&pr!v-%Xp!?*41fMz#KfOPNm7KcJkc%
z?Cy%rxN{U<2)7e5;ad*i!q=UU*XbdWGnkJ<6<(Q$aHh8*Jf|gd9}e9JD<nNi9_w^y
zvErI*Toyzl1wo?+x}RA=CzF3mBcP4QAKLA*UmA#AQ|R*Bf7oupjGG#bk~fwi`jzc#
zexhx=6hcbA+pUZph&Bm_&(~Ir4s-+%<u@OBdkSlq?2k~C^N{J~jgUB)xt4t-vDPyR
z=P(hgWY(ur?L4oCnHR<iUSR?m@8}&cVGXDi053kDYn`PBG&o$p^gm;~-SzV8%nSF_
zwBeMffVVwda81Z80+~YrbhWXAT`4!hp4gz~i3tV>DKFjh3A&3b21%ab7y(VLr-SK|
zL?GFrNE*+$`*MT9(-thp`v4TH-KlHmyT?P3PXYNtk$h0o**$uju9GyPVbn)$DOY-2
zkanKTYk@EHQV^%tfh8`;w=unK+AAD%3Oqs5E;sHI(;uNQx7Ph@Ns^X!EiM&>1y#^A
zmuL`P*y7O6`h|-q%~X2jX-=&R_3IxV&8Gb=tJkvUY+dA)R7B*S@-3ux?~sZ~xh9Lk
z$|12o*%xTHFAbc0)-Q_Vp=5p=I$14z`}QmDnu+aUjW@cJ%HQCLReqS0NM;oa!Ik_D
zefAc;+^gjl!$;d02vumOZ^{zvbyz?7XJs%?K@;q@HLZ1t??X-JSs58<6xd!xEV$kF
zM!QM!G-NzhNmuu*kORhi7W+abX4hs+e~Ru<=97(BlWlVk|8D&Uoek~-Ge2=}lZ=5b
z(9H6P`(#<(U+B>+Um&;=g-g6H!HFUfK9zv2ddqP7^gX7;T{k~W71-j>YtbGy{J5S_
z5S2}G76&WMr4A6-O))!|euL_doMzGafm=O_!cH!ZHy(1)ccZ(qw+)hjnz!yIfCm%X
zui2L(f2voS-+48pi8Pp_6^zkGe4dH=SJ0vs!3~%|F{h8v1+MJ|x~|9rg>M=llIK8I
zY8FfA9oNN_FSz;Ewwokc&!teH<L;dNCT^ODt=05iq$%ZGV9c9v*f)SPnHLJ5KH_O!
zlYB0207N~j#AQ-SVuIU<vJX$gAwF8r{N}5m3~$hXlhf9sIv*bU7%&Fu%Kky7;8z~!
z!bc9-r@a<~^;k2?St~o{YkMAl7X~`K7H|nZl@EF=0;RD)&2l6l$DeXZ4z2VKj5N{#
z?+aVg8?D(nx~~w@%&t7mXIM<EL%0OFM^3!c-8Ql2V4#$5@UP<>ex4)2PA^a8*1&!8
zumo`gdChQ$_l@Xj4sT}yOM<wHjpa6p`DpkDRhd2v<Zw|;yxw3V<r0mv{TZt@xDppQ
zBaHswUk+7AWV3FYeD!MoV28ltJ<h`tHP?WQP{6YG-Vr8zGhp^P&?3xa;D{>|y;@Iq
zT@cE#^235?sOBW9xd|7jl&xZ^Ai;e(Q(0=c3WU7$c{NW|k-yG?I+bOZ4&Y+dG+{r*
zj$Vn0&at-T3gN7$-!Ofvyblt|*lJq@TsK?@av}@t`aAlsY(K8XtTUE$w2iew!--Sc
zk-iWcRJp~v%@hJUe+VjCHhEN#TJl^~HkzXJ3v>#D;B*Vdx|*QkAfhVng2>0b{*Rx?
zX3ff-awUII@+pz5bLuP-a}S<@Vt4^1yn}suM--Sdq8x&Ypnl`zZlGELBe|P{x7xoP
z&-C{6!{ZgQ!%+y3-s(0|@|-U>6Y`)Alf57A#LbvgqxEaH_S%h8r->mS2kL4Q7|k2P
z!m9Q<v8@>g#9rGyFyKY=NQxO&&K+9TUUYce{<LO6x;d5OI1bOuIey(aF%J<NHIUZW
z5v=&^SB%tRw(FlZV_F@NB$A}`;x2(<<F&(4nMU>u`jHHoBb$-YySJ=vCaClVhPUgD
zkAU>-dmDwe>Iw?U%%UGt*E|YP$Qm#~5BSi~KuU#;CQC93>N1)V-$2J4P$|M7fZb@4
zf$N1}85xCBZe4Wq(Tq9kw0bjJmp*uQsM=4o_SqO6zw@Fxhb_iZPTXoe+Oj(@ejqZx
z1~UBegR<b0KW}N9dCO7?!d}7WDs50P6`Mp%0{MJ~qc!x<P{I|dbmRB8i)5-J!9^4*
zNI4ZNOc!SEdBz@BVP_hz-)r*ZQz1LSoROc*)s<XKDC?4Z<~JL?no2BkIH+N2nWFZK
zu(DQOARs*xJMU*(O3IfML|O#*;(maHA^cjYQ6Ll`%HXct!tP1uJ;cWzzgwrSuCVNq
z$vCrPp71#Ro;i;18@ZOe<O#i03Up@fQmB@L#Exf}4symavwi)tro@JHlm1&=w^s}g
zjB10%#KshV<#HlxH9~xIxCQV|tdin$8PIB7civ7-!*o+F-6D8J{rNI!_^4ndfNbfx
z6<#@${yT<?WPohXwuI9x^kAJ2>#Qy$J}|5^_FEipco1XlI0pQh|1}axMc~`pTdYcD
zKdKkk$}MaI;uK;T<-f1t%r#Ttp`H7n67rqXAb83t82`DTGh>;ie^BT!4e$qT9ry2c
z%wWQvQI~mJCpE!HZ;V1K80T!8IJmHQKWayI)Uep&xfE-qdfkSE3ZCr9*B%9JCe3@1
zEIX1%xe(w?QWlcE6Ol=lpENEBFV+Wv1PDQ*8|t>ef@8b*kH9WD@BfQk)13H>o1A+7
zs2N-xJ5pJ@?tUjc4^xd2>dc;AlYygg0^^Mz8W&=5Yf;c1kAvt9*uMr&LU^I*_X%>+
ztx1faiqe)L5De?CT-1?vNc!P<75!20CwXffy6?L@wP-$_k%2hjI8D1H5w_~m+CKpL
zXr`r&?;BI7YyI>T>QG(F%F6>-6h-+xg7t#-j>_F+!-sZp9TP9h=k`3-V>mBi%Ky7~
zE9-{d-%5B<raMe9?iDT)-w_z>#%@VV_h~0PT;`jb%Jm~v6btzd1MQOoAFwnTeGEKc
z(QlP!D9M2`B^#h~s6yBFT+Foz^2HiisdY_gYl=LbhTp=#i4=Ujrnv$?w6$}1ZrF1P
z9R`rNC97~CEWg`5KyS3|r(^8c17OF8e9v>Hr>&qy2IXnkZY0*#*Hn;uziI0}64py%
z6INruBfK}%t^ThxcK0kS6yL262&wOo$aES1q0qf}8ON9ACbBSw`Dz`8CvICC4*lrG
zW4I=*z20sx&C_{3Q!ii41hn#psQ>%wf)AB(gGe=(F&w5B{umk(peYKqJXI&sG{`hc
zxr%GF4E==%v}j)X)7Laq-dQk}<N)-UW1ib{ISB1n@J`!M`D!~H>&_FS^0wRByH|<;
zb%udD<_|PL_{rMWpCwlK;@N<AnPlE12kz_HvfZKG1G0p_D!ZWWNw=rVJR;A7lEAHm
zXHnfGb`m_wZ?Hfdo6adFhLo{yu%F+V+Wj8<BiuyUTQ%+Y^ebwNkzLS$D7=>j9!;;K
z1MjwQU6g#5orH74<NE!FgHw;V4az&iuT6e~3kjtTnAYk<?mJDfWx-K|n-KWPqJaeY
z-C^>#V9A0HQDy<Ld<SpaY(#*<n^ejst)bT6vgRV2BT~M<F5dIDfu8PP2nR;QWCLaC
z)X4FhG-NbeeT<V0VOdHjAhD!w)s~&{5dM98#1A1Ru!ATs0wR1TIxTR&^=yeH@1X{P
zO_DKar6e*af>dGMM&;=nqwK!y3|p$777S`bEW6XIxhsKW7btI30J}<x<O5@e1lS+6
za^$_uBNp=D`9U-KK8H-|96JeY;lm^0Q6t4{eevRegf45=l&<yT{~jUSxT|}@X~<Os
zq7}52@6A4ms6b2L!>cOxlnmFq&aEA=B3x8bk<-oN?*E`rsBKZUDRUIDM=7cRx&l1(
z$hnK&BWiZ5R)BFYOSq8{tR_#0y^~h0n5MRGV;>7qT2~*~8*4oRC!nzbR$5HYul-R%
z8+<|=<(Y2))>u<sOq<YRAH^g&<uuXu@UUMBHwv{R7X{;<67sp4S8<iBEq7HBFqnIV
z%=;a7ov^;7Mx*@ISXk!Ln#=eh!oBbL%n{Tz%#WaT81>a?zZzjC?ab8;whqr-84JMR
zRI&mMy4vm|@d22!fSaCIXPe#iaxHUnREAQtGHT|o1Y`lM*lNX-!71v#Z@0*mS2!#c
zgJ2Mo3UH<9?o%!F;RIN{p#YY*FOBq&Y_29qb%K_+VT++_b|SP;PLuWLSiVLeE)J#1
zmAT)T=-U|N0~ytV%RG=zC~jmo&q4jW9P$Ln76(RoX^%qnl)qFp<g?dpy3*Eem!*oi
z02DS1;K0JhPgKr0yKmX0iGS2TMk^KRO2=w@tVSKrdolwf;4Sp@%>H5fK+lCC6JmPp
zz$z(9bF8Z1Ul8@zy#w5_lNH+h5-OsOkKT$xKOwlGDrMU9u{^c4+{=G+F&+r5QXi19
ze5f=nvg@kV_gDuxe7e7-@G~D&47_G0InG{Q+v#_mz~Y!!A+!vD&X}(y;sctg<lPD$
z{&EYqy~_#^-luUrTdb9a5Lc>c7a+q<FQ_%`uqtu55{hV&6H_^Yw*(cTzdQ>2Xh7Sh
z7~)#`eI5gv8QcvKohHTl$`XsM6P>CMoc&pw6412U=xkHBdXb=6y>Rq0+X~nMqwp?{
z^@7VF`|eI?9uiqLBsIweLtLt~6J5-j?2DZt_UP?6;}BWIoF&G1th@qr#%Qu}o#vWE
zScW8+?Q@Z4K;f^XxT9qcuBu8Y6u82IeiLs75DsCyNia<5;OBNxqmva;aR<}>>3i+%
zPgy~L@tcIaAcjd$9QQ3Fh9>(mdtQGILSmahl+Ej47m0i~tBUJ6BKh0fp?fe_xxr?c
z@D0HGgd@N6Q3EI+3n>V+Q4;med(+v@5GRrxVHhH~KeR7Rc>DbUO@q-5*;Y816xuOi
zWvr0rd@k#fAwOoO--HkoEZT&RggEp{@%DJ4OLcYty!0n}e*0bJQ<IaYdE9fZFZi5)
z*8^`asBD>mGj;p-Wz^9G(vxOKwYouTQ6qQ`nX=RCYA`Cny?j?NSlp<7KFH6VGx8Xg
z9SPqnAX@`m>F@>W-^mZmo~E>1mSgJK)P+17k8lNOEc0MAL<NW+7UvKnqd47E-#=C>
z`T4pcwx$j-{P1<isnRF{_gOq(D1+L9oG7#iOF2M2zH!>sNaL=giuMual9=>S-WjPN
zI>=Z?&8vG7+qgewai!_NEpuzr+#KuXP&c)3b#bRs1a*pxA^d2)t!#C`i^R@Bo31+Q
z<&M~XZUS;Hu|@b#FN*nX^I5GliCg0ra^o@NP~%R3eNATPB=4oj^AI;eB|$RX=OSs;
z?TmqVM)FgzXeRMWftcdBWQXW=VSOT^!mIuzt4JfwHi~`4`H1K;`{uRwkEqXGE%SmU
z`lz!BxVS)|F&F$y_i;urZ-2_68**;UTJ%y!qsZ86qmmkK|54FZCp(c<mfbMQm}^bg
zT#dTC97b!2Lc*p&%#=iiEhI^uG<!*+s}8q`cRAhxonq`4RiAbZ!vi?bNh5y#)-9<E
zLOJZ`C{mlwEgP*$^K+fAm$bIE++bX532|_j?Uzoi<Yq1UI`(K-vKvVbNE`kE8bj_=
zu>lOJCIXXjw5ku@z-K!^h8Pt1h@4Tgx^tM$DHEE=`g2_5>1I}RVl)G$Dus%?zHVN&
zaa4Dfi-6$BchwtsL+^Fzqz^SxTQPgk44$LW1rYI+mnC6O>*u_u_1TSRmX;6Z=~OLA
zVn7|jGUs<dVpfKRESb`TT)!!|H&tY`PXFKYwo|w5v%7biS_bb<DrdZOq_2m_WMcsR
z7K0<ukQ99#tspG{<*lh0$0OLS@zVezXG)k7lPMl?rHs+Uv`{k{uBTSM>DbJv*?0Qr
zEG~)h$aud?Y5gy<PeDZn-)khQ#p>0`CR&Z{Jv`Fol;7_<p{U%>v^hJ0`u0TzqnQU*
zCI;9<zhBqV@x+SW%EG>wejoz5VJrz3U_Gx~`~*isQ<a;@BdEy8a9R4MFgD`_PAr>=
z6{KGJBsSv1-+{}?<SuYk0o6+?aBRgNag(+?@v?1>q2hQ3<e45u9xmrUBlv!1d+zvu
zjF^Z_o%_C)1>p}dPlhQ3(;%%OPvo7lZc#thZ9-E!dC-o84OX2FgSgEf>pgp6i)U-t
zE=LJEJw>;9u7>#_3YtSHckkMnsz)?fzq4a(d7iXs7r?D;<Y$2KErD}9H#=T+_sS_R
zJ2L%6^xNw#gtN?5^x8CLesVm3@owB(|8;_*x}9$Wxir`Q5qDy}fS}A*WkqKo28)&?
ze=C)uuZj4DmdwTBsb)Co5f?kudlzwJ;YUDFWO5cY0a^%bZN4A&`k|>@W9&-MIWwd8
zZC-87lQ8U1>P)_SUHLpwZ^ZE`ZoQ^E4k-Gj^DS4Sn77o{D{h!n_Uk{{(syHyJ$6<b
z!@}IO_?v#^sWf}OE@}5vL%LM`6MAPVA(Y0ndQVrGU0eD?ZWh`~Lss0GQnX!^n*hor
zzy~?8pVq}_R1I{GdbMI?OwI#<2Z?!)QZEl(v?8pMGI$`N%mlZcx9f|Nm%y|oAU+eX
zzBMdCl3vF3ZgFU>d52V3p?W$$=An&PR!7kM4N`X}86L4YUbO8ZXNHjbp2yILrX6W8
zjm=Y^xtaEg$;2z<|D#LZP3XU+(K)B`U0CB5Rvv864dWPOmBt9#!@r5NgLB@<(M*0j
z|2L|vwMsxZPA~n`4W>$KD@sF6Gls$L*g6Xx?AX&f@|lB_`3<nf)LPq~n+Ew1uC8>R
z<L6bswJGOBt&R4VM$>hF9j>No;!BgQ82&O3-_k0kCSxU1>1+d7=hV>73^D#2(DRhB
zK1oXEZp?}XNl<93T1V1<I6*+Q#GoLmjC$p+qz4}P*Zto+5r`^YUrjey>KmdxTFrvd
z_rSc1o8y}YCgEWOe0ygYCO2SFX{OwC*5ID{Ln44k-2SQBe3L#*>j;+WB&nM)-d%T3
zAix^Z`D6x#G)aL;hrB7ok3T;XP1;McJcY4*j+^SO0KAdRlyw-IXS<rTOFk00R=G$-
z0Rj76qMo+#yxFc6pag|~@jJdiouU(|vAf^Jj`g@`wx|IBK#F8~Fo<}T(9|cX<oMG2
zEKIY#n*fp&-8-;oJ8^!QCX|PS!3^xHU@E9;wq_<nt*q7qBB-*x+${yJ@q9&y6WfS)
zWmX&Z${c>ZKV(NGo3fFDwTPpK^Dv1G|2y4}NKHVeh99e{HTvB<m-C-}PY5&WNtA@9
zq+Q)MnX+k(&%u~iFcYiOuQYL%OC+J%&_d4&ELP?*Ua#=wPgh#UA`0&&k#=kr8oy|;
zk?UZO*qrVi8aa*9*0H}P?&b`DvM|uX!f7ezHSm9f#phv9=N{X$a7r?gmn}rdLrZF(
z^qw9Pm>kbZNN|ra%dg7x!VF>o**zd`rJ=y@&sO1HC$0ez8B>chlUJI$q3_=|+iY3i
zSJrjw4yO6#rkY_%Vz96Q6DO|#iVIRvF%ujosI)S6lTarDKnGL6yK5{=^W9T*50-2s
zfijj-;Lpf(X@YVUOinPklJ6^_RC0r(F-$(D3HaMMV>hp6=L6cuk#-^31aMr_=%Lu%
z2%H+eDGtJ7BFx^upG}sY{Xl$%^`t4;-<8<VNs-}_+ET+CfK^(K!c3S7-^MZ5vDW5S
z9O^A1=@}RPPad_bIwM2(>u#{f?P7HH1Z#H&L7Yk7)1RH$y3u=;!bbZvj?jY_s4HLn
zsdh*|6=laE$?T$7hFyG1MdrKw*YW>$a+;rHS9#b#MjW7&P#`Cn<v#8&(=r77-0c9o
z7iGS{3+>lpow%Ujl29q;G0i<>luPCF6j?6W-oDr|9*z|bHCe{stQP8xwaFf-ERCf$
z#cwUeyYV(+(w?AVEUYokzk4fp!j1u_Q)GtCrb1<BL6fE<qi*9ixDQg9e60BD(}LDz
zoDAYV$N`puDXrGJXM=$pGA!<VgZJ&BQLT=*SG^fzXuaef>zQC@`o$v(`LM9<=rh%X
zaPy!XH{8C5^SSuM=f0KcT9r=1KR3;yd-)$nCxw~gEMf&Hk9tl7Oz)#h%8y__x8q<y
z7Kh*(OL){Wr->F}Q&rTv>#2I-&K)j;m^;C2v{Y@;lgDU7E(u4WH@OTE8F#L6N26&$
z@`1koIky<*;43RTVUjgO`O+C~zdwava+;l8G(({IGzv*sEp0Bpr1#l7Ed#F1LfCf`
zsC9+NGkDFD8I9#Uf)t9zC!NW>3~{~Rs1L7Rb>Ur@*ihz}!OtoW9`9sadE{JL<wP5Q
z7EVd}@I}FN1wVyWt|ZjMRF)AI8%K;J19~NJBZfvqwL4%0*R!(y_kS6Pf!pc=^;91T
zyj<Vf<M3K@9+>N@%*6Rxp{vzt-6>b+1j17x7wB~{aW!Cwd-n#hiQWvq2=0ymS1Ch+
zUdOzT$6$5kMhGGlp-pq@fpMVPwxW(fX`Z~B??b$Q0H-jKXL_tjFBiw6@glrI5o-5R
zBXLLg(>#k6;*Xz|9dm)yhFoMMm&w*~PZUJoVMYx{fqhNm7350HSynI)m{edtsD`eg
z3snFH8SRO9u=7Mj{mYY-zf;*hSUV%LI4@j6njzRpB!8jnNd@OcY5YxaUunCOx1fM$
zT1P1UD5%tT^1L(QE)YH`Pp`wz5|D-HD$4dNk`%BHy+;WchH2wW252EJuo=(deD9Zj
zadm_<wKkzRiDLK96?)vvlFTdGd?*|s?&*Hd@E45IdMwi%+S7$!mb3YnVK!hBW*hhq
zq)-6kGnz1cuu`=FOb0plK&O$N!|{wL$QXB|0je&llZNtp%R_VJ^R{*4;MfRnNt9w7
zOR(f7myI0RZ+*VKQx}5FNK&u@OBX-sj$~BS5=`p;gIMpUo7Obkt+Ny}k7)9gsbKH|
zPGl0_wep|j)u#9FD{L+j=u+E7ph3Oh=FXdwhgFs{CG!@0UzeB}Y2jd|Nh_|)dO}i4
z0dox#m!=Y>>h$7twmdU|F-$-O96ZmyFZD=U#wUGF6@8?=N=1xL&CNP~q}FV;CCkaN
zX|AYLp?b-`C2d3$)<uvF!%@$m{M&qtR@`k5v-x_Q4=~Xo983=IXbb<Ua81LVP*J`K
ztg?~&9}$;Rv8Awj?nTxFH(;GiB0m$SRgs15$<~J#+<@Mws;>?MoQkB2r$oZAsej?&
z`Uu)v<O(E$-?rE7`9XX4=7`5~PUbO_N`)@}JPJdNvpAFf@!FSsQu1pCeiO5$6%s;4
zX~PDjy#mI|-z-h0he@hC8ASI@joMURugMnP4)QwRLCfSHN+M<3)}w!^DWd(VOjXoh
zt0c91JsXZQd5vTF>_^fR6rPW{8P`YCmK0JlKtjMCr!&R5k25!NeQ*}LWEzb_>I$>z
zHtiY9UC&maw?ikFGSI|}{7}kt?s;T!a;~AtAuYF{vEUcCnmvgT$}<Dq2*Ea47fv2-
z&1&>`;v~@45#q7M^u4tvq<9XN)Rer*5YITB9##?ao<%B@X(Tr+ff|7nwU%Xe<e>nd
zoBsfq&$`_New3jHn|^l?6w&|>dmT=0j<mw2G}>fTojLzSy0^bgM^i3?QUe^+6vr(D
z-O(cn@fD8iK<^+PR+eWN?M}DBa1ysRZeWm564cLlUx&_x{G|?VKs*muihcT_E^SPb
z$!0k@KF<@?k3}`!$8Y`y@>0C$*$qRaJpc?STgqcTfg;Mjp1Y9;53a$;WG5zz4Iltl
zjWllV)OTw+3~eg~h4GHR@&4kZ2Qnv7to$376b#yRBQ~iXzRM7&^b-I3mn^Au$EpTO
z=;HvuCb2%NS^6DNAAH5AR=1t|AqIB(oWaTh2%zg7MqTl!2x)A3hZb9z2sN{}_z)T;
zR?;UPdM+|~$(^TPIQOf>4FPbHtrlrm)bjMu{bDJMrGtX3yP%+dzvK~)b<~|_uq8Q7
zh;I5yX{}pCE^*GwA58=xqP|+cl;0gUNn?G3XdF5Ws^6rdcn08=ZI!=Yf>|RZ2R0Yk
z^O(_Y_2D`t4guph`6F)moOr3N71zRylQ{rnAYc+=LGEW)`Np!8rWRBeo7iY3N`lzv
zglC1drZ6NZd&YW~`_-5|w`1G}cP(cqH9x;@rWjSOacPi0T0dUVW}kML+tJyk<xCf5
z*UVr|L{k^9SI>dmFmEGkX-)qlfWC7aXvc~_MW&^T{X9e`u_@^`Z3imXbbq>}3-R+O
zo3y<~aW1T}xs@b(>r$RcfBrz4UF%_!iD1-_@><6no3qvVw;Sp+x2`NY#xkBr&aqFW
zK4eel$|}MM!W@-}d?b<CD*Es}tSz!Jy&DCzBo05IqcXIm(Kvcgs$FxT%#-GA{L}ph
zZd2~?Eu7C9QW^H9ydWXfh$)pIO$};#Uvg$yBJm+@7gu7j((>t);CkZkXmEs-;P%9K
zE+C)d`SCK)T;G2`%V-U2iWfJ;*)+1XC29H2Fqwlv2A2tb`NryJlCgNcBhc{9Ob-Ho
zr1Lp(smZh4FjC(B!()-~&eW*Kmc(ocM#ty;5Fyy0g_xH0@RZ}D+yzgHS7W~L6TGkl
z@y0l+r=VQ;*)ZZZ#&*l)UBKm-WE^ZrtFF+*B5?WR8j0937T8vsB*57VI`^Q90jnE)
z1KgfjG^TbY9shE(C0A;W04%D3Ew+tzS06v{#N^liXBe7A^ju?>6tfzPOLExD0LdRM
zHBubW0gs9TrG1y51~%;Gq|_0)Nh|;pQ}3XtzZR<%vJx4$48P^MFV!xmo6Uu%BzF#g
z1=wTsXXiS~=`11uo_{v<9+)0mP4f?>3$at=maiAYc0_lPcVDbJxVt$oOP?9U)#c94
z*))_b)NSA6DU#OGE;{F%L000YCI7#f!TObuMGNV+K9e6+6sPSRCZsHu1dCLg)(>W>
zO5@aRB<zSOQCpX^kg@V@yZWNNbjE6Q56cUBcr1MR?afEIF%tN!8AERJ^WHu2n>`Ug
z%q}OC6z*w$yBhb0%>o;Ra*)MzPNX*`UQ8S7a|_=f-_2nYyYQWiM$@oxorfZga?e_O
zq~oNHna3yohytDccchz~1;+TC%bx!Bi%%Raqu{68;u#(HBK9y_ZSjhHuAqSfoV_(a
zW_J{P8^5E^)TuQ;k1rUWPJ5Kix`>!lSW|xTvvC8ngLqfF2QoS}=)a-~iMMiMUvzxy
z4QYfgqvbDPB5iYW8VD>nOZvG4<1KXxLop+w@FM;^1<8t>VoF-OCT}yGigAH^($rE;
zxQ++9Pata~vdxYi*QpA;b{)X)0%Uwbk3(6vi|)dcGpQ1MGw%~Q$^hFTaDPB;j#2P!
z1vOD3zSUYBT#F-m%nAG8;=68gorRf0^`5#KsDXSW=*ssq-d7?gO%@IPZ-~?(aK&=!
z=o6&64{l8DenC1pffRYyk-*@9h^|W(*SY?Mb*4%{ft`x9#5|lQsnY!i7SRhZ_a$)8
z00`Usw7+)Q5NMdpSf)WOR)gQZu&Hg5=xZ3jez<loWB1+qVwIs_b8#hrxO@PzCb^T;
z=GvsG6$H{#11dn;cu{amp@n#dS`Y;`M8cdHT?;btNl+0qL5xxO1ZukDq`5J$H>xwy
z^z>+dodghm@Ma$wS4un{wBd1vLPb!I$r|etart}5#7oHJuC<uA;b5IZbzqIG3ge!3
zbqr6)B|9cNS>US}8vdZpvp$tLBRm9LPBzmpwiOrf7L@h1^Ww?6kOdX1Ni%-ACDc7d
zC!pxn{y*<mreYsLpPHAU=qglb;)bV!<wp_u7tqlHa)4-K>$B&Z3|{oKr$Oj<j+>r{
z;AN5tX1E!HA*ys|<4m?rta}Ws=7uV~2-=>n0O`|VPrLLD<X8EvDZb7f#=C&Qxo`g8
zh0UXmy__R+<FW|+VZ}T!>!B(fEOqCre431+zh|U&3)_K`Pvkqu0#6u8zH+N-%yn~=
z-(}&Ly|;jPEBp&Ce0*Lp9xyDy0#9)cqws^}0A$XJW-&?TRdk;<sEVcGiycnsrwU!l
zKPf;9lMd0Xv~Vn;o1M&Fo*hikv&Rw(7_o=NhN?b)fvm%;nK}mvK@~pIi7<}<5|fq1
z;GE#5@VDKqZyWiPI<tcRZtpRXyA}g4**sO}M*FYh5sijV5E*GH2LOMD^cnC`rE?}{
zMP5LN_(34aIo4o$GYG;V&-R-?iu3$?uEubx9<V;rq6VU4M5YRD$L;VEb}^QL!y?w_
zPFlu|d`|sm>DteqeKVG7(FXPV=)76F!eODXP<QzitR^^D$i>bc63*iC_;E3PYFivz
zq$mQ9+vB%CyzPc#4qnwBx(u;R1pK{l81FvK@zo`J<bQids{vW7R1wd*)-A((`>8j6
z&WFgMWLP%SgCT)sRELGnRrQw*enoVsQ)hNRlJ}iDzHtvUcON}*_xKYt4L{l;4Vd(u
zZGU_^B8|&p>d$yP#+#A$ZjXg`aF6gKpWJce%ne&=D-#480HVdOI%jr=rni#CEG~Ci
zqO=ef1XC0(#JqcM33^-FCfU=9%q4Q^dKV8izp5;=QcSqrWS=Jl?Wmm|itgVvWs1ba
zBOiU-K9X{a>1Cya*+mBabDPwRY}24MGEZ!!kf*-ahocua0^|9HWVdgb0wwmk^2ws8
z;jI1<BCRnEzQgb>f4YA{7tn8jmD|lPol=oOJhn5T{gh>Zhfj3>Cyxto^!8&zb!D{0
zn0LzGV^>q$00JN}j3hAuqKIsPvFKOXwR&#wscGK$pDp_IsG2GZhWqMe6e@4%7oM#t
z#eg+SXB+~Ye3i|xFj5pAnnDEtM2`)gQ~0pgIkBMmFwvuQX@Szd_7|gQ5_0u6nd(-O
zLm7C^c_tqsR<Wucl`sxW39bu!fg3<sDTlB_V*twA)#*)D;)^|!fW1r{Icz*>kQp89
zQvcjQ587bf3lh{XtV=*t&drwhPN}FwKA_-Jxm7Zt6k4obade5mMy%O5SqcaXBOPKM
z!%~S*Qo8%NEsU;mr;79xCRI$UyDY#_h$#Bl;#w2LW=I~3CS)@N^_;7O1OTY(&jtzw
zO8?`Zde-L5+3$2+3v;Ig3c~T#u#wX+vM+p<sW0wzd{Km$jlS7b#laPvD}oF-#}5kC
zOO@7qcHQZgGhbnZP?2dIxT>^aGGQq;j~K{@A@cd&`q4Gs^2_~-J^HE69%)&{IYIzT
zi{wkgs0>B^c`GIhC_5+cVvOfl2@by#O-&1{r_Ue%$d`t710<H$^E7nO0x65rNB`#I
ziT7s@F4p%P8(VI!s$H0R`pcRy;9+zBry}IP1E&EiG=`z~DelxIGaT^MkwN!I*pFur
zR@#R?Mf25=FQMTn{Cg)}>)b2~V4-nD)x}GcbGn{jFXGq!{f9xvD4@vu@l2Epo-V}A
zUkUUk@FTE{L(501<{JoPcg>pnlK~3sX6!0O1L^avMsGcsVV^d%EEDiXTdBKm$g?xM
zjugIla`~&hAzkIZEYWNtF5G=N>9?Zu*iq1AWF$ETX*!iL`h2}ouvWBeM_fZz2Ky~z
zf-Z7aiFfzXgSy<=&6ZWnD1G*W-$?_xd+|6+tmOqX#d`BrQ_@!)3DS5`woj=qTFIp0
z8>)y9x9Z!532{Y7yA%>$1$v_6#OjcZl|6P@HOLHMs_<w4|2=I*TKXUH<2g9;9H+q?
z;)sZS3un>0p=4<U5;n-{Bh@VCb*fuq%;AHtb@#FAvJbRRoF=OlAbLhmmfWkddN{R*
zAWyPu$vA)Bb(e<T+T5SBluhAWjo19@$g=x{jTS}+-3LJO^wGcrN7ih$5^{3*S-4bC
zn~`u}sCdKU)t?WOvEU;V_%PG0aS1~eypJ<a3tYrOc!P^Iu;c<$&o_rFSmBdIxkpRS
zoG(-7BdM#Fb&V{VZJ%fGc-|i8T>x}fgaRe%2^?OTU{EeDIsDw))Af-7$g4Xl$3Til
znQ8UZm$o1|v>(aC9x|yY5Cu+cIk7Zt&*_$paaa8Rc8wg)l<TPx1^1PxJKhjg<<~t>
zlX+o1a<J`=BWxs*;TG{0VW#R4r?JD%)lQ_6+wX}#_!}b8;Pr^&@tb%n8kPwSsxisj
zQ1w~M@7_qg8PO7=+WVUTC+>!E5t?aDoegAV1>IM8&ZpZ3L~|r7_!p4;9M@2>7|0VA
z%Z6q`CSsOmP|kL%jx%q~mLr{QwByo$7`Ny^a$I}PQ&b(=7a!~#XS}N$Y(dQl7`YnF
z^5ZW${@)rf+e4(<Kme*QGt8f?3ke4z9r*>+wZ~EnT>ZvxKuH;MNekhZL4UyRNVAtv
zqrWFCRA_<C*H5oAC~9JrRS>vU{viYAidYZcq_vD!oOoZgK&CIAgk^~zF#j4m-bIsC
z1f8u4@ScNBD<C}tz?(B#DJ4EqkImE%!)DoX99PNa=QF(7;uX+JF|?<UW{P@M`sf*(
z;5l3CL}9<2_zsop3kyNTQ2K&Cwk}HR$At7l`3R4DT+PQt(9{}#*E_Wm`C~~k{Zf0(
zo1RBmoY0S(VGGRaJ)kHOvOM;XDHXv^!JrXCP<45vMoJ1}wY6xV5`E7-b|LRSHcC<Z
z3zYf82o;P#i*>g8uhiyx!D;4#K?Jgoi*WRo(WxG&0|zG;`?aB$j{kh7<U2a@nQoL8
zB5tJM9}@q<vvirJtWPGm1xCMyu2CDWEiif&xeO?fB?BDR_8F{h9?FiJnD^;<&o1uY
zclwot8wF<~M%JEFLh=k7z;xF~4aVm$mo3Lgz9%Mx*R}esA{<hcE(dMg92Cw6Scy_M
z%kZJ<K@(fH_c}@F#HfF9gL0!@3er*WJY_F>+pyp4M!8qi`+m2~IL5srfO{^OtFAWC
zxfq9DjNyfB*IBer2RnipXcY(?Sjr0maM5#MDeA1qU7PP5DSq>FFQks08qB~ZTHqm-
z(`@jBhU1}C_Il=7#8j7Np@9J&{jdzz%-3P&sUzH<N2eGIh@i2*kSczmlU+f(YDYUT
zg=lNiv9bW7mU~)6QWd`NZyMtF$4<`AHKA2l?(Rysi<u$1$7k>P((n<&#*UqvOa)Zv
zerhhwU0__iOD?77H@@$O#V!cOu{HR}=yta8PY-~o=<{eM(y*EeBXtCjcqVnShVC<F
zEM5wC<PNBNSg`5L=|l;oD9MA}(UPi#(#hgrr4YId=mDapD027~A$pd<O$TjjWyv2H
z$X``Fy&;U;G04>*E0XulC~UMrj2%#Yw?w+Hcn=9iB}})Zm*o#gbDUEY`WtmOPkirK
z{VC$+cg&{#bbO#c^y+62lfpzJDcd;n|GDU|MVr6$bt;R^>>}nV3Svk$gdmyFOdyYh
zF|@VSG*A9Vy!=q)3j^L;Pm}l&ld|@uE%DO#K47?z<|;>F?@0dW{kOlqi36w&7Qob2
z4GUC>jjjg+-sd#sj?2`8ug-lA&Mlr3`-WCfE6Cm0pR6*kj7VVQ7a;4H*%QmofYtOy
z+hpEzy8S{oHAbKqwR?YqNmMVVZ9#*(!yTqzkr)4eBpb(b8s#SQK;U}cQP{)}2Ft}I
zk=J*rd(z?lPje9&>+y1kP>x!|_F`1P^%aGzctt<v$n>g<r~P-crqplCSl3ou2QCaT
zuu!>W?P04f2fDebvOEeTd*JA|??=vk|22ICC7i~Z!e`FIQJQCZOE#JC6@S=d+>&*7
z7MS5ET3o*z4f2zk*r!*GgByu9|E|3GyUHr<Krd{iem+|l-gu2`ES>-@a^vk0%fY9l
zrU=3D9b48Z@C)v>a@7)y>$e~LCjEW%YQtF0XioHEa0XX28zFXT)Z72BuKo%0?rDSR
zn=HCROutSJdWr!dty>P}fwN;><h73Ziq_BEkWPTd8ZL3W!#_J82DRfr`M|o=?;-J2
z3HWcDB#~7n4A>B+#_&~Doi~ii@r-9+{JSKEK<?_j+esfL=kvn6uaOJ^d2{U|lHTyh
z$S1FIo&x|FM#x;N+ErbLJ;Cija4AWVwWlGPv+BsOp$r96)Pwm}aGQ%`0VEkT4}-u(
zyRR+jjIv&j{2wkF$pN{xb*2X#U@&)>9!Y<DNc3;-)SNXBx3_2>R=CH6*%^%EE8{w>
z0@@(NFTx?Ss-_!kRJt~JUUWm66+Dh@4h(62F;5v^paz;2Gh@Q~j<d%AEa+=J@m$$y
z*$c9z42k?v_X-BdvvU2sLuDupEEmOpKT<$tClDXep#JZqtMM#^p)fY<I=u+EDaLY@
z)Ircm;9_kT;8&tGr?6G-B}+(uSnA!60yf=}7`8$;c8j8VVn=>MxdHg^a$W!&Le*^u
z$<b&4i+z~&dr>zCy?L;`PnmFEg#GNAIJF!`yHvy&`XQpiT}m>#OzrN&OY6PzI{dE6
zx|#D-k4wGJn(e-Vnjd)MBH}joKX(z<nsciv(9*mmg|JMOHAv7yE$tntiu<Nl)}+tP
zMv$H5?kKiScwiWIi5fWkb8-d8Wq&t8iO_SJqT)s@6r`No%1iq|5yc{C!w64<BC&L7
zdnvn&i>&1E$y(+{4!)QxKOTTZHikbq-SmB*(rJ~M>B}F0n}MUz5((3>G_Ezs5I4KX
r#JL|s@P~c4O9kf0%+nj?#7C?fjflWJ+Ypa~-^acHzCU;P0)vN(H;UP=

literal 0
HcmV?d00001

--
2.28.0.windows.1


Re: [edk2-platforms][PATCH V2 10/11] Platform/Sgi: Add SMBIOS Type19 Table

Sami Mujawar
 

Hi Pranav,

Please find my comments inline marked [SAMI].

Some comments in previous patches apply here as well and are not mentioned.

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 19 table (Memory Array Mapped Addr) that includes
information about the address mapping for a Physical Memory Array.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 6 ++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c | 92 ++++++++++++++++++++
4 files changed, 100 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 9061c491d461..f81494114188 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -22,6 +22,7 @@
Type7CacheInformation.c
Type16PhysicalMemoryArray.c
Type17MemoryDevice.c
+ Type19MemoryArrayMappedAddress.c
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 5413982e233b..c6dd72cb6b99 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -53,6 +53,12 @@ InstallMemoryDevice (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallMemoryArrayMappedAddress (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_ENCLOSURE = 0x1000,
SMBIOS_HANDLE_CLUSTER1,
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 223bf1d114e4..d5d1e6393184 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -33,6 +33,7 @@ ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallCacheInformation,
&InstallPhysicalMemoryArray,
&InstallMemoryDevice,
+ &InstallMemoryArrayMappedAddress,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c
new file mode 100644
index 000000000000..de458ab29e68
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type19MemoryArrayMappedAddress.c
@@ -0,0 +1,92 @@
+/** @file
+ SMBIOS Type 19 (Memory Array Mapped Address) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 19 (Memory Array Mapped Address) table for Arm's
+ Reference Design platforms. It includes information about the address mapping
+ for a Physical Memory Array.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.20
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SmbiosPlatformDxe.h"
+
+#define TYPE19_STRINGS \
+ "\0" /* Null string */
+
+/* SMBIOS Type19 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType19 {
+ SMBIOS_TABLE_TYPE19 Base;
+ UINT8 Strings[sizeof (TYPE19_STRINGS)];
+};
+#pragma pack()
+
+/* Memory Array Mapped Address */
+static struct ArmRdSmbiosType19 mArmRdSmbiosType19 = {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_ARRAY_MAPPED_ADDRESS, // Type 19
+ sizeof (SMBIOS_TABLE_TYPE19), // Length
+ SMBIOS_HANDLE_PI_RESERVED, // Assign an unused handle number
+ },
+ 0, // Starting address
+ 0, // Ending address
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Memory array handle
+ 1 // Partition width
+ },
+ // Text strings (unformatted area)
+ TYPE19_STRINGS
+};
+
+/**
+ Install SMBIOS memory array mapped address table
+
+ Install the SMBIOS memory array mapped address (type 19) table for RD
+ platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallMemoryArrayMappedAddress (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType19)->Handle;
+
+ mArmRdSmbiosType19.Base.StartingAddress = PcdGet64 (PcdSystemMemoryBase) / SIZE_1KB;
+ mArmRdSmbiosType19.Base.EndingAddress = ((PcdGet64 (PcdSystemMemoryBase) +
+ (PcdGet64 (PcdSystemMemorySize) + SIZE_16MB)) / // 16MB Trusted DRAM
+ SIZE_1KB) - 1;
[SAMI] Same comment as for patch 9/11. Can you atleast add some validation to ensure that the encoding would be correct, please?
[/SAMI]
+
+ /* Install type 19 table */
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType19
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type19 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}


Re: [edk2-platforms][PATCH V2 06/11] Platform/Sgi: Add SMBIOS Type4 Table

Sami Mujawar
 

Hi Pranav,

Please find my comments inline marked [SAMI].

Some comments in previous patches apply here as well and are not mentioned.

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar


On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 4 table (Processor Information) that includes
information about manufacture, family, processor id, maximum operating
frequency, and other information related to the processor.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 6 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 12 ++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type4ProcessorInformation.c | 209 ++++++++++++++++++++
5 files changed, 229 insertions(+)

diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
index a0f217f5107c..091de0c99c74 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
@@ -56,6 +56,7 @@
HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+ PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
[LibraryClasses.common.DXE_DRIVER]
FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index b3c1619ddc66..4652a9c62b88 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -18,6 +18,7 @@
Type0BiosInformation.c
Type1SystemInformation.c
Type3SystemEnclosure.c
+ Type4ProcessorInformation.c
[Packages]
ArmPkg/ArmPkg.dec
@@ -27,9 +28,11 @@
Platform/ARM/SgiPkg/SgiPlatform.dec
[LibraryClasses]
+ ArmLib
ArmPlatformLib
DebugLib
HobLib
+ PrintLib
UefiDriverEntryPoint
[Guids]
@@ -37,6 +40,9 @@
gArmSgiPlatformIdDescriptorGuid
[FixedPcd]
+ gArmPlatformTokenSpaceGuid.PcdClusterCount
+ gArmPlatformTokenSpaceGuid.PcdCoreCount
+ gArmSgiTokenSpaceGuid.PcdChipCount
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
[Protocols]
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 4a6f8be2a2c2..8a9be0cfc4c8 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -29,8 +29,20 @@ InstallSystemEnclosure (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallProcessorInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_ENCLOSURE = 0x1000,
+ SMBIOS_HANDLE_CLUSTER1,
+ SMBIOS_HANDLE_L1I_CACHE,
+ SMBIOS_HANDLE_L1D_CACHE,
+ SMBIOS_HANDLE_L2_CACHE,
+ SMBIOS_HANDLE_L3_CACHE,
+ SMBIOS_HANDLE_L4_CACHE,
};
#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 5f4b833dc9fe..269bd0f9d843 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -29,6 +29,7 @@ ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallBiosInformation,
&InstallSystemInformation,
&InstallSystemEnclosure,
+ &InstallProcessorInformation,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type4ProcessorInformation.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type4ProcessorInformation.c
new file mode 100644
index 000000000000..fda532b558af
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type4ProcessorInformation.c
@@ -0,0 +1,209 @@
+/** @file
+ SMBIOS Type 4 (Processor information) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 4 (Processor information) Table for Arm's
+ Reference Design platforms. It includes information about manufacture,
+ family, processor id, maximum operating frequency, and other information
+ related to the processor.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.5
+**/
+
+#include <Library/ArmLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PrintLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SgiPlatform.h"
+#include "SmbiosPlatformDxe.h"
+
+#define NEOVERSE_E1_THREADS_PER_CORE 2
+
+#define SOCKET_TYPE_BASE 3
+#define SOCKET_TYPE_NUM 1
+#define PROCESSOR_VERSION_BASE (SOCKET_TYPE_BASE + SOCKET_TYPE_NUM)
+#define PROCESSOR_VERSION_NUM 8
+#define SERIAL_NUMBER_BASE (PROCESSOR_VERSION_BASE + PROCESSOR_VERSION_NUM)
+#define TYPE4_STRINGS \
+ "0x000\0" /* Part Number */ \
+ "ARM LTD\0" /* manufactuer */ \
+ "Other\0" /* socket type */ \
+ "Unknown\0" /* Processor Version */ \
+ "Cortex-A75\0" \
+ "Neoverse-N1\0" \
+ "Neoverse-N1\0" \
+ "Neoverse-E1\0" \
+ "Neoverse-V1\0" \
+ "Neoverse-V1\0" \
+ "Neoverse-N2\0" \
+ "000-0\0" /* Serial number */ \
+ "783-3\0" \
+ "786-1\0" \
+ "786-1\0" \
+ "786-2\0" \
+ "78A-1\0" \
+ "78A-2\0" \
+ "7B7-1\0"
+
+/* SMBIOS Type4 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType4 {
+ SMBIOS_TABLE_TYPE4 Base;
+ UINT8 Strings[sizeof (TYPE4_STRINGS)];
+} ARM_TYPE4;
+#pragma pack()
+
+/* Processor information */
+static struct ArmRdSmbiosType4 mArmRdSmbiosType4 = {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_PROCESSOR_INFORMATION, // Type 4
+ sizeof (SMBIOS_TABLE_TYPE4), // Length
+ SMBIOS_HANDLE_CLUSTER1, // handle number
+ },
+ SOCKET_TYPE_BASE, // Socket type
+ CentralProcessor, // Processor type
+ ProcessorFamilyIndicatorFamily2,
+ // Use Processor Family 2 field
+ 2, // Manufacturer string number
+ {{0}, {0}}, // Processor id, update dynamically
+ PROCESSOR_VERSION_BASE, // Processor version, update dynamically
+ {0, 0, 0, 0, 0, 1}, // Non legacy mode for processor voltage
+ 0, // External clock frequency unknown
+ 2600, // Max speed in MHz
+ 2600, // Current speed in MHz
+ ( // Status
+ (1 << 6) | // CPU Socket Populated
+ (1 << 0) // CPU Enabled
+ ),
+ ProcessorUpgradeOther, // Processor Upgrade
+ SMBIOS_HANDLE_L1I_CACHE, // L1 Cache handle
+ SMBIOS_HANDLE_L2_CACHE, // L2 Cache handle
+ SMBIOS_HANDLE_L3_CACHE, // L3 Cache handle
+ 0, // Processor serial number not set
+ 0, // Processor asset tag not set
+ 1, // Part number, update dynamically
+ 0, // Core count, update dynamically
+ 0, // Enabled core count, update dynamically
+ 0, // Thread per socket count
+ ( // Processor characteristics
+ (1 << 2) | // 64-bit Capable
+ (1 << 3) | // Multi-Core
+ (1 << 5) | // Execute Protection
+ (1 << 6) | // Enhanced Virtualization
+ (1 << 7) // Power/Performance Control
+ ),
+ ProcessorFamilyARM // Processor Family 2
+ },
+ TYPE4_STRINGS
+};
+
+/**
+ Update the part-number string.
+
+ Get the part number from ProcessorId and update TYPE4_STRINGS
+
+ @param ProcessorId The processor Id read from MIDR register
+**/
+STATIC
+VOID
+UpdatePartNumber (
+ IN UINT64 ProcessorId
+ )
+{
+ CHAR8 PartNumber[4] = {0};
+ UINT16 PartNum;
+
+ PartNum = (UINT16)((ProcessorId >> 4) & 0xFFF);
+
+ /* Convert 3 digit hexadecimal partnumber to ASCII and update TYPE4_STRINGS */
+ AsciiSPrint(PartNumber, sizeof (PartNumber), "%03x", PartNum);
[SAMI] Space needed between AsciiSPrintand (.
+ CopyMem (&mArmRdSmbiosType4.Strings[2], PartNumber, sizeof (PartNumber));
[SAMI] Is the index for the string correct? Should it be 0?
Also can you check if the output works as expected? Can you send the smbiosview dump for this table, please?
[/SAMI]
+}
+
+/**
+ Install SMBIOS Processor information Table
+
+ Install the SMBIOS Processor information (type 4) table for Arm's Reference
+ Design platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_NOT_FOUND Unknown product id.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallProcessorInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+ UINT32 CoreCount;
+ UINT64 *ProcessorId = (UINT64 *)&(mArmRdSmbiosType4.Base.ProcessorId);
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType4)->Handle;
+ CoreCount = (FixedPcdGet32 (PcdCoreCount) * FixedPcdGet32 (PcdClusterCount));
+
+ /* Set the core count and processor speed for all platforms */
+ switch (SgiGetProductId ()) {
+ case Sgi575:
+ case RdN1Edge:
+ case RdV1:
+ mArmRdSmbiosType4.Base.CoreCount = CoreCount;
+ mArmRdSmbiosType4.Base.EnabledCoreCount = CoreCount;
+ mArmRdSmbiosType4.Base.ThreadCount = CoreCount;
+ break;
+ case RdN2:
+ mArmRdSmbiosType4.Base.CoreCount = CoreCount;
+ mArmRdSmbiosType4.Base.EnabledCoreCount = CoreCount;
+ mArmRdSmbiosType4.Base.ThreadCount = CoreCount;
+ mArmRdSmbiosType4.Base.MaxSpeed = 3200; // Frequency in MHz
+ mArmRdSmbiosType4.Base.CurrentSpeed = 3200; // Frequency in MHz
+ break;
+ case RdN1EdgeX2:
+ case RdV1Mc:
+ mArmRdSmbiosType4.Base.CoreCount = CoreCount * FixedPcdGet32 (PcdChipCount);
+ mArmRdSmbiosType4.Base.EnabledCoreCount = CoreCount * FixedPcdGet32 (PcdChipCount);
+ mArmRdSmbiosType4.Base.ThreadCount = CoreCount * FixedPcdGet32 (PcdChipCount);
+ break;
+ case RdE1Edge:
+ mArmRdSmbiosType4.Base.CoreCount = CoreCount / NEOVERSE_E1_THREADS_PER_CORE;
+ mArmRdSmbiosType4.Base.EnabledCoreCount = CoreCount / NEOVERSE_E1_THREADS_PER_CORE;
+ mArmRdSmbiosType4.Base.ThreadCount = CoreCount;
+ mArmRdSmbiosType4.Base.MaxSpeed = 2300; // Frequency in MHz
+ mArmRdSmbiosType4.Base.CurrentSpeed = 2300; // Frequency in MHz
+ break;
+ }
+
+ mArmRdSmbiosType4.Base.ProcessorVersion = PROCESSOR_VERSION_BASE + SgiGetProductId ();
+ mArmRdSmbiosType4.Base.SerialNumber = SERIAL_NUMBER_BASE + SgiGetProductId ();
[SAMI] Minor optimisation can be achieved by storing the value returned by SgiGetProductId ()in a local variable.
+
+ /* Update processor-id and part number */
+ *ProcessorId = ArmReadMidr();
[SAMI] Space needed between ArmReadMidr and (.
+ UpdatePartNumber (*ProcessorId);
+
+ /* Install type 4 table */
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType4
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type4 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}


Re: [edk2-platforms][PATCH V2 11/11] Platform/Sgi: Add SMBIOS Type32 Table

Sami Mujawar
 

Hi Pranav,

Some comments in previous patches apply here as well and are not mentioned.

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 32 table (System Boot Information) that includes
information about the System Boot Status.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 6 ++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type32SystemBootInformation.c | 84 ++++++++++++++++++++
4 files changed, 92 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index f81494114188..4258eb9deadb 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -23,6 +23,7 @@
Type16PhysicalMemoryArray.c
Type17MemoryDevice.c
Type19MemoryArrayMappedAddress.c
+ Type32SystemBootInformation.c
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index c6dd72cb6b99..0bbda4b4b45d 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -59,6 +59,12 @@ InstallMemoryArrayMappedAddress (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallSystemBootInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_ENCLOSURE = 0x1000,
SMBIOS_HANDLE_CLUSTER1,
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index d5d1e6393184..77b22678f62a 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -34,6 +34,7 @@ ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallPhysicalMemoryArray,
&InstallMemoryDevice,
&InstallMemoryArrayMappedAddress,
+ &InstallSystemBootInformation,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type32SystemBootInformation.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type32SystemBootInformation.c
new file mode 100644
index 000000000000..1d3eaab810eb
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type32SystemBootInformation.c
@@ -0,0 +1,84 @@
+/** @file
+ SMBIOS Type 32 (System Boot Information) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 32 (System Boot Information) table for Arm's
+ Reference Design platforms. It includes information about the System Boot
+ Status.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.33
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SmbiosPlatformDxe.h"
+
+#define TYPE32_STRINGS \
+ "\0" /* Null string */
+
+/* SMBIOS Type32 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType32 {
+ SMBIOS_TABLE_TYPE32 Base;
+ UINT8 Strings[sizeof (TYPE32_STRINGS)];
+};
+#pragma pack()
+
+/* System Boot Information */
+static struct ArmRdSmbiosType32 mArmRdSmbiosType32 = {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_SYSTEM_BOOT_INFORMATION, // Type 32
+ sizeof (SMBIOS_TABLE_TYPE32), // Length
+ SMBIOS_HANDLE_PI_RESERVED
+ },
+ {0}, // Reserved field
+ BootInformationStatusNoError // Boot status
+ },
+ // Text strings (unformatted area)
+ TYPE32_STRINGS
+};
+
+/**
+ Install SMBIOS system boot information
+
+ Install the SMBIOS system boot information (type 32) table for RD platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallSystemBootInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType32)->Handle;
+
+ /* Install type 32 table */
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType32
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type32 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}


Re: [edk2-platforms][PATCH V2 09/11] Platform/Sgi: Add SMBIOS Type17 Table

Sami Mujawar
 

Hi Pranav,

Please find my comments inline marked [SAMI].

Some comments in previous patches apply here as well and are not mentioned.

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 17 table (Memory Device) that includes the
specification of each installed memory device such as size of each
device, bank locator, memory device type, and other related information.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 14 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c | 288 ++++++++++++++++++++
4 files changed, 304 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index ebd19c1882bb..9061c491d461 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -21,6 +21,7 @@
Type4ProcessorInformation.c
Type7CacheInformation.c
Type16PhysicalMemoryArray.c
+ Type17MemoryDevice.c
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index e195fdea35af..5413982e233b 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -47,6 +47,12 @@ InstallPhysicalMemoryArray (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallMemoryDevice (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_ENCLOSURE = 0x1000,
SMBIOS_HANDLE_CLUSTER1,
@@ -56,6 +62,14 @@ enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_L3_CACHE,
SMBIOS_HANDLE_L4_CACHE,
SMBIOS_HANDLE_PHYSICAL_MEMORY,
+ SMBIOS_HANDLE_MEMORY_DEVICE0000, // Chip 0 Bank 0
+ SMBIOS_HANDLE_MEMORY_DEVICE0001, // Chip 0 Bank 1
+ SMBIOS_HANDLE_MEMORY_DEVICE0100, // Chip 1 Bank 0
+ SMBIOS_HANDLE_MEMORY_DEVICE0101, // Chip 1 Bank 1
+ SMBIOS_HANDLE_MEMORY_DEVICE0200, // Chip 2 Bank 0
+ SMBIOS_HANDLE_MEMORY_DEVICE0201, // Chip 2 Bank 1
+ SMBIOS_HANDLE_MEMORY_DEVICE0300, // Chip 3 Bank 0
+ SMBIOS_HANDLE_MEMORY_DEVICE0301, // Chip 3 Bank 1
};
#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 48073ad0ad27..223bf1d114e4 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -32,6 +32,7 @@ ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallProcessorInformation,
&InstallCacheInformation,
&InstallPhysicalMemoryArray,
+ &InstallMemoryDevice,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c
new file mode 100644
index 000000000000..fc7422f432a6
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type17MemoryDevice.c
@@ -0,0 +1,288 @@
+/** @file
+ SMBIOS Type 17 (Memory Device) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 17 (Memory Device) table for Arm's Reference
+ Design platforms. It includes the specification of each installed memory
+ device such as size of each device, bank locator, memory device type, and
+ other related information.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.18
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SmbiosPlatformDxe.h"
+
+#define MAX_CHIP_COUNT 4
+#define BANK0_BASE 1
+#define BANK1_BASE (BANK0_BASE + MAX_CHIP_COUNT)
+#define TYPE17_STRINGS \
+ "Chip 0 Bank 0\0" \
+ "Chip 1 Bank 0\0" \
+ "Chip 2 Bank 0\0" \
+ "Chip 3 Bank 0\0" \
+ "Chip 0 Bank 1\0" \
+ "Chip 1 Bank 1\0" \
+ "Chip 2 Bank 1\0" \
+ "Chip 3 Bank 1\0"
+
+/* SMBIOS Type17 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType17 {
+ SMBIOS_TABLE_TYPE17 Base;
+ UINT8 Strings[sizeof (TYPE17_STRINGS)];
+};
+#pragma pack()
+
+/* Memory Device */
+static struct ArmRdSmbiosType17 mArmRdSmbiosType17[] = {
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0000
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK0_BASE, // Chip 0 Bank 0
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0001
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK1_BASE, // Chip 0 Bank 1
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0100
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK0_BASE + 1, // Chip 1 Bank 0
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0101
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK1_BASE + 1, // Chip 1 Bank 1
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0200
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK0_BASE + 2, // Chip 2 Bank 0
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0201
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK1_BASE + 2, // Chip 2 Bank 1
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0300
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK0_BASE + 3, // Chip 3 Bank 0
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+ {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE, // Type 17
+ sizeof (SMBIOS_TABLE_TYPE17), // Length
+ SMBIOS_HANDLE_MEMORY_DEVICE0301
+ },
+ SMBIOS_HANDLE_PHYSICAL_MEMORY, // Physical memory array handle
+ 0xFFFE, // Memory error info handle
+ 0xFFFF, // Total width unknown
+ 0xFFFF, // Data width unknown
+ 0, // Size, Update dynamically
+ MemoryFormFactorOther, // Form Factor
+ 0, // Device set, not part of a set
+ 0, // Device locator
+ BANK1_BASE + 3, // Chip 3 Bank 1
+ MemoryTypeDram, // Memory type
+ {0, 1}, // Type details others
+ },
+ // Text strings (unformatted area)
+ TYPE17_STRINGS
+ },
+};
+
+/**
+ Install SMBIOS memory device Table.
+
+ Install the SMBIOS memory device (type 17) table for RD platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallMemoryDevice (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+ UINT8 Idx;
+
+ /* Get system memory information */
+ for (Idx = 0; Idx < (FixedPcdGet32 (PcdChipCount) * 2); Idx += 2) {
+ mArmRdSmbiosType17[Idx].Base.Size =
+ (PcdGet64 (PcdSystemMemorySize) + SIZE_16MB) / SIZE_1MB;
[SAMI] Can you check if this field is encoded correctly, please?
The specification states the following:
'Size of the memory device
If the value is 0, no memory device is installed in the
socket; if the size is unknown, the field value is
FFFFh. If the size is 32 GB-1 MB or greater, the
field value is 7FFFh and the actual size is stored in
the Extended Size field.
The granularity in which the value is specified
depends on the setting of the most-significant bit (bit
15). If the bit is 0, the value is specified in megabyte
units; if the bit is 1, the value is specified in kilobyte
units. For example, the value 8100h identifies a
256 KB memory device and 0100h identifies a
256 MB memory device.'

For SgiPlatforms maybe you will never hit this situation. However, the code in its current state will silently produce incorrect result, which I think would be misleading should somone use this as reference.
[/SAMI]
+ mArmRdSmbiosType17[Idx].Base.MemoryTechnology = MemoryTechnologyDram;
+ mArmRdSmbiosType17[Idx].Base.MemoryOperatingModeCapability.Bits.VolatileMemory = 1;
+
+ if (PcdGet64 (PcdDramBlock2Size) != 0) {
+ mArmRdSmbiosType17[Idx + 1].Base.Size =
+ PcdGet64 (PcdDramBlock2Size) / SIZE_1MB;
+ mArmRdSmbiosType17[Idx + 1].Base.MemoryTechnology = MemoryTechnologyDram;
+ mArmRdSmbiosType17[Idx + 1].Base.MemoryOperatingModeCapability.Bits.VolatileMemory = 1;
+ }
+ }
+
+ /* Install valid entries */
+ for (Idx = 0; Idx < ARRAY_SIZE (mArmRdSmbiosType17); Idx++) {
+ if (mArmRdSmbiosType17[Idx].Base.Size != 0) {
+ SmbiosHandle =
+ ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType17[Idx])->Handle;
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType17[Idx]
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type17 SMBIOS table.\n"
+ ));
+ break;
+ }
+ }
+ }
+
+ return Status;
+}


Re: [edk2-platforms][PATCH V2 08/11] Platform/Sgi: Add SMBIOS Type16 Table

Sami Mujawar
 

Hi Pranav,

Some comments in previous patches apply here as well and are not mentioned.

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 16 table (Physical Memory Array) describes a
collection of memory devices that operate together to form a memory
address. It includes information about number of devices, total memory
installed, error correction mechanism used and other related information.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 4 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 7 ++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type16PhysicalMemoryArray.c | 106 ++++++++++++++++++++
4 files changed, 118 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index ee00b773912b..ebd19c1882bb 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -20,6 +20,7 @@
Type3SystemEnclosure.c
Type4ProcessorInformation.c
Type7CacheInformation.c
+ Type16PhysicalMemoryArray.c
[Packages]
ArmPkg/ArmPkg.dec
@@ -44,6 +45,9 @@
gArmPlatformTokenSpaceGuid.PcdClusterCount
gArmPlatformTokenSpaceGuid.PcdCoreCount
gArmSgiTokenSpaceGuid.PcdChipCount
+ gArmSgiTokenSpaceGuid.PcdDramBlock2Size
+ gArmTokenSpaceGuid.PcdSystemMemoryBase
+ gArmTokenSpaceGuid.PcdSystemMemorySize
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
[Protocols]
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 6f3ad29f0797..e195fdea35af 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -41,6 +41,12 @@ InstallCacheInformation (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallPhysicalMemoryArray (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_ENCLOSURE = 0x1000,
SMBIOS_HANDLE_CLUSTER1,
@@ -49,6 +55,7 @@ enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_L2_CACHE,
SMBIOS_HANDLE_L3_CACHE,
SMBIOS_HANDLE_L4_CACHE,
+ SMBIOS_HANDLE_PHYSICAL_MEMORY,
};
#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 62d0f5ce8033..48073ad0ad27 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -31,6 +31,7 @@ ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallSystemEnclosure,
&InstallProcessorInformation,
&InstallCacheInformation,
+ &InstallPhysicalMemoryArray,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type16PhysicalMemoryArray.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type16PhysicalMemoryArray.c
new file mode 100644
index 000000000000..44a71fa8d18d
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type16PhysicalMemoryArray.c
@@ -0,0 +1,106 @@
+/** @file
+ SMBIOS Type 16 (Physical Memory Array) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 16 (Physical Memory Array) table for Arm's
+ Reference Design platforms. It describes a collection of memory devices that
+ operate together to form a memory address. It includes information about
+ number of devices, total memory installed, error correction mechanism used
+ and other related information.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.17
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SmbiosPlatformDxe.h"
+
+#define TYPE16_STRINGS \
+ "\0" /* Null string */
+
+/* SMBIOS Type16 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType16 {
+ SMBIOS_TABLE_TYPE16 Base;
+ UINT8 Strings[sizeof (TYPE16_STRINGS)];
+} ARM_TYPE16;
+#pragma pack()
+
+/* Physical Memory Array */
+static struct ArmRdSmbiosType16 mArmRdSmbiosType16 = {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY, // Type 16
+ sizeof (SMBIOS_TABLE_TYPE16), // Length
+ SMBIOS_HANDLE_PHYSICAL_MEMORY
+ },
+ MemoryArrayLocationSystemBoard, // Location
+ MemoryArrayUseSystemMemory, // Used as system memory
+ MemoryErrorCorrectionUnknown, // Error correction
+ 0x80000000, // Maximum capacity in KiB, uses Extended Maximum capacity field
+ 0xFFFE, // Memory error info handle, does not provide this info
+ 0, // Num of memory devices, update dymamically
+ 0 // Extended Maximum capacity, update dymamically
+ },
+ // Text strings (unformatted area)
+ TYPE16_STRINGS
+};
+
+/**
+ Install SMBIOS physical memory array table.
+
+ Install the SMBIOS physical memory array (type 16) table for Arm's Reference
+ Design platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallPhysicalMemoryArray (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+ UINT16 NumOfMemoryDevices = 1;
+ UINT64 InstalledMemory;
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType16)->Handle;
+
+ /* Include 16MB of Trusted DRAM as well */
+ InstalledMemory = PcdGet64 (PcdSystemMemorySize) + SIZE_16MB;
+ if (PcdGet64 (PcdDramBlock2Size) != 0) {
+ NumOfMemoryDevices++;
+ InstalledMemory += PcdGet64 (PcdDramBlock2Size);
+ }
+
+ mArmRdSmbiosType16.Base.ExtendedMaximumCapacity =
+ InstalledMemory * FixedPcdGet32 (PcdChipCount);
+ mArmRdSmbiosType16.Base.NumberOfMemoryDevices =
+ NumOfMemoryDevices * FixedPcdGet32 (PcdChipCount);
+
+ /* Install type 16 table */
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType16
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type16 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}


Re: [edk2-platforms][PATCH V2 07/11] Platform/Sgi: Add SMBIOS Type7 Table

Sami Mujawar
 

Hi Pranav,

Some comments in previous patches apply here as well and are not mentioned.

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 7 table (Cache Information) that includes
information about cache levels implemented, cache configuration, ways of
associativity and other information related to cache memory installed.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 6 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type7CacheInformation.c | 334 ++++++++++++++++++++
4 files changed, 342 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 4652a9c62b88..ee00b773912b 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -19,6 +19,7 @@
Type1SystemInformation.c
Type3SystemEnclosure.c
Type4ProcessorInformation.c
+ Type7CacheInformation.c
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 8a9be0cfc4c8..6f3ad29f0797 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -35,6 +35,12 @@ InstallProcessorInformation (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallCacheInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
enum SMBIOS_REFRENCE_HANDLES {
SMBIOS_HANDLE_ENCLOSURE = 0x1000,
SMBIOS_HANDLE_CLUSTER1,
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 269bd0f9d843..62d0f5ce8033 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -30,6 +30,7 @@ ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallSystemInformation,
&InstallSystemEnclosure,
&InstallProcessorInformation,
+ &InstallCacheInformation,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type7CacheInformation.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type7CacheInformation.c
new file mode 100644
index 000000000000..8b42ed3d622c
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type7CacheInformation.c
@@ -0,0 +1,334 @@
+/** @file
+ SMBIOS Type 7 (Cache information) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 7 (Cache information) table for Arm's
+ Reference Design platforms. It includes information about cache levels
+ implemented, cache configuration, ways of associativity and other
+ information related to cache memory installed.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.8
+**/
+
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SgiPlatform.h"
+#include "SmbiosPlatformDxe.h"
+
+#define TYPE7_STRINGS \
+ "L1 Instruction\0" /* L1I */ \
+ "L1 Data\0" /* L1D */ \
+ "L2\0" /* L2 */ \
+ "L3\0" /* L3 */ \
+ "SLC\0" /* L4 */
+
+/* SMBIOS Type7 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType7 {
+ SMBIOS_TABLE_TYPE7 Base;
+ UINT8 Strings[sizeof (TYPE7_STRINGS)];
+} ARM_TYPE7;
+#pragma pack()
+
+/* Cache information */
+static struct ArmRdSmbiosType7 mArmRdSmbiosType7[] = {
+ { // Entry 0, L1 instruction cache
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_CACHE_INFORMATION, // Type 7
+ sizeof (SMBIOS_TABLE_TYPE7), // Length
+ SMBIOS_HANDLE_L1I_CACHE, // Handle number
+ },
+ 1,
+ (
+ (1 << 8) | // Write-back
+ (1 << 7) | // Cache enabled
+ (1 << 3) | // Cache socketed
+ 0x0 // Cache level 1
+ ),
+ 0xFFFF, // Uses Maximum cache size 2 field
+ 0xFFFF, // Uses Installed cache size 2 field
+ {0, 1}, // Supported SRAM type unknown
+ {0, 1}, // Current SRAM type unknown
+ 0, // Cache Speed Unknown
+ 0x02, // Error correction type unknown
+ 0x03, // Instruction cache
+ 0, // Associativity, update dynamically
+ 0, // Maximum cache size 2, update dynamically
+ 0 // Installed cache size 2, update dynamically
+ },
+ // Text strings (unformatted area)
+ TYPE7_STRINGS
+ },
+ { // Entry 1, L1 data cache
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_CACHE_INFORMATION, // Type 7
+ sizeof (SMBIOS_TABLE_TYPE7), // Length
+ SMBIOS_HANDLE_L1D_CACHE, // Handle number
+ },
+ 2,
+ (
+ (1 << 8) | // Write-back
+ (1 << 7) | // Cache enabled
+ (1 << 3) | // Cache socketed
+ 0x0 // Cache level 1
+ ),
+ 0xFFFF, // Uses Maximum cache size 2 field
+ 0xFFFF, // Uses Installed cache size 2 field
+ {0, 1}, // Supported SRAM type unknown
+ {0, 1}, // Current SRAM type unknown
+ 0, // Cache Speed Unknown
+ 0x02, // Error correction type unknown
+ 0x04, // Data cache
+ 0, // Associativity, update dynamically
+ 0, // Maximum cache size 2, update dynamically
+ 0 // Installed cache size 2, update dynamically
+ },
+ // Text strings (unformatted area)
+ TYPE7_STRINGS
+ },
+ { // Entry 2, L2 cache
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_CACHE_INFORMATION, // Type 7
+ sizeof (SMBIOS_TABLE_TYPE7), // Length
+ SMBIOS_HANDLE_L2_CACHE, // Handle number
+ },
+ 3,
+ (
+ (1 << 8) | // Write-back
+ (1 << 7) | // Cache enabled
+ (1 << 3) | // Cache socketed
+ 0x1 // Cache level 2
+ ),
+ 0xFFFF, // Uses Maximum cache size 2 field
+ 0xFFFF, // Uses Installed cache size 2 field
+ {0, 1}, // Supported SRAM type unknown
+ {0, 1}, // Current SRAM type unknown
+ 0, // Cache Speed Unknown
+ 0x02, // Error correction type unknown
+ 0x05, // Unified cache
+ 0, // Associativity, update dynamically
+ 0, // Maximum cache size 2, update dynamically
+ 0 // Installed cache size 2, update dynamically
+ },
+ // Text strings (unformatted area)
+ TYPE7_STRINGS
+ },
+ { // Entry 3, L3 cache
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_CACHE_INFORMATION, // Type 7
+ sizeof (SMBIOS_TABLE_TYPE7), // Length
+ SMBIOS_HANDLE_L3_CACHE, // Handle number
+ },
+ 4,
+ (
+ (1 << 8) | // Write-back
+ (1 << 7) | // Cache enabled
+ (1 << 3) | // Cache socketed
+ 0x2 // Cache level 3
+ ),
+ 0xFFFF, // Uses Maximum cache size 2 field
+ 0xFFFF, // Uses Installed cache size 2 field
+ {0, 1}, // Supported SRAM type unknown
+ {0, 1}, // Current SRAM type unknown
+ 0, // Cache Speed Unknown
+ 0x02, // Error correction type unknown
+ 0x05, // Unified cache
+ 0, // Associativity, update dynamically
+ 0, // Maximum cache size 2, update dynamically
+ 0 // Installed cache size 2, update dynamically
+ },
+ // Text strings (unformatted area)
+ TYPE7_STRINGS
+ },
+ { // Entry 4, SLC Cache
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_CACHE_INFORMATION, // Type 7
+ sizeof (SMBIOS_TABLE_TYPE7), // Length
+ SMBIOS_HANDLE_L4_CACHE, // Handle number
+ },
+ 5,
+ (
+ (1 << 8) | // Write-back
+ (1 << 7) | // Cache enabled
+ (1 << 3) | // Cache socketed
+ 0x3 // Cache level 4
+ ),
+ 0xFFFF, // Uses Maximum cache size 2 field
+ 0xFFFF, // Uses Installed cache size 2 field
+ {0, 1}, // Supported SRAM type unknown
+ {0, 1}, // Current SRAM type unknown
+ 0, // Cache Speed Unknown
+ 0x02, // Error correction type unknown
+ 0x05, // Unified cache
+ 0, // Associativity, update dynamically
+ 0, // Maximum cache size 2, update dynamically
+ 0 // Installed cache size 2, update dynamically
+ },
+ // Text strings (unformatted area)
+ TYPE7_STRINGS
+ }
+};
+
+/**
+ Install SMBIOS Cache information Table
+
+ Install the SMBIOS Cache information (type 7) table for Arm's Reference
+ Design platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_NOT_FOUND Unknown product id.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallCacheInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+ UINT8 CacheIdx;
+
+ /* Update the cache attributes based on the product */
+ switch (SgiGetProductId ()) {
+ case Sgi575:
+ /* L1 instruction cache */
+ mArmRdSmbiosType7[0].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.Associativity = CacheAssociativity4Way;
+ /* L1 data cache */
+ mArmRdSmbiosType7[1].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.Associativity = CacheAssociativity16Way;
+ /* L2 cache */
+ mArmRdSmbiosType7[2].Base.MaximumCacheSize2 = 512; // 512KB
+ mArmRdSmbiosType7[2].Base.InstalledSize2 = 512; // 512KB
+ mArmRdSmbiosType7[2].Base.Associativity = CacheAssociativity8Way;
+ /* L3 cache */
+ mArmRdSmbiosType7[3].Base.MaximumCacheSize2 = 2048; // 2MB
+ mArmRdSmbiosType7[3].Base.InstalledSize2 = 2048; // 2MB
+ mArmRdSmbiosType7[3].Base.Associativity = CacheAssociativity16Way;
+ break;
+ case RdN1Edge:
+ case RdN1EdgeX2:
+ /* L1 instruction cache */
+ mArmRdSmbiosType7[0].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.Associativity = CacheAssociativity4Way;
+ /* L1 data cache */
+ mArmRdSmbiosType7[1].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.Associativity = CacheAssociativity4Way;
+ /* L2 cache */
+ mArmRdSmbiosType7[2].Base.MaximumCacheSize2 = 512; // 512KB
+ mArmRdSmbiosType7[2].Base.InstalledSize2 = 512; // 512KB
+ mArmRdSmbiosType7[2].Base.Associativity = CacheAssociativity8Way;
+ /* L3 cache */
+ mArmRdSmbiosType7[3].Base.MaximumCacheSize2 = 2048; // 2MB
+ mArmRdSmbiosType7[3].Base.InstalledSize2 = 2048; // 2MB
+ mArmRdSmbiosType7[3].Base.Associativity = CacheAssociativity16Way;
+ /* System level cache */
+ mArmRdSmbiosType7[4].Base.MaximumCacheSize2 = 8192; // 8MB SLC per chip
+ mArmRdSmbiosType7[4].Base.InstalledSize2 = 8192; // 8MB SLC per chip
+ mArmRdSmbiosType7[4].Base.Associativity = CacheAssociativity16Way;
+ break;
+ case RdE1Edge:
+ /* L1 instruction cache */
+ mArmRdSmbiosType7[0].Base.MaximumCacheSize2 = 32; // 32KB
+ mArmRdSmbiosType7[0].Base.InstalledSize2 = 32; // 32KB
+ mArmRdSmbiosType7[0].Base.Associativity = CacheAssociativity4Way;
+ /* L1 data cache */
+ mArmRdSmbiosType7[1].Base.MaximumCacheSize2 = 32; // 32KB
+ mArmRdSmbiosType7[1].Base.InstalledSize2 = 32; // 32KB
+ mArmRdSmbiosType7[1].Base.Associativity = CacheAssociativity4Way;
+ /* L2 cache */
+ mArmRdSmbiosType7[2].Base.MaximumCacheSize2 = 256; // 256KB
+ mArmRdSmbiosType7[2].Base.InstalledSize2 = 256; // 256KB
+ mArmRdSmbiosType7[2].Base.Associativity = CacheAssociativity4Way;
+ /* L3 cache */
+ mArmRdSmbiosType7[3].Base.MaximumCacheSize2 = 2048; // 2MB
+ mArmRdSmbiosType7[3].Base.InstalledSize2 = 2048; // 2MB
+ mArmRdSmbiosType7[3].Base.Associativity = CacheAssociativity16Way;
+ /* System level cache */
+ mArmRdSmbiosType7[4].Base.MaximumCacheSize2 = 8192; // 8MB SLC
+ mArmRdSmbiosType7[4].Base.InstalledSize2 = 8192; // 8MB SLC
+ mArmRdSmbiosType7[4].Base.Associativity = CacheAssociativity16Way;
+ break;
+ case RdV1:
+ case RdV1Mc:
+ /* L1 instruction cache */
+ mArmRdSmbiosType7[0].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.Associativity = CacheAssociativity4Way;
+ /* L1 data cache */
+ mArmRdSmbiosType7[1].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.Associativity = CacheAssociativity4Way;
+ /* L2 cache */
+ mArmRdSmbiosType7[2].Base.MaximumCacheSize2 = 1024; // 1MB
+ mArmRdSmbiosType7[2].Base.InstalledSize2 = 1024; // 1MB
+ mArmRdSmbiosType7[2].Base.Associativity = CacheAssociativity8Way;
+ /* System level cache */
+ mArmRdSmbiosType7[4].Base.MaximumCacheSize2 = 16384; // 16MB SLC per chip
+ mArmRdSmbiosType7[4].Base.InstalledSize2 = 16384; // 16MB SLC per chip
+ mArmRdSmbiosType7[4].Base.Associativity = CacheAssociativity16Way;
+ break;
+ case RdN2:
+ /* L1 instruction cache */
+ mArmRdSmbiosType7[0].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[0].Base.Associativity = CacheAssociativity4Way;
+ /* L1 data cache */
+ mArmRdSmbiosType7[1].Base.MaximumCacheSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.InstalledSize2 = 64; // 64KB
+ mArmRdSmbiosType7[1].Base.Associativity = CacheAssociativity4Way;
+ /* L2 cache */
+ mArmRdSmbiosType7[2].Base.MaximumCacheSize2 = 1024; // 1MB
+ mArmRdSmbiosType7[2].Base.InstalledSize2 = 1024; // 1MB
+ mArmRdSmbiosType7[2].Base.Associativity = CacheAssociativity8Way;
+ /* System level cache */
+ mArmRdSmbiosType7[4].Base.MaximumCacheSize2 = 32768; // 32MB SLC
+ mArmRdSmbiosType7[4].Base.InstalledSize2 = 32768; // 32MB SLC
+ mArmRdSmbiosType7[4].Base.Associativity = CacheAssociativity16Way;
+ break;
+ }
+
+ /* Install valid cache information tables */
+ for (CacheIdx = 0; CacheIdx < ARRAY_SIZE (mArmRdSmbiosType7); CacheIdx++) {
+ if (mArmRdSmbiosType7[CacheIdx].Base.MaximumCacheSize2 == 0) {
+ continue;
+ }
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType7[CacheIdx])->Handle;
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType7[CacheIdx]
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type7 SMBIOS table.\n"
+ ));
+ }
+ }
+
+ return Status;
+}


Re: [edk2-platforms][PATCH V2 05/11] Platform/Sgi: Add SMBIOS Type3 Table

Sami Mujawar
 

Hi Pranav,

Please find my comments inline marked [SAMI].

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 3 table (System Enclosure) that includes information
about manufacturer, type, serial number and other information related to
system enclosure.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 10 ++
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnclosure.c | 96 ++++++++++++++++++++
4 files changed, 108 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index f7beb1c66c80..b3c1619ddc66 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -17,6 +17,7 @@
SmbiosPlatformDxe.c
Type0BiosInformation.c
Type1SystemInformation.c
+ Type3SystemEnclosure.c
[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index d7b3aadba948..4a6f8be2a2c2 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -23,4 +23,14 @@ InstallSystemInformation (
IN EFI_SMBIOS_PROTOCOL *Smbios
);
+EFI_STATUS
+EFIAPI
+InstallSystemEnclosure (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
+enum SMBIOS_REFRENCE_HANDLES {
+ SMBIOS_HANDLE_ENCLOSURE = 0x1000,
+};
[SAMI] typedef for enum?
+
#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 7b478063e223..5f4b833dc9fe 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -28,6 +28,7 @@ STATIC
ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallBiosInformation,
&InstallSystemInformation,
+ &InstallSystemEnclosure,
};
/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnclosure.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnclosure.c
new file mode 100644
index 000000000000..ef0c36d37923
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type3SystemEnclosure.c
@@ -0,0 +1,96 @@
+/** @file
+ SMBIOS Type 3 (System enclosure) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 3 (System enclosure) table for Arm Reference
+ Design platforms. SMBIOS Type 3 table (System Enclosure) includes information
+ about manufacturer, type, serial number and other information related to
+ system enclosure.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.4
+**/
+
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SmbiosPlatformDxe.h"
+
+#define TYPE3_STRINGS \
+ "ARM LTD\0" /* Manufacturer */ \
+ "Version not set\0" /* Version */ \
+ "Serial not set\0" /* Serial */ \
+ "Asset Tag not set\0" /* Asset Tag */
+
+/* SMBIOS Type3 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType3 {
+ SMBIOS_TABLE_TYPE3 Base;
+ UINT8 Strings[sizeof (TYPE3_STRINGS)];
+};
+#pragma pack()
+
+/* System information */
+static struct ArmRdSmbiosType3 mArmRdSmbiosType3 = {
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_SYSTEM_ENCLOSURE, // Type 3
+ sizeof (SMBIOS_TABLE_TYPE1), // Length
+ SMBIOS_HANDLE_ENCLOSURE, // Assign an unused handle number
+ },
+ 1, // Manufacturer
+ 2, // Enclosure type unknown
+ 2, // Version
+ 3, // Serial
+ 4, // Asset Tag
+ ChassisStateSafe, // Boot chassis state
+ ChassisStateSafe, // Power supply state
+ ChassisStateSafe, // Thermal state
+ ChassisSecurityStatusUnknown, // Security Status
+ {0}, // BIOS vendor specific Information
+ },
+ // Text strings (unformatted)
+ TYPE3_STRINGS
+};
+
+/**
+ Install SMBIOS System Enclosure Table
+
+ Install the SMBIOS System Enclosure (type 3) table for Arm's Reference Design
+ platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallSystemEnclosure (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType3)->Handle;
+
+ /* Install type 3 table */
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType3
+ );
+ if (Status != EFI_SUCCESS) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type3 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}


Re: [edk2-platforms][PATCH V2 03/11] Platform/Sgi: Add Initial SMBIOS support

Sami Mujawar
 

Hi Pranav,

Please find my comments inline marked [SAMI].

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
SMBIOS provides basic hardware and firmware configuration information
through table-driven data structure. This patch adds SMBIOS driver
support that allows for installation of multiple SMBIOS types. Also add
SMBIOS Type0 (BIOS Information) table, that include information about
BIOS vendor name, version, SMBIOS version and other information related
to BIOS.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
 Platform/ARM/SgiPkg/SgiPlatform.dsc.inc                              |  10 ++
 Platform/ARM/SgiPkg/SgiPlatform.fdf                                  |   8 +-
 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf  |  46 +++++++
 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h    |  20 ++++
 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c    |  98 +++++++++++++++
 Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInformation.c | 125 ++++++++++++++++++++
 6 files changed, 306 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
index 42e3600d15f4..a0f217f5107c 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
@@ -109,6 +109,10 @@
   # ACPI Table Version
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
 
+  # SMBIOS entry point version
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion|0x0304
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
+
   #
   # PCIe
   #
@@ -247,6 +251,12 @@
   MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
   MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
 
+  #
+  # SMBIOS/DMI
+  #
+  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
+  Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+
   #
   # platform driver
   #
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
index da23804828e5..e11d943d6efc 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
+++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2018, ARM Limited. All rights reserved.
+#  Copyright (c) 2018-2021, ARM Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -102,6 +102,12 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
 !include $(BOARD_DXE_FV_COMPONENTS)
 
+  #
+  # SMBIOS/DMI
+  #
+  INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
+  INF Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+
   # Required by PCI
   INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
 
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
new file mode 100644
index 000000000000..3568380f8404
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -0,0 +1,46 @@
+## @file
+#  This driver installs SMBIOS information for RD Platforms
+#
+#  Copyright (c) 2021, ARM Limited. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = SmbiosPlatformDxe
+  FILE_GUID                      = 86e0aa8b-4f8d-44a5-a140-1f693d529c76
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = SmbiosTableEntryPoint
+
+[Sources]
+  SmbiosPlatformDxe.c
+  Type0BiosInformation.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+  ArmPlatformLib
+  DebugLib
+  HobLib
+  UefiDriverEntryPoint
+
+[Guids]
+  gEfiGlobalVariableGuid
+  gArmSgiPlatformIdDescriptorGuid
+
+[FixedPcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
+
+[Protocols]
+  gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
+
+[Guids]
+
+[Depex]
+  gEfiSmbiosProtocolGuid
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
new file mode 100644
index 000000000000..091e0ec8314e
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -0,0 +1,20 @@
+/** @file
+  Declarations required for SMBIOS DXE driver.
+
+  Functions declarations and data type declarations required for SMBIOS DXE
+  driver of the Arm Reference Design platforms.
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef SMBIOS_PLATFORM_DXE_H_
+#define SMBIOS_PLATFORM_DXE_H_
+
+EFI_STATUS
+EFIAPI
+InstallBiosInformation (
+  IN     EFI_SMBIOS_PROTOCOL    *Smbios
+  );
[SAMI] Doxygen header? Same comment for other patches in this series.
[/SAMI]
+
+#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
new file mode 100644
index 000000000000..eb3ba45ca654
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -0,0 +1,98 @@
+/** @file
+  Install SMBIOS tables for Arm's SGI/RD platforms.
+
+  This file is the driver entry point for installing SMBIOS tables on Arm's
+  Reference Design platforms. For each SMBIOS table installation handler
+  registered, the driver invokes the handler to register the respective table.
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Specification Reference:
+    - SMBIOS Reference Specification 3.4.0
+**/
+
+#include <IndustryStandard/SmBios.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <PiDxe.h>
+#include <Protocol/Smbios.h>
+
+#include "SgiPlatform.h"
+#include "SmbiosPlatformDxe.h"
+
+typedef EFI_STATUS (*ARM_RD_SMBIOS_TABLE_INSTALL_FPTR)(EFI_SMBIOS_PROTOCOL *);
+
+STATIC
+ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
+  &InstallBiosInformation,
+};
+
+/**
+  Driver entry point. Installs SMBIOS information.
+
+  For all the available SMBIOS table installation handlers, invoke each of
+  those handlers and let the handlers install the SMBIOS tables. The count
+  of handlers that fail to install the SMBIOS tables is maintained for
+  additional logging.
+
+  @param ImageHandle     Module's image handle.
+  @param SystemTable     Pointer of EFI_SYSTEM_TABLE.
+
+  @retval EFI_SUCCESS    All SMBIOS table install handlers invoked.
+  @retval EFI_NOT_FOUND  Unsupported platform.
+  @retval Others         Failed to invoke SMBIOS table install handlders.
+**/
+EFI_STATUS
+EFIAPI
+SmbiosTableEntryPoint (
+  IN     EFI_HANDLE             ImageHandle,
+  IN     EFI_SYSTEM_TABLE       *SystemTable
+  )
+{
+  EFI_STATUS Status;
+  EFI_SMBIOS_PROTOCOL *Smbios = NULL;
+  UINT8 CountFail = 0;
+  UINT8 Idx;
+
+  /* Install SMBIOS table only for supported product */
+  if (SgiGetProductId () == UnknownId) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Failed to install SMBIOS: Unknown product\n"
+      ));
+    return EFI_NOT_FOUND;
+  }
+
+  /* Find the SMBIOS protocol */
+  Status = gBS->LocateProtocol (
+                  &gEfiSmbiosProtocolGuid,
+                  NULL,
+                  (VOID **)&Smbios
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Failed to install SMBIOS: Unable to locate protocol\n"
+      ));
+    return Status;
+  }
+
+  /* Install the tables listed in mSmbiosTableList */
+  for (Idx = 0; Idx < ARRAY_SIZE (mSmbiosTableList); Idx++) {
+    Status = (*mSmbiosTableList[Idx]) (Smbios);
+    if (Status != EFI_SUCCESS) {
+      CountFail++;
+    }
+  }
+
+  DEBUG ((
+    DEBUG_INFO,
+    "Installed %d of %d available SMBIOS tables\n",
+    ARRAY_SIZE (mSmbiosTableList) - CountFail,
+    ARRAY_SIZE (mSmbiosTableList)
+    ));
+
+  return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInformation.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInformation.c
new file mode 100644
index 000000000000..67c20c336c3d
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type0BiosInformation.c
@@ -0,0 +1,125 @@
+/** @file
+  SMBIOS Type 0 (BIOS information) table for ARM RD platforms.
+
+  Install SMBIOS Type 0 (BIOS information) table for Arm's Reference Design
+  platforms.
+
+  Copyright (c) 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Specification Reference:
+    - SMBIOS Reference Specification 3.4.0, Chapter 7.1
+**/
+
+#include <Library/DebugLib.h>
+#include <Protocol/Smbios.h>
+
+#define TYPE0_STRINGS                                   \
+  "ARM LTD\0"                   /* Vendor */            \
+  "EDK II\0"                    /* BiosVersion */       \
+  __DATE__"\0"                  /* BiosReleaseDate */   \
+  "\0"
+
+/* SMBIOS Type0 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType0 {
[SAMI] typedef for the struct ? Please also revisit the structure naming convention in the coding standard.
Same comment for other patches in this series.
[/SAMI]
+  SMBIOS_TABLE_TYPE0  Base;
+  INT8                Strings[sizeof (TYPE0_STRINGS)];
[SAMI] Can CHAR8 be used instead of INT8?
+};
+#pragma pack()
+
+/* BIOS information */
+static struct ArmRdSmbiosType0 mArmRdSmbiosType0 = {
[SAMI] STATIC in capital letters.
[/SAMI]
+  {
+    {
+      // SMBIOS header
+      EFI_SMBIOS_TYPE_BIOS_INFORMATION, // Type 0
+      sizeof (SMBIOS_TABLE_TYPE0),      // Length
+      SMBIOS_HANDLE_PI_RESERVED,        // Assign an unused handle number
+    },
+    1,              // String number of vendor name in TYPE0_STRINGS
+    2,              // String number of BiosVersion
[SAMI] Can enums be used for the string numbers? Same comment for other patches in this series.
+    0,              // Bios starting address segment
+    3,              // String number of BiosReleaseDate
+    0,              // Bios ROM size
[SAMI] Neither BIOS ROM size not Extended BIOS ROM size is set. Is this specification compliant?
+    {               // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
+      0,            // Reserved
+      0,            // Unknown
+      0,            // BIOS Characteristics are not supported
+      0,            // ISA is supported
+      0,            // MCA is supported
+      0,            // EISA is supported
[SAMI] Can you revisit the comments for this structure initialisation, please? They look a bit out of place.
These look to be copied from the specification wherein the table column header mentions 'Meaning if Set'.
See if this can be clarified in the comments Or a suggestion would be to change the comments to something like '// ISA support'.
[/SAMI]
+      1,            // PCI supported
+      0,            // PC card (PCMCIA) is supported
+      1,            // Plug and Play is supported
+      0,            // APM is supported
+      1,            // BIOS upgradable
+      0,            // BIOS shadowing is allowed
+      0,            // VL-VESA is supported
+      0,            // ESCD support is available
+      0,            // Boot from CD is supported
+      1,            // selectable boot
+    },
+    {               // BIOSCharacteristicsExtensionBytes
+      (
+        (1 << 0) |  // ACPI Supported
+        (1 << 1)    // Legacy USB supported
+      ),
+      (
+        (1 << 3) |  // Content distribution enabled
+        (1 << 4)    // UEFI spec supported
+      )
+    },
+    0,              // SMBIOS Major Release, updated dynamically
+    0,              // SMBIOS Minor Release, updated dynamically
+    0xFF,           // Embedded Controller Firmware Major Release
+    0xFF            // Embedded Controller Firmware Minor Release
+  },
+  // Text strings (unformatted area)
+  TYPE0_STRINGS
+};
+
+/**
+  Install SMBIOS BIOS information Table.
+
+  Install the SMBIOS BIOS information (type 0) table for Arm's reference design
+  platforms.
+
+  @param[in] Smbios   SMBIOS protocol.
+
+  @retval EFI_SUCCESS           Record was added.
+  @retval EFI_OUT_OF_RESOURCES  Record was not added.
+  @retval EFI_ALREADY_STARTED   The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallBiosInformation (
[SAMI] Would it be good to change the function name to indicate Type0 SMBIOS table? Something like InstallType<n><Short description> ?
Same comment for other patches in this series.
[/SAMI]
+  IN     EFI_SMBIOS_PROTOCOL    *Smbios
+  )
+{
+  EFI_STATUS Status;
+  EFI_SMBIOS_HANDLE SmbiosHandle;
+
+  SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType0)->Handle;
+
+  /* Update firmware revision */
+  mArmRdSmbiosType0.Base.SystemBiosMajorRelease =
+    (PcdGet32 ( PcdFirmwareRevision ) >> 16) & 0xFF;
[SAMI] No space should be there before and after PcdFirmwareRevision. Same comment for the line below.
+  mArmRdSmbiosType0.Base.SystemBiosMinorRelease =
+    PcdGet32 ( PcdFirmwareRevision ) & 0xFF;
+
+  /* Install type 0 table */
+  Status = Smbios->Add (
+                     Smbios,
+                     NULL,
+                     &SmbiosHandle,
+                     (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType0
+                     );
+  if (Status != EFI_SUCCESS) {
[SAMI] if (EFI_ERROR (Status)) ?
+    DEBUG ((
+      DEBUG_ERROR,
+      "SMBIOS: Failed to install Type0 SMBIOS table.\n"
+      ));
+  }
+
+  return Status;
+}


Re: [edk2-platforms][PATCH V2 04/11] Platform/Sgi: Add SMBIOS Type1 Table

Sami Mujawar
 

Hi Pranav,

Please find my comments inline marked [SAMI].

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add the SMBIOS type 1 table (System Information) that includes
information about manufacturer, product name, version, serial number and
other information related to the system identification.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h | 6 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 1 +
Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInformation.c | 132 ++++++++++++++++++++
4 files changed, 140 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 3568380f8404..f7beb1c66c80 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -16,6 +16,7 @@
[Sources]
SmbiosPlatformDxe.c
Type0BiosInformation.c
+ Type1SystemInformation.c

[Packages]
ArmPkg/ArmPkg.dec
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
index 091e0ec8314e..d7b3aadba948 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.h
@@ -17,4 +17,10 @@ InstallBiosInformation (
IN EFI_SMBIOS_PROTOCOL *Smbios
);

+EFI_STATUS
+EFIAPI
+InstallSystemInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ );
+
#endif // SMBIOS_PLATFORM_DXE_H_
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index eb3ba45ca654..7b478063e223 100644
--- a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -27,6 +27,7 @@ typedef EFI_STATUS (*ARM_RD_SMBIOS_TABLE_INSTALL_FPTR)(EFI_SMBIOS_PROTOCOL *);
STATIC
ARM_RD_SMBIOS_TABLE_INSTALL_FPTR mSmbiosTableList[] = {
&InstallBiosInformation,
+ &InstallSystemInformation,
};

/**
diff --git a/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInformation.c b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInformation.c
new file mode 100644
index 000000000000..4559af81046b
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Drivers/SmbiosPlatformDxe/Type1SystemInformation.c
@@ -0,0 +1,132 @@
+/** @file
+ SMBIOS Type 1 (System information) table for ARM RD platforms.
+
+ This file installs SMBIOS Type 1 (System information) table for Arm's
+ Reference Design platforms. Type 1 table defines attributes of the
+ overall system such as manufacturer, product name, UUID etc.
+
+ Copyright (c) 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Specification Reference:
+ - SMBIOS Reference Specification 3.4.0, Chapter 7.2
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Protocol/Smbios.h>
+
+#include "SgiPlatform.h"
+
+#define PRODUCT_NAME_BASE 6 /* Product name base from TYPE1_STRINGS */
+#define TYPE1_STRINGS \
+ "ARM LTD\0" /* Manufacturer */ \
+ "Version not set\0" /* Version */ \
+ "Serial not set\0" /* Serial number */ \
+ "Not Applicable\0" /* SKU */ \
+ "Not Applicable\0" /* Family */ \
+ "SGI575\0" /* Product Names */ \
+ "RdN1Edge\0" \
+ "RdN1EdgeX2\0" \
+ "RdE1Edge\0" \
+ "RdV1\0" \
+ "RdV1Mc\0" \
+ "RdN2\0"
+
+/* SMBIOS Type1 structure */
+#pragma pack(1)
+struct ArmRdSmbiosType1 {
+ SMBIOS_TABLE_TYPE1 Base;
+ UINT8 Strings[sizeof (TYPE1_STRINGS)];
[SAMI] CHAR8 ?
+} ARM_TYPE1;
+#pragma pack()
+
+STATIC GUID mSmbiosUid[] = {
+ /* Sgi575 */
+ {0xdd7cad0a, 0x227c, 0x4ed4, {0x9f, 0x42, 0xa9, 0x8b, 0xd6, 0xa2, 0x42, 0x6c}},
+ /* Rd-N1-Edge */
+ {0x80984efe, 0x404a, 0x43e0, {0xad, 0xa4, 0x63, 0xa0, 0xe0, 0xc4, 0x5e, 0x60}},
+ /* Rd-N1-Edge-X2 */
+ {0x2cc4f916, 0x267a, 0x4251, {0x95, 0x6e, 0xf0, 0x49, 0x82, 0xbe, 0x94, 0x58}},
+ /* Rd-E1-Edge */
+ {0x567f35c4, 0x104f, 0x447b, {0xa0, 0x94, 0x89, 0x2f, 0xbd, 0xb6, 0x5a, 0x55}},
+ /* Rd-V1 */
+ {0xc481f0b1, 0x237c, 0x42d7, {0x98, 0xb2, 0xb4, 0xb4, 0x8d, 0xb5, 0x4f, 0x50}},
+ /* Rd-V1Mc */
+ {0x1f3a0806, 0x18b5, 0x4eca, {0xad, 0xcd, 0xba, 0x9b, 0x07, 0xb1, 0x0a, 0xcf}},
+ /* Rd-N2 */
+ {0xf2cded73, 0x37f9, 0x4ec9, {0xd9, 0xf9, 0x89, 0x9b, 0x74, 0x91, 0x20, 0x49}}
+};
+
+/* System information */
+static struct ArmRdSmbiosType1 mArmRdSmbiosType1 = {
[SAMI] STATIC in capital letters?
+ {
+ {
+ // SMBIOS header
+ EFI_SMBIOS_TYPE_SYSTEM_INFORMATION, // Type 1
+ sizeof (SMBIOS_TABLE_TYPE1), // Length
+ SMBIOS_HANDLE_PI_RESERVED, // Assign an unused handle number
+ },
+ 1, // Manufacturer
+ PRODUCT_NAME_BASE, // Product Name, update dynamically
+ 2, // Version
+ 3, // Serial
+ {0}, // UUID, Update dymanically
+ 1, // Wakeup type other
+ 4, // SKU
+ 5, // Family
+ },
+ // Text strings (unformatted)
+ TYPE1_STRINGS
+};
+
+/**
+ Install SMBIOS System information Table.
+
+ Install the SMBIOS system information (type 1) table for Arm's reference
+ design platforms.
+
+ @param[in] Smbios SMBIOS protocol.
+
+ @retval EFI_SUCCESS Record was added.
+ @retval EFI_NOT_FOUND Unknown product id.
+ @retval EFI_OUT_OF_RESOURCES Record was not added.
+ @retval EFI_ALREADY_STARTED The SmbiosHandle passed in is already in use.
+**/
+EFI_STATUS
+InstallSystemInformation (
+ IN EFI_SMBIOS_PROTOCOL *Smbios
+ )
+{
+ EFI_STATUS Status;
+ EFI_SMBIOS_HANDLE SmbiosHandle;
+
+ SmbiosHandle = ((EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType1)->Handle;
+
+ /* Choose the product name from TYPE1_STRINGS based on the product ID */
+ if (SgiGetProductId () != UnknownId) {
+ mArmRdSmbiosType1.Base.ProductName =
+ PRODUCT_NAME_BASE + (SgiGetProductId () - 1);
+ CopyGuid (&mArmRdSmbiosType1.Base.Uuid,
+ &mSmbiosUid[SgiGetProductId () - 1]);
[SAMI] Apart from the horizontal spacing issues, I think minor
optimisation can be achieved by calling SgiGetProductId () once and the
return value stored in a local variable for subsequent use.
[/SAMI]
+ } else {
+ return EFI_NOT_FOUND;
+ }
+
+ /* Install type 1 table */
+ Status = Smbios->Add (
+ Smbios,
+ NULL,
+ &SmbiosHandle,
+ (EFI_SMBIOS_TABLE_HEADER *)&mArmRdSmbiosType1
+ );
+ if (Status != EFI_SUCCESS) {
[SAMI] if (EFI_ERROR (Status)) ? Same comment for other patches in this
seires.
+ DEBUG ((
+ DEBUG_ERROR,
+ "SMBIOS: Failed to install Type1 SMBIOS table.\n"
+ ));
+ }
+
+ return Status;
+}
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [edk2-platforms][PATCH V2 02/11] Platform/Sgi: Add GetProductId API for SGI/RD Platforms

Sami Mujawar
 

Hi Pranav,

Please find my feedback inline marked [SAMI].

With those addressed.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add GetProductId API for SGI/RD Platform. The API returns a product id
in integer format based on the platform description data. The product id
is required for other drivers such as SMBIOS.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 21 +++++
Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c | 86 +++++++++++++++++++-
2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
index 1c5366878712..142fa29bb9fa 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -68,4 +68,25 @@ typedef struct {
UINTN MultiChipMode;
} SGI_PLATFORM_DESCRIPTOR;
+// Arm SGI/RD Product IDs
+enum ARM_RD_PRODUCT_ID {
+ UnknownId = 0,
+ Sgi575,
+ RdN1Edge,
+ RdN1EdgeX2,
+ RdE1Edge,
+ RdV1,
+ RdV1Mc,
+ RdN2
+};
[SAMI]
+
+// Arm ProductId look-up table
+typedef struct {
+ UINTN ProductId;
+ UINTN PlatformId;
+ UINTN ConfigId;
+ UINTN MultiChipMode;
+} SGI_PRODUCT_ID_LOOKUP;
+
+UINT8 SgiGetProductId ( VOID );
[SAMI] Please add doxygen header for this function. Also there should not be spaces before and after VOID.
[/SAMI]
#endif // __SGI_PLATFORM_H__
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
index 9731d7cccede..57b1658ab0af 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.c
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2018, ARM Limited. All rights reserved.
+* Copyright (c) 2018-2021, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -8,9 +8,12 @@
#include <Library/ArmPlatformLib.h>
#include <Library/BaseLib.h>
+#include <Library/HobLib.h>
#include <Ppi/ArmMpCoreInfo.h>
#include <Ppi/SgiPlatformId.h>
+#include "SgiPlatform.h"
+
UINT64 NtFwConfigDtBlob;
STATIC SGI_NT_FW_CONFIG_INFO_PPI mNtFwConfigDtInfoPpi;
@@ -21,6 +24,51 @@ STATIC ARM_CORE_INFO mCoreInfoTable[] = {
},
};
+STATIC SGI_PRODUCT_ID_LOOKUP SgiProductIdLookup[] = {
[SAMI] Can this be made STATIC CONST?
[/SAMI]
+ {
+ Sgi575,
+ SGI575_PART_NUM,
+ SGI575_CONF_NUM,
+ 0
+ },
+ {
+ RdN1Edge,
+ RD_N1E1_EDGE_PART_NUM,
+ RD_N1_EDGE_CONF_ID,
+ 0
+ },
+ {
+ RdN1EdgeX2,
+ RD_N1E1_EDGE_PART_NUM,
+ RD_N1_EDGE_CONF_ID,
+ 1
+ },
+ {
+ RdE1Edge,
+ RD_N1E1_EDGE_PART_NUM,
+ RD_E1_EDGE_CONF_ID,
+ 0
+ },
+ {
+ RdV1,
+ RD_V1_PART_NUM,
+ RD_V1_CONF_ID,
+ 0
+ },
+ {
+ RdV1Mc,
+ RD_V1_PART_NUM,
+ RD_V1_MC_CONF_ID,
+ 1
+ },
+ {
+ RdN2,
+ RD_N2_PART_NUM,
+ RD_N2_CONF_ID,
+ 0
+ }
+};
+
EFI_BOOT_MODE
ArmPlatformGetBootMode (
VOID
@@ -75,3 +123,39 @@ ArmPlatformGetPlatformPpiList (
*PpiListSize = sizeof (gPlatformPpiTable);
*PpiList = gPlatformPpiTable;
}
+
+/**
+ Derermine the product ID.
+
+ Determine the product ID by using the data in the Platform ID Descriptor HOB
+ to lookup for a matching product ID.
+
+ @retval Zero Failed identify platform.
+ @retval Others ARM_RD_PRODUCT_ID of the identified platform.
+**/
+UINT8
+SgiGetProductId (
+ VOID
[SAMI] Allign VOID at 2 spaces from the start of the line.
[/SAMI]
+ )
+{
+ VOID *SystemIdHob;
+ UINT8 Idx;
+ SGI_PLATFORM_DESCRIPTOR *HobData;
+
+ SystemIdHob = GetFirstGuidHob (&gArmSgiPlatformIdDescriptorGuid);
+ if (SystemIdHob == NULL) {
+ return UnknownId;
+ }
+
+ HobData = (SGI_PLATFORM_DESCRIPTOR *)GET_GUID_HOB_DATA (SystemIdHob);
+
+ for (Idx = 0; Idx < ARRAY_SIZE (SgiProductIdLookup); Idx++) {
+ if ((HobData->PlatformId == SgiProductIdLookup[Idx].PlatformId) &&
+ (HobData->ConfigId == SgiProductIdLookup[Idx].ConfigId) &&
+ (HobData->MultiChipMode == SgiProductIdLookup[Idx].MultiChipMode)) {
+ return SgiProductIdLookup[Idx].ProductId;
+ }
+ }
+
+ return UnknownId;
+}


Re: [edk2-platforms][PATCH V2 01/11] Platform/Sgi: Define RD-N2 platform id values

Sami Mujawar
 

Hi Pranav,

Thank you for this patch.

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@...>

Regards,

Sami Mujawar

On 16/05/2021 10:29 AM, Pranav Madhu wrote:
Add RD-N2 platform identification values including the part number
and configuration number. This information will be used in populating
the SMBIOS tables.

Signed-off-by: Pranav Madhu <pranav.madhu@...>
---
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
index 818879b5f81e..1c5366878712 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2018-2020, ARM Limited. All rights reserved.
+* Copyright (c) 2018-2021, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -39,6 +39,10 @@
#define RD_V1_CONF_ID 0x1
#define RD_V1_MC_CONF_ID 0x2
+// RD-N2 Platform Identification values
+#define RD_N2_PART_NUM 0x7B7
+#define RD_N2_CONF_ID 0x1
+
#define SGI_CONFIG_MASK 0x0F
#define SGI_CONFIG_SHIFT 0x1C
#define SGI_PART_NUM_MASK 0xFFF


Re: [PATCH v2] UefiCpuPkg/MpInitLib: Allocate a separate SEV-ES AP reset stack area

Lendacky, Thomas
 

On 5/16/21 11:22 PM, Laszlo Ersek wrote:
On 05/14/21 22:28, Lendacky, Thomas wrote:
BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3324&;data=04%7C01%7Cthomas.lendacky%40amd.com%7C8142daa079b04c0b3b5508d918eb6417%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637568221542784370%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=N9MNXaBazq2tiVRHWSPVXRdlcZ97JOf24mc7p0m5Tqw%3D&amp;reserved=0

The SEV-ES stacks currently share a page with the reset code and data.
Separate the SEV-ES stacks from the reset vector code and data to avoid
possible stack overflows from overwriting the code and/or data.

When SEV-ES is enabled, invoke the GetWakeupBuffer() routine a second time
to allocate a new area, below the reset vector and data.

Both the PEI and DXE versions of GetWakeupBuffer() are changed so that
when PcdSevEsIsEnabled is true, they will track the previous reset buffer
allocation in order to ensure that the new buffer allocation is below the
previous allocation. When PcdSevEsIsEnabled is false, the original logic
is followed.

Fixes: 7b7508ad784d16a5208c8d12dff43aef6df0835b
Is this really a *bugfix*?

I called what's being fixed "a gap in a generic protection mechanism",
in <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3324%23c9&;data=04%7C01%7Cthomas.lendacky%40amd.com%7C8142daa079b04c0b3b5508d918eb6417%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637568221542784370%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=BiE2cBZRVRT3anwklHEKBHdhg8v2KjV%2FiiPwtx%2Fpon4%3D&amp;reserved=0>.

I don't know if that makes this patch a feature addition (or feature
completion -- the feature being "more extensive page protections"), or a
bugfix.

The distinction matters because of the soft feature freeze:

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Release-Planning&;data=04%7C01%7Cthomas.lendacky%40amd.com%7C8142daa079b04c0b3b5508d918eb6417%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637568221542784370%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=1n8z7KFAlm3Vb7fPOFpYQFlZ5lQFOF%2FdLtujjqhns9s%3D&amp;reserved=0

We still have approximately 2 hours before the SFF starts. So if we can
*finish* reviewing this in 2 hours, then it can be merged during the
SFF, even if we call it a feature. But I'd like Marvin to take a look as
well, plus I'd like at least one of Eric and Ray to check.

... I'm tempted not to call it a bugfix, because the lack of this patch
does not break SEV-ES usage, as far as I understand.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Marvin Häuser <mhaeuser@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>

---

Changes since v1:
- Renamed the wakeup buffer variables to be unique in the PEI and DXE
libraries and to make it obvious they are SEV-ES specific.
- Use PcdGetBool (PcdSevEsIsEnabled) to make the changes regression-free
so that the new support is only use for SEV-ES guests.
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 19 +++++++-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 49 +++++++++++++-------
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 19 +++++++-
3 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 7839c249760e..93fc63bf93e3 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -29,6 +29,11 @@ VOID *mReservedApLoopFunc = NULL;
UINTN mReservedTopOfApStack;
volatile UINT32 mNumberToFinish = 0;

+//
+// Begin wakeup buffer allocation below 0x88000
+//
+STATIC EFI_PHYSICAL_ADDRESS mSevEsDxeWakeupBuffer = 0x88000;
+
/**
Enable Debug Agent to support source debugging on AP function.

@@ -102,7 +107,14 @@ GetWakeupBuffer (
// LagacyBios driver depends on CPU Arch protocol which guarantees below
// allocation runs earlier than LegacyBios driver.
//
- StartAddress = 0x88000;
+ if (PcdGetBool (PcdSevEsIsEnabled)) {
+ //
+ // SEV-ES Wakeup buffer should be under 0x88000 and under any previous one
+ //
+ StartAddress = mSevEsDxeWakeupBuffer;
+ } else {
+ StartAddress = 0x88000;
+ }
Status = gBS->AllocatePages (
AllocateMaxAddress,
MemoryType,
@@ -112,6 +124,11 @@ GetWakeupBuffer (
ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
StartAddress = (EFI_PHYSICAL_ADDRESS) -1;
+ } else if (PcdGetBool (PcdSevEsIsEnabled)) {
+ //
+ // Next SEV-ES wakeup buffer allocation must be below this allocation
+ //
+ mSevEsDxeWakeupBuffer = StartAddress;
}

DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index dc2a54aa31e8..b9a06747edbf 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1164,20 +1164,6 @@ GetApResetVectorSize (
AddressMap->SwitchToRealSize +
sizeof (MP_CPU_EXCHANGE_INFO);

- //
- // The AP reset stack is only used by SEV-ES guests. Do not add to the
- // allocation if SEV-ES is not enabled.
- //
- if (PcdGetBool (PcdSevEsIsEnabled)) {
- //
- // Stack location is based on APIC ID, so use the total number of
- // processors for calculating the total stack area.
- //
- Size += AP_RESET_STACK_SIZE * PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
-
- Size = ALIGN_VALUE (Size, CPU_STACK_ALIGNMENT);
- }
-
return Size;
}

@@ -1192,6 +1178,7 @@ AllocateResetVector (
)
{
UINTN ApResetVectorSize;
+ UINTN ApResetStackSize;

if (CpuMpData->WakeupBuffer == (UINTN) -1) {
ApResetVectorSize = GetApResetVectorSize (&CpuMpData->AddressMap);
@@ -1207,9 +1194,39 @@ AllocateResetVector (
CpuMpData->AddressMap.ModeTransitionOffset
);
//
- // The reset stack starts at the end of the buffer.
+ // The AP reset stack is only used by SEV-ES guests. Do not allocate it
+ // if SEV-ES is not enabled.
//
- CpuMpData->SevEsAPResetStackStart = CpuMpData->WakeupBuffer + ApResetVectorSize;
+ if (PcdGetBool (PcdSevEsIsEnabled)) {
+ //
+ // Stack location is based on ProcessorNumber, so use the total number
sneaky documenation fix :) I vaguely remember discussing earlier that
the APIC ID reference was incorrect. OK.
Yeah, I should have made mention of that in the commit message.


+ // of processors for calculating the total stack area.
+ //
+ ApResetStackSize = (AP_RESET_STACK_SIZE *
+ PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
+
+ //
+ // Invoke GetWakeupBuffer a second time to allocate the stack area
+ // below 1MB. The returned buffer will be page aligned and sized and
+ // below the previously allocated buffer.
+ //
+ CpuMpData->SevEsAPResetStackStart = GetWakeupBuffer (ApResetStackSize);
+
+ //
+ // Check to be sure that the "allocate below" behavior hasn't changed.
+ // This will also catch a failed allocation, as "-1" is returned on
+ // failure.
+ //
+ if (CpuMpData->SevEsAPResetStackStart >= CpuMpData->WakeupBuffer) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "SEV-ES AP reset stack is not below wakeup buffer\n"
+ ));
+
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ }
+ }
}
BackupAndPrepareWakeupBuffer (CpuMpData);
}
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 3989bd6a7a9f..90015c650c68 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -11,6 +11,8 @@
#include <Guid/S3SmmInitDone.h>
#include <Ppi/ShadowMicrocode.h>

+STATIC UINT64 mSevEsPeiWakeupBuffer = BASE_1MB;
+
/**
S3 SMM Init Done notification function.

@@ -220,7 +222,13 @@ GetWakeupBuffer (
// Need memory under 1MB to be collected here
//
WakeupBufferEnd = Hob.ResourceDescriptor->PhysicalStart + Hob.ResourceDescriptor->ResourceLength;
- if (WakeupBufferEnd > BASE_1MB) {
+ if (PcdGetBool (PcdSevEsIsEnabled) &&
+ WakeupBufferEnd > mSevEsPeiWakeupBuffer) {
+ //
+ // SEV-ES Wakeup buffer should be under 1MB and under any previous one
+ //
+ WakeupBufferEnd = mSevEsPeiWakeupBuffer;
+ } else if (WakeupBufferEnd > BASE_1MB) {
//
// Wakeup buffer should be under 1MB
//
@@ -244,6 +252,15 @@ GetWakeupBuffer (
}
DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n",
WakeupBufferStart, WakeupBufferSize));
+
+ if (PcdGetBool (PcdSevEsIsEnabled)) {
+ //
+ // Next SEV-ES wakeup buffer allocation must be below this
+ // allocation
+ //
+ mSevEsPeiWakeupBuffer = WakeupBufferStart;
+ }
+
return (UINTN)WakeupBufferStart;
}
}
The code in the patch seems sound to me, but, now that I've managed to
take a bit more careful look, I think the patch is incomplete.

Note the BackupAndPrepareWakeupBuffer() function call -- visible in the
context --, at the end of the AllocateResetVector() function! Before, we
had a single area allocated for the reset vector, which was
appropriately sized for SEV-ES stacks too, in case SEV-ES was enabled.

That was because MpInitLibInitialize() called GetApResetVectorSize()
too, and stored the result to the "BackupBufferSize" field. Thus, the
BackupAndPrepareWakeupBuffer() function, and its counterpart
RestoreWakeupBuffer(), would include the SEV-ES AP stacks area in the
backup/restore operations.
The restore is not performed for an SEV-ES guest (see FreeResetVector()),
because the memory is still needed.


But now, with SEV-ES enabled, we'll have a separate, discontiguous area
-- and neither BackupAndPrepareWakeupBuffer(), nor its counterpart
RestoreWakeupBuffer() take that into account.

Therefore I think, while this patch is regression-free for the SEV-ES
*disabled* case, it may corrupt memory (through not restoring the AP
stack area's original contents) with SEV-ES enabled.
This is the current behavior for SEV-ES. The wakeup buffer memory is
marked as reserved, at least in the DXE phase.


I think we need to turn "ApResetStackSize" into an explicit field. It
should have a defined value only when SEV-ES is enabled. And for that
case, we seem to need a *separate backup buffer* too.

FWIW, I'd really like to re-classify this BZ as a Feature Request (see
the Product field on BZ#3324), and I'd really like to delay the patch
until after edk2-stable202105. The patch is not necessary for using
SEV-ES, but it has a chance to break SEV-ES.
I'm ok with delaying this if you want.

Thanks,
Tom


Thanks
Laszlo


Re: [edk2-platforms][PATCH v2 1/6] Platform/ARM/SgiPkg: sync with edk2 StandaloneMmCpu path change

Thomas Abraham
 

On 5/17/21 11:20 AM, Etienne Carriere wrote:
Synchronize with edk2 package where StandaloneMmCpu component has
moved
from
StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
to StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Ilias Apalodimas <ilias.apalodimas@...>
Cc: Leif Lindholm <leif@...>
Cc: Sami Mujawar <sami.mujawar@...>
Cc: Sughosh Ganu <sughosh.ganu@...>
Cc: Thomas Abraham <thomas.abraham@...>
Signed-off-by: Etienne Carriere <etienne.carriere@...>
---
Changes since v1:
- split change in 3: this change relates to Platform/ARM/SgiPkg only.
---
Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 2 +-
Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Thomas Abraham <thomas.abraham@...>


Re: [PATCH v2 06/13] MdePkg/Register/Amd: define GHCB macros for SNP AP creation

Lendacky, Thomas
 

On 5/16/21 10:08 PM, Laszlo Ersek wrote:
Patches v2 01-05 look good to me, thanks for the updates. Now on to this
one:

On 05/13/21 01:46, Brijesh Singh wrote:
From: Tom Lendacky <thomas.lendacky@...>

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3D3275&;data=04%7C01%7Cthomas.lendacky%40amd.com%7C55d74e19a5554988e0b608d918e1215c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637568177488739589%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZIy6xEXWmxgVxZm6mrdlroM7OPQajEjUSD8W5z2ohu0%3D&amp;reserved=0
(1) The "3D" seems like a typo in the bug ticket URL. (This crept into
v2 somehow; v1 didn't have it.)


Version 2 of GHCB introduces NAE for creating AP when SEV-SNP is enabled
in the guest VM. See the GHCB specification, Table 5 "List of Supported
Non-Automatic Events" and sections 4.1.9 and 4.3.2, for further details.

While at it, define the VMSA state save area that is required for creating
the AP. The save area format is defined in AMD APM volume 2, Table B-4
(there is a mistake in the table that defines the size of the reserved
area at offset 0xc8 as a dword, when it is actually a word). The format of
the save area segment registers is further defined in AMD APM volume 2,
sections 10 and 15.5.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Reviewed-by: Liming Gao <gaoliming@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
MdePkg/Include/Register/Amd/Ghcb.h | 76 ++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
index 029904b1c63a..4d1ee29e0a5e 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -55,6 +55,7 @@
#define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL
#define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL
#define SVM_EXIT_SNP_PAGE_STATE_CHANGE 0x80000010ULL
+#define SVM_EXIT_SNP_AP_CREATION 0x80000013ULL
#define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL
#define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL

@@ -83,6 +84,12 @@
#define IOIO_SEG_ES 0
#define IOIO_SEG_DS (BIT11 | BIT10)

+//
+// AP Creation Information
+//
+#define SVM_VMGEXIT_SNP_AP_CREATE_ON_INIT 0
+#define SVM_VMGEXIT_SNP_AP_CREATE 1
+#define SVM_VMGEXIT_SNP_AP_DESTROY 2

typedef PACKED struct {
UINT8 Reserved1[203];
@@ -195,4 +202,73 @@ typedef struct {
SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY];
} SNP_PAGE_STATE_CHANGE_INFO;

+//
+// SEV-ES save area mapping structures used for SEV-SNP AP Creation.
+// Only the fields required to be set to a non-zero value are defined.
+//
+#define SEV_ES_RESET_CODE_SEGMENT_TYPE 0xA
+#define SEV_ES_RESET_DATA_SEGMENT_TYPE 0x2
+
+#define SEV_ES_RESET_LDT_TYPE 0x2
+#define SEV_ES_RESET_TSS_TYPE 0x3
+
+#pragma pack (1)
+typedef union {
+ struct {
+ UINT16 Type:4;
+ UINT16 Sbit:1;
+ UINT16 Dpl:2;
+ UINT16 Present:1;
+ UINT16 Avl:1;
+ UINT16 Reserved1:1;
+ UINT16 Db:1;
+ UINT16 Granularity:1;
+ } Bits;
+ UINT16 Uint16;
+} SEV_ES_SEGMENT_REGISTER_ATTRIBUTES;
+
+typedef struct {
+ UINT16 Selector;
+ SEV_ES_SEGMENT_REGISTER_ATTRIBUTES Attributes;
+ UINT32 Limit;
+ UINT64 Base;
+} SEV_ES_SEGMENT_REGISTER;
+
I'm not saying anything is incorrect about this, but I *am* going to
rant about the APM.
Yes, the APM is definitely lacking in this area.


It's simply impenetrable. I've been staring at it for ~50 minutes now,
and I still cannot fully connect it to your code.

[1] In sections "4.8.1 Code-Segment Descriptors" and "4.8.2 Data-Segment
Descriptors", the reader is introduced to the "normal" (not SEV-ES, not
virtualized, not SMM) segment descriptors. Why *these* are relevant
*here* is nothing short of mind-boggling, but please bear with me.

[2] In section "10.2.3 SMRAM State-Save Area", "Table 10-1. AMD64
Architecture SMM State-Save Area", the reader is introduced to the
2+2+4+8 segment register representation. The table only lists "Selector,
Attributes, Limit, Base" as fields, and nothing about the actual
contents. Way too little information. I guess this is covered by the
commit message reference "section 10".

[3] In section "15.5 VMRUN Instruction", "15.5.1 Basic Operation",
paragraph "Segment State in the VMCB", we're given a long-winded,
*textual* only -- no diagram! -- and *differential* (relative)
explanation, on top of the two, above-quoted, locations of the spec. I'm
sorry but this is just impossible to follow. Would it have been a
unaffordable to insert a self-contained diagram here, with
self-contained, absolute explanation?

So let me quote:

The segment registers are stored in the VMCB in a format similar to
that for SMM: both base and limit are fully expanded; segment
attributes are stored as 12-bit values formed by the concatenation
of bits 55:52 and 47:40 from the original 64-bit (in-memory) segment
descriptors; the descriptor “P” bit is used to signal NULL segments
(P=0) where permissible and/or relevant.

So, if we apply this to [1] and [2], the "Selector", "Limit" and "Base"
fields of the SMM and VMCB segment register representation are
explained. Further, we get the following for the Attributes field, by
manual reconstruction:

55 54 53 52 47 46 45 44 43 42 41 40

G D L AVL P [DPL] 1 1 C R A Code Segment Descriptor
* * * * * * ignored bits in 64-bit mode

G D/B - AVL P [DPL] 1 0 E W A Data Segment Descriptor
* * * * * * * * * * ignored bits in 64-bit mode

In other words, in the code segment descriptor, D, L, P, DPL, and C
matter. In the data segment descriptor, only P matters.
The APs won't be in 64-bit mode when being started. In reset, they will be
in real mode, so the attributes will be set up for that. Please see Table
4-3 and Table 4-4 for how these will matter.


In particular, what [3] says, quoted above, about the "P" bit, seems
sensible -- "P" is indeed *not* ignored for either segment descriptor
type (code or data).

But then we continure reading [3], and we find (quote again):

The loading of segment attributes from the VMCB (which may have been
overwritten by software) may result in attribute bit values that are
otherwise not allowed. However, only some of the attribute bits are
actually observed by hardware, depending on the segment register in
question:
* CS—D, L, P, and R.
* SS—B, P, E, W, and Code/Data
* DS, ES, FS, GS —D, P, DPL, E, W, and Code/Data.
* LDTR—P, S, and Type (LDT)
* TR—P, S, and Type (32- or 16-bit TSS)

I'm going to ignore SS, LDTR, and TR, but let's check CS and DS.

For CS, the spec says, "D, L, P, and R" are observed by the hardware.
But we've just shown that R is ignored *in general* for code segments in
64-bit mode! In other words, { D, L, P, R } is *not a subset* of { D, L,
P, DPL, C }.

For DS, the spec says here, "D, P, DPL, E, W, and Code/Data" are
observed. I'm going to give "Code/Data" a pass (bit 43 in the original
representation), but the rest is {D, P, DPL, E, W}, which is *not a
subset* of { P }.

Again, from [1], section 4.8.2 specifically, E (expand-down) and W
(writeable) are ignored, DPL is ignored, and D isn't even called D but
"D/B", and it is ignored. So how can the spec say in [3] that the
hardware observes { D, DPL, E, W } when all those are ignored per prior
definition [1]?

- And then I see no reference that SEV-ES uses the same
(circumstantially-defined) segment register format.


So anyway, I'll just accept that I don't understand the spec -- I think
it has inconsistencies. Let's look at the code:

- Bits 43:40 are packed into the "Type" bit-field. That means [1,C,R,A]
for the code segment descriptor, and [0,E,W,A] for data segment descriptors

SEV_ES_RESET_CODE_SEGMENT_TYPE has value 0xA (binary 1010), which maps
to: 1=1, C=0, R=1, A=0. Problem: per [1], the R bit is ignored, so I
have no clue why we set it.
For reset, when we are in real mode, that would correspond to executable /
readable type.


(2) Can you please explain that? Just in this discussion thread, no need
ot change the code or the commit message.
The idea is that we need to support real mode for the AP creation support,
but actually AP creation isn't limited to that. I didn't want to
overly-define the segment register for all the different modes, since it
would only be real mode that would be used by OVMF, but maybe that would
make it eaiser / clearer to understand...


The C ("conforming") bit actually matters. It is documented in section
"4.7.2 Code-Segment Descriptors" (Code-Segment Descriptor—Legacy Mode).
It seems like "non-conforming" is not a problem here, as long as we
don't use multiple privilege levels, which I think we don't, in
firmware-land. OK.

Then, on to SEV_ES_RESET_DATA_SEGMENT_TYPE. Value 0x2 (binary 0010).
Maps to [0,E,W,A], meaning 0=0, E=0, W=1, A=0.

(3) Why is W (writeable) set to 1, when, according to [1], it is ignored
in 64-bit mode?
Same as previous comment, the AP will be starting in real mode.



- "Sbit" seems to stand for bit 44 from the original representation --
constant 1. OK I think.

- "Dpl", "Present" and "Avl" look OK to me.

- Should "Reserved1" really be called that? It seems to map to bit 53 in
the original representation, and the L (long mode) bit actually matters
for the code segment. (It indeed appears undefined / reserved for data
segments.)

Am I *totally* mistaken here and we're not even talking long mode?
:)


- "Db" and "Granularity" look OK.

- SEV_ES_RESET_LDT_TYPE (value 2) matches "64-bit LDT" in "4.8.3 System
Descriptors", "Table 4-6. System-Segment Descriptor Types—Long Mode". OK.

- SEV_ES_RESET_TSS_TYPE (value 3) seems wrong. In Table 4-6, value 3 is
associated with "Reserved (Illegal)". And, according to "12.2.2 TSS
Descriptor", "The TSS descriptor is a system-segment descriptor", which
makes me think that I'm looking at value 3 in the proper table (Table 4-6).
I'll add a reference to table 14-2 under the "Processor Initialization"
section.



(4) Can you please explain why SEV_ES_RESET_TSS_TYPE is 3, and not (say)
0x9 ("Available 64-bit TSS") or 0xB ("Busy 64-bit TSS")?
For processor reset, type 3 represents a busy 16-bit TSS.

So I can work on clarifying the comments around all of the definitions and
indicate that values are defined for processor reset support and that more
definitions would be needed if the segment registers want to be examined /
set in different modes. Thoughts?

Thanks,
Tom


(Please note that this is only superficial pattern matching from my side
("TSS"); I don't know the first thing about "hardware task management".)


+typedef struct {
+ SEV_ES_SEGMENT_REGISTER Es;
+ SEV_ES_SEGMENT_REGISTER Cs;
+ SEV_ES_SEGMENT_REGISTER Ss;
+ SEV_ES_SEGMENT_REGISTER Ds;
+ SEV_ES_SEGMENT_REGISTER Fs;
+ SEV_ES_SEGMENT_REGISTER Gs;
+ SEV_ES_SEGMENT_REGISTER Gdtr;
+ SEV_ES_SEGMENT_REGISTER Ldtr;
+ SEV_ES_SEGMENT_REGISTER Idtr;
+ SEV_ES_SEGMENT_REGISTER Tr;
+ UINT8 Reserved1[42];
+ UINT8 Vmpl;
+ UINT8 Reserved2[5];
+ UINT64 Efer;
+ UINT8 Reserved3[112];
+ UINT64 Cr4;
+ UINT8 Reserved4[8];
+ UINT64 Cr0;
+ UINT64 Dr7;
+ UINT64 Dr6;
+ UINT64 Rflags;
+ UINT64 Rip;
+ UINT8 Reserved5[232];
+ UINT64 GPat;
+ UINT8 Reserved6[320];
+ UINT64 SevFeatures;
+ UINT8 Reserved7[48];
+ UINT64 XCr0;
+ UINT8 Reserved8[24];
+ UINT32 Mxcsr;
+ UINT16 X87Ftw;
+ UINT8 Reserved9[2];
+ UINT16 X87Fcw;
+} SEV_ES_SAVE_AREA;
+#pragma pack ()
+
#endif
This part looks good to me.

(I spent almost two hours reviewing this patch.)

Thanks!
Laszlo


Re: [PATCH] Maintainers.txt: add Sami Mujawar as top-level ArmVirtPkg reviewer

Leif Lindholm
 



On Fri, May 14, 2021 at 12:49 PM Laszlo Ersek <lersek@...> wrote:
>
> For distributing ArmVirtPkg patch review tasks better, move Sami Mujawar
> from the "ArmVirtPkg: Kvmtool" section to the top-level "ArmVirtPkg"
> section.
>
> Given that "ArmVirtPkg: Kvmtool" remains without a specific "R" role,
> remove "ArmVirtPkg: Kvmtool" altogether.
>
> Cc: Andrew Fish <afish@...>
> Cc: Ard Biesheuvel <ardb+tianocore@...>
> Cc: Julien Grall <julien@...>
> Cc: Leif Lindholm <leif@...>
> Cc: Michael D Kinney <michael.d.kinney@...>
> Cc: Philippe Mathieu-Daudé <philmd@...>
> Cc: Sami Mujawar <sami.mujawar@...>
> Signed-off-by: Laszlo Ersek <lersek@...>
> ---
>  Maintainers.txt | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index cafe6b1ab85d..1ec9185e70b9 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -143,33 +143,24 @@ M: Ard Biesheuvel <ardb+tianocore@...>
>  ArmVirtPkg
>  F: ArmVirtPkg/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/ArmVirtPkg
>  M: Laszlo Ersek <lersek@...>
>  M: Ard Biesheuvel <ardb+tianocore@...>
>  R: Leif Lindholm <leif@...>
> +R: Sami Mujawar <sami.mujawar@...>
>
>  ArmVirtPkg: modules used on Xen
>  F: ArmVirtPkg/ArmVirtXen.*
>  F: ArmVirtPkg/Library/XenArmGenericTimerVirtCounterLib/
>  F: ArmVirtPkg/Library/XenVirtMemInfoLib/
>  F: ArmVirtPkg/PrePi/
>  F: ArmVirtPkg/XenAcpiPlatformDxe/
>  F: ArmVirtPkg/XenPlatformHasAcpiDtDxe/
>  F: ArmVirtPkg/XenioFdtDxe/
>  R: Julien Grall <julien@...>
>
> -ArmVirtPkg: Kvmtool emulated platform support
> -F: ArmVirtPkg/ArmVirtKvmTool.*
> -F: ArmVirtPkg/KvmtoolPlatformDxe/
> -F: ArmVirtPkg/Library/Fdt16550SerialPortHookLib/
> -F: ArmVirtPkg/Library/KvmtoolPlatformPeiLib/
> -F: ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/
> -F: ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/
> -F: ArmVirtPkg/Library/NorFlashKvmtoolLib/
> -R: Sami Mujawar <sami.mujawar@...>
> -
>  BaseTools
>  F: BaseTools/
>  W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
>  M: Bob Feng <bob.c.feng@...>
>  M: Liming Gao <gaoliming@...>
>  R: Yuwei Chen <yuwei.chen@...>

Reviewed-by: Leif Lindholm <leif@...>


Re: [PATCH] EmulatorPkg: Update lldbefi.py to work with current lldb which uses python3

Rebecca Cran
 

Could someone review this please?

Thanks.
Rebecca Cran

On 5/9/21 1:26 PM, Rebecca Cran wrote:
The version of lldb shipping with macOS Big Sur is lldb-1205.0.27.3, and
it uses python3. Update lldbefi.py to work with it, including removing
the unused 'commands' import and fixing the print statements.
Signed-off-by: Rebecca Cran <rebecca@...>
---
EmulatorPkg/Unix/lldbefi.py | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/EmulatorPkg/Unix/lldbefi.py b/EmulatorPkg/Unix/lldbefi.py
index c3fb2675cb..952f8bf982 100755
--- a/EmulatorPkg/Unix/lldbefi.py
+++ b/EmulatorPkg/Unix/lldbefi.py
@@ -10,7 +10,6 @@ import lldb
import os
import uuid
import string
-import commands
import optparse
import shlex
@@ -389,7 +388,7 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
FileName = frame.thread.process.ReadCStringFromMemory (FileNamePtr, FileNameLen, Error)
if not Error.Success():
- print "!ReadCStringFromMemory() did not find a %d byte C string at %x" % (FileNameLen, FileNamePtr)
+ print("!ReadCStringFromMemory() did not find a %d byte C string at %x" % (FileNameLen, FileNamePtr))
# make breakpoint command continue
return False
@@ -398,7 +397,7 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
LoadAddress = frame.FindVariable ("LoadAddress").GetValueAsUnsigned() - 0x240
debugger.HandleCommand ("target modules add %s" % FileName)
- print "target modules load --slid 0x%x %s" % (LoadAddress, FileName)
+ print("target modules load --slid 0x%x %s" % (LoadAddress, FileName))
debugger.HandleCommand ("target modules load --slide 0x%x --file %s" % (LoadAddress, FileName))
else:
target = debugger.GetSelectedTarget()
@@ -408,7 +407,7 @@ def LoadEmulatorEfiSymbols(frame, bp_loc , internal_dict):
if FileName == ModuleName or FileName == SBModule.GetFileSpec().GetFilename():
target.ClearModuleLoadAddress (SBModule)
if not target.RemoveModule (SBModule):
- print "!lldb.target.RemoveModule (%s) FAILED" % SBModule
+ print("!lldb.target.RemoveModule (%s) FAILED" % SBModule)
# make breakpoint command continue
return False
@@ -490,15 +489,15 @@ def efi_guid_command(debugger, command, result, dict):
if len(args) >= 1:
if GuidStr in guid_dict:
- print "%s = %s" % (guid_dict[GuidStr], GuidStr)
- print "%s = %s" % (guid_dict[GuidStr], GuidToCStructStr (GuidStr))
+ print("%s = %s" % (guid_dict[GuidStr], GuidStr))
+ print("%s = %s" % (guid_dict[GuidStr], GuidToCStructStr (GuidStr)))
else:
- print GuidStr
+ print(GuidStr)
else:
# dump entire dictionary
width = max(len(v) for k,v in guid_dict.iteritems())
for value in sorted(guid_dict, key=guid_dict.get):
- print '%-*s %s %s' % (width, guid_dict[value], value, GuidToCStructStr(value))
+ print('%-*s %s %s' % (width, guid_dict[value], value, GuidToCStructStr(value)))
return
@@ -538,4 +537,4 @@ def __lldb_init_module (debugger, internal_dict):
if Breakpoint.GetNumLocations() == 1:
# Set the emulator breakpoints, if we are in the emulator
debugger.HandleCommand("breakpoint command add -s python -F lldbefi.LoadEmulatorEfiSymbols {id}".format(id=Breakpoint.GetID()))
- print 'Type r to run emulator. SecLldbScriptBreak armed. EFI modules should now get source level debugging in the emulator.'
+ print('Type r to run emulator. SecLldbScriptBreak armed. EFI modules should now get source level debugging in the emulator.')

19421 - 19440 of 94572