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

MdeModulePkg: Add alternative queue sizes in NVME driver #6024

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

VivianNK
Copy link
Contributor

@VivianNK VivianNK commented Aug 1, 2024

Description

Add a new PCD that toggles between the old driver behavior and new behavior that uses a different hardcoded queue size to support specific devices. Default behavior should not change.

MdeModulePkg/NvmExpressDxe: Improve NVMe controller init robustness

It has been observed that some faulty NVMe devices may return invalid values. The code in NvmExpressDxe recognizes the controller is not responding correctly and issues an ASSERT() often in DEBUG builds or a reset in RELEASE builds.

The following changes are made to NvmeControllerInit() to prevent a faulty NVMe device from halting the overall boot:

  1. Check the Vendor ID and Device ID to verify they are read properly and if not, return EFI_DEVICE_ERROR
  2. After the ASSERT() when Memory Page Size Minimum (Cap.Mpsmin),
    produce an error message and return EFI_DEVICE_ERROR

MdeModulePkg/NvmExpressDxe: Correct function parameter modifer

Updates the Cap parameter for ReadNvmeControllerCapabilities() to be OUT since the buffer pointed to is written within the function.


Cherry-picked the following commits:
microsoft/mu_basecore@9837db3
microsoft/mu_basecore@0a5a1d9
microsoft/mu_basecore@e397168


  • Breaking change?
  • Impacts security?
  • Includes tests?

How This Was Tested

Tested in Project MU

Integration Instructions

N/A

@@ -1187,6 +1187,9 @@
# @Prompt Delay access XHCI register after it issues HCRST (us)
gEfiMdeModulePkgTokenSpaceGuid.PcdDelayXhciHCReset|2000|UINT16|0x30001060

# Support alternative hardware queue sizes in NVME driver
Copy link
Member

Choose a reason for hiding this comment

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

Add @prompt and description of TRUE and FALSE setting and the default value.

Also, is this PCD really required? What would be the impact if just assuming this is always enabled. Prefer to not add new PCDs unless absolutely required.

makubacki and others added 3 commits September 12, 2024 14:32
Updates the `Cap` parameter for `ReadNvmeControllerCapabilities()`
to be `OUT` since the buffer pointed to is written within the
function.

Signed-off-by: Michael Kubacki <[email protected]>
It has been observed that some faulty NVMe devices may return
invalid values. The code in NvmExpressDxe recognizes the controller
is not responding correctly and issues an ASSERT() often in DEBUG
builds or a reset in RELEASE builds.

The following changes are made to NvmeControllerInit() to prevent a
faulty NVMe device from halting the overall boot:

1. Check the Vendor ID and Device ID to verify they are read properly
   and if not, return EFI_DEVICE_ERROR
2. After the ASSERT() when Memory Page Size Minimum (Cap.Mpsmin),
   produce an error message and return EFI_DEVICE_ERROR

Signed-off-by: Michael Kubacki <[email protected]>
Add a new PCD that toggles between the old driver behavior and new behavior
that uses a different hardcoded queue size to support specific devices.
Default behavior should not change.

Signed-off-by: Michael Kubacki <[email protected]>
Signed-off-by: Vivian Nowka-Keane <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants