* Re: [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation
2021-07-10 6:31 [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation Sunil V L
@ 2021-07-12 11:51 ` Pete Batard
2021-07-12 13:39 ` 回复: " gaoliming
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Pete Batard @ 2021-07-12 11:51 UTC (permalink / raw)
To: Sunil V L, devel
Cc: sunil.vl, Liming Gao, Bob Feng, Yuwei Chen, Abner Chang,
Daniel Schaefer
Hi Sunil,
Thanks a lot for this patch.
I confirm that it fixes the issue I raised in BZ3459.
If anyone is interested, you can find builds of the RISC-V NTFS driver,
where this patch has been applied, in the artefacts of
https://github.com/pbatard/ntfs-3g/actions/runs/1022633741.
With this:
On 2021.07.10 07:31, Sunil V L wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3459
>
> This patch adds support for R_RISCV_PCREL_LO12_S relocation type.
> The logic is same as existing R_RISCV_PCREL_LO12_I relocation
> except the difference between load vs store instruction formats.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> ---
> BaseTools/Source/C/GenFw/Elf64Convert.c | 55 +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 3d7e20aaff..0bb3ead228 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -557,6 +557,60 @@ WriteSectionRiscV64 (
> Value = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
>
> break;
>
>
>
> + case R_RISCV_PCREL_LO12_S:
>
> + if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL && mRiscVPass1SymSecIndex != 0) {
>
> + int i;
>
> + Value2 = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
>
> +
>
> + Value = ((UINT32)(RV_X(*(UINT32 *)Targ, 25, 7)) << 5);
>
> + Value = (Value | (UINT32)(RV_X(*(UINT32 *)Targ, 7, 5)));
>
> +
>
> + 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;
>
> + Value += 4096 * i;
>
> + if(-2048 > (INT32)Value) {
>
> + Value2 -= 1;
>
> + Value += 4096;
>
> + }
>
> + }
>
> + else if( 2047 < (INT32)Value) {
>
> + i = (Value / 4096);
>
> + Value2 += i;
>
> + Value -= 4096 * i;
>
> + if(2047 < (INT32)Value) {
>
> + Value2 += 1;
>
> + Value -= 4096;
>
> + }
>
> + }
>
> +
>
> + // Update the IMM of SD instruction
>
> + //
>
> + // |31 25|24 20|19 15|14 12 |11 7|6 0|
>
> + // |-------------------------------------------|-------|
>
> + // |imm[11:5] | rs2 | rs1 | funct3 |imm[4:0] | opcode|
>
> + // ---------------------------------------------------
>
> +
>
> + // First Zero out current IMM
>
> + *(UINT32 *)Targ &= ~0xfe000f80;
>
> +
>
> + // Update with new IMM
>
> + *(UINT32 *)Targ |= (RV_X(Value, 5, 7) << 25);
>
> + *(UINT32 *)Targ |= (RV_X(Value, 0, 5) << 7);
>
> +
>
> + // Update previous instruction
>
> + *(UINT32 *)mRiscVPass1Targ = (RV_X(Value2, 0, 20)<<12) | (RV_X(*(UINT32 *)mRiscVPass1Targ, 0, 12));
>
> + }
>
> + mRiscVPass1Sym = NULL;
>
> + mRiscVPass1Targ = NULL;
>
> + mRiscVPass1SymSecIndex = 0;
>
> + break;
>
> +
>
> case R_RISCV_PCREL_LO12_I:
>
> if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL && mRiscVPass1SymSecIndex != 0) {
>
> int i;
>
> @@ -1587,6 +1641,7 @@ WriteRelocations64 (
> case R_RISCV_PCREL_HI20:
>
> case R_RISCV_GOT_HI20:
>
> case R_RISCV_PCREL_LO12_I:
>
> + case R_RISCV_PCREL_LO12_S:
>
> break;
>
>
>
> default:
>
Tested-by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 7+ messages in thread* 回复: [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation
2021-07-10 6:31 [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation Sunil V L
2021-07-12 11:51 ` Pete Batard
@ 2021-07-12 13:39 ` gaoliming
2021-07-13 2:44 ` Abner Chang
2021-07-13 9:27 ` Daniel Schaefer
3 siblings, 0 replies; 7+ messages in thread
From: gaoliming @ 2021-07-12 13:39 UTC (permalink / raw)
To: 'Sunil V L', devel
Cc: sunil.vl, 'Bob Feng', 'Yuwei Chen',
'Pete Batard', 'Abner Chang',
'Daniel Schaefer'
Acked-by: Liming Gao <gaoliming@byosoft.com.cn>
> -----邮件原件-----
> 发件人: Sunil V L <sunilvl@ventanamicro.com>
> 发送时间: 2021年7月10日 14:31
> 收件人: devel@edk2.groups.io
> 抄送: sunil.vl@gmail.com; Sunil V L <sunilvl@ventanamicro.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Bob Feng <bob.c.feng@intel.com>; Yuwei Chen
> <yuwei.chen@intel.com>; Pete Batard <pete@akeo.ie>; Abner Chang
> <abner.chang@hpe.com>; Daniel Schaefer <daniel.schaefer@hpe.com>
> 主题: [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S
> relocation
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3459
>
> This patch adds support for R_RISCV_PCREL_LO12_S relocation type.
> The logic is same as existing R_RISCV_PCREL_LO12_I relocation
> except the difference between load vs store instruction formats.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> ---
> BaseTools/Source/C/GenFw/Elf64Convert.c | 55
> +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 3d7e20aaff..0bb3ead228 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -557,6 +557,60 @@ WriteSectionRiscV64 (
> Value = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
>
> break;
>
>
>
> + case R_RISCV_PCREL_LO12_S:
>
> + if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL &&
> mRiscVPass1SymSecIndex != 0) {
>
> + int i;
>
> + Value2 = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
>
> +
>
> + Value = ((UINT32)(RV_X(*(UINT32 *)Targ, 25, 7)) << 5);
>
> + Value = (Value | (UINT32)(RV_X(*(UINT32 *)Targ, 7, 5)));
>
> +
>
> + 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;
>
> + Value += 4096 * i;
>
> + if(-2048 > (INT32)Value) {
>
> + Value2 -= 1;
>
> + Value += 4096;
>
> + }
>
> + }
>
> + else if( 2047 < (INT32)Value) {
>
> + i = (Value / 4096);
>
> + Value2 += i;
>
> + Value -= 4096 * i;
>
> + if(2047 < (INT32)Value) {
>
> + Value2 += 1;
>
> + Value -= 4096;
>
> + }
>
> + }
>
> +
>
> + // Update the IMM of SD instruction
>
> + //
>
> + // |31 25|24 20|19 15|14 12 |11 7|6 0|
>
> + // |-------------------------------------------|-------|
>
> + // |imm[11:5] | rs2 | rs1 | funct3 |imm[4:0] | opcode|
>
> + // ---------------------------------------------------
>
> +
>
> + // First Zero out current IMM
>
> + *(UINT32 *)Targ &= ~0xfe000f80;
>
> +
>
> + // Update with new IMM
>
> + *(UINT32 *)Targ |= (RV_X(Value, 5, 7) << 25);
>
> + *(UINT32 *)Targ |= (RV_X(Value, 0, 5) << 7);
>
> +
>
> + // Update previous instruction
>
> + *(UINT32 *)mRiscVPass1Targ = (RV_X(Value2, 0, 20)<<12) |
> (RV_X(*(UINT32 *)mRiscVPass1Targ, 0, 12));
>
> + }
>
> + mRiscVPass1Sym = NULL;
>
> + mRiscVPass1Targ = NULL;
>
> + mRiscVPass1SymSecIndex = 0;
>
> + break;
>
> +
>
> case R_RISCV_PCREL_LO12_I:
>
> if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL &&
> mRiscVPass1SymSecIndex != 0) {
>
> int i;
>
> @@ -1587,6 +1641,7 @@ WriteRelocations64 (
> case R_RISCV_PCREL_HI20:
>
> case R_RISCV_GOT_HI20:
>
> case R_RISCV_PCREL_LO12_I:
>
> + case R_RISCV_PCREL_LO12_S:
>
> break;
>
>
>
> default:
>
> --
> 2.32.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation
2021-07-10 6:31 [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation Sunil V L
2021-07-12 11:51 ` Pete Batard
2021-07-12 13:39 ` 回复: " gaoliming
@ 2021-07-13 2:44 ` Abner Chang
2021-07-13 9:27 ` Daniel Schaefer
3 siblings, 0 replies; 7+ messages in thread
From: Abner Chang @ 2021-07-13 2:44 UTC (permalink / raw)
To: Sunil V L, devel@edk2.groups.io
Cc: sunil.vl@gmail.com, Liming Gao, Bob Feng, Yuwei Chen, Pete Batard,
Schaefer, Daniel
Acked-by: Abner Chang <abner.chang@hpe.com>
> -----Original Message-----
> From: Sunil V L [mailto:sunilvl@ventanamicro.com]
> Sent: Saturday, July 10, 2021 2:31 PM
> To: devel@edk2.groups.io
> Cc: sunil.vl@gmail.com; Sunil V L <sunilvl@ventanamicro.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Bob Feng <bob.c.feng@intel.com>; Yuwei
> Chen <yuwei.chen@intel.com>; Pete Batard <pete@akeo.ie>; Chang, Abner
> (HPS SW/FW Technologist) <abner.chang@hpe.com>; Schaefer, Daniel
> <daniel.schaefer@hpe.com>
> Subject: [PATCH] BaseTools GenFw: Add support for
> R_RISCV_PCREL_LO12_S relocation
>
> Ref:
> INVALID URI REMOVED
> d=3459__;!!NpxR!zrGTHXfzHm6WE6VZ2rABQ0yFTgu7frG5J213efKk-
> TmOGcp_9TSXv8E3b_XlNzo$
>
> This patch adds support for R_RISCV_PCREL_LO12_S relocation type.
> The logic is same as existing R_RISCV_PCREL_LO12_I relocation
> except the difference between load vs store instruction formats.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> ---
> BaseTools/Source/C/GenFw/Elf64Convert.c | 55
> +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 3d7e20aaff..0bb3ead228 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -557,6 +557,60 @@ WriteSectionRiscV64 (
> Value = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> break;
>
> + case R_RISCV_PCREL_LO12_S:
> + if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL &&
> mRiscVPass1SymSecIndex != 0) {
> + int i;
> + Value2 = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> +
> + Value = ((UINT32)(RV_X(*(UINT32 *)Targ, 25, 7)) << 5);
> + Value = (Value | (UINT32)(RV_X(*(UINT32 *)Targ, 7, 5)));
> +
> + 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;
> + Value += 4096 * i;
> + if(-2048 > (INT32)Value) {
> + Value2 -= 1;
> + Value += 4096;
> + }
> + }
> + else if( 2047 < (INT32)Value) {
> + i = (Value / 4096);
> + Value2 += i;
> + Value -= 4096 * i;
> + if(2047 < (INT32)Value) {
> + Value2 += 1;
> + Value -= 4096;
> + }
> + }
> +
> + // Update the IMM of SD instruction
> + //
> + // |31 25|24 20|19 15|14 12 |11 7|6 0|
> + // |-------------------------------------------|-------|
> + // |imm[11:5] | rs2 | rs1 | funct3 |imm[4:0] | opcode|
> + // ---------------------------------------------------
> +
> + // First Zero out current IMM
> + *(UINT32 *)Targ &= ~0xfe000f80;
> +
> + // Update with new IMM
> + *(UINT32 *)Targ |= (RV_X(Value, 5, 7) << 25);
> + *(UINT32 *)Targ |= (RV_X(Value, 0, 5) << 7);
> +
> + // Update previous instruction
> + *(UINT32 *)mRiscVPass1Targ = (RV_X(Value2, 0, 20)<<12) |
> (RV_X(*(UINT32 *)mRiscVPass1Targ, 0, 12));
> + }
> + mRiscVPass1Sym = NULL;
> + mRiscVPass1Targ = NULL;
> + mRiscVPass1SymSecIndex = 0;
> + break;
> +
> case R_RISCV_PCREL_LO12_I:
> if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL &&
> mRiscVPass1SymSecIndex != 0) {
> int i;
> @@ -1587,6 +1641,7 @@ WriteRelocations64 (
> case R_RISCV_PCREL_HI20:
> case R_RISCV_GOT_HI20:
> case R_RISCV_PCREL_LO12_I:
> + case R_RISCV_PCREL_LO12_S:
> break;
>
> default:
> --
> 2.32.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation
2021-07-10 6:31 [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation Sunil V L
` (2 preceding siblings ...)
2021-07-13 2:44 ` Abner Chang
@ 2021-07-13 9:27 ` Daniel Schaefer
2021-07-13 10:14 ` Sunil V L
3 siblings, 1 reply; 7+ messages in thread
From: Daniel Schaefer @ 2021-07-13 9:27 UTC (permalink / raw)
To: Sunil V L, devel
Cc: sunil.vl, Liming Gao, Bob Feng, Yuwei Chen, Pete Batard,
Abner Chang
Looks good. I compared it with existing R_RISCV_PCREL_LO12_I and looked
at the differences.
This one doesn't do use mRiscVPass1GotFixup.
I assume this is an optimization that's not possible here?
Haven't tested that it works but since it works for Pete:
Reviewed-by: Daniel Schaefer <daniel.schaefer@hpe.com>
Thanks!
On 7/10/21 2:31 PM, Sunil V L wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3459
>
> This patch adds support for R_RISCV_PCREL_LO12_S relocation type.
> The logic is same as existing R_RISCV_PCREL_LO12_I relocation
> except the difference between load vs store instruction formats.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> ---
> BaseTools/Source/C/GenFw/Elf64Convert.c | 55 +++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 3d7e20aaff..0bb3ead228 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -557,6 +557,60 @@ WriteSectionRiscV64 (
> Value = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> break;
>
> + case R_RISCV_PCREL_LO12_S:
> + if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL && mRiscVPass1SymSecIndex != 0) {
> + int i;
> + Value2 = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> +
> + Value = ((UINT32)(RV_X(*(UINT32 *)Targ, 25, 7)) << 5);
> + Value = (Value | (UINT32)(RV_X(*(UINT32 *)Targ, 7, 5)));
> +
> + 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;
> + Value += 4096 * i;
> + if(-2048 > (INT32)Value) {
> + Value2 -= 1;
> + Value += 4096;
> + }
> + }
> + else if( 2047 < (INT32)Value) {
> + i = (Value / 4096);
> + Value2 += i;
> + Value -= 4096 * i;
> + if(2047 < (INT32)Value) {
> + Value2 += 1;
> + Value -= 4096;
> + }
> + }
> +
> + // Update the IMM of SD instruction
> + //
> + // |31 25|24 20|19 15|14 12 |11 7|6 0|
> + // |-------------------------------------------|-------|
> + // |imm[11:5] | rs2 | rs1 | funct3 |imm[4:0] | opcode|
> + // ---------------------------------------------------
> +
> + // First Zero out current IMM
> + *(UINT32 *)Targ &= ~0xfe000f80;
> +
> + // Update with new IMM
> + *(UINT32 *)Targ |= (RV_X(Value, 5, 7) << 25);
> + *(UINT32 *)Targ |= (RV_X(Value, 0, 5) << 7);
> +
> + // Update previous instruction
> + *(UINT32 *)mRiscVPass1Targ = (RV_X(Value2, 0, 20)<<12) | (RV_X(*(UINT32 *)mRiscVPass1Targ, 0, 12));
> + }
> + mRiscVPass1Sym = NULL;
> + mRiscVPass1Targ = NULL;
> + mRiscVPass1SymSecIndex = 0;
> + break;
> +
> case R_RISCV_PCREL_LO12_I:
> if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL && mRiscVPass1SymSecIndex != 0) {
> int i;
> @@ -1587,6 +1641,7 @@ WriteRelocations64 (
> case R_RISCV_PCREL_HI20:
> case R_RISCV_GOT_HI20:
> case R_RISCV_PCREL_LO12_I:
> + case R_RISCV_PCREL_LO12_S:
> break;
>
> default:
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation
2021-07-13 9:27 ` Daniel Schaefer
@ 2021-07-13 10:14 ` Sunil V L
2021-07-20 5:48 ` 回复: " gaoliming
0 siblings, 1 reply; 7+ messages in thread
From: Sunil V L @ 2021-07-13 10:14 UTC (permalink / raw)
To: Daniel Schaefer
Cc: devel, sunil.vl, Liming Gao, Bob Feng, Yuwei Chen, Pete Batard,
Abner Chang
On Tue, Jul 13, 2021 at 05:27:30PM +0800, Daniel Schaefer wrote:
> Looks good. I compared it with existing R_RISCV_PCREL_LO12_I and looked at
> the differences.
>
Thanks Daniel.
> This one doesn't do use mRiscVPass1GotFixup.
> I assume this is an optimization that's not possible here?
GOT fixup is required only for load to avoid the indirection for symbol
resolution.
Thanks
Sunil
>
> Haven't tested that it works but since it works for Pete:
>
> Reviewed-by: Daniel Schaefer <daniel.schaefer@hpe.com>
>
> Thanks!
>
> On 7/10/21 2:31 PM, Sunil V L wrote:
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3459
> >
> > This patch adds support for R_RISCV_PCREL_LO12_S relocation type.
> > The logic is same as existing R_RISCV_PCREL_LO12_I relocation
> > except the difference between load vs store instruction formats.
> >
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > Cc: Yuwei Chen <yuwei.chen@intel.com>
> > Cc: Pete Batard <pete@akeo.ie>
> > Cc: Abner Chang <abner.chang@hpe.com>
> > Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> > ---
> > BaseTools/Source/C/GenFw/Elf64Convert.c | 55 +++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > index 3d7e20aaff..0bb3ead228 100644
> > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > @@ -557,6 +557,60 @@ WriteSectionRiscV64 (
> > Value = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> > break;
> > + case R_RISCV_PCREL_LO12_S:
> > + if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL && mRiscVPass1SymSecIndex != 0) {
> > + int i;
> > + Value2 = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> > +
> > + Value = ((UINT32)(RV_X(*(UINT32 *)Targ, 25, 7)) << 5);
> > + Value = (Value | (UINT32)(RV_X(*(UINT32 *)Targ, 7, 5)));
> > +
> > + 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;
> > + Value += 4096 * i;
> > + if(-2048 > (INT32)Value) {
> > + Value2 -= 1;
> > + Value += 4096;
> > + }
> > + }
> > + else if( 2047 < (INT32)Value) {
> > + i = (Value / 4096);
> > + Value2 += i;
> > + Value -= 4096 * i;
> > + if(2047 < (INT32)Value) {
> > + Value2 += 1;
> > + Value -= 4096;
> > + }
> > + }
> > +
> > + // Update the IMM of SD instruction
> > + //
> > + // |31 25|24 20|19 15|14 12 |11 7|6 0|
> > + // |-------------------------------------------|-------|
> > + // |imm[11:5] | rs2 | rs1 | funct3 |imm[4:0] | opcode|
> > + // ---------------------------------------------------
> > +
> > + // First Zero out current IMM
> > + *(UINT32 *)Targ &= ~0xfe000f80;
> > +
> > + // Update with new IMM
> > + *(UINT32 *)Targ |= (RV_X(Value, 5, 7) << 25);
> > + *(UINT32 *)Targ |= (RV_X(Value, 0, 5) << 7);
> > +
> > + // Update previous instruction
> > + *(UINT32 *)mRiscVPass1Targ = (RV_X(Value2, 0, 20)<<12) | (RV_X(*(UINT32 *)mRiscVPass1Targ, 0, 12));
> > + }
> > + mRiscVPass1Sym = NULL;
> > + mRiscVPass1Targ = NULL;
> > + mRiscVPass1SymSecIndex = 0;
> > + break;
> > +
> > case R_RISCV_PCREL_LO12_I:
> > if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL && mRiscVPass1SymSecIndex != 0) {
> > int i;
> > @@ -1587,6 +1641,7 @@ WriteRelocations64 (
> > case R_RISCV_PCREL_HI20:
> > case R_RISCV_GOT_HI20:
> > case R_RISCV_PCREL_LO12_I:
> > + case R_RISCV_PCREL_LO12_S:
> > break;
> > default:
> >
^ permalink raw reply [flat|nested] 7+ messages in thread* 回复: [PATCH] BaseTools GenFw: Add support for R_RISCV_PCREL_LO12_S relocation
2021-07-13 10:14 ` Sunil V L
@ 2021-07-20 5:48 ` gaoliming
0 siblings, 0 replies; 7+ messages in thread
From: gaoliming @ 2021-07-20 5:48 UTC (permalink / raw)
To: 'Sunil V L', 'Daniel Schaefer'
Cc: devel, sunil.vl, 'Bob Feng', 'Yuwei Chen',
'Pete Batard', 'Abner Chang'
Acked-by: Liming Gao <gaoliming@byosoft.com.cn>
> -----邮件原件-----
> 发件人: Sunil V L <sunilvl@ventanamicro.com>
> 发送时间: 2021年7月13日 18:14
> 收件人: Daniel Schaefer <daniel.schaefer@hpe.com>
> 抄送: devel@edk2.groups.io; sunil.vl@gmail.com; Liming Gao
> <gaoliming@byosoft.com.cn>; Bob Feng <bob.c.feng@intel.com>; Yuwei Chen
> <yuwei.chen@intel.com>; Pete Batard <pete@akeo.ie>; Abner Chang
> <abner.chang@hpe.com>
> 主题: Re: [PATCH] BaseTools GenFw: Add support for
> R_RISCV_PCREL_LO12_S relocation
>
> On Tue, Jul 13, 2021 at 05:27:30PM +0800, Daniel Schaefer wrote:
> > Looks good. I compared it with existing R_RISCV_PCREL_LO12_I and looked
> at
> > the differences.
> >
> Thanks Daniel.
>
> > This one doesn't do use mRiscVPass1GotFixup.
> > I assume this is an optimization that's not possible here?
>
> GOT fixup is required only for load to avoid the indirection for symbol
> resolution.
>
> Thanks
> Sunil
> >
> > Haven't tested that it works but since it works for Pete:
> >
> > Reviewed-by: Daniel Schaefer <daniel.schaefer@hpe.com>
> >
> > Thanks!
> >
> > On 7/10/21 2:31 PM, Sunil V L wrote:
> > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3459
> > >
> > > This patch adds support for R_RISCV_PCREL_LO12_S relocation type.
> > > The logic is same as existing R_RISCV_PCREL_LO12_I relocation
> > > except the difference between load vs store instruction formats.
> > >
> > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > >
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Bob Feng <bob.c.feng@intel.com>
> > > Cc: Yuwei Chen <yuwei.chen@intel.com>
> > > Cc: Pete Batard <pete@akeo.ie>
> > > Cc: Abner Chang <abner.chang@hpe.com>
> > > Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> > > ---
> > > BaseTools/Source/C/GenFw/Elf64Convert.c | 55
> +++++++++++++++++++++++++
> > > 1 file changed, 55 insertions(+)
> > >
> > > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > index 3d7e20aaff..0bb3ead228 100644
> > > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> > > @@ -557,6 +557,60 @@ WriteSectionRiscV64 (
> > > Value = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> > > break;
> > > + case R_RISCV_PCREL_LO12_S:
> > > + if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL &&
> mRiscVPass1SymSecIndex != 0) {
> > > + int i;
> > > + Value2 = (UINT32)(RV_X(*(UINT32 *)mRiscVPass1Targ, 12, 20));
> > > +
> > > + Value = ((UINT32)(RV_X(*(UINT32 *)Targ, 25, 7)) << 5);
> > > + Value = (Value | (UINT32)(RV_X(*(UINT32 *)Targ, 7, 5)));
> > > +
> > > + 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;
> > > + Value += 4096 * i;
> > > + if(-2048 > (INT32)Value) {
> > > + Value2 -= 1;
> > > + Value += 4096;
> > > + }
> > > + }
> > > + else if( 2047 < (INT32)Value) {
> > > + i = (Value / 4096);
> > > + Value2 += i;
> > > + Value -= 4096 * i;
> > > + if(2047 < (INT32)Value) {
> > > + Value2 += 1;
> > > + Value -= 4096;
> > > + }
> > > + }
> > > +
> > > + // Update the IMM of SD instruction
> > > + //
> > > + // |31 25|24 20|19 15|14 12 |11 7|6
> 0|
> > > + // |-------------------------------------------|-------|
> > > + // |imm[11:5] | rs2 | rs1 | funct3 |imm[4:0] | opcode|
> > > + // ---------------------------------------------------
> > > +
> > > + // First Zero out current IMM
> > > + *(UINT32 *)Targ &= ~0xfe000f80;
> > > +
> > > + // Update with new IMM
> > > + *(UINT32 *)Targ |= (RV_X(Value, 5, 7) << 25);
> > > + *(UINT32 *)Targ |= (RV_X(Value, 0, 5) << 7);
> > > +
> > > + // Update previous instruction
> > > + *(UINT32 *)mRiscVPass1Targ = (RV_X(Value2, 0, 20)<<12) |
> (RV_X(*(UINT32 *)mRiscVPass1Targ, 0, 12));
> > > + }
> > > + mRiscVPass1Sym = NULL;
> > > + mRiscVPass1Targ = NULL;
> > > + mRiscVPass1SymSecIndex = 0;
> > > + break;
> > > +
> > > case R_RISCV_PCREL_LO12_I:
> > > if (mRiscVPass1Targ != NULL && mRiscVPass1Sym != NULL &&
> mRiscVPass1SymSecIndex != 0) {
> > > int i;
> > > @@ -1587,6 +1641,7 @@ WriteRelocations64 (
> > > case R_RISCV_PCREL_HI20:
> > > case R_RISCV_GOT_HI20:
> > > case R_RISCV_PCREL_LO12_I:
> > > + case R_RISCV_PCREL_LO12_S:
> > > break;
> > > default:
> > >
^ permalink raw reply [flat|nested] 7+ messages in thread