Hi Juergen,
> On 6 Jul 2022, at 12:33 pm, Juergen Gross <jgr...@suse.com> 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 <jul...@xen.org> wrote:
>>>>
>>>>
>>>>
>>>> On 05/07/2022 14:28, Rahul Singh wrote:
>>>>> Hi Julien,
>>>>
>>>> Hi Rahul,
>>>>
>>>>>> On 28 Jun 2022, at 4:18 pm, Julien Grall <jul...@xen.org> 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
