[Yakkety] UBUNTU: [Config] CONFIG_ARM64_ERRATUM_845719=y
diff mbox

Message ID 1481042241-16835-1-git-send-email-ming.lei@canonical.com
State New
Headers show

Commit Message

Ming Lei Dec. 6, 2016, 4:37 p.m. UTC
Looks this option is missed in Yakkety, and should be
enabled always.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 debian.master/config/config.common.ubuntu | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Colin King Dec. 6, 2016, 4:42 p.m. UTC | #1
On 06/12/16 16:37, Ming Lei wrote:
> Looks this option is missed in Yakkety, and should be
> enabled always.
> 
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  debian.master/config/config.common.ubuntu | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
> index aaba225..26ddbf7 100644
> --- a/debian.master/config/config.common.ubuntu
> +++ b/debian.master/config/config.common.ubuntu
> @@ -505,7 +505,7 @@ CONFIG_ARM64_ERRATUM_826319=y
>  CONFIG_ARM64_ERRATUM_827319=y
>  CONFIG_ARM64_ERRATUM_832075=y
>  CONFIG_ARM64_ERRATUM_834220=y
> -# CONFIG_ARM64_ERRATUM_843419 is not set
> +CONFIG_ARM64_ERRATUM_843419=y
>  CONFIG_ARM64_ERRATUM_845719=y
>  CONFIG_ARM64_HW_AFDBM=y
>  # CONFIG_ARM64_LSE_ATOMICS is not set
> 
This is enabled for Xenial and not Yakkety (I wonder why the config
enforcer is not picking this up...)

There is no bug number in this patch, can that be included?  Also
reference to "Cortex-A53: 845719: a load might read incorrect data"
maybe of use in the patch as it informs us what CPU issue it works around.

Colin
Seth Forshee Dec. 6, 2016, 4:47 p.m. UTC | #2
On Tue, Dec 06, 2016 at 04:42:48PM +0000, Colin Ian King wrote:
> On 06/12/16 16:37, Ming Lei wrote:
> > Looks this option is missed in Yakkety, and should be
> > enabled always.
> > 
> > Signed-off-by: Ming Lei <ming.lei@canonical.com>
> > ---
> >  debian.master/config/config.common.ubuntu | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
> > index aaba225..26ddbf7 100644
> > --- a/debian.master/config/config.common.ubuntu
> > +++ b/debian.master/config/config.common.ubuntu
> > @@ -505,7 +505,7 @@ CONFIG_ARM64_ERRATUM_826319=y
> >  CONFIG_ARM64_ERRATUM_827319=y
> >  CONFIG_ARM64_ERRATUM_832075=y
> >  CONFIG_ARM64_ERRATUM_834220=y
> > -# CONFIG_ARM64_ERRATUM_843419 is not set
> > +CONFIG_ARM64_ERRATUM_843419=y
> >  CONFIG_ARM64_ERRATUM_845719=y
> >  CONFIG_ARM64_HW_AFDBM=y
> >  # CONFIG_ARM64_LSE_ATOMICS is not set
> > 
> This is enabled for Xenial and not Yakkety (I wonder why the config
> enforcer is not picking this up...)
> 
> There is no bug number in this patch, can that be included?  Also
> reference to "Cortex-A53: 845719: a load might read incorrect data"
> maybe of use in the patch as it informs us what CPU issue it works around.

Some config options got mistakenly changed in yakkety, especially for
non-x86 architectures. Only options marked as ENFORCED in the
annotations file are enforced.

So the patch probably should also mark this option as enforced. It will
also be needed in zesty.

Seth
Ming Lei Dec. 6, 2016, 5:04 p.m. UTC | #3
On Wed, Dec 7, 2016 at 12:47 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Tue, Dec 06, 2016 at 04:42:48PM +0000, Colin Ian King wrote:
>> On 06/12/16 16:37, Ming Lei wrote:
>> > Looks this option is missed in Yakkety, and should be
>> > enabled always.
>> >
>> > Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> > ---
>> >  debian.master/config/config.common.ubuntu | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
>> > index aaba225..26ddbf7 100644
>> > --- a/debian.master/config/config.common.ubuntu
>> > +++ b/debian.master/config/config.common.ubuntu
>> > @@ -505,7 +505,7 @@ CONFIG_ARM64_ERRATUM_826319=y
>> >  CONFIG_ARM64_ERRATUM_827319=y
>> >  CONFIG_ARM64_ERRATUM_832075=y
>> >  CONFIG_ARM64_ERRATUM_834220=y
>> > -# CONFIG_ARM64_ERRATUM_843419 is not set
>> > +CONFIG_ARM64_ERRATUM_843419=y
>> >  CONFIG_ARM64_ERRATUM_845719=y
>> >  CONFIG_ARM64_HW_AFDBM=y
>> >  # CONFIG_ARM64_LSE_ATOMICS is not set
>> >
>> This is enabled for Xenial and not Yakkety (I wonder why the config
>> enforcer is not picking this up...)
>>
>> There is no bug number in this patch, can that be included?  Also
>> reference to "Cortex-A53: 845719: a load might read incorrect data"
>> maybe of use in the patch as it informs us what CPU issue it works around.
>
> Some config options got mistakenly changed in yakkety, especially for
> non-x86 architectures. Only options marked as ENFORCED in the
> annotations file are enforced.
>
> So the patch probably should also mark this option as enforced. It will
> also be needed in zesty.

But all CONFIG_ARM64_ERRATUM_* aren't marked yet as ENFORCED
in debian.master/config/annotations for yakkety.

Thanks,
Ming

>
> Seth
Seth Forshee Dec. 6, 2016, 9:58 p.m. UTC | #4
On Wed, Dec 07, 2016 at 01:04:44AM +0800, Ming Lei wrote:
> On Wed, Dec 7, 2016 at 12:47 AM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > On Tue, Dec 06, 2016 at 04:42:48PM +0000, Colin Ian King wrote:
> >> On 06/12/16 16:37, Ming Lei wrote:
> >> > Looks this option is missed in Yakkety, and should be
> >> > enabled always.
> >> >
> >> > Signed-off-by: Ming Lei <ming.lei@canonical.com>
> >> > ---
> >> >  debian.master/config/config.common.ubuntu | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
> >> > index aaba225..26ddbf7 100644
> >> > --- a/debian.master/config/config.common.ubuntu
> >> > +++ b/debian.master/config/config.common.ubuntu
> >> > @@ -505,7 +505,7 @@ CONFIG_ARM64_ERRATUM_826319=y
> >> >  CONFIG_ARM64_ERRATUM_827319=y
> >> >  CONFIG_ARM64_ERRATUM_832075=y
> >> >  CONFIG_ARM64_ERRATUM_834220=y
> >> > -# CONFIG_ARM64_ERRATUM_843419 is not set
> >> > +CONFIG_ARM64_ERRATUM_843419=y
> >> >  CONFIG_ARM64_ERRATUM_845719=y
> >> >  CONFIG_ARM64_HW_AFDBM=y
> >> >  # CONFIG_ARM64_LSE_ATOMICS is not set
> >> >
> >> This is enabled for Xenial and not Yakkety (I wonder why the config
> >> enforcer is not picking this up...)
> >>
> >> There is no bug number in this patch, can that be included?  Also
> >> reference to "Cortex-A53: 845719: a load might read incorrect data"
> >> maybe of use in the patch as it informs us what CPU issue it works around.
> >
> > Some config options got mistakenly changed in yakkety, especially for
> > non-x86 architectures. Only options marked as ENFORCED in the
> > annotations file are enforced.
> >
> > So the patch probably should also mark this option as enforced. It will
> > also be needed in zesty.
> 
> But all CONFIG_ARM64_ERRATUM_* aren't marked yet as ENFORCED
> in debian.master/config/annotations for yakkety.

Undoubtedly not everything that ought to be enforced is marked as
enforced today. If it's critical that all of those options remain
enbled, feel free to submit a patch which marks all of them as enforced.

It's less important for the stable releases, but for the development
release it will help us make sure we don't turn off options which need
to be enabled. It would have prevented this problem in yakkety, for
instance :-)

Seth
Colin King Dec. 6, 2016, 10:02 p.m. UTC | #5
On 06/12/16 21:58, Seth Forshee wrote:
> On Wed, Dec 07, 2016 at 01:04:44AM +0800, Ming Lei wrote:
>> On Wed, Dec 7, 2016 at 12:47 AM, Seth Forshee
>> <seth.forshee@canonical.com> wrote:
>>> On Tue, Dec 06, 2016 at 04:42:48PM +0000, Colin Ian King wrote:
>>>> On 06/12/16 16:37, Ming Lei wrote:
>>>>> Looks this option is missed in Yakkety, and should be
>>>>> enabled always.
>>>>>
>>>>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>>>>> ---
>>>>>  debian.master/config/config.common.ubuntu | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
>>>>> index aaba225..26ddbf7 100644
>>>>> --- a/debian.master/config/config.common.ubuntu
>>>>> +++ b/debian.master/config/config.common.ubuntu
>>>>> @@ -505,7 +505,7 @@ CONFIG_ARM64_ERRATUM_826319=y
>>>>>  CONFIG_ARM64_ERRATUM_827319=y
>>>>>  CONFIG_ARM64_ERRATUM_832075=y
>>>>>  CONFIG_ARM64_ERRATUM_834220=y
>>>>> -# CONFIG_ARM64_ERRATUM_843419 is not set
>>>>> +CONFIG_ARM64_ERRATUM_843419=y
>>>>>  CONFIG_ARM64_ERRATUM_845719=y
>>>>>  CONFIG_ARM64_HW_AFDBM=y
>>>>>  # CONFIG_ARM64_LSE_ATOMICS is not set
>>>>>
>>>> This is enabled for Xenial and not Yakkety (I wonder why the config
>>>> enforcer is not picking this up...)
>>>>
>>>> There is no bug number in this patch, can that be included?  Also
>>>> reference to "Cortex-A53: 845719: a load might read incorrect data"
>>>> maybe of use in the patch as it informs us what CPU issue it works around.
>>>
>>> Some config options got mistakenly changed in yakkety, especially for
>>> non-x86 architectures. Only options marked as ENFORCED in the
>>> annotations file are enforced.
>>>
>>> So the patch probably should also mark this option as enforced. It will
>>> also be needed in zesty.
>>
>> But all CONFIG_ARM64_ERRATUM_* aren't marked yet as ENFORCED
>> in debian.master/config/annotations for yakkety.
> 
> Undoubtedly not everything that ought to be enforced is marked as
> enforced today. If it's critical that all of those options remain
> enbled, feel free to submit a patch which marks all of them as enforced.
> 
> It's less important for the stable releases, but for the development
> release it will help us make sure we don't turn off options which need
> to be enabled. It would have prevented this problem in yakkety, for
> instance :-)

My gut feeling is this fits into our remit to sort out rather than those
submitting these kind of config changes as it's our kernel team tool and
method for catching these and we shouldn't expect community to be
concerned with the config enforcer details.

Colin

> 
> Seth
>
Brad Figg Dec. 12, 2016, 7:24 p.m. UTC | #6

Luis Henriques Dec. 14, 2016, 11:16 a.m. UTC | #7
On Tue, Dec 06, 2016 at 10:02:12PM +0000, Colin Ian King wrote:
> >>>> This is enabled for Xenial and not Yakkety (I wonder why the config
> >>>> enforcer is not picking this up...)
> >>>>
> >>>> There is no bug number in this patch, can that be included?  Also
> >>>> reference to "Cortex-A53: 845719: a load might read incorrect data"
> >>>> maybe of use in the patch as it informs us what CPU issue it works around.
> >>>
> >>> Some config options got mistakenly changed in yakkety, especially for
> >>> non-x86 architectures. Only options marked as ENFORCED in the
> >>> annotations file are enforced.
> >>>
> >>> So the patch probably should also mark this option as enforced. It will
> >>> also be needed in zesty.
> >>
> >> But all CONFIG_ARM64_ERRATUM_* aren't marked yet as ENFORCED
> >> in debian.master/config/annotations for yakkety.
> > 
> > Undoubtedly not everything that ought to be enforced is marked as
> > enforced today. If it's critical that all of those options remain
> > enbled, feel free to submit a patch which marks all of them as enforced.
> > 
> > It's less important for the stable releases, but for the development
> > release it will help us make sure we don't turn off options which need
> > to be enabled. It would have prevented this problem in yakkety, for
> > instance :-)
> 
> My gut feeling is this fits into our remit to sort out rather than those
> submitting these kind of config changes as it's our kernel team tool and
> method for catching these and we shouldn't expect community to be
> concerned with the config enforcer details.

