From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: huawei.com, ip: 45.249.212.35, mailfrom: jonathan.cameron@huawei.com) Received: from huawei.com (huawei.com [45.249.212.35]) by groups.io with SMTP; Wed, 28 Aug 2019 01:18:11 -0700 Received: from DGGEMS405-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id 1819E46B6A6C0DB9CCA9; Wed, 28 Aug 2019 16:18:08 +0800 (CST) Received: from localhost (10.202.226.61) by DGGEMS405-HUB.china.huawei.com (10.3.19.205) with Microsoft SMTP Server id 14.3.439.0; Wed, 28 Aug 2019 16:18:04 +0800 Date: Wed, 28 Aug 2019 09:17:56 +0100 From: jonathan.cameron@huawei.com To: Abner Chang CC: Subject: Re: [edk2-devel] [edk2-staging/RISC-V PATCH v1 1/14]: BaseTools: Update EDK2 build tool for RISC-V platform Message-ID: <20190828091756.00004ffc@huawei.com> In-Reply-To: <1566885632-5747-1-git-send-email-abner.chang@hpe.com> References: <1566885632-5747-1-git-send-email-abner.chang@hpe.com> Organization: Huawei X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; i686-w64-mingw32) MIME-Version: 1.0 X-Originating-IP: [10.202.226.61] X-CFilter-Loop: Reflected Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit 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 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 > --- > 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: