Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add IPMI Serial definitions #6454

Merged
merged 3 commits into from
Dec 9, 2024
Merged

Add IPMI Serial definitions #6454

merged 3 commits into from
Dec 9, 2024

Conversation

gtossk2
Copy link
Contributor

@gtossk2 gtossk2 commented Nov 21, 2024

Description

Add definitions for IPMI Serial and it can be used in ManageabilityPkg.

How This Was Tested

Tested in tianocore/edk2-platforms#240

Integration Instructions

N/A

@gtossk2 gtossk2 mentioned this pull request Nov 21, 2024
Merged
abdattar
abdattar reviewed Nov 21, 2024
View reviewed changes
Copy link
Contributor

@abdattar abdattar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes looks good.

changab
changab approved these changes Nov 21, 2024
View reviewed changes
MdePkg/Include/IndustryStandard/IpmiSerial.h Outdated Show resolved Hide resolved
changab
changab approved these changes Nov 22, 2024
View reviewed changes
Copy link
Member

@changab changab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gtossk2, I will merge this change on next Mon as we are now in the code freeze period.

@changab
Copy link
Member

changab commented Nov 22, 2024

@gtossk2 The quiet period has now ended, however, we still need another approval.

@lgao4
Copy link
Contributor

lgao4 commented Dec 6, 2024

@gtossk2 The quiet period has now ended, however, we still need another approval.

Are those PCDs really required to be configured? I want to confirm whether there is the real usage model to configure those PCDs. If no, PCDs are not required to be introduced.

@gtossk2
Copy link
Contributor Author

gtossk2 commented Dec 6, 2024

@gtossk2 The quiet period has now ended, however, we still need another approval.

Are those PCDs really required to be configured? I want to confirm whether there is the real usage model to configure those PCDs. If no, PCDs are not required to be introduced.

Hi @lgao4, Those PCDs based on IPMI spec should be configured per platform requirement. They are using in this PR - tianocore/edk2-platforms#240

@lgao4
Copy link
Contributor

lgao4 commented Dec 8, 2024

@gtossk2 The quiet period has now ended, however, we still need another approval.

Are those PCDs really required to be configured? I want to confirm whether there is the real usage model to configure those PCDs. If no, PCDs are not required to be introduced.

Hi @lgao4, Those PCDs based on IPMI spec should be configured per platform requirement. They are using in this PR - tianocore/edk2-platforms#240

Do you see the real usage case to configure those PCDs on the platform?

@gtossk2
Copy link
Contributor Author

gtossk2 commented Dec 9, 2024

Do you see the real usage case to configure those PCDs on the platform?

For our virtual platform, we need to configure those PCDs, such as PcdIpmiSerialRequesterAddress to identify Host / Remote Software and PcdIpmiSerialRequestRetryInterval per BMC design.

mdkinney
mdkinney reviewed Dec 9, 2024
View reviewed changes
MdePkg/MdePkg.dec Outdated Show resolved Hide resolved
mdkinney
mdkinney reviewed Dec 9, 2024
View reviewed changes
MdePkg/MdePkg.dec Outdated Show resolved Hide resolved
@gtossk2
MdePkg/IndustryStandard: Add definitions for IPMI Serial
b35cd0c 
Add IPMI Serial definitions

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: John Chung <john.chung@arm.com>
@gtossk2
MdePkg: Add new PCDs for IPMI Serial
b7956a6 
This change adds new PCDs for IPMI Serial.

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: John Chung <john.chung@arm.com>
@mdkinney mdkinney added the push Auto push patch series in PR if all checks pass label Dec 9, 2024
@mergify mergify bot merged commit 481c433 into tianocore:master Dec 9, 2024
126 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@changab changab changab approved these changes

@abdattar abdattar abdattar approved these changes

@mdkinney mdkinney mdkinney approved these changes

@lgao4 lgao4 Awaiting requested review from lgao4

@LiuZhiguang001 LiuZhiguang001 Awaiting requested review from LiuZhiguang001

@nicklela nicklela Awaiting requested review from nicklela

Assignees
No one assigned
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gtossk2 @changab @lgao4 @mdkinney @abdattar