On Thu, Oct 21, 2021 at 05:39:34PM +0200, Paolo Bonzini wrote:
On 06/10/21 22:37, Michael Roth wrote:
+struct sev_sync_data {
- uint32_t token;
- bool pending;
- bool done;
- bool aborted;
- uint64_t info;
+};
Please add a comment explaining roughly the design and what the fields are for. Maybe the bools can be replaced by an enum { DONE, ABORT, SYNC, RUNNING } (running is for pending==false)?
That makes sense. And SYNC is basically pending==true.
Though since it would be the test code calling sev_check_guest_sync() that sets pending to true after each sync, "RUNNABLE" might be a better name since whether it's actually running depends on a separate call to vcpu_run().
Also, for the part that you can feel free to ignore: this seems to be similar to the ucall mechanism. Is it possible to implement the ucall interface in terms of this one (or vice versa)?
One idea could be to:
move ucall to the main lib/ directory
make it use a struct of function pointers, whose default implementation
would be in the existing lib/ARCH/ucall.c files
add a function to register the struct for the desired implementation
make sev.c register its own implementation
That seems doable. We'd also need to register an opaque pointer that, in the case of SEV, would be for the shared page used for syncing. The guest would also need the pointers for the ops/opaque, but ucall_init() could additionally set some globals to provide the GVAs for them in the guest environment.
Will look at that for the next spin.
Thanks,
Paolo