linux-kernel - Re: [RFC PATCH] gpio: uapi: v2 proposal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMpxmJV9SL9pJpc5MwhREiP0On999CRW2Em4SbRKf9PQHOtPCg@mail.gmail.com>
Date:   Tue, 9 Jun 2020 11:57:27 +0200
From:   Bartosz Golaszewski <bgolaszewski@...libre.com>
To:     Kent Gibson <warthog618@...il.com>
Cc:     Bartosz Golaszewski <brgl@...ev.pl>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-gpio <linux-gpio@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [RFC PATCH] gpio: uapi: v2 proposal

wt., 9 cze 2020 o 11:43 Kent Gibson <warthog618@...il.com> napisał(a):
>
> On Tue, Jun 09, 2020 at 10:03:42AM +0200, Bartosz Golaszewski wrote:
> > sob., 6 cze 2020 o 03:56 Kent Gibson <warthog618@...il.com> napisał(a):
> > >
> >
> > [snip!]
> >
> > > >
> > > > I'd say yes - consolidation and reuse of data structures is always
> > > > good and normally they are going to be wrapped in some kind of
> > > > low-level user-space library anyway.
> > > >
> > >
> > > Ok, and I've changed the values field name to bitmap, along with the change
> > > to a bitmap type, so the stuttering is gone.
> > >
> > > And, as the change to bitmap substantially reduced the size of
> > > gpioline_config, I now embed that in the gpioline_info instead of
> > > duplicating all the other fields.  The values field will be zeroed
> > > when returned within info.
> > >
> >
> > Could you post an example, I'm not sure I follow.
> >
>
> The gpioline_info_v2 now looks like this:
>
> /**
>  * struct gpioline_info_v2 - Information about a certain GPIO line
>  * @name: the name of this GPIO line, such as the output pin of the line on
>  * the chip, a rail or a pin header name on a board, as specified by the
>  * gpio chip, may be empty
>  * @consumer: a functional name for the consumer of this GPIO line as set
>  * by whatever is using it, will be empty if there is no current user but
>  * may also be empty if the consumer doesn't set this up
>  * @config: the configuration of the line.  Note that the values field is
>  * always zeroed.
>  * @offset: the local offset on this GPIO device, fill this in when
>  * requesting the line information from the kernel
>  * @padding: reserved for future use
>  */
> struct gpioline_info_v2 {
>         char name[GPIO_MAX_NAME_SIZE];
>         char consumer[GPIO_MAX_NAME_SIZE];
>         struct gpioline_config config;
>         __u32 offset;
>         __u32 padding[GPIOLINE_INFO_V2_PAD_SIZE]; /* for future use */
> };
>
> Previously that had all the fields from config - other than the values.
>
> When that is populated the config.values will always be zeroed.
>

We'll probably abstract this away in libgpiod and your Go library but
for someone looking at the ABI it may be confusing because a zeroed
values array is still valid. I don't have a better idea though.

[snip!]

>
> > > > > And would it be useful for userspace to be able to influence the size of
> > > > > the event buffer (currently fixed at 16 events per line)?
> > > > >
> > > >
> > > > Good question. I would prefer to not overdo it though. The event
> > > > request would need to contain the desired kfifo size and we'd only
> > > > allow to set it on request, right?
> > > >
> > >
> > > Yeah, it would only be relevant if edge detection was set and, as per
> > > edge detection itself, would only be settable via the request, not
> > > via set_config.  It would only be a suggestion, as the kfifo size gets
> > > rounded up to a power of 2 anyway.  It would be capped - I'm open to
> > > suggestions for a suitable max value.  And the 0 value would mean use
> > > the default - currently 16 per line.
> > >
> >
> > This sounds good. How about 512 for max value for now and we can
> > always increase it if needed. I don't think we should explicitly cap
> > it though - let the user specify any value and just silently limit it
> > to 512 in the kernel.
> >
>
> It will be an internal cap only - no error if the user requests more.
> I was thinking 1024, which corresponds to the maximum default - 16*64.
>

Yes, this sounds good too.

Bart