diff mbox series

[v2,2/9] busybox: disable defconfig options from simpler fragments

Message ID 20241028082913.418505-3-ejo@pengutronix.de
State Accepted, archived
Commit ad4bd190836d2de4bda6a7e59b55bcf8d118fd14
Headers show
Series busybox: cleanup and fix config fragments | expand

Commit Message

Enrico Jörns Oct. 28, 2024, 8:29 a.m. UTC
This disables options from the defconfig file that are enabled by
explicit config fragments.

Having them enabled in the defconfig renders the fragments useless and
takes away the ability to disable options with

  SRC_URI:remove = "<fragment>.cfg".

The respective options were all deactivated once but got accidentally
enabled in 4335cd24 ("busybox: refresh the defconfig from 1.33.0").

This commit disables the features for:

- sha1sum.cfg
- sha256sum.cfg
- resize.cfg
- pgrep.cfg
- rev.cfg

Signed-off-by: Enrico Jörns <ejo@pengutronix.de>
---
 meta/recipes-core/busybox/busybox/defconfig | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Andrej Valek Oct. 28, 2024, 9:06 a.m. UTC | #1
Hello Enrico,

I tried to follow (and applied) your patch, but it looks little bit 
inconsistent to me. In the first round you enabled SHAx_HWACCEL, which 
is broken on some architectures. That's the reason, why is disabled by 
default. Ok, so first apply looks like this:

  # CONFIG_FEATURE_BUFFERS_GO_IN_BSS is not set
  CONFIG_PASSWORD_MINLEN=6
  CONFIG_MD5_SMALL=1
+CONFIG_SHA1_SMALL=3
+CONFIG_SHA1_HWACCEL=y
+CONFIG_SHA256_HWACCEL=y
  CONFIG_SHA3_SMALL=1
  CONFIG_FEATURE_NON_POSIX_CP=y
  # CONFIG_FEATURE_VERBOSE_CP_MESSAGE is not set
@@ -884,6 +887,7 @@ CONFIG_FEATURE_HWIB=y

Then I applied next patch:

  # CONFIG_FEATURE_BUFFERS_GO_IN_BSS is not set
  CONFIG_PASSWORD_MINLEN=6
  CONFIG_MD5_SMALL=1
+CONFIG_SHA1_SMALL=3
+CONFIG_SHA1_HWACCEL=y
+CONFIG_SHA256_HWACCEL=y
  CONFIG_SHA3_SMALL=1
  CONFIG_FEATURE_NON_POSIX_CP=y
  # CONFIG_FEATURE_VERBOSE_CP_MESSAGE is not set
@@ -278,8 +281,8 @@ CONFIG_FEATURE_LS_USERNAME=y
  CONFIG_FEATURE_LS_COLOR=y
  # CONFIG_FEATURE_LS_COLOR_IS_DEFAULT is not set
  CONFIG_MD5SUM=y
-CONFIG_SHA1SUM=y
-CONFIG_SHA256SUM=y
+# CONFIG_SHA1SUM is not set
+# CONFIG_SHA256SUM is not set
  # CONFIG_SHA512SUM is not set
  # CONFIG_SHA3SUM is not set

@@ -383,8 +386,8 @@ CONFIG_DEFAULT_SETFONT_DIR=""

and you disabled CONFIG_SHA1SUM, but keep enabled the acceleration? Is 
there any special reason for that?

Anyway, there was one commit in the usptream which was touching the 
acceleration. So I have to try it on the broken machine and see if it's 
still broken or not.

> On Fri, 2024-10-25 at 19:26 +0200, Andrej wrote:
>> Ok, I was thinking that it was done by intention, but it happened 14y
>> ago
>> https://github.com/openembedded/openembedded-core/commit/615a98ed9a02
>> 1da245513790c064761a0a5a67e9 . So I guess not.
> I am not sure if I fully understand what you are referring to here.
> Could you provide some details about what happened there?
> OE didn't have those fragments 14y ago I guess.
So the idea behind is that you're going to remove the fragments and keep 
just a one defconfig. But according to your commits you kept the 
fragments there, even if you put some values into defconfig.

Regards,
Andrej


