edk2 icon indicating copy to clipboard operation
edk2 copied to clipboard

MdeModulePkg/Bus/Pci: Initial support for DOE

Open alistair23 opened this issue 1 year ago • 8 comments

Description

This is initial support for the PCIe Data Object Exchange (DOE) mailbox protocol. This add a new EFI_PCI_IO_DOE struct to the PCI_IO_DEVICE struct which supports DOE operations.

This is the next step towards supporting SPDM over DOE.

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

How This Was Tested

Running against a DOE device in QEMU

Integration Instructions

alistair23 avatar Jun 03 '24 05:06 alistair23

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

mergify[bot] avatar Jun 04 '24 07:06 mergify[bot]

@Wenxing-hou and @jyao1 for reference

alistair23 avatar Jun 06 '24 03:06 alistair23

I think we need MdePkg and MdeModulePkg owner to review and approval.

jyao1 avatar Jun 07 '24 02:06 jyao1

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[bot] avatar Aug 09 '24 23:08 github-actions[bot]

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

mergify[bot] avatar Aug 09 '24 23:08 mergify[bot]

It's not stale, it just hasn't been reviewed. Maybe @github-actions can ping someone?

alistair23 avatar Aug 12 '24 01:08 alistair23

Ping!

alistair23 avatar Aug 26 '24 10:08 alistair23

Ping!

alistair23 avatar Sep 11 '24 11:09 alistair23

Ping!

alistair23 avatar Oct 22 '24 22:10 alistair23

Squashed and fixed the header comment

alistair23 avatar Oct 23 '24 00:10 alistair23

Ping!

alistair23 avatar Nov 12 '24 01:11 alistair23

Ping!

alistair23 avatar Nov 19 '24 06:11 alistair23

Project is in a hard freeze in preparation for a release at the end of this week. I will continue review after release.

mdkinney avatar Nov 19 '24 16:11 mdkinney

Ping!

alistair23 avatar Jan 06 '25 05:01 alistair23

Ping! I assume the release has happened?

alistair23 avatar Mar 07 '25 06:03 alistair23

My feedback about squashing the 2 commits for MdePkg into 1 commit has not been done yet. Please update so we can resolve those conversations.

mdkinney avatar Mar 07 '25 16:03 mdkinney

My feedback about squashing the 2 commits for MdePkg into 1 commit has not been done yet. Please update so we can resolve those conversations.

I squashed the two commits

alistair23 avatar Mar 10 '25 01:03 alistair23

A new protocol is being added to MdeModulePkg. It protocol declaration and GUID value are missing from MdeModulePkg.dec.

Is a new protocol really required? What components produce the protocol and what components consume the protocol. If the intended consumers are UEFI PCI Device Drivers, then driver that choose to use this protocol will only work on EDK II based systems since this is defined as an EDK II specific protocol. This means that driver will have to check for presence of the protocol and implement alternate behavior if the protocol is not present.

If this protocol is required for interoperability, then it needs to be added to industry standard specs, and the code first approach should be considered.

mdkinney avatar Mar 10 '25 15:03 mdkinney

A new protocol is being added to MdeModulePkg. It protocol declaration and GUID value are missing from MdeModulePkg.dec.

How do I determine the values for MdeModulePkg.dec?

Is a new protocol really required? What components produce the protocol and what components consume the protocol. If the intended consumers are UEFI PCI Device Drivers, then driver that choose to use this protocol will only work on EDK II based systems since this is defined as an EDK II specific protocol. This means that driver will have to check for presence of the protocol and implement alternate behavior if the protocol is not present.

I think it should be a protocol. The idea is that PCIe devices that support DOE can produce the protocol and then features that build on top of DOE (like SPDM) can consume it. Similar to other transport protocols.

If this protocol is required for interoperability, then it needs to be added to industry standard specs, and the code first approach should be considered.

That might be the eventual outcome

alistair23 avatar Mar 12 '25 05:03 alistair23

A new protocol is being added to MdeModulePkg. It protocol declaration and GUID value are missing from MdeModulePkg.dec.

How do I determine the values for MdeModulePkg.dec?

Is a new protocol really required? What components produce the protocol and what components consume the protocol. If the intended consumers are UEFI PCI Device Drivers, then driver that choose to use this protocol will only work on EDK II based systems since this is defined as an EDK II specific protocol. This means that driver will have to check for presence of the protocol and implement alternate behavior if the protocol is not present.

I think it should be a protocol. The idea is that PCIe devices that support DOE can produce the protocol and then features that build on top of DOE (like SPDM) can consume it. Similar to other transport protocols.

UEFI Drivers for PCI add-in cards will not likely implement a protocol that is not defined in UEFI Specification. This PR looks like content that is being added to verify the entire DOE/SPDM feature between platform and PCI devices to build a proposal for the industry standard specification required for industry adoption. If that is the case, then it should not be added to edk2 repo at this time. It can reside in edk2-staging or as "Draft" PR in edk2 for collaboration. This is what the code-first approach is for, so please review the recently revised process for code-first and consider moving this content into that approach.

  • https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-First-Process

Adding EDK II specific protocols to a package such as the MdeModulePkg is typically only done if the consumer and producer of the protocol are EDK II specific components. When the interoperability requirements extend to potentially cover non EDK II platform firmware implementations, an EDK II specific protocol is not appropriate.

If this protocol is required for interoperability, then it needs to be added to industry standard specs, and the code first approach should be considered.

That might be the eventual outcome

mdkinney avatar Mar 12 '25 15:03 mdkinney

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

mergify[bot] avatar May 07 '25 03:05 mergify[bot]

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[bot] avatar Jul 06 '25 23:07 github-actions[bot]

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[bot] avatar Jul 13 '25 23:07 github-actions[bot]