Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the compact patterns for crt0.s and entry.s. #417

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Nelson1225
Copy link

@Nelson1225 Nelson1225 commented Jul 15, 2021

Hi Guys,

There are two problems here that cause testcases to runtime fail for freedom-e-sdk,

  • We shouldn't use a0 for compact patterns, since it usually used for function parameters.

1:
auipc a0, %pcrel_hi(global_pointer)
addi a0, a0, %pcrel_lo(1b)
ld gp, 0(a0)
add gp, gp, a0
la gp, __global_pointer$
.option pop

/* Stack pointer is expected to be initialized before _start */

/* If we're not hart 0, skip the initialization work */
la t0, __metal_boot_hart
bne a0, t0, _skip_init

Obviously my code is wrong here, a0 usually used for function parameters, so if I use it to do anything special, then the value of a0 will be changed. Maybe t0/t1 is more suitable.

  • We shouldn't use la.got.gprel before the data movement if LMA != VMA.

When rodata is placed nearly to text, then we can use la to access metal_segment_data_source_start
directly; When the rodata is placed nearly to rwdata, then use the lla.gprel is more suitable. But once the rodata is placed far away from both text and rwdata, generally we should use la.got.gprel to access it. Unfortunately, la.got.gprel cannot work for metal_segment_data_source_start, if LMA and VMA are different in the linker script. Since we need to move the data, including GOT sections, from the LMA to VMA, then we can get the right values of got entries. Therefore, use la.got.gprel to access the metal_segment_data_source_start will get the uninitilaize value.

One of the solution is that - use compact stub to access the metal_segment_data_source_start, just like what we did above, to initialize gp. But we need four instructions and one 8 bytes stub, so the code size will be slightly larger (4 * 4 + 8 = 24 bytes, originally, the la.got.gprel may be relaxed to a 4 bytes ld, so it will be 20 bytes more).

There are two problems here that cause testcases to runtime fail
for freedom-e-sdk,

* Don't use a0 for compact patterns, since it usually used for
  function parameters.

1:
  auipc a0, %pcrel_hi(__global_pointer__)
  addi a0, a0, %pcrel_lo(1b)
  ld   gp, 0(a0)
  add  gp, gp, a0
  la gp, __global_pointer$
.option pop

  /* Stack pointer is expected to be initialized before _start */

  /* If we're not hart 0, skip the initialization work */
  la t0, __metal_boot_hart
  bne a0, t0, _skip_init

Obviously my code is wrong here, a0 usually used for function
parameters, so if I use it to do anything special, then the
value of a0 will be changed.  Maybe t0/t1 is more suitable.

* Don't use la.got.gprel before the data movement if LMA != VMA.

When rodata is placed nearly to text, then we can use la to access
metal_segment_data_source_start directly; When the rodata is placed
nearly to rwdata, then use the lla.gprel is more suitable.  But once
the rodata is placed far away from both text and rwdata, generally we
should use la.got.gprel to access it. Unfortunately, la.got.gprel
cannot work for metal_segment_data_source_start, if LMA and VMA are
different in the linker script.  Since we need to move the data,
including GOT sections, from the LMA to VMA, then we can get the
right values of got entries.  Therefore, use la.got.gprel to access
the metal_segment_data_source_start will get the uninitilaize value.

One of the solution is that - use compact stub to access the
metal_segment_data_source_start, just like what we did above, to
initialize gp.  But we need four instructions and one 8 bytes stub,
so the code size will be slightly larger (4 * 4 + 8 = 24 bytes.
Originally, the la.got.gprel may be relaxed to a 4 bytes ld, so it
may be 12-20 bytes more, according to the linker script).
@e-puerto
Copy link
Contributor

@Nelson1225 I have some difficulties understanding why using a0 for compact patterns will cause runtime fail.

@Nelson1225
Copy link
Author

@Nelson1225 I have some difficulties understanding why using a0 for compact patterns will cause runtime fail.

It is probably fine in entry.s, but in crt0.s,

/* Compact pattern changes the value of a0. */
1:
auipc a0, %pcrel_hi(global_pointer)
addi a0, a0, %pcrel_lo(1b)
ld gp, 0(a0)
add gp, gp, a0
la gp, __global_pointer$

/* If we're not hart 0, skip the initialization work */
la t0, __metal_boot_hart
bne a0, t0, _skip_init

The a0 will be used later in bne, so if we use a0 as a template register for compact pattern, then a0 will be changed, which seems wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants