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

[Rahul Singh <Rahul.Singh@xxxxxxx>]

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.

Regards,
Rahul