Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why do domains have a mandatory id? #163

Open
mbolivar-nordic opened this issue Mar 2, 2023 · 3 comments
Open

Why do domains have a mandatory id? #163

mbolivar-nordic opened this issue Mar 2, 2023 · 3 comments

Comments

@mbolivar-nordic
Copy link
Contributor

As of 4fc81d4, the "openamp,domain-v1" binding says id is required.

Why?

Why do we need an id at all, given that we can already refer to a
domain explicitly by its phandle, etc.?

At least within Zephyr, I cannot think of a reason why we would use
this property, so I think it should at least be made optional.

@sstabellini
Copy link
Collaborator

Each Xen domain has an "id" and it is called "domid". If it is not specified, it would be automatically generated by Xen. Dom0 (domid == 0 is special so we need a way to detect if a domain has domid == 0 or domid != 0. PLM, which is the Xilinx firmware that handles AMP domains at the firmware level, also requires ids to identify domains (PLM calls them subsystems).

So, the reason why "id" is required is that certain types of domains are required to specify an "id". Could we find a way to make "id" optional by default but required depending on the compatible string? Maybe, but for simplicity we didn't go down that route.

Given that it is easy to add an "id" even when not necessary, and given the additional complexity of having to handle required vs. optional "id" cases, I don't know if it is worth making changes to the spec in that regard.

Suggestions are welcome.

@mbolivar-nordic
Copy link
Contributor Author

Given that it is easy to add an "id" even when not necessary

I disagree with this "easy" here -- in effect, this statement forces every potential Zephyr user of system devicetree to specify a useless value for every domain they configure. This will cause support burden and confusion.

Suggestions are welcome.

This seems very Xen specific. It seems like a xen,domain-id property would be more appropriate.

@sstabellini
Copy link
Collaborator

sstabellini commented Mar 7, 2023

The id is certainly somewhat specific to the domain type. At this point, I don't know how many domain types need "id" and how many don't. Certainly, the presence of id could be made dependent on the domain type. Typically in device tree we try to reuse the same property name for the same thing even from different vendors, so I think it should be called "id" instead of "xen,domain-id". That way, it will be common across all domain types that require it.

If you are willing to make the changes to the specification to describe that:

  • xilinx,subsystem* domains require "id"
  • xen,domain* domains require "id"

Then I think we can make "id" optional for others.

I was not looking forward to maintaining a list of domains type that require "id" and a list of domain types that do not. That is why it was not done before.

mbolivar-nordic added a commit to mbolivar-nordic/lopper that referenced this issue May 10, 2023
Previous use cases for execution domains generally required an id
property. However, as the specification is applied in more contexts,
it's becoming clear that it may not always be needed. In particular,
the Zephyr RTOS is not expected to have initial use cases for this
property (though it may in the future start using it).

To avoid forcing these users to maintain unique id properties that are
not needed, refine the binding so that 'id' is only needed in some
cases.

Fixes: devicetree-org#163
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Reviewed-by: Stefano Stabellini <stefano.stabellini@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants