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.

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

ShellPkg: Add support to parse SPMI table in acpiview #5980

Closed
wants to merge 1 commit into from

Conversation

shkrc
Copy link

@shkrc shkrc commented Jul 26, 2024

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4818

Added support for acpiview shell utility to parse and print SPMI table

Cc: Abdul Lateef Attar [email protected]
Cc: Abner Chang [email protected]
Cc: Paul Grimes [email protected]
Cc: Zhichao Gao [email protected]
Signed-off-by: Shankar C [email protected]

Description

<Include a description of the change and why this change was made.>

<For each item, place an "x" in between [ and ] if true. Example: [x] (you can also check items in GitHub UI)>

<Create the PR as a Draft PR if it is only created to run CI checks.>

<Delete lines in <> tags before creating the PR.>

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

<Describe the test(s) that were run to verify the changes.>

Integration Instructions

<Describe how these changes should be integrated. Use N/A if nothing is required.>

@shkrc shkrc force-pushed the acpiview-spmi branch 3 times, most recently from 67104c6 to 620c27a Compare July 27, 2024 14:00
@github-actions github-actions bot requested a review from ZhichaoGao July 27, 2024 14:01
@abdattar
Copy link
Contributor

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4818

Added support for acpiview shell utility to parse and print SPMI table

Cc: Abdul Lateef Attar [email protected] Cc: Abner Chang [email protected] Cc: Paul Grimes [email protected]

Shankar, you need to add Signed-off-by: your name .
Also need to Cc maintainers.
Cc: Zhichao Gao [email protected]

@shkrc
Copy link
Author

shkrc commented Jul 29, 2024

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4818
Added support for acpiview shell utility to parse and print SPMI table
Cc: Abdul Lateef Attar [email protected] Cc: Abner Chang [email protected] Cc: Paul Grimes [email protected]

Shankar, you need to add Signed-off-by: your name . Also need to Cc maintainers. Cc: Zhichao Gao [email protected]

hi, i added in the commit but it did not reflect in the description. ill just edit the description

@lgao4
Copy link
Contributor

lgao4 commented Jul 30, 2024

Could you attach the dump result for SPMI table?

@abdattar
Copy link
Contributor

abdattar commented Aug 2, 2024

@shkrc can you paste the acpiview -S SPMI output ?

@shkrc
Copy link
Author

shkrc commented Aug 2, 2024

spmi.log
I have attached the log here

@shkrc
Copy link
Author

shkrc commented Aug 13, 2024

Hi @lgao4 can you help review this patch?

@abdattar
Copy link
Contributor

abdattar commented Sep 1, 2024

Hi @ZhichaoGao could you please merge the changes.

@abdattar abdattar requested a review from changab September 17, 2024 05:01
@abdattar
Copy link
Contributor

could you please push the code changes?

Copy link

mergify bot commented Sep 17, 2024

PR can not be merged due to conflict. Please rebase and resubmit

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4818

Added support for acpiview shell utility to parse and print SPMI table

Cc: Abdul Lateef Attar <[email protected]>
Cc: Abner Chang <[email protected]>
Cc: Paul Grimes <[email protected]>
Cc: Zhichao Gao <[email protected]>
Signed-off-by: Shankar C <[email protected]>
@shkrc
Copy link
Author

shkrc commented Sep 24, 2024

Hi @lgao4 @ZhichaoGao can you help review this patch?

{ L"Interface Type", 1, 36, L"%d", NULL, NULL, ValidateInterfaceType, NULL },
{ L"Reserved", 1, 37, L"%x", NULL, NULL, NULL, NULL },
{ L"Specification Revision", 2, 38, L"0x%x", NULL, NULL, NULL, NULL },
{ L"Interrupt Type", 1, 40, L"%d", NULL, NULL, NULL, NULL },
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should check that:
[7:2] - Reserved (must be 0)

and to have a pretty print maybe, format the other bits with ParseAcpiBitFields():

[1] - I/O APIC/SAPIC interrupt (Global System Interrupt)
[0] - SCI triggered through GPE (use 0b for SSIF)
0 = not supported
1 = supported

Similar checks/formatting could be done regarding the GPE field:

 (Note: This field is valid only if Bit[0] of the InterruptType field is set.

and PCI Device Flag:

[7:1] - Reserved
[0] - PCI Device Flag. For PCI IPMI devices, this bit is set. For
non-PCI devices, this bit is cleared.

and other fields

IN VOID *Context
)
{
if (*Ptr > 5) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to have these values in the MdePkg:

0 Reserved
1 Keyboard Controller Style (KCS)
2 Server Management Interface Chip (SMIC)
3 Block Transfer (BT)

and use a macro instead of 5 here

STATIC CONST ACPI_PARSER SpmiParser[] = {
PARSE_ACPI_HEADER (&mAcpiHdrInfo),
{ L"Interface Type", 1, 36, L"%d", NULL, NULL, ValidateInterfaceType, NULL },
{ L"Reserved", 1, 37, L"%x", NULL, NULL, NULL, NULL },
Copy link
Contributor

Choose a reason for hiding this comment

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

Also for this field:

This field must always be 01h to be compatible with any
software that implements previous versions of this spec.

Copy link

This PR has been automatically marked as stale because it has not had activity in 60 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Due to lack of updates, this item is pending deletion. label Nov 24, 2024
Copy link

github-actions bot commented Dec 1, 2024

This pull request has been automatically been closed because it did not have any activity in 60 days and no follow up within 7 days after being marked stale. Thank you for your contributions.

@github-actions github-actions bot closed this Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Due to lack of updates, this item is pending deletion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants