Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5585 bytes --]

On Sat, Jan 21, 2023 at 02:58:19PM -0300, Daniel Henrique Barboza wrote:
> Conor,
> 
> Thanks for the Icicle-kit walk-through!

nw chief

> I'll not claim that I fully understood it,
> but I understood enough to handle the situation ATM.

tbf, I struggle to explain/visualise that stuff with the "windows" etc
well. I wrote myself a program to visualise it for a good reason!
Well it was done in Rust, so there were two good reasons ;)

> Without this change, this is where the FDT is being installed in the board when
> I start it with 8Gb of RAM (retrieved via 'info roms'):
> 
> addr=00000000bfe00000 size=0x00a720 mem=ram name="fdt"
> 
> Which surprised me at first because this is almost at the end of the LO area which has
> 1Gb and I figured it would be in the middle of another RAM area. I took another read
> at what we're doing in riscv_load_fdt():
> 
> -----------
> temp = (dram_base < 3072 * MiB) ?  MIN(dram_end, 3072 * MiB) : dram_end;
> fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> -----------
> 
> This code can be read as "if the starting address of the RAM is lower than 3Gb, put
> the FDT no further than 3Gb (0xc0000000). Otherwise, put it at the end of dram",
> where "dram_base" is the starting address of the RAM block that the function
> receives.
> 
> For icicle-kit, this is being passed as  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> 0x80000000, which is 2Gb.
> 
> So, regardless of how much RAM we have (dram_end), the FDT will always be capped at
> 3Gb. At this moment, this fits exactly at the end of the LO area for the Icicle Kit.
> Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 to fix
> 32 bit guest boot and it happened to also work for the Microchip SoC.

That's hilariously convenient hahah

> So yeah, I thought that I was fixing a bug and in the end I caused one. This patch
> needs to go.
> 
> Alistair, I believe I should re-send v2, this time explaining why the existing function
> will not break the Microchip board because we'll never put the FDT out of the LO area
> of the board. Does this work for you?
> Conor, one more thing:
> 
> 
> On 1/19/23 21:15, Conor Dooley wrote:
> > Hey Daniel,
> > 
> > Got through the stuff I wanted to get done tonight faster than
> > expected...
> > 
> > On Thu, Jan 19, 2023 at 05:17:33PM -0300, Daniel Henrique Barboza wrote:
> > > Are you testing it by using the command line
> > > you mentioned in the "qemu icicle kit es" thread?
> > > 
> > > $(QEMU)/qemu-system-riscv64 \
> > > 	-M microchip-icicle-kit \
> > > 	-m 2G -smp 5 \
> > > 	-kernel $(vmlinux_bin) \
> > > 	-dtb $(devkit).dtb \
> > > 	-initrd $(initramfs) \
> > > 	-display none \
> > > 	-serial null \
> > > 	-serial stdio
> > 
> > Yah, effectively. It's not quite that, but near enough as makes no real
> > difference:
> > qemu-icicle:
> > 	$(QEMU)/qemu-system-riscv64 -M microchip-icicle-kit \
> > 		-m 2G -smp 5 \
> > 		-kernel $(vmlinux_bin) \
> > 		-dtb $(wrkdir)/riscvpc.dtb \
> > 		-initrd $(initramfs) \
> > 		-display none -serial null \
> > 		-serial stdio \
> > 		-D qemu.log -d unimp
> > 
> > I just tried to make things somewhat more intelligible for that thread.
> 
> I tried it out with kernel v6.0.0 (I saw you mentioning in the other thread that
> this was the latest kernel you were able to boot this way)

Yah, I said that because I didn't want them to have to mess with DT.
Later kernels do work, but need DT modifications as things are now
configured for the below case.
> > The current Devicetree in Linux & U-Boot corresponds to a configuration
> > like:
> > ⢰⠒⠒⠒⠒⡖⠒⠒⠒⣶⠒0x80000000
> > ⢸    ⡇   ⣿ ⡇
> > ⢸    ⡇   ⡟ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸    ⡇   ⡇ ⡇
> > ⢸⡖⠒⠒⢲⡇   ⡇ 0x40000000
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇
> > ⢸⡇  ⢸⡇   ⡇ ⡇ <-- 32- & 64-bit start at 0x0
> > ⠘⠓⠒0⠚⠓⠒1⠒⠓⠒0x00000000
> > 
> > That DT looks like:
> > 	ddrc_cache_lo: memory@80000000 {
> > 		device_type = "memory";
> > 		reg = <0x0 0x80000000 0x0 0x40000000>;
> > 		status = "okay";
> > 	};
> > 
> > 	ddrc_cache_hi: memory@1040000000 {
> > 		device_type = "memory";
> > 		reg = <0x10 0x40000000 0x0 0x40000000>;
> > 		status = "okay";
> > 	};

This one doesn't work in QEMU, so for those kernels I just delete the
ddrc_cache_hi node, and v6.2-rcN works in that way.

> and it booted up until
> the kernel complained about missing initramfs. Any tips on how I can build an
> initrd disk for the board?

Ehh, any old initramfs for RISC-V should work, right? I suppose passing
a normal rootfs does either - I just mostly work w/ hardware & use NFS
there, so have nothing scripted to build a rootfs for me, which is why
I've been using initramfs.
I build one using buildroot, with a config like:
https://raw.githubusercontent.com/ConchuOD/riscv-env/dev/conf/lowmem/buildroot_initramfs_config

I then do (ripped from my makefile rule):
		$(linux_srcdir)/usr/gen_initramfs.sh \
		-o initramfs.cpio -u $(shell id -u) -g $(shell id -g) \
		initramfs.txt \
		$(path_to_buildroot_sysroot)

I'm lazy and CBA finding somewhere else to host this, so I put one here:
https://github.com/ConchuOD/riscv-env/releases/download/v2022.03/initramfs.cpio.gz

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]