Re: [Xen-devel] [PATCH 10/18] xen/arm: Implement GIC suspend/resume functions (gicv2 only)

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 14 Nov 2018, Julien Grall wrote:
> > > >   @@ -1319,6 +1341,129 @@ static void gicv2_do_LPI(unsigned int lpi)
> > > >       BUG();
> > > >   }
> > > >   +static void gicv2_alloc_context(struct gicv2_context *gc)
> > > > +{
> > > 
> > > Is it necessary to allocate them at boot? Can we make them static or
> > > allocate them when we suspend?
> > > 
> > 
> > We need to allocate dynamically because the size of allocated data depends
> > on the number of irq lines, which is not known at the compile time.
> 
> Well you know the upper bound. Why can't you use the upper bound?
> 
> > Alternative is to allocate on suspend, but I believe it is better to do this
> > when the system boots.
> 
> Why is it better?

I'll reply here also to your other point because they are related:

> Suspend/resume is not a critical feature in common case. So I would
> prefer if we disable it when we can't alloc memory.


It is true that suspend/resume is not a critical feature for the common
case, but proceeding as "normal" when a memory allocation fails is not a
good idea: if the hypervisor is so low in memory as to fail in an
allocation like this one, it is not going to be able to work right. In
no other cases in Xen we continue on memory allocation failures, even
for less-critical features.

I suggest that we either allocate statically using the upper bound as
you suggested, although it leads to some memory being wasted. Or, and
this is my favorite option, we allocate it dynamically but we return a
proper error on memory allocation failures. We should at the very
least print an error.

I would prefer if it is done at boot time so that the user can figure
out that their configuration is wrong and fix it straight away, but it
could also be done at suspend time. Better to fail reliably early,
rather than failing unpredictably later.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel