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

make DefineCAmkESVMFileServer() more flexible #31

Merged
merged 2 commits into from
May 3, 2023

Conversation

axel-h
Copy link
Member

@axel-h axel-h commented Jul 7, 2022

  • allow custom type and instance names
  • allow passing files for DefineCAmkESVMFileServer(), this avoids calling AddToFileServer()
  • AddToFileServer() becomes just a light wrapper
  • use instance specific name for archive and lib

This allows doing customizing the file server component name and doing all in one call:

DefineCAmkESVMFileServer(
    INSTANCE "myFileServer"
    FILES
        "linux:${VM_IMG_LINUX}"
        "linux-initrd:${VM_IMG_ROOTFS}"
        "linux-dtb:${VM_IMG_DTB_PATCHED}"
    DEPENDS
        target_dtb_gen
)

See seL4/camkes-vm-examples#37 for a usage examples.

@axel-h axel-h marked this pull request as draft July 7, 2022 01:52
@axel-h axel-h marked this pull request as ready for review July 26, 2022 11:10
@wom-bat
Copy link
Member

wom-bat commented Aug 9, 2022

Need to investigate and fix the failing tests...

@axel-h
Copy link
Member Author

axel-h commented Aug 9, 2022

Need to investigate and fix the failing tests...

Don't bother, I think I have a clue what fails there and what I have to improve to make it work. Just need to find some time to work in this again.

camkes_vm_helpers.cmake Outdated Show resolved Hide resolved
@axel-h
Copy link
Member Author

axel-h commented Mar 4, 2023

Could I get some feedback here, any chances to merge this?

Copy link
Member

@kent-mcleod kent-mcleod left a comment

Choose a reason for hiding this comment

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

Nice improvement! The ability to set CPIO output paths introduced in seL4/seL4_tools#151 will need to be merged before the second commit here can be merged.

@axel-h
Copy link
Member Author

axel-h commented Mar 23, 2023

Adding a feature that allows an empty file string also in DefineCAmkESVMFileServer(). This can be used for files that do no exist in every configuration. The caller can use a variable in this case instead of a fixed string, and leave the variable empty. This is used in the vm_minimal example, where not all platforms have initrd.

@axel-h
Copy link
Member Author

axel-h commented Mar 23, 2023

Adding support for file lists as parameter also, because this comes handy for the existing examples.

@axel-h axel-h force-pushed the patch-axel-5 branch 4 times, most recently from 0a858c7 to 81a19b2 Compare March 23, 2023 16:03
@axel-h
Copy link
Member Author

axel-h commented Mar 23, 2023

Avoid creating some unnecessary intermediate helper CMake targets.

@axel-h axel-h force-pushed the patch-axel-5 branch 3 times, most recently from e29c75b to 9fdb072 Compare March 28, 2023 18:09
@axel-h
Copy link
Member Author

axel-h commented Mar 28, 2023

Changed the order of the commits and split them up further. That allows cherry-picking the CPIO custom folder feature without taking the change in the CMake FileServer functions.

@axel-h
Copy link
Member Author

axel-h commented Mar 29, 2023

Removing the dependency of this PR on seL4/seL4_tools#151 and seL4/camkes-vm-examples#37. It's not necessary to use a specific folder, if the instance name is simply used in the file name of the archive and lib. I will make another PR to move the build artifact out out of the build root folder, so this does not block this PR.

@axel-h axel-h force-pushed the patch-axel-5 branch 2 times, most recently from 2078b78 to c98e3de Compare March 30, 2023 18:35
@axel-h axel-h force-pushed the patch-axel-5 branch 4 times, most recently from 3c51daf to 8349314 Compare April 12, 2023 16:20
@axel-h
Copy link
Member Author

axel-h commented Apr 12, 2023

Create dedicated targets for each file server instances, and set the properties for fixed names within each target. This is cleaner than putting everything into a single vm_fserver_config target. And it allows keeping the property names FILES and DEPS, which does not break any existing usages that might exist here.

@axel-h axel-h force-pushed the patch-axel-5 branch 4 times, most recently from e780d10 to b5208df Compare April 19, 2023 14:12
- Define variables to hold the actual names.
- Add comments about he steps

Signed-off-by: Axel Heider <[email protected]>
- allow custom type and instance names
- allow passing files for DefineCAmkESVMFileServer(), this avoids
  calling AddToFileServer()
- AddToFileServer() becomes just a light wrapper
- use instance specific name for archive and lib
- remove the target vm_fserver_config, as it obsolete now

Signed-off-by: Axel Heider <[email protected]>
@axel-h axel-h added the hw-test camkes-vm-examples hardware builds + runs for this PR label Apr 28, 2023
@axel-h
Copy link
Member Author

axel-h commented May 3, 2023

Can we merge this?

Copy link
Member

@lsf37 lsf37 left a comment

Choose a reason for hiding this comment

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

Since the dependency on seL4/seL4_tools#151 is gone, I think we can merge this one, yes.

@lsf37 lsf37 merged commit d08eb2f into seL4:master May 3, 2023
@axel-h axel-h deleted the patch-axel-5 branch May 4, 2023 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hw-test camkes-vm-examples hardware builds + runs for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants