linux-kernel - Re: [PATCH V6] gpio: virtio: Add IRQ support

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 21 Oct 2021 12:58:25 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Arnd Bergmann <arnd@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        "Enrico Weigelt, metux IT consult" <info@...ux.net>,
        Viresh Kumar <vireshk@...nel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Bill Mills <bill.mills@...aro.org>,
        Alex Bennée <alex.bennee@...aro.org>,
        Cornelia Huck <cohuck@...hat.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        "stratos-dev@...lists.linaro.org" <stratos-dev@...lists.linaro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Marc Zyngier <maz@...nel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH V6] gpio: virtio: Add IRQ support

On Thu, Oct 21, 2021 at 12:52 PM Viresh Kumar <viresh.kumar@...aro.org> wrote:
> On 21-10-21, 12:42, Andy Shevchenko wrote:
> > On Thu, Oct 21, 2021 at 7:34 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
> > > On 20-10-21, 18:10, Andy Shevchenko wrote:

...

> > > > >  struct virtio_gpio_config {
> > > > >         __le16 ngpio;
> > > > >         __u8 padding[2];
> > > > > @@ -44,4 +56,17 @@ struct virtio_gpio_response_get_names {
> > > > >         __u8 value[];
> > > > >  };
> > > > >
> > > > > +/* Virtio GPIO IRQ Request / Response */
> > > > > +struct virtio_gpio_irq_request {
> > > > > +       __le16 gpio;
> > > > > +};
> > > > > +
> > > > > +struct virtio_gpio_irq_response {
> > > > > +       __u8 status;
> > > > > +};
> > > > >
> > > > I’m wondering if those above should be packed.
> > >
> > > You are talking about the newly added ones or the ones before ?
> > >
> > > In any case, they are all already packed (i.e. they have explicit
> > > padding wherever required) and properly aligned. Compiler won't add
> > > any other padding to them.
> >
> > Is it only for 64-bit to 64-bit communications?
>
> That's what I have been looking at.
>
> > If there is a possibility to have 32-bit to 64-bit or vice versa
> > communication you have a problem.
>
> This should work as well.
>
> The structure will get aligned to the size of largest element and each
> element will be aligned to itself. I don't see how this will break
> even in case of 32/64 bit communication.

I admit I haven't looked into the specification, but in the past we
had had quite an issue exactly in GPIO on kernel side because of this
kind of design mistake. The problem here if in the future one wants to
supply more than one item at a time, it will be not possible with this
interface. Yes, I understand that in current design it's rather missed
scalability, but hey, I believe in the future we may need
performance-wise calls.

-- 
With Best Regards,
Andy Shevchenko