Re: [PATCH] powerpc/boot: Fix the initrd being overwritten under qemu

[Segher Boessenkool <segher@kernel.crashing.org>]

On Thu, Oct 24, 2019 at 12:31:24PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 23/10/2019 22:21, Segher Boessenkool wrote:
> > On Wed, Oct 23, 2019 at 12:36:35PM +1100, Oliver O'Halloran wrote:
> >> When booting under OF the zImage expects the initrd address and size to be
> >> passed to it using registers r3 and r4. SLOF (guest firmware used by QEMU)
> >> currently doesn't do this so the zImage is not aware of the initrd
> >> location.  This can result in initrd corruption either though the zImage
> >> extracting the vmlinux over the initrd, or by the vmlinux overwriting the
> >> initrd when relocating itself.
> >>
> >> QEMU does put the linux,initrd-start and linux,initrd-end properties into
> >> the devicetree to vmlinux to find the initrd. We can work around the SLOF
> >> bug by also looking those properties in the zImage.
> > 
> > This is not a bug.  What boot protocol requires passing the initrd start
> > and size in GPR3, GPR4?
> 
> So far I was unable to identify it...

Maybe this comes from yaboot?
https://git.ozlabs.org/?p=yaboot.git;a=blob;f=second/yaboot.c;h=9b66ab44e1be0ee82b88e386a5d0358428766e73;hb=HEAD#l1186

> > The CHRP binding (what SLOF implements) requires passing two zeroes here.
> > And ePAPR requires passing the address of a device tree and a zero, plus
> > something in GPR6 to allow distinguishing what it does.
> > 
> > As Alexey says, initramfs works just fine, so please use that?  initrd was
> > deprecated when this code was written already.
> 
> I did not say about anything working fine :)

Yeah, I read that from your words, wrong it seems.  Sorry.  I often used
INITRAMFS_SOURCE for kernels for use with SLOF, it's just so convenient.

> In my case I was using a new QEMU which does full FDT on client-arch-support and that thing would put the original
> linux,initrd-start/end to the FDT even though the initrd was unpacked and the properties were changes in SLOF. With that
> fixed, this is an alternative fix for SLOF but I am not pushing it out as I have no idea about the bindings and this
> also breaks "vmlinux".
> 
> 
> diff --git a/slof/fs/client.fs b/slof/fs/client.fs
> index 8a7f6ac4326d..138177e4c2a3 100644
> --- a/slof/fs/client.fs
> +++ b/slof/fs/client.fs
> @@ -45,6 +45,17 @@ VARIABLE  client-callback \ Address of client's callback function
>    >r  ciregs >r7 !  ciregs >r6 !  client-entry-point @ ciregs >r5 !
>    \ Initialise client-stack-pointer
>    cistack ciregs >r1 !
> +
> +  s" linux,initrd-end" get-chosen IF decode-int -rot 2drop ELSE 0 THEN
> +  s" linux,initrd-start" get-chosen IF decode-int -rot 2drop ELSE 0 THEN
> +  2dup - dup IF
> +    ciregs >r4 !
> +    ciregs >r3 !
> +    drop
> +  ELSE
> +    3drop
> +  THEN

Something like that should work fine.  Do it in go-32 and go-64 though?
Or is that the wrong spot?


Segher