Make ipmctl more resilient to unhealthy PMem modules

Only zero out headers for PCD and LSA when calling delete -pcd. Will reduce
runtime on non-functional PMem modules from 10 minutes down to 10 seconds. In
FwCmdSetPlatformConfigData, use the same flow for PCD and LSA when we
set PcdSize, there is no reason for there to be a difference.

For non-functional PMem modules, allow delete -goal to work on them and
exclude them from checking for existing goals on the socket (since we are
not going to provision them with create -goal).

Don't bail out when there are errors retrieving PCD during Retrieve Goal
Configs. However, make sure to preserve the behavior used in the past for
when EFI_NO_RESPONSE is returned ("One or more dimms are busy"...)

Signed-off-by: Nolan Hergert <nolan.hergert@intel.com>

Author: nolanhergert

DcpmPkg/driver/Core/Dimm.c

@@ -2857,7 +2857,6 @@ FwCmdSetPlatformConfigData (
       ReturnCode = EFI_INVALID_PARAMETER;
       goto Finish;
     }
-    PcdSize = RawDataSize;
   } else if (PartitionId == PCD_LSA_PARTITION_ID) {
     if (gPCDCacheEnabled) {
       if (NULL == pDimm->pPcdLsa) {
@@ -2866,8 +2865,10 @@ FwCmdSetPlatformConfigData (
       pTempCache = pDimm->pPcdLsa;
       pTempCacheSz = pDimm->PcdLsaPartitionSize;
     }
-    PcdSize = pDimm->PcdLsaPartitionSize;
   }
+
+  PcdSize = RawDataSize;
+
   if (PcdSize == 0) {
     ReturnCode = EFI_INVALID_PARAMETER;
     goto Finish;

DcpmPkg/driver/Core/Namespace.c

@@ -1526,7 +1526,7 @@ WriteLabelStorageArea(
 }
 
 /**
-  Zero the Label Storage Area on the specified DIMM.
+  Zero the Label Storage Area Header on the specified DIMM.
 
   @param[in] DimmPid Dimm ID of DIMM on which to write LSA
 
@@ -1535,13 +1535,16 @@ WriteLabelStorageArea(
   @retval EFI_SUCCESS LSA written correctly
 **/
 EFI_STATUS
-ZeroLabelStorageArea(
+ZeroLabelStorageAreaHeader(
   IN     UINT16 DimmPid
   )
 {
   EFI_STATUS ReturnCode = EFI_INVALID_PARAMETER;
   DIMM *pDimm = NULL;
-  UINT8 *pZeroRawLsa = NULL;
+  UINT8 *pZeroRawLsaHeader = NULL;
+  // 2 LSA header index blocks corresponding to 256KB partitions. We only
+  // have 128KB partitions in PMem (256B index blocks), but rounding up just in case.
+  CONST UINT16 BytesToZero = 2 * 512;
 
   NVDIMM_ENTRY();
 
@@ -1550,22 +1553,22 @@ ZeroLabelStorageArea(
     goto Finish;
   }
 
-  pZeroRawLsa = AllocateZeroPool(pDimm->PcdLsaPartitionSize);
-  if (pZeroRawLsa == NULL) {
+  pZeroRawLsaHeader = AllocateZeroPool(BytesToZero);
+  if (pZeroRawLsaHeader == NULL) {
     ReturnCode = EFI_OUT_OF_RESOURCES;
     goto Finish;
   }
 
-  NVDIMM_DBG("Zero-ing the LSA on DIMM 0x%x ...", pDimm->DeviceHandle.AsUint32);
+  NVDIMM_DBG("Zero-ing the LSA header on DIMM 0x%x ...", pDimm->DeviceHandle.AsUint32);
   ReturnCode = FwCmdSetPlatformConfigData(pDimm, PCD_LSA_PARTITION_ID,
-    pZeroRawLsa, pDimm->PcdLsaPartitionSize);
+    pZeroRawLsaHeader, BytesToZero);
   if (EFI_ERROR(ReturnCode)) {
     NVDIMM_DBG("FwCmdSetPlatformConfigData returned: " FORMAT_EFI_STATUS "", ReturnCode);
     goto Finish;
   }
 
 Finish:
-  FREE_POOL_SAFE(pZeroRawLsa);
+  FREE_POOL_SAFE(pZeroRawLsaHeader);
   NVDIMM_EXIT_I64(ReturnCode);
   return ReturnCode;
 }

DcpmPkg/driver/Core/Namespace.h

@@ -330,7 +330,7 @@ WriteLabelStorageArea(
   );
 
 /**
-  Zero the Label Storage Area on the specified DIMM.
+  Zero the Label Storage Area Header on the specified DIMM.
 
   @param[in] DimmPid Dimm ID of DIMM on which to write LSA
 
@@ -339,7 +339,7 @@ WriteLabelStorageArea(
   @retval EFI_SUCCESS LSA written correctly
 **/
 EFI_STATUS
-ZeroLabelStorageArea(
+ZeroLabelStorageAreaHeader(
   IN     UINT16 DimmPid
   );
 

DcpmPkg/driver/Core/Region.c

@@ -860,51 +860,46 @@ RetrieveISsFromPlatformConfigData(
     return EFI_INVALID_PARAMETER;
   }
 
-  for (pDimmNode = GetFirstNode(pDimmList);
-      !IsNull(pDimmList, pDimmNode);
-      pDimmNode = GetNextNode(pDimmList, pDimmNode)) {
+  LIST_FOR_EACH(pDimmNode, pDimmList) {
     pDimm = DIMM_FROM_NODE(pDimmNode);
 
+    // Set default values
+    pDimm->ConfigStatus = DIMM_CONFIG_UNDEFINED;
+    pDimm->IsNew = 0;
+    pDimm->Configured = FALSE;
+
     if (!IsDimmManageable(pDimm) || DIMM_MEDIA_NOT_ACCESSIBLE(pDimm->BootStatusBitmask)) {
       continue;
     }
 
+    // Free previous use of pcd header if needed
+    FREE_POOL_SAFE(pPcdConfHeader);
     ReturnCode = GetPlatformConfigDataOemPartition(pDimm, FALSE, &pPcdConfHeader);
 #ifdef MEMORY_CORRUPTION_WA
-      if (ReturnCode == EFI_DEVICE_ERROR) {
-        ReturnCode = GetPlatformConfigDataOemPartition(pDimm, FALSE, &pPcdConfHeader);
-      }
+    if (ReturnCode == EFI_DEVICE_ERROR) {
+      ReturnCode = GetPlatformConfigDataOemPartition(pDimm, FALSE, &pPcdConfHeader);
+    }
 #endif // MEMORY_CORRUPTIO_WA
     if (EFI_ERROR(ReturnCode)) {
+      // Ignore all errors except for PMem module busy with sanitize operation
       if (EFI_NO_RESPONSE == ReturnCode) {
         /* Save the return code here and continue with the execution for rest of the dimms.
           This is done to make the UEFI initialization succeed. During UEFI init,
           return code will be ignored but we have to error out when the actual command is executed. */
         IReturnCode = ReturnCode;
       }
-      /* set these values like they were never set */
-      pDimm->ConfigStatus = DIMM_CONFIG_UNDEFINED;
-      pDimm->IsNew = 0;
-      pDimm->Configured = FALSE;
+      ReturnCode = EFI_SUCCESS;
       continue;
     }
 
     if (pPcdConfHeader->CurrentConfStartOffset == 0 || pPcdConfHeader->CurrentConfDataSize == 0) {
       NVDIMM_DBG("There is no Current Config table");
-      FreePool(pPcdConfHeader);
-      pPcdConfHeader = NULL;
-      /* set these values like they were never set */
-      pDimm->ConfigStatus = DIMM_CONFIG_UNDEFINED;
-      pDimm->IsNew = 0;
-      pDimm->Configured = FALSE;
       continue;
     }
 
     pPcdCurrentConf = GET_NVDIMM_CURRENT_CONFIG(pPcdConfHeader);
 
     if (!IsPcdCurrentConfHeaderValid(pPcdCurrentConf, pDimm->PcdOemPartitionSize)) {
-      FreePool(pPcdConfHeader);
-      pPcdConfHeader = NULL;
       continue;
     }
 
@@ -1945,6 +1940,7 @@ RetrieveGoalConfigsFromPlatformConfigData(
   )
 {
   EFI_STATUS ReturnCode = EFI_SUCCESS;
+  EFI_STATUS IReturnCode = EFI_SUCCESS;
   DIMM *pDimm = NULL;
   LIST_ENTRY *pDimmNode = NULL;
   NVDIMM_CONFIGURATION_HEADER *pPcdConfHeader = NULL;
@@ -1957,7 +1953,6 @@ RetrieveGoalConfigsFromPlatformConfigData(
   UINT32 RegionGoalsNum = 0;
   REGION_GOAL *pNewRegionGoal = NULL;
   BOOLEAN New = FALSE;
-  BOOLEAN ValidConfigGoal = TRUE;
   UINT32 SequenceIndex = 0;
   ACPI_REVISION PcdCinRev;
   UINT8 InterleaveChangeStatus = 0;
@@ -1978,56 +1973,61 @@ RetrieveGoalConfigsFromPlatformConfigData(
   LIST_FOR_EACH(pDimmNode, pDimmList) {
     pDimm = DIMM_FROM_NODE(pDimmNode);
 
-    // Skip PMem modules that we can't read from
+    // Set default values
+    pDimm->GoalConfigStatus = GOAL_CONFIG_STATUS_NO_GOAL_OR_SUCCESS;
+    pDimm->RegionsGoalConfig = FALSE;
+    pDimm->PcdSynced = TRUE;
+
     if (!IsDimmManageable(pDimm) || DIMM_MEDIA_NOT_ACCESSIBLE(pDimm->BootStatusBitmask)) {
       continue;
     }
 
+    // Free previous use of pcd header if needed
+    FREE_POOL_SAFE(pPcdConfHeader);
     ReturnCode = GetPlatformConfigDataOemPartition(pDimm, RestoreCorrupt, &pPcdConfHeader);
 #ifdef MEMORY_CORRUPTION_WA
-  if (ReturnCode == EFI_DEVICE_ERROR) {
-    ReturnCode = GetPlatformConfigDataOemPartition(pDimm, RestoreCorrupt, &pPcdConfHeader);
-  }
+    if (ReturnCode == EFI_DEVICE_ERROR) {
+      ReturnCode = GetPlatformConfigDataOemPartition(pDimm, RestoreCorrupt, &pPcdConfHeader);
+    }
 #endif // MEMORY_CORRUPTIO_WA
     if (EFI_ERROR(ReturnCode)) {
-      goto FinishError;
+      // Ignore all errors except for PMem module busy with sanitize operation
+      if (EFI_NO_RESPONSE == ReturnCode) {
+        /* Save the return code here and continue with the execution for rest of the dimms.
+          This is done to make the UEFI initialization succeed. During UEFI init,
+          return code will be ignored but we have to error out when the actual command is executed. */
+        IReturnCode = ReturnCode;
+      }
+      ReturnCode = EFI_SUCCESS;
+      continue;
     }
 
     if (NULL != pPcdConfHeader) {
       pPcdConfInput = GET_NVDIMM_PLATFORM_CONFIG_INPUT(pPcdConfHeader);
       pPcdConfOutput = GET_NVDIMM_PLATFORM_CONFIG_OUTPUT(pPcdConfHeader);
     }
 
-    ValidConfigGoal = TRUE;
-
     // If no PCD Header, CIN record then no goal
     if ((NULL == pPcdConfHeader) || (pPcdConfHeader->ConfInputStartOffset == 0) || (pPcdConfHeader->ConfInputDataSize == 0)) {
       NVDIMM_DBG("There is no Config Input table");
-      ValidConfigGoal = FALSE;
+      continue;
     }
     // CIN is corrupt
     else if (!IsPcdConfInputHeaderValid(pPcdConfInput, pDimm->PcdOemPartitionSize)) {
       pPcdConfHeader->ConfInputStartOffset = 0;
       pPcdConfHeader->ConfInputDataSize = 0;
       NVDIMM_DBG("The Config Input table is corrupted, Ignoring it");
-      ValidConfigGoal = FALSE;
+      continue;
     }
     // If CIN and COUT sequence are the same, then goal attempted to be applied already
     else if ((pPcdConfHeader->ConfOutputStartOffset != 0) && (pPcdConfHeader->ConfOutputDataSize != 0) &&
       IsPcdConfOutputHeaderValid(pPcdConfOutput, pDimm->PcdOemPartitionSize) &&
       (pPcdConfInput->SequenceNumber == pPcdConfOutput->SequenceNumber)) {
       NVDIMM_DBG("The config goal is already applied");
-      ValidConfigGoal = FALSE;
-    }
-
-    if (!ValidConfigGoal) {
-      pDimm->GoalConfigStatus = GOAL_CONFIG_STATUS_NO_GOAL_OR_SUCCESS;
-      pDimm->RegionsGoalConfig = FALSE;
-      pDimm->PcdSynced = TRUE;
-      FREE_POOL_SAFE(pPcdConfHeader);
       continue;
     }
 
+    // We have a valid goal after this point
     PcdCinRev = pPcdConfInput->Header.Revision;
 
     pDimm->PcdSynced = FALSE;
@@ -2252,6 +2252,8 @@ RetrieveGoalConfigsFromPlatformConfigData(
   ClearInternalGoalConfigsInfo(pDimmList);
 Finish:
   FREE_POOL_SAFE(pPcdConfHeader);
+
+  ReturnCode = (IReturnCode != EFI_SUCCESS) ? IReturnCode : ReturnCode;
   NVDIMM_EXIT_I64(ReturnCode);
   return ReturnCode;
 }
@@ -5043,8 +5045,7 @@ CheckForExistingGoalConfigPerSocket(
     }
     LIST_FOR_EACH(pDimmNode, &gNvmDimmData->PMEMDev.Dimms) {
       pDimm = DIMM_FROM_NODE(pDimmNode);
-
-      if (!IsDimmManageable(pDimm) || (Socket != pDimm->SocketId)) {
+      if (!IsDimmManageable(pDimm) || pDimm->NonFunctional || (Socket != pDimm->SocketId)) {
         continue;
       }
 

DcpmPkg/driver/Protocol/Driver/NvmDimmConfig.c

@@ -3729,7 +3729,10 @@ ModifyPcdConfig(
   UINT32 Index = 0;
   UINT32 SecurityState = 0;
   NVDIMM_CONFIGURATION_HEADER *pConfigHeader = NULL;
-  UINT32 ConfigSize = 0;
+  // Only need to zero out headers, not the whole 64K partition.
+  // Adding 256 bytes just to make sure
+  UINT32 ConfigSize = sizeof(NVDIMM_CONFIGURATION_HEADER) + sizeof(NVDIMM_PLATFORM_CONFIG_INPUT)
+                      + sizeof(NVDIMM_CURRENT_CONFIG) + sizeof(NVDIMM_PLATFORM_CONFIG_OUTPUT) + 256;
 
   NVDIMM_ENTRY();
 
@@ -3780,7 +3783,7 @@ ModifyPcdConfig(
 
     //zero LSA
     if (ConfigIdMask & DELETE_PCD_CONFIG_LSA_MASK) {
-      TmpReturnCode = ZeroLabelStorageArea(pDimms[Index]->DimmID);
+      TmpReturnCode = ZeroLabelStorageAreaHeader(pDimms[Index]->DimmID);
       if (EFI_ERROR(TmpReturnCode)) {
         KEEP_ERROR(ReturnCode, TmpReturnCode);
         SetObjStatusForDimm(pCommandStatus, pDimms[Index], NVM_ERR_OPERATION_FAILED);
@@ -3806,15 +3809,6 @@ ModifyPcdConfig(
         continue;
       }
 
-      //determine the size of the PCD partition, which will be used at the end to write the partion back to PCD
-      TmpReturnCode = GetPcdOemDataSize(pConfigHeader, &ConfigSize);
-      if (EFI_ERROR(TmpReturnCode)) {
-        KEEP_ERROR(ReturnCode, TmpReturnCode);
-        SetObjStatusForDimm(pCommandStatus, pDimms[Index], NVM_ERR_OPERATION_FAILED);
-        NVDIMM_DBG("Failed to get PCD size");
-        continue;
-      }
-
       //clear CIN
       if (ConfigIdMask & DELETE_PCD_CONFIG_CIN_MASK) {
         pConfigHeader->ConfInputDataSize = 0x0;
@@ -6615,7 +6609,6 @@ DeleteGoalConfig (
   /** Verify input parameters and determine a list of DIMMs **/
   ReturnCode = VerifyTargetDimms(pDimmIds, DimmIdsCount, pSocketIds, SocketIdsCount,
       REQUIRE_DCPMMS_MANAGEABLE |
-      REQUIRE_DCPMMS_FUNCTIONAL |
       REQUIRE_DCPMMS_MEDIA_ACCESSIBLE,
       pDimms, &DimmsNum, pCommandStatus);
   if (EFI_ERROR(ReturnCode) || pCommandStatus->GeneralStatus != NVM_ERR_OPERATION_NOT_STARTED) {