[PATCH 0/3] Support position independent execution

[Anup Patel]

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 at lists.infradead.org> On Behalf Of Vincent
> Chen
> Sent: 02 March 2021 20:56
> To: opensbi at lists.infradead.org
> Cc: vincent.chen at 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Regards,
Anup