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.109]) by mx.groups.io with SMTP id smtpd.web10.1814.1681970907216263753 for ; Wed, 19 Apr 2023 23:08:27 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="no key for verify" header.i=@amperemail.onmicrosoft.com header.s=selector1-amperemail-onmicrosoft-com header.b=sjkibaPs; spf=pass (domain: os.amperecomputing.com, ip: 40.107.96.109, mailfrom: tinhnguyen@os.amperecomputing.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XsUPt9oW0y4W89gfQTVJagOFaMIupwFgOJHgess30YOixF+cUc8ZEpttlXryYv/j9gULY3zbLH3BtEmW/7VdKufa6E9+QDBELi9SUnVmlkMcvY9TAG8V2il5vBq4MSSuwHtmgobwqqvL9guApsaGCYu8mxiiAr+JCYna9SZy0OOj5MSOGCbxZFY+FmamQLCU5Mwhyq1or3MqOA0Boc1gHgWEGJpKE3t1ZgS2yL+KYGVB45ei0H7j3jsIKqHKmikxVIPVoHjykrm7xb0vqy5wxNqdPL6VEkm2artJp9BHAMXNlK3a+zyFLkoGDpJ4EBeIlh4uPjZcCyIu61N1UFCLnQ== 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=lYpq2aGLlfdUDGK8zw+8ZLmMiMXr4lUpV8eyohHSl4U=; b=aTo4uYsXdhsWFrvXTmuW7979Es5yKrafmiO4Lpk3gDylc8INwjxNNm4X9DOiYQvuKFHrLubhi5bqWDUhFGHAwaNYiRcs/i4gqZoQt5lFEc5BUkHUxnffj/vN3Nyu+xiQvMZpeJ9O1gxtkOAU2jItVjz7u1sfJMOkMAxLd4VCNIvg2657qGkEgAZry2UFieSX1G/nZEcd34OIA+zpa+HhAvPVRQ2WwBXnrKjQI++rsINFzTbxKnEyHBFnaxJjupowQjfKxRKPP/PYCYNkR/vdYc8CoJU7vdytXpfmAWsk+1JE9wP2nmz1o3NwzUChPVnvFFqli5UC6cpKm1b25HHj6w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=os.amperecomputing.com; dmarc=pass action=none header.from=amperemail.onmicrosoft.com; dkim=pass header.d=amperemail.onmicrosoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amperemail.onmicrosoft.com; s=selector1-amperemail-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=lYpq2aGLlfdUDGK8zw+8ZLmMiMXr4lUpV8eyohHSl4U=; b=sjkibaPsjUtjbstsJEePsM4WaLLc1ccoGIIFQvlVCJbrJgs6REKW9K5C/YAiqmjOtC8MlZbzvRchBdttqe8ut6MMcTR59/cA8Q7uVjJYSuIQ/o2Yq56gjHnuhzA4SABP/oySfIAG4FDcovaNSIB9lQw9uax+Nsm8B4ud0/6dtM0= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amperemail.onmicrosoft.com; Received: from DM5PR0102MB3336.prod.exchangelabs.com (2603:10b6:4:9f::11) by PH7PR01MB8074.prod.exchangelabs.com (2603:10b6:510:2ba::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6319.20; Thu, 20 Apr 2023 06:08:21 +0000 Received: from DM5PR0102MB3336.prod.exchangelabs.com ([fe80::bb9e:46d1:ae4b:caf2]) by DM5PR0102MB3336.prod.exchangelabs.com ([fe80::bb9e:46d1:ae4b:caf2%6]) with mapi id 15.20.6319.020; Thu, 20 Apr 2023 06:08:20 +0000 Message-ID: <0e8628de-5da9-849d-db7d-ae0b6be126b1@amperemail.onmicrosoft.com> Date: Thu, 20 Apr 2023 13:08:11 +0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [edk2-devel] [edk2-platforms][PATCH V2 02/14] ManageabilityPkg: Support Maximum Transfer Unit To: devel@edk2.groups.io, abner.chang@amd.com Cc: Isaac Oram , Abdul Lateef Attar , Nickle Wang , Igor Kulchytskyy References: <20230418071543.1951-1-abner.chang@amd.com> <20230418071543.1951-3-abner.chang@amd.com> From: "Tinh Nguyen" In-Reply-To: <20230418071543.1951-3-abner.chang@amd.com> X-ClientProxiedBy: SI2P153CA0035.APCP153.PROD.OUTLOOK.COM (2603:1096:4:190::14) To DM5PR0102MB3336.prod.exchangelabs.com (2603:10b6:4:9f::11) Return-Path: tinhnguyen@os.amperecomputing.com MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DM5PR0102MB3336:EE_|PH7PR01MB8074:EE_ X-MS-Office365-Filtering-Correlation-Id: 46675043-e8d1-44fd-c4f4-08db4165a404 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ruPiwbLH1w2r+9P2OOylSYEfi72HSMrEouLD4Q/V7uYFQuLgMgd0s6xh0GlAP4hhSY8KXlbvFJhcsDPX1ymqTGAbrLmsUPJrejDE6oYjISWfwBjoD1tTPcAsJxUxPjJeuEiBeEUcG04sbKgGvB4ZYtL9wUVa/L4Kq+iOHXTWlK4TBNtBr34Ix2M1B8ni/WKI8SxSlsRsJnuAH84iCSDVF5iybafG2fEy/uQ3JBkTMgTLXN1hjLNNzewSFcKrmbCMNHCrTa4TnF+t1O8QNGtsxZ4BEQ95ZXKVNYviuu9mqxcTQUweLBppaljno71dsI5CA+wkYy3iHrKVdjYLdhAOa8tmPdWf4QteHL5eLyINgfR83+fHmIr831y99WB3M7wxviENzbbysXHWjrO7BQnCdoWrwxgy46pfXR4ucfin8jWemEIoWxj23ALzxC2byVCdvOHMOUSqoFpkGkIk6e0ZlDI5+d1fz/jYz3rF2uDGhbyFasBcfnKByQZnrUPgNGKVlTq/NVz6aVwR++9ZydhoiESF0ARiSec5DZbavsu2CXjEAG5TtojyK/Edo6QHAeEbIUxA2N/OOtZ7JBVogMPV8goKTCPLLS4O6zrGNBnKDKbjoU3OzOkgBHj0Y25yd1YeJnTN2M6PJvRTN68CEUe0CIs5FhxaMefijMQ9yoIW1jY= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR0102MB3336.prod.exchangelabs.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(136003)(396003)(366004)(346002)(39850400004)(376002)(451199021)(8676002)(8936002)(5660300002)(38100700002)(38350700002)(316002)(4326008)(41300700001)(2906002)(31696002)(30864003)(53546011)(6506007)(6512007)(26005)(6486002)(52116002)(6666004)(2616005)(83380400001)(42882007)(186003)(31686004)(19627235002)(54906003)(83170400001)(66556008)(66946007)(66476007)(478600001)(43740500002)(44824005);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MHNaTjZuOXh3UkxzdzJjVUQyZXMrZTFIMmczQ1hvbTljZWZRSEZpU01ERU5U?= =?utf-8?B?YTRzb05OKzZDWDd6RUthRlBhdEZCZmlGYXNXV2xsK2hTVERESFRrZkl3N1Ex?= =?utf-8?B?WlRMQi96NGo4djhoSy9nUE10TkFFeTJRbk1iZURWL1RERzlvbGsvWWFVdDRa?= =?utf-8?B?ejFZSDBSRE43RTljcWtUSGljbk1oY3o1UU5jOTlzcnpyUzhMaHFySjVCcEl1?= =?utf-8?B?V1RvcUlwbjJwT0JacEFicUs5cEY0MmdnR3VraDNnQ05ZVXZHUDJIMG5SOHRZ?= =?utf-8?B?anBFeHgvYWNtMklXYWtNY2xRS01hQUlpcTNsMWNBck1JOGs1TUd1dlBIVjVJ?= =?utf-8?B?aFdGT1l5SG9kR3dsWmN0WklIbW5IeC9zTTVuNU8wQ3hTTDdyb1hZb2FRUlVG?= =?utf-8?B?VTJESW9kblJxbDE1aUVJZ3dCdE9aSk1PbWhwclEyek0vYWFudktBaGdxcSto?= =?utf-8?B?T0F5d2o4UDFldlFRa1owaUhJcnRlZ3hHaytjR2RlTllsRW9uT1dOZk1oREdq?= =?utf-8?B?UVhwM1hIbXhIbVN4a28wK1FoZG1SVlF0RkttaFFleVJqMVpKR1NLUWd3cit0?= =?utf-8?B?d3hoU09jZExVaWRQR2gyOCs0Y2J2WjdXVFhLSnJ3eXdIRmc1MXRtOVdvSHFP?= =?utf-8?B?WHZRUjZLbGJGZU5CdTVQN1pPR1RPL1NBd1NGVWsrVHdHREp2VCs1NlZpV1dr?= =?utf-8?B?YXAxSEZEd2U2M25WYmpmOHZyU0Fjb2hyRnFJQ0Q1Z2ZtRWxUNjN2OVVseGll?= =?utf-8?B?MTNnVnNRQ2phclVWMUs4VG1GRzdzUlB3djlaNlZOZm5BT0ZDZ2tnY25ydnBh?= =?utf-8?B?U2g3VE45a2xXTUEvdmIzNnJpalNRbWp3ZEgyK3B0c1NJYnU5d0QrQ1hKK2lX?= =?utf-8?B?c0M5NHdEQm05QS9zdlZaeGc5UHc5RU8rZTlEY2JKcW5GYjdreEtaQmg4WnF5?= =?utf-8?B?UU1CaVJHVDRQVWFYNXdXMG1sb2JYbTAwRmI3UC9KcEh2V1NxQ000TjVnSmt5?= =?utf-8?B?Qyt0M3puamh0aGZ3V1pnWFJXbTdTVGVYa3gybHAvY2RiZ3p4dFNkc3l3MkU3?= =?utf-8?B?aFpOUkUvNEI4S0lvTEN5VHRTc3B2ekFxR3NiVldRRnFsWk15WklFaCtIWVlV?= =?utf-8?B?dTVUc1NxSTNlUDcwVGJ2ZE0vQk9tTDdva2ZEcFpzZEpqdVNqNXhJUnhnUFZv?= =?utf-8?B?b0ppbDVvazBnejFpV0k3QkIxUHFOejQ4Y0tsQXBEYnJhMGkrbWJMa3pqYjZD?= =?utf-8?B?Z0Q4ZnRITjkxbmhOaUxzRTN0RWlDc0NESXR5MHZ1MEJnajlJT3BzOEJhNXFW?= =?utf-8?B?R1lzREVVREw2eWZuMENTaDFtZndZSENvRFRPZUFBc2VRd3Iwb3VWK1A5UitH?= =?utf-8?B?aDUvb25zeFZaejRMV1VQN1FGeHJyU1ZxT0pPU1J6N1FUZGg1OTlxTHdvVjhs?= =?utf-8?B?NmV2Slh4MHZEV0xlcU5uY1NrNHNIZ0JUZGtaQmdScityRzkxOUEwcEozM25M?= =?utf-8?B?TlM0OE9BR3RkUkYySWdTUVZudkFoR1g3aDNHR0xsa2NibjQvaTIwdzJoa3Bv?= =?utf-8?B?WndJd01KZmFmekpzK2xiWGRKaWJlUmJ5TjVoaEh2SnYrZjliOEgrMDhkZjZJ?= =?utf-8?B?UnpocWZ3a1VSZEZLZXl6VVhRSkNLdFpqWmVsbThlK3lJTTd0QnlaOG1DN2tt?= =?utf-8?B?TUk5WDhjNjIzSGNPRURNeFJ0UzlQaWNpYnRydm5iNWRzd2RzL2Y5RDhWdGRU?= =?utf-8?B?SVEzVnp6ZjFtMGJLenN6WGxWbUxhWlQ4NXF0VnZwN3poN1dGM1NuaVAwMGM0?= =?utf-8?B?anhtQWc0S3VRZnRKcFI1dlYydkNOVTNONUZlUDlaTC84MkxyRy9vbXBnL0Q3?= =?utf-8?B?Y0FlOVByWmFxcGRub2pWOG45VVMzZURVM0dEY0p4VXI5OSs5djhjMldLdVgw?= =?utf-8?B?VGp2S3djOWNzVjhuRWJQek9hQlVWZ3JxQVhkbGIyRXY3U3BZc3ZNdnNaVTMy?= =?utf-8?B?a0YxOC9kTmxtQVA1ZXBuREp1YkgxK2t1ZXBjTk9lUXAwOVIwNm15MDQvWVRQ?= =?utf-8?B?aStqcUhFREh3VjdiRDEwbS8wKzQvRlZLcDRDYWR3UExSNFJ0QWMvYzBVaHpr?= =?utf-8?B?UStTVHozMWkvNFVibWlnLzROaDBhZzdJQXhMK2IwclpGUG5odm92YnBIdVhr?= =?utf-8?Q?N9Mo1d1o8LWvetaD1M1UTFR8BYi8EhYfb8VYxC6gEA62?= X-OriginatorOrg: amperemail.onmicrosoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: 46675043-e8d1-44fd-c4f4-08db4165a404 X-MS-Exchange-CrossTenant-AuthSource: DM5PR0102MB3336.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2023 06:08:20.7591 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Q9C1tbh85hGXu05tr3lDpDA6k/Du+/FMYgCgYYZaeHXSK+YJG/vYtMfNnjzYCDNPhz4u8GPvWoAV/6OzYv0mk1YjpFWPQ+7C9cldSlJcOfZAx+jcTp1NOhbg1ympsaLI X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR01MB8074 Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Abner, I have some inline comments below On 18/04/2023 14:15, Chang, Abner via groups.io wrote: > From: Abner Chang > > Update GetTransportCapability to support > Maximum Transfer Unit (MTU) of transport interface. > > Signed-off-by: Abner Chang > Cc: Isaac Oram > Cc: Abdul Lateef Attar > Cc: Nickle Wang > Cc: Igor Kulchytskyy > --- > .../Library/ManageabilityTransportLib.h | 33 ++++++++--- > .../Common/ManageabilityTransportKcs.h | 2 + > .../IpmiProtocol/Pei/IpmiPpiInternal.h | 8 ++- > .../BaseManageabilityTransportNull.c | 18 ++++-- > .../Dxe/ManageabilityTransportKcs.c | 57 +++++++++++-------- > .../Universal/IpmiProtocol/Dxe/IpmiProtocol.c | 24 ++++++-- > .../Universal/IpmiProtocol/Pei/IpmiPpi.c | 51 ++++++++++------- > .../Universal/IpmiProtocol/Smm/IpmiProtocol.c | 24 ++++++-- > 8 files changed, 145 insertions(+), 72 deletions(-) > > diff --git a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.h b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.h > index c022b4ac5c..d86d0d87d5 100644 > --- a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.h > +++ b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.h > @@ -14,6 +14,9 @@ > #define MANAGEABILITY_TRANSPORT_TOKEN_VERSION ((MANAGEABILITY_TRANSPORT_TOKEN_VERSION_MAJOR << 8) |\ > MANAGEABILITY_TRANSPORT_TOKEN_VERSION_MINOR) > > +#define MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY(a) (1 << ((a & MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_MASK) >>\ > + MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_BIT_POSITION)) > + > typedef struct _MANAGEABILITY_TRANSPORT_FUNCTION_V1_0 MANAGEABILITY_TRANSPORT_FUNCTION_V1_0; > typedef struct _MANAGEABILITY_TRANSPORT MANAGEABILITY_TRANSPORT; > typedef struct _MANAGEABILITY_TRANSPORT_TOKEN MANAGEABILITY_TRANSPORT_TOKEN; > @@ -68,8 +71,17 @@ typedef UINT32 MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS; > /// Additional transport interface features. > /// > typedef UINT32 MANAGEABILITY_TRANSPORT_CAPABILITY; > +/// Bit 0 > #define MANAGEABILITY_TRANSPORT_CAPABILITY_MULTIPLE_TRANSFER_TOKENS 0x00000001 > -#define MANAGEABILITY_TRANSPORT_CAPABILITY_ASYNCHRONOUS_TRANSFER 0x00000002 > +/// Bit 1 > +#define MANAGEABILITY_TRANSPORT_CAPABILITY_ASYNCHRONOUS_TRANSFER 0x00000002 > +/// Bit 2 - Reserved > +/// Bit 7:3 - Transport interface maximum payload size, which is (2 ^ bit[7:3] - 1) > +/// bit[7:3] means no maximum payload. I am confused with your definition here. Why does it have to be a power of 2? And we should separate request payload size and response payload size Can you clarify more about that? Another question, only PEI_IPMI_PPI_INTERNAL contains MaxPayloadSize, how do IPMI/MCTP/PLDM protocol provide Maxpayloadsize to caller? > +#define MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_MASK 0x000000f8 > +#define MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_BIT_POSITION 3 > +#define MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AVAILABLE 0x00 > +/// Bit 8:31 - Reserved > > /// > /// Definitions of Manageability transport interface functions. > @@ -187,15 +199,20 @@ AcquireTransportSession ( > ); > > /** > - This function returns the transport capabilities. > - > - @param [out] TransportFeature Pointer to receive transport capabilities. > - See the definitions of > - MANAGEABILITY_TRANSPORT_CAPABILITY. > - > + This function returns the transport capabilities according to > + the manageability protocol. > + > + @param [in] TransportToken Transport token acquired from manageability > + transport library. > + @param [out] TransportFeature Pointer to receive transport capabilities. > + See the definitions of > + MANAGEABILITY_TRANSPORT_CAPABILITY. > + @retval EFI_SUCCESS TransportCapability is returned successfully. > + @retval EFI_INVALID_PARAMETER TransportToken is not a valid token. > **/ > -VOID > +EFI_STATUS > GetTransportCapability ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability > ); > > diff --git a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/ManageabilityTransportKcs.h b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/ManageabilityTransportKcs.h > index f1758ffd8f..2cdf60ba7e 100644 > --- a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/ManageabilityTransportKcs.h > +++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/ManageabilityTransportKcs.h > @@ -32,6 +32,8 @@ typedef struct { > #define IPMI_KCS_GET_STATE(s) (s >> 6) > #define IPMI_KCS_SET_STATE(s) (s << 6) > > +#define MCTP_KCS_MTU_IN_POWER_OF_2 8 > + > /// 5 sec, according to IPMI spec > #define IPMI_KCS_TIMEOUT_5_SEC 5000*1000 > #define IPMI_KCS_TIMEOUT_1MS 1000 > diff --git a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInternal.h b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInternal.h > index bbe0c8c5cb..4b6bdc97a9 100644 > --- a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInternal.h > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInternal.h > @@ -17,9 +17,11 @@ > #define MANAGEABILITY_IPMI_PPI_INTERNAL_FROM_LINK(a) CR (a, PEI_IPMI_PPI_INTERNAL, PeiIpmiPpi, MANAGEABILITY_IPMI_PPI_INTERNAL_SIGNATURE) > > typedef struct { > - UINT32 Signature; > - MANAGEABILITY_TRANSPORT_TOKEN *TransportToken; > - PEI_IPMI_PPI PeiIpmiPpi; > + UINT32 Signature; > + MANAGEABILITY_TRANSPORT_TOKEN *TransportToken; > + MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability; > + UINT32 TransportMaximumPayload; > + PEI_IPMI_PPI PeiIpmiPpi; > } PEI_IPMI_PPI_INTERNAL; > > #endif // MANAGEABILITY_IPMI_PPI_INTERNAL_H_ > diff --git a/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.c b/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.c > index 49fc8c0f71..3aa68578a6 100644 > --- a/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.c > +++ b/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLib/BaseManageabilityTransportNull.c > @@ -31,19 +31,25 @@ AcquireTransportSession ( > } > > /** > - This function returns the transport capabilities. > - > - @param [out] TransportFeature Pointer to receive transport capabilities. > - See the definitions of > - MANAGEABILITY_TRANSPORT_CAPABILITY. > + This function returns the transport capabilities according to > + the manageability protocol. > > + @param [in] TransportToken Transport token acquired from manageability > + transport library. > + @param [out] TransportFeature Pointer to receive transport capabilities. > + See the definitions of > + MANAGEABILITY_TRANSPORT_CAPABILITY. > + @retval EFI_SUCCESS TransportCapability is returned successfully. > + @retval EFI_INVALID_PARAMETER TransportToken is not a valid token. > **/ > -VOID > +EFI_STATUS > GetTransportCapability ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability > ) > { > *TransportCapability = 0; > + return EFI_SUCCESS; > } > > /** > diff --git a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/ManageabilityTransportKcs.c b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/ManageabilityTransportKcs.c > index ab416e5449..7d85378fc1 100644 > --- a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/ManageabilityTransportKcs.c > +++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/ManageabilityTransportKcs.c > @@ -62,7 +62,7 @@ KcsTransportInit ( > } > > if (HardwareInfo.Kcs == NULL) { > - DEBUG ((DEBUG_INFO, "%a: Hardware information is not provided, use dfault settings.\n", __FUNCTION__)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Hardware information is not provided, use dfault settings.\n", __FUNCTION__)); > mKcsHardwareInfo.MemoryMap = MANAGEABILITY_TRANSPORT_KCS_IO_MAP_IO; > mKcsHardwareInfo.IoBaseAddress.IoAddress16 = PcdGet16 (PcdIpmiKcsIoBaseAddress); > mKcsHardwareInfo.IoDataInAddress.IoAddress16 = mKcsHardwareInfo.IoBaseAddress.IoAddress16 + IPMI_KCS_DATA_IN_REGISTER_OFFSET; > @@ -81,21 +81,21 @@ KcsTransportInit ( > // Get protocol specification name. > ManageabilityProtocolName = HelperManageabilitySpecName (TransportToken->ManageabilityProtocolSpecification); > > - DEBUG ((DEBUG_INFO, "%a: KCS transport hardware for %s is:\n", __FUNCTION__, ManageabilityProtocolName)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: KCS transport hardware for %s is:\n", __FUNCTION__, ManageabilityProtocolName)); > if (mKcsHardwareInfo.MemoryMap) { > - DEBUG ((DEBUG_INFO, "Memory Map I/O\n", __FUNCTION__)); > - DEBUG ((DEBUG_INFO, "Base Memory Address : 0x%08x\n", mKcsHardwareInfo.IoBaseAddress.IoAddress32)); > - DEBUG ((DEBUG_INFO, "Data in Address : 0x%08x\n", mKcsHardwareInfo.IoDataInAddress.IoAddress32)); > - DEBUG ((DEBUG_INFO, "Data out Address : 0x%08x\n", mKcsHardwareInfo.IoDataOutAddress.IoAddress32)); > - DEBUG ((DEBUG_INFO, "Command Address : 0x%08x\n", mKcsHardwareInfo.IoCommandAddress.IoAddress32)); > - DEBUG ((DEBUG_INFO, "Status Address : 0x%08x\n", mKcsHardwareInfo.IoStatusAddress.IoAddress32)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Memory Map I/O\n", __FUNCTION__)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Base Memory Address : 0x%08x\n", mKcsHardwareInfo.IoBaseAddress.IoAddress32)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data in Address : 0x%08x\n", mKcsHardwareInfo.IoDataInAddress.IoAddress32)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data out Address : 0x%08x\n", mKcsHardwareInfo.IoDataOutAddress.IoAddress32)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Command Address : 0x%08x\n", mKcsHardwareInfo.IoCommandAddress.IoAddress32)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Status Address : 0x%08x\n", mKcsHardwareInfo.IoStatusAddress.IoAddress32)); > } else { > - DEBUG ((DEBUG_INFO, "I/O Map I/O\n")); > - DEBUG ((DEBUG_INFO, "Base I/O port : 0x%04x\n", mKcsHardwareInfo.IoBaseAddress.IoAddress16)); > - DEBUG ((DEBUG_INFO, "Data in I/O port : 0x%04x\n", mKcsHardwareInfo.IoDataInAddress.IoAddress16)); > - DEBUG ((DEBUG_INFO, "Data out I/O port: 0x%04x\n", mKcsHardwareInfo.IoDataOutAddress.IoAddress16)); > - DEBUG ((DEBUG_INFO, "Command I/O port : 0x%04x\n", mKcsHardwareInfo.IoCommandAddress.IoAddress16)); > - DEBUG ((DEBUG_INFO, "Status I/O port : 0x%04x\n", mKcsHardwareInfo.IoStatusAddress.IoAddress16)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "I/O Map I/O\n")); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Base I/O port : 0x%04x\n", mKcsHardwareInfo.IoBaseAddress.IoAddress16)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data in I/O port : 0x%04x\n", mKcsHardwareInfo.IoDataInAddress.IoAddress16)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data out I/O port: 0x%04x\n", mKcsHardwareInfo.IoDataOutAddress.IoAddress16)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Command I/O port : 0x%04x\n", mKcsHardwareInfo.IoCommandAddress.IoAddress16)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Status I/O port : 0x%04x\n", mKcsHardwareInfo.IoStatusAddress.IoAddress16)); > } if those code is just for debugging, you should put them into DEBUG_CODE_BEGIN() and DEBUG_CODE_END() > > return EFI_SUCCESS; > @@ -221,7 +221,7 @@ KcsTransportTransmitReceive ( > EFI_STATUS Status; > MANAGEABILITY_IPMI_TRANSPORT_HEADER *TransmitHeader; > > - if (TransportToken == NULL || TransferToken == NULL) { > + if ((TransportToken == NULL) || (TransferToken == NULL)) { > DEBUG ((DEBUG_ERROR, "%a: Invalid transport token or transfer token.\n", __FUNCTION__)); > return; > } > @@ -298,6 +298,7 @@ AcquireTransportSession ( > DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for MANAGEABILITY_TRANSPORT_KCS\n", __FUNCTION__)); > return EFI_OUT_OF_RESOURCES; > } > + > KcsTransportToken->Token.Transport = AllocateZeroPool (sizeof (MANAGEABILITY_TRANSPORT)); > if (KcsTransportToken->Token.Transport == NULL) { > FreePool (KcsTransportToken); > @@ -329,21 +330,29 @@ AcquireTransportSession ( > } > > /** > - This function returns the transport capabilities. > - > - @param [out] TransportFeature Pointer to receive transport capabilities. > - See the definitions of > - MANAGEABILITY_TRANSPORT_CAPABILITY. > - > + This function returns the transport capabilities according to > + the manageability protocol. > + > + @param [in] TransportToken Transport token acquired from manageability > + transport library. > + @param [out] TransportFeature Pointer to receive transport capabilities. > + See the definitions of > + MANAGEABILITY_TRANSPORT_CAPABILITY. > + @retval EFI_SUCCESS TransportCapability is returned successfully. > + @retval EFI_INVALID_PARAMETER TransportToken is not a valid token. > **/ > -VOID > +EFI_STATUS > GetTransportCapability ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability > ) > { > - if (TransportCapability != NULL) { > - *TransportCapability = 0; > + if ((TransportToken == NULL) || (TransportCapability == NULL)) { > + return EFI_INVALID_PARAMETER; > } > + > + *TransportCapability = 0; > + return EFI_SUCCESS; > } > > /** > diff --git a/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c > index 05175ee448..51d5c7f0ba 100644 > --- a/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c > @@ -17,9 +17,9 @@ > > #include "IpmiProtocolCommon.h" > > -MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL; > -CHAR16 *mTransportName; > - > +MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL; > +CHAR16 *mTransportName; > +UINT32 TransportMaximumPayload; > MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION mHardwareInformation; > > /** > @@ -92,8 +92,6 @@ DxeIpmiEntry ( > MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability; > MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS TransportAdditionalStatus; > > - GetTransportCapability (&TransportCapability); > - > Status = HelperAcquireManageabilityTransport ( > &gManageabilityProtocolIpmiGuid, > &mTransportToken > @@ -103,8 +101,22 @@ DxeIpmiEntry ( > return Status; > } > > + Status = GetTransportCapability (mTransportToken, &TransportCapability); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to GetTransportCapability().\n", __FUNCTION__)); > + return Status; > + } > + > + TransportMaximumPayload = MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY (TransportCapability); > + if (TransportMaximumPayload == (1 << MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AVAILABLE)) { > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface maximum payload is undefined.\n", __FUNCTION__)); > + } else { > + TransportMaximumPayload -= 1; > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface for IPMI protocol has maximum payload %x.\n", __FUNCTION__, TransportMaximumPayload)); > + } > + > mTransportName = HelperManageabilitySpecName (mTransportToken->Transport->ManageabilityTransportSpecification); > - DEBUG ((DEBUG_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__, mTransportName)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__, mTransportName)); > > // > // Setup hardware information according to the transport interface. > diff --git a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c > index f839cd7387..8bf1e794f0 100644 > --- a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c > @@ -51,19 +51,19 @@ PeiIpmiSubmitCommand ( > IN OUT UINT32 *ResponseDataSize > ) > { > - EFI_STATUS Status; > - PEI_IPMI_PPI_INTERNAL *PeiIpmiPpiinternal; > - > - PeiIpmiPpiinternal = MANAGEABILITY_IPMI_PPI_INTERNAL_FROM_LINK(This); > - Status = CommonIpmiSubmitCommand ( > - PeiIpmiPpiinternal->TransportToken, > - NetFunction, > - Command, > - RequestData, > - RequestDataSize, > - ResponseData, > - ResponseDataSize > - ); > + EFI_STATUS Status; > + PEI_IPMI_PPI_INTERNAL *PeiIpmiPpiinternal; > + > + PeiIpmiPpiinternal = MANAGEABILITY_IPMI_PPI_INTERNAL_FROM_LINK (This); > + Status = CommonIpmiSubmitCommand ( > + PeiIpmiPpiinternal->TransportToken, > + NetFunction, > + Command, > + RequestData, > + RequestDataSize, > + ResponseData, > + ResponseDataSize > + ); > return Status; > } > > @@ -87,29 +87,28 @@ PeiIpmiEntry ( > CHAR16 *TransportName; > PEI_IPMI_PPI_INTERNAL *PeiIpmiPpiinternal; > EFI_PEI_PPI_DESCRIPTOR *PpiDescriptor; > - MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability; > MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS TransportAdditionalStatus; > MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION HardwareInformation; > > - PeiIpmiPpiinternal = (PEI_IPMI_PPI_INTERNAL *)AllocateZeroPool (sizeof(PEI_IPMI_PPI_INTERNAL)); > + PeiIpmiPpiinternal = (PEI_IPMI_PPI_INTERNAL *)AllocateZeroPool (sizeof (PEI_IPMI_PPI_INTERNAL)); > if (PeiIpmiPpiinternal == NULL) { > DEBUG ((DEBUG_ERROR, "%a: Not enough memory for PEI_IPMI_PPI_INTERNAL.\n", __FUNCTION__)); > return EFI_OUT_OF_RESOURCES; > } > - PpiDescriptor = (EFI_PEI_PPI_DESCRIPTOR *)AllocateZeroPool (sizeof(EFI_PEI_PPI_DESCRIPTOR)); > + > + PpiDescriptor = (EFI_PEI_PPI_DESCRIPTOR *)AllocateZeroPool (sizeof (EFI_PEI_PPI_DESCRIPTOR)); > if (PpiDescriptor == NULL) { > DEBUG ((DEBUG_ERROR, "%a: Not enough memory for EFI_PEI_PPI_DESCRIPTOR.\n", __FUNCTION__)); > return EFI_OUT_OF_RESOURCES; > } > > - PeiIpmiPpiinternal->Signature = MANAGEABILITY_IPMI_PPI_INTERNAL_SIGNATURE; > + PeiIpmiPpiinternal->Signature = MANAGEABILITY_IPMI_PPI_INTERNAL_SIGNATURE; > PeiIpmiPpiinternal->PeiIpmiPpi.IpmiSubmitCommand = PeiIpmiSubmitCommand; > > PpiDescriptor->Flags = EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST; > PpiDescriptor->Guid = &gPeiIpmiPpiGuid; > PpiDescriptor->Ppi = &PeiIpmiPpiinternal->PeiIpmiPpi; > > - GetTransportCapability (&TransportCapability); > Status = HelperAcquireManageabilityTransport ( > &gManageabilityProtocolIpmiGuid, > &PeiIpmiPpiinternal->TransportToken > @@ -119,8 +118,22 @@ PeiIpmiEntry ( > return Status; > } > > + Status = GetTransportCapability (PeiIpmiPpiinternal->TransportToken, &PeiIpmiPpiinternal->TransportCapability); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to GetTransportCapability().\n", __FUNCTION__)); > + return Status; > + } > + > + PeiIpmiPpiinternal->TransportMaximumPayload = MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY (PeiIpmiPpiinternal->TransportCapability); > + if (PeiIpmiPpiinternal->TransportMaximumPayload == (1 << MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AVAILABLE)) { > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface maximum payload is undefined.\n", __FUNCTION__)); > + } else { > + PeiIpmiPpiinternal->TransportMaximumPayload -= 1; > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface for IPMI protocol has maximum payload 0x%x.\n", __FUNCTION__, PeiIpmiPpiinternal->TransportMaximumPayload)); > + } > + > TransportName = HelperManageabilitySpecName (PeiIpmiPpiinternal->TransportToken->Transport->ManageabilityTransportSpecification); > - DEBUG ((DEBUG_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__, TransportName)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__, TransportName)); > > // > // Setup hardware information according to the transport interface. > diff --git a/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c > index 87a5436bdf..e4cd166b7a 100644 > --- a/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c > @@ -18,9 +18,9 @@ > > #include "IpmiProtocolCommon.h" > > -MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL; > -CHAR16 *mTransportName; > - > +MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL; > +CHAR16 *mTransportName; > +UINT32 TransportMaximumPayload; > MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION mHardwareInformation; > > /** > @@ -93,8 +93,6 @@ SmmIpmiEntry ( > MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability; > MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS TransportAdditionalStatus; > > - GetTransportCapability (&TransportCapability); > - > Status = HelperAcquireManageabilityTransport ( > &gManageabilityProtocolIpmiGuid, > &mTransportToken > @@ -104,8 +102,22 @@ SmmIpmiEntry ( > return Status; > } > > + Status = GetTransportCapability (mTransportToken, &TransportCapability); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to GetTransportCapability().\n", __FUNCTION__)); > + return Status; > + } > + > + TransportMaximumPayload = MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY (TransportCapability); > + if (TransportMaximumPayload == (1 << MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AVAILABLE)) { > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface maximum payload is undefined.\n", __FUNCTION__)); > + } else { > + TransportMaximumPayload -= 1; > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface for IPMI protocol has maximum payload 0x%x.\n", __FUNCTION__, TransportMaximumPayload)); > + } > + > mTransportName = HelperManageabilitySpecName (mTransportToken->Transport->ManageabilityTransportSpecification); > - DEBUG ((DEBUG_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__, mTransportName)); > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__, mTransportName)); > > // > // Setup hardware information according to the transport interface.