Skip to content

Conversation

@danbugs
Copy link
Contributor

@danbugs danbugs commented Jun 17, 2025

This is an extra feature flag that allows disabling our paging setup (when removed). This is needed to further enable custom guests and should not impact any of our current workloads. init-paging is on by default.

@danbugs danbugs added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Jun 17, 2025
@ludfjig
Copy link
Contributor

ludfjig commented Jun 17, 2025

Nice! Have you considered a runtime config vs cargo feature?

Copy link
Contributor

@simongdavies simongdavies left a comment

Choose a reason for hiding this comment

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

A couple of suggestions

@danbugs
Copy link
Contributor Author

danbugs commented Jun 17, 2025

Nice! Have you considered a runtime config vs cargo feature?

Yes, but for this use-case I think a cargo feature is preferred. Users that are disabling paging will most probably be wanting run a specific type of binary (e.g., Nanvix) where you either want paging or not, so I don't think they'll want to mix-and-match different configs there at runtime. Plus, having to bubble that config option all the way to setup_initial_sregs would be a bit messy, imo.

@ludfjig
Copy link
Contributor

ludfjig commented Jun 17, 2025

Nice! Have you considered a runtime config vs cargo feature?

Yes, but for this use-case I think a cargo feature is preferred. Users that are disabling paging will most probably be wanting run a specific type of binary (e.g., Nanvix) where you either want paging or not, so I don't think they'll want to mix-and-match different configs there at runtime. Plus, having to bubble that config option all the way to setup_initial_sregs would be a bit messy, imo.

I think I personally would prefer a runtime config (opt out), to avoid a large feature matrix.

@danbugs
Copy link
Contributor Author

danbugs commented Jun 17, 2025

Nice! Have you considered a runtime config vs cargo feature?

Yes, but for this use-case I think a cargo feature is preferred. Users that are disabling paging will most probably be wanting run a specific type of binary (e.g., Nanvix) where you either want paging or not, so I don't think they'll want to mix-and-match different configs there at runtime. Plus, having to bubble that config option all the way to setup_initial_sregs would be a bit messy, imo.

I think I personally would prefer a runtime config (opt out), to avoid a large feature matrix.

But then you'd just have a larger runtime config matrix? Not sure if there's an 100% right option here. Are you ok w/ proceeding w/ a feature at first?

cc: @simongdavies ?*

@simongdavies
Copy link
Contributor

Nice! Have you considered a runtime config vs cargo feature?

Yes, but for this use-case I think a cargo feature is preferred. Users that are disabling paging will most probably be wanting run a specific type of binary (e.g., Nanvix) where you either want paging or not, so I don't think they'll want to mix-and-match different configs there at runtime. Plus, having to bubble that config option all the way to setup_initial_sregs would be a bit messy, imo.

I think I personally would prefer a runtime config (opt out), to avoid a large feature matrix.

I don't think it makes sense to have this as a runtime config, since this is something that's either required statically or not, I understand the concern about proliferation of features but this one really should be a feature not a configuration option IMO

@danbugs danbugs force-pushed the cond-init-paging branch 3 times, most recently from ea16e07 to 9285035 Compare June 17, 2025 22:19
simongdavies
simongdavies previously approved these changes Jun 17, 2025
simongdavies
simongdavies previously approved these changes Jun 17, 2025
@danbugs danbugs force-pushed the cond-init-paging branch 6 times, most recently from 67201a9 to 2fea180 Compare June 17, 2025 23:39
This is an extra feature flag that allows disabling our paging setup. This is needed to further enable custom guests and
should not impact any of our current workloads. init-paging is on by default.

Signed-off-by: danbugs <[email protected]>
@danbugs danbugs force-pushed the cond-init-paging branch from 2fea180 to 68256f1 Compare June 17, 2025 23:49
@danbugs danbugs enabled auto-merge (squash) June 18, 2025 00:09
@danbugs danbugs merged commit 30e1ff2 into hyperlight-dev:main Jun 18, 2025
32 checks passed
@danbugs danbugs deleted the cond-init-paging branch June 18, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants