On Tue, Jul 19, 2016 at 5:19 PM, Thomas Leonard <[email protected]> wrote:
> On 19 July 2016 at 16:47, Martin Lucina <[email protected]> wrote: > > Hi all, > > > > the mirage-solo5 package (i.e. the Solo5 "platform bindings") currently > > bundle C stubs for unrelated packages which get linked into a > > libsolo5camlbindings.a. > > > > Based on some discussion on Slack and also ongoing in a PR Mindy pointed > me > > to (https://github.com/mirage/io-page/pull/34), is the general plan to > move > > as much C stubs as possible directly into the packages that actually use > > them? > > Yes. Otherwise, we tend to fix bugs in only one copy, or change the > signature in only one place (and the typechecker can't help you here, > because they're C interfaces). > > We should certainly do this when we can share the same source C file > across platforms. Currently the build tooling isn't very good. We have > a hacky oasis script that gets the Xen cflags. Hopefully this is > better with topkg, but I haven't tried that. > > It's less clear what to do if the code is both platform and package > specific. In that case we should probably make a separate opam package > for it and avoid using the linker hack. > > > If so, I have a bunch of work ahead of me and will need to postpone > > releasing Mirage/Solo5 to opam.ocaml.org until this is complete, since > if I > > release what we have now then cleaning this up afterwards will be very > > painful. > > > > The following is an inventory of C stubs we've "accumulated" in > > mirage-solo5 so far: > > > > alloc_pages_stubs.c: > > caml_alloc_pages(), for io-page even though Solo5 devices do not > > *currently* require page-aligned buffers. > > > > barrier_stubs.c: > > caml_memory_barrier(): Not sure who is the downstream consumer of > this? > > This is used internally by mirage-platform, so I'd expect this to live > in mirage-platform-solo5. > Hmm, but it's also used by shared-memory-ring. I think the platform > should expose an OCaml interface to this. > In mirage-platform it looks like it's called by OCaml function Xenctrl.xen_mb(), but I think this function might be dead. I suspect the only user is the shared-memory-ring code and we forgot to remove this when we moved shared-memory-ring out of the platform? > > caml_cstruct_unsafe_load_uint32(), caml_cstruct_unsafe_save_uint32(): > > Presumably consumed by cstruct? > > Looks like it's actually shared-memory-ring using it. Odd. > IIRC the shared memory ring protocol requires the use of atomic 32-bit loads and stores, or else bad stuff happens. Cstruct uses ocplib-endian normally, but the ocplib-endian functions use (used to use?) a series of byte-sized loads and stores to cope with different possible alignments. This was not a fun bug to work on :-) > > > checksum_stubs.c: > > A copy of the checksum stubs from tcpip. Should be built out of that > > package. > > I think tcpip already builds this. We should remove it from > mirage-platform now. > > > clock_stubs.c: > > unix_gettimeofday(), caml_get_monotonic_time(): These are used by (at > > least) mirage-clock-xen, which we re-used for Solo5 since it has the > > same interfaces. > > They're also used by the platform itself: > > https://github.com/mirage/mirage-platform/blob/9f1a5db1b3681f4e3b4b747931903d7085cce042/xen/lib/time.ml#L34 > > > cstruct_stubs.c: > > caml_blit_bigstring_to_string(), caml_blit_string_to_bigstring(), > > caml_blit_bigstring_to_bigstring(), caml_compare_bigstring(), > > caml_fill_bigstring(), caml_check_alignment_bigstring(). > > > > Based on the name (came from the original Xen bindings), I presume > > these are for cstruct? > > Yes, should move to cstruct. > > > main.c: > > caml_poll(): Used by the Mirage/Solo5 scheduler, this actually > belongs > > here though could go in a better-named file. > > > > caml_get_cmdline(): Belongs in mirage-bootvar-solo5. > > > > mm.c: > > Unimplemented versions of stub_heap_get_pages_total(), > > stub_heap_get_pages_used(). These never get called, not sure who's > > supposed to be using them. > > OS.MM (in mirage-platform) uses these: > > > https://github.com/mirage/mirage-platform/blob/9f1a5db1b3681f4e3b4b747931903d7085cce042/xen/lib/mM.ml > > > solo5_{block,console,net}_stubs.c: > > These landed here as a shortcut while we were doing the port, they > > belong in the respective mirage-{block,console,net}-solo5 packages. > > > > Thoughts? Also any comments / corrections to the above inventory much > > appreciated. > I wonder if we could simplify things a little bit by standardising and documenting the C-level interface a little more? For example in the Io_page caml_alloc_pages we've got some #ifdefs to use `_xmalloc` on `__MINIOS__`, `malloc` (should really be `aligned_malloc`) on `_WIN32` and `posix_memalign` on the rest. Perhaps minios and solo5 should provide a `posix_memalign` function? Cheers, Dave
_______________________________________________ MirageOS-devel mailing list [email protected] https://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel
