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

Add ukvmem Aarch64 page fault handler #908

Closed
wants to merge 4 commits into from

Conversation

mogasergiu
Copy link
Member

@mogasergiu mogasergiu commented May 25, 2023

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.pl on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): arm64
  • Platform(s): kvm
  • Application(s): N/A

Additional configuration

Description of changes

Add the required functionalities to implement a basic ukvmem page fault handler for Aarch64. This PR depends on #907 #772 #848 , therefore it includes them.

NOTE

This is part of a larger set of Pull Requests. To make things easier and to ensure that things build as expected, it is recommended that testing is done based on #912, which includes everything. The splitting has been done to ease the review process.

@mogasergiu mogasergiu requested review from a team as code owners May 25, 2023 12:21
@unikraft-bot unikraft-bot added arch/arm arch/arm64 arch/x86_64 area/arch Unikraft Architecture area/include Part of include/uk area/kconfig Part of the Unikraft KConfig option system area/lib Internal Unikraft Microlibrary area/makefile Part of the Unikraft Makefile build system area/plat Unikraft Patform lang/c Issues or PRs to do with C/C++ lang/python Issues or PRs to do with Python lib/ukboot plat/common Common to all platforms plat/driver plat/driver/virtio plat/kvm Unikraft for KVM plat/xen Unikraft for Xen labels May 25, 2023
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Jun 22, 2023
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Jun 22, 2023
Copy link
Member

@michpappas michpappas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mogasergiu only a few minor comments from me 👍🏼

In the titles of the commit messages we don't put the whole path, but rather a namespace identifier (when in doubt check the git history). In this case you can replace include/arch/arm/arm64 with arch/arm64.

Also, if I'm not mistaken, you can remove the dependencies and rebase so that this can be merged.

@@ -104,6 +104,10 @@ struct ukarch_pagetable {
#define PAGE_ATTR_SHAREABLE_IS (1 << PAGE_ATTR_SHAREABLE_SHIFT)
#define PAGE_ATTR_SHAREABLE_OS (2 << PAGE_ATTR_SHAREABLE_SHIFT)

/* Page fault error code bits */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We define the rest of ESR bits in arch.h maybe you could move these for consistency there too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I am not so sure, I now remember that the reason why I did this is because x86 also does this in arch/x86/x86_64/include/uk/asm/paging.h.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but in the scope of plat re-arch these will be moved into arch/ too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. IMO this should still stay this way to be consistent with x86. When plat re-arch comes, I believe this migration should be all part of the same commit. Therefore, I would like to keep it like this for now, because there is going to be some time until plat re-arch comes around.

@mogasergiu
Copy link
Member Author

Also, if I'm not mistaken, you can remove the dependencies and rebase so that this can be merged.

Hi, thanks a lot for taking a look :D. Unfortunately, this PR depends on #848, so that would probably need to be merged first :/.

mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Aug 6, 2023
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Aug 6, 2023
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Aug 6, 2023
@michpappas michpappas mentioned this pull request Aug 7, 2023
4 tasks
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Aug 10, 2023
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Aug 10, 2023
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Aug 10, 2023
@razvand razvand assigned razvand and unassigned marcrittinghaus Aug 11, 2023
Implement `Add PT_Lx_PTE_SET_PADDR()`, which allows to set the
physical address in a page table entry.

Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>
Define the bits required to tell whether a page fault was generated
due to a read, write or execute action.

Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>
Implement the equivalent page fault intermediary handler of the x86
one, by using the `AArch64` specific equivalent bitfields to check
for the type of the fault.

Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>
Since you cannot use `ukvmem` without the paging API's, it makes
sense to add a dependency on the `CONFIG_PAGING` configuration entry.

Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>
Copy link
Member

@michpappas michpappas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Michalis Pappas michalis@unikraft.io

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved-by: Razvan Deaconescu razvand@unikraft.io

unikraft-bot pushed a commit that referenced this pull request Aug 11, 2023
Define the bits required to tell whether a page fault was generated
due to a read, write or execute action.

Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #908
unikraft-bot pushed a commit that referenced this pull request Aug 11, 2023
Implement the equivalent page fault intermediary handler of the x86
one, by using the `AArch64` specific equivalent bitfields to check
for the type of the fault.

Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #908
unikraft-bot pushed a commit that referenced this pull request Aug 11, 2023
Since you cannot use `ukvmem` without the paging API's, it makes
sense to add a dependency on the `CONFIG_PAGING` configuration entry.

Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #908
@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch/arm arch/arm64 arch/x86_64 area/arch Unikraft Architecture area/include Part of include/uk area/kconfig Part of the Unikraft KConfig option system area/lib Internal Unikraft Microlibrary area/makefile Part of the Unikraft Makefile build system area/plat Unikraft Patform ci/merged Merged by CI lang/c Issues or PRs to do with C/C++ lang/python Issues or PRs to do with Python lib/ukboot plat/common Common to all platforms plat/driver/virtio plat/driver plat/kvm Unikraft for KVM plat/xen Unikraft for Xen
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

None yet

6 participants