Skip to content

modules: add and migrate to the Xen module #91643

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

soburi
Copy link
Member

@soburi soburi commented Jun 15, 2025

Currently, the MIT-licensed Xen public headers were incorporated
under the include/zephyr/xen/public.

However, the recent policy is to not include sources other than
Apache-2 in the main tree, so we will externalize them.

related: #91644

File differences:

The repository contains the RELEASE-4.17.0 file,
which is close to the version before the migration,
but there are some differences in arch-arm.h,
due to the following two commits.
These are considered to need to be incorporated,
so they have been changed from the pre-migration version.

Also, in the pre-migration version,

DEFINE_XEN_GUEST_HANDLE(uint8_t);

was added.
But this can be handled by the line

__DEFINE_XEN_GUEST_HANDLE(uint8, uint8_t);

So the differences related to this are not applied
and left the code from RELEASE-4.17.0.

Also, visibility was controlled with CONFIG_XEN_DOM0,
but this is not necessarily required, so it has been omitted.

Configuration behavior change:

The behavior of CONFIG_XEN_INTERFACE_VERSION has been changed
so that if 0 is set,
the LATEST version defined in the header will be used.
The default value has also been changed to 0.
(In other words, if you do not specify it, it will follow the header version.)

Accordingly, we have updated the path and reviewed the location of
the config items.

Copy link

github-actions bot commented Jun 15, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
xen 🆕 N/A (Added) soburi/xen@2aed8da (zephyr) N/A

DNM label due to: 1 added project

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@soburi soburi marked this pull request as ready for review June 15, 2025 02:47
@github-actions github-actions bot added area: Process area: Xen Platform area: ARM64 ARM (64-bit) Architecture area: UART Universal Asynchronous Receiver-Transmitter area: Samples Samples labels Jun 15, 2025
@soburi soburi added the DNM (manifest) This PR should not be merged (controlled by action-manifest) label Jun 15, 2025
@firscity
Copy link
Collaborator

Hi @soburi
Thank you for contribution and activity in the Xen part of Zephyr RTOS!

I checked related issue #91644, commit messages, your Xen fork and code of this PR.
As I said previously, in general I like the idea to moving Xen related headers to external module, but I have a few concerns and questions.

First of all, Xen support in Zephyr consists of few scenarios/drivers, that may run on few platforms. It means that after migration to new approach with public headers, they should be re-tested before taking them to the main tree (it may not be fast). Two basic scenarios for Xen+Zephyr are running as Domain-0 and as Domain-U. It can be done on different platforms: Qemu, Renesas Gen3-Gen4, Raspberry PI5 etc.

Second, I am not sure about the approach for public headers repo. I think creating a whole fork history of Xen hypervisor, manually removing everything except public headers, and patch them every time - is not the best approach, because it will require manual actions from me as maintainer for new Xen versions (the same as it is now). Unfortunately, I can not provide you with better idea now, but I had to mention this. I will try to find out a solution for this or at least I am open for discussion.

Third, I know that this PR is some kind of draft or proposal, but in some parts I don't actually understand the approach. For example, you are removing Kconfig file from arch/arm64 directory and placing it in module instead of creating own Kconfig in module (Zephyr still has Xen drivers, even Apache code in mentioned directory). In my understanding we either should move Xen support to module entirely (I don't think it great idea) or move only public headers there with own Kconfig, CMakeLists etc. Already existing Apache code should remain as is to minimize change size.

Once again, thank you for great effort and PoC, hope my comment will help us to start discussion and find best solution.
Also, @lorc @luca-fancellu guys, please take a look at this PR if you have time, your opinion will be very helpful.

BR.

Add the Xen include files as a module.

Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>
@soburi
Copy link
Member Author

soburi commented Jun 29, 2025

@firscity

Thank you for your review, and sorry for my late response.

First of all, Xen support in Zephyr consists of few scenarios/drivers, that may run on few platforms. It means that after migration to new approach with public headers, they should be re-tested before taking them to the main tree (it may not be fast). Two basic scenarios for Xen+Zephyr are running as Domain-0 and as Domain-U. It can be done on different platforms: Qemu, Renesas Gen3-Gen4, Raspberry PI5 etc.

I switched to a more conservative approach.
That is, I changed it so that the current files are externalized as they are.
However, the current files are formatted to match the Zephyr convention. I reverted this to make it easy to manage.

I think this will make it somewhat easier to revalidate.
I will issue a separate PR for any additional changes that are required.

