From: "Chang, Abner" <abner.chang@amd.com>
To: Tinh Nguyen <tinhnguyen@amperemail.onmicrosoft.com>,
Tinh Nguyen <tinhnguyen@os.amperecomputing.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "patches@amperecomputing.com" <patches@amperecomputing.com>,
"michael.d.kinney@intel.com" <michael.d.kinney@intel.com>,
"gaoliming@byosoft.com.cn" <gaoliming@byosoft.com.cn>,
"zhiguang.liu@intel.com" <zhiguang.liu@intel.com>
Subject: Re: [PATCH 1/2] MdePkg/IndustryStandard: Adds definitions for IPMI SSIF
Date: Tue, 2 May 2023 08:18:14 +0000 [thread overview]
Message-ID: <MN2PR12MB3966DBBD0CD3C795DAB33E2FEA6F9@MN2PR12MB3966.namprd12.prod.outlook.com> (raw)
In-Reply-To: <517169d4-095b-d574-879e-1356f191ee23@amperemail.onmicrosoft.com>
[AMD Official Use Only - General]
> -----Original Message-----
> From: Tinh Nguyen <tinhnguyen@amperemail.onmicrosoft.com>
> Sent: Tuesday, May 2, 2023 4:10 PM
> To: Chang, Abner <Abner.Chang@amd.com>; Tinh Nguyen
> <tinhnguyen@os.amperecomputing.com>; devel@edk2.groups.io
> Cc: patches@amperecomputing.com; michael.d.kinney@intel.com;
> gaoliming@byosoft.com.cn; zhiguang.liu@intel.com
> 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 <tinhnguyen@os.amperecomputing.com>
> >> Sent: Friday, April 28, 2023 12:00 PM
> >> 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 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 <tinhnguyen@os.amperecomputing.com>
> >> ---
> >> 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
next prev parent reply other threads:[~2023-05-02 8:18 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-28 3:59 [PATCH 0/2] Add IPMI SSIF definitions Tinh Nguyen
2023-04-28 3:59 ` [PATCH 1/2] MdePkg/IndustryStandard: Adds definitions for IPMI SSIF Tinh Nguyen
2023-04-28 5:17 ` 回复: " gaoliming
2023-04-28 9:01 ` Tinh Nguyen
2023-04-29 3:22 ` Chang, Abner
2023-05-02 8:09 ` Tinh Nguyen
2023-05-02 8:18 ` Chang, Abner [this message]
2023-04-28 3:59 ` [PATCH 2/2] MdePkg/IndustryStandard/IpmiNetFnApp.h: Add more definitions Tinh Nguyen
2023-04-28 5:41 ` 回复: " gaoliming
2023-04-29 3:32 ` Chang, Abner
2023-04-29 3:35 ` [PATCH 0/2] Add IPMI SSIF definitions Chang, Abner
2023-05-02 8:08 ` Tinh Nguyen
2023-05-02 8:21 ` Chang, Abner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN2PR12MB3966DBBD0CD3C795DAB33E2FEA6F9@MN2PR12MB3966.namprd12.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox