Refactor/rename parts of Context?

[iximeow]

The Context (for Context)

I've had a nagging feeling something is a little "off" about Context since we had to add parent_ctx.. I said I'd ruminate on this back when @acfoltzer added it to correct our unsafe impl Send for InstanceHandle a few months ago, and my ruminating has finally yielded fruit (maybe). For context, this field is necessary because if an instance is moved between threads when yielded, when we Context::swap back into the instance we need to update this to swap back to the right host context (lest we return onto the wrong stack and 💥).

The What

It's seemed to me that parent_ctx, backstop_callback, and backstop_data shouldn't really be on Context particularly because they should only exist in one Context: the guest instance we're running. The host's parent_ctx might end up set to f.ex the guest Context but never used, backstop_callback only matters in lucet_context_backstop and so should never be reached in the host, backstop_data is a parameter that only exists for the backstop callback...

So the realization is this: it seems fair to say these are all properties of an Instance, of a specific Context. Further, lucet_context_backstop (and lucet_context_bootstrap) really aren't functions operating on a Context, they operate on an Instance and manipulate the guest's Context.

I think a good name for parent_ctx, backstop_callback, and backstop_data might be InstanceExitData or something of that nature - these taken together are necessary for an Instance to correctly exit its context either in a normal return or in a terminal fault.

The Why

I conceptually treat Context as an encapsulation of the ABI around a function call: arguments, return values, signal handler state, fin. Overloading it with Instance-specific data seems to go beyond the spirit of its intent. If this isn't how yall see it, then maybe this doubles as an argument for it being read that way!

As an additional benefit it moves some instance data off the gnarly 10*8 + 8*16 + 8*2 + 16-style offsets into a Context where my smart brain knows we catch errors in tests but my code review brain gets utterly horrified.

And A How

This functionally would look like lucet_context_{activate,backstop,bootstrap} to lucet_instance_{activate,backstop,bootstrap} (and maybe giving them their own .S or something, I'm less opinionated there), as well as a, few, instructions. I think it's straightforward overall, if this sounds agreeable - this issue would basically be the PR text I'd write there, anyway. 😸

[acfoltzer]

This sounds good to me. My suspicion is that as coroutines become A Thing, it will make even less sense to have these fields on the context, since there will no longer be a 1-1 relationship between the host context and the guest context.

Do you think the new design works in a world where we potentially have multiple guest stacks?

Also, do you think it would work in a world where we swap onto a host-specific stack when doing hostcalls?

[awortman-fastly]

Do you think the new design works in a world where we potentially have multiple guest stacks?

My loose idea was to stuff InstanceExitData at the base of a guest stack in Context::init. On one hand, if we're only running one guest stack at a time, then only one would return at a time, and it would use whatever InstanceExitData was set to when it started running. So even an InstanceExitData on Instance would do the trick. The other option is to move this back onto the base of a guest stack, which it was waaay back before GuestData even existed - I prefer that a little more just for reasons of flexibility if we have multiple stacks, but moving it at the point we need it should be fine?

Also, do you think it would work in a world where we swap onto a host-specific stack when doing hostcalls?

I think this is not prohibitive, at least. So long as a hostcall doesn't return to a different guest stack (conceptually: rescheduled across coroutines?), we could set them up with a lucet_hostcall_backstop similar to an instance. If we did want hostcalls to potentially return to different stacks (hostcalls == yield points for threads?), having the guest initialize some pointers for nebulous Scheduling Stuff in #[lucet_hostcall] seems like a sufficient springboard to muck around with whatever we'd pass to lucet_hostcall_backstop (a guest Context?).

[cratelyn]

I'm happy to pick this one up, I've wanted to do this for a while 🙂