[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 5/8] xen/evtchn: don't close the static event channel.


  • To: Juergen Gross <jgross@xxxxxxxx>
  • From: Rahul Singh <Rahul.Singh@xxxxxxx>
  • Date: Thu, 7 Jul 2022 12:45:49 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Uk03DcIlcU6IXh9gtiFDnM0oFDBzzoxV/R5zUzRo4RQ=; b=PHbkgyTzCrX9FNXyOKWWaNHVE1JcFwSRT7tYkmH+u6Kwodrw88Hf5sLD6SdFr0FsgqKFt3VHv3/ZnVqhXwJAgRr6NYb5dVLjtemNve5bMR/Un4NyafnfgVs1pW2lz18nZWygLkqD4ipy5UPsmINWeA/VsiPKZbqhs/xYpYijF6w0ubeBSvU+zdtNLNzugHRMDhOTuiQbUz5KfFBbLUpHwsT4VBthp0UVigPwaOLBN+DcYDQMYsAba/0ZH9900ekllmXzBRkYRgcOq51OJcjyVaYgHnkfleAWBlz4D1Sav0cccx/TOblGfAYZJVM+q/wiqGm3vmZjV5gSBUawZbt0YQ==
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Uk03DcIlcU6IXh9gtiFDnM0oFDBzzoxV/R5zUzRo4RQ=; b=SAXmts+K86VSqM2/jTzhWHmZE3OzbbWrLdUUx6QINsNEADgh0z1HXyPkEWAYFfzJty04UTs55HKxmVQdirv5+F/MykYQR7hcM16HNBz9V8Pa9rWmbxEDWMPYVrw/0/m3svshTdhLqhqtQLPJwLgsMrLkzC2ihXaO5rLWhy5ag2CLuh7kCYoej2O9sL7EhzRZYGx5MJxCpKQmqCfIe6aXRCPpzpa/siRlra1Wg55VLJWjAokeXKP0SkyC/R4tYC90hBHJovDROrU1bPc+LDUYyhYRH8Y2WCvXfSDigf9O08GnTJ8gzNCeURcVe5Hw5nr61FuTN2cipRmQ1h6TUhC5+Q==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=nWNl/X3nBwqEMzlww1lZXiKenIgQLXCeyp/WwVx9TEQCIJj2LBMEDix5qx5AQolOD/4B7vkN1rJk+89ABwBSgH4Q0ujD9/7AABSA04JH5FrB2wbODkdRNBuxjsAc6vLj451L/16pZAeO6o5hrg1XXpiCUwUr9HGtNSy8sl50YFDiRCNf37/LySMDwlfFSXVRqnw7QlcYYJlKhuBZpnltqbnjG9veMMCfEgeLS/BBISTwv/HNNGE+NV62HTSqc/yWWz35sKm5nILSEPrrvXYsYhblRkmEnoPpskqBuhLJxq6u3QiKKFgs0eJawgOUBdWzW+gTWBPaq4eEgYsb8048Vg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MiY2SMERUMZafZ0x/VquCTMsrGAQv/u3HQEMQ4Y9Wy9pflDPw+HWHxe0DGhu/szjwSMJvrgXdLA1Kh5t0juoe3rBzzncv2lDKPzdF6sj3NA9DQXjn2zTeXVGesHl6BxXSr8NUifTREFXGF0M4wRqnGbherP+0MnU2fbqw/9O42crDrOWQUgep5wEl0NKmGlwUZZSkS4dibDAlpP5mW2UIftfvlKvuVxfn5ztsxX57zf7JWnCZBd2psYVvnpccOSY8b+VZpJwkB66FSZmNyTcLUiQgNH/FtzOve8r1WwSje+QRLGhYd6Rp55TBKZyhkyQN5WPXUx9/DMCn4Os7PCKOw==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Julien Grall <julien@xxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Thu, 07 Jul 2022 12:46:14 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHYhkX79rj81mJ150e3nBZ0EA7O6K1bhmuAgAGTuoCAAAWegIAHwJEAgAAJPQCAAAcgAIAAB1iAgArhiICAAAgBAIABW/CAgAAGRACAAAgBgIABpqMA
  • Thread-topic: [PATCH 5/8] xen/evtchn: don't close the static event channel.

Hi Juergen,

> On 6 Jul 2022, at 12:33 pm, Juergen Gross <jgross@xxxxxxxx> wrote:
> 
> On 06.07.22 13:04, Julien Grall wrote:
>> (+ Juergen for the Linux question)
>> On 06/07/2022 11:42, Rahul Singh wrote:
>>> Hi Julien,
>>> 
>>>> On 5 Jul 2022, at 2:56 pm, Julien Grall <julien@xxxxxxx> wrote:
>>>> 
>>>> 
>>>> 
>>>> On 05/07/2022 14:28, Rahul Singh wrote:
>>>>> Hi Julien,
>>>> 
>>>> Hi Rahul,
>>>> 
>>>>>> On 28 Jun 2022, at 4:18 pm, Julien Grall <julien@xxxxxxx> wrote:
>>>>>>> a new driver in linux kernel, etc where right now we just need to 
>>>>>>> introduce an extra IOCTL in linux to support this feature.
>>>>>> 
>>>>>> I don't understand why would need a new driver, etc. Given that you are 
>>>>>> introducing a new IOCTL you could pass a flag to say "This is a static 
>>>>>> event channel so don't close it".
>>>>> I tried to implement other solutions to this issue. We can introduce a 
>>>>> new event channel state “ECS_STATIC” and set the
>>>>> event channel state to ECS_STATIC when Xen allocate and create the static 
>>>>> event channels.
>>>> 
>>>> From what you wrote, ECS_STATIC is just an interdomain behind but where 
>>>> you want Xen to prevent closing the port.
>>>> 
>>>> From Xen PoV, it is still not clear why this is a problem to let Linux 
>>>> closing such port. From the guest PoV, there are other way to pass this 
>>>> information (see below).
>>> 
>>> If Linux closes the port, the static event channel created by Xen 
>>> associated with such port will not be available to use afterward.
>>> 
>>> When I started implemented the static event channel series, I thought the 
>>> static event channel has to be available for use during
>>> the lifetime of the guest. This patch avoids closing the port if the Linux 
>>> user-space application wants to use the event channel again.
>>> 
>>> This patch is fixing the problem for Linux OS, and I agree with you that we 
>>> should not modify the Xen to fix the Linux problem.
>>> Therefore, If the guest decided to close the static event channel, Xen will 
>>> close the port. Event Chanel associated with the port
>>> will not be available for use after that.I will discard this patch in the 
>>> next series.
>>> 
>>>> 
>>>>> From guest OS we can check if the event channel is static (via 
>>>>> EVTCHNOP_status()  hypercall ), if the event channel is
>>>>> static don’t try to close the event channel. If guest OS try to close the 
>>>>> static event channel Xen will return error as static event channel can’t 
>>>>> be closed.
>>>> Why do you need this? You already need a binding indicating which ports 
>>>> will be pre-allocated. So you could update your binding to pass a flag 
>>>> telling Linux "don't close it".
>>>> 
>>>> I have already proposed that before and I haven't seen any explanation why 
>>>> this is not a viable solution.
>>> 
>>> Sorry I didn’t mention this earlier, I started with your suggestion to fix 
>>> the issue but after going through the Linux evtchn driver code
>>> it is not straight forward to tell Linux don’t close the port. Let me try 
>>> to explain.
>>> 
>>> In Linux, struct user_evtchn {} is the struct that hold the information for 
>>> each user evtchn opened. We can add one bool parameter in this struct to 
>>> tell Linux driver
>>> via IOCTL if evtchn is static. When user application close the fd 
>>> "/dev/xen/evtchn” , evtchn_release() will traverse all the evtchn and call 
>>> evtchn_unbind_from_user()
>>> for each evtchn. evtchn_unbind_from_user() will call  
>>> __unbind_from_irq(irq) that will call xen_evtchn_close() . We need 
>>> references to "struct user_evtchn” in
>>> function __unbind_from_irq() to pass as argument to xen_evtchn_close() not 
>>> to close the static event channel.  I am not able to find any way to get
>>> struct user_evtchn in function __unbind_from_irq() , without modifying the 
>>> other Linux structure.
> 
> The "static" flag should be added to struct irq_info. In case all relevant
> event channels are really user ones, we could easily add another "static"
> flag to evtchn_make_refcounted(), which is already used to set a user
> event channel specific value into struct irq_info when binding the event
> channel.
> 

As suggested by you, I modified the Linux Kernel by adding “static" flag in 
struct irq_info and
it works fine. We can skip the closing of static channel if required. 

I will send the patch for review once I will send the patch for new ioctl for 
static event channel.

Regards,
Rahul



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.