Re: [PATCH] MdeModulePkg/RamDiskDxe: RamDisk driver to assign non-zero SPA range index

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bassa,
> Damian
> Sent: Tuesday, June 29, 2021 2:22 AM
> To: devel@edk2.groups.io
> Cc: Rusocki, Krzysztof <krzysztof.rusocki@intel.com>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/RamDiskDxe: RamDisk driver to
> assign non-zero SPA range index
> 
> Driver should not use default zero value to assign SPA range since it is invalid
> value according to ACPI spec.
> After the change driver will look for highest index existing in the table.
> If maximum number is already taken it will look for holes in table.


Hello Damian Bassa,

Thanks a lot for the patch.

The reason the current implementation leave the 'SPARangeStructureIndex' field
as value 0 for RamDisk is that in Section 5.2.25.2 of the ACPI 6.1 specification
(also applies for ACPI 6.3):

|> For Virtual CD Region and Virtual Disk Region (both volatile and persistent),
|> the following fields - Proximity Domain, SPA Range Structure Index, Flags, and
|> Address Range Memory Mapping Attribute, are not relevant and shall be set to 0.

Assigning a non-zero value proposed in this patch seems not following the above
statement.

Best Regards,
Hao Wu


> 
> Signed-off-by: Damian Bassa <damian.bassa@intel.com>
> ---
>  .../Disk/RamDiskDxe/RamDiskProtocol.c         | 131 ++++++++++++++++++
>  1 file changed, 131 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> index 4333e00053..b47e3af929 100644
> --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
> @@ -116,6 +116,136 @@ RamDiskPublishSsdt (
>    return Status;
>  }
> 
> +/**
> +  Finds highest Spa Range Index in Nfit table.
> +
> +  @param[in] TableHeader            Points to Nfit table.
> +
> +  @retval Index                     Highest Spa range index
> +  @retval 0                         Free index not found
> +**/
> +UINT16
> +GetHighestSpaRangeIndex (
> +  VOID   *TableHeader
> +  )
> +{
> +  INT32 Length;
> +  UINT16 HighestIndex;
> +
> +  HighestIndex = 0;
> +
> +  EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER                        *NfitStructHeader;
> +  EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE
> *SpaRange;
> +
> +  Length = ((EFI_ACPI_DESCRIPTION_HEADER *)TableHeader)->Length -
> + sizeof (EFI_ACPI_6_1_NVDIMM_FIRMWARE_INTERFACE_TABLE);
> +  NfitStructHeader = (EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER *)
> +                     ((UINT8 *)TableHeader + sizeof
> + (EFI_ACPI_6_1_NVDIMM_FIRMWARE_INTERFACE_TABLE));
> +
> +  while (Length > sizeof(EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER)) {
> +    if (NfitStructHeader->Length <
> sizeof(EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER)) {
> +      break;
> +    }
> +    if ((NfitStructHeader->Type ==
> EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE_TYPE)
> &&
> +        (NfitStructHeader->Length >= sizeof
> (EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE))) {
> +         SpaRange =
> (EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE
> *)NfitStructHeader;
> +         if (SpaRange->SPARangeStructureIndex > HighestIndex) {
> +           HighestIndex = SpaRange->SPARangeStructureIndex;
> +         }
> +    }
> +
> +    //
> +    // Move to the header of next NFIT structure.
> +    //
> +    Length       -= NfitStructHeader->Length;
> +    NfitStructHeader = (EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER *)
> +                       ((UINT8 *)NfitStructHeader +
> +NfitStructHeader->Length);
> +  }
> +  return HighestIndex;
> +}
> +
> +/**
> +  Finds if Spa range index exists in Nfit table.
> +
> +  @param[in] SpaRangeIndex          Number of index to look for.
> +  @param[in] TableHeader            Points to Nfit table.
> +
> +  @retval TRUE                      Index number already exists in the table
> +  @retval FALSE                     Index number doesn't exist in the table
> +**/
> +BOOLEAN
> +DoesSpaIndexExist (
> +  UINT16 SpaRangeIndex,
> +  VOID   *TableHeader
> +  )
> +{
> +  INT32 Length;
> +  EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER                        *NfitStructHeader;
> +  EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE
> *SpaRange;
> +
> +  Length = ((EFI_ACPI_DESCRIPTION_HEADER *)TableHeader)->Length -  +
> + sizeof (EFI_ACPI_6_1_NVDIMM_FIRMWARE_INTERFACE_TABLE);
> +  NfitStructHeader = (EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER *)
> +                     ((UINT8 *)TableHeader + sizeof
> + (EFI_ACPI_6_1_NVDIMM_FIRMWARE_INTERFACE_TABLE));
> +
> +  while (Length > sizeof(EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER)) {
> +    if (NfitStructHeader->Length <
> sizeof(EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER)) {
> +      break;
> +    }
> +    if ((NfitStructHeader->Type ==
> EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE_TYPE)
> &&
> +        (NfitStructHeader->Length >= sizeof
> (EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE))) {
> +         SpaRange =
> (EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE
> *)NfitStructHeader;
> +         if (SpaRange->SPARangeStructureIndex == SpaRangeIndex) {
> +           return TRUE;
> +         }
> +    }
> +
> +    //
> +    // Move to the header of next NFIT structure.
> +    //
> +    Length       -= NfitStructHeader->Length;
> +    NfitStructHeader = (EFI_ACPI_6_1_NFIT_STRUCTURE_HEADER *)
> +                       ((UINT8 *)NfitStructHeader +
> +NfitStructHeader->Length);
> +  }
> +  return FALSE;
> +}
> +
> +/**
> +  Finds lowest available Spa range index in Nfit table.
> +
> +  @param[in] TableHeader            Points to Nfit table.
> +
> +  @retval Index                     Lowest free Spa range index
> +  @retval 0                         Free index not found
> +**/
> +UINT16
> +GetFreeSpaIndex (
> +   VOID   *TableHeader
> +  )
> +{
> +  UINT16 Index;
> +
> +  Index = GetHighestSpaRangeIndex (TableHeader);
> +
> +  //
> +  // If highest range found is not maximum allowed number use value
> +increased by 1
> +  //
> +  if (Index != MAX_UINT16) {
> +    return Index + 1;
> +  }
> +  //
> +  // If highest possible index value is already used, see if there is
> +any that is not taken
> +  //
> +  for (Index = 1; Index < MAX_UINT16; Index++)
> +  {
> +    if (!DoesSpaIndexExist(Index, TableHeader)) {
> +      return Index;
> +    }
> +  }
> +  DEBUG ((
> +    EFI_D_ERROR,
> +    "GetFreeSpaIndex: Nfit index not found, table is full\n"
> +    ));
> +  return 0;
> +}
> 
>  /**
>    Publish the RAM disk NVDIMM Firmware Interface Table (NFIT) to the ACPI
> @@ -334,6 +464,7 @@ RamDiskPublishNfit (
>    SpaRange->Length = sizeof
> (EFI_ACPI_6_1_NFIT_SYSTEM_PHYSICAL_ADDRESS_RANGE_STRUCTURE);
>    SpaRange->SystemPhysicalAddressRangeBase   = PrivateData->StartingAddr;
>    SpaRange->SystemPhysicalAddressRangeLength = PrivateData->Size;
> +  SpaRange->SPARangeStructureIndex = GetFreeSpaIndex (Nfit);
>    CopyGuid (&SpaRange->AddressRangeTypeGUID, &PrivateData->TypeGuid);
> 
>    Checksum             = CalculateCheckSum8((UINT8 *)Nfit, NfitHeader->Length);
> --
> 2.27.0.windows.1
> 
> Intel Technology Poland sp. z o.o.
> ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia
> Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 |
> Kapita zakadowy 200.000 PLN.
> Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe
> zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci,
> prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek
> przegldanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the sole
> use of the intended recipient(s). If you are not the intended recipient, please
> contact the sender and delete all copies; any review or distribution by others is
> strictly prohibited.
> 
> 
> 
> 
>