MdeModulePkg/NvmExpressDxe: Improve robustness [Rebase & FF] (#516)

## Description

Resolves #515

The main change the first commit:

```
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. Replace the ASSERT() when Memory Page Size Minimum (Cap.Mpsmin)
   with an error message and return EFI_DEVICE_ERROR
```

The additional changes are added due to improvements found in nearby
code.

2:

```
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.
```

3:

```
MdePkg/Nvme.h: Add missing NVMe capability descriptions

Adds missing structure member documentation.
```

This will also be sent to the edk2 mailing list, however, it is being
checked
in here first due to some devices that may be able to benefit from the
change
and the upcoming 202308 freeze.

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

- Verified NVMe boot on a physical system
- Verified NVMe driver binding on QEMU Q35 with an emulated NVMe device

## Integration Instructions

N/A

---------

Signed-off-by: Michael Kubacki <[email protected]>

MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c

@@ -30,8 +30,10 @@ UINTN  mNvmeControllerNumber = 0;
 **/
 EFI_STATUS
 ReadNvmeControllerCapabilities (
-  IN NVME_CONTROLLER_PRIVATE_DATA  *Private,
-  IN NVME_CAP                      *Cap
+  // MU_CHANGE [BEGIN] - Correct Cap parameter modifier
+  IN  NVME_CONTROLLER_PRIVATE_DATA  *Private,
+  OUT NVME_CAP                      *Cap
+  // MU_CHANGE [END] - Correct Cap parameter modifier
   )
 {
   EFI_PCI_IO_PROTOCOL  *PciIo;
@@ -743,13 +745,36 @@ NvmeControllerInit (
   NVME_AQA             Aqa;
   NVME_ASQ             Asq;
   NVME_ACQ             Acq;
+  UINT16               VidDid[2]; // MU_CHANGE - Improve NVMe controller init robustness
   UINT8                Sn[21];
   UINT8                Mn[41];
 
+  // MU_CHANGE [BEGIN] - Improve NVMe controller init robustness
+  PciIo = Private->PciIo;
+
+  //
+  // Verify the controller is still accessible
+  //
+  Status = PciIo->Pci.Read (
+                        PciIo,
+                        EfiPciIoWidthUint16,
+                        PCI_VENDOR_ID_OFFSET,
+                        ARRAY_SIZE (VidDid),
+                        VidDid
+                        );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return EFI_DEVICE_ERROR;
+  }
+
+  if ((VidDid[0] == 0xFFFF) || (VidDid[1] == 0xFFFF)) {
+    return EFI_DEVICE_ERROR;
+  }
+
   //
   // Enable this controller.
   //
-  PciIo  = Private->PciIo;
+  // MU_CHANGE [END] - Improve NVMe controller init robustness
   Status = PciIo->Attributes (
                     PciIo,
                     EfiPciIoAttributeOperationSupported,
@@ -788,7 +813,17 @@ NvmeControllerInit (
   //
   // Currently the driver only supports 4k page size.
   //
-  ASSERT ((Private->Cap.Mpsmin + 12) <= EFI_PAGE_SHIFT);
+
+  // MU_CHANGE [BEGIN] - Improve NVMe controller init robustness
+
+  // Currently, this means Cap.Mpsmin must be zero for an EFI_PAGE_SHIFT size of 12.
+  // ASSERT ((Private->Cap.Mpsmin + 12) <= EFI_PAGE_SHIFT);
+  if ((Private->Cap.Mpsmin + 12) > EFI_PAGE_SHIFT) {
+    DEBUG ((DEBUG_ERROR, "NvmeControllerInit: Mpsmin is larger than expected (0x%02x).\n", Private->Cap.Mpsmin));
+    return EFI_DEVICE_ERROR;
+  }
+
+  // MU_CHANGE [END] - Improve NVMe controller init robustness
 
   Private->Cid[0]        = 0;
   Private->Cid[1]        = 0;

MdePkg/Include/IndustryStandard/Nvme.h

@@ -52,23 +52,25 @@
 //
 // 3.1.1 Offset 00h: CAP - Controller Capabilities
 //
+// MU_CHANGE [BEGIN] - Add missing capability descriptions
 typedef struct {
   UINT16    Mqes;       // Maximum Queue Entries Supported
   UINT8     Cqr    : 1; // Contiguous Queues Required
   UINT8     Ams    : 2; // Arbitration Mechanism Supported
   UINT8     Rsvd1  : 5;
-  UINT8     To;     // Timeout
-  UINT16    Dstrd  : 4;
+  UINT8     To;         // Timeout
+  UINT16    Dstrd  : 4; // Doorbell Stride
   UINT16    Nssrs  : 1; // NVM Subsystem Reset Supported NSSRS
   UINT16    Css    : 8; // Command Sets Supported - Bit 37
   UINT16    Bps    : 1; // Boot Partition Support - Bit 45 in NVMe1.4
   UINT16    Rsvd3  : 2;
-  UINT8     Mpsmin : 4;
-  UINT8     Mpsmax : 4;
-  UINT8     Pmrs   : 1;
-  UINT8     Cmbs   : 1;
+  UINT8     Mpsmin : 4; // Memory Page Size Minimum
+  UINT8     Mpsmax : 4; // Memory Page Size Maximum
+  UINT8     Pmrs   : 1; // Persistent Memory Region Supported
+  UINT8     Cmbs   : 1; // Controller Memory Buffer Supported
   UINT8     Rsvd4  : 6;
 } NVME_CAP;
+// MU_CHANGE [END] - Add missing capability descriptions
 
 //
 // 3.1.2 Offset 08h: VS - Version