On 28.10.2024 09:29, Enrico Jörns wrote:
> This disables options from the defconfig file that are enabled by
> explicit config fragments.
>
> Having them enabled in the defconfig renders the fragments useless and
> takes away the ability to disable options with
>
>    SRC_URI:remove = "<fragment>.cfg".
>
> The respective options were all deactivated once but got accidentally
> enabled in 4335cd24 ("busybox: refresh the defconfig from 1.33.0").
>
> This commit disables the features for:
>
> - sha1sum.cfg
> - sha256sum.cfg
> - resize.cfg
> - pgrep.cfg
> - rev.cfg
>
> Signed-off-by: Enrico Jörns<ejo@pengutronix.de>
> ---
>   meta/recipes-core/busybox/busybox/defconfig | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-core/busybox/busybox/defconfig
> index 6c5a8764ce..0ec65c65af 100644
> --- a/meta/recipes-core/busybox/busybox/defconfig
> +++ b/meta/recipes-core/busybox/busybox/defconfig
> @@ -281,8 +281,8 @@ CONFIG_FEATURE_LS_USERNAME=y
>   CONFIG_FEATURE_LS_COLOR=y
>   # CONFIG_FEATURE_LS_COLOR_IS_DEFAULT is not set
>   CONFIG_MD5SUM=y
> -CONFIG_SHA1SUM=y
> -CONFIG_SHA256SUM=y
> +# CONFIG_SHA1SUM is not set
> +# CONFIG_SHA256SUM is not set
>   # CONFIG_SHA512SUM is not set
>   # CONFIG_SHA3SUM is not set
>   
> @@ -386,8 +386,8 @@ CONFIG_DEFAULT_SETFONT_DIR=""
>   CONFIG_LOADKMAP=y
>   CONFIG_OPENVT=y
>   CONFIG_RESET=y
> -CONFIG_RESIZE=y
> -CONFIG_FEATURE_RESIZE_PRINT=y
> +# CONFIG_RESIZE is not set
> +# CONFIG_FEATURE_RESIZE_PRINT is not set
>   CONFIG_SETCONSOLE=y
>   # CONFIG_FEATURE_SETCONSOLE_LONG_OPTIONS is not set
>   # CONFIG_SETKEYCODES is not set
> @@ -689,7 +689,7 @@ CONFIG_RDATE=y
>   # CONFIG_RDEV is not set
>   # CONFIG_READPROFILE is not set
>   CONFIG_RENICE=y
> -CONFIG_REV=y
> +# CONFIG_REV is not set
>   # CONFIG_RTCWAKE is not set
>   # CONFIG_SCRIPT is not set
>   # CONFIG_SCRIPTREPLAY is not set
> @@ -1070,7 +1070,7 @@ CONFIG_KILLALL=y
>   # CONFIG_LSOF is not set
>   # CONFIG_MPSTAT is not set
>   # CONFIG_NMETER is not set
> -CONFIG_PGREP=y
> +# CONFIG_PGREP is not set
>   # CONFIG_PKILL is not set
>   CONFIG_PIDOF=y
>   # CONFIG_FEATURE_PIDOF_SINGLE is not set
Enrico Jörns Oct. 28, 2024, 10:39 a.m. UTC | #2
Hej Andrej,

Am Montag, dem 28.10.2024 um 10:06 +0100 schrieb Andrej Valek:
>  Hello Enrico,
>  
>  I tried to follow (and applied) your patch, but it looks little bit inconsistent to me. In the
> first round you enabled SHAx_HWACCEL, which is broken on some architectures. That's the reason,
> why is disabled by default. Ok, so first apply looks like this:

let me rephrase my commit message.

The SHAx_HWACCEL is broken on some architectures, right.
However, when you take the defconfig, without the fragments, it will currently be enabled.
Regardless of what's in my patch.

Why? Because CONFIG_SHA1_HWACCEL in Busybox' kconfig is 'default y', has no dependency on other
options and is not listed in the 'defconfig' file so far.

Thus, my first patch should not change the current situation.

>   # CONFIG_FEATURE_BUFFERS_GO_IN_BSS is not set
>   CONFIG_PASSWORD_MINLEN=6
>   CONFIG_MD5_SMALL=1
>  +CONFIG_SHA1_SMALL=3
>  +CONFIG_SHA1_HWACCEL=y
>  +CONFIG_SHA256_HWACCEL=y
>   CONFIG_SHA3_SMALL=1
>   CONFIG_FEATURE_NON_POSIX_CP=y
>   # CONFIG_FEATURE_VERBOSE_CP_MESSAGE is not set
>  @@ -884,6 +887,7 @@ CONFIG_FEATURE_HWIB=y
>  
>  Then I applied next patch:
>  
>   # CONFIG_FEATURE_BUFFERS_GO_IN_BSS is not set
>   CONFIG_PASSWORD_MINLEN=6
>   CONFIG_MD5_SMALL=1
>  +CONFIG_SHA1_SMALL=3
>  +CONFIG_SHA1_HWACCEL=y
>  +CONFIG_SHA256_HWACCEL=y
>   CONFIG_SHA3_SMALL=1
>   CONFIG_FEATURE_NON_POSIX_CP=y
>   # CONFIG_FEATURE_VERBOSE_CP_MESSAGE is not set
>  @@ -278,8 +281,8 @@ CONFIG_FEATURE_LS_USERNAME=y
>   CONFIG_FEATURE_LS_COLOR=y
>   # CONFIG_FEATURE_LS_COLOR_IS_DEFAULT is not set
>   CONFIG_MD5SUM=y
>  -CONFIG_SHA1SUM=y
>  -CONFIG_SHA256SUM=y
>  +# CONFIG_SHA1SUM is not set
>  +# CONFIG_SHA256SUM is not set
>   # CONFIG_SHA512SUM is not set
>   # CONFIG_SHA3SUM is not set
>   
>  @@ -383,8 +386,8 @@ CONFIG_DEFAULT_SETFONT_DIR=""
>  
>  and you disabled CONFIG_SHA1SUM, but keep enabled the acceleration? Is there any special reason
> for that?

There is one, because its the inverse of what the fragments apply, which is:

  $ cat ./meta/recipes-core/busybox/busybox/sha1sum.cfg                                            
  CONFIG_SHA1SUM=y
  CONFIG_SHA1_SMALL=3
  # CONFIG_SHA1_HWACCEL is not set

  $ cat ./meta/recipes-core/busybox/busybox/sha256sum.cfg 
  CONFIG_SHA256SUM=y
  # CONFIG_SHA256_HWACCEL is not set

I'm pretty sure not having touched the fragments.

The CONFIG_SHA1_HWACCEL options could have been changed in the defconfig, too.
But this is nothing that I've brought in. It's a result of
https://github.com/openembedded/openembedded-core/commit/21753f16a364e32050cf8d79bfa7e0f89be52ce7

Thus I'd assume my changes are consistent. Am I wrong?

>  Anyway, there was one commit in the usptream which was touching the acceleration. So I have to
> try it on the broken machine and see if it's still broken or not.

If you refer to oe-core upstream, yes there have been changes.
This is why I needed to rebase and rework my branch.
Or are there other changes I am not aware of, yet?

> 
> > On Fri, 2024-10-25 at 19:26 +0200, Andrej wrote:
> >  
> > > Ok, I was thinking that it was done by intention, but it happened 14y
> > > ago 
> > > https://github.com/openembedded/openembedded-core/commit/615a98ed9a02
> > > 1da245513790c064761a0a5a67e9 . So I guess not. 
>  
> > I am not sure if I fully understand what you are referring to here.
> > Could you provide some details about what happened there?
> > OE didn't have those fragments 14y ago I guess.
>  So the idea behind is that you're going to remove the fragments and keep just a one defconfig.
> But according to your commits you kept the fragments there, even if you put some values into
> defconfig.

Still not sure. I had no intention of removing the fragments.
And I'm unsure how this relates to the referenced commit.
My intention was to actually make them useful again, which is quite the opposite I guess.


Regards, Enrico

>  Regards,
>  Andrej
> 
>  
> On 28.10.2024 09:29, Enrico Jörns wrote:
>  
> > This disables options from the defconfig file that are enabled by
> > explicit config fragments.
> > 
> > Having them enabled in the defconfig renders the fragments useless and
> > takes away the ability to disable options with
> > 
> >   SRC_URI:remove = "<fragment>.cfg".
> > 
> > The respective options were all deactivated once but got accidentally
> > enabled in 4335cd24 ("busybox: refresh the defconfig from 1.33.0").
> > 
> > This commit disables the features for:
> > 
> > - sha1sum.cfg
> > - sha256sum.cfg
> > - resize.cfg
> > - pgrep.cfg
> > - rev.cfg
> > 
> > Signed-off-by: Enrico Jörns <ejo@pengutronix.de>
> > ---
> >  meta/recipes-core/busybox/busybox/defconfig | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-
> > core/busybox/busybox/defconfig
> > index 6c5a8764ce..0ec65c65af 100644
> > --- a/meta/recipes-core/busybox/busybox/defconfig
> > +++ b/meta/recipes-core/busybox/busybox/defconfig
> > @@ -281,8 +281,8 @@ CONFIG_FEATURE_LS_USERNAME=y
> >  CONFIG_FEATURE_LS_COLOR=y
> >  # CONFIG_FEATURE_LS_COLOR_IS_DEFAULT is not set
> >  CONFIG_MD5SUM=y
> > -CONFIG_SHA1SUM=y
> > -CONFIG_SHA256SUM=y
> > +# CONFIG_SHA1SUM is not set
> > +# CONFIG_SHA256SUM is not set
> >  # CONFIG_SHA512SUM is not set
> >  # CONFIG_SHA3SUM is not set
> >  
> > @@ -386,8 +386,8 @@ CONFIG_DEFAULT_SETFONT_DIR=""
> >  CONFIG_LOADKMAP=y
> >  CONFIG_OPENVT=y
> >  CONFIG_RESET=y
> > -CONFIG_RESIZE=y
> > -CONFIG_FEATURE_RESIZE_PRINT=y
> > +# CONFIG_RESIZE is not set
> > +# CONFIG_FEATURE_RESIZE_PRINT is not set
> >  CONFIG_SETCONSOLE=y
> >  # CONFIG_FEATURE_SETCONSOLE_LONG_OPTIONS is not set
> >  # CONFIG_SETKEYCODES is not set
> > @@ -689,7 +689,7 @@ CONFIG_RDATE=y
> >  # CONFIG_RDEV is not set
> >  # CONFIG_READPROFILE is not set
> >  CONFIG_RENICE=y
> > -CONFIG_REV=y
> > +# CONFIG_REV is not set
> >  # CONFIG_RTCWAKE is not set
> >  # CONFIG_SCRIPT is not set
> >  # CONFIG_SCRIPTREPLAY is not set
> > @@ -1070,7 +1070,7 @@ CONFIG_KILLALL=y
> >  # CONFIG_LSOF is not set
> >  # CONFIG_MPSTAT is not set
> >  # CONFIG_NMETER is not set
> > -CONFIG_PGREP=y
> > +# CONFIG_PGREP is not set
> >  # CONFIG_PKILL is not set
> >  CONFIG_PIDOF=y
> >  # CONFIG_FEATURE_PIDOF_SINGLE is not set
>  
>
Andrej Valek Oct. 29, 2024, 4:32 p.m. UTC | #3
Hello Enrico,

On 28.10.2024 11:39, Enrico Jörns wrote:
> Hej Andrej,
>
> Am Montag, dem 28.10.2024 um 10:06 +0100 schrieb Andrej Valek:
>>   Hello Enrico,
>>   
>>   I tried to follow (and applied) your patch, but it looks little bit inconsistent to me. In the
>> first round you enabled SHAx_HWACCEL, which is broken on some architectures. That's the reason,
>> why is disabled by default. Ok, so first apply looks like this:
> let me rephrase my commit message.
>
> The SHAx_HWACCEL is broken on some architectures, right.
> However, when you take the defconfig, without the fragments, it will currently be enabled.
> Regardless of what's in my patch.
>
> Why? Because CONFIG_SHA1_HWACCEL in Busybox' kconfig is 'default y', has no dependency on other
> options and is not listed in the 'defconfig' file so far.
>
> Thus, my first patch should not change the current situation.
Ok, I see the point. So basically you can drop the sha1sum.cfg and 
sha256sum.cfg and merge it to defconfing. The result will be, that the 
acceleration is disabled.
>>    # CONFIG_FEATURE_BUFFERS_GO_IN_BSS is not set
>>    CONFIG_PASSWORD_MINLEN=6
>>    CONFIG_MD5_SMALL=1
>>   +CONFIG_SHA1_SMALL=3
>>   +CONFIG_SHA1_HWACCEL=y
>>   +CONFIG_SHA256_HWACCEL=y
>>    CONFIG_SHA3_SMALL=1
>>    CONFIG_FEATURE_NON_POSIX_CP=y
>>    # CONFIG_FEATURE_VERBOSE_CP_MESSAGE is not set
>>   @@ -884,6 +887,7 @@ CONFIG_FEATURE_HWIB=y
>>   
>>   Then I applied next patch:
>>   
>>    # CONFIG_FEATURE_BUFFERS_GO_IN_BSS is not set
>>    CONFIG_PASSWORD_MINLEN=6
>>    CONFIG_MD5_SMALL=1
>>   +CONFIG_SHA1_SMALL=3
>>   +CONFIG_SHA1_HWACCEL=y
>>   +CONFIG_SHA256_HWACCEL=y
>>    CONFIG_SHA3_SMALL=1
>>    CONFIG_FEATURE_NON_POSIX_CP=y
>>    # CONFIG_FEATURE_VERBOSE_CP_MESSAGE is not set
>>   @@ -278,8 +281,8 @@ CONFIG_FEATURE_LS_USERNAME=y
>>    CONFIG_FEATURE_LS_COLOR=y
>>    # CONFIG_FEATURE_LS_COLOR_IS_DEFAULT is not set
>>    CONFIG_MD5SUM=y
>>   -CONFIG_SHA1SUM=y
>>   -CONFIG_SHA256SUM=y
>>   +# CONFIG_SHA1SUM is not set
>>   +# CONFIG_SHA256SUM is not set
>>    # CONFIG_SHA512SUM is not set
>>    # CONFIG_SHA3SUM is not set
>>    
>>   @@ -383,8 +386,8 @@ CONFIG_DEFAULT_SETFONT_DIR=""
>>   
>>   and you disabled CONFIG_SHA1SUM, but keep enabled the acceleration? Is there any special reason
>> for that?
> There is one, because its the inverse of what the fragments apply, which is:
>
>    $ cat ./meta/recipes-core/busybox/busybox/sha1sum.cfg
>    CONFIG_SHA1SUM=y
>    CONFIG_SHA1_SMALL=3
>    # CONFIG_SHA1_HWACCEL is not set
>
>    $ cat ./meta/recipes-core/busybox/busybox/sha256sum.cfg
>    CONFIG_SHA256SUM=y
>    # CONFIG_SHA256_HWACCEL is not set
>
> I'm pretty sure not having touched the fragments.
>
> The CONFIG_SHA1_HWACCEL options could have been changed in the defconfig, too.
> But this is nothing that I've brought in. It's a result of
> https://github.com/openembedded/openembedded-core/commit/21753f16a364e32050cf8d79bfa7e0f89be52ce7
>
> Thus I'd assume my changes are consistent. Am I wrong?
No no, it's ok.
>
>>   Anyway, there was one commit in the usptream which was touching the acceleration. So I have to
>> try it on the broken machine and see if it's still broken or not.
> If you refer to oe-core upstream, yes there have been changes.
> This is why I needed to rebase and rework my branch.
> Or are there other changes I am not aware of, yet?
Not sure if you caught the point. I mean, that I will take a look on the 
upstream fixes for acceleration and then we can enable it by default.
>>> On Fri, 2024-10-25 at 19:26 +0200, Andrej wrote:
>>>   
>>>> Ok, I was thinking that it was done by intention, but it happened 14y
>>>> ago
>>>> https://github.com/openembedded/openembedded-core/commit/615a98ed9a02
>>>> 1da245513790c064761a0a5a67e9 . So I guess not.
>>   
>>> I am not sure if I fully understand what you are referring to here.
>>> Could you provide some details about what happened there?
>>> OE didn't have those fragments 14y ago I guess.
>>   So the idea behind is that you're going to remove the fragments and keep just a one defconfig.
>> But according to your commits you kept the fragments there, even if you put some values into
>> defconfig.
> Still not sure. I had no intention of removing the fragments.
> And I'm unsure how this relates to the referenced commit.
> My intention was to actually make them useful again, which is quite the opposite I guess.
>
>
> Regards, Enrico
>
>>   Regards,
>>   Andrej
>>
>>   
>> On 28.10.2024 09:29, Enrico Jörns wrote:
>>   
>>> This disables options from the defconfig file that are enabled by
>>> explicit config fragments.
>>>
>>> Having them enabled in the defconfig renders the fragments useless and
>>> takes away the ability to disable options with
>>>
>>>    SRC_URI:remove = "<fragment>.cfg".
>>>
>>> The respective options were all deactivated once but got accidentally
>>> enabled in 4335cd24 ("busybox: refresh the defconfig from 1.33.0").
>>>
>>> This commit disables the features for:
>>>
>>> - sha1sum.cfg
>>> - sha256sum.cfg
>>> - resize.cfg
>>> - pgrep.cfg
>>> - rev.cfg
>>>
>>> Signed-off-by: Enrico Jörns <ejo@pengutronix.de>
>>> ---
>>>   meta/recipes-core/busybox/busybox/defconfig | 12 ++++++------
>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-
>>> core/busybox/busybox/defconfig
>>> index 6c5a8764ce..0ec65c65af 100644
>>> --- a/meta/recipes-core/busybox/busybox/defconfig
>>> +++ b/meta/recipes-core/busybox/busybox/defconfig
>>> @@ -281,8 +281,8 @@ CONFIG_FEATURE_LS_USERNAME=y
>>>   CONFIG_FEATURE_LS_COLOR=y
>>>   # CONFIG_FEATURE_LS_COLOR_IS_DEFAULT is not set
>>>   CONFIG_MD5SUM=y
>>> -CONFIG_SHA1SUM=y
>>> -CONFIG_SHA256SUM=y
>>> +# CONFIG_SHA1SUM is not set
>>> +# CONFIG_SHA256SUM is not set
>>>   # CONFIG_SHA512SUM is not set
>>>   # CONFIG_SHA3SUM is not set
>>>   
>>> @@ -386,8 +386,8 @@ CONFIG_DEFAULT_SETFONT_DIR=""
>>>   CONFIG_LOADKMAP=y
>>>   CONFIG_OPENVT=y
>>>   CONFIG_RESET=y
>>> -CONFIG_RESIZE=y
>>> -CONFIG_FEATURE_RESIZE_PRINT=y
>>> +# CONFIG_RESIZE is not set
>>> +# CONFIG_FEATURE_RESIZE_PRINT is not set
>>>   CONFIG_SETCONSOLE=y
>>>   # CONFIG_FEATURE_SETCONSOLE_LONG_OPTIONS is not set
>>>   # CONFIG_SETKEYCODES is not set
>>> @@ -689,7 +689,7 @@ CONFIG_RDATE=y
>>>   # CONFIG_RDEV is not set
>>>   # CONFIG_READPROFILE is not set
>>>   CONFIG_RENICE=y
>>> -CONFIG_REV=y
>>> +# CONFIG_REV is not set
>>>   # CONFIG_RTCWAKE is not set
>>>   # CONFIG_SCRIPT is not set
>>>   # CONFIG_SCRIPTREPLAY is not set
>>> @@ -1070,7 +1070,7 @@ CONFIG_KILLALL=y
>>>   # CONFIG_LSOF is not set
>>>   # CONFIG_MPSTAT is not set
>>>   # CONFIG_NMETER is not set
>>> -CONFIG_PGREP=y
>>> +# CONFIG_PGREP is not set
>>>   # CONFIG_PKILL is not set
>>>   CONFIG_PIDOF=y
>>>   # CONFIG_FEATURE_PIDOF_SINGLE is not set
>>   
>>   
So if the result is, that the configuration hasn't been changed at the 
end, I'm fine with the changes you did ;).

