linux-kernel - Re: [Stratos-dev] [PATCH V4 2/2] 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]

Message-ID: <20210809073020.y6ruibdm37xnx7hg@vireshk-i7>
Date:   Mon, 9 Aug 2021 13:00:20 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        Viresh Kumar <vireshk@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Cornelia Huck <cohuck@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE" 
        <virtualization@...ts.linux-foundation.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Marc Zyngier <maz@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Enrico Weigelt, metux IT consult" <info@...ux.net>,
        Jason Wang <jasowang@...hat.com>,
        Stratos Mailing List <stratos-dev@...lists.linaro.org>
Subject: Re: [Stratos-dev] [PATCH V4 2/2] gpio: virtio: Add IRQ support

On 06-08-21, 10:00, Arnd Bergmann wrote:
> On Fri, Aug 6, 2021 at 9:44 AM Viresh Kumar via Stratos-dev
> <stratos-dev@...lists.linaro.org> wrote:
> >
> > On 05-08-21, 15:10, Arnd Bergmann wrote:
> > > I hope this can still be simplified by working out better which state
> > > transitions are needed exactly. In particular, I would expect that we
> > > can get away with not sending a VIRTIO_GPIO_MSG_IRQ_TYPE
> > > for 'mask' state changes at all, but use that only for forcing 'enabled'
> > > state changes.
> >
> > Something like this ?
> 
> > static void virtio_gpio_irq_mask(struct irq_data *d)
> > {
> >         /* Nothing to do here */
> > }
> 
> You'd have to do /something/ here I think, if only setting the flag
> that we don't want to deliver the next interrupt.
> 
> > static void virtio_gpio_irq_unmask(struct irq_data *d)
> > {
> >         struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> >         struct virtio_gpio *vgpio = gpiochip_get_data(gc);
> >
> >         /* Queue the buffer unconditionally on unmask */
> >         virtio_gpio_irq_prepare(vgpio, d->hwirq);
> > }
> 
> And check the flag here to not requeue it if it's masked.

I am not sure I understand why this would be required. If the
interrupt is getting disabled, then unmask will not get called here
and so we won't requeue anything. Same will happen with threaded
handlers where the interrupt gets unmasked at a later point of time.

> Now, there is already a flag in the irq descriptor, so rather than
> having double accounting, the easy way may be to
> just use irqd_irq_masked()/irq_state_set_masked(), or
> have the irq core take care of this.

-- 
viresh