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

[Juergen Gross <jgross@xxxxxxxx>]

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.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature