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 overwriting first byte in THR register during send #8

Closed
wants to merge 1 commit into from
Closed

Fix overwriting first byte in THR register during send #8

wants to merge 1 commit into from

Conversation

pribyltomas
Copy link

During send on TWI the first byte is lost because there is not TWI_SR_TXRDY check and THR register is immediately overwritten.

@ehristev ehristev self-assigned this Aug 3, 2018
@ehristev
Copy link

ehristev commented Aug 3, 2018

commit 0afbb0e
Author: Alan Ott [email protected]
Date: Tue Nov 28 22:25:23 2017 -0500

i2c: at91_i2c: Wait for TXRDY after sending the first byte

The driver must wait for TXRDY after each byte is pushed into
the i2c FIFO before pushing the next byte. Previously this was
not done for the first byte, causing a race condition with zeros
sometimes being sent for the next byte (which is typically the
first actual data byte).

Signed-off-by: Alan Ott <[email protected]>
Reviewed-by: Heiko Schocher <[email protected]>

@ehristev
Copy link

ehristev commented Aug 3, 2018

Is this commit from u-boot mainline identical to your patch ?

@ldesroches
Copy link

I think so

@pribyltomas
Copy link
Author

Yes, it is identical.

@ehristev
Copy link

ehristev commented Aug 3, 2018

Do you need that commit from mainline cherry-picked into our branch? Or any other reason ?
I can pull it from mainline if you require it.
But I cannot merge your request since this change is already present in U-boot mainline under different sign-off.

@ehristev ehristev closed this Aug 8, 2018
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.

3 participants