Regards,
Andrej
Enrico Jörns Oct. 29, 2024, 8:47 p.m. UTC | #4
Hi Andrej,

thanks for your review!

Am Dienstag, dem 29.10.2024 um 17:32 +0100 schrieb Andrej Valek:
> Hello Enrico,
> 
> On 28.10.2024 11:39, Enrico Jörns wrote:
> > Hej Andrej,
> > 
> > Am Montag, dem 28.10.2024 um 10:06 +0100 schrieb Andrej Valek:
> > >   Hello Enrico,
> > >   
> > >   I tried to follow (and applied) your patch, but it looks little bit inconsistent to me. In
> > > the
> > > first round you enabled SHAx_HWACCEL, which is broken on some architectures. That's the
> > > reason,
> > > why is disabled by default. Ok, so first apply looks like this:
> > let me rephrase my commit message.
> > 
> > The SHAx_HWACCEL is broken on some architectures, right.
> > However, when you take the defconfig, without the fragments, it will currently be enabled.
> > Regardless of what's in my patch.
> > 
> > Why? Because CONFIG_SHA1_HWACCEL in Busybox' kconfig is 'default y', has no dependency on other
> > options and is not listed in the 'defconfig' file so far.
> > 
> > Thus, my first patch should not change the current situation.
> Ok, I see the point. So basically you can drop the sha1sum.cfg and 
> sha256sum.cfg and merge it to defconfing. The result will be, that the 
> acceleration is disabled.

