diff mbox series

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

Message ID 20230803104438.24720-2-dpsmith@apertussolutions.com (mailing list archive)
State New, archived
Headers show
Series Rebranding dom0less to hyperlaunch part 1 | expand

Commit Message

Daniel P. Smith Aug. 3, 2023, 10:44 a.m. UTC
With on going development of hyperlaunch, changes to the device tree definitions
has been necessary. This commit updates the specification for all current changes
along with changes expected to be made in finalizing the capability.

This commit also adds a HYPERLAUNCH section to the MAINTAINERS file and places
this documentation under its purview. It also reserves the path
`xen/common/domain-builder` for the hyperlaunch domain builder code base.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 MAINTAINERS                                   |   9 +
 .../designs/launch/hyperlaunch-devicetree.rst | 566 ++++++++++--------
 2 files changed, 309 insertions(+), 266 deletions(-)

Comments

Michal Orzel Aug. 3, 2023, 11:45 a.m. UTC | #1
Hi Daniel,

On 03/08/2023 12:44, Daniel P. Smith wrote:
> 
> 
> With on going development of hyperlaunch, changes to the device tree definitions
> has been necessary. This commit updates the specification for all current changes
> along with changes expected to be made in finalizing the capability.
> 
> This commit also adds a HYPERLAUNCH section to the MAINTAINERS file and places
> this documentation under its purview. It also reserves the path
> `xen/common/domain-builder` for the hyperlaunch domain builder code base.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>  MAINTAINERS                                   |   9 +
>  .../designs/launch/hyperlaunch-devicetree.rst | 566 ++++++++++--------
>  2 files changed, 309 insertions(+), 266 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8a02a6c19..694412a961 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -332,6 +332,15 @@ M: Nick Rosbrook <rosbrookn@gmail.com>
>  S:     Maintained
>  F:     tools/golang
> 
> +HYPERLAUNCH
> +M:     Daniel P. Smith <dpsmith@apertussolutions.com>
> +M:     Christopher Clark <christopher.w.clark@gmail.com>
> +W:     https://wiki.xenproject.org/wiki/Hyperlaunch
> +S:     Supported
> +F:     docs/design/launch/hyperlaunch.rst
> +F:     docs/design/launch/hyperlaunch-devicetree.rst
> +F:     xen/common/domain-builder/
> +
>  HYPFS
>  M:     Juergen Gross <jgross@suse.com>
>  S:     Supported
> diff --git a/docs/designs/launch/hyperlaunch-devicetree.rst b/docs/designs/launch/hyperlaunch-devicetree.rst
> index b49c98cfbd..0bc719e4ae 100644
> --- a/docs/designs/launch/hyperlaunch-devicetree.rst
> +++ b/docs/designs/launch/hyperlaunch-devicetree.rst
> @@ -2,10 +2,11 @@
>  Xen Hyperlaunch Device Tree Bindings
>  -------------------------------------
> 
> -The Xen Hyperlaunch device tree adopts the dom0less device tree structure and
> -extends it to meet the requirements for the Hyperlaunch capability. The primary
> -difference is the introduction of the ``hypervisor`` node that is under the
> -``/chosen`` node. The move to a dedicated node was driven by:
> +The Xen Hyperlaunch device tree is informed by the dom0less device tree
> +structure with extensions to meet the requirements for the Hyperlaunch
> +capability. A major depature from the dom0less device tree is the introduction
> +of the ``hypervisor`` node that is under the ``/chosen`` node. The move to a
> +dedicated node was driven by:
> 
>  1. Reduces the need to walk over nodes that are not of interest, e.g. only
>     nodes of interest should be in ``/chosen/hypervisor``
> @@ -13,331 +14,364 @@ difference is the introduction of the ``hypervisor`` node that is under the
>  2. Allows for the domain construction information to easily be sanitized by
>     simple removing the ``/chosen/hypervisor`` node.
> 
> -Example Configuration
> ----------------------
> -
> -Below are two example device tree definitions for the hypervisor node. The
> -first is an example of a multiboot-based configuration for x86 and the second
> -is a module-based configuration for Arm.
> -
> -Multiboot x86 Configuration:
> -""""""""""""""""""""""""""""
> -
> -::
> -
> -    hypervisor {
> -        #address-cells = <1>;
> -        #size-cells = <0>;
> -        compatible = “hypervisor,xen”
> -
> -        // Configuration container
> -        config {
> -            compatible = "xen,config";
> -
> -            module {
> -                compatible = "module,microcode", "multiboot,module";
> -                mb-index = <1>;
> -            };
> -
> -            module {
> -                compatible = "module,xsm-policy", "multiboot,module";
> -                mb-index = <2>;
> -            };
> -        };
> -
> -        // Boot Domain definition
> -        domain {
> -            compatible = "xen,domain";
> -
> -            domid = <0x7FF5>;
> -
> -            // FUNCTION_NONE            (0)
> -            // FUNCTION_BOOT            (1 << 0)
> -            // FUNCTION_CRASH           (1 << 1)
> -            // FUNCTION_CONSOLE         (1 << 2)
> -            // FUNCTION_XENSTORE        (1 << 30)
> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
> -            functions = <0x00000001>;
> -
> -            memory = <0x0 0x20000>;
> -            cpus = <1>;
> -            module {
> -                compatible = "module,kernel", "multiboot,module";
> -                mb-index = <3>;
> -            };
> -
> -            module {
> -                compatible = "module,ramdisk", "multiboot,module";
> -                mb-index = <4>;
> -            };
> -            module {
> -                compatible = "module,config", "multiboot,module";
> -                mb-index = <5>;
> -            };
> -
> -        // Classic Dom0 definition
> -        domain {
> -            compatible = "xen,domain";
> -
> -            domid = <0>;
> -
> -            // PERMISSION_NONE          (0)
> -            // PERMISSION_CONTROL       (1 << 0)
> -            // PERMISSION_HARDWARE      (1 << 1)
> -            permissions = <3>;
> -
> -            // FUNCTION_NONE            (0)
> -            // FUNCTION_BOOT            (1 << 0)
> -            // FUNCTION_CRASH           (1 << 1)
> -            // FUNCTION_CONSOLE         (1 << 2)
> -            // FUNCTION_XENSTORE        (1 << 30)
> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
> -            functions = <0xC0000006>;
> -
> -            // MODE_PARAVIRTUALIZED     (1 << 0) /* PV | PVH/HVM */
> -            // MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
> -            // MODE_LONG                (1 << 2) /* 64 BIT | 32 BIT */
> -            mode = <5>; /* 64 BIT, PV */
> -
> -            // UUID
> -            domain-uuid = [B3 FB 98 FB 8F 9F 67 A3];
> -
> -            cpus = <1>;
> -            memory = <0x0 0x20000>;
> -            security-id = “dom0_t;
> -
> -            module {
> -                compatible = "module,kernel", "multiboot,module";
> -                mb-index = <6>;
> -                bootargs = "console=hvc0";
> -            };
> -            module {
> -                compatible = "module,ramdisk", "multiboot,module";
> -                mb-index = <7>;
> -            };
> -    };
> -
> -The multiboot modules supplied when using the above config would be, in order:
> +The Hypervisor node
> +-------------------
> 
> -* (the above config, compiled)
> -* CPU microcode
> -* XSM policy
> -* kernel for boot domain
> -* ramdisk for boot domain
> -* boot domain configuration file
> -* kernel for the classic dom0 domain
> -* ramdisk for the classic dom0 domain
> +The ``hypervisor`` node is a top level container for all information relating
> +to how the hyperlaunch is to proceed. This includes definitions of the domains
> +that will be built by hypervisor on start up. The node will be named
> +``hypervisor``  with a ``compatible`` property to identify which hypervisors
> +the configuration is intended. The hypervisor node will consist of one or more
> +config nodes and one or more domain nodes.
> 
> -Module Arm Configuration:
> -"""""""""""""""""""""""""
> +Properties
> +""""""""""
> 
> -::
> +compatible
> +  Identifies which hypervisors the configuration is compatible. Required.
> 
> -    hypervisor {
> -        compatible = “hypervisor,xen”
> +  Format: "hypervisor,<hypervisor name>", e.g "hypervisor,xen"
> 
> -        // Configuration container
> -        config {
> -            compatible = "xen,config";
> +Child Nodes
> +"""""""""""
> 
> -            module {
> -                compatible = "module,microcode”;
> -                module-addr = <0x0000ff00 0x80>;
> -            };
> +* config
> +* domain
> 
> -            module {
> -                compatible = "module,xsm-policy";
> -                module-addr = <0x0000ff00 0x80>;
> +Config Node
> +-----------
> 
> -            };
> -        };
> +A ``config`` node is for passing configuration data and identifying any boot
> +modules that is of interest to the hypervisor.  For example this would be where
> +Xen would be informed of microcode or XSM policy locations. Each ``config``
> +node will require a unique device-tree compliant name as there may be one or
> +more ``config`` nodes present in a single dtb file. To identify which
> +hypervisor the configuration is intended, the required ``compatible`` property
> +must be present.
> 
> -        // Boot Domain definition
> -        domain {
> -            compatible = "xen,domain";
> -
> -            domid = <0x7FF5>;
> -
> -            // FUNCTION_NONE            (0)
> -            // FUNCTION_BOOT            (1 << 0)
> -            // FUNCTION_CRASH           (1 << 1)
> -            // FUNCTION_CONSOLE         (1 << 2)
> -            // FUNCTION_XENSTORE        (1 << 30)
> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
> -            functions = <0x00000001>;
> -
> -            memory = <0x0 0x20000>;
> -            cpus = <1>;
> -            module {
> -                compatible = "module,kernel";
> -                module-addr = <0x0000ff00 0x80>;
> -            };
> +While the config node is not meant to replace the hypervisor commandline, there
> +may be cases where it is better suited for passing configuration details at
> +boot time.  This additional information may be carried in properties assigned
> +to a ``config`` node. If there are any boot modules that are intended for the
> +hypervisor, then a ``module`` child node should be provided to identify the
> +boot module.
> 
> -            module {
> -                compatible = "module,ramdisk";
> -                module-addr = <0x0000ff00 0x80>;
> -            };
> -            module {
> -                compatible = "module,config";
> -                module-addr = <0x0000ff00 0x80>;
> -            };
> +Properties
> +""""""""""
> 
> -        // Classic Dom0 definition
> -        domain@0 {
> -            compatible = "xen,domain";
> -
> -            domid = <0>;
> -
> -            // PERMISSION_NONE          (0)
> -            // PERMISSION_CONTROL       (1 << 0)
> -            // PERMISSION_HARDWARE      (1 << 1)
> -            permissions = <3>;
> -
> -            // FUNCTION_NONE            (0)
> -            // FUNCTION_BOOT            (1 << 0)
> -            // FUNCTION_CRASH           (1 << 1)
> -            // FUNCTION_CONSOLE         (1 << 2)
> -            // FUNCTION_XENSTORE        (1 << 30)
> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
> -            functions = <0xC0000006>;
> -
> -            // MODE_PARAVIRTUALIZED     (1 << 0) /* PV | PVH/HVM */
> -            // MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
> -            // MODE_LONG                (1 << 2) /* 64 BIT | 32 BIT */
> -            mode = <5>; /* 64 BIT, PV */
> -
> -            // UUID
> -            domain-uuid = [B3 FB 98 FB 8F 9F 67 A3];
> -
> -            cpus = <1>;
> -            memory = <0x0 0x20000>;
> -            security-id = “dom0_t”;
> -
> -            module {
> -                compatible = "module,kernel";
> -                module-addr = <0x0000ff00 0x80>;
> -                bootargs = "console=hvc0";
> -            };
> -            module {
> -                compatible = "module,ramdisk";
> -                module-addr = <0x0000ff00 0x80>;
> -            };
> -    };
> +compatible
> +  Identifies the hypervisor the confiugration is intended. Required.
> 
> -The modules that would be supplied when using the above config would be:
> +  Format: "<hypervisor name>,config", e.g "xen,config"
> 
> -* (the above config, compiled into hardware tree)
> -* CPU microcode
> -* XSM policy
> -* kernel for boot domain
> -* ramdisk for boot domain
> -* boot domain configuration file
> -* kernel for the classic dom0 domain
> -* ramdisk for the classic dom0 domain
> +bootargs
> +  This is used to provide the boot params for Xen.
> 
> -The hypervisor device tree would be compiled into the hardware device tree and
> -provided to Xen using the standard method currently in use. The remaining
> -modules would need to be loaded in the respective addresses specified in the
> -`module-addr` property.
> +  Format: String, e.g. "flask=silo"
> 
> +Child Nodes
> +"""""""""""
> 
> -The Hypervisor node
> --------------------
> +* module
> 
> -The hypervisor node is a top level container for the domains that will be built
> -by hypervisor on start up. On the ``hypervisor`` node the ``compatible``
> -property is used to identify the type of hypervisor node present..
> +Domain Node
> +-----------
> 
> -compatible
> -  Identifies the type of node. Required.
> +A ``domain`` node is for describing the construction of a domain. Since there
> +may be one or more domain nodes, each one requires a unique, DTB compliant name
> +and a ``compatible`` property to identify as a domain node.
> 
> -The Config node
> ----------------
> +A ``domain`` node  may provide a ``domid`` property which will be used as the
> +requested domain id for the domain with a value of “0” signifying to use the
> +next available domain id, which is the default behavior if omitted. It should
> +be noted that a domain configuration is not able to request a domid of “0”.
> +Beyond that, a domain node may have any of the following optional properties.
> 
> -A config node is for detailing any modules that are of interest to Xen itself.
> -For example this would be where Xen would be informed of microcode or XSM
> -policy locations. If the modules are multiboot modules and are able to be
> -located by index within the module chain, the ``mb-index`` property should be
> -used to specify the index in the multiboot module chain.. If the module will be
> -located by physical memory address, then the ``module-addr`` property should be
> -used to identify the location and size of the module.
> +Properties
> +""""""""""
> 
>  compatible
> -  Identifies the type of node. Required.
> -
> -The Domain node
> ----------------
> +  Identifies the node as a domain node and for which hypervisor. Required.
> 
> -A domain node is for describing the construction of a domain. It may provide a
> -domid property which will be used as the requested domain id for the domain
> -with a value of “0” signifying to use the next available domain id, which is
> -the default behavior if omitted. A domain configuration is not able to request
> -a domid of “0”. After that a domain node may have any of the following
> -parameters,
> -
> -compatible
> -  Identifies the type of node. Required.
> +  Format: "<hypervisor name>,domain", e.g "xen,domain"
> 
>  domid
> -  Identifies the domid requested to assign to the domain. Required.
> +  Identifies the domid requested to assign to the domain.
> 
> -permissions
> +  Format: Integer, e.g <0>
> +
> +role
>    This sets what Discretionary Access Control permissions
>    a domain is assigned. Optional, default is none.
> 
> -functions
> -  This identifies what system functions a domain will fulfill.
> +  Format: Bitfield, e.g <3> or <0x00000003>
> +
> +          ROLE_NONE                (0)
> +          ROLE_UNBOUNDED_DOMAIN    (1U<<0)
> +          ROLE_CONTROL_DOMAIN      (1U<<1)
> +          ROLE_HARDWARE_DOMAIN     (1U<<2)
> +          ROLE_XENSTORE_DOMAIN     (1U<<3)
> +
> +capability
> +  This identifies what system capabilities a domain may have beyond the role it
> +  was assigned.
>    Optional, the default is none.
> 
> -.. note::  The `functions` bits that have been selected to indicate
> -   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last two bits
> -   (30, 31) such that should these features ever be fully retired, the flags may
> -   be dropped without leaving a gap in the flag set.
> +  Format: Bitfield, e.g <3221225487> or <0xC0000007>
> +
> +          CAP_NONE            (0)
> +          CAP_CONSOLE_IO      (1U<<0)
> 
>  mode
>    The mode the domain will be executed under. Required.
> 
> +  Format: Bitfield, e.g <5> or <0x00000005>
> +
> +          MODE_PARAVIRTUALIZED     (1 << 0) PV | PVH/HVM
> +          MODE_ENABLE_DEVICE_MODEL (1 << 1) HVM | PVH
> +          MODE_LONG                (1 << 2) 64 BIT | 32 BIT
> +
>  domain-uuid
>    A globally unique identifier for the domain. Optional,
>    the default is NULL.
> 
> +  Format: Byte Array, e.g [B3 FB 98 FB 8F 9F 67 A3]
> +
>  cpus
>    The number of vCPUs to be assigned to the domain. Optional,
>    the default is “1”.
> 
> +  Format: Integer, e.g <0>
> +
>  memory
> -  The amount of memory to assign to the domain, in KBs.
> +  The amount of memory to assign to the domain, in KBs. This field uses a DTB
> +  Reg which contains a start and size. For memory allocation start may or may
> +  not have significance but size will always be used for the amount of memory
>    Required.
> 
> +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
There is a mismatch between the description and above format:
- KB vs MB
- string vs reg format
- the x86 example uses string and Arm uses reg format

> +
>  security-id
>    The security identity to be assigned to the domain when XSM
>    is the access control mechanism being used. Optional,
> -  the default is “domu_t”.
> +  the default is “system_u:system_r:domU_t”.
> +
> +  Format: string, e.g. "system_u:system_r:domU_t"
This is specifying full label (as expected) whereas the examples use only type

> +
> +Child Nodes
> +"""""""""""
> +
> +* module
> +
> +Module node
> +-----------
> 
> -The Module node
> ----------------
> +This node describes a boot module loaded by the boot loader. A ``module`` node
> +will often appear repeatedly and will require a unique and DTB compliant name
> +for each instance. The compatible property is required to identify that the
> +node is a ``module`` node, the type of boot module, and what it represents.
> 
> -This node describes a boot module loaded by the boot loader. The required
> -compatible property follows the format: module,<type> where type can be
> -“kernel”, “ramdisk”, “device-tree”, “microcode”, “xsm-policy” or “config”. In
> -the case the module is a multiboot module, the additional property string
> -“multiboot,module” may be present. One of two properties is required and
> -identifies how to locate the module. They are the mb-index, used for multiboot
> -modules, and the module-addr for memory address based location.
> +Depending on the type of boot module, the ``module`` node will require either a
> +``module-index`` or ``module-addr`` property must be present. They provide the
> +boot module specific way of locating the boot module in memory.
> +
> +Properties
> +""""""""""
> 
>  compatible
>    This identifies what the module is and thus what the hypervisor
>    should use the module for during domain construction. Required.
> 
> -mb-index
> -  This identifies the index for this module in the multiboot module chain.
> +  Format: "module,<module type>"[, "module,<locating type>"]
> +          module type: kernel, ramdisk, device-tree, microcode, xsm-policy,
> +                       config
> +
> +          locating type: index, addr
> +
> +module-index
> +  This identifies the index for this module when in a module chain.
>    Required for multiboot environments.
> 
> +  Format: Integer, e.g. <0>
> +
>  module-addr
>    This identifies where in memory this module is located. Required for
>    non-multiboot environments.
> 
> +  Format: DTB Reg <start size>, e.g. <0x0 0x20000>
I guess the number of cells for start and size will be taken from #address-celss and #size-cells
defined either in /chosen or a config/domain node?

Also, what is the plan for the existing dom0less dt properties?
Will they need to be moved to new /hypervisor node or we will have to parse both /chosen and /hypervisor nodes?

~Michal
Jan Beulich Aug. 3, 2023, 12:19 p.m. UTC | #2
On 03.08.2023 12:44, Daniel P. Smith wrote:
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -332,6 +332,15 @@ M:	Nick Rosbrook <rosbrookn@gmail.com>
>  S:	Maintained
>  F:	tools/golang
>  
> +HYPERLAUNCH
> +M:	Daniel P. Smith <dpsmith@apertussolutions.com>
> +M:	Christopher Clark <christopher.w.clark@gmail.com>
> +W:	https://wiki.xenproject.org/wiki/Hyperlaunch
> +S:	Supported
> +F:	docs/design/launch/hyperlaunch.rst
> +F:	docs/design/launch/hyperlaunch-devicetree.rst
> +F:	xen/common/domain-builder/

I would generally suggest that maintainership changes come in a separate
patch. Furthermore aiui lots of stuff is going to be moved from elsewhere,
and such code may better stay under its original maintainership (unless
it was agreed that it would shift). So initially maybe best to name the
original maintainers here under M: and add the two of you with R: ?

I also don't think it makes sense to include a not-yet-populated path
here; who knows what this is going to change to by the time things get
committed.

Jan
Daniel P. Smith Aug. 3, 2023, 4:57 p.m. UTC | #3
On 8/3/23 07:45, Michal Orzel wrote:
> Hi Daniel,
> 
> On 03/08/2023 12:44, Daniel P. Smith wrote:
>>
>>
>> With on going development of hyperlaunch, changes to the device tree definitions
>> has been necessary. This commit updates the specification for all current changes
>> along with changes expected to be made in finalizing the capability.
>>
>> This commit also adds a HYPERLAUNCH section to the MAINTAINERS file and places
>> this documentation under its purview. It also reserves the path
>> `xen/common/domain-builder` for the hyperlaunch domain builder code base.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   MAINTAINERS                                   |   9 +
>>   .../designs/launch/hyperlaunch-devicetree.rst | 566 ++++++++++--------
>>   2 files changed, 309 insertions(+), 266 deletions(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d8a02a6c19..694412a961 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -332,6 +332,15 @@ M: Nick Rosbrook <rosbrookn@gmail.com>
>>   S:     Maintained
>>   F:     tools/golang
>>
>> +HYPERLAUNCH
>> +M:     Daniel P. Smith <dpsmith@apertussolutions.com>
>> +M:     Christopher Clark <christopher.w.clark@gmail.com>
>> +W:     https://wiki.xenproject.org/wiki/Hyperlaunch
>> +S:     Supported
>> +F:     docs/design/launch/hyperlaunch.rst
>> +F:     docs/design/launch/hyperlaunch-devicetree.rst
>> +F:     xen/common/domain-builder/
>> +
>>   HYPFS
>>   M:     Juergen Gross <jgross@suse.com>
>>   S:     Supported
>> diff --git a/docs/designs/launch/hyperlaunch-devicetree.rst b/docs/designs/launch/hyperlaunch-devicetree.rst
>> index b49c98cfbd..0bc719e4ae 100644
>> --- a/docs/designs/launch/hyperlaunch-devicetree.rst
>> +++ b/docs/designs/launch/hyperlaunch-devicetree.rst
>> @@ -2,10 +2,11 @@
>>   Xen Hyperlaunch Device Tree Bindings
>>   -------------------------------------
>>
>> -The Xen Hyperlaunch device tree adopts the dom0less device tree structure and
>> -extends it to meet the requirements for the Hyperlaunch capability. The primary
>> -difference is the introduction of the ``hypervisor`` node that is under the
>> -``/chosen`` node. The move to a dedicated node was driven by:
>> +The Xen Hyperlaunch device tree is informed by the dom0less device tree
>> +structure with extensions to meet the requirements for the Hyperlaunch
>> +capability. A major depature from the dom0less device tree is the introduction
>> +of the ``hypervisor`` node that is under the ``/chosen`` node. The move to a
>> +dedicated node was driven by:
>>
>>   1. Reduces the need to walk over nodes that are not of interest, e.g. only
>>      nodes of interest should be in ``/chosen/hypervisor``
>> @@ -13,331 +14,364 @@ difference is the introduction of the ``hypervisor`` node that is under the
>>   2. Allows for the domain construction information to easily be sanitized by
>>      simple removing the ``/chosen/hypervisor`` node.
>>
>> -Example Configuration
>> ----------------------
>> -
>> -Below are two example device tree definitions for the hypervisor node. The
>> -first is an example of a multiboot-based configuration for x86 and the second
>> -is a module-based configuration for Arm.
>> -
>> -Multiboot x86 Configuration:
>> -""""""""""""""""""""""""""""
>> -
>> -::
>> -
>> -    hypervisor {
>> -        #address-cells = <1>;
>> -        #size-cells = <0>;
>> -        compatible = “hypervisor,xen”
>> -
>> -        // Configuration container
>> -        config {
>> -            compatible = "xen,config";
>> -
>> -            module {
>> -                compatible = "module,microcode", "multiboot,module";
>> -                mb-index = <1>;
>> -            };
>> -
>> -            module {
>> -                compatible = "module,xsm-policy", "multiboot,module";
>> -                mb-index = <2>;
>> -            };
>> -        };
>> -
>> -        // Boot Domain definition
>> -        domain {
>> -            compatible = "xen,domain";
>> -
>> -            domid = <0x7FF5>;
>> -
>> -            // FUNCTION_NONE            (0)
>> -            // FUNCTION_BOOT            (1 << 0)
>> -            // FUNCTION_CRASH           (1 << 1)
>> -            // FUNCTION_CONSOLE         (1 << 2)
>> -            // FUNCTION_XENSTORE        (1 << 30)
>> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
>> -            functions = <0x00000001>;
>> -
>> -            memory = <0x0 0x20000>;
>> -            cpus = <1>;
>> -            module {
>> -                compatible = "module,kernel", "multiboot,module";
>> -                mb-index = <3>;
>> -            };
>> -
>> -            module {
>> -                compatible = "module,ramdisk", "multiboot,module";
>> -                mb-index = <4>;
>> -            };
>> -            module {
>> -                compatible = "module,config", "multiboot,module";
>> -                mb-index = <5>;
>> -            };
>> -
>> -        // Classic Dom0 definition
>> -        domain {
>> -            compatible = "xen,domain";
>> -
>> -            domid = <0>;
>> -
>> -            // PERMISSION_NONE          (0)
>> -            // PERMISSION_CONTROL       (1 << 0)
>> -            // PERMISSION_HARDWARE      (1 << 1)
>> -            permissions = <3>;
>> -
>> -            // FUNCTION_NONE            (0)
>> -            // FUNCTION_BOOT            (1 << 0)
>> -            // FUNCTION_CRASH           (1 << 1)
>> -            // FUNCTION_CONSOLE         (1 << 2)
>> -            // FUNCTION_XENSTORE        (1 << 30)
>> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
>> -            functions = <0xC0000006>;
>> -
>> -            // MODE_PARAVIRTUALIZED     (1 << 0) /* PV | PVH/HVM */
>> -            // MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
>> -            // MODE_LONG                (1 << 2) /* 64 BIT | 32 BIT */
>> -            mode = <5>; /* 64 BIT, PV */
>> -
>> -            // UUID
>> -            domain-uuid = [B3 FB 98 FB 8F 9F 67 A3];
>> -
>> -            cpus = <1>;
>> -            memory = <0x0 0x20000>;
>> -            security-id = “dom0_t;
>> -
>> -            module {
>> -                compatible = "module,kernel", "multiboot,module";
>> -                mb-index = <6>;
>> -                bootargs = "console=hvc0";
>> -            };
>> -            module {
>> -                compatible = "module,ramdisk", "multiboot,module";
>> -                mb-index = <7>;
>> -            };
>> -    };
>> -
>> -The multiboot modules supplied when using the above config would be, in order:
>> +The Hypervisor node
>> +-------------------
>>
>> -* (the above config, compiled)
>> -* CPU microcode
>> -* XSM policy
>> -* kernel for boot domain
>> -* ramdisk for boot domain
>> -* boot domain configuration file
>> -* kernel for the classic dom0 domain
>> -* ramdisk for the classic dom0 domain
>> +The ``hypervisor`` node is a top level container for all information relating
>> +to how the hyperlaunch is to proceed. This includes definitions of the domains
>> +that will be built by hypervisor on start up. The node will be named
>> +``hypervisor``  with a ``compatible`` property to identify which hypervisors
>> +the configuration is intended. The hypervisor node will consist of one or more
>> +config nodes and one or more domain nodes.
>>
>> -Module Arm Configuration:
>> -"""""""""""""""""""""""""
>> +Properties
>> +""""""""""
>>
>> -::
>> +compatible
>> +  Identifies which hypervisors the configuration is compatible. Required.
>>
>> -    hypervisor {
>> -        compatible = “hypervisor,xen”
>> +  Format: "hypervisor,<hypervisor name>", e.g "hypervisor,xen"
>>
>> -        // Configuration container
>> -        config {
>> -            compatible = "xen,config";
>> +Child Nodes
>> +"""""""""""
>>
>> -            module {
>> -                compatible = "module,microcode”;
>> -                module-addr = <0x0000ff00 0x80>;
>> -            };
>> +* config
>> +* domain
>>
>> -            module {
>> -                compatible = "module,xsm-policy";
>> -                module-addr = <0x0000ff00 0x80>;
>> +Config Node
>> +-----------
>>
>> -            };
>> -        };
>> +A ``config`` node is for passing configuration data and identifying any boot
>> +modules that is of interest to the hypervisor.  For example this would be where
>> +Xen would be informed of microcode or XSM policy locations. Each ``config``
>> +node will require a unique device-tree compliant name as there may be one or
>> +more ``config`` nodes present in a single dtb file. To identify which
>> +hypervisor the configuration is intended, the required ``compatible`` property
>> +must be present.
>>
>> -        // Boot Domain definition
>> -        domain {
>> -            compatible = "xen,domain";
>> -
>> -            domid = <0x7FF5>;
>> -
>> -            // FUNCTION_NONE            (0)
>> -            // FUNCTION_BOOT            (1 << 0)
>> -            // FUNCTION_CRASH           (1 << 1)
>> -            // FUNCTION_CONSOLE         (1 << 2)
>> -            // FUNCTION_XENSTORE        (1 << 30)
>> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
>> -            functions = <0x00000001>;
>> -
>> -            memory = <0x0 0x20000>;
>> -            cpus = <1>;
>> -            module {
>> -                compatible = "module,kernel";
>> -                module-addr = <0x0000ff00 0x80>;
>> -            };
>> +While the config node is not meant to replace the hypervisor commandline, there
>> +may be cases where it is better suited for passing configuration details at
>> +boot time.  This additional information may be carried in properties assigned
>> +to a ``config`` node. If there are any boot modules that are intended for the
>> +hypervisor, then a ``module`` child node should be provided to identify the
>> +boot module.
>>
>> -            module {
>> -                compatible = "module,ramdisk";
>> -                module-addr = <0x0000ff00 0x80>;
>> -            };
>> -            module {
>> -                compatible = "module,config";
>> -                module-addr = <0x0000ff00 0x80>;
>> -            };
>> +Properties
>> +""""""""""
>>
>> -        // Classic Dom0 definition
>> -        domain@0 {
>> -            compatible = "xen,domain";
>> -
>> -            domid = <0>;
>> -
>> -            // PERMISSION_NONE          (0)
>> -            // PERMISSION_CONTROL       (1 << 0)
>> -            // PERMISSION_HARDWARE      (1 << 1)
>> -            permissions = <3>;
>> -
>> -            // FUNCTION_NONE            (0)
>> -            // FUNCTION_BOOT            (1 << 0)
>> -            // FUNCTION_CRASH           (1 << 1)
>> -            // FUNCTION_CONSOLE         (1 << 2)
>> -            // FUNCTION_XENSTORE        (1 << 30)
>> -            // FUNCTION_LEGACY_DOM0     (1 << 31)
>> -            functions = <0xC0000006>;
>> -
>> -            // MODE_PARAVIRTUALIZED     (1 << 0) /* PV | PVH/HVM */
>> -            // MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
>> -            // MODE_LONG                (1 << 2) /* 64 BIT | 32 BIT */
>> -            mode = <5>; /* 64 BIT, PV */
>> -
>> -            // UUID
>> -            domain-uuid = [B3 FB 98 FB 8F 9F 67 A3];
>> -
>> -            cpus = <1>;
>> -            memory = <0x0 0x20000>;
>> -            security-id = “dom0_t”;
>> -
>> -            module {
>> -                compatible = "module,kernel";
>> -                module-addr = <0x0000ff00 0x80>;
>> -                bootargs = "console=hvc0";
>> -            };
>> -            module {
>> -                compatible = "module,ramdisk";
>> -                module-addr = <0x0000ff00 0x80>;
>> -            };
>> -    };
>> +compatible
>> +  Identifies the hypervisor the confiugration is intended. Required.
>>
>> -The modules that would be supplied when using the above config would be:
>> +  Format: "<hypervisor name>,config", e.g "xen,config"
>>
>> -* (the above config, compiled into hardware tree)
>> -* CPU microcode
>> -* XSM policy
>> -* kernel for boot domain
>> -* ramdisk for boot domain
>> -* boot domain configuration file
>> -* kernel for the classic dom0 domain
>> -* ramdisk for the classic dom0 domain
>> +bootargs
>> +  This is used to provide the boot params for Xen.
>>
>> -The hypervisor device tree would be compiled into the hardware device tree and
>> -provided to Xen using the standard method currently in use. The remaining
>> -modules would need to be loaded in the respective addresses specified in the
>> -`module-addr` property.
>> +  Format: String, e.g. "flask=silo"
>>
>> +Child Nodes
>> +"""""""""""
>>
>> -The Hypervisor node
>> --------------------
>> +* module
>>
>> -The hypervisor node is a top level container for the domains that will be built
>> -by hypervisor on start up. On the ``hypervisor`` node the ``compatible``
>> -property is used to identify the type of hypervisor node present..
>> +Domain Node
>> +-----------
>>
>> -compatible
>> -  Identifies the type of node. Required.
>> +A ``domain`` node is for describing the construction of a domain. Since there
>> +may be one or more domain nodes, each one requires a unique, DTB compliant name
>> +and a ``compatible`` property to identify as a domain node.
>>
>> -The Config node
>> ----------------
>> +A ``domain`` node  may provide a ``domid`` property which will be used as the
>> +requested domain id for the domain with a value of “0” signifying to use the
>> +next available domain id, which is the default behavior if omitted. It should
>> +be noted that a domain configuration is not able to request a domid of “0”.
>> +Beyond that, a domain node may have any of the following optional properties.
>>
>> -A config node is for detailing any modules that are of interest to Xen itself.
>> -For example this would be where Xen would be informed of microcode or XSM
>> -policy locations. If the modules are multiboot modules and are able to be
>> -located by index within the module chain, the ``mb-index`` property should be
>> -used to specify the index in the multiboot module chain.. If the module will be
>> -located by physical memory address, then the ``module-addr`` property should be
>> -used to identify the location and size of the module.
>> +Properties
>> +""""""""""
>>
>>   compatible
>> -  Identifies the type of node. Required.
>> -
>> -The Domain node
>> ----------------
>> +  Identifies the node as a domain node and for which hypervisor. Required.
>>
>> -A domain node is for describing the construction of a domain. It may provide a
>> -domid property which will be used as the requested domain id for the domain
>> -with a value of “0” signifying to use the next available domain id, which is
>> -the default behavior if omitted. A domain configuration is not able to request
>> -a domid of “0”. After that a domain node may have any of the following
>> -parameters,
>> -
>> -compatible
>> -  Identifies the type of node. Required.
>> +  Format: "<hypervisor name>,domain", e.g "xen,domain"
>>
>>   domid
>> -  Identifies the domid requested to assign to the domain. Required.
>> +  Identifies the domid requested to assign to the domain.
>>
>> -permissions
>> +  Format: Integer, e.g <0>
>> +
>> +role
>>     This sets what Discretionary Access Control permissions
>>     a domain is assigned. Optional, default is none.
>>
>> -functions
>> -  This identifies what system functions a domain will fulfill.
>> +  Format: Bitfield, e.g <3> or <0x00000003>
>> +
>> +          ROLE_NONE                (0)
>> +          ROLE_UNBOUNDED_DOMAIN    (1U<<0)
>> +          ROLE_CONTROL_DOMAIN      (1U<<1)
>> +          ROLE_HARDWARE_DOMAIN     (1U<<2)
>> +          ROLE_XENSTORE_DOMAIN     (1U<<3)
>> +
>> +capability
>> +  This identifies what system capabilities a domain may have beyond the role it
>> +  was assigned.
>>     Optional, the default is none.
>>
>> -.. note::  The `functions` bits that have been selected to indicate
>> -   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last two bits
>> -   (30, 31) such that should these features ever be fully retired, the flags may
>> -   be dropped without leaving a gap in the flag set.
>> +  Format: Bitfield, e.g <3221225487> or <0xC0000007>
>> +
>> +          CAP_NONE            (0)
>> +          CAP_CONSOLE_IO      (1U<<0)
>>
>>   mode
>>     The mode the domain will be executed under. Required.
>>
>> +  Format: Bitfield, e.g <5> or <0x00000005>
>> +
>> +          MODE_PARAVIRTUALIZED     (1 << 0) PV | PVH/HVM
>> +          MODE_ENABLE_DEVICE_MODEL (1 << 1) HVM | PVH
>> +          MODE_LONG                (1 << 2) 64 BIT | 32 BIT
>> +
>>   domain-uuid
>>     A globally unique identifier for the domain. Optional,
>>     the default is NULL.
>>
>> +  Format: Byte Array, e.g [B3 FB 98 FB 8F 9F 67 A3]
>> +
>>   cpus
>>     The number of vCPUs to be assigned to the domain. Optional,
>>     the default is “1”.
>>
>> +  Format: Integer, e.g <0>
>> +
>>   memory
>> -  The amount of memory to assign to the domain, in KBs.
>> +  The amount of memory to assign to the domain, in KBs. This field uses a DTB
>> +  Reg which contains a start and size. For memory allocation start may or may
>> +  not have significance but size will always be used for the amount of memory
>>     Required.
>>
>> +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
> There is a mismatch between the description and above format:
> - KB vs MB
> - string vs reg format
> - the x86 example uses string and Arm uses reg format

Hmmm. I missed this in the hyperlaunch v1 update. In the original design 
that came from the working group it was going to use a reg as suggest by 
dom0less. During development of v1, we found it was not rich enough to 
express how Dom0 could be allocated memory and switched to a string to 
mirror the dom0 memory hypervisor command line parameter.

A question for those involved with dom0less, is what are the opinions 
about using this form for memory allocation. Is it required/possible to 
be able to instruct the hypervisor what physical address to use as the 
start of a domain's memory?

>> +
>>   security-id
>>     The security identity to be assigned to the domain when XSM
>>     is the access control mechanism being used. Optional,
>> -  the default is “domu_t”.
>> +  the default is “system_u:system_r:domU_t”.
>> +
>> +  Format: string, e.g. "system_u:system_r:domU_t"
> This is specifying full label (as expected) whereas the examples use only type

Ack, the examples needs fixing.

>> +
>> +Child Nodes
>> +"""""""""""
>> +
>> +* module
>> +
>> +Module node
>> +-----------
>>
>> -The Module node
>> ----------------
>> +This node describes a boot module loaded by the boot loader. A ``module`` node
>> +will often appear repeatedly and will require a unique and DTB compliant name
>> +for each instance. The compatible property is required to identify that the
>> +node is a ``module`` node, the type of boot module, and what it represents.
>>
>> -This node describes a boot module loaded by the boot loader. The required
>> -compatible property follows the format: module,<type> where type can be
>> -“kernel”, “ramdisk”, “device-tree”, “microcode”, “xsm-policy” or “config”. In
>> -the case the module is a multiboot module, the additional property string
>> -“multiboot,module” may be present. One of two properties is required and
>> -identifies how to locate the module. They are the mb-index, used for multiboot
>> -modules, and the module-addr for memory address based location.
>> +Depending on the type of boot module, the ``module`` node will require either a
>> +``module-index`` or ``module-addr`` property must be present. They provide the
>> +boot module specific way of locating the boot module in memory.
>> +
>> +Properties
>> +""""""""""
>>
>>   compatible
>>     This identifies what the module is and thus what the hypervisor
>>     should use the module for during domain construction. Required.
>>
>> -mb-index
>> -  This identifies the index for this module in the multiboot module chain.
>> +  Format: "module,<module type>"[, "module,<locating type>"]
>> +          module type: kernel, ramdisk, device-tree, microcode, xsm-policy,
>> +                       config
>> +
>> +          locating type: index, addr
>> +
>> +module-index
>> +  This identifies the index for this module when in a module chain.
>>     Required for multiboot environments.
>>
>> +  Format: Integer, e.g. <0>
>> +
>>   module-addr
>>     This identifies where in memory this module is located. Required for
>>     non-multiboot environments.
>>
>> +  Format: DTB Reg <start size>, e.g. <0x0 0x20000>
> I guess the number of cells for start and size will be taken from #address-celss and #size-cells
> defined either in /chosen or a config/domain node?

Correct, in the working group, that is what we were informed was the 
desired approach.

> Also, what is the plan for the existing dom0less dt properties?
> Will they need to be moved to new /hypervisor node or we will have to parse both /chosen and /hypervisor nodes?

In the proposal I sent to xen-devel in response to Luca's RFC for 
rebranding dom0less features under hyperlaunch, that is the purpose of 
this commit. Get this document up to date with what was done in v1 along 
with what we are planning/working on for hyperlaunch. One could think of 
this as effectively the API to the capabilities hyperlaunch will 
provide. Not just how to construct a domain, but what kinds of domains 
can be constructed by hyperlaunch. Step one of the proposal is to 
publish a patch upon which we all can iterate over and get to an 
agreement on a suitable interface for all. The next step would be the 
introduction of hyperlaunch dom0less compatibility mode, that would see 
the moving of the parsing logic for the existing dom0less nodes under 
/xen/common/domain-builder. It would continue to exist there even after 
hyperlaunch proper is merged and can remain there for backward 
compatibility until there is a decision to retire the compatibility 
interface.

v/r,
dps
Daniel P. Smith Aug. 3, 2023, 5:17 p.m. UTC | #4
On 8/3/23 08:19, Jan Beulich wrote:
> On 03.08.2023 12:44, Daniel P. Smith wrote:
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -332,6 +332,15 @@ M:	Nick Rosbrook <rosbrookn@gmail.com>
>>   S:	Maintained
>>   F:	tools/golang
>>   
>> +HYPERLAUNCH
>> +M:	Daniel P. Smith <dpsmith@apertussolutions.com>
>> +M:	Christopher Clark <christopher.w.clark@gmail.com>
>> +W:	https://wiki.xenproject.org/wiki/Hyperlaunch
>> +S:	Supported
>> +F:	docs/design/launch/hyperlaunch.rst
>> +F:	docs/design/launch/hyperlaunch-devicetree.rst
>> +F:	xen/common/domain-builder/
> 
> I would generally suggest that maintainership changes come in a separate
> patch. Furthermore aiui lots of stuff is going to be moved from elsewhere,
> and such code may better stay under its original maintainership (unless
> it was agreed that it would shift). So initially maybe best to name the
> original maintainers here under M: and add the two of you with R: ?

I can do this as a separate patch and mark it as fix for `d4f3125f1b 
docs/designs/launch: Hyperlaunch design document` where Christopher and 
I are the original authors of the only existing files covered under this 
new MAINTAINERSHIP entry currently.

As far as code moving here, the dom0less rebranding proposal called for 
an additional MAINTAINERS section titled HYPERLAUNCH DOM0LESS 
COMPATIBILITY that would retain the maintainers (or new ones if Arm 
wanted to propose others) from the ARM section.

The purpose of putting Christopher and I at the top are for several reasons,

  - The code in v1 was conglomerations of reuses/relocated code and a 
substantial amount of new code around it.

- As mentioned regarding the HYPERLAUNCH DOM0LESS COMPATIBILITY section, 
there may be paths below HYPERLAUNCH that are owned by others, but 
ultimately we conceived, designed, and created the capability. So it 
falls on us to ensure anything done in sub-feature, doesn't break or 
violate the larger design we sought to achieve while also not letting it 
fall back on THE REST.

> I also don't think it makes sense to include a not-yet-populated path
> here; who knows what this is going to change to by the time things get
> committed.

Well, if my proposed plan is executed as I suggested, hopefully there 
would be a series very soon that would move the dom0less device tree 
parsing under that path. Now to be inline with the above and address 
your concern, as the HYPERLAUNCH DOM0LESS COMPATIBILITY section is added 
to cover the file(s) the series adds, the HYPERLAUNCH section could then 
be updated with the top level path.

v/r,
dps
Stefano Stabellini Aug. 3, 2023, 11:38 p.m. UTC | #5
On Thu, 3 Aug 2023, Daniel P. Smith wrote:
> > Also, what is the plan for the existing dom0less dt properties?
> > Will they need to be moved to new /hypervisor node or we will have to parse
> > both /chosen and /hypervisor nodes?
> 
> In the proposal I sent to xen-devel in response to Luca's RFC for rebranding
> dom0less features under hyperlaunch, that is the purpose of this commit. Get
> this document up to date with what was done in v1 along with what we are
> planning/working on for hyperlaunch. One could think of this as effectively
> the API to the capabilities hyperlaunch will provide. Not just how to
> construct a domain, but what kinds of domains can be constructed by
> hyperlaunch. Step one of the proposal is to publish a patch upon which we all
> can iterate over and get to an agreement on a suitable interface for all. The
> next step would be the introduction of hyperlaunch dom0less compatibility
> mode, that would see the moving of the parsing logic for the existing dom0less
> nodes under /xen/common/domain-builder. It would continue to exist there even
> after hyperlaunch proper is merged and can remain there for backward
> compatibility until there is a decision to retire the compatibility interface.

I like this plan. The two interfaces are so similar that it is basically
one interface with a couple of tiny differences.

So I expect we would move the existing dom0less parsing code to common/,
add a couple of extensions (such as parsing /hypervisor in addition to
/chosen) and use it as it.

Later on, after a few years of using /hypervisor instead of /chosen, if
nobody is using /chosen anymore, we could retire /chosen completely. But
this is just one DT node/property that gets retired (there are a couple
of others). I don't imagine we'll have a full new implementation of the
DT parsing logic that supersedes the existing implementation of it
(especially considering the difficulty of maintaining 2 different parsing
logics in the hypervisor for similar interfaces).

Same thing for the DT interface documentation. I don't think we need two
DT interface docs? We could start with the existing dom0less interface
(docs/misc/arm/device-tree/booting.txt), and move it somewhere common
like docs/misc/device-tree.

Then add any changes or extensions required by other architecture, such
as x86 and RISC-V.

For sure for x86 we need "module-index". I don't know if anything else
is must-have to get it to work on x86 but if there is, we should add
those too.
Stefano Stabellini Aug. 4, 2023, 12:09 a.m. UTC | #6
On Thu, 3 Aug 2023, Stefano Stabellini wrote:
> On Thu, 3 Aug 2023, Daniel P. Smith wrote:
> > > Also, what is the plan for the existing dom0less dt properties?
> > > Will they need to be moved to new /hypervisor node or we will have to parse
> > > both /chosen and /hypervisor nodes?
> > 
> > In the proposal I sent to xen-devel in response to Luca's RFC for rebranding
> > dom0less features under hyperlaunch, that is the purpose of this commit. Get
> > this document up to date with what was done in v1 along with what we are
> > planning/working on for hyperlaunch. One could think of this as effectively
> > the API to the capabilities hyperlaunch will provide. Not just how to
> > construct a domain, but what kinds of domains can be constructed by
> > hyperlaunch. Step one of the proposal is to publish a patch upon which we all
> > can iterate over and get to an agreement on a suitable interface for all. The
> > next step would be the introduction of hyperlaunch dom0less compatibility
> > mode, that would see the moving of the parsing logic for the existing dom0less
> > nodes under /xen/common/domain-builder. It would continue to exist there even
> > after hyperlaunch proper is merged and can remain there for backward
> > compatibility until there is a decision to retire the compatibility interface.
> 
> I like this plan. The two interfaces are so similar that it is basically
> one interface with a couple of tiny differences.
> 
> So I expect we would move the existing dom0less parsing code to common/,
> add a couple of extensions (such as parsing /hypervisor in addition to
> /chosen) and use it as it.
> 
> Later on, after a few years of using /hypervisor instead of /chosen, if
> nobody is using /chosen anymore, we could retire /chosen completely. But
> this is just one DT node/property that gets retired (there are a couple
> of others). I don't imagine we'll have a full new implementation of the
> DT parsing logic that supersedes the existing implementation of it
> (especially considering the difficulty of maintaining 2 different parsing
> logics in the hypervisor for similar interfaces).
> 
> Same thing for the DT interface documentation. I don't think we need two
> DT interface docs? We could start with the existing dom0less interface
> (docs/misc/arm/device-tree/booting.txt), and move it somewhere common
> like docs/misc/device-tree.
> 
> Then add any changes or extensions required by other architecture, such
> as x86 and RISC-V.
> 
> For sure for x86 we need "module-index". I don't know if anything else
> is must-have to get it to work on x86 but if there is, we should add
> those too.


For clarity, I think we should definitely have
docs/design/launch/hyperlaunch.rst, and maybe we should also have
hyperlaunch-devicetree.rst as an overview description and user guide.
That's useful.

But in terms of official device tree bindings interface description
(basically what in Linux would go under
Documentation/devicetree/bindings/), I think it would be best to only
have a single document. The current one is
docs/misc/arm/device-tree/booting.txt.
Michal Orzel Aug. 4, 2023, 7:06 a.m. UTC | #7
Hi Daniel,

On 03/08/2023 18:57, Daniel P. Smith wrote:
> 
> 
> On 8/3/23 07:45, Michal Orzel wrote:
>> Hi Daniel,
>>
>> On 03/08/2023 12:44, Daniel P. Smith wrote:
>>>
>>>
>>> With on going development of hyperlaunch, changes to the device tree definitions
>>> has been necessary. This commit updates the specification for all current changes
>>> along with changes expected to be made in finalizing the capability.
>>>
>>> This commit also adds a HYPERLAUNCH section to the MAINTAINERS file and places
>>> this documentation under its purview. It also reserves the path
>>> `xen/common/domain-builder` for the hyperlaunch domain builder code base.
>>>
>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

[...]
>>> +
>>>   memory
>>> -  The amount of memory to assign to the domain, in KBs.
>>> +  The amount of memory to assign to the domain, in KBs. This field uses a DTB
>>> +  Reg which contains a start and size. For memory allocation start may or may
>>> +  not have significance but size will always be used for the amount of memory
>>>     Required.
>>>
>>> +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
>> There is a mismatch between the description and above format:
>> - KB vs MB
>> - string vs reg format
>> - the x86 example uses string and Arm uses reg format
> 
> Hmmm. I missed this in the hyperlaunch v1 update. In the original design
> that came from the working group it was going to use a reg as suggest by
> dom0less. During development of v1, we found it was not rich enough to
> express how Dom0 could be allocated memory and switched to a string to
> mirror the dom0 memory hypervisor command line parameter.
On Arm, dom0_mem cmdline parameter is used to specify only size (no min,max)

> 
> A question for those involved with dom0less, is what are the opinions
> about using this form for memory allocation. Is it required/possible to
> be able to instruct the hypervisor what physical address to use as the
> start of a domain's memory?
"memory" dt property is used to specify just amount of memory for domain in KBs using reg format.
It is not used to specify the static memory region (with start and size). For that, we have another property called "xen,static-mem".
Therefore, it would be possible to switch memory to string but it would not be compatible with the current use anymore.

~Michal
Julien Grall Aug. 4, 2023, 9:03 a.m. UTC | #8
Hi Daniel,

On 03/08/2023 11:44, Daniel P. Smith wrote:
> +compatible
> +  Identifies which hypervisors the configuration is compatible. Required.
>   
> -    hypervisor {
> -        compatible = “hypervisor,xen”
> +  Format: "hypervisor,<hypervisor name>", e.g "hypervisor,xen"

I read "e.g" as "for example". You don't explicitely say which 
compatible will be supported by Xen, so one could write "hypervisor,foo" 
and expect to work.

Also, it is not fully clear why you need both the hypervisor and each 
domain node to have a compatible with the hypervisor name in it.

[...]

> +compatible
> +  Identifies the hypervisor the confiugration is intended. Required.

Also typo: s/confiugration/configuration/

>   
> -The modules that would be supplied when using the above config would be:
> +  Format: "<hypervisor name>,config", e.g "xen,config"
>   
> -* (the above config, compiled into hardware tree)
> -* CPU microcode
> -* XSM policy
> -* kernel for boot domain
> -* ramdisk for boot domain
> -* boot domain configuration file
> -* kernel for the classic dom0 domain
> -* ramdisk for the classic dom0 domain
> +bootargs
> +  This is used to provide the boot params for Xen.

How is this different from the command line parameter chosen? And if you 
want to keep both, what is the expected priority if a user provides both?

>   
> -The hypervisor device tree would be compiled into the hardware device tree and
> -provided to Xen using the standard method currently in use. The remaining
> -modules would need to be loaded in the respective addresses specified in the
> -`module-addr` property.
> +  Format: String, e.g. "flask=silo"
>   
> +Child Nodes
> +"""""""""""
>   
> -The Hypervisor node
> --------------------
> +* module
>   
> -The hypervisor node is a top level container for the domains that will be built
> -by hypervisor on start up. On the ``hypervisor`` node the ``compatible``
> -property is used to identify the type of hypervisor node present..
> +Domain Node
> +-----------
>   
> -compatible
> -  Identifies the type of node. Required.
> +A ``domain`` node is for describing the construction of a domain. Since there
> +may be one or more domain nodes, each one requires a unique, DTB compliant name
> +and a ``compatible`` property to identify as a domain node.
>   
> -The Config node
> ----------------
> +A ``domain`` node  may provide a ``domid`` property which will be used as the
> +requested domain id for the domain with a value of “0” signifying to use the
> +next available domain id, which is the default behavior if omitted. It should
> +be noted that a domain configuration is not able to request a domid of “0”

Why do you need this restriction? And more importantly how would you 
describe dom0 in hyperlaunch?

> +Beyond that, a domain node may have any of the following optional properties.
>   
> -A config node is for detailing any modules that are of interest to Xen itself.
> -For example this would be where Xen would be informed of microcode or XSM
> -policy locations. If the modules are multiboot modules and are able to be
> -located by index within the module chain, the ``mb-index`` property should be
> -used to specify the index in the multiboot module chain.. If the module will be
> -located by physical memory address, then the ``module-addr`` property should be
> -used to identify the location and size of the module.
> +Properties
> +""""""""""
>   
>   compatible
> -  Identifies the type of node. Required.
> -
> -The Domain node
> ----------------
> +  Identifies the node as a domain node and for which hypervisor. Required.
>   
> -A domain node is for describing the construction of a domain. It may provide a
> -domid property which will be used as the requested domain id for the domain
> -with a value of “0” signifying to use the next available domain id, which is
> -the default behavior if omitted. A domain configuration is not able to request
> -a domid of “0”. After that a domain node may have any of the following
> -parameters,
> -
> -compatible
> -  Identifies the type of node. Required.
> +  Format: "<hypervisor name>,domain", e.g "xen,domain"
>   
>   domid
> -  Identifies the domid requested to assign to the domain. Required.
> +  Identifies the domid requested to assign to the domain.
>   
> -permissions
> +  Format: Integer, e.g <0>
> +
> +role
>     This sets what Discretionary Access Control permissions
>     a domain is assigned. Optional, default is none.
>   
> -functions
> -  This identifies what system functions a domain will fulfill.
> +  Format: Bitfield, e.g <3> or <0x00000003>
> +
> +          ROLE_NONE                (0)
> +          ROLE_UNBOUNDED_DOMAIN    (1U<<0)
> +          ROLE_CONTROL_DOMAIN      (1U<<1)
> +          ROLE_HARDWARE_DOMAIN     (1U<<2)
> +          ROLE_XENSTORE_DOMAIN     (1U<<3)

Please describe what each roles are meant for.

> +
> +capability
> +  This identifies what system capabilities a domain may have beyond the role it
> +  was assigned.
>     Optional, the default is none.
>   
> -.. note::  The `functions` bits that have been selected to indicate
> -   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last two bits
> -   (30, 31) such that should these features ever be fully retired, the flags may
> -   be dropped without leaving a gap in the flag set.
> +  Format: Bitfield, e.g <3221225487> or <0xC0000007>

I thik we should favor the hexadecimal version because this will be 
somewhat easier to read.

Also, the Device-Tree values work in term of 32-bit cell. Also, how do 
you plan to handle the case where you have more than 32 capabilities?

> +
> +          CAP_NONE            (0)
> +          CAP_CONSOLE_IO      (1U<<0)

Please describe the capabilities.

>   
>   mode
>     The mode the domain will be executed under. Required.
>   
> +  Format: Bitfield, e.g <5> or <0x00000005>
> +
> +          MODE_PARAVIRTUALIZED     (1 << 0) PV | PVH/HVM
> +          MODE_ENABLE_DEVICE_MODEL (1 << 1) HVM | PVH
> +          MODE_LONG                (1 << 2) 64 BIT | 32 BIT
> +
>   domain-uuid
>     A globally unique identifier for the domain. Optional,
>     the default is NULL.
>   
> +  Format: Byte Array, e.g [B3 FB 98 FB 8F 9F 67 A3]
> +
>   cpus
>     The number of vCPUs to be assigned to the domain. Optional,
>     the default is “1”.
>   
> +  Format: Integer, e.g <0>

This is odd to suggest to give '0' as an example. Wouldn't Xen throw an 
error if a user provide it?

> +
>   memory
> -  The amount of memory to assign to the domain, in KBs.
> +  The amount of memory to assign to the domain, in KBs. This field uses a DTB
> +  Reg which contains a start and size. For memory allocation start may or may
> +  not have significance but size will always be used for the amount of memory
>     Required.

The description doesn't match...

>   
> +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"

... the format. But strings are difficult to parse. If you want to 
provide 3 different values (possibly optional), then it would be best to 
have 3 different properties.

I will continue to review the rest later.

Cheers,
Daniel P. Smith Aug. 8, 2023, 5:22 p.m. UTC | #9
On 8/3/23 19:38, Stefano Stabellini wrote:
> On Thu, 3 Aug 2023, Daniel P. Smith wrote:
>>> Also, what is the plan for the existing dom0less dt properties?
>>> Will they need to be moved to new /hypervisor node or we will have to parse
>>> both /chosen and /hypervisor nodes?
>>
>> In the proposal I sent to xen-devel in response to Luca's RFC for rebranding
>> dom0less features under hyperlaunch, that is the purpose of this commit. Get
>> this document up to date with what was done in v1 along with what we are
>> planning/working on for hyperlaunch. One could think of this as effectively
>> the API to the capabilities hyperlaunch will provide. Not just how to
>> construct a domain, but what kinds of domains can be constructed by
>> hyperlaunch. Step one of the proposal is to publish a patch upon which we all
>> can iterate over and get to an agreement on a suitable interface for all. The
>> next step would be the introduction of hyperlaunch dom0less compatibility
>> mode, that would see the moving of the parsing logic for the existing dom0less
>> nodes under /xen/common/domain-builder. It would continue to exist there even
>> after hyperlaunch proper is merged and can remain there for backward
>> compatibility until there is a decision to retire the compatibility interface.
> 
> I like this plan. The two interfaces are so similar that it is basically
> one interface with a couple of tiny differences.
> 
> So I expect we would move the existing dom0less parsing code to common/,
> add a couple of extensions (such as parsing /hypervisor in addition to
> /chosen) and use it as it.

Do you mean /chosen/hypervisor?

> Later on, after a few years of using /hypervisor instead of /chosen, if
> nobody is using /chosen anymore, we could retire /chosen completely. But
> this is just one DT node/property that gets retired (there are a couple
> of others). I don't imagine we'll have a full new implementation of the
> DT parsing logic that supersedes the existing implementation of it
> (especially considering the difficulty of maintaining 2 different parsing
> logics in the hypervisor for similar interfaces).

+1

> Same thing for the DT interface documentation. I don't think we need two
> DT interface docs? We could start with the existing dom0less interface
> (docs/misc/arm/device-tree/booting.txt), and move it somewhere common
> like docs/misc/device-tree.

So in the plan that I sent, patch series 3 was were I was going to 
consolidate docs/design/launch/hyperlaunch-devicetree.rst and 
docs/misc/arm/device-tree/booting.txt into a single document under 
docs/features/hyperlaunch/device-tree.rst along with moving 
docs/features/dom0less.pandoc to 
docs/features/hyperlaunch/dom0less-compatibility-mode.pandoc. The 
thinking here was to get all the feature documentation together in a 
single place, but I would be open to putting the suggested consolidated 
device-tree.rst mentioned above into 
docs/misc/device-tree/hyperlaunch.rst if that is preferred.

> Then add any changes or extensions required by other architecture, such
> as x86 and RISC-V.
> 
> For sure for x86 we need "module-index". I don't know if anything else
> is must-have to get it to work on x86 but if there is, we should add
> those too.

Hmm good point, since in the suggested patch series 3 from the plan, 
this probably should get cropped down to what we actually have 
implemented for x86 hyperlaunch and then get expanded as it becomes 
feature complete.

v/r,
dps
Daniel P. Smith Aug. 8, 2023, 7:53 p.m. UTC | #10
On 8/3/23 20:09, Stefano Stabellini wrote:
> On Thu, 3 Aug 2023, Stefano Stabellini wrote:
>> On Thu, 3 Aug 2023, Daniel P. Smith wrote:
>>>> Also, what is the plan for the existing dom0less dt properties?
>>>> Will they need to be moved to new /hypervisor node or we will have to parse
>>>> both /chosen and /hypervisor nodes?
>>>
>>> In the proposal I sent to xen-devel in response to Luca's RFC for rebranding
>>> dom0less features under hyperlaunch, that is the purpose of this commit. Get
>>> this document up to date with what was done in v1 along with what we are
>>> planning/working on for hyperlaunch. One could think of this as effectively
>>> the API to the capabilities hyperlaunch will provide. Not just how to
>>> construct a domain, but what kinds of domains can be constructed by
>>> hyperlaunch. Step one of the proposal is to publish a patch upon which we all
>>> can iterate over and get to an agreement on a suitable interface for all. The
>>> next step would be the introduction of hyperlaunch dom0less compatibility
>>> mode, that would see the moving of the parsing logic for the existing dom0less
>>> nodes under /xen/common/domain-builder. It would continue to exist there even
>>> after hyperlaunch proper is merged and can remain there for backward
>>> compatibility until there is a decision to retire the compatibility interface.
>>
>> I like this plan. The two interfaces are so similar that it is basically
>> one interface with a couple of tiny differences.
>>
>> So I expect we would move the existing dom0less parsing code to common/,
>> add a couple of extensions (such as parsing /hypervisor in addition to
>> /chosen) and use it as it.
>>
>> Later on, after a few years of using /hypervisor instead of /chosen, if
>> nobody is using /chosen anymore, we could retire /chosen completely. But
>> this is just one DT node/property that gets retired (there are a couple
>> of others). I don't imagine we'll have a full new implementation of the
>> DT parsing logic that supersedes the existing implementation of it
>> (especially considering the difficulty of maintaining 2 different parsing
>> logics in the hypervisor for similar interfaces).
>>
>> Same thing for the DT interface documentation. I don't think we need two
>> DT interface docs? We could start with the existing dom0less interface
>> (docs/misc/arm/device-tree/booting.txt), and move it somewhere common
>> like docs/misc/device-tree.
>>
>> Then add any changes or extensions required by other architecture, such
>> as x86 and RISC-V.
>>
>> For sure for x86 we need "module-index". I don't know if anything else
>> is must-have to get it to work on x86 but if there is, we should add
>> those too.
> 
> 
> For clarity, I think we should definitely have
> docs/design/launch/hyperlaunch.rst, and maybe we should also have
> hyperlaunch-devicetree.rst as an overview description and user guide.
> That's useful.
> 
> But in terms of official device tree bindings interface description
> (basically what in Linux would go under
> Documentation/devicetree/bindings/), I think it would be best to only
> have a single document. The current one is
> docs/misc/arm/device-tree/booting.txt.

Does the proposal to your first message align with your follow-up here?

v/r,
dps
Daniel P. Smith Aug. 8, 2023, 8:49 p.m. UTC | #11
On 8/4/23 05:03, Julien Grall wrote:
> Hi Daniel,
> 
> On 03/08/2023 11:44, Daniel P. Smith wrote:
>> +compatible
>> +  Identifies which hypervisors the configuration is compatible. 
>> Required.
>> -    hypervisor {
>> -        compatible = “hypervisor,xen”
>> +  Format: "hypervisor,<hypervisor name>", e.g "hypervisor,xen"
> 
> I read "e.g" as "for example". You don't explicitely say which 
> compatible will be supported by Xen, so one could write "hypervisor,foo" 
> and expect to work.
> 
> Also, it is not fully clear why you need both the hypervisor and each 
> domain node to have a compatible with the hypervisor name in it.

Ack, it should be explicit to what is expected for a Xen configuration. 
As for compatible on the domain node, I think that was from being overly 
cautious, it can be dropped.

This did get me reflecting that the compatibility was added there as 
there was some initial participation in standardization efforts going on 
at the time. I am not sure what has come of those, but the question it 
raised is that domain is a Xen specific term, whereas generally vm is 
accepted as the generic term. Maybe this node needs renaming?

>> +compatible
>> +  Identifies the hypervisor the confiugration is intended. Required.
> 
> Also typo: s/confiugration/configuration/

Ack.

>> -The modules that would be supplied when using the above config would be:
>> +  Format: "<hypervisor name>,config", e.g "xen,config"
>> -* (the above config, compiled into hardware tree)
>> -* CPU microcode
>> -* XSM policy
>> -* kernel for boot domain
>> -* ramdisk for boot domain
>> -* boot domain configuration file
>> -* kernel for the classic dom0 domain
>> -* ramdisk for the classic dom0 domain
>> +bootargs
>> +  This is used to provide the boot params for Xen.
> 
> How is this different from the command line parameter chosen? And if you 
> want to keep both, what is the expected priority if a user provides both?

A DT file for x86, there is only a need to provide the hypervisor node, 
for which we already needed a hypervisor config section to describe XSM 
policy and microcode, at this point at least. It was decided in an 
effort to provide flexibility, the ability to specify command line 
parameters to the hypervisor in DT config. It is expected these 
parameters would function as a base parameters that would be overridden 
by those provided via the multiboot kernel entry.

Now as you point out for Arm, this becomes a bit redundant, to a degree, 
as the Xen command line is already under the /chosen node. But even here 
it would be beneficial, as a hyperlaunch configuration could be 
distributed consisting of a dtb that has core parameters set with base 
values along with a set of kernels and ramdisks. At boot, the 
hyperlaunch dtb could then be concatenated with the device specific dtb.


>> -The hypervisor device tree would be compiled into the hardware device 
>> tree and
>> -provided to Xen using the standard method currently in use. The 
>> remaining
>> -modules would need to be loaded in the respective addresses specified 
>> in the
>> -`module-addr` property.
>> +  Format: String, e.g. "flask=silo"
>> +Child Nodes
>> +"""""""""""
>> -The Hypervisor node
>> --------------------
>> +* module
>> -The hypervisor node is a top level container for the domains that 
>> will be built
>> -by hypervisor on start up. On the ``hypervisor`` node the ``compatible``
>> -property is used to identify the type of hypervisor node present..
>> +Domain Node
>> +-----------
>> -compatible
>> -  Identifies the type of node. Required.
>> +A ``domain`` node is for describing the construction of a domain. 
>> Since there
>> +may be one or more domain nodes, each one requires a unique, DTB 
>> compliant name
>> +and a ``compatible`` property to identify as a domain node.
>> -The Config node
>> ----------------
>> +A ``domain`` node  may provide a ``domid`` property which will be 
>> used as the
>> +requested domain id for the domain with a value of “0” signifying to 
>> use the
>> +next available domain id, which is the default behavior if omitted. 
>> It should
>> +be noted that a domain configuration is not able to request a domid 
>> of “0”
> 
> Why do you need this restriction? And more importantly how would you 
> describe dom0 in hyperlaunch?

I would start by saying one of the goals/purposes behind hyperlaunch is 
to remove the binding that "dom0" is identified by having domid 0. That 
is what the roles patch recently submitted is working to achieve. "Dom0" 
is a role in the system, which I tried calling the "unbounded role" but 
that seems to have caused some confusion.

That aside, IMHO because of the legacy around domid 0, I don't think it 
should be assignable through hyperlaunch. Additionally, there should be 
an identifier that signifies auto-assign the domid and that value should 
not conflict/limit what domids are usable by the hypervisor. Given we 
should not be assigning domid 0 through this interface, it makes it the 
perfect candidate value.

>> +Beyond that, a domain node may have any of the following optional 
>> properties.
>> -A config node is for detailing any modules that are of interest to 
>> Xen itself.
>> -For example this would be where Xen would be informed of microcode or 
>> XSM
>> -policy locations. If the modules are multiboot modules and are able 
>> to be
>> -located by index within the module chain, the ``mb-index`` property 
>> should be
>> -used to specify the index in the multiboot module chain.. If the 
>> module will be
>> -located by physical memory address, then the ``module-addr`` property 
>> should be
>> -used to identify the location and size of the module.
>> +Properties
>> +""""""""""
>>   compatible
>> -  Identifies the type of node. Required.
>> -
>> -The Domain node
>> ----------------
>> +  Identifies the node as a domain node and for which hypervisor. 
>> Required.
>> -A domain node is for describing the construction of a domain. It may 
>> provide a
>> -domid property which will be used as the requested domain id for the 
>> domain
>> -with a value of “0” signifying to use the next available domain id, 
>> which is
>> -the default behavior if omitted. A domain configuration is not able 
>> to request
>> -a domid of “0”. After that a domain node may have any of the following
>> -parameters,
>> -
>> -compatible
>> -  Identifies the type of node. Required.
>> +  Format: "<hypervisor name>,domain", e.g "xen,domain"
>>   domid
>> -  Identifies the domid requested to assign to the domain. Required.
>> +  Identifies the domid requested to assign to the domain.
>> -permissions
>> +  Format: Integer, e.g <0>
>> +
>> +role
>>     This sets what Discretionary Access Control permissions
>>     a domain is assigned. Optional, default is none.
>> -functions
>> -  This identifies what system functions a domain will fulfill.
>> +  Format: Bitfield, e.g <3> or <0x00000003>
>> +
>> +          ROLE_NONE                (0)
>> +          ROLE_UNBOUNDED_DOMAIN    (1U<<0)
>> +          ROLE_CONTROL_DOMAIN      (1U<<1)
>> +          ROLE_HARDWARE_DOMAIN     (1U<<2)
>> +          ROLE_XENSTORE_DOMAIN     (1U<<3)
> 
> Please describe what each roles are meant for.

Agreed, but this obviously is in flux based on the current review of the 
roles patch.

>> +
>> +capability
>> +  This identifies what system capabilities a domain may have beyond 
>> the role it
>> +  was assigned.
>>     Optional, the default is none.
>> -.. note::  The `functions` bits that have been selected to indicate
>> -   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last 
>> two bits
>> -   (30, 31) such that should these features ever be fully retired, 
>> the flags may
>> -   be dropped without leaving a gap in the flag set.
>> +  Format: Bitfield, e.g <3221225487> or <0xC0000007>
> 
> I thik we should favor the hexadecimal version because this will be 
> somewhat easier to read.

I too favor the hex version, but as I recall, DT/libfdt doesn't 
care/enforce.

> Also, the Device-Tree values work in term of 32-bit cell. Also, how do 
> you plan to handle the case where you have more than 32 capabilities?

I would add a capabalities2 field, this is how SELinux/Flask handle the 
same problem.

>> +
>> +          CAP_NONE            (0)
>> +          CAP_CONSOLE_IO      (1U<<0)
> 
> Please describe the capabilities.

Agreed and again, will change based on final version of roles patch.

>>   mode
>>     The mode the domain will be executed under. Required.
>> +  Format: Bitfield, e.g <5> or <0x00000005>
>> +
>> +          MODE_PARAVIRTUALIZED     (1 << 0) PV | PVH/HVM
>> +          MODE_ENABLE_DEVICE_MODEL (1 << 1) HVM | PVH
>> +          MODE_LONG                (1 << 2) 64 BIT | 32 BIT
>> +
>>   domain-uuid
>>     A globally unique identifier for the domain. Optional,
>>     the default is NULL.
>> +  Format: Byte Array, e.g [B3 FB 98 FB 8F 9F 67 A3]
>> +
>>   cpus
>>     The number of vCPUs to be assigned to the domain. Optional,
>>     the default is “1”.
>> +  Format: Integer, e.g <0>
> 
> This is odd to suggest to give '0' as an example. Wouldn't Xen throw an 
> error if a user provide it?

Good catch, though I am now thinking individual examples are not needed 
and the full example below should be sufficient.

>> +
>>   memory
>> -  The amount of memory to assign to the domain, in KBs.
>> +  The amount of memory to assign to the domain, in KBs. This field 
>> uses a DTB
>> +  Reg which contains a start and size. For memory allocation start 
>> may or may
>> +  not have significance but size will always be used for the amount 
>> of memory
>>     Required.
> 
> The description doesn't match...

Ack, others caught that as well. Will be fixed.

>> +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
> 
> ... the format. But strings are difficult to parse. If you want to 
> provide 3 different values (possibly optional), then it would be best to 
> have 3 different properties.

That format comes from the command line dom0 memory parameter, in the 
hyperlaunch series I reused that existing parser that I am fairly 
confident will be maintained.

> I will continue to review the rest later.

Great, thank you so much for your feedback thus far.

v/r,
dps
Daniel P. Smith Aug. 8, 2023, 9:17 p.m. UTC | #12
On 8/4/23 03:06, Michal Orzel wrote:
> Hi Daniel,
> 
> On 03/08/2023 18:57, Daniel P. Smith wrote:
>>
>>
>> On 8/3/23 07:45, Michal Orzel wrote:
>>> Hi Daniel,
>>>
>>> On 03/08/2023 12:44, Daniel P. Smith wrote:
>>>>
>>>>
>>>> With on going development of hyperlaunch, changes to the device tree definitions
>>>> has been necessary. This commit updates the specification for all current changes
>>>> along with changes expected to be made in finalizing the capability.
>>>>
>>>> This commit also adds a HYPERLAUNCH section to the MAINTAINERS file and places
>>>> this documentation under its purview. It also reserves the path
>>>> `xen/common/domain-builder` for the hyperlaunch domain builder code base.
>>>>
>>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> 
> [...]
>>>> +
>>>>    memory
>>>> -  The amount of memory to assign to the domain, in KBs.
>>>> +  The amount of memory to assign to the domain, in KBs. This field uses a DTB
>>>> +  Reg which contains a start and size. For memory allocation start may or may
>>>> +  not have significance but size will always be used for the amount of memory
>>>>      Required.
>>>>
>>>> +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
>>> There is a mismatch between the description and above format:
>>> - KB vs MB
>>> - string vs reg format
>>> - the x86 example uses string and Arm uses reg format
>>
>> Hmmm. I missed this in the hyperlaunch v1 update. In the original design
>> that came from the working group it was going to use a reg as suggest by
>> dom0less. During development of v1, we found it was not rich enough to
>> express how Dom0 could be allocated memory and switched to a string to
>> mirror the dom0 memory hypervisor command line parameter.
> On Arm, dom0_mem cmdline parameter is used to specify only size (no min,max)

I understand. For general domain construction, these are legitimate 
values and hyperlaunch is meant to be able to fully construct a running 
environment just as if it was constructed by a toolstack. We must also 
be able to handle situations where a general configuration is being 
reused across systems and must be able to express the minimum memory 
each domain must have, how much should be attempted to be allocated, and 
if ballooning is enabled, being able to articulate that to the 
hyperlaunch domain builder.

I did not say this in reply to Julien, but I am not opposed to three 
separate parameters if that is what the conscience arrives at. I found 
the existing parser and the structure it creates as a useful and 
reusable component for obtaining and storing the values.

>>
>> A question for those involved with dom0less, is what are the opinions
>> about using this form for memory allocation. Is it required/possible to
>> be able to instruct the hypervisor what physical address to use as the
>> start of a domain's memory?
> "memory" dt property is used to specify just amount of memory for domain in KBs using reg format.
> It is not used to specify the static memory region (with start and size). For that, we have another property called "xen,static-mem".
> Therefore, it would be possible to switch memory to string but it would not be compatible with the current use anymore.

It would be compatible in the sense that dom0less-compatibility-mode 
could still accept it as just amount of memory. Which brings up a good 
point, whether I do it here or it is done in the patch series that will 
introduce dom0less-compatibilty-mode, there probably will need to be a 
top level flag under the hypervisor node to indicate it is a dom0less 
compatible configuration.

v/r,
dps
Julien Grall Aug. 8, 2023, 9:45 p.m. UTC | #13
Hi Daniel,

On 08/08/2023 21:49, Daniel P. Smith wrote:
> On 8/4/23 05:03, Julien Grall wrote:
>> Hi Daniel,
>>
>> On 03/08/2023 11:44, Daniel P. Smith wrote:
>>> +compatible
>>> +  Identifies which hypervisors the configuration is compatible. 
>>> Required.
>>> -    hypervisor {
>>> -        compatible = “hypervisor,xen”
>>> +  Format: "hypervisor,<hypervisor name>", e.g "hypervisor,xen"
>>
>> I read "e.g" as "for example". You don't explicitely say which 
>> compatible will be supported by Xen, so one could write 
>> "hypervisor,foo" and expect to work.
>>
>> Also, it is not fully clear why you need both the hypervisor and each 
>> domain node to have a compatible with the hypervisor name in it.
> 
> Ack, it should be explicit to what is expected for a Xen configuration. 
> As for compatible on the domain node, I think that was from being overly 
> cautious, it can be dropped.
> 
> This did get me reflecting that the compatibility was added there as 
> there was some initial participation in standardization efforts going on 
> at the time. I am not sure what has come of those, but the question it 
> raised is that domain is a Xen specific term, whereas generally vm is 
> accepted as the generic term. Maybe this node needs renaming?

IIRC Stefano attempted to (partially?) standardized the Device-Tree 
configuration for domains. So I will let him comment here.

> 
>>> +compatible
>>> +  Identifies the hypervisor the confiugration is intended. Required.
>>
>> Also typo: s/confiugration/configuration/
> 
> Ack.
> 
>>> -The modules that would be supplied when using the above config would 
>>> be:
>>> +  Format: "<hypervisor name>,config", e.g "xen,config"
>>> -* (the above config, compiled into hardware tree)
>>> -* CPU microcode
>>> -* XSM policy
>>> -* kernel for boot domain
>>> -* ramdisk for boot domain
>>> -* boot domain configuration file
>>> -* kernel for the classic dom0 domain
>>> -* ramdisk for the classic dom0 domain
>>> +bootargs
>>> +  This is used to provide the boot params for Xen.
>>
>> How is this different from the command line parameter chosen? And if 
>> you want to keep both, what is the expected priority if a user 
>> provides both?
> 
> A DT file for x86, there is only a need to provide the hypervisor node, 
> for which we already needed a hypervisor config section to describe XSM 
> policy and microcode, at this point at least. It was decided in an 
> effort to provide flexibility, the ability to specify command line 
> parameters to the hypervisor in DT config. It is expected these 
> parameters would function as a base parameters that would be overridden 
> by those provided via the multiboot kernel entry.
]>
> Now as you point out for Arm, this becomes a bit redundant, to a degree, 
> as the Xen command line is already under the /chosen node. But even here 
> it would be beneficial, as a hyperlaunch configuration could be 
> distributed consisting of a dtb that has core parameters set with base 
> values along with a set of kernels and ramdisks. At boot, the 
> hyperlaunch dtb could then be concatenated with the device specific dtb.

I don't have a strong opinions on how it should be done. My point is 
more that the priority should be document.

> 
> 
>>> -The hypervisor device tree would be compiled into the hardware 
>>> device tree and
>>> -provided to Xen using the standard method currently in use. The 
>>> remaining
>>> -modules would need to be loaded in the respective addresses 
>>> specified in the
>>> -`module-addr` property.
>>> +  Format: String, e.g. "flask=silo"
>>> +Child Nodes
>>> +"""""""""""
>>> -The Hypervisor node
>>> --------------------
>>> +* module
>>> -The hypervisor node is a top level container for the domains that 
>>> will be built
>>> -by hypervisor on start up. On the ``hypervisor`` node the 
>>> ``compatible``
>>> -property is used to identify the type of hypervisor node present..
>>> +Domain Node
>>> +-----------
>>> -compatible
>>> -  Identifies the type of node. Required.
>>> +A ``domain`` node is for describing the construction of a domain. 
>>> Since there
>>> +may be one or more domain nodes, each one requires a unique, DTB 
>>> compliant name
>>> +and a ``compatible`` property to identify as a domain node.
>>> -The Config node
>>> ----------------
>>> +A ``domain`` node  may provide a ``domid`` property which will be 
>>> used as the
>>> +requested domain id for the domain with a value of “0” signifying to 
>>> use the
>>> +next available domain id, which is the default behavior if omitted. 
>>> It should
>>> +be noted that a domain configuration is not able to request a domid 
>>> of “0”
>>
>> Why do you need this restriction? And more importantly how would you 
>> describe dom0 in hyperlaunch?
> 
> I would start by saying one of the goals/purposes behind hyperlaunch is 
> to remove the binding that "dom0" is identified by having domid 0. That 
> is what the roles patch recently submitted is working to achieve. "Dom0" 
> is a role in the system, which I tried calling the "unbounded role" but 
> that seems to have caused some confusion.
> 
> That aside, IMHO because of the legacy around domid 0, I don't think it 
> should be assignable through hyperlaunch.

I understand the end goal. But I am not entirely convinced this will be 
wanted for everyone. So it might be preferable to avoid using '0' as 
'assign any free domid' as this would not prevent to describe dom0 in 
hyperlaunch if needed in the future.

> Additionally, there should be 
> an identifier that signifies auto-assign the domid and that value should 
> not conflict/limit what domids are usable by the hypervisor.

Why is this requirement? Why can't we simply rely on the property is not 
present?

>  Given we 
> should not be assigning domid 0 through this interface, it makes it the 
> perfect candidate value.
To be honest, even if you don't want to allow an admin to allocate ID 0, 
  I think using it is confusing because of the legacy meaning behind it.

I would likely be confused every time I read a device-tree. Also, given 
you already have a way to say 'assign a domain ID', it is not clear to 
me why you really need a second way to do it.

[...]

>>> +
>>> +capability
>>> +  This identifies what system capabilities a domain may have beyond 
>>> the role it
>>> +  was assigned.
>>>     Optional, the default is none.
>>> -.. note::  The `functions` bits that have been selected to indicate
>>> -   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last 
>>> two bits
>>> -   (30, 31) such that should these features ever be fully retired, 
>>> the flags may
>>> -   be dropped without leaving a gap in the flag set.
>>> +  Format: Bitfield, e.g <3221225487> or <0xC0000007>
>>
>> I thik we should favor the hexadecimal version because this will be 
>> somewhat easier to read.
> 
> I too favor the hex version, but as I recall, DT/libfdt doesn't 
> care/enforce.

Indeed the device-tree compiler is able to cope with both. However, we 
don't have to mention the two. It would be ok to only mention the one we 
prefer (i.e. hexadecimal) so the reader will more naturally use it.

> 
>> Also, the Device-Tree values work in term of 32-bit cell. Also, how do 
>> you plan to handle the case where you have more than 32 capabilities?
> 
> I would add a capabalities2 field, this is how SELinux/Flask handle the 
> same problem.

You should not need to introduce a new field. Instead you can add a 
second cell. But we would need to describe the ordering because for 
backward compatibility the cell 0 would want to cover bits [0:31] and 
cell 2 bits [64:31].

[...]

>>> +
>>>   memory
>>> -  The amount of memory to assign to the domain, in KBs.
>>> +  The amount of memory to assign to the domain, in KBs. This field 
>>> uses a DTB
>>> +  Reg which contains a start and size. For memory allocation start 
>>> may or may
>>> +  not have significance but size will always be used for the amount 
>>> of memory
>>>     Required.
>>
>> The description doesn't match...
> 
> Ack, others caught that as well. Will be fixed.
> 
>>> +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
>>
>> ... the format. But strings are difficult to parse. If you want to 
>> provide 3 different values (possibly optional), then it would be best 
>> to have 3 different properties.
> 
> That format comes from the command line dom0 memory parameter, in the 
> hyperlaunch series I reused that existing parser that I am fairly 
> confident will be maintained.

Fair enough. However, I am still unconvinced this is the way to go. I 
don't have a strong argument other than 'memory' is already a number for 
dom0less DT and it sounds strange to change it.

Stefano, Bertrand, any opinions?

Cheers,
Julien Grall Aug. 8, 2023, 10:16 p.m. UTC | #14
Hi,

On 03/08/2023 11:44, Daniel P. Smith wrote:
> ----------------
> +This node describes a boot module loaded by the boot loader. A ``module`` node
> +will often appear repeatedly and will require a unique and DTB compliant name
> +for each instance.

For clarification, do you mean module@<unit>? or something different?

> The compatible property is required to identify that the
> +node is a ``module`` node, the type of boot module, and what it represents.
>   
> -This node describes a boot module loaded by the boot loader. The required
> -compatible property follows the format: module,<type> where type can be
> -“kernel”, “ramdisk”, “device-tree”, “microcode”, “xsm-policy” or “config”. In
> -the case the module is a multiboot module, the additional property string
> -“multiboot,module” may be present. One of two properties is required and
> -identifies how to locate the module. They are the mb-index, used for multiboot
> -modules, and the module-addr for memory address based location.
> +Depending on the type of boot module, the ``module`` node will require either a
> +``module-index`` or ``module-addr`` property must be present. They provide the
> +boot module specific way of locating the boot module in memory.
> +
> +Properties
> +""""""""""
>   
>   compatible
>     This identifies what the module is and thus what the hypervisor
>     should use the module for during domain construction. Required.
>   
> -mb-index
> -  This identifies the index for this module in the multiboot module chain.
> +  Format: "module,<module type>"[, "module,<locating type>"]
> +          module type: kernel, ramdisk, device-tree, microcode, xsm-policy,
> +                       config

All but the last are pretty self-explanatory. Can you clarify what the 
last one is?

> +
> +          locating type: index, addr

It is not clear to me why you need to specify the locating type in the 
compatible. Would not it be sufficient to check the presence of either 
module-index or module-addr?

If you still want both, then which property belong to which compatible?

> +
> +module-index
> +  This identifies the index for this module when in a module chain.
>     Required for multiboot environments.

'multiboot' is somewhat overloaded as we also use it to describe the 
binding in the device-tree. So I would clarify which multiboot you are 
referring to.

I assume this is x86 multiboot. That said, my knowledge about it is 
limited. How would a user be able to find the index to write down here?

>   
> +  Format: Integer, e.g. <0>
> +
>   module-addr
>     This identifies where in memory this module is located. Required for
>     non-multiboot environments.
>   
> +  Format: DTB Reg <start size>, e.g. <0x0 0x20000>

What is the expected number of cells?

> +
>   bootargs
>     This is used to provide the boot params to kernel modules.
>   
> +  Format: String, e.g. "ro quiet"
> +
>   .. note::  The bootargs property is intended for situations where the same kernel multiboot module is used for more than one domain.


I realize this wasn't added in your patch. But it is not entirely clear 
what this means given that an admin may still want to use 'bootargs' 
even if there is a single kernel.

> +
> +Example Configuration
> +---------------------
> +
> +Below are two example device tree definitions for the hypervisor node. The
> +first is an example of a multiboot-based configuration for x86 and the second
> +is a module-based configuration for Arm.
> +
> +Multiboot x86 Configuration:
> +""""""""""""""""""""""""""""
> +
> +::
> +
> +    /dts-v1/;
> +
> +    / {
> +        chosen {
> +            hypervisor {
> +                compatible = "hypervisor,xen", "xen,x86";
> +
> +                dom0 {
> +                    compatible = "xen,domain";
> +
> +                    domid = <0>;

This is actually a good example where '0' would become confusing because 
the name of the domain is 'dom0' so one could mistakenly assume that it 
means domid 0 will be assigned.

> +
> +                    role = <9>;

Reading this, I wonder if using number is actually a good idea. While 
this is machine friendly, this is not human friendly.

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?

> +                    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>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> +
> +
> +
> +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.

> +                        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?

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

Cheers,
Stefano Stabellini Aug. 9, 2023, 12:43 a.m. UTC | #15
On Tue, 8 Aug 2023, Daniel P. Smith wrote:
> On 8/3/23 19:38, Stefano Stabellini wrote:
> > On Thu, 3 Aug 2023, Daniel P. Smith wrote:
> > > > Also, what is the plan for the existing dom0less dt properties?
> > > > Will they need to be moved to new /hypervisor node or we will have to
> > > > parse
> > > > both /chosen and /hypervisor nodes?
> > > 
> > > In the proposal I sent to xen-devel in response to Luca's RFC for
> > > rebranding
> > > dom0less features under hyperlaunch, that is the purpose of this commit.
> > > Get
> > > this document up to date with what was done in v1 along with what we are
> > > planning/working on for hyperlaunch. One could think of this as
> > > effectively
> > > the API to the capabilities hyperlaunch will provide. Not just how to
> > > construct a domain, but what kinds of domains can be constructed by
> > > hyperlaunch. Step one of the proposal is to publish a patch upon which we
> > > all
> > > can iterate over and get to an agreement on a suitable interface for all.
> > > The
> > > next step would be the introduction of hyperlaunch dom0less compatibility
> > > mode, that would see the moving of the parsing logic for the existing
> > > dom0less
> > > nodes under /xen/common/domain-builder. It would continue to exist there
> > > even
> > > after hyperlaunch proper is merged and can remain there for backward
> > > compatibility until there is a decision to retire the compatibility
> > > interface.
> > 
> > I like this plan. The two interfaces are so similar that it is basically
> > one interface with a couple of tiny differences.
> > 
> > So I expect we would move the existing dom0less parsing code to common/,
> > add a couple of extensions (such as parsing /hypervisor in addition to
> > /chosen) and use it as it.
> 
> Do you mean /chosen/hypervisor?

I meant /hypervisor as in "the node under which the hyperlaunch
configuration is presented". If HyperLaunch uses /chosen/hypervisor,
then that's what I meant :-)


> > Later on, after a few years of using /hypervisor instead of /chosen, if
> > nobody is using /chosen anymore, we could retire /chosen completely. But
> > this is just one DT node/property that gets retired (there are a couple
> > of others). I don't imagine we'll have a full new implementation of the
> > DT parsing logic that supersedes the existing implementation of it
> > (especially considering the difficulty of maintaining 2 different parsing
> > logics in the hypervisor for similar interfaces).
> 
> +1
> 
> > Same thing for the DT interface documentation. I don't think we need two
> > DT interface docs? We could start with the existing dom0less interface
> > (docs/misc/arm/device-tree/booting.txt), and move it somewhere common
> > like docs/misc/device-tree.
> 
> So in the plan that I sent, patch series 3 was were I was going to consolidate
> docs/design/launch/hyperlaunch-devicetree.rst and
> docs/misc/arm/device-tree/booting.txt into a single document under
> docs/features/hyperlaunch/device-tree.rst along with moving
> docs/features/dom0less.pandoc to
> docs/features/hyperlaunch/dom0less-compatibility-mode.pandoc. The thinking
> here was to get all the feature documentation together in a single place, but
> I would be open to putting the suggested consolidated device-tree.rst
> mentioned above into docs/misc/device-tree/hyperlaunch.rst if that is
> preferred.

Yes I think we should have a single consolidated document, because it is
not a good idea to support very similar but subtly different interfaces
on ARM/x86. docs/misc/device-tree/hyperlaunch.rst should work fine.

And I think we should try to minimize differences and have a single
common parsing code.

Any role definition introduced by hyperlaunch originally for x86 would
undoubtedly be useful on ARM. Conversely, all the existing ARM dom0less
tools (ImageBuilder) might be useful on x86 too. RISC-V might use it too
in the near future.


> > Then add any changes or extensions required by other architecture, such
> > as x86 and RISC-V.
> > 
> > For sure for x86 we need "module-index". I don't know if anything else
> > is must-have to get it to work on x86 but if there is, we should add
> > those too.
> 
> Hmm good point, since in the suggested patch series 3 from the plan, this
> probably should get cropped down to what we actually have implemented for x86
> hyperlaunch and then get expanded as it becomes feature complete.

Yes exactly. So I think the best way to move forward is something along
these lines:
a) work on a single arch-neutral hyperlaunch/dom0less DT interface
   document for ARM/x86/RISC-V
b) in parallel, move dom0less parsing code to common (AMD might be able
   to work on this, to be confirmed)
c) add x86 features as needed to the code and to the interface doc,
   "module-index" would probably be among the first ones
d) as we add more features, we expand the hyperlaunch/dom0less DT
   interface document


I think a) only really needs the smallest amount of changes to make it
work on non-ARM architectures
Stefano Stabellini Aug. 9, 2023, 12:43 a.m. UTC | #16
On Tue, 8 Aug 2023, Daniel P. Smith wrote:
> On 8/3/23 20:09, Stefano Stabellini wrote:
> > On Thu, 3 Aug 2023, Stefano Stabellini wrote:
> > > On Thu, 3 Aug 2023, Daniel P. Smith wrote:
> > > > > Also, what is the plan for the existing dom0less dt properties?
> > > > > Will they need to be moved to new /hypervisor node or we will have to
> > > > > parse
> > > > > both /chosen and /hypervisor nodes?
> > > > 
> > > > In the proposal I sent to xen-devel in response to Luca's RFC for
> > > > rebranding
> > > > dom0less features under hyperlaunch, that is the purpose of this commit.
> > > > Get
> > > > this document up to date with what was done in v1 along with what we are
> > > > planning/working on for hyperlaunch. One could think of this as
> > > > effectively
> > > > the API to the capabilities hyperlaunch will provide. Not just how to
> > > > construct a domain, but what kinds of domains can be constructed by
> > > > hyperlaunch. Step one of the proposal is to publish a patch upon which
> > > > we all
> > > > can iterate over and get to an agreement on a suitable interface for
> > > > all. The
> > > > next step would be the introduction of hyperlaunch dom0less
> > > > compatibility
> > > > mode, that would see the moving of the parsing logic for the existing
> > > > dom0less
> > > > nodes under /xen/common/domain-builder. It would continue to exist there
> > > > even
> > > > after hyperlaunch proper is merged and can remain there for backward
> > > > compatibility until there is a decision to retire the compatibility
> > > > interface.
> > > 
> > > I like this plan. The two interfaces are so similar that it is basically
> > > one interface with a couple of tiny differences.
> > > 
> > > So I expect we would move the existing dom0less parsing code to common/,
> > > add a couple of extensions (such as parsing /hypervisor in addition to
> > > /chosen) and use it as it.
> > > 
> > > Later on, after a few years of using /hypervisor instead of /chosen, if
> > > nobody is using /chosen anymore, we could retire /chosen completely. But
> > > this is just one DT node/property that gets retired (there are a couple
> > > of others). I don't imagine we'll have a full new implementation of the
> > > DT parsing logic that supersedes the existing implementation of it
> > > (especially considering the difficulty of maintaining 2 different parsing
> > > logics in the hypervisor for similar interfaces).
> > > 
> > > Same thing for the DT interface documentation. I don't think we need two
> > > DT interface docs? We could start with the existing dom0less interface
> > > (docs/misc/arm/device-tree/booting.txt), and move it somewhere common
> > > like docs/misc/device-tree.
> > > 
> > > Then add any changes or extensions required by other architecture, such
> > > as x86 and RISC-V.
> > > 
> > > For sure for x86 we need "module-index". I don't know if anything else
> > > is must-have to get it to work on x86 but if there is, we should add
> > > those too.
> > 
> > 
> > For clarity, I think we should definitely have
> > docs/design/launch/hyperlaunch.rst, and maybe we should also have
> > hyperlaunch-devicetree.rst as an overview description and user guide.
> > That's useful.
> > 
> > But in terms of official device tree bindings interface description
> > (basically what in Linux would go under
> > Documentation/devicetree/bindings/), I think it would be best to only
> > have a single document. The current one is
> > docs/misc/arm/device-tree/booting.txt.
> 
> Does the proposal to your first message align with your follow-up here?

You are referring to a common docs/misc/device-tree/hyperlaunch.rst,
right? Then yes.
Stefano Stabellini Aug. 9, 2023, 12:44 a.m. UTC | #17
On Tue, 8 Aug 2023, Julien Grall wrote:
> On 08/08/2023 21:49, Daniel P. Smith wrote:
> > On 8/4/23 05:03, Julien Grall wrote:
> > > Hi Daniel,
> > > 
> > > On 03/08/2023 11:44, Daniel P. Smith wrote:
> > > > +compatible
> > > > +  Identifies which hypervisors the configuration is compatible.
> > > > Required.
> > > > -    hypervisor {
> > > > -        compatible = “hypervisor,xen”
> > > > +  Format: "hypervisor,<hypervisor name>", e.g "hypervisor,xen"
> > > 
> > > I read "e.g" as "for example". You don't explicitely say which compatible
> > > will be supported by Xen, so one could write "hypervisor,foo" and expect
> > > to work.
> > > 
> > > Also, it is not fully clear why you need both the hypervisor and each
> > > domain node to have a compatible with the hypervisor name in it.
> > 
> > Ack, it should be explicit to what is expected for a Xen configuration. As
> > for compatible on the domain node, I think that was from being overly
> > cautious, it can be dropped.
> > 
> > This did get me reflecting that the compatibility was added there as there
> > was some initial participation in standardization efforts going on at the
> > time. I am not sure what has come of those, but the question it raised is
> > that domain is a Xen specific term, whereas generally vm is accepted as the
> > generic term. Maybe this node needs renaming?
> 
> IIRC Stefano attempted to (partially?) standardized the Device-Tree
> configuration for domains. So I will let him comment here.

Yes it is called System Device Tree there is a public version of it
here:
https://github.com/devicetree-org/lopper/tree/master/specification/source
https://github.com/devicetree-org/lopper/blob/3e501d6c87f32d26fe5906760d1f661dbc0b4400/specification/source/system-device-tree.dts#L797

Here is what I would suggest for the hyperlaunch DT interface:

- try to minimize changes with the existing dom0less DT interface
- only introduce changes that are strictly necessary
- when changes are necessary, try to align with the System DT
  specification (which I can update if required)
- try to avoid changes that are different from dom0less and SystemDT
  both


> > > > +compatible
> > > > +  Identifies the hypervisor the confiugration is intended. Required.
> > > 
> > > Also typo: s/confiugration/configuration/
> > 
> > Ack.
> > 
> > > > -The modules that would be supplied when using the above config would
> > > > be:
> > > > +  Format: "<hypervisor name>,config", e.g "xen,config"
> > > > -* (the above config, compiled into hardware tree)
> > > > -* CPU microcode
> > > > -* XSM policy
> > > > -* kernel for boot domain
> > > > -* ramdisk for boot domain
> > > > -* boot domain configuration file
> > > > -* kernel for the classic dom0 domain
> > > > -* ramdisk for the classic dom0 domain
> > > > +bootargs
> > > > +  This is used to provide the boot params for Xen.
> > > 
> > > How is this different from the command line parameter chosen? And if you
> > > want to keep both, what is the expected priority if a user provides both?
> > 
> > A DT file for x86, there is only a need to provide the hypervisor node, for
> > which we already needed a hypervisor config section to describe XSM policy
> > and microcode, at this point at least. It was decided in an effort to
> > provide flexibility, the ability to specify command line parameters to the
> > hypervisor in DT config. It is expected these parameters would function as a
> > base parameters that would be overridden by those provided via the multiboot
> > kernel entry.
> ]>
> > Now as you point out for Arm, this becomes a bit redundant, to a degree, as
> > the Xen command line is already under the /chosen node. But even here it
> > would be beneficial, as a hyperlaunch configuration could be distributed
> > consisting of a dtb that has core parameters set with base values along with
> > a set of kernels and ramdisks. At boot, the hyperlaunch dtb could then be
> > concatenated with the device specific dtb.
> 
> I don't have a strong opinions on how it should be done. My point is more that
> the priority should be document.

Any change we make to the existing interface is more effort for us. I
would try to avoid changes. But of course we'll need new properties to
define domain roles (e.g. hardware domain, control domain).

For instance, do we need a hypervisor node? The XSM policy can be
specified already in dom0less and we could use the same strategy for
microcode.

 
> > > > -The hypervisor device tree would be compiled into the hardware device
> > > > tree and
> > > > -provided to Xen using the standard method currently in use. The
> > > > remaining
> > > > -modules would need to be loaded in the respective addresses specified
> > > > in the
> > > > -`module-addr` property.
> > > > +  Format: String, e.g. "flask=silo"
> > > > +Child Nodes
> > > > +"""""""""""
> > > > -The Hypervisor node
> > > > --------------------
> > > > +* module
> > > > -The hypervisor node is a top level container for the domains that will
> > > > be built
> > > > -by hypervisor on start up. On the ``hypervisor`` node the
> > > > ``compatible``
> > > > -property is used to identify the type of hypervisor node present..
> > > > +Domain Node
> > > > +-----------
> > > > -compatible
> > > > -  Identifies the type of node. Required.
> > > > +A ``domain`` node is for describing the construction of a domain. Since
> > > > there
> > > > +may be one or more domain nodes, each one requires a unique, DTB
> > > > compliant name
> > > > +and a ``compatible`` property to identify as a domain node.
> > > > -The Config node
> > > > ----------------
> > > > +A ``domain`` node  may provide a ``domid`` property which will be used
> > > > as the
> > > > +requested domain id for the domain with a value of “0” signifying to
> > > > use the
> > > > +next available domain id, which is the default behavior if omitted. It
> > > > should
> > > > +be noted that a domain configuration is not able to request a domid of
> > > > “0”
> > > 
> > > Why do you need this restriction? And more importantly how would you
> > > describe dom0 in hyperlaunch?
> > 
> > I would start by saying one of the goals/purposes behind hyperlaunch is to
> > remove the binding that "dom0" is identified by having domid 0. That is what
> > the roles patch recently submitted is working to achieve. "Dom0" is a role
> > in the system, which I tried calling the "unbounded role" but that seems to
> > have caused some confusion.
> > 
> > That aside, IMHO because of the legacy around domid 0, I don't think it
> > should be assignable through hyperlaunch.
> 
> I understand the end goal. But I am not entirely convinced this will be wanted
> for everyone. So it might be preferable to avoid using '0' as 'assign any free
> domid' as this would not prevent to describe dom0 in hyperlaunch if needed in
> the future.

In general I find that forcing people to adopt security features is
detrimental because:
- people that wants the security feature would have used it anyway
- people that doesn't want it, still doesn't want it and now they are
  frustrated

So I think we should let users specify domid 0. Is that a configuration
for a secure or a safe system? It is not. But some systems are just for
handling cats pictures.

If after the hyperlaunch patches domid 0 becomes meaningless then at
that point I would make a change to the DT interface potentially for Xen
to refuse to continue because the feature requested is unavailable.


> > Additionally, there should be an identifier that signifies auto-assign the
> > domid and that value should not conflict/limit what domids are usable by the
> > hypervisor.
> 
> Why is this requirement? Why can't we simply rely on the property is not
> present?
> 
> >  Given we should not be assigning domid 0 through this interface, it makes
> > it the perfect candidate value.
> To be honest, even if you don't want to allow an admin to allocate ID 0,  I
> think using it is confusing because of the legacy meaning behind it.
> 
> I would likely be confused every time I read a device-tree.

I agree


> Also, given you
> already have a way to say 'assign a domain ID', it is not clear to me why you
> really need a second way to do it.
> 
> [...]
> 
> > > > +
> > > > +capability
> > > > +  This identifies what system capabilities a domain may have beyond the
> > > > role it
> > > > +  was assigned.
> > > >     Optional, the default is none.
> > > > -.. note::  The `functions` bits that have been selected to indicate
> > > > -   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last two
> > > > bits
> > > > -   (30, 31) such that should these features ever be fully retired, the
> > > > flags may
> > > > -   be dropped without leaving a gap in the flag set.
> > > > +  Format: Bitfield, e.g <3221225487> or <0xC0000007>
> > > 
> > > I thik we should favor the hexadecimal version because this will be
> > > somewhat easier to read.
> > 
> > I too favor the hex version, but as I recall, DT/libfdt doesn't
> > care/enforce.
> 
> Indeed the device-tree compiler is able to cope with both. However, we don't
> have to mention the two. It would be ok to only mention the one we prefer
> (i.e. hexadecimal) so the reader will more naturally use it.
> 
> > 
> > > Also, the Device-Tree values work in term of 32-bit cell. Also, how do you
> > > plan to handle the case where you have more than 32 capabilities?
> > 
> > I would add a capabalities2 field, this is how SELinux/Flask handle the same
> > problem.
> 
> You should not need to introduce a new field. Instead you can add a second
> cell. But we would need to describe the ordering because for backward
> compatibility the cell 0 would want to cover bits [0:31] and cell 2 bits
> [64:31].
> 
> [...]
> 
> > > > +
> > > >   memory
> > > > -  The amount of memory to assign to the domain, in KBs.
> > > > +  The amount of memory to assign to the domain, in KBs. This field uses
> > > > a DTB
> > > > +  Reg which contains a start and size. For memory allocation start may
> > > > or may
> > > > +  not have significance but size will always be used for the amount of
> > > > memory
> > > >     Required.
> > > 
> > > The description doesn't match...
> > 
> > Ack, others caught that as well. Will be fixed.
> > 
> > > > +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
> > > 
> > > ... the format. But strings are difficult to parse. If you want to provide
> > > 3 different values (possibly optional), then it would be best to have 3
> > > different properties.
> > 
> > That format comes from the command line dom0 memory parameter, in the
> > hyperlaunch series I reused that existing parser that I am fairly confident
> > will be maintained.
> 
> Fair enough. However, I am still unconvinced this is the way to go. I don't
> have a strong argument other than 'memory' is already a number for dom0less DT
> and it sounds strange to change it.
> 
> Stefano, Bertrand, any opinions?

The current way to express domain memory in dom0less is not my favorite.
However, for the sake of minimizing changes, I think we should re-use it
as-is. That is my preference.

If the min/max parameters, currently unsupported, are must-have, then
yes we could introduce them as a change to the dom0less interface. 

I think we have 3 options:

- Expand the existing memory node in a backware compatible way, i.e. min
  and max could be the second and third cell of "memory"

- Introduce a new parameter to express memory, min and max, it cannot be
  called "memory" as it would conflict with the same name we use in
  dom0less
    - for instance, memory_string or memory_str if we use a string type
    - or memory_limits or memory_amounts if we use numerical types

- Introduce 2 new parameters to specify min and max separately in
  addition to memory

I am fine with all three, but I think this is not a high-priority item
as most domains would work fine with just memory specified (without min
and max).
Stefano Stabellini Aug. 9, 2023, 12:45 a.m. UTC | #18
On Tue, 8 Aug 2023, Daniel P. Smith wrote:
> On 8/4/23 03:06, Michal Orzel wrote:
> > Hi Daniel,
> > 
> > On 03/08/2023 18:57, Daniel P. Smith wrote:
> > > 
> > > 
> > > On 8/3/23 07:45, Michal Orzel wrote:
> > > > Hi Daniel,
> > > > 
> > > > On 03/08/2023 12:44, Daniel P. Smith wrote:
> > > > > 
> > > > > 
> > > > > With on going development of hyperlaunch, changes to the device tree
> > > > > definitions
> > > > > has been necessary. This commit updates the specification for all
> > > > > current changes
> > > > > along with changes expected to be made in finalizing the capability.
> > > > > 
> > > > > This commit also adds a HYPERLAUNCH section to the MAINTAINERS file
> > > > > and places
> > > > > this documentation under its purview. It also reserves the path
> > > > > `xen/common/domain-builder` for the hyperlaunch domain builder code
> > > > > base.
> > > > > 
> > > > > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> > 
> > [...]
> > > > > +
> > > > >    memory
> > > > > -  The amount of memory to assign to the domain, in KBs.
> > > > > +  The amount of memory to assign to the domain, in KBs. This field
> > > > > uses a DTB
> > > > > +  Reg which contains a start and size. For memory allocation start
> > > > > may or may
> > > > > +  not have significance but size will always be used for the amount
> > > > > of memory
> > > > >      Required.
> > > > > 
> > > > > +  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
> > > > There is a mismatch between the description and above format:
> > > > - KB vs MB
> > > > - string vs reg format
> > > > - the x86 example uses string and Arm uses reg format
> > > 
> > > Hmmm. I missed this in the hyperlaunch v1 update. In the original design
> > > that came from the working group it was going to use a reg as suggest by
> > > dom0less. During development of v1, we found it was not rich enough to
> > > express how Dom0 could be allocated memory and switched to a string to
> > > mirror the dom0 memory hypervisor command line parameter.
> > On Arm, dom0_mem cmdline parameter is used to specify only size (no min,max)
> 
> I understand. For general domain construction, these are legitimate values and
> hyperlaunch is meant to be able to fully construct a running environment just
> as if it was constructed by a toolstack.

This is a good goal to have.


> We must also be able to handle
> situations where a general configuration is being reused across systems and
> must be able to express the minimum memory each domain must have, how much
> should be attempted to be allocated, and if ballooning is enabled, being able
> to articulate that to the hyperlaunch domain builder.

I agree and most of those can already be specified in dom0less. The
minimum/maximum memory amounts are an exception.


> I did not say this in reply to Julien, but I am not opposed to three separate
> parameters if that is what the conscience arrives at. I found the existing
> parser and the structure it creates as a useful and reusable component for
> obtaining and storing the values.

Yes, if they are required we just need to agree on a format to specify
minimum/maximum. We have options.


> > > A question for those involved with dom0less, is what are the opinions
> > > about using this form for memory allocation. Is it required/possible to
> > > be able to instruct the hypervisor what physical address to use as the
> > > start of a domain's memory?
> > "memory" dt property is used to specify just amount of memory for domain in
> > KBs using reg format.
> > It is not used to specify the static memory region (with start and size).
> > For that, we have another property called "xen,static-mem".
> > Therefore, it would be possible to switch memory to string but it would not
> > be compatible with the current use anymore.
> 
> It would be compatible in the sense that dom0less-compatibility-mode could
> still accept it as just amount of memory. Which brings up a good point,
> whether I do it here or it is done in the patch series that will introduce
> dom0less-compatibilty-mode, there probably will need to be a top level flag
> under the hypervisor node to indicate it is a dom0less compatible
> configuration.

I don't think is a good idea to have a dom0less-compatibility-mode. I
think we should have a single interface between dom0less and
hyperlaunch. I think we should start with the existing dom0less
interface and add anything missing (e.g.  module-index, min/max).
Stefano Stabellini Aug. 9, 2023, 1:33 a.m. UTC | #19
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
>
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index d8a02a6c19..694412a961 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -332,6 +332,15 @@  M:	Nick Rosbrook <rosbrookn@gmail.com>
 S:	Maintained
 F:	tools/golang
 
+HYPERLAUNCH
+M:	Daniel P. Smith <dpsmith@apertussolutions.com>
+M:	Christopher Clark <christopher.w.clark@gmail.com>
+W:	https://wiki.xenproject.org/wiki/Hyperlaunch
+S:	Supported
+F:	docs/design/launch/hyperlaunch.rst
+F:	docs/design/launch/hyperlaunch-devicetree.rst
+F:	xen/common/domain-builder/
+
 HYPFS
 M:	Juergen Gross <jgross@suse.com>
 S:	Supported
diff --git a/docs/designs/launch/hyperlaunch-devicetree.rst b/docs/designs/launch/hyperlaunch-devicetree.rst
index b49c98cfbd..0bc719e4ae 100644
--- a/docs/designs/launch/hyperlaunch-devicetree.rst
+++ b/docs/designs/launch/hyperlaunch-devicetree.rst
@@ -2,10 +2,11 @@ 
 Xen Hyperlaunch Device Tree Bindings
 -------------------------------------
 
-The Xen Hyperlaunch device tree adopts the dom0less device tree structure and
-extends it to meet the requirements for the Hyperlaunch capability. The primary
-difference is the introduction of the ``hypervisor`` node that is under the
-``/chosen`` node. The move to a dedicated node was driven by:
+The Xen Hyperlaunch device tree is informed by the dom0less device tree
+structure with extensions to meet the requirements for the Hyperlaunch
+capability. A major depature from the dom0less device tree is the introduction
+of the ``hypervisor`` node that is under the ``/chosen`` node. The move to a
+dedicated node was driven by:
 
 1. Reduces the need to walk over nodes that are not of interest, e.g. only
    nodes of interest should be in ``/chosen/hypervisor``
@@ -13,331 +14,364 @@  difference is the introduction of the ``hypervisor`` node that is under the
 2. Allows for the domain construction information to easily be sanitized by
    simple removing the ``/chosen/hypervisor`` node.
 
-Example Configuration
----------------------
-
-Below are two example device tree definitions for the hypervisor node. The
-first is an example of a multiboot-based configuration for x86 and the second
-is a module-based configuration for Arm.
-
-Multiboot x86 Configuration:
-""""""""""""""""""""""""""""
-
-::
-
-    hypervisor {
-        #address-cells = <1>;
-        #size-cells = <0>;
-        compatible = “hypervisor,xen”
-
-        // Configuration container
-        config {
-            compatible = "xen,config";
-
-            module {
-                compatible = "module,microcode", "multiboot,module";
-                mb-index = <1>;
-            };
-
-            module {
-                compatible = "module,xsm-policy", "multiboot,module";
-                mb-index = <2>;
-            };
-        };
-
-        // Boot Domain definition
-        domain {
-            compatible = "xen,domain";
-
-            domid = <0x7FF5>;
-
-            // FUNCTION_NONE            (0)
-            // FUNCTION_BOOT            (1 << 0)
-            // FUNCTION_CRASH           (1 << 1)
-            // FUNCTION_CONSOLE         (1 << 2)
-            // FUNCTION_XENSTORE        (1 << 30)
-            // FUNCTION_LEGACY_DOM0     (1 << 31)
-            functions = <0x00000001>;
-
-            memory = <0x0 0x20000>;
-            cpus = <1>;
-            module {
-                compatible = "module,kernel", "multiboot,module";
-                mb-index = <3>;
-            };
-
-            module {
-                compatible = "module,ramdisk", "multiboot,module";
-                mb-index = <4>;
-            };
-            module {
-                compatible = "module,config", "multiboot,module";
-                mb-index = <5>;
-            };
-
-        // Classic Dom0 definition
-        domain {
-            compatible = "xen,domain";
-
-            domid = <0>;
-
-            // PERMISSION_NONE          (0)
-            // PERMISSION_CONTROL       (1 << 0)
-            // PERMISSION_HARDWARE      (1 << 1)
-            permissions = <3>;
-
-            // FUNCTION_NONE            (0)
-            // FUNCTION_BOOT            (1 << 0)
-            // FUNCTION_CRASH           (1 << 1)
-            // FUNCTION_CONSOLE         (1 << 2)
-            // FUNCTION_XENSTORE        (1 << 30)
-            // FUNCTION_LEGACY_DOM0     (1 << 31)
-            functions = <0xC0000006>;
-
-            // MODE_PARAVIRTUALIZED     (1 << 0) /* PV | PVH/HVM */
-            // MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
-            // MODE_LONG                (1 << 2) /* 64 BIT | 32 BIT */
-            mode = <5>; /* 64 BIT, PV */
-
-            // UUID
-            domain-uuid = [B3 FB 98 FB 8F 9F 67 A3];
-
-            cpus = <1>;
-            memory = <0x0 0x20000>;
-            security-id = “dom0_t;
-
-            module {
-                compatible = "module,kernel", "multiboot,module";
-                mb-index = <6>;
-                bootargs = "console=hvc0";
-            };
-            module {
-                compatible = "module,ramdisk", "multiboot,module";
-                mb-index = <7>;
-            };
-    };
-
-The multiboot modules supplied when using the above config would be, in order:
+The Hypervisor node
+-------------------
 
-* (the above config, compiled)
-* CPU microcode
-* XSM policy
-* kernel for boot domain
-* ramdisk for boot domain
-* boot domain configuration file
-* kernel for the classic dom0 domain
-* ramdisk for the classic dom0 domain
+The ``hypervisor`` node is a top level container for all information relating
+to how the hyperlaunch is to proceed. This includes definitions of the domains
+that will be built by hypervisor on start up. The node will be named
+``hypervisor``  with a ``compatible`` property to identify which hypervisors
+the configuration is intended. The hypervisor node will consist of one or more
+config nodes and one or more domain nodes.
 
-Module Arm Configuration:
-"""""""""""""""""""""""""
+Properties
+""""""""""
 
-::
+compatible
+  Identifies which hypervisors the configuration is compatible. Required.
 
-    hypervisor {
-        compatible = “hypervisor,xen”
+  Format: "hypervisor,<hypervisor name>", e.g "hypervisor,xen"
 
-        // Configuration container
-        config {
-            compatible = "xen,config";
+Child Nodes
+"""""""""""
 
-            module {
-                compatible = "module,microcode”;
-                module-addr = <0x0000ff00 0x80>;
-            };
+* config
+* domain
 
-            module {
-                compatible = "module,xsm-policy";
-                module-addr = <0x0000ff00 0x80>;
+Config Node
+-----------
 
-            };
-        };
+A ``config`` node is for passing configuration data and identifying any boot
+modules that is of interest to the hypervisor.  For example this would be where
+Xen would be informed of microcode or XSM policy locations. Each ``config``
+node will require a unique device-tree compliant name as there may be one or
+more ``config`` nodes present in a single dtb file. To identify which
+hypervisor the configuration is intended, the required ``compatible`` property
+must be present.
 
-        // Boot Domain definition
-        domain {
-            compatible = "xen,domain";
-
-            domid = <0x7FF5>;
-
-            // FUNCTION_NONE            (0)
-            // FUNCTION_BOOT            (1 << 0)
-            // FUNCTION_CRASH           (1 << 1)
-            // FUNCTION_CONSOLE         (1 << 2)
-            // FUNCTION_XENSTORE        (1 << 30)
-            // FUNCTION_LEGACY_DOM0     (1 << 31)
-            functions = <0x00000001>;
-
-            memory = <0x0 0x20000>;
-            cpus = <1>;
-            module {
-                compatible = "module,kernel";
-                module-addr = <0x0000ff00 0x80>;
-            };
+While the config node is not meant to replace the hypervisor commandline, there
+may be cases where it is better suited for passing configuration details at
+boot time.  This additional information may be carried in properties assigned
+to a ``config`` node. If there are any boot modules that are intended for the
+hypervisor, then a ``module`` child node should be provided to identify the
+boot module.
 
-            module {
-                compatible = "module,ramdisk";
-                module-addr = <0x0000ff00 0x80>;
-            };
-            module {
-                compatible = "module,config";
-                module-addr = <0x0000ff00 0x80>;
-            };
+Properties
+""""""""""
 
-        // Classic Dom0 definition
-        domain@0 {
-            compatible = "xen,domain";
-
-            domid = <0>;
-
-            // PERMISSION_NONE          (0)
-            // PERMISSION_CONTROL       (1 << 0)
-            // PERMISSION_HARDWARE      (1 << 1)
-            permissions = <3>;
-
-            // FUNCTION_NONE            (0)
-            // FUNCTION_BOOT            (1 << 0)
-            // FUNCTION_CRASH           (1 << 1)
-            // FUNCTION_CONSOLE         (1 << 2)
-            // FUNCTION_XENSTORE        (1 << 30)
-            // FUNCTION_LEGACY_DOM0     (1 << 31)
-            functions = <0xC0000006>;
-
-            // MODE_PARAVIRTUALIZED     (1 << 0) /* PV | PVH/HVM */
-            // MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
-            // MODE_LONG                (1 << 2) /* 64 BIT | 32 BIT */
-            mode = <5>; /* 64 BIT, PV */
-
-            // UUID
-            domain-uuid = [B3 FB 98 FB 8F 9F 67 A3];
-
-            cpus = <1>;
-            memory = <0x0 0x20000>;
-            security-id = “dom0_t”;
-
-            module {
-                compatible = "module,kernel";
-                module-addr = <0x0000ff00 0x80>;
-                bootargs = "console=hvc0";
-            };
-            module {
-                compatible = "module,ramdisk";
-                module-addr = <0x0000ff00 0x80>;
-            };
-    };
+compatible
+  Identifies the hypervisor the confiugration is intended. Required.
 
-The modules that would be supplied when using the above config would be:
+  Format: "<hypervisor name>,config", e.g "xen,config"
 
-* (the above config, compiled into hardware tree)
-* CPU microcode
-* XSM policy
-* kernel for boot domain
-* ramdisk for boot domain
-* boot domain configuration file
-* kernel for the classic dom0 domain
-* ramdisk for the classic dom0 domain
+bootargs
+  This is used to provide the boot params for Xen.
 
-The hypervisor device tree would be compiled into the hardware device tree and
-provided to Xen using the standard method currently in use. The remaining
-modules would need to be loaded in the respective addresses specified in the
-`module-addr` property.
+  Format: String, e.g. "flask=silo"
 
+Child Nodes
+"""""""""""
 
-The Hypervisor node
--------------------
+* module
 
-The hypervisor node is a top level container for the domains that will be built
-by hypervisor on start up. On the ``hypervisor`` node the ``compatible``
-property is used to identify the type of hypervisor node present..
+Domain Node
+-----------
 
-compatible
-  Identifies the type of node. Required.
+A ``domain`` node is for describing the construction of a domain. Since there
+may be one or more domain nodes, each one requires a unique, DTB compliant name
+and a ``compatible`` property to identify as a domain node.
 
-The Config node
----------------
+A ``domain`` node  may provide a ``domid`` property which will be used as the
+requested domain id for the domain with a value of “0” signifying to use the
+next available domain id, which is the default behavior if omitted. It should
+be noted that a domain configuration is not able to request a domid of “0”.
+Beyond that, a domain node may have any of the following optional properties.
 
-A config node is for detailing any modules that are of interest to Xen itself.
-For example this would be where Xen would be informed of microcode or XSM
-policy locations. If the modules are multiboot modules and are able to be
-located by index within the module chain, the ``mb-index`` property should be
-used to specify the index in the multiboot module chain.. If the module will be
-located by physical memory address, then the ``module-addr`` property should be
-used to identify the location and size of the module.
+Properties
+""""""""""
 
 compatible
-  Identifies the type of node. Required.
-
-The Domain node
----------------
+  Identifies the node as a domain node and for which hypervisor. Required.
 
-A domain node is for describing the construction of a domain. It may provide a
-domid property which will be used as the requested domain id for the domain
-with a value of “0” signifying to use the next available domain id, which is
-the default behavior if omitted. A domain configuration is not able to request
-a domid of “0”. After that a domain node may have any of the following
-parameters,
-
-compatible
-  Identifies the type of node. Required.
+  Format: "<hypervisor name>,domain", e.g "xen,domain"
 
 domid
-  Identifies the domid requested to assign to the domain. Required.
+  Identifies the domid requested to assign to the domain.
 
-permissions
+  Format: Integer, e.g <0>
+
+role
   This sets what Discretionary Access Control permissions
   a domain is assigned. Optional, default is none.
 
-functions
-  This identifies what system functions a domain will fulfill.
+  Format: Bitfield, e.g <3> or <0x00000003>
+
+          ROLE_NONE                (0)
+          ROLE_UNBOUNDED_DOMAIN    (1U<<0)
+          ROLE_CONTROL_DOMAIN      (1U<<1)
+          ROLE_HARDWARE_DOMAIN     (1U<<2)
+          ROLE_XENSTORE_DOMAIN     (1U<<3)
+
+capability
+  This identifies what system capabilities a domain may have beyond the role it
+  was assigned.
   Optional, the default is none.
 
-.. note::  The `functions` bits that have been selected to indicate
-   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last two bits
-   (30, 31) such that should these features ever be fully retired, the flags may
-   be dropped without leaving a gap in the flag set.
+  Format: Bitfield, e.g <3221225487> or <0xC0000007>
+
+          CAP_NONE            (0)
+          CAP_CONSOLE_IO      (1U<<0)
 
 mode
   The mode the domain will be executed under. Required.
 
+  Format: Bitfield, e.g <5> or <0x00000005>
+
+          MODE_PARAVIRTUALIZED     (1 << 0) PV | PVH/HVM
+          MODE_ENABLE_DEVICE_MODEL (1 << 1) HVM | PVH
+          MODE_LONG                (1 << 2) 64 BIT | 32 BIT
+
 domain-uuid
   A globally unique identifier for the domain. Optional,
   the default is NULL.
 
+  Format: Byte Array, e.g [B3 FB 98 FB 8F 9F 67 A3]
+
 cpus
   The number of vCPUs to be assigned to the domain. Optional,
   the default is “1”.
 
+  Format: Integer, e.g <0>
+
 memory
-  The amount of memory to assign to the domain, in KBs.
+  The amount of memory to assign to the domain, in KBs. This field uses a DTB
+  Reg which contains a start and size. For memory allocation start may or may
+  not have significance but size will always be used for the amount of memory
   Required.
 
+  Format: String  min:<sz> | max:<sz> | <sz>, e.g. "256M"
+
 security-id
   The security identity to be assigned to the domain when XSM
   is the access control mechanism being used. Optional,
-  the default is “domu_t”.
+  the default is “system_u:system_r:domU_t”.
+
+  Format: string, e.g. "system_u:system_r:domU_t"
+
+Child Nodes
+"""""""""""
+
+* module
+
+Module node
+-----------
 
-The Module node
----------------
+This node describes a boot module loaded by the boot loader. A ``module`` node
+will often appear repeatedly and will require a unique and DTB compliant name
+for each instance. The compatible property is required to identify that the
+node is a ``module`` node, the type of boot module, and what it represents.
 
-This node describes a boot module loaded by the boot loader. The required
-compatible property follows the format: module,<type> where type can be
-“kernel”, “ramdisk”, “device-tree”, “microcode”, “xsm-policy” or “config”. In
-the case the module is a multiboot module, the additional property string
-“multiboot,module” may be present. One of two properties is required and
-identifies how to locate the module. They are the mb-index, used for multiboot
-modules, and the module-addr for memory address based location.
+Depending on the type of boot module, the ``module`` node will require either a
+``module-index`` or ``module-addr`` property must be present. They provide the
+boot module specific way of locating the boot module in memory.
+
+Properties
+""""""""""
 
 compatible
   This identifies what the module is and thus what the hypervisor
   should use the module for during domain construction. Required.
 
-mb-index
-  This identifies the index for this module in the multiboot module chain.
+  Format: "module,<module type>"[, "module,<locating type>"]
+          module type: kernel, ramdisk, device-tree, microcode, xsm-policy,
+                       config
+
+          locating type: index, addr
+
+module-index
+  This identifies the index for this module when in a module chain.
   Required for multiboot environments.
 
+  Format: Integer, e.g. <0>
+
 module-addr
   This identifies where in memory this module is located. Required for
   non-multiboot environments.
 
+  Format: DTB Reg <start size>, e.g. <0x0 0x20000>
+
 bootargs
   This is used to provide the boot params to kernel modules.
 
+  Format: String, e.g. "ro quiet"
+
 .. note::  The bootargs property is intended for situations where the same kernel multiboot module is used for more than one domain.
+
+Example Configuration
+---------------------
+
+Below are two example device tree definitions for the hypervisor node. The
+first is an example of a multiboot-based configuration for x86 and the second
+is a module-based configuration for Arm.
+
+Multiboot x86 Configuration:
+""""""""""""""""""""""""""""
+
+::
+
+    /dts-v1/;
+
+    / {
+        chosen {
+            hypervisor {
+                compatible = "hypervisor,xen", "xen,x86";
+
+                dom0 {
+                    compatible = "xen,domain";
+
+                    domid = <0>;
+
+                    role = <9>;
+                    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>;
+                    };
+                };
+            };
+        };
+    };
+
+
+
+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 */
+
+                    memory = <0x0 0x20000>;
+                    cpus = <1>;
+                    module {
+                        compatible = "module,kernel";
+                        module-addr = <0x0000ff00 0x80>;
+                    };
+
+                    module {
+                        compatible = "module,ramdisk";
+                        module-addr = <0x0000ff00 0x80>;
+                    };
+
+                // Guest definition
+                dom1 {
+                    compatible = "xen,domain";
+
+                    domid = <0>;
+
+                    role = <0>;
+                    capability = <1>;
+
+                    mode = <12>; /* 64 BIT, PVH */
+
+                    // UUID
+                    domain-uuid = [C2 5D 91 CB 60 4B 45 75 89 04 FF 09 64 54 1A 74];
+
+                    cpus = <1>;
+                    memory = <0x0 0x20000>;
+                    security-id = “dom0_t”;
+
+                    module {
+                        compatible = "module,kernel";
+                        module-addr = <0x0000ff00 0x80>;
+                        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. The remaining
+modules would need to be loaded in the respective addresses specified in the
+`module-addr` property.