Re: [PATCH v2 3/5] arm/dom0less: put dom0less feature code in a separate module

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 3 Oct 2023, Julien Grall wrote:
> On 03/10/2023 20:52, Stefano Stabellini wrote:
> > On Tue, 3 Oct 2023, Michal Orzel wrote:
> > > On 03/10/2023 09:44, Luca Fancellu wrote:
> > > > Given that the status after the move to common of the above functions is
> > > > not very clean, I’ve decided to don’t do that,
> > > > however if you are fine with it, I can do the modification and who is
> > > > going to work further on the subject can consolidate
> > > > and make them build for other architecture.
> > > > 
> > > Another option would be to hold off for a while until work on
> > > hyperlaunch/RISCV dom0less starts to better understand the needs,
> > > concepts and to avoid multiple code movement which results in a horrible
> > > history. I know this is not nice but I can tell you that
> > > I had to stop working on some features like FLASK with dom0less, static
> > > domids for dom0less domUs, because according to the hyperlaunch design,
> > > this will need to be common. With hyperlaunch, everything starts with the
> > > domain configuration that is supposed to be arch neutral, so
> > > until this is done, it's difficult to do anything in this area.
> > 
> > This is not good. In an ideal world, Hyperlaunch shouldn't block
> > progress for dom0less. We shouldn't have to wait many months to make
> > progress on FLASK with dom0less, static domids for dom0less domUs, etc.
> > because of potential Hyperlaunch implications.
> 
> It depends what are the implications. If it means that the bindings will
> change a release after. Then I think we should instead work on standardizing
> Hyperlaunch (or whichever name we decide to use) earlier so our users can rely
> on the interface for multiple revisions.
> 
> We could of course decide to maintain both interfaces. But this means more
> maintenance work which could have been avoided by fast tracking Hyperlaunch
> (it is not like we don't know it is coming...).
> 
> > In my option a delay of few weeks might be OK; we should be reasonable.
> > But a delay of few months is not. Cosidering review times, release
> > schedules etc. it could become a very significant delay. >
> > Also, hyperlaunch contributors are not familiar with dom0less and are
> > not familiar with arm. (This is so true that they have their own
> > reimplementation of the parser.) I think the dom0less separation / code
> > movement is better done by us in the arm community because we know the
> > code far better.
> 
> I think we need both the arm and hyperlaunch community to work together (see
> more below).
> 
> > 
> > So I think Luca's suggestion above is in the right direction. I would
> > follow Luca's suggestion with only one difference: I would keep
> > prepare_dtb_domU in the arm code, together with make_gicv*_domU_node and
> > make_vpl011_uart_node. I would move the rest to common.
> 
> Luca's pointed out that some function (such as construct_domU) would contain
> reference to Arm specific code. So with your proposal, I am under the
> impression that we would move code that would then end up to be moved again in
> a few months time. So it is defeating your goal (even though the movement will
> hopefully be smaller).

I was assuming that e.g. construct_domU would reference ARM code, but
the ARM code would live under arch/arm. So yes construct_domU would need
some refactoring in the future to make the call generic instead of
arm-specific, but wouldn't require significant additional code movement.

But maybe I am wrong. I don't know if Luca has a clearer picture in his
mind.


> As I wrote above, I don't think the Arm community alone is in the position to
> decide what should be in common and what should be in arch specific. We need
> the hyperlaunch community to agree on their approach so we can know which
> split makes sense. This is similar to the on-going MMU split to cater the MPU.
> We looked the MPU code to decide of the best split.
> 
> A potential approach would be to look at the RISC-V implementation of dom0less
> and see the common parts. But then we are going in the the scope creep you
> mention earlier.

I didn't do a proper investigation and I didn't look at the RISC-V or
hyperlaunch code in details. From my discussions with both groups, here
is my understanding of arm-specific things we currently have:

- 1:1 memory mapping / static memory / static heap because they are
  currently unimplemented on other arches
- most domU device tree building because most of virtual devices are
  different (timer, interrupt controller, uart, etc)
- the equivalent for dom0: the gic/timer device tree generated nodes
- the blacklist at the beginning of handle_node
- for hyperlaunch, we need to support "module-index" as a way to get the
  physical address of a module
- reserved_memory doesn't exist on x86
- a couple of dom0less device tree properties are arm-specific, such as
  "sve" and "vpl011"

This is me hand-waiving, so it might not be useful.


> So overall, I feel that Lucas' approach is better until Hyperlaunch gain
> momentum.

I am OK with Luca's original approach. I just wanted to open the
discussion in case there is a better way.