Re: [Xen-devel] [PATCH v2 09/21] xen/arm: move cmdline out of boot_modules

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 10 Jul 2018, Julien Grall wrote:
> 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.

After our discussion this morning, I introduced a separate array to
store the cmdline of the boot modules. The second array uses the device
tree node name as index.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel