Re: [PATCH v2 1/2] docs: update hyperlaunch device tree

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 8 Aug 2023, Julien Grall wrote:
> > +
> > +                    role = <9>;
> 
> Reading this, I wonder if using number is actually a good idea. While this is
> machine friendly, this is not human friendly.

+1


> The most human friendly interface would be to use string, but I understand
> this is more complex to parse. So maybe we could use some pre-processing (like
> Linux does) to ease the creation of the hyperlaunch DT.
> 
> Bertrand, Stefano, what do you think?

I think that some preprocessing (e.g. ImageBuilder) is very likely required.
At the same time I think that "role" could make sense as a string and parsed
as a string without issues. I am happy either way.


> > +                    mode = <12>;
> > +
> > +                    domain-uuid = [B3 FB 98 FB 8F 9F 67 A3 8A 6E 62 5A 09
> > 13 F0 8C];
> > +
> > +                    cpus = <1>;
> > +                    memory = "1024M";
> > +
> > +                    kernel {
> > +                        compatible = "module,kernel", "module,index";
> > +                        module-index = <1>;
> > +                    };
> > +
> > +                    initrd {
> > +                        compatible = "module,ramdisk", "module,index";
> > +                        module-index = <2>;
> > +                    };
> > +                };
> > +
> > +                dom1 {
> > +                    compatible = "xen,domain";
> > +                    domid = <1>;
> > +                    role = <0>;
> > +                    capability = <1>;
> > +                    mode = <12>;
> > +                    domain-uuid = [C2 5D 91 CB 60 4B 45 75 89 04 FF 09 64
> > 54 1A 74];
> > +                    cpus = <1>;
> > +                    memory = "1024M";
> > +
> > +                    kernel {
> > +                        compatible = "module,kernel", "module,index";
> > +                        module-index = <3>;
> > +                        bootargs = "console=hvc0 earlyprintk=xen
> > root=/dev/ram0 rw";
> > +                    };
> > +
> > +                    initrd {
> > +                        compatible = "module,ramdisk", "module,index";
> > +                        module-index = <4>;
> > +                    };
> > +                };


I think dom1 should be written as follows:

    dom1 {
        compatible = "xen,domain";

        domid = <1>;
        role = <0>;
        capability = <1>;
        mode = <12>;
        domain-uuid = [C2 5D 91 CB 60 4B 45 75 89 04 FF 09 64

        cpus = <1>;
        memory = <0 1048576>;

        kernel {
            compatible = "multiboot,kernel" "multiboot,module";
            module-index = <3>;
            bootargs = "console=hvc0 earlyprintk=xen0 rw";
        };

        initrd {
            compatible = "multiboot,ramdisk" "multiboot,module"
            module-index = <4>;
        };
    };



> > +            };
> > +        };
> > +    };
> > +
> > +
> > +
> > +The multiboot modules supplied when using the above config would be, in
> > order:
> > +
> > +* (the above config, compiled)
> > +* kernel for PVH unbounded domain
> > +* ramdisk for PVH unbounded domain
> > +* kernel for PVH guest domain
> > +* ramdisk for PVH guest domain
> > +
> > +Module Arm Configuration:
> > +"""""""""""""""""""""""""
> > +
> > +::
> > +
> > +    /dts-v1/;
> > +
> > +    / {
> > +        chosen {
> > +            hypervisor {
> > +                compatible = “hypervisor,xen”
> > +
> > +                // Configuration container
> > +                config {
> > +                    compatible = "xen,config";
> > +
> > +                    module {
> > +                        compatible = "module,xsm-policy";
> > +                        module-addr = <0x0000ff00 0x80>;
> > +
> > +                    };
> > +                };
> > +
> > +                // Unbounded Domain definition
> > +                dom0 {
> > +                    compatible = "xen,domain";
> > +
> > +                    domid = <0>;
> > +
> > +                    role = <9>;
> > +
> > +                    mode = <12>; /* 64 BIT, PVH */
> 
> Arm guest have similar feature compare to PVH guest but they are strictly not
> the same. So we have been trying to avoid using the term on Arm.
> 
> I would prefer if we continue to avoid using the word 'PVH' to describe Arm.
> Lets just call them 'Arm guest'.
> 
> > +
> > +                    memory = <0x0 0x20000>;
> 
> Here you use the integer version, but AFAICT this wasn't described in the
> binding above.
> 
> > +                    security-id = “dom0_t”;
> > +
> > +                    module {
> > +                        compatible = "module,kernel";
> > +                        module-addr = <0x0000ff00 0x80>;
> 
> Reading the binding, this is suggest that the first cell is the start address
> and the second is the size. Cells are 32-bits. So what if you have a 64-bit
> address?
> 
> For 'reg' property, the DT addressed this by using #address-cells and
> #size-cells to indicate the number of cells for each.

I would not deviate from the dom0less spec on this one, which uses reg
for modules addresses and sizes.


> > +                        bootargs = "console=hvc0";
> > +                    };
> > +                    module {
> > +                        compatible = "module,ramdisk";
> > +                        module-addr = <0x0000ff00 0x80>;
> > +                    };
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +The modules that would be supplied when using the above config would be:
> > +
> > +* (the above config, compiled into hardware tree)
> > +* XSM policy
> > +* kernel for unbounded domain
> > +* ramdisk for unbounded domain
> > +* kernel for guest domain
> > +* ramdisk for guest domain
> > +
> > +The hypervisor device tree would be compiled into the hardware device tree
> > and
> > +provided to Xen using the standard method currently in use. 
> 
> It is not clear what you mean by 'compiled in'. Do you mean the /hypervisor
> node will be present in the device-tree provided to Xen?

That is the way I read it as well. I think this statement is unnecessary
as we assume there is only 1 device tree passed to Xen, so this
specification would cover the relevant part of it.


> > The remaining
> > +modules would need to be loaded in the respective addresses specified in
> > the
> > +`module-addr` property.
> 
> Cheers,
> 
> -- 
> Julien Grall
>