public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Daniel Schaefer" <daniel.schaefer@hpe.com>
To: Heinrich Schuchardt <xypron.glpk@gmx.de>,
	Sunil V L <sunilvl@ventanamicro.com>, <devel@edk2.groups.io>
Cc: <sunil.vl@gmail.com>, Abner Chang <abner.chang@hpe.com>,
	Bob Feng <bob.c.feng@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	Yuwei Chen <yuwei.chen@intel.com>
Subject: Re: [PATCH] Add support for RISCV GOT/PLT relocations
Date: Fri, 28 May 2021 13:31:26 +0800	[thread overview]
Message-ID: <366dedb1-3e32-5949-ac6e-617db72be1d5@hpe.com> (raw)
In-Reply-To: <6A934026-DA4B-4151-BBE0-34D80B255880@gmx.de>

On 5/28/21 12:42 PM, Heinrich Schuchardt wrote:
> Am 27. Mai 2021 16:41:13 MESZ schrieb Sunil V L <sunilvl@ventanamicro.com>:
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3096
>>
>> This patch adds support for R_RISCV_CALL_PLT and R_RISCV_GOT_HI20
>> relocations.
>>
> 
> gnu-efi tries to avoid GOT based relocations on ARM using
> 
> #if defined(__GNUC__) && !__STDC_HOSTED__
> #pragma GCC visibility push (hidden)
> #endif
> 
> Maybe this is something we should additionally explore.

That's an interesting idea. I see they put this in the header that's 
imported by everything. I tried to put it in

BaseTools/Source/C/Include/RiscV64/ProcessorBind.h

where we define the EDK2 types and I still get errors like this:

WriteSections64(): 
/Build/RiscvVirt/DEBUG_GCC5/RISCV64/MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe/DEBUG/EhciDxe.dll 
unsupported ELF EM_RISCV64 relocation 0x13. 

WriteSections64(): 
/Build/RiscvVirt/DEBUG_GCC5/RISCV64/MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe/DEBUG/UsbKbDxe.dll 
unsupported ELF EM_RISCV64 relocation 0x14. 

GenFw: ERROR 3000: Invalid

Perhaps something else, a compiler flag, is needed? 
 
 



This patch isn't just about GOT though. Sunil also found mistakes in 
some other relocation types that cause issues with newer GCC toolchains.
So it's needed either way.

More context for the others: Currently we've been using an old toolchain 
from 2018 and people had to download our precompiled binaries or build 
it from a specific commit.
Now with these patches they can just use their distribution's riscv64 
toolchain and compile edk2 with it. At least Debian and Fedora ship them.

> 
> Best regards
> 
> Heinrich
> 
> 
> 
> 
>> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>> Cc: Abner Chang <abner.chang@hpe.com>
>> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> BaseTools/Source/C/GenFw/Elf64Convert.c | 45 +++++++++++++++++++++----
>> 1 file changed, 39 insertions(+), 6 deletions(-)
>>
>> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> index d097db8632..d05dcf9992 100644
>> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
>> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
>> @@ -129,6 +129,8 @@ STATIC UINT32 mDebugOffset;
>> STATIC UINT8       *mRiscVPass1Targ = NULL;>
>> STATIC Elf_Shdr    *mRiscVPass1Sym = NULL;>
>> STATIC Elf64_Half  mRiscVPass1SymSecIndex = 0;>
>> +STATIC INT32       mRiscVPass1Offset;>
>> +STATIC INT32       mRiscVPass1GotFixup;>
>>>
>> //>
>> // Initialization Function>
>> @@ -479,11 +481,11 @@ WriteSectionRiscV64 (
>>      break;>
>>>
>>    case R_RISCV_32:>
>> -    *(UINT32 *)Targ = (UINT32)((UINT64)(*(UINT32 *)Targ) -
>> SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx]);>
>> +    *(UINT64 *)Targ = Sym->st_value + Rel->r_addend;>
>>      break;>
>>>
>>    case R_RISCV_64:>
>> -    *(UINT64 *)Targ = *(UINT64 *)Targ - SymShdr->sh_addr +
>> mCoffSectionsOffset[Sym->st_shndx];>
>> +    *(UINT64 *)Targ = Sym->st_value + Rel->r_addend;>
>>      break;>
>>>
>>    case R_RISCV_HI20:>
>> @@ -533,6 +535,18 @@ WriteSectionRiscV64 (
>>      mRiscVPass1SymSecIndex = 0;>
>>      break;>
>>>
>> +  case R_RISCV_GOT_HI20:>
>> +    Value = (Sym->st_value - Rel->r_offset);>
>> +    mRiscVPass1Offset = RV_X(Value, 0, 12);>
>> +    Value = RV_X(Value, 12, 20);>
>> +    *(UINT32 *)Targ = (Value << 12) | (RV_X(*(UINT32*)Targ, 0, 12));>
>> +>
>> +    mRiscVPass1Targ = Targ;>
>> +    mRiscVPass1Sym = SymShdr;>
>> +    mRiscVPass1SymSecIndex = Sym->st_shndx;>
>> +    mRiscVPass1GotFixup = 1;>
>> +    break;>
>> +>
>>    case R_RISCV_PCREL_HI20:>
>>      mRiscVPass1Targ = Targ;>
>>      mRiscVPass1Sym = SymShdr;>
>> @@ -545,11 +559,17 @@ WriteSectionRiscV64 (
>> if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL &&
>> mRiscVPass1SymSecIndex != 0) {>
>>        int i;>
>>        Value2 = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));>
>> -      Value = (UINT32)(RV_X(*(UINT32 *)Targ, 20, 12));>
>> -      if(Value & (RISCV_IMM_REACH/2)) {>
>> -        Value |= ~(RISCV_IMM_REACH-1);>
>> +>
>> +      if(mRiscVPass1GotFixup) {>
>> +        Value = (UINT32)(mRiscVPass1Offset);>
>> +      } else {>
>> +        Value = (UINT32)(RV_X(*(UINT32 *)Targ, 20, 12));>
>> +        if(Value & (RISCV_IMM_REACH/2)) {>
>> +          Value |= ~(RISCV_IMM_REACH-1);>
>> +        }>
>>        }>
>> Value = Value - (UINT32)mRiscVPass1Sym->sh_addr +
>> mCoffSectionsOffset[mRiscVPass1SymSecIndex];>
>> +>
>>        if(-2048 > (INT32)Value) {>
>>          i = (((INT32)Value * -1) / 4096);>
>>          Value2 -= i;>
>> @@ -569,12 +589,22 @@ WriteSectionRiscV64 (
>>          }>
>>        }>
>>>
>> -      *(UINT32 *)Targ = (RV_X(Value, 0, 12) << 20) |
>> (RV_X(*(UINT32*)Targ, 0, 20));>
>> +      if(mRiscVPass1GotFixup) {>
>> +        *(UINT32 *)Targ = (RV_X((UINT32)Value, 0, 12) << 20)>
>> +                            | (RV_X(*(UINT32*)Targ, 0, 20));>
>> +        /* Convert LD instruction to ADDI */>
>> +        *(UINT32 *)Targ = ((*(UINT32 *)Targ & ~0x707f) | 0x13);>
>> +      }>
>> +      else {>
>> +        *(UINT32 *)Targ = (RV_X(Value, 0, 12) << 20) |
>> (RV_X(*(UINT32*)Targ, 0, 20));>
>> +      }>
>> *(UINT32 *)mRiscVPass1Targ = (RV_X(Value2, 0, 20)<<12) | (RV_X(*(UINT32
>> *)mRiscVPass1Targ, 0, 12));>
>>      }>
>>      mRiscVPass1Sym = NULL;>
>>      mRiscVPass1Targ = NULL;>
>>      mRiscVPass1SymSecIndex = 0;>
>> +    mRiscVPass1Offset = 0;>
>> +    mRiscVPass1GotFixup = 0;>
>>      break;>
>>>
>>    case R_RISCV_ADD64:>
>> @@ -586,6 +616,7 @@ WriteSectionRiscV64 (
>>    case R_RISCV_GPREL_I:>
>>    case R_RISCV_GPREL_S:>
>>    case R_RISCV_CALL:>
>> +  case R_RISCV_CALL_PLT:>
>>    case R_RISCV_RVC_BRANCH:>
>>    case R_RISCV_RVC_JUMP:>
>>    case R_RISCV_RELAX:>
>> @@ -1528,6 +1559,7 @@ WriteRelocations64 (
>>              case R_RISCV_GPREL_I:>
>>              case R_RISCV_GPREL_S:>
>>              case R_RISCV_CALL:>
>> +            case R_RISCV_CALL_PLT:>
>>              case R_RISCV_RVC_BRANCH:>
>>              case R_RISCV_RVC_JUMP:>
>>              case R_RISCV_RELAX:>
>> @@ -1537,6 +1569,7 @@ WriteRelocations64 (
>>              case R_RISCV_SET16:>
>>              case R_RISCV_SET32:>
>>              case R_RISCV_PCREL_HI20:>
>> +            case R_RISCV_GOT_HI20:>
>>              case R_RISCV_PCREL_LO12_I:>
>>                break;>
>>>
> 

      reply	other threads:[~2021-05-28  5:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-27 14:41 [PATCH] Add support for RISCV GOT/PLT relocations Sunil V L
2021-05-28  3:24 ` Daniel Schaefer
2021-05-28  3:27   ` Abner Chang
2021-05-28  6:56   ` Sunil V L
2021-05-28  4:42 ` Heinrich Schuchardt
2021-05-28  5:31   ` Daniel Schaefer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=366dedb1-3e32-5949-ac6e-617db72be1d5@hpe.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox