[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv5 26/46] plat/common: Add counter workaround for Cortex-A73 erratum 858921



Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: 2018年9月7日 22:20
> To: Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; Simon Kuenzer
> <simon.kuenzer@xxxxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 26/46] plat/common: Add counter
> workaround for Cortex-A73 erratum 858921
> 
> Hi,
> 
> On 09/07/2018 11:01 AM, Wei Chen (Arm Technology China) wrote:
> >> -----Original Message-----
> >> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> >> Sent: 2018年9月7日 17:58
> >> To: Julien Grall <Julien.Grall@xxxxxxx>; Wei Chen (Arm Technology China)
> >> <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx
> >> Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> >> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 26/46] plat/common: Add
> counter
> >> workaround for Cortex-A73 erratum 858921
> >>
> >>
> >>
> >> On 13.08.2018 11:03, Julien Grall wrote:
> >>> Hi Wei,
> >>>
> >>> On 10/08/18 08:08, Wei Chen wrote:
> >>>> The errata #858921 describes that Cortex-A73 (r0p0 - r0p2)
> >>>> counter read can return a wrong value when the counter crosses
> >>>> a 32bit boundary, but newer Cortex-A73 are not affected.
> >>>>
> >>>> The workaround involves performing the read twice, compare
> >>>> bit[32] of the two read values. If bit[32] is different,
> >>>> keep the first value, otherwise keep the second value.
> >>>>
> >>>> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> >>>> ---
> >>>>    arch/arm/arm64/Config.uk |  9 +++++++++
> >>>>    plat/common/arm/time.c   | 20 ++++++++++++++++++++
> >>>>    2 files changed, 29 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/arm64/Config.uk b/arch/arm/arm64/Config.uk
> >>>> index 7797516..07bc8ec 100644
> >>>> --- a/arch/arm/arm64/Config.uk
> >>>> +++ b/arch/arm/arm64/Config.uk
> >>>> @@ -46,3 +46,12 @@ config MARCH_ARM64_CORTEXA75
> >>>>            Compile for Armv8.2 Cortex-A75 (and compatible) CPUs
> >>>>    endchoice
> >>>> +
> >>>> +config ARM64_ERRATUM_858921
> >>>> +    bool "Workaround for Cortex-A73 erratum 858921"
> >>>> +    default n
> >>>> +    depends on MARCH_ARM64_CORTEXA73
> >>>
> >>> I don't think this is correct here. MARCH_ARM64_CORTEXA73 is about how
> >>> the code was optimized for a given processor. It would still be possible
> >>> to run a code compiled with generic option on Cortex-A73.
> >>>
> >>> So you want to at least drop the depends on here and possibly default y
> >>> for CORTEX_A73 and GENERIC.
> >>>
> >>> Cheers,
> >>
> >> I am okay with Juliens suggestion. I agree that it is safer to have the
> >> erratum enabled on default for people that do not know if their SOC is
> >> affected or not.
> >> We may need to think about the MARCH_ARM64_NATIVE and the other cases
> >> where the could would run on A73. Probably these should also go to the
> >> `depends on` list?
> >>
> >
> > Ok, that sounds sensible. I will fix it in next version, and add
> > MARCH_ARM64_NATIVE to depends on list.
> 
> What would prevent a binary optimized for Cortex-A53 to run on
> Cortex-A73? Technically, the optimization is just about how the
> instructions are going to get scheduled and may some ARMv8.x features
> turned on.
> 
> Also, if you want your binary to run on multiple platform, it feels
> slightly odd to impose the errata for everyone. Imagine that now you are
> going to read twice the system register and doing some math on it...
> 

So, how about setting default to y and just depending on ARM64? (We can reach 
here
Means we have selected arm64 already, so, no explicit dependence for it)

> Lastly, bear in mind you may have big.LITTLE system in place...
> 

Ahh, I don't want to consider so much in this stage, it would be an
infinite expanded topic that will make me crazy ; (

> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.