[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.
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |