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.


+
+  ## 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 (#103850): https://edk2.groups.io/g/devel/message/103850
Mute This Topic: https://groups.io/mt/98552184/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to