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

hypercall revision 18 #59

Open
anttikantee opened this issue Jan 4, 2014 · 23 comments
Open

hypercall revision 18 #59

anttikantee opened this issue Jan 4, 2014 · 23 comments

Comments

@anttikantee
Copy link
Member

We probably need to version the hypercall layer at some point. Start collecting things to fix here:

  1. anonmmap() is overloaded: it both serves the kernel counterpart to proplib and as a way to allocate executable memory. explicitly splitting it into two interfaces might be better
  2. rumpuser_bio(): make it a hypercall private to rumpvfs? or even better: create a rumpblk device component
  3. console hypercall: ability to read console input (in addition to the current rumpuser_putchar() write ability)
  4. clarify the hypervisor upcalls for interrupt enter/exit (yes, not strictly hypercalls, but a rumpuser ABI addition nonetheless)
  5. hypercall clock resolution: instead of relying on ticks (which rely on HZ), there should be high-precision time counter (which can be rumpuser_clock_gettime()) and a way to know its resolution (which does not currently exist)
  6. take a machete to rumpuser_dl_bootstrap() and separate the calls
  7. remove rumpuser_dl_globalsym(), it's no longer necessary
  8. pass knowledge of PAGE_SIZE from the rump kernel to the hypervisor, so as to make it possible to optimize the hypercall memory allocator for the common case
  9. add some sort of "change scheduling priority" hypercall
  10. add a variant of rumpuser_clock_sleep() which does not cause rump kernel scheduling (for delay())

[to be continued as I recall earlier ideas]

@justincormack
Copy link
Member

Its a bit odd having size_t in the hypercall definitions, as that is kind of posix specific, if its going to be changed, how about changing to ulong or int64_t?

@justincormack
Copy link
Member

It would be nice if rumpblk was not attached to rumpvfs, I need a different block device implementation, so it would be nice if I didnt have to have this one as well, although it is not huge. Ideally ETFS would be a module too, if rumpvfs is going to be restructured - its the sort of thing that you might want to customise.

@anttikantee
Copy link
Member Author

Yes to rumpblk. There are two reasons why it's attached the rumpvfs:

  1. historic
  2. avoids too many components from having to be linked to run a file system driver. You already need -lrumpdev -lrumpvfs -lrumpdev_disk, and so now you'd need -lrumpdev_rumpblk too.

I think "2" could be solved by some automated program which would take as input e.g. rumpfs_ffs and dump all the other required components.

@anttikantee
Copy link
Member Author

I think I need to trick some C type guru to critically auditing the hypercall interface types. There's all kinds of confusion with signedness etc. I'm not sure how size_t could be problem, though. If the host does not provide it, the hypercall implementation can define it by itself (that's partially the reason why rumpuser.h doesn't include any headers). I think the chances of size_t being inconsistently defined are close to 0.

@justincormack
Copy link
Member

Re dependencies, if you are using shared libs cant they just be linked, so you link rumpdev_rumpblk to rumpdev_disk, rumpvfs rumpdev and then you can just load it and get all the deps pulled in. If you use static libs then that won't work obviously, but most people use dynamic.

@anttikantee
Copy link
Member Author

The DT_NEEDED approach doesn't really work with how the NetBSD build system works, because you first need to build a lib before you can link to it, and the build it done per-subdir instead of in dependency order.

@justincormack
Copy link
Member

Re size_t, if you are going to define it yourself you want it called eg rumpuser_size_t, not use the reserved namespace.

On signedness, 64 bit types should never be unsigned in my view. And all types should be 64 bit...

@anttikantee
Copy link
Member Author

Isn't _t a reserved namespace, so technically rumpuser_size_t is in a reserved namespace too ...

Anyway, there's some need for adjustments, but not the most critical thing.

@anttikantee
Copy link
Member Author

added console hypercalls to the list

@anttikantee
Copy link
Member Author

