Re: [Xen-devel] [PATCH v2] xen: Use evtchn_type_t as a type for event channels

[Jürgen Groß <jgross@xxxxxxxx>]

On 08.03.20 14:19, Yan Yankovskyi wrote:
> On Sat, Mar 07, 2020 at 02:41:44PM -0500, Boris Ostrovsky wrote:
> > On 3/7/20 8:43 AM, Yan Yankovskyi wrote:
> > > Make event channel functions pass event channel port using
> > > evtchn_port_t type. It eliminates signed <-> unsigned conversion.
> >
> > >   static int find_virq(unsigned int virq, unsigned int cpu)
> > >   {
> > >         struct evtchn_status status;
> > > -       int port, rc = -ENOENT;
> > > +       evtchn_port_t port;
> > > +       int rc = -ENOENT;
> > >   
> > >   	memset(&status, 0, sizeof(status));
> > >         for (port = 0; port < xen_evtchn_max_channels(); port++) {
> > > @@ -962,7 +963,8 @@ EXPORT_SYMBOL_GPL(xen_evtchn_nr_channels);
> > >   int bind_virq_to_irq(unsigned int virq, unsigned int cpu, bool percpu)
> > >   {
> > >         struct evtchn_bind_virq bind_virq;
> > > -       int evtchn, irq, ret;
> > > +       evtchn_port_t evtchn = xen_evtchn_max_channels();
> > > +       int irq, ret;
> > >   
> > >   	mutex_lock(&irq_mapping_update_lock);
> > >   
> > > @@ -990,7 +992,6 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu, bool percpu)
> > >                         if (ret == -EEXIST)
> > >                                 ret = find_virq(virq, cpu);
> > >                         BUG_ON(ret < 0);
> > > -                       evtchn = ret;
> >
> > This looks suspicious. What would you be passing to
> > xen_irq_info_virq_setup() below?
> Right, this line should be preserved.
>
> > I also think that, given that this patch is trying to get types in
> > order, find_virq() will need more changes: it is supposed to return
> > evtchn_port_t. But then it also wants to return a (signed) error.
>   
> As we don't care which error we got during find_virq call, we can just
> return 0 in case of error, and port number otherwise. Port 0 is never
> valid, so this approach can work for the other functions as well.
> On the other hand, passing port using pointer and returning actual
> error message, as it's done in xenbus_alloc_evtchn(), sounds like a
> better approach overall. What do you think?
You can use the same approach as Xen tools do and define something like:

typedef int evtchn_port_or_error_t;


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel