[0/3] Support position independent execution

This patch set enables OpenSBI to support position-independent execution
so that one OpenSBI generic firmware can run on multiple platforms, even
if they have different memory base addresses. However, the PIC code will
decrease a bit of performance. The 2nd and 3rd patches are used to reduce
this impact of the PIC.

The performance loss of PIC comes from two additional operations. One is
from the symbol relocation in the beginning stage, and another one is
from the GOT reference (auipc + load GOT) in the OpenSBI lifetime. The
former only affects the boot time, but the latter will affect the
performance of the OpenSBI payload such as the Linux kernel. After
analyzing the fw_payload.elf of the generic platform, 27 symbols use the
GOT reference method to get their address. Fortunately, 17 of these
symbols are accessed once during the initialized step. Only the remaining
10 symbols may be called repeatedly during the lifetime, which are
1. hartid_to_domain_table
2. hartid_to_scratch_table
3. sbi_hart_expected_trap
4. sbi_tlb_local_hfence_vvma
5. sbi_tlb_local_hfence_gvma
6. sbi_tlb_local_sfence_vma_asid
7. sbi_tlb_local_fence_i
8. sbi_tlb_local_hfence_gvma
9. sbi_tlb_local_hfence_vvma
10. sbi_tlb_local_sfence_vma
From these 10 symbols, I think the IPI handler may be one of the critical
and commonly used paths affected by the PIC mode. Therefore, I create two
kernel modules, One keeps issuing the REMOTE_FENCE_I request and the other
one keeps issuing REMOTE_SFENCE_VMA request. These two modules are executed
on the unleashed board to measure the consumed time of the sbi_tlb_request().
I found the PIC causes a 1.6% and 4.1% performance loss in addressing
REMOTE_FENCE_I and REMOTE_SFENCE_VMA, respectively. In addition, to observe
the impact of PIC mode in the general usage cases, I use SPEC2006 to
benchmark performance. However, I could not observe any performance impact
from the results. Based on the above two results, I think the performance
impact of the PIC may not be obvious in general usage. Just in case, this
patch still creates an FW_PIC option for users to configure the build options.

Vincent Chen (3):
  firmware: Support position independent execution
  firmware: fw_base: Use lla to access the global symbols defined in
    fw_base.S
  firware: Use lla to access all global symbols

 Makefile                |  10 +++++
 firmware/fw_base.S      | 116 ++++++++++++++++++++++++++++++++++++------------
 firmware/fw_base.ldS    |  13 ++++++
 firmware/fw_dynamic.S   |  18 ++++----
 firmware/fw_jump.S      |  12 ++++-
 firmware/fw_payload.S   |   8 +++-
 include/sbi/riscv_elf.h |   9 ++++
 7 files changed, 147 insertions(+), 39 deletions(-)
 create mode 100644 include/sbi/riscv_elf.h

Hi Vincent,

Overall the changes are in quite good shape.

I have few high-level comments before I can review fw_base.S in detail:
1) No need to change toplevel Makefile for appending compiler and linker
    options. It is better to update $(firmware-genflags-y), $(firmware-cflags-y)
    and $(firmware-ldflags-y) in firmware/objects.mk
2) I would suggest to squash PATCH2 and PATCH3 into one PATCH and
    make this PATCH as PATCH1
3) The current PATCH1 can be PATCH2

> -----Original Message-----
> From: opensbi <opensbi-bounces@lists.infradead.org> On Behalf Of Vincent
> Chen
> Sent: 02 March 2021 20:56
> To: opensbi@lists.infradead.org
> Cc: vincent.chen@sifive.com
> Subject: [PATCH 0/3] Support position independent execution
> 
> This patch set enables OpenSBI to support position-independent execution
> so that one OpenSBI generic firmware can run on multiple platforms, even if
> they have different memory base addresses. However, the PIC code will
> decrease a bit of performance. The 2nd and 3rd patches are used to reduce
> this impact of the PIC.
> 
> The performance loss of PIC comes from two additional operations. One is
> from the symbol relocation in the beginning stage, and another one is from
> the GOT reference (auipc + load GOT) in the OpenSBI lifetime. The former
> only affects the boot time, but the latter will affect the performance of the
> OpenSBI payload such as the Linux kernel. After analyzing the fw_payload.elf
> of the generic platform, 27 symbols use the GOT reference method to get
> their address. Fortunately, 17 of these symbols are accessed once during the
> initialized step. Only the remaining
> 10 symbols may be called repeatedly during the lifetime, which are 1.
> hartid_to_domain_table 2. hartid_to_scratch_table 3.
> sbi_hart_expected_trap 4. sbi_tlb_local_hfence_vvma 5.
> sbi_tlb_local_hfence_gvma 6. sbi_tlb_local_sfence_vma_asid 7.
> sbi_tlb_local_fence_i 8. sbi_tlb_local_hfence_gvma 9.
> sbi_tlb_local_hfence_vvma 10. sbi_tlb_local_sfence_vma From these 10
> symbols, I think the IPI handler may be one of the critical and commonly used
> paths affected by the PIC mode. Therefore, I create two kernel modules,
> One keeps issuing the REMOTE_FENCE_I request and the other one keeps
> issuing REMOTE_SFENCE_VMA request. These two modules are executed on
> the unleashed board to measure the consumed time of the
> sbi_tlb_request().
> I found the PIC causes a 1.6% and 4.1% performance loss in addressing
> REMOTE_FENCE_I and REMOTE_SFENCE_VMA, respectively. In addition, to
> observe the impact of PIC mode in the general usage cases, I use SPEC2006
> to benchmark performance. However, I could not observe any performance
> impact from the results. Based on the above two results, I think the
> performance impact of the PIC may not be obvious in general usage. Just in
> case, this patch still creates an FW_PIC option for users to configure the build
> options.
> 
> Vincent Chen (3):
>   firmware: Support position independent execution
>   firmware: fw_base: Use lla to access the global symbols defined in
>     fw_base.S
>   firware: Use lla to access all global symbols
> 
>  Makefile                |  10 +++++
>  firmware/fw_base.S      | 116
> ++++++++++++++++++++++++++++++++++++------------
>  firmware/fw_base.ldS    |  13 ++++++
>  firmware/fw_dynamic.S   |  18 ++++----
>  firmware/fw_jump.S      |  12 ++++-
>  firmware/fw_payload.S   |   8 +++-
>  include/sbi/riscv_elf.h |   9 ++++
>  7 files changed, 147 insertions(+), 39 deletions(-)  create mode 100644
> include/sbi/riscv_elf.h
> 
> --
> 2.7.4
> 
> 
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Regards,
Anup

On Thu, Mar 4, 2021 at 3:33 PM Anup Patel <Anup.Patel@wdc.com> wrote:
>
> Hi Vincent,
>
> Overall the changes are in quite good shape.
>
> I have few high-level comments before I can review fw_base.S in detail:
> 1) No need to change toplevel Makefile for appending compiler and linker
>     options. It is better to update $(firmware-genflags-y), $(firmware-cflags-y)
>     and $(firmware-ldflags-y) in firmware/objects.mk
> 2) I would suggest to squash PATCH2 and PATCH3 into one PATCH and
>     make this PATCH as PATCH1
> 3) The current PATCH1 can be PATCH2
>
OK, I got it. I will follow your suggestions to modify this patch.
Thank you.