Re: [edk2-devel] [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 Wu, Hao A
> Sent: Tuesday, June 29, 2021 8:24 AM
> To: devel@edk2.groups.io; Bassa, Damian <damian.bassa@intel.com>
> Cc: Rusocki, Krzysztof <krzysztof.rusocki@intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/RamDiskDxe: RamDisk driver
> to assign non-zero SPA range index
> 
> > -----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):


Fix a typo: "also applies for ACPI 6.4"

Best Regards,
Hao Wu


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