[AMD Official Use Only - General]
> -----Original Message----- > From: Tinh Nguyen <[email protected]> > Sent: Tuesday, May 2, 2023 4:10 PM > To: Chang, Abner <[email protected]>; Tinh Nguyen > <[email protected]>; [email protected] > Cc: [email protected]; [email protected]; > [email protected]; [email protected] > Subject: Re: [PATCH 1/2] MdePkg/IndustryStandard: Adds definitions for > IPMI SSIF > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > Hi Abner, > > On 29/04/2023 10:22, Chang, Abner wrote: > > [AMD Official Use Only - General] > > > > > > Hi Tinh, > > Below is my comments, > > > >> -----Original Message----- > >> From: Tinh Nguyen <[email protected]> > >> Sent: Friday, April 28, 2023 12:00 PM > >> To: [email protected] > >> Cc: [email protected]; [email protected]; > >> [email protected]; [email protected]; Chang, Abner > >> <[email protected]>; Tinh Nguyen > >> <[email protected]> > >> Subject: [PATCH 1/2] MdePkg/IndustryStandard: Adds definitions for > >> IPMI SSIF > >> > >> Caution: This message originated from an External Source. Use proper > >> caution when opening attachments, clicking links, or responding. > >> > >> > >> Specification reference: > >> > https://www.intel.com/content/www/us/en/products/docs/servers/ipmi/i > >> pmi-second-gen-interface-spec-v2-rev1-1.html > >> > >> Signed-off-by: Tinh Nguyen <[email protected]> > >> --- > >> MdePkg/MdePkg.dec | 26 ++++++ > >> MdePkg/Include/IndustryStandard/IpmiSsif.h | 98 > >> ++++++++++++++++++++ > >> 2 files changed, 124 insertions(+) > >> > >> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > >> 7488ccda7a00..518e4200e9af 100644 > >> --- a/MdePkg/MdePkg.dec > >> +++ b/MdePkg/MdePkg.dec > >> @@ -10,6 +10,7 @@ > >> # Copyright (c) 2022, Loongson Technology Corporation Limited. All > >> rights reserved.<BR> # Copyright (c) 2021 - 2022, Arm Limited. All > >> rights reserved.<BR> # Copyright (C) 2023 Advanced Micro Devices, > >> Inc. All rights reserved.<BR> > >> +# Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR> > >> # > >> # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -2353,6 > >> +2354,31 @@ [PcdsFixedAtBuild,PcdsPatchableInModule] > >> # @Prompt IPMI KCS Interface I/O Base Address > >> > >> > gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsIoBaseAddress|0xca2|UINT16|0x00 > >> 000031 > >> > >> + ## This is SMBus slave address for the SSIF to the BMC. > >> + # The recommended value defined by IPMI specification is 0x20 > >> + (section > >> 12.12). > >> + # @Prompt IPMI SSIF SMBus slave address > >> + > >> > gEfiMdePkgTokenSpaceGuid.PcdIpmiSmbusSlaveAddr|0x20|UINT8|0x00000 > >> 032 > >> + > >> + ## This is the maximum number of IPMI SSIF request retries. > >> + # The IPMI specification specified min value is 5 (section 12.17). > >> + # @Prompt Number of IPMI SSIF request retries. > >> + > >> + > >> > gEfiMdePkgTokenSpaceGuid.PcdIpmiSsifRequestRetryCount|0x05|UINT8|0 > >> x000 > >> + 00033 > >> + > >> + ## This is the required interval for each IPMI request retry. > >> + # The IPMI specification specified a time range of 60ms to 250ms > >> + (section > >> 12.17). > >> + # The default setting is min. > >> + # @Prompt Time between IPMI SSIF request retries. > >> + > >> + > >> > gEfiMdePkgTokenSpaceGuid.PcdIpmiSsifRequestRetryInterval|60000|UINT3 > >> 2| > >> + 0x00000034 > > Please give the unit to PCD name, for example > PcdIpmiSsifRequestRetryIntervalMicrosecond that looks more clear to > readers. > > > Yeah, it looks good when adding this > >> + > >> + ## This value is the maximum retries of an IPMI SSIF response # > >> + @Prompt Number of IPMI SSIF response retries. > >> + > >> + > >> > gEfiMdePkgTokenSpaceGuid.PcdIpmiSsifResponseRetryCount|250|UINT8|0 > >> x000 > >> + 00035 > > Is 250 times of retry defined in spec? Seems to me it is too many? > > The specification does not define this value. But the current SSIF driver into > Linux is using 250. > We should use the same value with Linux; it helps BMC handle the retry from > the host easier. Ok, I got it. Thanks Abner > > > > >> + > >> + ## This is the required interval for each IPMI response retry. > >> + # The IPMI specification specified min value is 60ms (section 12.17). > >> + # @Prompt Time-out for a response, internal > >> + > >> + > >> > gEfiMdePkgTokenSpaceGuid.PcdIpmiSsifResponseRetryInterval|60000|UINT > >> 32 > >> + |0x00000036 > >> + > >> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, > PcdsDynamicEx] > >> ## This value is used to set the base address of PCI express hierarchy. > >> # @Prompt PCI Express Base Address. > >> diff --git a/MdePkg/Include/IndustryStandard/IpmiSsif.h > >> b/MdePkg/Include/IndustryStandard/IpmiSsif.h > >> new file mode 100644 > >> index 000000000000..4a97438109a9 > >> --- /dev/null > >> +++ b/MdePkg/Include/IndustryStandard/IpmiSsif.h > >> @@ -0,0 +1,98 @@ > >> +/** @file > >> + IPMI SSIF Definitions > >> + > >> + Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR> > >> + SPDX-License-Identifier: BSD-2-Clause-Patent > >> + > >> + @par Revision Reference: > >> + - IPMI Specification > >> + Version 2.0, Rev. 1.1 > >> + > >> + > >> > +https://www.intel.com/content/www/us/en/products/docs/servers/ipmi/ > >> ipmi > >> +-second-gen-interface-spec-v2-rev1-1.html > >> +**/ > >> + > >> +#ifndef IPMI_SSIF_H_ > >> +#define IPMI_SSIF_H_ > > An additional whitespace between "#define" and "IPMI_SSIF_H_". > will add in v2 > > > >> + > >> +/// > >> +/// Definitions for SMBUS Commands for SSIF /// Table 12 - Summary > >> +of SMBUS Commands for SSIF /// > >> + > >> +// Write block > > Please have a consist comment format "///". > > > thanks for remind > > - Tinh > > >> +#define IPMI_SSIF_SMBUS_CMD_SINGLE_PART_WRITE 0x02 > >> +#define IPMI_SSIF_SMBUS_CMD_MULTI_PART_WRITE_START 0x06 > >> +#define IPMI_SSIF_SMBUS_CMD_MULTI_PART_WRITE_MIDDLE 0x07 > >> +#define IPMI_SSIF_SMBUS_CMD_MULTI_PART_WRITE_END 0x08 > >> + > >> +// Read block > > Please have a consist comment format "///". > > > > > > Thanks > > Abner > > > >> +#define IPMI_SSIF_SMBUS_CMD_SINGLE_PART_READ 0x03 > >> +#define IPMI_SSIF_SMBUS_CMD_MULTI_PART_READ_START 0x03 > >> +#define IPMI_SSIF_SMBUS_CMD_MULTI_PART_READ_MIDDLE 0x09 > >> +#define IPMI_SSIF_SMBUS_CMD_MULTI_PART_READ_END 0x09 > >> +#define IPMI_SSIF_SMBUS_CMD_MULTI_PART_READ_RETRY 0x0A > >> + > >> +/// > >> +/// Definitions for Multi-Part Read Transactions /// Section 12.5 /// > >> +#define IPMI_SSIF_MULTI_PART_READ_START_SIZE 0x1E > >> +#define IPMI_SSIF_MULTI_PART_READ_START_PATTERN1 0x00 > #define > >> +IPMI_SSIF_MULTI_PART_READ_START_PATTERN2 0x01 > >> +#define IPMI_SSIF_MULTI_PART_READ_END_PATTERN 0xFF > >> + > >> +/// > >> +/// IPMI SSIF maximum message size > >> +/// > >> +#define IPMI_SSIF_INPUT_MESSAGE_SIZE_MAX 0xFF > >> +#define IPMI_SSIF_OUTPUT_MESSAGE_SIZE_MAX 0xFF > >> + > >> +/// > >> +/// IPMI SMBus system interface maximum packet size in byte /// > >> +#define IPMI_SSIF_MAXIMUM_PACKET_SIZE_IN_BYTES 0x20 > >> + > >> +typedef enum { > >> + IpmiSsifPacketStart = 0, > >> + IpmiSsifPacketMiddle, > >> + IpmiSsifPacketEnd, > >> + IpmiSsifPacketSingle, > >> + IpmiSsifPacketMax > >> +} IPMI_SSIF_PACKET_ATTRIBUTE; > >> + > >> +#pragma pack (1) > >> +/// > >> +/// IPMI SSIF Interface Request Format /// Section 12.2 and 12.3 /// > >> +typedef struct { > >> + UINT8 NetFunc; > >> + UINT8 Command; > >> +} IPMI_SSIF_REQUEST_HEADER; > >> + > >> +/// > >> +/// IPMI SSIF Interface Response Format /// Section 12.4 and 12.5 > >> +/// typedef struct { > >> + UINT8 StartPattern[2]; > >> + UINT8 NetFunc; > >> + UINT8 Command; > >> +} IPMI_SSIF_RESPONSE_PACKET_START; > >> + > >> +typedef struct { > >> + UINT8 BlockNumber; > >> +} IPMI_SSIF_RESPONSE_PACKET_MIDDLE; > >> + > >> +typedef struct { > >> + UINT8 EndPattern; > >> +} IPMI_SSIF_RESPONSE_PACKET_END; > >> + > >> +typedef struct { > >> + UINT8 NetFunc; > >> + UINT8 Command; > >> +} IPMI_SSIF_RESPONSE_SINGLE_PACKET; > >> + > >> +#pragma pack () > >> + > >> +#endif /* IPMI_SSIF_H_ */ > >> -- > >> 2.40.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103851): https://edk2.groups.io/g/devel/message/103851 Mute This Topic: https://groups.io/mt/98552184/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
