Re: [PATCH v3 10/10] xen/arm: Enable errata for secondary CPU on hotplug after the boot

From: Stefano Stabellini <sstabellini@kernel.org>
To: Julien Grall <julien.grall@arm.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Davorin Mista <dm@aggios.com>,
	Xen Devel <xen-devel@lists.xen.org>,
	Dario Faggioli <dfaggioli@suse.com>,
	Mirela Simonovic <mirela.simonovic@aggios.com>
Subject: Re: [PATCH v3 10/10] xen/arm: Enable errata for secondary CPU on hotplug after the boot
Date: Mon, 14 May 2018 09:59:18 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1805140955310.8839@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <b5666982-5268-0007-a026-fe45446a5080@arm.com>

On Mon, 14 May 2018, Julien Grall wrote:
> On 11/05/18 22:47, Stefano Stabellini wrote:
> > On Fri, 11 May 2018, Dario Faggioli wrote:
> > > On Fri, 2018-05-11 at 14:08 +0100, Julien Grall wrote:
> > > > The whole idea here is we have only one place taking the decision and
> > > > we
> > > > don't spread BUG_ON()/panic/stop_cpu everywhere. The benefit is
> > > > having
> > > > only one place to fix over multiple one because very likely the
> > > > decision
> > > > is the same everywhere.
> > > > 
> > > > I agree that today it will end up to crashing the system because of
> > > > the
> > > > BUG_ON. But that's a separate topic.
> > > > 
> > > Yes!!! :-D
> > > 
> > > I.e., as I've said countless times, I would think that a series which
> > > introduces a CPU_STARTING notifier that fails, should also deal with
> > > adjusting the CPU process accordingly.
> > > 
> > > *BUT* if you ARM people are ok with arch/arm/ code that does that,
> > > perhaps with a comment saying something like:
> > > 
> > > "This will cause us to hit the BUG_ON() in notify_cpu_starting(). To
> > > fix that, we need to properly change the CPU bringup code, which will
> > > happen in a leter series."
> > > 
> > > that would also work, I guess. :-)
> > 
> > Yes, I think that returning error with an in-code comment on top is the
> > best solution.
> 
> It is the second best solution ;). If we consider the notifier can return an
> error, then best solution is to fix notify_cpu_starting().
>
> I would be ok with the second best solution if we have someone to fix it for
> Xen 4.12. Per my understanding, Mirela is not going to do it. So what's the
> plan here?

I can look at fixing notify_cpu_starting(). I am also OK with you
reworking the vmap code as you suggested below. Regardless, I think
Mirela should go ahead with the comment now. Then, either you or me are
going to come in and remove the comment one way or another (either
fixing notify_cpu_starting or imposing all the callbacks to never return
an error).

> Another solution is to impose all the enable callbacks to never return an
> error (AFAICT Linux is just ignoring the return of the callback)).
> 
> Today, we happen to return an error only in the case vmap is failing (used to
> remapped vector table read-write). It might be possible to avoid the potential
> re-mapping failure by reworking the code.
> 
> I could explore that solution if we prefer going towards imposing all the
> enable callbacks to never return an error.

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