lkml.org 
[lkml]   [2020]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] xen: Use evtchn_type_t as a type for event channels
From
Date


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?

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.

> }
>
> ret = xen_irq_info_virq_setup(cpu, irq, evtchn, virq);
> @@ -1019,7 +1020,7 @@ static void unbind_from_irq(unsigned int irq)
> mutex_unlock(&irq_mapping_update_lock);
> }
>



> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -391,7 +391,7 @@ EXPORT_SYMBOL_GPL(xenbus_grant_ring);
> * error, the device will switch to XenbusStateClosing, and the error will be
> * saved in the store.
> */
> -int xenbus_alloc_evtchn(struct xenbus_device *dev, int *port)
> +int xenbus_alloc_evtchn(struct xenbus_device *dev, evtchn_port_t *port)

Right. But then why is the declaration in include/xen/xenbus.h (at the
very end of the patch) different?

> {
> struct evtchn_alloc_unbound alloc_unbound;
> int err;
> @@ -414,7 +414,7 @@ EXPORT_SYMBOL_GPL(xenbus_alloc_evtchn);
> /**
> * Free an existing event channel. Returns 0 on success or -errno on error.
> */
> -int xenbus_free_evtchn(struct xenbus_device *dev, int port)
> +int xenbus_free_evtchn(struct xenbus_device *dev, evtchn_port_t port)

Here too.

> {
> struct evtchn_close close;
> int err;
> @@ -423,7 +423,7 @@ int xenbus_free_evtchn(struct xenbus_device *dev, int port)

And why not here, especially since you updated format?

>
> err = HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
> if (err)
> - xenbus_dev_error(dev, err, "freeing event channel %d", port);
> + xenbus_dev_error(dev, err, "freeing event channel %u", port);
>
> return err;
> }



>
> diff --git a/include/xen/interface/event_channel.h b/include/xen/interface/event_channel.h
> index 45650c9a06d5..cf80e338fbb0 100644
> --- a/include/xen/interface/event_channel.h
> +++ b/include/xen/interface/event_channel.h
> @@ -220,7 +220,7 @@ struct evtchn_expand_array {
> #define EVTCHNOP_set_priority 13
> struct evtchn_set_priority {
> /* IN parameters. */
> - uint32_t port;
> + evtchn_port_t port;

This definition comes from Xen so I think it needs to be fixed there first.

> uint32_t priority;
> };
>
> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> index 89a889585ba0..4f35216064ba 100644
> --- a/include/xen/xenbus.h
> +++ b/include/xen/xenbus.h
> @@ -218,8 +218,8 @@ int xenbus_unmap_ring(struct xenbus_device *dev,
> grant_handle_t *handles, unsigned int nr_handles,
> unsigned long *vaddrs);
>
> -int xenbus_alloc_evtchn(struct xenbus_device *dev, int *port);
> -int xenbus_free_evtchn(struct xenbus_device *dev, int port);
> +int xenbus_alloc_evtchn(struct xenbus_device *dev, unsigned int *port);
> +int xenbus_free_evtchn(struct xenbus_device *dev, unsigned int port);

These.


-boris

\
 
 \ /
  Last update: 2020-03-07 20:42    [W:2.412 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site