[PATCH] busybox: add devmem

[PATCH] busybox: add devmem

[Adrian Bunk]

This is a tiny but pretty useful tool.

Signed-off-by: Adrian Bunk <bunk@stusta.de>
---
 meta/recipes-core/busybox/busybox/defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-core/busybox/busybox/defconfig
index d11707abc3..1519159a49 100644
--- a/meta/recipes-core/busybox/busybox/defconfig
+++ b/meta/recipes-core/busybox/busybox/defconfig
@@ -791,7 +791,7 @@ CONFIG_DC=y
 # CONFIG_DEVFSD_FG_NP is not set
 # CONFIG_DEVFSD_VERBOSE is not set
 # CONFIG_FEATURE_DEVFS is not set
-# CONFIG_DEVMEM is not set
+CONFIG_DEVMEM=y
 # CONFIG_EJECT is not set
 # CONFIG_FEATURE_EJECT_SCSI is not set
 # CONFIG_FBSPLASH is not set
-- 
2.11.0

Re: [PATCH] busybox: add devmem

[Khem Raj]

On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
>
> This is a tiny but pretty useful tool.
>

question is, do we need this enabled in default config, or could be add it via
some DISTRO_FEATURE meant for validation etc. May be if you explain your
usecase then we might be able to make a better assessment.

> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> ---
>  meta/recipes-core/busybox/busybox/defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-core/busybox/busybox/defconfig
> index d11707abc3..1519159a49 100644
> --- a/meta/recipes-core/busybox/busybox/defconfig
> +++ b/meta/recipes-core/busybox/busybox/defconfig
> @@ -791,7 +791,7 @@ CONFIG_DC=y
>  # CONFIG_DEVFSD_FG_NP is not set
>  # CONFIG_DEVFSD_VERBOSE is not set
>  # CONFIG_FEATURE_DEVFS is not set
> -# CONFIG_DEVMEM is not set
> +CONFIG_DEVMEM=y
>  # CONFIG_EJECT is not set
>  # CONFIG_FEATURE_EJECT_SCSI is not set
>  # CONFIG_FBSPLASH is not set
> --
> 2.11.0
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] busybox: add devmem

[Andre McCurdy]

On Wed, Jan 30, 2019 at 8:50 AM Khem Raj <raj.khem@gmail.com> wrote:
> On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > This is a tiny but pretty useful tool.
>
> question is, do we need this enabled in default config, or could be add it via
> some DISTRO_FEATURE meant for validation etc. May be if you explain your
> usecase then we might be able to make a better assessment.

It's not just a validation tool. I've seen production code which sets
up GPIOs etc using calls to devmem from an init script.

Of course it's not needed by everyone, but it's a useful tool for some
and it's so small it doesn't really deserve it's own conditional logic
in the recipe.

> > Signed-off-by: Adrian Bunk <bunk@stusta.de>> > ---
> >  meta/recipes-core/busybox/busybox/defconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-core/busybox/busybox/defconfig
> > index d11707abc3..1519159a49 100644
> > --- a/meta/recipes-core/busybox/busybox/defconfig
> > +++ b/meta/recipes-core/busybox/busybox/defconfig
> > @@ -791,7 +791,7 @@ CONFIG_DC=y
> >  # CONFIG_DEVFSD_FG_NP is not set
> >  # CONFIG_DEVFSD_VERBOSE is not set
> >  # CONFIG_FEATURE_DEVFS is not set
> > -# CONFIG_DEVMEM is not set
> > +CONFIG_DEVMEM=y
> >  # CONFIG_EJECT is not set
> >  # CONFIG_FEATURE_EJECT_SCSI is not set
> >  # CONFIG_FBSPLASH is not set
> > --
> > 2.11.0

Re: [PATCH] busybox: add devmem

[Khem Raj]

On Wed, Jan 30, 2019 at 11:48 AM Andre McCurdy <armccurdy@gmail.com> wrote:
>
> On Wed, Jan 30, 2019 at 8:50 AM Khem Raj <raj.khem@gmail.com> wrote:
> > On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > This is a tiny but pretty useful tool.
> >
> > question is, do we need this enabled in default config, or could be add it via
> > some DISTRO_FEATURE meant for validation etc. May be if you explain your
> > usecase then we might be able to make a better assessment.
>
> It's not just a validation tool. I've seen production code which sets
> up GPIOs etc using calls to devmem from an init script.
>
> Of course it's not needed by everyone, but it's a useful tool for some
> and it's so small it doesn't really deserve it's own conditional logic
> in the recipe.
>

Thanks for information, lets see what Adrian's case is.

> > > Signed-off-by: Adrian Bunk <bunk@stusta.de>> > ---
> > >  meta/recipes-core/busybox/busybox/defconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-core/busybox/busybox/defconfig
> > > index d11707abc3..1519159a49 100644
> > > --- a/meta/recipes-core/busybox/busybox/defconfig
> > > +++ b/meta/recipes-core/busybox/busybox/defconfig
> > > @@ -791,7 +791,7 @@ CONFIG_DC=y
> > >  # CONFIG_DEVFSD_FG_NP is not set
> > >  # CONFIG_DEVFSD_VERBOSE is not set
> > >  # CONFIG_FEATURE_DEVFS is not set
> > > -# CONFIG_DEVMEM is not set
> > > +CONFIG_DEVMEM=y
> > >  # CONFIG_EJECT is not set
> > >  # CONFIG_FEATURE_EJECT_SCSI is not set
> > >  # CONFIG_FBSPLASH is not set
> > > --
> > > 2.11.0

Re: [PATCH] busybox: add devmem

[Scott Ellis]

Is the busybox devmem functionally different then the standalone devmem2
package?

Re: [PATCH] busybox: add devmem

[Adrian Bunk]

On Wed, Jan 30, 2019 at 08:50:02AM -0800, Khem Raj wrote:
> On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
> >
> > This is a tiny but pretty useful tool.
> >
> 
> question is, do we need this enabled in default config, or could be add it via
> some DISTRO_FEATURE meant for validation etc. May be if you explain your
> usecase then we might be able to make a better assessment.

Sorry for being too terse.

devmem allows reading and writing hardware configuration registers,
which is useful both for debugging and for scripts.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] busybox: add devmem

[Khem Raj]

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

On Wed, Jan 30, 2019 at 12:31 PM Adrian Bunk <bunk@stusta.de> wrote:

> On Wed, Jan 30, 2019 at 08:50:02AM -0800, Khem Raj wrote:
> > On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
> > >
> > > This is a tiny but pretty useful tool.
> > >
> >
> > question is, do we need this enabled in default config, or could be add
> it via
> > some DISTRO_FEATURE meant for validation etc. May be if you explain your
> > usecase then we might be able to make a better assessment.
>
> Sorry for being too terse.
>
> devmem allows reading and writing hardware configuration registers,
> which is useful both for debugging and for scripts.
>

Thanks for the info this seems useful can you also report how much does it
increase size of busybox

>
> cu
> Adrian
>
> --
>
>        "Is there not promise of rain?" Ling Tan asked suddenly out
>         of the darkness. There had been need of rain for many days.
>        "Only a promise," Lao Er said.
>                                        Pearl S. Buck - Dragon Seed
>
>

[-- Attachment #2: Type: text/html, Size: 1651 bytes --]

Re: [PATCH] busybox: add devmem

[Richard Purdie]

On Wed, 2019-01-30 at 14:18 -0800, Khem Raj wrote:
> 
> 
> On Wed, Jan 30, 2019 at 12:31 PM Adrian Bunk <bunk@stusta.de> wrote:
> > On Wed, Jan 30, 2019 at 08:50:02AM -0800, Khem Raj wrote:
> > > On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de>
> > wrote:
> > > >
> > > > This is a tiny but pretty useful tool.
> > > >
> > > 
> > > question is, do we need this enabled in default config, or could
> > be add it via
> > > some DISTRO_FEATURE meant for validation etc. May be if you
> > explain your
> > > usecase then we might be able to make a better assessment.
> > 
> > Sorry for being too terse.
> > 
> > devmem allows reading and writing hardware configuration registers,
> > which is useful both for debugging and for scripts.
> > 
> 
> Thanks for the info this seems useful can you also report how much
> does it increase size of busybox 

One reason I'm a little nervous of devmem in busybox is security attack
surface. You can of course argue that there are 101 ways to mimic
devmem so this isn't a concern. When you consider some configurations
can have suid busybox, it becomes more of a worry. Our default
separates out the suid pieces for the attack surface reason.

It is useful so I am torn but its worth keeping this in mind...

Cheers,

Richard

Re: [PATCH] busybox: add devmem

[Leon Woestenberg]

Hi all,

On Wed, Jan 30, 2019 at 11:32 PM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> One reason I'm a little nervous of devmem in busybox is security attack
> surface.
> It is useful so I am torn but its worth keeping this in mind...
>
I agree with this reasoning.

devmem(2) really is a development tool, and indeed I would leave it
out of any defaults in Yocto.
There are numerous attempts to minimize cruft, and in typical images
devmem should not be used.
For the people that need it, it is typically easier to add such tools
to their image than it is to minimize their image.

As for automated deployment, I would rather see that an imaginary
PACKAGECONFIG[debug-tweaks] or such would include this busybox tool
through configuration rather than have it opt-out.

Just my two cents (and yes devmem is in my images but they are for
development, not in releases).

Regards,

Leon.

Re: [PATCH] busybox: add devmem

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]

On Wed, Jan 30, 2019 at 10:31:58PM +0000, Richard Purdie wrote:
> On Wed, 2019-01-30 at 14:18 -0800, Khem Raj wrote:
> > 
> > 
> > On Wed, Jan 30, 2019 at 12:31 PM Adrian Bunk <bunk@stusta.de> wrote:
> > > On Wed, Jan 30, 2019 at 08:50:02AM -0800, Khem Raj wrote:
> > > > On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de>
> > > wrote:
> > > > >
> > > > > This is a tiny but pretty useful tool.
> > > > >
> > > > 
> > > > question is, do we need this enabled in default config, or could
> > > be add it via
> > > > some DISTRO_FEATURE meant for validation etc. May be if you
> > > explain your
> > > > usecase then we might be able to make a better assessment.
> > > 
> > > Sorry for being too terse.
> > > 
> > > devmem allows reading and writing hardware configuration registers,
> > > which is useful both for debugging and for scripts.
> > > 
> > 
> > Thanks for the info this seems useful can you also report how much
> > does it increase size of busybox 
> 
> One reason I'm a little nervous of devmem in busybox is security attack
> surface. You can of course argue that there are 101 ways to mimic
> devmem so this isn't a concern. When you consider some configurations
> can have suid busybox, it becomes more of a worry. Our default
> separates out the suid pieces for the attack surface reason.
> 
> It is useful so I am torn but its worth keeping this in mind...

I would also ask if we should be enabling more stuff in busybox, period.
Customizing busybox for what you're trying to _do_ with a custom setup
is one of those top 5 TODO items with making a cut down image and
customizing the kernel config.  Outside of -tiny and initramfs/similar
cases, there's not a great reason to use
almost-but-not-quite-complete-busybox-applet compared with the regular
app.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] busybox: add devmem

[Philip Balister]

On 01/30/2019 05:50 PM, Khem Raj wrote:
> On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
>>
>> This is a tiny but pretty useful tool.

Also there is http://layers.openembedded.org/layerindex/recipe/1069/

I was told a while ago the busybox version has some issues. Hunting
through brain cells, I think it always does a read back after writing
which can be bad for some hardware. So I've always used the recipe from
meta-oe instead.

Philip

>>
> 
> question is, do we need this enabled in default config, or could be add it via
> some DISTRO_FEATURE meant for validation etc. May be if you explain your
> usecase then we might be able to make a better assessment.
> 
>> Signed-off-by: Adrian Bunk <bunk@stusta.de>
>> ---
>>  meta/recipes-core/busybox/busybox/defconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meta/recipes-core/busybox/busybox/defconfig b/meta/recipes-core/busybox/busybox/defconfig
>> index d11707abc3..1519159a49 100644
>> --- a/meta/recipes-core/busybox/busybox/defconfig
>> +++ b/meta/recipes-core/busybox/busybox/defconfig
>> @@ -791,7 +791,7 @@ CONFIG_DC=y
>>  # CONFIG_DEVFSD_FG_NP is not set
>>  # CONFIG_DEVFSD_VERBOSE is not set
>>  # CONFIG_FEATURE_DEVFS is not set
>> -# CONFIG_DEVMEM is not set
>> +CONFIG_DEVMEM=y
>>  # CONFIG_EJECT is not set
>>  # CONFIG_FEATURE_EJECT_SCSI is not set
>>  # CONFIG_FBSPLASH is not set
>> --
>> 2.11.0
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] busybox: add devmem

[Philip Balister]

On 01/30/2019 09:09 PM, Scott Ellis wrote:
> Is the busybox devmem functionally different then the standalone devmem2
> package?

Replying again ....

I've been told they are functionally different and to use devmem2. I
think the issue with the busybox version is that it does a readback when
writing, this can be bad for some hardware.

Philip

Re: [PATCH] busybox: add devmem

[Leon Woestenberg]

On Thu, Jan 31, 2019 at 11:16 AM Philip Balister <philip@balister.org> wrote:
>
> On 01/30/2019 09:09 PM, Scott Ellis wrote:
> > Is the busybox devmem functionally different then the standalone devmem2
> > package?
>
> Replying again ....
>
> I've been told they are functionally different and to use devmem2. I
> think the issue with the busybox version is that it does a readback when
> writing, this can be bad for some hardware.
>
And the fact that the devmem2 tool is written with no fixed integer
width. You have to understand that on a 64-bit system, reading a
"word" from a 32-bit hardware peripheral (most are), you are touching
two registers.

        case 'h':
            read_result = *((unsigned short *) virt_addr);
            break;
        case 'w':
            read_result = *((unsigned long *) virt_addr);
            break;

I rewrote it once to use uint<N>_t but I do not think there is a
proper upstream repository / maintenance going on.

Not sure about devmem in Busybox.

Anyway, devmem being useful for development, I do not see why it
should be in BusyBox by default.  We are not shipping devmem2 and
should not.
We can have such reasoning for every tool out there. I rather would
have a single switch to include tools like these.

I disagree that devmem(2) should be included for purposes such as
pin-muxing. Yes, I use it weekly for that during bring-up, but the
released result should be in DTS or DTS overlays, or boot loader code
in the end.

Regards,
-- 
Leon Woestenberg
http://www.sidebranch.com

Re: [PATCH] busybox: add devmem

[Adrian Bunk]

On Thu, Jan 31, 2019 at 11:06:22AM +0100, Philip Balister wrote:
> On 01/30/2019 05:50 PM, Khem Raj wrote:
> > On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
> >>
> >> This is a tiny but pretty useful tool.
> 
> Also there is http://layers.openembedded.org/layerindex/recipe/1069/
> 
> I was told a while ago the busybox version has some issues. Hunting
> through brain cells, I think it always does a read back after writing
> which can be bad for some hardware. So I've always used the recipe from
> meta-oe instead.

The broken one for that is actually devmem2, the busybox copy of the 
code has the read back commented out.

> Philip

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] busybox: add devmem

[Adrian Bunk]

On Wed, Jan 30, 2019 at 02:18:06PM -0800, Khem Raj wrote:
> On Wed, Jan 30, 2019 at 12:31 PM Adrian Bunk <bunk@stusta.de> wrote:
> 
> > On Wed, Jan 30, 2019 at 08:50:02AM -0800, Khem Raj wrote:
> > > On Wed, Jan 30, 2019 at 1:34 AM Adrian Bunk <bunk@stusta.de> wrote:
> > > >
> > > > This is a tiny but pretty useful tool.
> > > >
> > >
> > > question is, do we need this enabled in default config, or could be add
> > it via
> > > some DISTRO_FEATURE meant for validation etc. May be if you explain your
> > > usecase then we might be able to make a better assessment.
> >
> > Sorry for being too terse.
> >
> > devmem allows reading and writing hardware configuration registers,
> > which is useful both for debugging and for scripts.
> >
> 
> Thanks for the info this seems useful can you also report how much does it
> increase size of busybox

In a -O2 build for arm64 busybox.nosuid grows 4 kB.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] busybox: add devmem

[Adrian Bunk]

On Wed, Jan 30, 2019 at 05:47:03PM -0500, Tom Rini wrote:
> 
> I would also ask if we should be enabling more stuff in busybox, period.

I don't think the current status quo would be a good starting point for that.

Before submitting I saw CONFIG_PATCH=y, which is a lot more of a
"Why would I ever need this on an embedded device?".

Not saying that you are wrong, but the starting point should be a review
of the current config.

> Customizing busybox for what you're trying to _do_ with a custom setup
> is one of those top 5 TODO items with making a cut down image and
> customizing the kernel config.  Outside of -tiny and initramfs/similar
> cases, there's not a great reason to use
> almost-but-not-quite-complete-busybox-applet compared with the regular
> app.

I frequently end up in projects where I do have great reason for that:

For root filesystems of the 32-64 MB size it is not necessary to use a 
different libc or use -Os, but using the busybox applets when they are 
sufficient saves > 10% filesystem size compared to the full versions.

E.g. 0.45 MB for a standalone tar is much when the little functionality 
you actually need is also provided by the busybox applet.

Plus there is also the convenience point that most of the tools you need 
are already installed just by adding busybox with the default config.

> Tom

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] busybox: add devmem

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]

On Thu, Jan 31, 2019 at 09:04:53PM +0200, Adrian Bunk wrote:
> On Wed, Jan 30, 2019 at 05:47:03PM -0500, Tom Rini wrote:
> > 
> > I would also ask if we should be enabling more stuff in busybox, period.
> 
> I don't think the current status quo would be a good starting point for that.
> 
> Before submitting I saw CONFIG_PATCH=y, which is a lot more of a
> "Why would I ever need this on an embedded device?".
> 
> Not saying that you are wrong, but the starting point should be a review
> of the current config.

Yes, somewhere between an audit of busybox and making it easier /
clearer on how to remove busybox on a "regular" image (core-image-*,
etc) are on my TODO list.  You might also get surprised btw where things
like patch get, erm, differently-used in systems in automated ways.

> > Customizing busybox for what you're trying to _do_ with a custom setup
> > is one of those top 5 TODO items with making a cut down image and
> > customizing the kernel config.  Outside of -tiny and initramfs/similar
> > cases, there's not a great reason to use
> > almost-but-not-quite-complete-busybox-applet compared with the regular
> > app.
> 
> I frequently end up in projects where I do have great reason for that:
> 
> For root filesystems of the 32-64 MB size it is not necessary to use a 
> different libc or use -Os, but using the busybox applets when they are 
> sufficient saves > 10% filesystem size compared to the full versions.
> 
> E.g. 0.45 MB for a standalone tar is much when the little functionality 
> you actually need is also provided by the busybox applet.
> 
> Plus there is also the convenience point that most of the tools you need 
> are already installed just by adding busybox with the default config.

Right.  This is the -tiny and similar cases.  Busybox is great for a lot
of things.  But there's also common cases where it's not.

But the reasons I object here are, aside from what others have brought
up about devmem vs devmem2 vs putting stuff in DTS files, etc, is that I
really don't want to see more implicit deps to busybox added in.
busybox is in virtually every single OE fs image, so I am going to argue
that every new utility we add there needs a real good justification.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]