Re: [RFC PATCH v2 20/22] i386/xen: HVMOP_set_param / HVM_PARAM_CALLBACK_IRQ

[Paul Durrant]

On 21/12/2022 01:41, David Woodhouse wrote:
> On Mon, 2022-12-12 at 16:16 +0000, Paul Durrant wrote:
> > > @@ -287,24 +289,53 @@ static bool kvm_xen_hcall_memory_op(struct kvm_xen_exit 
> > > *exit,
> > >        return true;
> > >    }
> > >    
> > > +static int handle_set_param(struct kvm_xen_exit *exit, X86CPU *cpu,
> > > +                            uint64_t arg)
> > > +{
> > > +    CPUState *cs = CPU(cpu);
> > > +    struct xen_hvm_param hp;
> > > +    int err = 0;
> > > +
> > > +    if (kvm_copy_from_gva(cs, arg, &hp, sizeof(hp))) {
> > > +        err = -EFAULT;
> > > +        goto out;
> > > +    }
> > > +
> > > +    if (hp.domid != DOMID_SELF) {
> >
> > Xen actually allows the domain's own id to be specified as well as the
> > magic DOMID_SELF.
> >
> > > +        err = -EINVAL;
> >
> > And this should be -ESRCH.
>
> Oops, fixed that after posting v4 series. Fixed in:
>
> https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xenfv
>
> I fixed the similar -EPERM in evtchn_status_op() too.
>
> > > +        goto out;
> > > +    }
> > > +
> > > +    switch (hp.index) {
> > > +    case HVM_PARAM_CALLBACK_IRQ:
> > > +        err = xen_evtchn_set_callback_param(hp.value);
> > > +        break;
> > > +    default:
> > > +        return false;
> > > +    }
> > > +
> > > +out:
> > > +    exit->u.hcall.result = err;
> >
> > This is a bit on the ugly side isn't it? Why not return the err and have
> > kvm_xen_hcall_hvm_op() deal with passing it back?
>
> Because 'return false' means qemu will whine about it being
> unimplemented.

Ah, ok. Yes, I did suggest turning that into a trace, which would mean 
that only those who cared would see such a whine.

  Paul