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

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Fri, 2018-05-11 at 15:44 +0200, Mirela Simonovic wrote:
> Hi Dario and Julien,
> 
Err... you've dropped the list and everyone else but me. Re-adding...

> Thanks for the feedback to both. 
>
You're welcome. :-) 

> I think we need to roll back here. I
> believe the root cause of this is an attempt to do errata workarounds
> using notifiers.
> But let me try to enumerate all the options I see as possible
> solutions:
> 
> 1) Don't use notifiers to do errata workaround. Do it before
> CPU_STARTING fires, essentially from start_secondary() before calling
> notify_cpu_starting(). But we need to stop the CPU within
> start_secondary() if enabling errata fails. In start_secondary()
> stop_cpu is already done so I don't see why would an additional call
> be a problem.
> 
I'm no expert of ARM's start_secondary(). Indeed it looks like it can
fail already, so what you say seems to make sense, but really, I better
don't comment on this and leave it to ARM people.

> 2) Still try to use notifiers. We have few options here:
> 2.a) Enabling capability must not fail because a notifier at this
> stage should not fail. This would mean that function to which
> 'enable'
> ptr points (defined in struct arm_cpu_capabilities) has to return
> void
> instead of int. This doesn't seem right to me.
>
It's not.

> 2.b) Change scheduler and whatever else is needed (right place to
> refer to escalation of the scope of series :) in order to make
> CPU_STARTING possible to fail. 
>
Nope, this is just as wrong as 2.a.

> I'm not the person to do that since it
> affects way beyond what I suppose to do. Please note here that I'm
> not
> running away from doing the job. I'm just concerned that this will
> compromise the actual work I suppose to do from the funding/time
> perspective.
> 2.c) Return an error and hit BUG_ON. Add comments as Dario proposed.
> I
> need to state here that I probably won't be the one to implement the
> following series that allows CPU_STARTING to fail.
> 
Yes, this is correct, from the point of view of ARM vs common code
interaction.

Whether or not it is ok to leave this pending, is again up to ARM
people, IMO, as it would be ARM that risks panicing, if the notifier
fails.

> Option 1) or 2.c) sounds like a good compromise to me. What do you
> think?
> 
Well, all I can say is that you should stay away from notifiers
priority --especially of the one of cpu_schedule_callback(). As long as
you do that, I won't be on your way. :-D

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