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

Describe the relationship of this repository to mainline U-Boot. #16

Closed
wants to merge 1 commit into from

Conversation

tim-seoss
Copy link

Closes #15

I've made this a PR against this repo, since this patch is by-design not intended for mainline u-boot

@ehristev ehristev requested a review from noglitch December 11, 2020 15:59
Copy link
Member

@noglitch noglitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good addition indeed. @ehristev to decide if he wants to carry an out-of-mainline patch.
Thank you so much for taking the time to contribute!
Best regards,
Nicolas

5. Compile and test your patch set against mainline U-Boot.

6. Submit your patches to the U-Boot project.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ehristev : no need to talk about at91 custodian tree here? It might add more confusion: maybe simpler is better, in this case forget my question ;-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@noglitch what do you mean ? is the at91 custodian mentioned anywhere ? I haven't seen it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommended this particular workflow, because in my case, the mainline u-boot would not boot my board at the time that I tested it (sd card probing bug).

My assumption was that the https://github.com/linux4sam/u-boot-at91 would be the best tree to start with, since it's generally better tested on at91 SoCs (and then forward-port) - but happy to take whatever guidance you prefer.

Similarly the mainline U-Boot tree and the sam4linux tree are currently mentioned in the PR, but maybe it would be better if https://gitlab.denx.de/u-boot/custodians/u-boot-atmel was mentioned (either as-well, or instead-of the mainline tree).

Perhaps this would be a better workflow:

  1. With reference to https://www.linux4sam.org/ compile U-Boot for a supported evaluation board which implements the same MPU that your new design targets.

  2. Test your compiled U-Boot on the evaluation board, as far as possible exercising the same functionality that you expect to use on your own board design.

  3. Repeat steps 1. and 2. with the latest code from the mainline U-Boot tree at https://gitlab.denx.de/u-boot/u-boot

  4. Add support for your new board to a private branch on your copy of the mainline U-Boot source code.

  5. Compile and test your patch set against mainline U-Boot.

  6. Submit your patches for feedback and integration into the U-Boot project.

@ehristev
Copy link

Applied this patch to u-boot-2021.01-at91

@ehristev ehristev closed this Jan 12, 2021
ehristev pushed a commit that referenced this pull request Apr 5, 2021
Atish reports that on RISC-V, accessing the EFI variables causes
a kernel panic. An objdump of the file verifies that, since the
global pointer for efi_var_buf ends up in .GOT section which is
not mapped in virtual address space for Linux.

<snip of efi_var_mem_find>

0000000000000084 <efi_var_mem_find>:
  84:   715d                    addi    sp,sp,-80

* objdump -dr
0000000000000086 <.LCFI2>:
  86:   e0a2                    sd  s0,64(sp)
  88:   fc26                    sd  s1,56(sp)
  8a:   e486                    sd  ra,72(sp)
  8c:   f84a                    sd  s2,48(sp)
  8e:   f44e                    sd  s3,40(sp)
  90:   f052                    sd  s4,32(sp)
  92:   ec56                    sd  s5,24(sp)
  94:   00000497            auipc   s1,0x0
            94: R_RISCV_GOT_HI20    efi_var_buf
  98:   0004b483            ld  s1,0(s1) # 94 <.LCFI2+0xe>
            98: R_RISCV_PCREL_LO12_I    .L0
            98: R_RISCV_RELAX   *ABS*

* objdump -t
0000000000000084 g     F .text.efi_runtime  00000000000000b8 efi_var_mem_find

With the patch applied:

* objdump -dr
0000000000000086 <.LCFI2>:
  86:   e0a2                    sd  s0,64(sp)
  88:   fc26                    sd  s1,56(sp)
  8a:   e486                    sd  ra,72(sp)
  8c:   f84a                    sd  s2,48(sp)
  8e:   f44e                    sd  s3,40(sp)
  90:   f052                    sd  s4,32(sp)
  92:   ec56                    sd  s5,24(sp)
  94:   00000497            auipc   s1,0x0
            94: R_RISCV_PCREL_HI20  .LANCHOR0
            94: R_RISCV_RELAX   *ABS*
  98:   00048493            mv  s1,s1
            98: R_RISCV_PCREL_LO12_I    .L0
            98: R_RISCV_RELAX   *ABS*

* objdump -t
0000000000000008 l     O .data.efi_runtime  0000000000000008 efi_var_buf

On arm64 this works, because there's no .GOT entries for this
and everything is converted to relative references.

* objdump -dr (identical pre-post patch, only the new function shows up)
00000000000000b4 <efi_var_mem_find>:
  b4:   aa0003ee    mov x14, x0
  b8:   9000000a    adrp    x10, 0 <efi_var_mem_compare>
            b8: R_AARCH64_ADR_PREL_PG_HI21  .data.efi_runtime
  bc:   91000140    add x0, x10, #0x0
            bc: R_AARCH64_ADD_ABS_LO12_NC   .data.efi_runtime
  c0:   aa0103ed    mov x13, x1
  c4:   79400021    ldrh    w1, [x1]
  c8:   aa0203eb    mov x11, x2
  cc:   f9400400    ldr x0, [x0, #8]
  d0:   b940100c    ldr w12, [x0, #16]
  d4:   8b0c000c    add x12, x0, x12

So let's switch efi_var_buf to static and create a helper function for
anyone that needs to update it.

Fixes: e01aed4 ("efi_loader: Enable run-time variable support for tee based variables")
Reported-by: Atish Patra <[email protected]>
Tested-by: Atish Patra <[email protected]>
Signed-off-by: Ilias Apalodimas <[email protected]>
Reviewed-by: Heinrich Schuchardt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Relationship with upstream u-boot is not documented
3 participants