Re: [PATCH v7 26/51] hw/xen: Add xen_evtchn device for event channel emulation

[Paul Durrant]

On 17/01/2023 10:23, David Woodhouse wrote:
> On Tue, 2023-01-17 at 10:00 +0000, Paul Durrant wrote:
> > > @@ -712,6 +717,11 @@ static int kvm_xen_soft_reset(void)
> > >         CPUState *cpu;
> > >         int err;
> > >     
> > > +    err = xen_evtchn_set_callback_param(0);
> >
> > Doesn't this always result in -ENOSYS?
>
> Hm?
>
> Even at this point in the series, HVM_PARAM_CALLBACK_TYPE_VECTOR works
> and doesn't result in -ENOSYS.
>
> But even if xen_evtchn_set_callback_param() *was* a stub that just
> returned -ENOSYS at this point, that would be OK, surely? We add the
> (other) HVM_PARAM_CALLBACK_TYPE_* support later, which warrants
> separate review because of the GSI and iothread lock fun.

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?

  Paul