riscv: Respect the -initrd flag.

[shlevy]

Logic for initrd start address borrowed from arm/boot.c

[shlevy]

Thanks to @sorear for the pointers!

Requires http://lists.infradead.org/pipermail/linux-riscv/2018-February/000056.html to be useful

[michaeljclark]

we should avoid whitespace changes to code that isn't changing

[shlevy]

Fixed

[michaeljclark]

@palmer-dabbelt I like this patch, in some form...

It would also be nice to split the monitor and the kernel as there is also a -bios flag to respect.

e.g.

-bios bbl -kernel vmlinux -initrd initramfs

There are various reasons why this will be useful. One is CI, as riscv-pk and riscv-linux tag hooks could build and deploy test images independently. We won't need to rebuild bbl when we update the kernel, which would be more like other boot loaders. Thirdly, we could substitute other monitor implementations. Essentially it's the M-mode code that hooks mtvec. We'd need a protocol for the boot loader to locate the kernel if it's not embedded. This would also help with adding PMP protection for the monitor and not using a single megapage RWX mapping that includes both the boot loader / monitor and the kernel. i.e. allow us to implement kernel memory protection like all other major platforms. For this, bbl can't be part of the .initdata and needs to be distinct from the kernel.

I discussed this with @palmer-dabbelt and similarly we would add a kernel payload entry to device-tree which if present, bbl would jump to the address in device-tree. We'd then build it with the dummy-payload. This would be backward compatible, as if the device-tree entry is not present, bbl would jump to the embedded payload e.g. (plus associated change to riscv-pk).

qemu_fdt_setprop_cell(fdt, "/chosen",
                                  "riscv,kernel-start", kernel_start);

With respect to this code, are we copying from a GPL licensed file into a MIT licensed file? We might have to switch the license of the machines to GPL. When I implemented virt, I wrote the device tree stuff from scratch and based it on the old sifive_board and the hardware besides virtio is all different (PLIC, CLINT, etc). The sysbus_create_simple("virtio-mmio") came from a patch of @sorear. We do follow the code convention of other hardware and have the prefix VIRT_. In any case I will have to talk to SiFive about changing the license to GPL if we start using code from hw/arm/virt.c. If you do a diff -u hw/arm/virt.c hw/riscv/virt.c you won't find anything common other than convention for things such as the memory map (which is of course totally different)

Finally, if we add this change, should it be on all machines with device-tree? or just virt?

@sagark @sorear I think we need authors to agree to a GPL license change, otherwise we prefix the MIT license with GPL. MIT code is able to be GPL, but the MIT clauses need to remain present, unless the authors agree to relicense as GPL. I don't think this is a problem...

I've just submitted a v6 patch series upstream. I think we should do this stuff on another branch. I'll experiment with this code and look into adding a change to riscv-pk so we can also support -bios.

Thanks! It will be neat when we can separate boot loader from the kernel, as well as the initrd from the kernel as per this patch...

[shlevy]

@michaeljclark Will respond to the rest later, just wanted to let you know that the only thing I took from the arm side of things was the base address calculation (and the comment above it). No idea how that affects licensing questions.

[michaeljclark]

@shlevy I think its best to play safe and license the machines as GPL, esepcially if we copy an actual segment of code from hw/arm/virt.c (versus calling the same APIs with unique code).

It's a gray/grey area if one looks at the other code (e.g. some companies prevent folk working on LLVM from looking at GCC code), conforms to its coding conventions and what not, but is indeed unique. I don't believe we have so far copied anything other than convention from hw/arm/virt.c and that fact we use the same APIs for fdt, however the fdt code was written from scratch (mostly from looking at the fdt header interfaces). The riscv device-tree makes unique use of phandles that are not present in other ports and besides our virtio nodes being similar (they are a little different due to interrupt-parent), the code to construct them was written from scratch.

Interestingly, all of Fabrice's original code is MIT licensed.

[sorear]

1. I'm not taking a position on whether we change the licence on that file, but I am fully happy with my contributions moving to GPLv2.

2. We might ask on qemu-devel@ at some point what the device tree option should be called, since it's not really riscv-specific. x86/seabios does something similar, but doesn't use device tree.

[michaeljclark]

The kernel_start node is an interface with the first stage boot loader. The code that reads it would be riscv-pk/bbl, u-boot or any other bootloader.

"riscv,kernel-start" seems fine, or "riscv,boot-kernel" (see below)

It's obviously not going to have "linux" as per the initrd load address "linux,initrd-start" which is read by linux. Here the linux prefix makes sense, but clearly not for kernel load address. If there is an example from another FDT platform e.g. powerpc, then we can follow their convention.

Okay. I see some powerpc code uses this:

"qemu,boot-kernel"

I don't like the idea of using qemu as a prefix as bootloaders on real hardware could use this device-tree node to for example locate a kernel in XIP flash. i.e. it's not necessarily qemu specific.

[michaeljclark]

-bios will actually work quite nicely with bbl configured with a dummy payload (the default), and some device tree scan code that substitutes the jump to payload. The kernel is already set up to be in S mode with paging disabled, so it will be quite clean. riscv-pk and riscv-linux then become independent, on hardware that can point to the kernel address in device-tree. If not, it will work just as it does presently.

[shlevy]

@michaeljclark Should I point this PR to a new branch?

Re -bios flag, I'm definitely in favor of not rebuilding bbl every time I rebuild my kernel, I'm happy to take a look at the implementation if we settle on an interface. I agree with @sorear that this isn't especially RISC-V specific, but also a qemu prefix doesn't make sense to me either.

Happy to consider this patch GPL or whatever license is appropriate.

[michaeljclark]

We'll have to think about branches. Given we're going through the upstreaming process, things are a bit odd as we don't have our port in master, and riscv-next is our semi-stable branch.

riscv-next keeps getting the squashed updates from the qemu-upstream-vn branches after they have had enough testing feedback.

You could make the pull request against qemu-upstream-v6 or we could perhaps create a riscv-wip where we stage patches that are still work in progress, for this patch in particular we have to get the licensing issue sorted unless we clean room it or cherry pick the code from an MIT licensed file.

From doing a grep on the source, there seems to be several MIT licensed files that handle initrd in device-tree, in the target/ppc directory.

[michaeljclark]

I've created a riscv-wip branch which is based on the tip of qemu-upstream-v6 which is where future upstream feedback review goes until we submit a patch series. We can part patches on riscv-wip. The only issue with merging it at present is the license issue. It's stolen code, if we don't adopt the license header it came from (if it is more restrictive). It seems that MIT licensed files are effectively dual licensed are subsumed by the GPL, but code from GPL files can't move the other way into MIT licensed files. It is annoying.

[michaeljclark]

Otherwise I guess we have to merge a patch to switch the license to GPL, so its compatible.

[shlevy]

Rebased on riscv-wip

[shlevy]

@michaeljclark Who should make the call about the license question?

[michaeljclark]

@palmer-dabbelt said okay to GPL. I guess we can just switch the file to GPL. There is not anything particularly proprietary in there. Let me patch the license header...

[michaeljclark]

Merged.