Ok, it's not clear if I should wait for a v2 for this patch.  (And there's
only one explicit ACK anyway... :-) )

Cheers,
--
Luís
Tim Gardner Dec. 14, 2016, 1:48 p.m. UTC | #8
On 12/14/2016 04:16 AM, Luis Henriques wrote:
> On Tue, Dec 06, 2016 at 10:02:12PM +0000, Colin Ian King wrote:
>>>>>> This is enabled for Xenial and not Yakkety (I wonder why the config
>>>>>> enforcer is not picking this up...)
>>>>>>
>>>>>> There is no bug number in this patch, can that be included?  Also
>>>>>> reference to "Cortex-A53: 845719: a load might read incorrect data"
>>>>>> maybe of use in the patch as it informs us what CPU issue it works around.
>>>>>
>>>>> Some config options got mistakenly changed in yakkety, especially for
>>>>> non-x86 architectures. Only options marked as ENFORCED in the
>>>>> annotations file are enforced.
>>>>>
>>>>> So the patch probably should also mark this option as enforced. It will
>>>>> also be needed in zesty.
>>>>
>>>> But all CONFIG_ARM64_ERRATUM_* aren't marked yet as ENFORCED
>>>> in debian.master/config/annotations for yakkety.
>>>
>>> Undoubtedly not everything that ought to be enforced is marked as
>>> enforced today. If it's critical that all of those options remain
>>> enbled, feel free to submit a patch which marks all of them as enforced.
>>>
>>> It's less important for the stable releases, but for the development
>>> release it will help us make sure we don't turn off options which need
>>> to be enabled. It would have prevented this problem in yakkety, for
>>> instance :-)
>>
>> My gut feeling is this fits into our remit to sort out rather than those
>> submitting these kind of config changes as it's our kernel team tool and
>> method for catching these and we shouldn't expect community to be
>> concerned with the config enforcer details.
> 
> Ok, it's not clear if I should wait for a v2 for this patch.  (And there's
> only one explicit ACK anyway... :-) )
> 
> Cheers,
> --
> Luís
> 

Luis - I think this is OK to apply. The annotation update needs to go in
the development kernel anyways (which I've done).

rtg
Luis Henriques Dec. 14, 2016, 2:05 p.m. UTC | #9
On Wed, Dec 14, 2016 at 06:48:09AM -0700, Tim Gardner wrote:
> 
> Luis - I think this is OK to apply. The annotation update needs to go in
> the development kernel anyways (which I've done).

Ups!  Sorry Tim, but it looks like I had already done that before, as another
version of this patch had already been sent to the mailing-list.

Cheers,
--
Luís

Patch
diff mbox

diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
index aaba225..26ddbf7 100644
--- a/debian.master/config/config.common.ubuntu
+++ b/debian.master/config/config.common.ubuntu
@@ -505,7 +505,7 @@  CONFIG_ARM64_ERRATUM_826319=y
 CONFIG_ARM64_ERRATUM_827319=y
 CONFIG_ARM64_ERRATUM_832075=y
 CONFIG_ARM64_ERRATUM_834220=y
-# CONFIG_ARM64_ERRATUM_843419 is not set
+CONFIG_ARM64_ERRATUM_843419=y
 CONFIG_ARM64_ERRATUM_845719=y
 CONFIG_ARM64_HW_AFDBM=y
 # CONFIG_ARM64_LSE_ATOMICS is not set