Re: [edk2-devel] [edk2-staging/RISC-V PATCH v1 1/14]: BaseTools: Update EDK2 build tool for RISC-V platform

[jonathan.cameron@huawei.com]

Hi Abner,

Just noticed in passing that this series doesn't seem to have a 3/14?

Speaking personally it would be useful to have a cover letter with
a quick summary of the series content.

Thanks,

Jonathan

On Tue, 27 Aug 2019 14:00:19 +0800
Abner Chang <abner.chang@hpe.com> wrote:

> Elf64Convert.c
> - Relocation process to hadnle below opcodes,
>  * PCRELHI20
>  * PCRELLO12
>  * ADD32
>  * SUB32
> 
> GenFvInternalLib.c
> - This atches jump instrcution at the position of first instrcution fetched by RISC-V processor after Zeroth Stage Boot Loader (ZSBL).
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Abner Chang <abner.chang@hpe.com>
> ---
>  BaseTools/Source/C/GenFv/GenFvInternalLib.c | 311 ++++++++++++----------------
>  BaseTools/Source/C/GenFw/Elf64Convert.c     |  68 ++++++
>  2 files changed, 197 insertions(+), 182 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> index 01da00c..92abb7c 100644
> --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
> @@ -1956,157 +1956,6 @@ Returns:
>    return EFI_UNSUPPORTED;
>  }
>  
> -EFI_STATUS
> -UpdateRiscvResetVectorIfNeeded (
> -  MEMORY_FILE            *FvImage,
> -  FV_INFO                *FvInfo,
> -  EFI_FFS_FILE_HEADER    *VtfFileImage
> -  )
> -/*++
> -
> -Routine Description:
> -  This parses the FV looking for SEC and patches that address into the 
> -  beginning of the FV header.
> -
> -  For RISC-V ISA, the reset vector is at 0xfff~ff00h or 200h
> -
> -Arguments:
> -  FvImage       Memory file for the FV memory image/
> -  FvInfo        Information read from INF file.
> -  VtfFileImage  Instance of VTF file.
> -
> -Returns:
> -
> -  EFI_SUCCESS             Function Completed successfully.
> -  EFI_ABORTED             Error encountered.
> -  EFI_INVALID_PARAMETER   A required parameter was NULL.
> -  EFI_NOT_FOUND           PEI Core file not found.
> -
> ---*/
> -{
> -  EFI_FFS_FILE_HEADER       *PeiCoreFile;
> -  EFI_FFS_FILE_HEADER       *SecCoreFile;
> -  EFI_STATUS                Status;
> -  EFI_FILE_SECTION_POINTER  Pe32Section;
> -  UINT32                    EntryPoint;
> -  UINT32                    BaseOfCode;
> -  UINT16                    MachineType;
> -  EFI_PHYSICAL_ADDRESS      PeiCorePhysicalAddress;
> -  EFI_PHYSICAL_ADDRESS      SecCorePhysicalAddress;
> -  EFI_PHYSICAL_ADDRESS      TrapAddress;
> -
> -  //
> -  // Verify input parameters
> -  //
> -  if (FvImage == NULL || FvInfo == NULL) {
> -    Error (NULL, 0, 3000, "Invalid", "FvImage or FvInfo is NULL");
> -    return EFI_INVALID_PARAMETER;
> -  }
> -  //
> -  // Initialize FV library
> -  //
> -  InitializeFvLib (FvImage->FileImage, FvInfo->Size);
> -
> -  //
> -  // Find the Sec Core
> -  //
> -  Status = GetFileByType (EFI_FV_FILETYPE_SECURITY_CORE, 1, &SecCoreFile);
> -  if (EFI_ERROR (Status) || SecCoreFile == NULL) {
> -    //
> -    // Maybe hardware does SEC job and we only have PEI Core?
> -    //
> -
> -    //
> -    // Find the PEI Core. It may not exist if SEC loads DXE core directly
> -    //
> -    PeiCorePhysicalAddress = 0;
> -    Status = GetFileByType (EFI_FV_FILETYPE_PEI_CORE, 1, &PeiCoreFile);
> -    if (!EFI_ERROR(Status) && PeiCoreFile != NULL) {
> -      //
> -      // PEI Core found, now find PE32 or TE section
> -      //
> -      Status = GetSectionByType (PeiCoreFile, EFI_SECTION_PE32, 1, &Pe32Section);
> -      if (Status == EFI_NOT_FOUND) {
> -        Status = GetSectionByType (PeiCoreFile, EFI_SECTION_TE, 1, &Pe32Section);
> -      }
> -    
> -      if (EFI_ERROR (Status)) {
> -        Error (NULL, 0, 3000, "Invalid", "could not find either a PE32 or a TE section in PEI core file!");
> -        return EFI_ABORTED;
> -      }
> -    
> -      Status = GetPe32Info (
> -                (VOID *) ((UINTN) Pe32Section.Pe32Section + GetSectionHeaderLength(Pe32Section.CommonHeader)),
> -                &EntryPoint,
> -                &BaseOfCode,
> -                &MachineType
> -                );
> -    
> -      if (EFI_ERROR (Status)) {
> -        Error (NULL, 0, 3000, "Invalid", "could not get the PE32 entry point for the PEI core!");
> -        return EFI_ABORTED;
> -      }
> -      //
> -      // Physical address is FV base + offset of PE32 + offset of the entry point
> -      //
> -      PeiCorePhysicalAddress = FvInfo->BaseAddress;
> -      PeiCorePhysicalAddress += (UINTN) Pe32Section.Pe32Section + GetSectionHeaderLength(Pe32Section.CommonHeader) - (UINTN) FvImage->FileImage;
> -      PeiCorePhysicalAddress += EntryPoint;
> -      DebugMsg (NULL, 0, 9, "PeiCore physical entry point address", "Address = 0x%llX", (unsigned long long) PeiCorePhysicalAddress);
> -      RiscvPatchVtf (VtfFileImage, (UINT32)PeiCorePhysicalAddress);
> -    }
> -    return EFI_SUCCESS;
> -  }
> -  
> -  //
> -  // Sec Core found, now find PE32 section
> -  //
> -  Status = GetSectionByType (SecCoreFile, EFI_SECTION_PE32, 1, &Pe32Section);
> -  if (Status == EFI_NOT_FOUND) {
> -    Status = GetSectionByType (SecCoreFile, EFI_SECTION_TE, 1, &Pe32Section);
> -  }
> -
> -  if (EFI_ERROR (Status)) {
> -    Error (NULL, 0, 3000, "Invalid", "could not find a PE32 section in the SEC core file.");
> -    return EFI_ABORTED;
> -  }
> -
> -  Status = GetPe32Info (
> -            (VOID *) ((UINTN) Pe32Section.Pe32Section + GetSectionHeaderLength(Pe32Section.CommonHeader)),
> -            &EntryPoint,
> -            &BaseOfCode,
> -            &MachineType
> -            );
> -  if (EFI_ERROR (Status)) {
> -    Error (NULL, 0, 3000, "Invalid", "could not get the PE32 entry point for the SEC core.");
> -    return EFI_ABORTED;
> -  }
> -  
> -  if ((MachineType != EFI_IMAGE_MACHINE_RISCV32) && (MachineType != EFI_IMAGE_MACHINE_RISCV64)) {
> -    //
> -    // If SEC is not RISC-V we have nothing to do
> -    //
> -    return EFI_SUCCESS;
> -  }
> -  
> -  //
> -  // Physical address is FV base + offset of PE32 + offset of the entry point
> -  //
> -  SecCorePhysicalAddress = FvInfo->BaseAddress;
> -  SecCorePhysicalAddress += (UINTN) Pe32Section.Pe32Section + GetSectionHeaderLength(Pe32Section.CommonHeader) - (UINTN) FvImage->FileImage;
> -  SecCorePhysicalAddress += EntryPoint;
> -  DebugMsg (NULL, 0, 0x14, "SecCore physical entry point address", "Address = 0x%llX", (unsigned long long) SecCorePhysicalAddress);
> -  RiscvPatchVtf (VtfFileImage, (UINT32)SecCorePhysicalAddress);
> -  //
> -  // Update RISC-V trap handler.
> -  //
> -  TrapAddress = (UINTN) Pe32Section.Pe32Section + GetSectionHeaderLength(Pe32Section.CommonHeader) + EntryPoint;
> -  TrapAddress -= 40;
> -  RiscvPatchVtfTrapHandler (VtfFileImage, TrapAddress);
> -
> -  DebugMsg (NULL, 0, 9, "Update Reset vector in FV Header", NULL);
> -  return EFI_SUCCESS;
> -}
>  
>  EFI_STATUS
>  FindCorePeSection(
> @@ -2581,6 +2430,106 @@ Returns:
>  }
>  
>  EFI_STATUS
> +UpdateRiscvResetVectorIfNeeded (
> +  MEMORY_FILE            *FvImage,
> +  FV_INFO                *FvInfo
> +  )
> +/*++
> +
> +Routine Description:
> +  This parses the FV looking for SEC and patches that address into the 
> +  beginning of the FV header.
> +
> +  For RISC-V ISA, the reset vector is at 0xfff~ff00h or 200h
> +
> +Arguments:
> +  FvImage       Memory file for the FV memory image/
> +  FvInfo        Information read from INF file.
> +
> +Returns:
> +
> +  EFI_SUCCESS             Function Completed successfully.
> +  EFI_ABORTED             Error encountered.
> +  EFI_INVALID_PARAMETER   A required parameter was NULL.
> +  EFI_NOT_FOUND           PEI Core file not found.
> +
> +--*/
> +{
> +  EFI_STATUS                Status;
> +  UINT16                    MachineType;
> +  EFI_FILE_SECTION_POINTER  SecPe32;
> +  EFI_PHYSICAL_ADDRESS      SecCoreEntryAddress;
> +
> +  UINT32 bSecCore;
> +  UINT32 tmp;
> +
> +
> +  //
> +  // Verify input parameters
> +  //
> +  if (FvImage == NULL || FvInfo == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  //
> +  // Initialize FV library
> +  //
> +  InitializeFvLib (FvImage->FileImage, FvInfo->Size);
> +
> +  //
> +  // Find the Sec Core
> +  //
> +  Status = FindCorePeSection(FvImage->FileImage, FvInfo->Size, EFI_FV_FILETYPE_SECURITY_CORE, &SecPe32); 
> +  if(EFI_ERROR(Status)) {
> +    printf("skip because Secutiry Core not found\n");
> +    return EFI_SUCCESS;
> +  }
> +
> +  DebugMsg (NULL, 0, 9, "Update SEC core in FV Header", NULL);
> +
> +  Status = GetCoreMachineType(SecPe32, &MachineType);
> +  if(EFI_ERROR(Status)) {
> +    Error(NULL, 0, 3000, "Invalid", "Could not get the PE32 machine type for SEC core.");
> +    return EFI_ABORTED;
> +  }
> +
> +  if ((MachineType != EFI_IMAGE_MACHINE_RISCV32) && (MachineType != EFI_IMAGE_MACHINE_RISCV64)) {
> +    Error(NULL, 0, 3000, "Invalid", "Could not update SEC core because Machine type is not RiscV.");
> +    return EFI_ABORTED;
> +  }
> +
> +  Status = GetCoreEntryPointAddress(FvImage->FileImage, FvInfo, SecPe32, &SecCoreEntryAddress);
> +  if(EFI_ERROR(Status)) {
> +    Error(NULL, 0, 3000, "Invalid", "Could not get the PE32 entry point address for SEC Core.");
> +    return EFI_ABORTED;
> +  }
> +
> +  VerboseMsg("SecCore entry point Address = 0x%llX", (unsigned long long) SecCoreEntryAddress);
> +  VerboseMsg("BaseAddress = 0x%llX", (unsigned long long) FvInfo->BaseAddress);
> +  bSecCore = (SecCoreEntryAddress - FvInfo->BaseAddress);
> +  VerboseMsg("offset = 0x%llX", bSecCore);
> +
> +  if(bSecCore > 0x0fffff) {
> +    Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 1MB of start of the FV");
> +    return EFI_ABORTED;
> +  }
> +
> +  tmp = bSecCore;
> +  bSecCore = 0;
> +  //J-type
> +  bSecCore  = (tmp&0x100000)<<11; //imm[20]    at bit[31]
> +  bSecCore |= (tmp&0x0007FE)<<20; //imm[10:1]  at bit[30:21]
> +  bSecCore |= (tmp&0x000800)<<9;  //imm[11]    at bit[20]
> +  bSecCore |= (tmp&0x0FF000);     //imm[19:12] at bit[19:12]
> +  bSecCore |= 0x6F; //JAL opcode
> +
> +  memcpy(FvImage->FileImage, &bSecCore, sizeof(bSecCore));
> +
> +  return EFI_SUCCESS;
> +}
> +
> +
> +
> +EFI_STATUS
>  GetPe32Info (
>    IN UINT8                  *Pe32,
>    OUT UINT32                *EntryPoint,
> @@ -3037,7 +2986,6 @@ Returns:
>      FvHeader->Checksum      = 0;
>      FvHeader->Checksum      = CalculateChecksum16 ((UINT16 *) FvHeader, FvHeader->HeaderLength / sizeof (UINT16));
>    }
> -
>    //
>    // Add files to FV
>    //
> @@ -3069,39 +3017,22 @@ Returns:
>        goto Finish;
>      }
>  
> -    if (mRiscV) {
> +    if (!mArm && !mRiscV) {
>        //
> -      // Update RISCV reset vector.
> +      // Update reset vector (SALE_ENTRY for IPF)
> +      // Now for IA32 and IA64 platform, the fv which has bsf file must have the
> +      // EndAddress of 0xFFFFFFFF. Thus, only this type fv needs to update the
> +      // reset vector. If the PEI Core is found, the VTF file will probably get
> +      // corrupted by updating the entry point.
>        //
> -      DebugMsg (NULL, 0, INFO_LOG_LEVEL, "Update RISCV reset vector", NULL);
> -      Status = UpdateRiscvResetVectorIfNeeded (&FvImageMemoryFile, &mFvDataInfo, VtfFileImage);
> -      if (EFI_ERROR (Status)) {
> -          Error (NULL, 0, 3000, "Invalid", "Could not update the reset vector for RISC-V.");
> +      if ((mFvDataInfo.BaseAddress + mFvDataInfo.Size) == FV_IMAGES_TOP_ADDRESS) {
> +        Status = UpdateResetVector (&FvImageMemoryFile, &mFvDataInfo, VtfFileImage);
> +        if (EFI_ERROR(Status)) {
> +          Error (NULL, 0, 3000, "Invalid", "Could not update the reset vector.");
>            goto Finish;
> -      }
> -      //
> -      // Update Checksum for FvHeader
> -      //
> -      FvHeader->Checksum = 0;
> -      FvHeader->Checksum = CalculateChecksum16 ((UINT16 *) FvHeader, FvHeader->HeaderLength / sizeof (UINT16));
> -    } else {
> -        if (!mArm) {
> -          //
> -          // Update reset vector (SALE_ENTRY for IPF)
> -          // Now for IA32 and IA64 platform, the fv which has bsf file must have the
> -          // EndAddress of 0xFFFFFFFF. Thus, only this type fv needs to update the
> -          // reset vector. If the PEI Core is found, the VTF file will probably get
> -          // corrupted by updating the entry point.
> -          //
> -          if ((mFvDataInfo.BaseAddress + mFvDataInfo.Size) == FV_IMAGES_TOP_ADDRESS) {
> -            Status = UpdateResetVector (&FvImageMemoryFile, &mFvDataInfo, VtfFileImage);
> -            if (EFI_ERROR(Status)) {
> -              Error (NULL, 0, 3000, "Invalid", "Could not update the reset vector.");
> -              goto Finish;
> -            }
> -            DebugMsg (NULL, 0, 9, "Update Reset vector in VTF file", NULL);
> -          }
>          }
> +        DebugMsg (NULL, 0, 9, "Update Reset vector in VTF file", NULL);
> +      }
>      }
>    } 
>  
> @@ -3119,6 +3050,22 @@ Returns:
>      FvHeader->Checksum = CalculateChecksum16 ((UINT16 *) FvHeader, FvHeader->HeaderLength / sizeof (UINT16));
>    }
>  
> +  if (mRiscV) {
> +     //
> +     // Update RISCV reset vector.
> +     //
> +     Status = UpdateRiscvResetVectorIfNeeded (&FvImageMemoryFile, &mFvDataInfo);
> +     if (EFI_ERROR (Status)) {
> +       Error (NULL, 0, 3000, "Invalid", "Could not update the reset vector for RISC-V.");
> +       goto Finish;
> +    }
> +    //
> +    // Update Checksum for FvHeader
> +    //
> +    FvHeader->Checksum = 0;
> +    FvHeader->Checksum = CalculateChecksum16 ((UINT16 *) FvHeader, FvHeader->HeaderLength / sizeof (UINT16));
> +  }
> +
>    //
>    // Update FV Alignment attribute to the largest alignment of all the FFS files in the FV
>    //
> @@ -3853,7 +3800,7 @@ Returns:
>      ImageContext.DestinationAddress = NewPe32BaseAddress;
>      Status                          = PeCoffLoaderRelocateImage (&ImageContext);
>      if (EFI_ERROR (Status)) {
> -      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of %s", FileName);
> +      Error (NULL, 0, 3000, "Invalid", "RelocateImage() call failed on rebase of %s Status=%d", FileName, Status);
>        free ((VOID *) MemoryImagePointer);
>        return Status;
>      }
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 4857485..77b4d53 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -946,8 +946,60 @@ WriteSections64 (
>              RiscvSymSecIndex = 0;
>              break;
>  
> +          case R_RISCV_PCREL_HI20:
> +            RiscvHi20Targ = Targ;
> +            RiscvHi20Sym = SymShdr;
> +            RiscvSymSecIndex = Sym->st_shndx;
> +
> +            Value = (UINT32)(RV_X(*(UINT32 *)RiscvHi20Targ, 12, 20));
> +            printf("PCREL_HI20 Sym:[%s] value:0x%x SymShdr->sh_addr:0x%lx mCoffSectionOffset:%x \n", GetSymName(Sym), Value, SymShdr->sh_addr, mCoffSectionsOffset[Sym->st_shndx]);
> +            break;
> +          case R_RISCV_PCREL_LO12_I:
> +            if (RiscvHi20Targ != NULL && RiscvHi20Sym != NULL && RiscvSymSecIndex != 0) {
> +              int i;
> +              Value2 = (UINT32)(RV_X(*(UINT32 *)RiscvHi20Targ, 12, 20));
> +              Value = (UINT32)(RV_X(*(UINT32 *)Targ, 20, 12));
> +              if(Value & (RISCV_IMM_REACH/2)) {
> +                Value |= ~(RISCV_IMM_REACH-1);
> +              }
> +              printf("PCREL_LO12_I Sym:[%s] value:0x%x SymShdr->sh_addr:0x%lx mCoffSectionOffset:%x \n", GetSymName(Sym), Value, SymShdr->sh_addr, mCoffSectionsOffset[Sym->st_shndx]);
> +              Value = Value - RiscvHi20Sym->sh_addr + mCoffSectionsOffset[RiscvSymSecIndex];
> +              if(-2048 > (INT32)Value) {           
> +                i = (-Value / 4096);               
> +                //Error (NULL, 0, 3000, "Invalid", "WriteSections64(): PCREL_LO12_I relocation out of range. %d i=%d", Value, i);
> +                printf("WriteSections64(): PCREL_LO12_I relocation out of range. Value:%d Value2:%d i=%d\n", Value, Value2, i);
> +                Value2 -= i;
> +                Value += 4096 * i;
> +                if(-2048 > (INT32)Value) {
> +                  Value2 -= 1;
> +                  Value += 4096;
> +                }
> +              }
> +              else if( 2047 < (INT32)Value) {
> +                i = (Value / 4096);
> +                //Error (NULL, 0, 3000, "Invalid", "WriteSections64(): PCREL_LO12_I relocation out of range. %d i=%d", Value, i);
> +                printf("WriteSections64(): PCREL_LO12_I relocation out of range. Value:%d Value2:%d i=%d\n", Value, Value2, i);
> +                Value2 += i;
> +                Value -= 4096 * i;
> +                if(2047 < (INT32)Value) {
> +                  Value2 += 1;
> +                  Value -= 4096;
> +                }
> +              }
> +
> +              *(UINT32 *)Targ = (RV_X(Value, 0, 12) << 20) | (RV_X(*(UINT32*)Targ, 0, 20));
> +              *(UINT32 *)RiscvHi20Targ = (RV_X(Value2, 0, 20)<<12) | (RV_X(*(UINT32 *)RiscvHi20Targ, 0, 12));
> +              printf("PCREL_LO12_I Sym:[%s] relocated value:0x%x(%d) value2:0x%x(%d) SymShdr->sh_addr:0x%lx mCoffSectionOffset:%x \n", GetSymName(Sym), Value, Value, Value2, Value2,  SymShdr->sh_addr, mCoffSectionsOffset[Sym->st_shndx]);
> +            }
> +            RiscvHi20Sym = NULL;
> +            RiscvHi20Targ = NULL;
> +            RiscvSymSecIndex = 0;
> +            break;
> +
>            case R_RISCV_ADD64:
>            case R_RISCV_SUB64:
> +          case R_RISCV_ADD32:
> +          case R_RISCV_SUB32:
>            case R_RISCV_BRANCH:
>            case R_RISCV_JAL:
>            case R_RISCV_GPREL_I:
> @@ -1120,6 +1172,20 @@ WriteRelocations64 (
>                  EFI_IMAGE_REL_BASED_ABSOLUTE);
>                break;
>  
> +            case R_RISCV_ADD32:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
> +            case R_RISCV_SUB32:
> +              CoffAddFixup(
> +                (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> +                + (Rel->r_offset - SecShdr->sh_addr)),
> +                EFI_IMAGE_REL_BASED_ABSOLUTE);
> +              break;
> +
>              case R_RISCV_BRANCH:
>                CoffAddFixup(
>                  (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info]
> @@ -1145,6 +1211,8 @@ WriteRelocations64 (
>              case R_RISCV_SET8:
>              case R_RISCV_SET16:
>              case R_RISCV_SET32:
> +            case R_RISCV_PCREL_HI20:
> +            case R_RISCV_PCREL_LO12_I:
>                break;
>  
>              default: