Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2] xen: Use evtchn_type_t as a type for event channels
To:     Yan Yankovskyi <yyankovskyi@gmail.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc:     Jan Beulich <jbeulich@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org
References: <20200307134322.GA27756@kbp1-lhp-F74019>
 <d190793c-fe6b-263e-7793-ccd73f9ccad4@oracle.com>
 <20200308131944.GA18740@kbp1-lhp-F74019>
From:   =?UTF-8?B?SsO8cmdlbiBHcm/Dnw==?= <jgross@suse.com>
Message-ID: <eccf0d74-fdc6-d3bb-ca79-5761008c3efd@suse.com>
Date:   Tue, 10 Mar 2020 07:21:58 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: <20200308131944.GA18740@kbp1-lhp-F74019>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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