On Tue, 2023-01-17 at 11:06 +0000, Paul Durrant wrote:
On 17/01/2023 11:02, David Woodhouse wrote:
On Tue, 2023-01-17 at 10:56 +0000, Paul Durrant wrote:
I'm just having a hard time seeing why passing 0 to
xen_evtchn_set_callback_param() does anything useful...
+ switch (param >> CALLBACK_VIA_TYPE_SHIFT) {
+ case HVM_PARAM_CALLBACK_TYPE_VECTOR: {
+ struct kvm_xen_hvm_attr xa = {
+ .type = KVM_XEN_ATTR_TYPE_UPCALL_VECTOR,
+ .u.vector = (uint8_t)param,
+ };
HVM_PARAM_CALLBACK_TYPE_VECTOR is 2 AFAICT, so it won't hit that
case.
Also, you appear to be passing the unshifted param to kernel
anyway.
What is the call trying to achieve?
Zero is HVM_PARAM_CALLBACK_TYPE_GSI, with GSI==0. It's basically
disabling event channel delivery for the new kernel.
AFAICT it is doing nothing at this point. Unless I am going insane it
results in this codepath:
+ default:
+ ret = -ENOSYS;
+ break;
+ }
+
+ if (!ret) {
+ s->callback_param = param;
+ s->evtchn_in_kernel = in_kernel;
+ }
So it doesn't result in any cleanup. What am I missing?
Indeed, it doesn't result in any cleanup at *this* point in the series
because HVM_PARAM_CALLBACK_TYPE_GSI hasn't been implemented yet; it's
in a later patch.
The series is broken up into sensible individual patches for review,
not so people can actually *run* with some partial subset of them.
Otherwise I'd have to implement vmstate migration versioning for every
change in the series, and that way lies madness :)
I *will* add a comment in the code explaining what zero means though,
and note in the commit message that it doesn't actually have any effect
will, but will do in a few patches' time.