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

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Thu, 2018-05-10 at 16:13 +0100, Julien Grall wrote:
> On 10/05/18 16:00, Mirela Simonovic wrote:
> > > If you add your callback to CPU_UP_PREPARE, instead than to
> > > CPU_STARTING, SCHED_OP(init_pdata) wouldn't be called, without
> > > having
> > > to fiddle with priorities.
> 
> This function will enable capabilities on a given CPU, most of them
> are 
> touching system registers. So it is necessary to add the callback to 
> CPU_STARTING.
> 
Right, I guess that answers my question.

> I always like to understand what I maintain :). Why do you need to 
> change the priority if you just return an error in the notifier?
> 
> At the moment, notify_cpu_starting has a BUG_ON() in it. But ideally
> I 
> would like to either replace that BUG_ON by cpu_stop or just
> returning 
> an error to give a chance of the architecture deciding what to do
> (this 
> does not have to be done today).
> 
The problem is that, currently, once we've reached CPU_STARTING, we
assume that the CPU bringup has gone ok, and things can't fail.

Therefore, the only place when we undo what CPU_STARTING does is during
CPU_DEAD, i.e., during hotunplug/suspend/teardown.

I understand the point of having to run stuff on the CPU that is coming
up, as well as your more general point.

However, I don't know whether allowing CPU_STARTING to fail is the best
way to achieve what you want to achieve, nor whether the BUG_ON in
notify_cpu_starting() is the only issue you'll face trying to do that.

I'd say that, if CPU_STARTING can fail, we need an appropriate rollback
step, i.e., the flow must become something like (but I'd appreciate the
opinion of x86 and core hypervisor maintainers about this):

CPU_UP_PREPARE --> CPU_STARTING xx> CPU_DIDNT_START --> CPU_UP_CANCEL

At which point, e.g., from the scheduler point of view, we can try to
put a call to SCHED_OP(deinit_pdata) in CPU_DIDNT_START, and that would
avoid the problem Mirela is facing, without having to play with
priorities.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

Attachment: signature.asc
Description: This is a digitally signed message part

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