added "4) clarify the hypervisor upcalls for interrupt enter/exit". not 100% sure what this means yet (will know more when I try to optimize softint processing). So this is mostly pencilled down so that I don't forget to look into it.

@justincormack
Copy link
Member

I think rumpuser_getrandom needs looking at. The only uses call the PRNG case not proper randomness. It should probably be a private interface for an in kernel driver that injects some real entropy at boot and occasionally later. There is no real reason to provide a PRNG interface as NetBSD has perfectly good ones and platforms might well not. If the platform cant provide real randomness then of course you cant run the driver.

@anttikantee
Copy link
Member Author

If the driver should be a part of every rump kernel, it technically should be in rumpkern. I want to avoid having more or less mandatory drivers outside of rumpkern, because it makes management of them quite difficult, i.e. if you always have to type "-lrumpkern_rnd -lrump", why not just making it "-lrump".

If the hypercall interface is used incorrectly currently, it's one matter. If the interface prevents it from ever being used correct, it's a completely different matter. But the general idea is that host entropy is provided by the interface. If the host can't provide entropy, well, that's again another matter.

I've document the [intentions of] rumpuser_getrandom() in the rumpuser man page.

So, is the interface wrong, or just how it's currently used and implemented?

@justincormack
Copy link
Member

Currently it can provide access to either /dev/random or /dev/urandom, I dont really see the rationale for providing urandom access. In particular there is no way to estimate the amount of entropy it contains, so you cant feed the entropy estimate, so there is no way to provide a proper random source based on that.

The blocking interface is not that useful. A host that doesnt have any entropy source (eg embedded system) is going to have to block forever. So the hard random nonblock case is the useful one.

The interface consumers do need fixing, they only use the urandom interface.

@anttikantee
Copy link
Member Author

Added "5: clock resolution hypercall"

@anttikantee
Copy link
Member Author

Added "6) take a machete to rumpuser_dl_bootstrap() and separate the calls"

First of all, with attribute((constructor)) the link set locating calls are no longer absolutely necessary.

Second, populating the kernel symbol table is both wasteful (can be populated when the linker is first used, which is ~never) and causes portability question ("how do I implement this on !ELF?")

@anttikantee
Copy link
Member Author

Added "7) remove rumpuser_dl_globalsym()"

@anttikantee
Copy link
Member Author

Added "8) pass knowledge of PAGE_SIZE from the rump kernel to the hypervisor"

@anttikantee
Copy link
Member Author

Added "9) add some sort of "change scheduling priority" hypercall"

Let's say that a userland program calls nice(). That info should reach the underlying scheduler somehow. Currently it goes to sched_nice() in scheduler.c, and then nothing happens. An informational hypercall for changing a thread's scheduling priority would allow for better mimicking of real environments.

This is up for discussion.

Edit: also, this depends on adding set/getpriority() as rump kernel syscalls.

@justincormack
Copy link
Member

It would be nice to have rumpuser_printk() which does not, unlike rumpuser_dprintf(), do formatted output. Mostly formatted output is used to dump structures, and for most purposes is not needed. This would mean you could safely do nothing for rumpuser_dprintf(), saving implementing some code for minimal environments.

@anttikantee
Copy link
Member Author

You didn't say what rumpuser_printk() would do ;)

If it's just essentially puts(), I don't understand why it needs to be a hypercall.

Note that rumpuser_dprintf() is never required for functionality, and you can always safely not implement it and have things work.

@justincormack
Copy link
Member

Yes, just puts(); yes I guess none of the cases are actually needed so maybe not implementing is the best answer...

@anttikantee
Copy link
Member Author

Plus, in a pinch, you can implement rumpuser_dprintf() as just a puts() of the fmt string.

@anttikantee
Copy link
Member Author

Added "10")

For efficiently implementing "delay()" in a virtual environment, we need a sleep hypercall which doesn't sleep ... uhm, I mean which doesn't sleep in the rump kernel context, i.e. doesn't do unschedule() / schedule().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants