edk2
edk2 copied to clipboard
MdeModulePkg/Bus/Pci: Initial support for DOE
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
PR can not be merged due to conflict. Please rebase and resubmit
@Wenxing-hou and @jyao1 for reference
I think we need MdePkg and MdeModulePkg owner to review and approval.
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.
PR can not be merged due to conflict. Please rebase and resubmit
It's not stale, it just hasn't been reviewed. Maybe @github-actions can ping someone?
Ping!
Ping!
Ping!
Squashed and fixed the header comment
Ping!
Ping!
Project is in a hard freeze in preparation for a release at the end of this week. I will continue review after release.
Ping!
Ping! I assume the release has happened?
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.
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
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.
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
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
PR can not be merged due to conflict. Please rebase and resubmit
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.
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.