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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx>
> > 
> > [...]
> > > > > +
> > > > >    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).