Second, I am not sure about the approach for public headers repo. I think creating a whole fork history of Xen hypervisor, manually removing everything except public headers, and patch them every time - is not the best approach, because it will require manual actions from me as maintainer for new Xen versions (the same as it is now). Unfortunately, I can not provide you with better idea now, but I had to mention this. I will try to find out a solution for this or at least I am open for discussion.

Merging is a very difficult issue.

Leaving the xen/include/public/ directory unchanged in the repository is a good way to make merging easier, but doing so makes it disable to set up a proper include path.
(We would need to include the zephyr directory for management in the include path, or eliminate the proper namespace, like #include <public/xen.h> )

One idea is to create a working branch where we just move the directory, and then merge from there.
This will apply the git history to the merge, so it should be somewhat easier than directly checking the differences with diff.
Here, I create the header-only branch and insert a merge commit into it.

Third, I know that this PR is some kind of draft or proposal, but in some parts I don't actually understand the approach. For example, you are removing Kconfig file from arch/arm64 directory and placing it in module instead of creating own Kconfig in module (Zephyr still has Xen drivers, even Apache code in mentioned directory). In my understanding we either should move Xen support to module entirely (I don't think it great idea) or move only public headers there with own Kconfig, CMakeLists etc. Already existing Apache code should remain as is to minimize change size.

Since Xen supports other architectures as well, I don't think Xen's Kconfig should be placed under arch/arm64 in principle.
(Of course, arm64-specific stuff should go here.)
Now that this change has created a suitable location for it, it's more appropriate to place Kconfig here.

Once again, thank you for great effort and PoC, hope my comment will help us to start discussion and find best solution. Also, @lorc @luca-fancellu guys, please take a look at this PR if you have time, your opinion will be very helpful.

I believe there are still many issues that require your cooperation.
Thank you in advance.

@firscity
Copy link
Collaborator

firscity commented Jul 7, 2025

Hi @soburi
I had not much time for this activity lately due to high load on my position, so I'm sorry for late replies to this PR.

I spent some time thinking about the approach and ways to resolve issues, that are concerning to me. Then I checked @carlescufi comment about a TSC discussion in #91644, this increased my doubts about the way it is going to be implemented. Also, this comment made me curious about the actual need of moving it to a separate project/module from Zephyr. I'm not familiar with current Zephyr development trends, so I need someone to clarify if we actually need to remove MIT code/headers from Zephyr RTOS sources?

Regarding previous comments:

That is, I changed it so that the current files are externalized as they are.
However, the current files are formatted to match the Zephyr convention. I reverted this to make it easy to manage.

Definitely, current implementation has issues with maintenance due to differences between Xen and Zephyr coding styles. Previously coding style and content was a major issue when I tried to submit it here, so I tried to remove everything that is not used and to fix coding style in headers, which were needed. If we will place them in separate module, I 100% agree that they should left as they are in Xen source tree.

One idea is to create a working branch where we just move the directory, and then merge from there.
This will apply the git history to the merge, so it should be somewhat easier than directly checking the differences with diff.
Here, I create the header-only branch and insert a merge commit into it.

I believe that it is OK to just take a xen/include/public content from Xen source tree of some "RELEASE-X.XX.X" and place it as a content of module repo. Every new commit will just substitute a headers from "RELEASE-X.XX.X" to newer "RELEASE-Y.YY.Y". In such scenario we don't need to have a whole Xen commit history of "RELEASE-X.XX.X", this will significantly decrease west update time and will not make it harder to maintain since I believe that this module will be updated on (Xen) release basis.

Since Xen supports other architectures as well, I don't think Xen's Kconfig should be placed under arch/arm64 in principle.
(Of course, arm64-specific stuff should go here.)
Now that this change has created a suitable location for it, it's more appropriate to place Kconfig here.

Currently Xen in Zephyr is supported only for Arm64, thus it was placed there. When someone will add support of some other arch, I believe it will be a right time to move it from there. I still think that it should not be placed in module, that should just contain Xen public header since CONFIG_XEN touches a lot of thing inside Zephyr RTOS now.

I believe there are still many issues that require your cooperation.
Thank you in advance.

Thank you very much for your effort, I will still try to assist you as much as I can.

Best regards,
Dmytro.

Currently, the MIT-licensed Xen public headers were incorporated
under the `include/zephyr/xen/public`.

However, the recent policy is to not include sources other than
Apache-2 in the main tree, so we will externalize them.

Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM64 ARM (64-bit) Architecture area: Process area: Samples Samples area: UART Universal Asynchronous Receiver-Transmitter area: Xen Platform DNM (manifest) This PR should not be merged (controlled by action-manifest) manifest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants