Re: [Xen-devel] [PATCH v2 10/10] xen/arm: Call check_local_cpu_errata for secondary CPU only on boot

[Julien Grall <julien.grall@xxxxxxx>]

On 25/04/18 16:13, Mirela Simonovic wrote:
> Hi Julien,
Hi Mirela,

> On Mon, Apr 23, 2018 at 1:46 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> > Hi,
> >
> > On 20/04/18 13:25, Mirela Simonovic wrote:
> > > Checking CPU errata should be done only when a CPU is initially booted.
> > > It is assumed that the CPU which is hotplugged after the system/Xen boots,
> > > was initially hotplugged during the system/Xen boot, so errata is checked
> > > by each CPU only once, on boot.
> >
> > It is a good idea to document the assumption in the code. This will help to
> > know what is missing for other use case.
> >
> > > Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
> > >
> > > ---
> > > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > CC: Julien Grall <julien.grall@xxxxxxx>
> > > ---
> > >    xen/arch/arm/smpboot.c | 4 ++--
> > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > > index d01b51592d..5d6c6cadec 100644
> > > --- a/xen/arch/arm/smpboot.c
> > > +++ b/xen/arch/arm/smpboot.c
> > > @@ -366,8 +366,8 @@ void start_secondary(unsigned long boot_phys_offset,
> > >          if ( system_state != SYS_STATE_boot )
> > >            setup_virt_paging_secondary();
> > > -
> > > -    check_local_cpu_errata();
> > > +    else
> > > +        check_local_cpu_errata();
> >
> > No, check_local_cpu_errata should be called for everyone. This check should
> Could you please clarify what you meant with "for everyone"? My
> understanding is that you suggested this in
> https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg00979.html
> Did something change meanwhile?
We discussed a bit more how big.LITTLE fits in the picture since them. 
Technically you have to check Xen has all the known workaround 
associated to the processor so it can run safely. If not, you need to 
park that CPU.
> > be moved in the function with a TODO explaining what needs to be done.
> > Likely this will be go over the CPU errata and see if there are any issue
> > with the one currently selected.
> Please clarify, I don't follow this.
Most of the errata requires to modify Xen code. This is done using the 
alternative at Xen boot. If you boot a CPU with an errata that has not 
been enabled, mostly likely you want to park that CPU.
In future, you could imagine to do the same with features (e.g atomics, 
pointer authentification...). You want to know whether the processors 
has a feature the hypervisor has enabled.
> > Also, I just realized that any "cpu capability" (e.g spectre workaround)
> > that requires to be enabled will not be done on hotplugged CPU. You likely
> > need to implement a version of enable_errata_workaround for them.
> Could you please point me to a place where this is done on boot?
Have a look at the function enable_errata_workarounds, this is done late 
at boot and go through all CPU to enable the different errata.
Cheers,

--
Julien Grall

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