On Fri, 2014-06-06 at 17:00 +0800, Fu Wei wrote:
multiboot_fdt.c would be a better name than multiboot_xen.c, if that's what you intend the abstraction to be about, but TBH this seems like a premature refactoring to me. If/when someone invents up "multiboot over $SOMETHING_ELSE" then refactoring can be done then.
For multiboot_xen.c, it includes (1)the boot info injection into FDT for multi binaries(following xen specific boot binaries order) (2)the xen specific compatibility strings and other info
The intention with http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot was that nothing there was really Xen specific. I think this is the case.
Although it includes some FDT-related code, it is not for fdt. It is for Xen.
Well, Xen is a consumer of this interface but the interface itself is supposed to be more widely useful than that.
Remember that this multiboot stuff is intended to be generic, even if Xen people are involved in defining it and Xen is currently the only user. I really want to avoid defining something which cannot be used by someone else if they want.
Could optionally extend that with a multiboot --type=xen option to make grub include the xen,* names *in addition to* the mulitboot,* ones -- which would be nice because it would make Xen 4.4 Just Work (although I would also intend to backport the Xen patch I mentioned).
yes, for this version of patch, you can just use --type="multiboot,linux-zimage" to customize the compatibility strings. This feature is in http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/Multiboot, I have implemented it in this patch.
What I meant was that we could arrange that: multiboot /xen-4.1-amd64.gz [...] module /vmlinuz-3.10-3-amd64 [...]
Would produce a module with compatible = "multiboot,linux-zimage", "boot,module" But: multiboot --type=xen /xen-4.1-amd64.gz [...] module /vmlinuz-3.10-3-amd64 [...] would produce: compatible = "xen,linux-zimage", "multiboot,linux-zimage", "boot,module" i.e enable additional Xen compatibility strings. This would be an extension to the spec as described in the wiki which is why I described it as optional/would be nice. Feel free to ignore this suggested extension.
OK, you want to use "--type" in multiboot command to identify multiboot kernel. please update wiki page.
That's not quite what I meant. So...
It could be a option, will try to implement it in next patch.
... I think this extension is just going to confuse things right now. Lets leave it out.
+#define GRUB_MULTIBOOT_FLAGS_CMDLINE (0x2)
I'm not sure if this is intended as "module has a command line" or "module can validly have a command line". The former is equivalent to module->cmdline != NULL, I think and the second is something which I don't think grub needs to worry about -- it's up to the multiboot kernel how it wants to deal with command lines for modules where it doesn't make sense (i.e. ignore them).
it is also a option, if you know some type of module don't need the command line, you can set this, then the code will simply ignore the strings after the module file name, otherwise, the grub will save the arguments to the struct multiboot_module node. But how to deal with this is up to the multiboot kernel.
I don't think it is up to grub to enforce this sort of thing. If I want to give a command line to my initrd then that's my business, even if it might be meaningless to the kernel ... Including a command line for something which doesn't logically require one is fine -- it'll be ignored by the multiboot kernel.
I think you should remove both of these GRUB_MULTIBOOT_FLAGS_*. It'll make the code simpler and remove unnecessary restrictions which aren't defined by the spec.
I can remove these flags. The original thought for the flags is to avoid unnecessary process and potential faulty input. If I already know there is not cmdline for initrd/xsm, why I still put these meaningless strings into buffer?
It might be unusual to pass a command line with an initrd/xsm but it's by no means completely implausible that a multiboot capable kernel might want to consume such a thing.
(In other words "because the spec says that any module can have a command line")
Since in effect grub defines the other forms of multiboot anyway it's possible that there should be a separate documentation patch which adds the spec to the grub docs?
the definitions above will just affect Xen.
Support for the other property names is there mainly to make it possible to write a generic u-boot environment which can be used with both Xen and native Linux boot.scr, but of course you don't care one jot about that here. Using /chosen/modules@XXX/bootargs is absolutely the right thing for you to be using in the context of multiboot.
If so, please check docs/misc/arm/device-tree/booting.txt (master branch of git://xenbits.xen.org/xen.git)
bootargs (optional)
Command line associated with this module. This is deprecated and should be replaced by the bootargs variations described below.
I hope I did not misunderstand this. :-)
This is wrong and confusing. Especially since the "variations described below" explicitly include this field as an option!
I shall send a patch to fix it.
Ian.