Just to get it right: Do we talk about the HWACCEL only or about enabling SHA1SUM and SHA256SUM
unconditionally and fully removing the fragments?

Personally, I'd be fine with this since the algorithm are quite essential anyway.

If that's the only thing to change, I'd start making a v3 for it.

> > 
> > >   Anyway, there was one commit in the usptream which was touching the acceleration. So I have
> > > to
> > > try it on the broken machine and see if it's still broken or not.
> > If you refer to oe-core upstream, yes there have been changes.
> > This is why I needed to rebase and rework my branch.
> > Or are there other changes I am not aware of, yet?
> Not sure if you caught the point. I mean, that I will take a look on the 
> upstream fixes for acceleration and then we can enable it by default.

Ok, so you referred to *busybox* upstream, not oe-core. Got it.

I've found https://git.busybox.net/busybox/commit/?id=bf57f732a5b6842f6fa3e0f90385f039e5d6a92c
Is this the fix you referred to?

> > > > [...]
> > >   
> So if the result is, that the configuration hasn't been changed at the 
> end, I'm fine with the changes you did ;).

Sounds good!

Regards, Enrico

> Regards,
> Andrej
>
Peter Kjellerstedt Nov. 2, 2024, 8:42 p.m. UTC | #5
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Enrico Jörns
> Sent: den 29 oktober 2024 21:47
> To: Andrej Valek <andrej.v@skyrain.eu>; Richard Purdie <richard.purdie@linuxfoundation.org>
> Cc: yocto@pengutronix.de; openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH v2 2/9] busybox: disable defconfig options from simpler fragments
> 
> Hi Andrej,
> 
> thanks for your review!
> 
> Am Dienstag, dem 29.10.2024 um 17:32 +0100 schrieb Andrej Valek:
> > Hello Enrico,
> >
> > On 28.10.2024 11:39, Enrico Jörns wrote:
> > > Hej Andrej,
> > >
> > > Am Montag, dem 28.10.2024 um 10:06 +0100 schrieb Andrej Valek:
> > > >   Hello Enrico,
> > > >
> > > >   I tried to follow (and applied) your patch, but it looks little
> bit inconsistent to me. In
> > > > the
> > > > first round you enabled SHAx_HWACCEL, which is broken on some
> architectures. That's the
> > > > reason,
> > > > why is disabled by default. Ok, so first apply looks like this:
> > > let me rephrase my commit message.
> > >
> > > The SHAx_HWACCEL is broken on some architectures, right.
> > > However, when you take the defconfig, without the fragments, it will
> currently be enabled.
> > > Regardless of what's in my patch.
> > >
> > > Why? Because CONFIG_SHA1_HWACCEL in Busybox' kconfig is 'default y',
> has no dependency on other
> > > options and is not listed in the 'defconfig' file so far.
> > >
> > > Thus, my first patch should not change the current situation.
> > Ok, I see the point. So basically you can drop the sha1sum.cfg and
> > sha256sum.cfg and merge it to defconfing. The result will be, that the
> > acceleration is disabled.
> 
> Just to get it right: Do we talk about the HWACCEL only or about enabling
> SHA1SUM and SHA256SUM
> unconditionally and fully removing the fragments?
> 
> Personally, I'd be fine with this since the algorithm are quite essential
> anyway.
> 
> If that's the only thing to change, I'd start making a v3 for it.
> 
> > >
> > > >   Anyway, there was one commit in the usptream which was touching
> the acceleration. So I have
> > > > to
> > > > try it on the broken machine and see if it's still broken or not.
> > > If you refer to oe-core upstream, yes there have been changes.
> > > This is why I needed to rebase and rework my branch.
> > > Or are there other changes I am not aware of, yet?
> > Not sure if you caught the point. I mean, that I will take a look on the
> > upstream fixes for acceleration and then we can enable it by default.
> 
> Ok, so you referred to *busybox* upstream, not oe-core. Got it.
> 
> I've found
> https://git.busybox.net/busybox/commit/?id=bf57f732a5b6842f6fa3e0f90385f039e5d6a92c
> Is this the fix you referred to?

I have sent a patch to backport the above commit. I also sent two patches to 
remove the no longer needed sha_accel.cfg file, and to clean up the sha*sum.cfg 
files.

> > > > > [...]
> > > >
> > So if the result is, that the configuration hasn't been changed at the
> > end, I'm fine with the changes you did ;).
> 
> Sounds good!
> 
> Regards, Enrico
> 
> > Regards,
> > Andrej

//Peter
diff mbox series

Patch

diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-core/busybox/busybox/defconfig
index 6c5a8764ce..0ec65c65af 100644
--- a/meta/recipes-core/busybox/busybox/defconfig
+++ b/meta/recipes-core/busybox/busybox/defconfig
@@ -281,8 +281,8 @@  CONFIG_FEATURE_LS_USERNAME=y
 CONFIG_FEATURE_LS_COLOR=y
 # CONFIG_FEATURE_LS_COLOR_IS_DEFAULT is not set
 CONFIG_MD5SUM=y
-CONFIG_SHA1SUM=y
-CONFIG_SHA256SUM=y
+# CONFIG_SHA1SUM is not set
+# CONFIG_SHA256SUM is not set
 # CONFIG_SHA512SUM is not set
 # CONFIG_SHA3SUM is not set
 
@@ -386,8 +386,8 @@  CONFIG_DEFAULT_SETFONT_DIR=""
 CONFIG_LOADKMAP=y
 CONFIG_OPENVT=y
 CONFIG_RESET=y
-CONFIG_RESIZE=y
-CONFIG_FEATURE_RESIZE_PRINT=y
+# CONFIG_RESIZE is not set
+# CONFIG_FEATURE_RESIZE_PRINT is not set
 CONFIG_SETCONSOLE=y
 # CONFIG_FEATURE_SETCONSOLE_LONG_OPTIONS is not set
 # CONFIG_SETKEYCODES is not set
@@ -689,7 +689,7 @@  CONFIG_RDATE=y
 # CONFIG_RDEV is not set
 # CONFIG_READPROFILE is not set
 CONFIG_RENICE=y
-CONFIG_REV=y
+# CONFIG_REV is not set
 # CONFIG_RTCWAKE is not set
 # CONFIG_SCRIPT is not set
 # CONFIG_SCRIPTREPLAY is not set
@@ -1070,7 +1070,7 @@  CONFIG_KILLALL=y
 # CONFIG_LSOF is not set
 # CONFIG_MPSTAT is not set
 # CONFIG_NMETER is not set
-CONFIG_PGREP=y
+# CONFIG_PGREP is not set
 # CONFIG_PKILL is not set
 CONFIG_PIDOF=y
 # CONFIG_FEATURE_PIDOF_SINGLE is not set