[PATCH v2 0/3] Add IPMI SSIF definitions

[PATCH v2 0/3] Add IPMI SSIF definitions

[Tinh Nguyen]

Change from v1:
  - Separate the commit for the IPMI SSIF header and IPMI SSIF PCDs
  - Correct some indent and comment style.
  - Change the PCDs name for a more readable.
  - Support KCS/SMIC Interface Capabilities too.

Tinh Nguyen (3):
  MdePkg/IndustryStandard: Add definitions for IPMI SSIF
  MdePkg: Add new PCDs for IPMI SSIF
  MdePkg/IndustryStandard: Add Interface Capabilities definitions

 MdePkg/MdePkg.dec                              | 26 ++++++
 MdePkg/Include/IndustryStandard/IpmiNetFnApp.h | 46 +++++++++
 MdePkg/Include/IndustryStandard/IpmiSsif.h     | 98 ++++++++++++++++++++
 3 files changed, 170 insertions(+)
 create mode 100644 MdePkg/Include/IndustryStandard/IpmiSsif.h

--
2.40.0

[PATCH v2 1/3] MdePkg/IndustryStandard: Add definitions for IPMI SSIF

[Tinh Nguyen]

Specification reference:
https://www.intel.com/content/www/us/en/products/docs/servers/ipmi/ipmi-second-gen-interface-spec-v2-rev1-1.html

Signed-off-by: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
---
 MdePkg/Include/IndustryStandard/IpmiSsif.h | 98 ++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/IpmiSsif.h b/MdePkg/Include/IndustryStandard/IpmiSsif.h
new file mode 100644
index 000000000000..a5bfa14a9dfa
--- /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_
+
+///
+/// Definitions for SMBUS Commands for SSIF
+/// Table 12 - Summary of SMBUS Commands for SSIF
+///
+
+/// Write block
+#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
+#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

Re: [PATCH v2 1/3] MdePkg/IndustryStandard: Add definitions for IPMI SSIF

[Chang, Abner]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <Abner.Chang@amd.com>

BTW, I don't see maintainers are CCed in the commit message. Not sure if this is acceptable as usually we can see who are the maintainers in the commit message (either in patch email or git commit history) instead of looking at Cc in email.
Abner

> -----Original Message-----
> From: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
> Sent: Wednesday, May 3, 2023 1:40 AM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; michael.d.kinney@intel.com;
> gaoliming@byosoft.com.cn; zhiguang.liu@intel.com; Chang, Abner
> <Abner.Chang@amd.com>; Tinh Nguyen
> <tinhnguyen@os.amperecomputing.com>
> Subject: [PATCH v2 1/3] MdePkg/IndustryStandard: Add 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 <tinhnguyen@os.amperecomputing.com>
> ---
>  MdePkg/Include/IndustryStandard/IpmiSsif.h | 98
> ++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/MdePkg/Include/IndustryStandard/IpmiSsif.h
> b/MdePkg/Include/IndustryStandard/IpmiSsif.h
> new file mode 100644
> index 000000000000..a5bfa14a9dfa
> --- /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_
> +
> +///
> +/// Definitions for SMBUS Commands for SSIF /// Table 12 - Summary of
> +SMBUS Commands for SSIF ///
> +
> +/// Write block
> +#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
> +#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

Re: [PATCH v2 0/3] Add IPMI SSIF definitions

[Chang, Abner]

[AMD Official Use Only - General]

Hi Tinh,
Thanks for this effort. I have no problem with this patch set but you still have to get R-B from maintainers. Two feedbacks here,
- Please put BZ # in commit message in the patch 1/3-3/3.
   Ex: BZ#: 4434 - Add IPMI SSIF definitions
- As the feedback I gave to patch 1/3. I think we better have maintainers Cc in the commit message.

Thanks
Abner

> -----Original Message-----
> From: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
> Sent: Wednesday, May 3, 2023 1:40 AM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; michael.d.kinney@intel.com;
> gaoliming@byosoft.com.cn; zhiguang.liu@intel.com; Chang, Abner
> <Abner.Chang@amd.com>; Tinh Nguyen
> <tinhnguyen@os.amperecomputing.com>
> Subject: [PATCH v2 0/3] Add IPMI SSIF definitions
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Change from v1:
>   - Separate the commit for the IPMI SSIF header and IPMI SSIF PCDs
>   - Correct some indent and comment style.
>   - Change the PCDs name for a more readable.
>   - Support KCS/SMIC Interface Capabilities too.
> 
> Tinh Nguyen (3):
>   MdePkg/IndustryStandard: Add definitions for IPMI SSIF
>   MdePkg: Add new PCDs for IPMI SSIF
>   MdePkg/IndustryStandard: Add Interface Capabilities definitions
> 
>  MdePkg/MdePkg.dec                              | 26 ++++++
>  MdePkg/Include/IndustryStandard/IpmiNetFnApp.h | 46 +++++++++
>  MdePkg/Include/IndustryStandard/IpmiSsif.h     | 98
> ++++++++++++++++++++
>  3 files changed, 170 insertions(+)
>  create mode 100644 MdePkg/Include/IndustryStandard/IpmiSsif.h
> 
> --
> 2.40.0

Re: [edk2-devel] [PATCH v2 0/3] Add IPMI SSIF definitions

[Tinh Nguyen]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Hi Abner,

I have updated the v3.

Thanks,
Tinh

[-- Attachment #2: Type: text/html, Size: 74 bytes --]