Re: [Xen-devel] [PATCH RFC 03/15] xen/arm: extend device tree based multiboot protocol

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 03/07/18 23:16, Stefano Stabellini wrote:
> On Tue, 3 Jul 2018, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 02/07/18 22:31, Stefano Stabellini wrote:
> > > On Thu, 14 Jun 2018, Julien Grall wrote:
> > > > On 13/06/18 23:15, Stefano Stabellini wrote:
> > > > > +
> > > > > +- cpus (optional)
> > > > > +
> > > > > +    A string specifying the number of vcpus to allocate to the guest.
> > > > > If
> > > > > +    not specified it defaults to "1".
> > > > Same remarks as for "mem".
> > > I think it would be nicer if we kept the same format used for xl config
> > > files for these two options.Especially given that we already have the
> > > functions in the hypervisor to parse them (Xen knows how to parse
> > > dom0_max_vcpus and dom0_mem for instance). It is going to be easier to > use
> > > and it doesn't come with a cost for Xen.
> > I can't see any reason to keep those 2 options similar to libxl when all the
> > rest is embracing DT format. The user would already have to spend some times
> > to look at the new bindings anyway, not using string is not going to be a big
> > difficulty.
> >
> > While I agree that Xen knows how to parse dom0_mem, dom0_max_vcpus, Xen also
> > knows how to read numbers from cells which is widely used within the
> > hypervisor. So both solution are going to be easy to use in the hypervisor.
> > However, the former is always going to be faster, less error prone than the
> > latter.
> Memory and cpus are the only two VM options we have so far. Kernels and
> ramdisks are part of the multiboot protocol, they are about passing
> binaries from the bootloader to Xen in memory, rather than specifying VM
> properties. They describe something in memory, a bit like a device on
> device tree, while memory and cpus are options a bit like the existing
> bootargs. There are "mem" and "dom0_mem" options as part of the common
> bootargs but they are passed as strings, not as device tree fields.
>
> So I don't think we have anything else "embracing DT format" yet.
I do consider multiboot protocol part of your new VM format.

> For instance, if we were to implement this on x86 (which I am not), I
> would image we would be using the x86 version of multiboot to pass the
> additional kernels and ramdisks, and we would also add an extra strings
> field with the VM options, similar to the string field used as command
> line arguments.
>
> This is to say that I don't think we necessarily have to abide to device
> tree best recommendations to pass the VM options (but we could still
> choose to). There is an argument for using the same format as per the VM
> config file to make it easier for users, but as Edgar also pointed out,
> we don't want to do that if it means adding a complex parser. There is
> also an argument for using a format that could be reused on non-device
> tree platforms.
Well, the DT format gives you so much more flexibility over string.
Let's take the example of describing a Device. With DT you could just do:

mydev {
  prop1;
  prop2;
  host-regs = <.....>;    /* Host MMIO region to map on ... */
  regs = <......> ;      /* Guest MMIO region ... */
  interrupts = <......>;
  ...
}

Let's now take your example with the string

"iomem=0x400000,0x1000 irqs=112,113"

Leaving aside the fact you assume direct mapping, I always hated that 
format in xl because it is hard to correlate with a given device. With 
the DT solution, you have everything related to a device in one place 
and can easily see if you made any mistake.
Also, the iomem solution will have some shortcoming on Arm because it 
takes a GFN and not full physical address. This does not play very well 
with multiple page granularity and I would rather avoid to spread that 
error for dom0less.
Overall, this is much easier for a user to read and write DT over any 
kind of string. It also does not add much in Xen because we already have 
all the DT helpers in hand. And ... parsing cells is a matter of few 
instructions over converting a string to number.
I understand you want to cover everyone (including x86), but it would be 
much easier to convince them to use DT. It is more powerful over 
anything else without adding extra complexity (and time) to parse 
string. I also think the DT solution would be beneficial in long term in 
addition of the current xl format.
This would help to create in similar fashion guest either from Xen or 
userspace.
> This is why I think that for cpus and memory is relatively simple, I
> would just use the same format as dom0_mem and dom0_max_vcpus for
> simplicity of everybody involved (xen and the user).
You still don't explain how this is more simple...

Someone familiar with DT will expect to specify the information in a DT 
way. By that I mean describing the region in cells not in a string.
Xen also has everything to read an arbitrary number of cells.

> The other options
> we'll need are iomem, which takes an address and size, irqs that takes a
> list of integers, and dtdev which takes a device tree path.
That's what you need for your use case, thought not quite enough. You 
still have to provide node for device.
But then, how about PV drivers? How about potentially describing a 
specific memory layout?...
> We could
> make them all a single string such as:
I am afraid that a no-go for me. This is error-prone for the user, not 
readable and a nightmare to parse.
To be honest, I don't think following xl format is the right way to go. 
It is a pain to describe nicely devices or even a memory layout. The DT 
format (or any tree format) feels more suitable here.
Cheers,

--
Julien Grall

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