icexin / eggos

A Go unikernel running on x86 bare metal
http://eggos.icexin.com
MIT License
2.21k stars 112 forks source link

Call various kernel inits directly, rather than via an import #99

Open jspc opened 2 years ago

jspc commented 2 years ago

See: https://github.com/icexin/eggos/issues/98

Because of the way go loads init functions from packages, any steps which must occur before kernel init (setting loglevels, perhaps) requires a bit of hacking.

By generating these calls as a first class init function for a given unikernel we can avoid having to hack around the golang init function loader.

This change adds a generate command to egg, which can be to allow developers to maintain their own eggos include file to allow for developers to change how their unikernel is run, such as being able to change the value of GOMAXPROCS, or disable networking.

The build command now checks for the presence of this file. If it does not exist then the build command will use the current method of generating a file and overlay. Otherwise it uses the user specified file.

Finally, the file we generate is prefixed with zz_ to try and influence when the file loads, in order to let user specified init functions (like setting log levels) to run first

jspc commented 2 years ago

Hrm, not sure how to interpret that test failure- I actually can't even get mage -v test to run locally.

Any pointers @icexin ?

icexin commented 2 years ago

It seems that a driver is initialized twice, causing the io address to be remapped. This may be the reason. I'm not too sure. You can debug in this direction.

jspc commented 2 years ago

Nice one, thanks!

jspc commented 2 years ago

Ah, yeah- I see now. Because the repo contains eggos.go, which already contains all the init stuff, when we then build the test unikernel we end up initialising stuff twice- once in eggos.go, and once in zz_load_eggos.go.

I need to think about this more. I could remove eggos.go and use egg generate to create zz_load_eggos.go instead- this keeps backwards compatibility, but might be ugly.

icexin commented 2 years ago

Thanks @jspc, I will find time to review the code

jspc commented 2 years ago

Cheers @icexin - no rush, enjoy your weekend!

icexin commented 2 years ago

Hi, @jspc:

This PR addresses the ability to manually configure driver modules. The solution is to generate a go file to decide which modules to load and in what order. Here are some of my thoughts on this PR:

  1. Users may need a deep understanding of eggos to decide which modules to keep and which modules to delete. For example, the pci module needs be initialized before the e1000 module. Of course eggos is in the early stages and it's fine to do so.
  2. The default template code is a string in the go file, which is slightly less maintainable.

In the long run, a module manager may be needed to provide module initialization, dependency management and configuration management. But it is out of scope of this PR.

Any thought?

icexin commented 2 years ago

BTW, mage qemu will fail because app/kmain is still referencing _ "github.com/icexin/eggos". I think if the initialization code is managed by egg generate, the eggos.go file in the root directory may not need to keep.

mewmew commented 2 years ago

As there are still some design considerations (e.g. https://github.com/icexin/eggos/pull/99#issuecomment-1031607291), I'd prefer to keep discussing various options before merging this PR (or a new PR with an alternative approach).

We can consider what potential solutions there may be for using/initializing only parts of eggos, and try to evaluate pros/cons for those different approaches. Perhaps for this discussion, it's better to open an issue where we can keep such discussions concentrated.

For instance, it may be possible to envision using eggos as a library which is imported by the user's unikernel. Such an approach may either use default configurations (e.g. by import _ "github.com/icexin/eggos"), or configure different aspects, such as what drivers to use.

jspc commented 2 years ago

The two original methods of using eggos in a unikernel work exactly the same as before. Everything is completely 100% backwars compatible.

  1. Loading import _ "github.com/icexin/eggos" still works the same as before; that module has the same code as before, it's just the filename that changes
  2. Running egg {build|pack|etc} does the same thing- if the generated file does not exist then egg will template it and build it using the current overlay.json method.

The major change this PR introduces is to make that generated file contain various driver.Init() calls directly, rather than importing eggos.

The reason it does this is to allow things like loglevels to be set as early as possible without having to hack around the module loader.

If the additional egg generate command is confusing matters, by making it seem like the workflow must contain that step, then we can change the documentation.

Being able to adjust what boots (I, for instance, don't need to configure mice and keyboard support, and I'm building for a machine with more than 8 cores) was useful for me, and will unstick us later when we have more driver support, but if it's premature then that's cool too.

@icexin - good point re: the templated string. I pretty much followed what was there before. I started writing a tool to generate that file based on inputs, but before I knew it I was nearly writing make menuconfig and so I went back to a string.