Re: [PATCH 0/2] Reading size-cells and address-cells from a node should walk up the

[Robin Randhawa]

Hi Matthias.

Thanks for this. 

I can confirm that this fixes the reported problem. You have my Tested-By.
FWIW, the changes look fine to me too.

Cheers,
Robin

Re: Bug: qemu_arm64: Cannot access the second flash bank

[Robin Randhawa]

On Thu, 2020-01-09 at 15:57 +0100, Matthias Brugger wrote:

[...]

> The property expects size-cells to be two, but U-Boot will use one
> cell if no
> size-cells are defined in the device node (which is not the case) and
> therefor
> will see
> 
> Bank1: Flashbase 0x0 0x0 Flashsize 0x400
> Bank2: Flashbase 0x400 0x0   Flashsize 0x400

My knowledge of DT is superficial. However, looking at the following
lines from the spec:

- A |spec|-compliant boot program shall supply #address-cells and
#size-cells on all nodes that have children.

- If missing, a client program should assume a default value of 2 for
#address-cells, and a value of 1 for #size-cells.

.. and contrasting with the root node and device node in question from
the DTS for this platform:

/ {
interrupt-parent = <0x8001>;
#size-cells = <0x2>;
#address-cells = <0x2>;
compatible = "linux,dummy-virt";
.
.

flash@0 {
bank-width = <0x4>;
reg = <0x0 0x0 0x0 0x400 0x0 0x400 0x0 0x400>;
compatible = "cfi-flash";
};

.. it seems to me that while the flash node is missing #size-cells,
given that #size-cells _is_ defined in the parent node ("the node that
has children") then that value (0x2) is the one u-boot should have used
but didn't.

Maybe the u-boot DT interpreting logic needs to check if the parent
node also does not specify #size-cells before making the assumption
that the value 1 is to be used ?

Thanks,
Robin

Re: Bug: qemu_arm64: Cannot access the second flash bank

[Robin Randhawa]

Hi Matthias.

On Thu, 2020-01-09 at 12:12 +0100, Matthias Brugger wrote:

[...]

> Can you pinpoint me to where I can find the DTS used by U-boot.

As per my understanding the DTB for this virtual platform is generated
by qemu and handed to u-boot.

I dumped the DTB to the host filesystem using GDB and 'dump mem' then
dtc'd it to get the DTS. 

It's at:
https://pastebin.ubuntu.com/p/2KzPKdxddx/


> I tried to run that myself but wasn't able to see any output. Which
> U-Boot
> config do you use? rpi_3_defconfig?

I hit this problem for the qemu_arm64 u-boot platform target for which
I used qemu_arm64_defconfig [1].

Let me know if you need more info.

Cheers,
Robin

[1]: https://github.com/u-boot/u-boot/blob/master/configs/qemu_arm64_defconfig

Bug: qemu_arm64: Cannot access the second flash bank

[Robin Randhawa]

Hi folks.

[CC'ing some hopefully relevant folks].

As of:

commit 0ba41ce1b7816c229cc19e0621148b98f990cb68
libfdt: return correct value if #size-cells property is not present

.. accesses to the second flash bank on the qemu_arm64 virtual board
appear broken.

To demonstrate, consider that the physical memory map for the 2 flash
banks is:

Bank 1: 0x_ - 0x03FC_
Bank 2: 0x0400_ - 0x7FC0_

Now, consider the abbreviated output of the flinfo command pre and post
the above commit:

Pre:
===

=> flinfo

Bank # 1: CFI conformant flash (32 x 16)  Size: 64 MB in 256 Sectors
  Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018
  Erase timeout: 16384 ms, write timeout: 3 ms
  Buffer write timeout: 3 ms, buffer size: 2048 bytes

  Sector Start Addresses:
     RO   0004   RO   0008   RO   000C0010
  00140018001C00200024
  .
  .
  03E803EC03F003F403F8
  03FC

Bank # 2: CFI conformant flash (32 x 16)  Size: 64 MB in 256 Sectors
  Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018
  Erase timeout: 16384 ms, write timeout: 3 ms
  Buffer write timeout: 3 ms, buffer size: 2048 bytes

  Sector Start Addresses:
  0400   RO   04040408040C0410
  04140418041C04200424
  .
  .
  07E807EC07F007F407F8
  07FC

Post:


=> flinfo

Bank # 1: CFI conformant flash (32 x 16)  Size: 64 MB in 256 Sectors
  Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018
  Erase timeout: 16384 ms, write timeout: 3 ms
  Buffer write timeout: 3 ms, buffer size: 2048 bytes

  Sector Start Addresses:
     RO   0004   RO   0008   RO   000C0010
  00140018001C00200024
  .
  .
  03E803EC03F003F403F8
  03FC

Bank # 2: CFI conformant flash (32 x 16)  Size: 64 MB in 256 Sectors
  Intel Extended command set, Manufacturer ID: 0x89, Device ID: 0x0018
  Erase timeout: 16384 ms, write timeout: 3 ms
  Buffer write timeout: 3 ms, buffer size: 2048 bytes

  Sector Start Addresses:
  400404408
40C410
  41441841C
420424
  .
  .
 
40003E840003EC40003F040
003F440003F8
  40003FC

As a result, the second bank is unusable for environment stores
(CONFIG_ENV_ADDR is 0x400):

=> saveenv
Saving Environment to Flash... Error: start and/or end address not on
sector boundary
Error: start and/or end address not on sector boundary
Failed (1)

Rewinding the u-boot repo to before this commit fixes the problem.

Manually (uncleanly) reverting the commit and it's dependent commits
fixes the problem.

Here are the HEAD commits from the relevant repos that I used for the data 
above:

qemu: commit dd5b0f95490883cd8bc7d070db8de70d5c979cbc
u-boot: commit 6cb87cbb1475f668689f95911d1521ee6ba7f55c

Here is the qemu invocation I used:

$ dd if=/dev/zero of=./flash0-with-uboot.img bs=1M count=64 && dd 
if=/path/to/u-boot.bin of=./flash0-with-uboot.img conv=notrunc
$ qemu-system-aarch64 -M virt -cpu cortex-a53 -m 1024M -nographic -drive 
if=pflash,format=raw,index=0,file=flash0-with-uboot.img  -drive 
if=pflash,format=raw,index=1,file=flash1.img

I'm happy to help test any fixes if and as needed.

Cheers,
Robin

Re: [U-Boot] [PATCH] efi_loader: Fix crash on 32-bit systems

[Robin Randhawa]

Hi Alex.

On Wed 14 Sep 08:34:47 2016, Alexander Graf wrote:

[...]
 
> Very nice catch!

Thanks!

[...]
 
> Can you please double-check that this is the only place the type 
> mismatch happened? 

So I skimmed through the boot and run-time service API implementations 
and couldn't spot another instance of a mismatch. Ideally it would be 
neat to have an automated way to check for these things - maybe 
Coccinelle or something.

> For this change however, the fix is already correct:
> 
>   Reviewed-by: Alexander Graf 

Thanks,
Robin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] efi_loader: Fix crash on 32-bit systems

[Robin Randhawa]

A type mismatch in the efi_allocate_pool boot service flow causes
hazardous memory scribbling on 32-bit systems.

This is efi_allocate_pool's prototype:

static efi_status_t EFIAPI efi_allocate_pool(int pool_type,
unsigned long size,
void **buffer);

Internally, it invokes efi_allocate_pages as follows:

efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12,
(void*)buffer);

This is efi_allocate_pages' prototype:

efi_status_t efi_allocate_pages(int type, int memory_type,
unsigned long pages,
uint64_t *memory);

The problem: efi_allocate_pages does this internally:

*memory = addr;

This fix in efi_allocate_pool uses a transitional uintptr_t cast to
ensure the correct outcome, irrespective of the system's native word
size.

This was observed when bootefi'ing the EFI instance of FreeBSD's first
stage bootstrap (boot1.efi) on a 32-bit ARM platform (Qemu VExpress +
Cortex-a9).

Signed-off-by: Robin Randhawa <robin.randh...@arm.com>
---
 lib/efi_loader/efi_boottime.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index be6f5e8..784891b 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -134,9 +134,11 @@ static efi_status_t EFIAPI efi_allocate_pool(int 
pool_type, unsigned long size,
 void **buffer)
 {
efi_status_t r;
+   efi_physical_addr_t t;
 
EFI_ENTRY("%d, %ld, %p", pool_type, size, buffer);
-   r = efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12, 
(void*)buffer);
+   r = efi_allocate_pages(0, pool_type, (size + 0xfff) >> 12, );
+   *buffer = (void *)(uintptr_t)t;
return EFI_EXIT(r);
 }
 
-- 
2.9.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Uboot hangs on starting kernel...

[Robin Randhawa]

Greetings.

On Thu10 Dec 2009, at 20:29, Wierd O wrote:

 I am not sure why the kernel stops short of loading. The other thing that 
 confuses me is that i couldnt know whther the proble is from the image or 
 uboot.

You are assuming that the kernel 'stops short of loading'. It is quite likely 
that the kernel has gone along a fair but and attempted to bring up your board 
but has panicked at a very early stage before the kernel log buffer was flushed 
to the console thereby preventing you from getting a feel for what really went 
wrong. There could be any number of reasons for this of course.

My recommendation would be to start hacking the kernel from a very early stage 
of initialisation. If you can get to Linux's '__log_buf' and print out the 
ASCII contents straight to the UART, I bet you'll get a handle on the problem. 
It'll most likely be a kernel misconfiguration issue but you'll get the idea 
one hopes.

Cheers,
Robin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot