Re: [Xen-devel] [PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 17 Jan 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 17/01/18 00:42, Stefano Stabellini wrote:
> > On Tue, 16 Jan 2018, Julien Grall wrote:
> > >   #define MIDR_RANGE(model, min, max)     \
> > > @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[]
> > > = {
> > >                      (1 << MIDR_VARIANT_SHIFT) | 2),
> > >       },
> > >   #endif
> > > +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > 
> > We need to add a basic description in the desc field as it is printed by
> > update_cpu_capabilities.
> 
> desc field is not mandatory, and in that case I think the print would be
> confusing. At the difference of the other errata, we have more check in
> install_bp_hardening_vec that may result to skip the hardening.
> 
> The errata here is covering all variant/revision of A75 models for safety
> reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a
> branch predictor trained in one context will affect speculative execution in
> another context. This field is checked in install_bp_hardening_vec so you
> avoid to harden the vector tables and small performance hit.
> 
> IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec
> announcing whether the vector tables has been harden and the kind of
> hardening.
> 
> What do you think?

I understand what you are saying and I agree with you. Maybe we should
change update_cpu_capabilities to print "detected need for workaround:
blah" but you don't have to do it in this series.

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