Hi Stefano,
On 10/07/2018 01:00, Stefano Stabellini wrote:
On Mon, 9 Jul 2018, Julien Grall wrote:
Hi,
On 07/07/18 00:12, Stefano Stabellini wrote:
Remove the cmdline field from struct boot_module, cmdline is stored
independently out of the boot_modules array as dom0_cmdline.
I am not entirely convince of this patch, this does not seem to go towards a
better code base because dom0_cmdline is only set if "bootargs". This may
raise some confusing to the developer.
I'll add a comment on top of dom0_cmdline to clarify its purpose:
/*
* Dom0 command line as passed via Device Tree as "bootargs" for the
* Dom0 kernel module.
*/
I would still prefer to keep the command-line in the boot module structure and
find a way to associated the bootmodule with a node.
How do you suggest we find a good way to associate a boot module with a
node?
I have thought about this problem quite a bit. Although I admit this
patch is not super nice, it is the best option I found. I have actually
developed 2 other completely different implementations of this fix and
they were all worse than this. This is the third incarnation. It is
actually surprisingly easy to do worse than this patch.
I actually have an idea. Looking at the Device-Tree specification [1], a
path should always be unique. This means that for a given path a/b/c/d,
d will always be unique.
Reading the binding you introduced, the multiboot node will be a child
of a node with the compatible "xen,domain". As the name of the parent
node will be unique, you can tag the boot-module with that name. No need
for the full path as all the configuration node should follow the
pattern /chosen/<domain-name>.
When the guest is built, you have the Device-Tree path of the
configuration in hand. From that you can deduce the name that you could
re-use to find the correct boot-module.
You may still require a separate handling similar to your next patch of
Dom0 as with the current wording they could be a level deeper. But we
can tight the wording for the new bindings (though, I think it is tight
enough).
What do you think?
Now regarding the current implementation, it does not follow the defined
specification. The multiboot nodes are looked everywhere in the DT
rather than only /chosen. This is an implementation bug and should be fixed.
Cheers,
[1] https://www.devicetree.org/specifications/
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel