From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (NAM02-SN1-obe.outbound.protection.outlook.com [40.107.96.76]) by mx.groups.io with SMTP id smtpd.web10.6353.1686630937499121923 for ; Mon, 12 Jun 2023 21:35:37 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=o7A2dXaU; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: amd.com, ip: 40.107.96.76, mailfrom: abner.chang@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MQPK9maYMLPc0dB2PPbbuP+gyBBIsjsqRw9D3cxkmtre1hXmQha8xtcH41Ed2JEEvspbOjwTRvDLYqPu7orqWuBFsSg1+kWCWi2kmBBTcd75y3NOCsCPPdfJ7lRam6cmaRXKF+/zMn7IT/TS43vZ79QLpidp5NeCUzjCkmoZY7b2fb0yxCguIcvCdUM0cReRzncVzCo8kWUhipSOVqtQGsaJmk6jtffhAQtRulAePTAMyHRCZI6hzU5mMqF52/N5j05BjO13rfSFREwKmtRVKw1zzztzcahaYI/2pSscazEgHEADZP9Wnxv7POXUfbMIxna2ZMtX7trHVswhcsSJmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=2TwmqTVlPBss/cEQGgx4GvQX8iTqkrQ2ZvuIZrywZTE=; b=IfV3yg1WaUn2M7LnUC4DSmZzy/B9+ODjQTUmDQ6aqt0LdGxUUpqCJ0wfTNbiBpnO3pU66qo5IALsoTv4mLi+syjTYMRVcqvEyQjeDqJXGc5nDkyHIPvR7/pI9J0WVlfEUl1lLDpHSAueNOrpwWgoyVoLVwV5PdOzzWEuItUyioun+/6xZ2KvX0pYtbD1pLaZZwXsvxG5FtiuCydmIIwU5FaD22Llg2KhpGDj/trZIIAa0PBQROpOo5FrfFlt2LvkXWn7L0wd8+0ZGTu/zzOerllIaZCT7cBSATfBTyt1lLfLFY5aOxkxPeF6BaNM37qKCkbh4SAkPQvvKf76NmtneA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=2TwmqTVlPBss/cEQGgx4GvQX8iTqkrQ2ZvuIZrywZTE=; b=o7A2dXaUOxlozcprHyj6tWRWUludxa7CSUAwmQin1y6I0JeVGfmfWVJzX7F/iFBr42La+RgWKd92GKHUwv0NaWa8FRlYbFcPnpHZbcEGCKjrvSY9ZVrU5F2J173LVbw84KIKJIFPei7AwdUWVUf5zo4e1OAo/kliV48di3bMO+o= Received: from MN2PR12MB3966.namprd12.prod.outlook.com (2603:10b6:208:165::18) by IA1PR12MB6580.namprd12.prod.outlook.com (2603:10b6:208:3a0::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6455.39; Tue, 13 Jun 2023 04:35:33 +0000 Received: from MN2PR12MB3966.namprd12.prod.outlook.com ([fe80::edb0:bed8:c650:5040]) by MN2PR12MB3966.namprd12.prod.outlook.com ([fe80::edb0:bed8:c650:5040%6]) with mapi id 15.20.6455.043; Tue, 13 Jun 2023 04:35:33 +0000 From: "Chang, Abner" To: "devel@edk2.groups.io" , "isaac.w.oram@intel.com" , Arun K CC: "Desimone, Nathaniel L" , Ramkumar Krishnamoorthi , "Gao, Liming" Subject: Re: [edk2-devel][edk2-platforms][PATCH V3-1] IpmiFeaturePkg:Provided multiple IPMI interface support in DXE and SMM Thread-Topic: [edk2-devel][edk2-platforms][PATCH V3-1] IpmiFeaturePkg:Provided multiple IPMI interface support in DXE and SMM Thread-Index: AQHZnSyvCWQvvaPlU027p17cENlsz6+HbL3ggAC4b/A= Date: Tue, 13 Jun 2023 04:35:33 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_ActionId=86cb1830-df33-4e2d-82aa-e0dc24622bd5;MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_ContentBits=0;MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_Enabled=true;MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_Method=Standard;MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_Name=General;MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_SetDate=2023-06-13T04:30:20Z;MSIP_Label_4342314e-0df4-4b58-84bf-38bed6170a0f_SiteId=3dd8961f-e488-4e60-8e11-a82d994e183d; authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MN2PR12MB3966:EE_|IA1PR12MB6580:EE_ x-ms-office365-filtering-correlation-id: 97072b4e-5e9e-4495-b2a4-08db6bc7a06c x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: BGMkUBd6uRrnNCqOzJHXhEwMRcYgekOCZIw/r/gf6wxpJnj5bzHVQrz0zzo5ZQ5cx7K+GD/DElH6z64/zDqVDqI1Pqp9H2I3BZvURLAJa/xAz4UZrgOUX3UUjgw8yQ8KvR76ItiVDSu2020HX5t9ysGsVz2F7a7JwRWt6NqTkkSy2TxShyR+zjeqrt5rndeTaeKtwSmk1Ic/rZzR3nCkqu5Pb1enUBHxst3rr43E+4BBIxYighnnVA+VVjflhXizJAS5rPy3kmRMJO+Lfy/OGfc78ThGnM/DlFCp9U5/18rSHLnkGuXKHUCqhk0L/jX467C3CoJR44jzqRs7PPGMgxMLLL9uufQAMretvA99B1NdYlWXSnEWjUOTXM8Cw38py8Qij4DyV5MK1vd74wBvF5KWHqHf1GnORmwUnWbdv1+0BfUp/3wdAlXtAybMlcO1QFQ/ZVckwFyUotnGn+E1bBgcuOBd470Hytpm53o1el6/Q1FzaUspXPjIxrOOmhuPeBH7h6VW2IbPuRue0xCTH3FBvtWIQeKMivPp5MUNiHSzc4TYUyC+Ug7ADHxl1PlUumfZbIbLBqzoHpYFsy2ene67gbJjalpGnIuLcNxpb25WQL6ObkO2Coga2BxXI4LJHIBhdOhpfSvjx2LKuJ47rA== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MN2PR12MB3966.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(346002)(366004)(39860400002)(136003)(376002)(396003)(451199021)(2906002)(52536014)(30864003)(5660300002)(8936002)(41300700001)(4326008)(64756008)(66946007)(66476007)(76116006)(66446008)(66556008)(66899021)(316002)(110136005)(478600001)(54906003)(19627235002)(8676002)(966005)(71200400001)(7696005)(53546011)(186003)(86362001)(40140700001)(55016003)(83380400001)(26005)(6506007)(38070700005)(33656002)(9686003)(38100700002)(122000001)(559001)(579004);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?3iS3A+Wbt2LAzraW1+Tkg1RmJUj1TF/duCyJvU0G6ZP7kac7Bq0EvCmUJcdb?= =?us-ascii?Q?/JZJZ7HDkBnbmbgK1p8/TGWe90sXcQjN8T7xlRLvmbKpv9HpCdhvyksTe+1v?= =?us-ascii?Q?d9RVxZMhP49aazhxb2YmoGU4v7iiHndLdUrH4RQeatUfjZpPYAnNJOfq1M+1?= =?us-ascii?Q?1+m8jxLSCZ/zNTMm6x0nAkMvm8cctuYljjEvRz41i4Tu3m/MF++Kn50YX3NK?= =?us-ascii?Q?+VJykMXAuUAh/Fp7QQis8olCrObu58+mHiEjGAY0e6HjL3C+XcyyoSCfwloY?= =?us-ascii?Q?MVJqQuoki2b9enpSsjPL5bgrKTpcboTVPgM/Ebav49XsYPgtMV9CjHt18BE1?= =?us-ascii?Q?Gnap02oXRS+T0JSspdYnGCohXGRvRL7K2QOKN1RoBLwlrcW/2SbiBFSG1S7B?= =?us-ascii?Q?DYYtXIDV+v5lL/h8GakkPHn5N/Utbr0a8TT2wxJ+ZkOQCRi6gHQqRg4PhyBa?= =?us-ascii?Q?gXPO9PtM/TU2cd16Ju3h1RakFouUD+vX5vYrMTaq5ybRJxDWsx79E6xUIMuW?= =?us-ascii?Q?cUTKx+P0qCxLJRmUpxMCk7i+4qqyskhAJmYJfMMPYkx9GDZgY4vbwBcTkVMU?= =?us-ascii?Q?ThcE5YjjOv8gd3MBDZ58wFt3rUHzKVDj3Z60uMHCmhqky1tAMWx3XWd9Z63X?= =?us-ascii?Q?4IsPLGQ+nRdPKq2NDf5BcgOh3fUh9FtjJ/Mw3CO/JGcqgFJ1RA+t9YOEKSyD?= =?us-ascii?Q?CyOQIAWwVZ/Nqx2N9qtyn/u7xfaKogw7o6TL64nZP0UYZ/WGvDzbxietzEyu?= =?us-ascii?Q?vdtRycYHu2EWzip3UZnneer3APyUL5dAkok31gdjJA15mgNMhn0huv32Gp8r?= =?us-ascii?Q?EjXU+iX7YcE59qmkhlwRPN2108oRq/nyrFhjER8wLYd6yOcCibjj7TsVqVRP?= =?us-ascii?Q?MoELrwoJEbRervfwuNvW6gZuY3muzC2LCqISTPORq6TDFJORM0veUsaP4tCw?= =?us-ascii?Q?mBAr3J8YqfKJa4Kb3T1BVsAYgXuNgDcRfr0UqiEeOSWE/L1u++4zvLlqssHj?= =?us-ascii?Q?wEWi4bMGfrnqp6V2lv2t9HzqU07vFxyln2q+mF2UfoSilAhucG2E8rQHVpEQ?= =?us-ascii?Q?uDJhouVa57DaXl5D0Xy2zg3Ty88kmCMK46B8SmDxwvm5fP4f0MEGLDF2vsp0?= =?us-ascii?Q?v7CRyBYtwXFfJKTqVQStUoSHtVjSz91xkKNr+8qg2eKdi7ZGGsAOHmxvJ57Y?= =?us-ascii?Q?WE9qWjVjWtK9mc+n5LpT3/Zr3Ucfg2fhe6zS5kle2EQX22+QGJdkyS+3uSld?= =?us-ascii?Q?B5JTA2bFlgsj/3tQ2LOffbcNLuBXQx0SJ9NuME99/WSlImGmXGVShW9kYr0y?= =?us-ascii?Q?HWgmnlZbBIGUtaiwwLVUOfUxWCTYvYobfoR8t2g6Ogt8IyLsQ4pyaWLyKhDY?= =?us-ascii?Q?lvW4wlKAUwyHeYNZK/eSAwGtIrxSHTtFzOg76ViRCvHpzMIjUwn3vpt77irf?= =?us-ascii?Q?HZeKuLiToIvmMpZeqE6ImDX3OWAFeIZ7obf/WXtcgSfpUejXBS050MII2LvJ?= =?us-ascii?Q?/kB5HZfVqyp8G7t6TXic+1cQLrJ7HVcAuuDLdF3dNjwgstfeiyh4C5ampQbq?= =?us-ascii?Q?rONXG+AYLbIEkB/LSpw=3D?= MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB3966.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 97072b4e-5e9e-4495-b2a4-08db6bc7a06c X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jun 2023 04:35:33.6940 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: OMmcookkMDqr1oSlCOjR1UT//NihFRfko3JUc4+YXC+PxWV+3wVrjtLvG3RFHlZ7GM0ptjMRw9R9GqCN1CM4dw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6580 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable [AMD Official Use Only - General] Hi Isaac, I had replied to AMI and had them to work on the ManageabilityPkg directly = (I think it was few months ago) as we migrated all IPMI stuff from Intel Ip= miFeaturePkg to ManageabilityPkg. Do you think we still have to spend effort on IpmiFeaturePkg? AMD also has = one IPMI feature waiting for upstream to edk2-platform, work on both packag= es will lead to divergency. I suggest we just have AMI to revise their code to compliant with Manageabi= lityPkg. Regards, Abner > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Isaac > Oram via groups.io > Sent: Tuesday, June 13, 2023 8:27 AM > To: Arun K ; devel@edk2.groups.io > Cc: Desimone, Nathaniel L ; Ramkumar > Krishnamoorthi ; Gao, Liming > > Subject: Re: [edk2-devel][edk2-platforms][PATCH V3-1] > IpmiFeaturePkg:Provided multiple IPMI interface support in DXE and SMM > > Caution: This message originated from an External Source. Use proper caut= ion > when opening attachments, clicking links, or responding. > > > Comments inline, prefaced with "[Isaac]". Mostly style things that you c= ould > probably fix with uncrustify also. > > The main concern is the controls seem confused with #defines and PCD. > Simplifying to just use the PCD directly seems clear to me and eliminatin= g the > #if logic in favor of regular C logic is also preferred. > > Thanks, > Isaac > > -----Original Message----- > From: Arun K > Sent: Monday, June 12, 2023 5:52 AM > To: devel@edk2.groups.io; Arun K > Cc: Oram, Isaac W ; Desimone, Nathaniel L > ; Ramkumar Krishnamoorthi > ; Gao, Liming > Subject: [edk2-devel][edk2-platforms][PATCH V3-1] IpmiFeaturePkg:Provided > multiple IPMI interface support in DXE and SMM > > Created IpmiTransport2 PPI/Protocol to support multiple IPMI BMC Interfac= e > support such as KCS/BT/SSIF with 2 API's > IpmiSubmitCommand2 & IpmiSubmitCommand2Ex. > IpmiSubmitCommand2 - This API use the default interface > (PcdDefaultSystemInterface) to send IPMI command. > IpmiSubmitCommand2Ex - This API use the specific interface type to send > IPMI command which is passed as an argument. > > Cc: Isaac Oram > Cc: Nate DeSimone > Cc: Liming Gao > > Signed-off-by: Arun K > --- > .../GenericIpmi/Common/IpmiBmc.h | 10 + > .../GenericIpmi/Common/IpmiBmcCommon.h | 2 + > .../GenericIpmi/Common/IpmiHooks.c | 256 ++++++++++++++++++ > .../GenericIpmi/Common/IpmiHooks.h | 93 ++++++- > .../GenericIpmi/Dxe/GenericIpmi.inf | 14 +- > .../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c | 205 ++++++++++++++ > .../GenericIpmi/Smm/SmmGenericIpmi.c | 200 +++++++++++++- > .../GenericIpmi/Smm/SmmGenericIpmi.inf | 12 + > .../IpmiFeaturePkg/IpmiFeaturePkg.dec | 45 +++ > 9 files changed, 832 insertions(+), 5 deletions(-) > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiBmc.h > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiBmc.h > index d306a085e5..19fb2a26a3 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiBmc.h > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > m > +++ on/IpmiBmc.h > @@ -3,6 +3,7 @@ > > > @copyright > > Copyright 1999 - 2021 Intel Corporation.
> > + Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > > > @@ -21,6 +22,7 @@ > #include > > #include > > #include > > +#include > > > > #include "IpmiBmcCommon.h" > > #include "KcsBmc.h" > > @@ -41,4 +43,12 @@ > SM_IPMI_BMC_SIGNATURE \ > > ) > > > > +#define INSTANCE_FROM_IPMI_TRANSPORT2_THIS(a) \ > > + CR ( \ > > + a, \ > > + IPMI_BMC_INSTANCE_DATA, \ > > + IpmiTransport2, \ > > + SM_IPMI_BMC_SIGNATURE \ > > + ) > > + > > #endif // _IPMI_BMC_H_ > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiBmcCommon.h > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiBmcCommon.h > index 06eab62aae..3b252f5f1c 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiBmcCommon.h > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > m > +++ on/IpmiBmcCommon.h > @@ -3,6 +3,7 @@ > > > @copyright > > Copyright 1999 - 2021 Intel Corporation.
> > + Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > > > @@ -55,6 +56,7 @@ typedef struct { > UINT8 SoftErrorCount; > > UINT16 IpmiIoBase; > > IPMI_TRANSPORT IpmiTransport; > > + IPMI_TRANSPORT2 IpmiTransport2; > > EFI_HANDLE IpmiSmmHandle; > > } IPMI_BMC_INSTANCE_DATA; > > > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiHooks.c > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiHooks.c > index b2788e5a4c..19e5c1c04b 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiHooks.c > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > m > +++ on/IpmiHooks.c > @@ -3,6 +3,7 @@ > > > @copyright > > Copyright 2014 - 2021 Intel Corporation.
> > + Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > > > @@ -48,6 +49,11 @@ Returns: > > > --*/ > > { > > + > > + if (This =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > [Isaac] The preceding line is improperly indented. > > + } > > + > > // > > // This Will be unchanged ( BMC/KCS style ) > > // > > @@ -64,6 +70,251 @@ Returns: > ); > > } // IpmiSendCommand() > > > > +EFI_STATUS > > +EFIAPI > > +IpmiSendCommand2 ( > > + IN IPMI_TRANSPORT2 *This, > > + IN UINT8 NetFunction, > > + IN UINT8 Lun, > > + IN UINT8 Command, > > + IN UINT8 *CommandData, > > + IN UINT32 CommandDataSize, > > + IN OUT UINT8 *ResponseData, > > + IN OUT UINT32 *ResponseDataSize > > + ) > > +/*++ > > + > > +Routine Description: > > + > > + This API use the default interface (PcdDefaultSystemInterface) to > + send IPMI command > > + in the right mode to the appropiate device, ME or BMC. > > + > > +Arguments: > > + > > + This - Pointer to IPMI protocol instance > > + NetFunction - Net Function of command to send > > + Lun - LUN of command to send > > + Command - IPMI command to send > > + CommandData - Pointer to command data buffer, if needed > > + CommandDataSize - Size of command data buffer > > + ResponseData - Pointer to response data buffer > > + ResponseDataSize - Pointer to response data buffer size > > + > > +Returns: > > + > > + EFI_INVALID_PARAMETER - One of the input values is bad > > + EFI_DEVICE_ERROR - IPMI command failed > > + EFI_BUFFER_TOO_SMALL - Response buffer is too small > > + EFI_UNSUPPORTED - Command is not supported by BMC > > + EFI_SUCCESS - Command completed successfully > > + > > +--*/ > > +{ > > + > > + IPMI_BMC_INSTANCE_DATA *IpmiInstance; > > + > > + if (This =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > [Isaac] The preceding line is improperly indented. > > + } > > + > > + IpmiInstance =3D INSTANCE_FROM_IPMI_TRANSPORT2_THIS(This); > > + > > +#if KcsInterfaceSupport > [Isaac] I don't think that using #defines helps the code. I think obscur= ing the > PCD makes the code harder to understand. Also not following the > MACRO_NAMING_CONVENTION. > If you really prefer the macro, change it to something like > IS_KCS_INTERFACE_PCD_ENABLED. > I also prefer using regular C code instead of c preprocessors. All major > toolchains support optimization that should remove any benefit to using #= if. > If we use regular if statements, then code is compiled and thus tested fo= r build > more regularly. > > + if ((IpmiInstance->IpmiTransport2.InterfaceType =3D=3D SysInterfaceKcs= ) > + && > > + (IpmiInstance->IpmiTransport2.Interface.KcsInterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiSendCommand ( > > + &IpmiInstance->IpmiTransport, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + CommandDataSize, > > + ResponseData, > > + ResponseDataSize > > + ); > > + } > > +#endif > > + > > +#if BtInterfaceSupport > > + if ((IpmiInstance->IpmiTransport2.InterfaceType =3D=3D SysInterfaceBt)= && > > + (IpmiInstance->IpmiTransport2.Interface.Bt.InterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiBtSendCommandToBmc ( > > + &IpmiInstance->IpmiTransport2, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + (UINT8) CommandDataSize, > > + ResponseData, > > + (UINT8*) ResponseDataSize, > > + NULL > > + ); > > + } > > +#endif > > + > > +#if SsifInterfaceSupport > > + if ((IpmiInstance->IpmiTransport2.InterfaceType =3D=3D SysInterfaceSsi= f) > + && > > + (IpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiSsifSendCommandToBmc ( > > + &IpmiInstance->IpmiTransport2, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + (UINT8) CommandDataSize, > > + ResponseData, > > + (UINT8*) ResponseDataSize, > > + NULL > > + ); > > + } > > +#endif > > + > > +#if IpmbInterfaceSupport > > + if ((IpmiInstance->IpmiTransport2.InterfaceType =3D=3D SysInterfaceIpm= b) > + && > > + (IpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiIpmbSendCommandToBmc ( > > + &IpmiInstance->IpmiTransport2, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + (UINT8) CommandDataSize, > > + ResponseData, > > + (UINT8*) ResponseDataSize, > > + NULL > > + ); > > + } > > +#endif > > + return EFI_UNSUPPORTED; > [Isaac] Should we comment this a bit more? It isn't really obvious if hi= tting this > is a valid configuration. Should there be an assert here to indicate tha= t one of > the interfaces must be enabled? Are the function comments about default > interface correct? Same for next function. > > +} // IpmiSendCommand2() > > + > > +EFI_STATUS > > +EFIAPI > > +IpmiSendCommand2Ex ( > > + IN IPMI_TRANSPORT2 *This, > > + IN UINT8 NetFunction, > > + IN UINT8 Lun, > > + IN UINT8 Command, > > + IN UINT8 *CommandData, > > + IN UINT32 CommandDataSize, > > + IN OUT UINT8 *ResponseData, > > + IN OUT UINT32 *ResponseDataSize, > > + IN SYSTEM_INTERFACE_TYPE InterfaceType > > + ) > > +{ > > +/*++ > > +Routine Description: > > + > > + This API use the specific interface type to send IPMI command > > + in the right mode to the appropiate device, ME or BMC. > > + > > +Arguments: > > + > > + This - Pointer to IPMI protocol instance > > + NetFunction - Net Function of command to send > > + Lun - LUN of command to send > > + Command - IPMI command to send > > + CommandData - Pointer to command data buffer, if needed > > + CommandDataSize - Size of command data buffer > > + ResponseData - Pointer to response data buffer > > + ResponseDataSize - Pointer to response data buffer size > > + InterfaceType - BMC Interface type. > > + > > +Returns: > > + > > + EFI_INVALID_PARAMETER - One of the input values is bad > > + EFI_DEVICE_ERROR - IPMI command failed > > + EFI_BUFFER_TOO_SMALL - Response buffer is too small > > + EFI_UNSUPPORTED - Command is not supported by BMC > > + EFI_SUCCESS - Command completed successfully > > + > > +--*/ > > + > > + IPMI_BMC_INSTANCE_DATA *IpmiInstance; > > + > > + if (This =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + IpmiInstance =3D INSTANCE_FROM_IPMI_TRANSPORT2_THIS(This); > > + > > +#if KcsInterfaceSupport > > + if ((InterfaceType =3D=3D SysInterfaceKcs) && > > + (IpmiInstance->IpmiTransport2.Interface.KcsInterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiSendCommand ( > > + &IpmiInstance->IpmiTransport, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + CommandDataSize, > > + ResponseData, > > + ResponseDataSize > > + ); > > + } > > +#endif > > + > > +#if BtInterfaceSupport > > + if ((InterfaceType =3D=3D SysInterfaceBt) && > > + (IpmiInstance->IpmiTransport2.Interface.Bt.InterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiBtSendCommandToBmc ( > > + &IpmiInstance->IpmiTransport2, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + (UINT8) CommandDataSize, > > + ResponseData, > > + (UINT8*) ResponseDataSize, > > + NULL > > + ); > > + } > > +#endif > > + > > +#if SsifInterfaceSupport > > + if ((InterfaceType =3D=3D SysInterfaceSsif) && > > + (IpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiSsifSendCommandToBmc ( > > + &IpmiInstance->IpmiTransport2, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + (UINT8) CommandDataSize, > > + ResponseData, > > + (UINT8*) ResponseDataSize, > > + NULL > > + ); > > + } > > +#endif > > + > > +#if IpmbInterfaceSupport > > + if ((InterfaceType =3D=3D SysInterfaceIpmb) && > > + (IpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitialized)) { > > + > > + return IpmiIpmbSendCommandToBmc ( > > + &IpmiInstance->IpmiTransport2, > > + NetFunction, > > + Lun, > > + Command, > > + CommandData, > > + (UINT8) CommandDataSize, > > + ResponseData, > > + (UINT8*) ResponseDataSize, > > + NULL > > + ); > > + } > > +#endif > > + return EFI_UNSUPPORTED; > > +} > > + > > EFI_STATUS > > EFIAPI > > IpmiGetBmcStatus ( > > @@ -89,6 +340,11 @@ Returns: > > > --*/ > > { > > + > > + if ((This =3D=3D NULL) || (BmcStatus =3D=3D NULL) || (ComAddress =3D= =3D NULL)) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > return IpmiBmcStatus ( > > This, > > BmcStatus, > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiHooks.h > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiHooks.h > index 823cc08c61..3933e07443 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > mon/IpmiHooks.h > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Com > m > +++ on/IpmiHooks.h > @@ -3,13 +3,21 @@ > > > @copyright > > Copyright 2014 - 2021 Intel Corporation.
> > + Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > > > #ifndef _IPMI_HOOKS_H > > #define _IPMI_HOOKS_H > > > > -#include "IpmiBmc.h" > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > [Isaac] Add a space before < in the two preceding lines. > > > > // > > // Internal(hook) function list > > @@ -54,6 +62,89 @@ Returns: > --*/ > > ; > > > > +EFI_STATUS > > +EFIAPI > > +IpmiSendCommand2 ( > > + IN IPMI_TRANSPORT2 *This, > > + IN UINT8 NetFunction, > > + IN UINT8 Lun, > > + IN UINT8 Command, > > + IN UINT8 *CommandData, > > + IN UINT32 CommandDataSize, > > + IN OUT UINT8 *ResponseData, > > + IN OUT UINT32 *ResponseDataSize > > + ) > > +/*++ > > + > > +Routine Description: > > + > > + This API use the default interface (PcdDefaultSystemInterface) to > + send IPMI command > > + in the right mode to the appropiate device, ME or BMC. > > + > > +Arguments: > > + > > + This - Pointer to IPMI protocol instance > > + NetFunction - Net Function of command to send > > + Lun - LUN of command to send > > + Command - IPMI command to send > > + CommandData - Pointer to command data buffer, if needed > > + CommandDataSize - Size of command data buffer > > + ResponseData - Pointer to response data buffer > > + ResponseDataSize - Pointer to response data buffer size > > + > > +Returns: > > + > > + EFI_INVALID_PARAMETER - One of the input values is bad > > + EFI_DEVICE_ERROR - IPMI command failed > > + EFI_BUFFER_TOO_SMALL - Response buffer is too small > > + EFI_UNSUPPORTED - Command is not supported by BMC > > + EFI_SUCCESS - Command completed successfully > > + > > +--*/ > > +; > > + > > +EFI_STATUS > > +EFIAPI > > +IpmiSendCommand2Ex ( > > + IN IPMI_TRANSPORT2 *This, > > + IN UINT8 NetFunction, > > + IN UINT8 Lun, > > + IN UINT8 Command, > > + IN UINT8 *CommandData, > > + IN UINT32 CommandDataSize, > > + IN OUT UINT8 *ResponseData, > > + IN OUT UINT32 *ResponseDataSize, > > + IN SYSTEM_INTERFACE_TYPE InterfaceType > > + ) > > +/*++ > > +Routine Description: > > + > > + This API use the specific interface type to send IPMI command > > + in the right mode to the appropiate device, ME or BMC. > > + > > +Arguments: > > + > > + This - Pointer to IPMI protocol instance > > + NetFunction - Net Function of command to send > > + Lun - LUN of command to send > > + Command - IPMI command to send > > + CommandData - Pointer to command data buffer, if needed > > + CommandDataSize - Size of command data buffer > > + ResponseData - Pointer to response data buffer > > + ResponseDataSize - Pointer to response data buffer size > > + InterfaceType - BMC Interface type. > > + > > +Returns: > > + > > + EFI_INVALID_PARAMETER - One of the input values is bad > > + EFI_DEVICE_ERROR - IPMI command failed > > + EFI_BUFFER_TOO_SMALL - Response buffer is too small > > + EFI_UNSUPPORTED - Command is not supported by BMC > > + EFI_SUCCESS - Command completed successfully > > + > > +--*/ > > +; > > + > > EFI_STATUS > > EFIAPI > > IpmiGetBmcStatus ( > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > GenericIpmi.inf > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > GenericIpmi.inf > index 8d80aeb6b5..1564ceb08a 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > GenericIpmi.inf > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > +++ GenericIpmi.inf > @@ -3,6 +3,7 @@ > # > > # @copyright > > # Copyright 2010 - 2021 Intel Corporation.
> > +# Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > # SPDX-License-Identifier: BSD-2-Clause-Patent > > ## > > > > @@ -49,16 +50,25 @@ > IoLib > > ReportStatusCodeLib > > TimerLib > > + BmcCommonInterfaceLib > > + BtInterfaceLib > > + SsifInterfaceLib > > + IpmbInterfaceLib > > > > [Protocols] > > gIpmiTransportProtocolGuid # PROTOCOL ALWAYS_PRODUCED > > gEfiVideoPrintProtocolGuid > > - > > -[Guids] > > + gIpmiTransport2ProtocolGuid > > > > [Pcd] > > gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiIoBaseAddress > > gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiBmcReadyDelayTimer > > + gIpmiFeaturePkgTokenSpaceGuid.PcdDefaultSystemInterface > > + gIpmiFeaturePkgTokenSpaceGuid.PcdBtInterfaceSupport > > + gIpmiFeaturePkgTokenSpaceGuid.PcdSsifInterfaceSupport > > + gIpmiFeaturePkgTokenSpaceGuid.PcdIpmbInterfaceSupport > > + gIpmiFeaturePkgTokenSpaceGuid.PcdKcsInterfaceSupport > > + gIpmiFeaturePkgTokenSpaceGuid.PcdBtControlPort > > > > [Depex] > > gEfiRuntimeArchProtocolGuid AND > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > IpmiInit.c > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > IpmiInit.c > index c333ca2e06..74d96f8684 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > IpmiInit.c > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/ > +++ IpmiInit.c > @@ -3,6 +3,7 @@ > > > @copyright > > Copyright 1999 - 2021 Intel Corporation.
> > + Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > > > @@ -13,6 +14,7 @@ > #include "IpmiBmc.h" > > #include "IpmiPhysicalLayer.h" > > #include > > +#include > > #ifdef FAST_VIDEO_SUPPORT > > #include > > #endif > > @@ -351,6 +353,130 @@ Returns: > return Status; > > } // GetDeviceId() > > > > +/** > > + Initialize the API and parameters for IPMI Transport2 Instance > > + > > + @param[in] IpmiInstance Pointer to IPMI Instance > > + > > + @return VOID Nothing. > > + > > +**/ > > +VOID > > +InitIpmiTransport2 ( > > + IN IPMI_BMC_INSTANCE_DATA *IpmiInstance > > + ) > > +{ > > + > > + IpmiInstance->IpmiTransport2.InterfaceType =3D FixedPcdGet8 > (PcdDefaultSystemInterface); > > + IpmiInstance->IpmiTransport2.IpmiTransport2BmcStatus =3D BmcStatusOk; > > + IpmiInstance->IpmiTransport2.IpmiSubmitCommand2 =3D > IpmiSendCommand2; > > + IpmiInstance->IpmiTransport2.IpmiSubmitCommand2Ex =3D > IpmiSendCommand2Ex; > > + > > +#if BtInterfaceSupport > > + InitBtInterfaceData(&IpmiInstance->IpmiTransport2); > > +#endif > > + > > +#if SsifInterfaceSupport > > + InitSsifInterfaceData(&IpmiInstance->IpmiTransport2); > > +#endif > > + > > +#if IpmbInterfaceSupport > > + InitIpmbInterfaceData(&IpmiInstance->IpmiTransport2); > > +#endif > > +} > > + > > +/** > > + Notify call back function. > > + > > + @param[in] Event Event which caused this handler. > > + @param[in] Context Context passed during Event Handler registration= . > > + > > + @return VOID Nothing. > > + > > +**/ > > +VOID > > +EFIAPI > > +DxeNotifyCallback ( > > + IN EFI_EVENT Event, > > + IN VOID *Context > > + ) > > +{ > > + EFI_STATUS Status; > > + IPMI_INTERFACE_STATE InterfaceState; > > + EFI_HANDLE Handle; > > + > > + InterfaceState =3D IpmiInterfaceNotReady; > > + > > +#if SsifInterfaceSupport > > + InitSsifInterfaceData(&mIpmiInstance->IpmiTransport2); > > + > > + if (mIpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitialized) { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } > > +#endif > > + > > +#if IpmbInterfaceSupport > > + InitIpmbInterfaceData(&mIpmiInstance->IpmiTransport2); > > + > > + if (mIpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitialized) { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } > > +#endif > > + // Default Interface data should be initialized to install Ipmi Transp= ort2 > Protocol. > > + if (InterfaceState !=3D IpmiInterfaceInitialized) { > > + return; > > + } > > + > > + Handle =3D NULL; > > + Status =3D gBS->InstallProtocolInterface ( > > + &Handle, > > + &gIpmiTransport2ProtocolGuid, > > + EFI_NATIVE_INTERFACE, > > + &mIpmiInstance->IpmiTransport2 > > + ); > > + ASSERT_EFI_ERROR (Status); > > +} > > + > > +/** > > + Registers protocol notify call back. > > + > > + @param[in] ProtocolGuid Pointer to Protocol Guid to register > > + call back. > > + > > + @retval EFI_INVALID_PARAMETER If the ProtocolGuid is 0 or NULL. > > + @retval Others Status of call back registration. > > + > > +**/ > > +EFI_STATUS > > +DxeRegisterProtocolCallback ( > > + IN EFI_GUID *ProtocolGuid > > + ) > > +{ > > + EFI_STATUS Status; > > + EFI_EVENT NotifyEvent; > > + VOID *Registration; > > + > > + if ((ProtocolGuid =3D=3D NULL) || > > + ((ProtocolGuid !=3D NULL) && IsZeroBuffer (ProtocolGuid, sizeof > + (EFI_GUID)))) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Status =3D gBS->CreateEvent ( > > + EVT_NOTIFY_SIGNAL, > > + TPL_NOTIFY, > > + DxeNotifyCallback, > > + NULL, > > + &NotifyEvent); > > + > > + if (!EFI_ERROR (Status)) { > > + Status =3D gBS->RegisterProtocolNotify ( > > + ProtocolGuid, > > + NotifyEvent, > > + &Registration); > > + } > > + > > + return Status; > > +} > > > > /** > > This function initializes KCS interface to BMC. > > @@ -376,7 +502,10 @@ InitializeIpmiKcsPhysicalLayer ( > UINT8 ErrorCount; > > EFI_HANDLE Handle; > > UINT8 Index; > > + IPMI_INTERFACE_STATE InterfaceState =3D IpmiInterfaceNotReady; > > +#if KcsInterfaceSupport > > EFI_STATUS_CODE_VALUE StatusCodeValue[MAX_SOFT_COUNT]; > > +#endif > > > > ErrorCount =3D 0; > > mImageHandle =3D ImageHandle; > > @@ -405,6 +534,8 @@ InitializeIpmiKcsPhysicalLayer ( > mIpmiInstance->Signature =3D SM_IPMI_BMC_SIGN= ATURE; > > mIpmiInstance->SlaveAddress =3D BMC_SLAVE_ADDRES= S; > > mIpmiInstance->BmcStatus =3D BMC_NOTREADY; > > + > > +#if KcsInterfaceSupport > > mIpmiInstance->IpmiTransport.IpmiSubmitCommand =3D > IpmiSendCommand; > > mIpmiInstance->IpmiTransport.GetBmcStatus =3D IpmiGetBmcStatus= ; > > > > @@ -454,7 +585,81 @@ InitializeIpmiKcsPhysicalLayer ( > ); > > ASSERT_EFI_ERROR (Status); > > } > > +#endif > > + > > + // Initialise the IPMI transport2 > [Isaac] I find the mismatch in spelling with some use of s and more use o= f z to > be confusing. > > + InitIpmiTransport2(mIpmiInstance); > [Isaac] Space before ( > > + > > + // Check interface data initialized successfully else register notif= y protocol. > > + for (Index =3D SysInterfaceKcs; Index < SysInterfaceMax; Index++) { > > + > > + switch (Index) { > > +#if KcsInterfaceSupport > [Isaac] A lot of the following code is not indented properly. > > + case SysInterfaceKcs: > > + if ((mIpmiInstance->BmcStatus !=3D BMC_HARDFAIL) && > + (mIpmiInstance->BmcStatus !=3D BMC_UPDATE_IN_PROGRESS)) { > > + BMC_INTERFACE_STATUS BmcStatus; > [Isaac] I would prefer this at the beginning of the function with other l= ocal > variables. > > + > + mIpmiInstance->IpmiTransport2.Interface.KcsInterfaceState =3D > + IpmiInterfaceInitialized; > > + Status =3D CheckSelfTestByInterfaceType( > [Isaac] Please insert space before (. Please add newline between the > interesting code and the variable init. > > + &mIpmiInstance->IpmiTransport2, > > + &BmcStatus, > > + SysInterfaceKcs); > [Isaac] Put ); on its own line as per other code conventions. > > + if (!EFI_ERROR (Status) && (BmcStatus !=3D > + BmcStatusHardFail)) { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } else { > > + > + mIpmiInstance->IpmiTransport2.Interface.KcsInterfaceState =3D > + IpmiInterfaceInitError; > > + } > > + } > > + break; > > +#endif > > + > > +#if BtInterfaceSupport > > + case SysInterfaceBt: > > + if > + (mIpmiInstance->IpmiTransport2.Interface.Bt.InterfaceState =3D=3D > + IpmiInterfaceInitialized){ > [Isaac] Space before { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } > > + break; > > +#endif > > + > > +#if SsifInterfaceSupport > > + case SysInterfaceSsif: > > + if > + (mIpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitialized) { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } else if > + (mIpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitError) { > > + // Register protocol notify for SMBUS Protocol. > > + Status =3D DxeRegisterProtocolCallback ( > > + > + &mIpmiInstance->IpmiTransport2.Interface.Ssif.SsifInterfaceApiGuid); > [Isaac] Indent this line 2 characters from the beginning of the function = name > on the prior line per coding style. Also put ); on its own line. > > + } > > + break; > > +#endif > > +#if IpmbInterfaceSupport > > + case SysInterfaceIpmb: > > + if > + (mIpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitialized) { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } else if > + (mIpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitError) { > > + // Register Protocol notify for I2C Protocol. > > + Status =3D DxeRegisterProtocolCallback ( > > + > + &mIpmiInstance->IpmiTransport2.Interface.Ipmb.IpmbInterfaceApiGuid); > [Isaac] Put ); on its own line as per other code conventions. > > + } > > + break; > > +#endif > > + default: > > + break; > > + } > > + } > > + > > + // Any one of the Interface data should be initialized to install IP= MI > Transport2 Protocol. > > + if (InterfaceState !=3D IpmiInterfaceInitialized) { > > + return EFI_SUCCESS; > > + } > > > > + Handle =3D NULL; > > + Status =3D gBS->InstallProtocolInterface ( > > + &Handle, > > + &gIpmiTransport2ProtocolGuid, > > + EFI_NATIVE_INTERFACE, > > + &mIpmiInstance->IpmiTransport2 > > + ); > > + ASSERT_EFI_ERROR (Status); > > return EFI_SUCCESS; > > } > > } // InitializeIpmiKcsPhysicalLayer() > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/SmmGenericIpmi.c > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/SmmGenericIpmi.c > index 1af2d18f79..adf59374b3 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/SmmGenericIpmi.c > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/ > +++ SmmGenericIpmi.c > @@ -3,6 +3,7 @@ > > > @copyright > > Copyright 1999 - 2021 Intel Corporation.
> > + Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > > > @@ -24,6 +25,7 @@ > #include "IpmiBmcCommon.h" > > #include "IpmiBmc.h" > > #include > > +#include > > > > IPMI_BMC_INSTANCE_DATA *mIpmiInstance; > > EFI_HANDLE mImageHandle; > > @@ -115,6 +117,123 @@ Returns: > return Status; > > } > > > > +/** > > + Initialize the API and parameters for IPMI Transport2 Instance > [Isaac] There are a lot of four space indents. > > + > > + @param[in] IpmiInstance Pointer to IPMI Instance > > + > > + @return VOID Nothing. > > + > > +**/ > > +VOID > > +InitIpmiTransport2 ( > > + IN IPMI_BMC_INSTANCE_DATA *IpmiInstance > > + ) > > +{ > > + > > + IpmiInstance->IpmiTransport2.InterfaceType =3D FixedPcdGet8 > (PcdDefaultSystemInterface); > > + IpmiInstance->IpmiTransport2.IpmiTransport2BmcStatus =3D BmcStatusOk; > > + IpmiInstance->IpmiTransport2.IpmiSubmitCommand2 =3D > IpmiSendCommand2; > > + IpmiInstance->IpmiTransport2.IpmiSubmitCommand2Ex =3D > IpmiSendCommand2Ex; > > + > > +#if BtInterfaceSupport > > + InitBtInterfaceData (&IpmiInstance->IpmiTransport2); > > +#endif > > + > > +#if SsifInterfaceSupport > > + InitSsifInterfaceData (&IpmiInstance->IpmiTransport2); > > +#endif > > + > > +#if IpmbInterfaceSupport > > + InitIpmbInterfaceData (&IpmiInstance->IpmiTransport2); > > +#endif > > +} > > + > > +/** > > + Notify call back to initialize the interfaces and install Smm Ipmi > > + protocol. > > + > > + @param[in] Protocol Pointer to the protocol guid. > > + @param[in] Interface Pointer to the protocol instance. > > + @param[in] Handle Handle on which the protocol is installed. > > + > > + @return EFI_STATUS Status of Notify call back. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +SmmNotifyCallback ( > > + IN CONST EFI_GUID *Protocol, > > + IN VOID *Interface, > > + IN EFI_HANDLE Handle > > + ) > > +{ > > + > > + EFI_STATUS Status; > > + IPMI_INTERFACE_STATE InterfaceState; > > + > > + InterfaceState =3D IpmiInterfaceNotReady; > > + > > +#if SsifInterfaceSupport > > + InitSsifInterfaceData(&mIpmiInstance->IpmiTransport2); > > + > > + if (mIpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitialized){ > [Isaac] Space before { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } > > +#endif > > + > > +#if IpmbInterfaceSupport > > + InitIpmbInterfaceData(&mIpmiInstance->IpmiTransport2); > > +#endif > > + > > + if (mIpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitialized){ > [Isaac] Space before { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } > > + if (InterfaceState !=3D IpmiInterfaceInitialized) { > > + return EFI_SUCCESS; > > + } > > + > > + // Default Interface data should be initialized to install Ipmi Transp= ort2 > Protocol. > > + if (InterfaceState =3D=3D IpmiInterfaceInitialized) { > > + Handle =3D NULL; > > + Status =3D gSmst->SmmInstallProtocolInterface ( > > + &Handle, > > + &gSmmIpmiTransport2ProtocolGuid, > > + EFI_NATIVE_INTERFACE, > > + &mIpmiInstance->IpmiTransport2 > > + ); > > + } > > + return EFI_SUCCESS; > > +} > > + > > +/** > > + Registers Protocol call back > > + > > + @param ProtocolGuid Pointer to Protocol GUID to register call = back > > + > > + @retval EFI_INVALID_PARAMETER If the ProtocolGuid is 0 or NULL. > > + @retval Others Status of Notify registration. > > +**/ > > +EFI_STATUS > > +SmmRegisterProtocolCallback ( > > + IN EFI_GUID *ProtocolGuid > > +) > [Isaac] Not Indented the same way other functions are. > > +{ > > + EFI_STATUS Status; > > + VOID *Registration; > > + > > + if ((ProtocolGuid =3D=3D NULL) || > > + ((ProtocolGuid !=3D NULL) && IsZeroBuffer (ProtocolGuid, sizeof > + (EFI_GUID)))) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Status =3D gSmst->SmmRegisterProtocolNotify ( > > + ProtocolGuid, > > + SmmNotifyCallback, > > + &Registration ); > > + return Status; > > +} > > + > > EFI_STATUS > > SmmInitializeIpmiKcsPhysicalLayer ( > > IN EFI_HANDLE ImageHandle, > > @@ -142,10 +261,13 @@ Returns: > UINT8 ErrorCount; > > EFI_HANDLE Handle; > > EFI_STATUS_CODE_VALUE StatusCodeValue[MAX_SOFT_COUNT]; > > + IPMI_INTERFACE_STATE InterfaceState; > > + UINT8 Index; > > > > DEBUG ((DEBUG_INFO,"SmmInitializeIpmiKcsPhysicalLayer entry \n")); > > - ErrorCount =3D 0; > > - mImageHandle =3D ImageHandle; > > + ErrorCount =3D 0; > > + mImageHandle =3D ImageHandle; > > + InterfaceState =3D IpmiInterfaceNotReady; > > > > mIpmiInstance =3D AllocateZeroPool (sizeof (IPMI_BMC_INSTANCE_DATA)); > > ASSERT (mIpmiInstance !=3D NULL); > > @@ -170,6 +292,7 @@ Returns: > mIpmiInstance->IpmiTransport.IpmiSubmitCommand =3D > IpmiSendCommand; > > mIpmiInstance->IpmiTransport.GetBmcStatus =3D IpmiGetBmcStatus= ; > > > > +#if KcsInterfaceSupport > > DEBUG ((DEBUG_INFO,"IPMI: Waiting for Getting BMC DID in SMM \n")); > > // > > // Get the Device ID and check if the system is in Force Update mode= . > > @@ -191,6 +314,79 @@ Returns: > &mIpmiInstance->IpmiTransport > > ); > > ASSERT_EFI_ERROR (Status); > > +#endif > > + > > + InitIpmiTransport2(mIpmiInstance); > [Isaac] Space before ( > > + > > + // Check interface data initialized successfully else register notif= y protocol. > > + for (Index =3D SysInterfaceKcs; Index < SysInterfaceMax; Index++) { > > + > > + switch (Index) { > > +#if KcsInterfaceSupport > > + case SysInterfaceKcs: > > + if ((mIpmiInstance->BmcStatus !=3D BMC_HARDFAIL) && > + (mIpmiInstance->BmcStatus !=3D BMC_UPDATE_IN_PROGRESS)) { > > + BMC_INTERFACE_STATUS BmcStatus; > [Isaac] Better to be with other local variables at beginning of function. > > + > + mIpmiInstance->IpmiTransport2.Interface.KcsInterfaceState =3D > + IpmiInterfaceInitialized; > > + Status =3D CheckSelfTestByInterfaceType( > [Isaac] Insert space before ( > > + > + &mIpmiInstance->IpmiTransport2, > > + &BmcStatus, > > + SysInterfaceKcs); > [Isaac] Place ); on own line. > > + if (!EFI_ERROR (Status) && (BmcStatus !=3D > + BmcStatusHardFail)) { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } else { > > + > + mIpmiInstance->IpmiTransport2.Interface.KcsInterfaceState =3D > + IpmiInterfaceInitError; > > + } > > + } > > + break; > > +#endif > > + > > +#if BtInterfaceSupport > > + case SysInterfaceBt: > > + if > + (mIpmiInstance->IpmiTransport2.Interface.Bt.InterfaceState =3D=3D > + IpmiInterfaceInitialized){ > [Isaac] Insert space before { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } > > + break; > > +#endif > > + > > +#if SsifInterfaceSupport > > + case SysInterfaceSsif: > > + if > + (mIpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitialized){ > [Isaac] Insert space before { > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } else if > + (mIpmiInstance->IpmiTransport2.Interface.Ssif.InterfaceState =3D=3D > + IpmiInterfaceInitError) { > > + // Register protocol notify for SMBUS Protocol. > > + Status =3D SmmRegisterProtocolCallback > + (&mIpmiInstance->IpmiTransport2.Interface.Ssif.SsifInterfaceApiGuid); > > + } > > + break; > > +#endif > > + > > +#if IpmbInterfaceSupport > > + case SysInterfaceIpmb: > > + if > + (mIpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitialized){ > > + InterfaceState =3D IpmiInterfaceInitialized; > > + } else if > + (mIpmiInstance->IpmiTransport2.Interface.Ipmb.InterfaceState =3D=3D > + IpmiInterfaceInitError) { > > + // Register protocol notify for SMBUS Protocol. > > + Status =3D SmmRegisterProtocolCallback > + (&mIpmiInstance->IpmiTransport2.Interface.Ipmb.IpmbInterfaceApiGuid); > > + } > > + break; > > +#endif > > + default: > > + break; > > + } > > + } > > + > > + // Default Interface data should be initialized to install Ipmi Tran= sport2 > Protocol. > > + if (InterfaceState =3D=3D IpmiInterfaceInitialized) { > > + Handle =3D NULL; > > + Status =3D gSmst->SmmInstallProtocolInterface ( > > + &Handle, > > + &gSmmIpmiTransport2ProtocolGuid, > > + EFI_NATIVE_INTERFACE, > > + &mIpmiInstance->IpmiTransport2 > > + ); > > + if (EFI_ERROR(Status)) { > > + DEBUG ((DEBUG_ERROR,"IPMI Transport2 protocol install > + Status =3D %r \n",Status)); > > + } > > + } > > > > DEBUG ((DEBUG_INFO,"SmmInitializeIpmiKcsPhysicalLayer exit \n")); > > > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/SmmGenericIpmi.inf > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/SmmGenericIpmi.inf > index f430195d1e..12dc17ae84 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/SmmGenericIpmi.inf > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Sm > m/ > +++ SmmGenericIpmi.inf > @@ -3,6 +3,7 @@ > # > > # @copyright > > # Copyright 2010 - 2021 Intel Corporation.
> > +# Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > # SPDX-License-Identifier: BSD-2-Clause-Patent > > ## > > > > @@ -39,15 +40,26 @@ > IoLib > > ReportStatusCodeLib > > TimerLib > > + BmcCommonInterfaceLib > > + BtInterfaceLib > > + SsifInterfaceLib > > + IpmbInterfaceLib > > > > [Protocols] > > gSmmIpmiTransportProtocolGuid # PROTOCOL > ALWAYS_PRODUCED > > + gSmmIpmiTransport2ProtocolGuid # PROTOCOL > ALWAYS_PRODUCED > > > > [Guids] > > > > [Pcd] > > gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiSmmIoBaseAddress > > gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiBmcReadyDelayTimer > > + gIpmiFeaturePkgTokenSpaceGuid.PcdDefaultSystemInterface > > + gIpmiFeaturePkgTokenSpaceGuid.PcdBtControlPort > > + gIpmiFeaturePkgTokenSpaceGuid.PcdBtInterfaceSupport > > + gIpmiFeaturePkgTokenSpaceGuid.PcdSsifInterfaceSupport > > + gIpmiFeaturePkgTokenSpaceGuid.PcdKcsInterfaceSupport > > + gIpmiFeaturePkgTokenSpaceGuid.PcdIpmbInterfaceSupport > > > > [Depex] > > gIpmiTransportProtocolGuid > > diff --git > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d > ec > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d > ec > index 8c1b902446..2131ec475b 100644 > --- > a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d > ec > +++ > b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d > +++ ec > @@ -9,6 +9,7 @@ > # > > # Copyright (c) 2019-2021, Intel Corporation. All rights reserved.
> > # Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved. > > +# Copyright (c) 1985 - 2023, American Megatrends International LLC. > +
> > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -43,16 +44,26 @@ > # > > IpmiBaseLib|Include/Library/IpmiBaseLib.h > > > > + ## @libraryclass Provides generic functions among all interfaces. > > + BmcCommonInterfaceLib|Include/Library/BmcCommonInterfaceLib.h > > + BtInterfaceLib|Include/Library/BtInterfaceLib.h > > + SsifInterfaceLib|Include/Library/SsifInterfaceLib.h > > + IpmbInterfaceLib|Include/Library/IpmbInterfaceLib.h > > + > > [Guids] > > gIpmiFeaturePkgTokenSpaceGuid =3D {0xc05283f6, 0xd6a8, 0x48f3, {0x9b= , > 0x59, 0xfb, 0xca, 0x71, 0x32, 0x0f, 0x12}} > > + gPeiIpmiHobGuid =3D {0xcb4d3e13, 0x1e34, 0x4373, {0x8a,= 0x81, > 0xe9, 0x0, 0x10, 0xf1, 0xdb, 0xa4}} > > > > [Ppis] > > gPeiIpmiTransportPpiGuid =3D {0x7bf5fecc, 0xc5b5, 0x4b25, {0x81, 0x1b, > 0xb4, 0xb5, 0xb, 0x28, 0x79, 0xf7}} > > + gPeiIpmiTransport2PpiGuid =3D {0x8122CEBD, 0xF4FD, 0x4EA8, { 0x97, > + 0x6C, 0xF0, 0x30, 0xAD, 0xDC, 0x4C, 0xB4 }} > > > > [Protocols] > > gIpmiTransportProtocolGuid =3D {0x6bb945e8, 0x3743, 0x433e, {0xb9, 0x= 0e, > 0x29, 0xb3, 0x0d, 0x5d, 0xc6, 0x30}} > > gSmmIpmiTransportProtocolGuid =3D {0x8bb070f1, 0xa8f3, 0x471d, {0x86, > 0x16, 0x77, 0x4b, 0xa3, 0xf4, 0x30, 0xa0}} > > gEfiVideoPrintProtocolGuid =3D {0x3dbf3e06, 0x9d0c, 0x40d3, {0xb2,= 0x17, > 0x45, 0x5f, 0x33, 0x9e, 0x29, 0x09}} > > + gIpmiTransport2ProtocolGuid =3D { 0x4A1D0E66, 0x5271, 0x4E22, { 0x83, > + 0xFE, 0x90, 0x92, 0x1B, 0x74, 0x82, 0x13 }} > > + gSmmIpmiTransport2ProtocolGuid =3D { 0x1DBD1503, 0x0A60, 0x4230, { > + 0xAA, 0xA3, 0x80, 0x16, 0xD8, 0xC3, 0xDE, 0x2F }} > > > > [PcdsFeatureFlag] > > > gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable|FALSE|BOOLEAN|0x > A0000001 > > @@ -61,6 +72,40 @@ > > gIpmiFeaturePkgTokenSpaceGuid.PcdMaxSOLChannels|3|UINT8|0xF000000 > 1 > > #When True, BIOS will send a Pre-Boot signal to BMC > > > gIpmiFeaturePkgTokenSpaceGuid.PcdSignalPreBootToBmc|FALSE|BOOLEAN| > 0xF0000002 > > + # typedef enum { > > + # SysInterfaceUnknown, // Unknown interface type. > > + # SysInterfaceKcs, // Kcs interface =3D 1. > > + # SysInterfaceSmic, // Smic interface =3D 2. > > + # SysInterfaceBt, // Bt interface =3D 3. > > + # SysInterfaceSsif, // Ssif interface =3D 4. > > + # SysInterfaceMax // Maximum interface type. > > + # } SYSTEM_INTERFACE_TYPE; > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdDefaultSystemInterface|1|UINT8|0xF00 > 0 > + 0003 > > + > > + #BT Base address, retry counter and delay parameters > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBtCommandRetryCounter|0x0004E400 > |UINT > + 32|0xF0000004 > > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBtControlPort|0xE4|UINT16|0xF00000 > 05 > > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBtBufferPort|0xE5|UINT16|0xF000000 > 6 > > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBtDelayPerRetry|15|UINT32|0xF00000 > 07 > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBtInterruptMaskPort|0xE6|UINT16|0xF > 00 > + 00008 > > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBtBufferSize|0x40|UINT8|0xF0000009 > > + > > + #SSIF slave address, retry counter and delay parameters > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdSsifSlaveAddress|0x10|UINT16|0xF000 > 00 > + 0A > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdSsifRequestRetriesDelay|0xCB20|UINT3 > 2 > + |0xF000000B > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdSsifCommandtRetryCounter|0x5|UINT1 > 6|0 > + xF000000C > > + > > + #Interface access type for BMC communication. 0-MMIO, 1-IO > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiDefaultAccessType|1|UINT8|0xF00 > 00 > + 00D > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdMmioBaseAddress|0x0|UINT32|0xF00 > 0000E > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBaseAddressRange|0x0|UINT32|0xF00 > 0000 > + F > > + > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBmcSlaveAddress|0x20|UINT32|0xF00 > 0001 > + 0 > > + > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdKcsInterfaceSupport|1|UINT8|0xF0000 > 01 > + 1 > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdBtInterfaceSupport|1|UINT8|0xF00000 > 12 > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdSsifInterfaceSupport|1|UINT8|0xF0000 > 0 > + 13 > > + > + > gIpmiFeaturePkgTokenSpaceGuid.PcdIpmbInterfaceSupport|1|UINT8|0xF00 > 000 > + 14 > > > > [PcdsDynamic, PcdsDynamicEx] > > > gIpmiFeaturePkgTokenSpaceGuid.PcdFRB2EnabledFlag|TRUE|BOOLEAN|0xD > 0000001 > > -- > 2.38.1.windows.1 > -The information contained in this message may be confidential and > proprietary to American Megatrends (AMI). This communication is intended > to be read only by the individual or entity to whom it is addressed or by= their > designee. If the reader of this message is not the intended recipient, yo= u are > on notice that any distribution of this message, in any form, is strictly > prohibited. Please promptly notify the sender by reply e-mail or by telep= hone > at 770-246-8600, and then delete or destroy all copies of the transmissio= n. > > >=20 >