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

From: JÃrgen GroÃ
Date: Tue Mar 10 2020 - 02:22:03 EST


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