Re: [PATCH 2/8] xen/evtchn: modify evtchn_alloc_unbound to allocate specified port

[Julien Grall <julien@xxxxxxx>]

On 21/07/2022 13:50, Rahul Singh wrote:
> Hi Julien,
Hi Rahul,

> > On 20 Jul 2022, at 12:16 pm, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi Rahul,
> >
> > On 20/07/2022 10:59, Rahul Singh wrote:
> > > > On 13 Jul 2022, at 1:29 pm, Julien Grall <julien@xxxxxxx> wrote:
> > > >
> > > >
> > > >
> > > > On 13/07/2022 13:12, Bertrand Marquis wrote:
> > > > > > On 13 Jul 2022, at 12:31, Julien Grall <julien@xxxxxxx> wrote:
> > > > > > > I can't
> > > > > > > see why it would be wrong to have a more tight limit on static ports
> > > > > > > than on traditional ("dynamic") ones. Even if only to make sure so
> > > > > > > many dynamic ones are left.
> > > > > > This is similar to Xen forbidding to close a static port: it is not the 
> > > > > > hypervisor business to check that there are enough event channel ports freed 
> > > > > > for dynamic allocation.
> > > > > On other side we need to be cautious not to add too much complexity in the code 
> > > > > by trying to make things always magically work.
> > > > > If you want Xen to be accessible to non expert by magically working all the 
> > > > > time, there would be a lot of work to do.
> > > > It is not clear to me whether you are referring to a developper or admin here.
> > > >
> > > > On the admin side, we need to make sure they have an easy way to configure 
> > > > event channels. One knob is always going to easier than two knobs.
> > > >
> > > > On the developper side, this could be resolved by better documentation in the 
> > > > code/interface.
> > > >
> > > > Cheers,
> > > To conclude the discussion, If everyone agree I will add the below patch or 
> > > similar in the next version to restrict the
> > > max number of evtchn supported as suggested.
> > I am fine if the limit for domU is fixed by Xen for now. However, for dom0, 
> > 4096 is potentially too low if you have many PV drivers (each backend will need 
> > a few event channels). So I don't think this wants to be fixed by Xen.
> Agree.
> > I am not entirely sure we want to limit the number of event channels for dom0. 
> > But if you want to, then this will have to be done via a command line option 
> > (or device-tree property).
> We need to support the static event channel for dom0 also, in that case, we 
> need to restrict the max number of evtchn for dom0 to mitigate the security 
> issue.
It sounds like there are some misundertanding or I am missing some 
context. The static event channels will be allocated at boot, so the 
worse that can happen is it will be slower to boot.
My point regarding fifo was more in the generic case of allowing the 
caller to select the port. This would be a concern in the context of 
non-cooperative live-migration. An easy way is to restrict the number of 
ports. For you, this is just an increase in boot time.
Furthermore, there is an issue for dom0less domUs because we don't limit 
the number of port by default. This means that a domU can allocate a 
large amount of memory in Xen (we need some per-event channel state). 
Hence why I suggested to update max_evtchn_channel.
> If the admin set the value greater than 4096 (or what we agreed on) and static 
> event channel support is enabled we will print the warning to the user related 
> to fill
> the hole issue for FIFO ABI.
See above. I don't see the need for a warning. The admin will notice 
that it is slower to boot.
See above.

Cheers,


> Regards,
> Rahul
--
Julien Grall