linux-kernel - Re: [PATCH v2] irqchip: add TS7800v1 fpga based controller driver

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: <87fsfggone.wl-maz@kernel.org>
Date:   Sat, 22 Oct 2022 11:04:37 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     firas ashkar <firas.ashkar@...oirfairelinux.com>
Cc:     alex@...riz.org.uk, tglx@...utronix.de,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] irqchip: add TS7800v1 fpga based controller driver

On Fri, 21 Oct 2022 19:14:48 +0100,
firas ashkar <firas.ashkar@...oirfairelinux.com> wrote:
> > > > > +static void ts7800_ic_chained_handle_irq(struct irq_desc
> > > > > *desc)
> > > > > +{
> > > > > +       struct ts7800_irq_data *data =
> > > > > irq_desc_get_handler_data(desc);
> > > > > +       struct irq_chip *chip = irq_desc_get_chip(desc);
> > > > > +       u32 mask_bits = readl(data->base + IRQ_MASK_REG);
> > > > > +       u32 status = readl(data->bridge);
> > > > > +
> > > > > +       chained_irq_enter(chip, desc);
> > > > > +
> > > > > +       if (unlikely(status == 0)) {
> > > > > +               handle_bad_irq(desc);
> > > > > +               goto out;
> > > > > +       }
> > > > > +
> > > > > +       do {
> > > > > +               unsigned int bit = __ffs(status);
> > > > > +               int irq = irq_find_mapping(data->domain, bit);
> > > > > +
> > > > > +               if (irq && (mask_bits & BIT(bit)))
> > > > > +                       generic_handle_irq(irq);
> > > > 
> > > > Again, this is not appropriate. I've pointed you to the right API
> > > > in
> > > > my previous review of this patch.
> > > 'generic_handle_domain_irq' causing some issues
> > 
> > Which issue?
> # insmod /tmp/virt-dma.ko 
> # insmod /tmp/ts-dmac.ko 
> generic_handle_domain_irq failed, error -22
> irq 2, desc: a285c39f, depth: 0, count: 0, unhandled: 0
> ->handle_irq():  e7046ee9, ts7800_ic_chained_handle_irq+0x0/0x1ec
> [irq_ts7800]
> ->irq_data.chip(): 70d81143, 0xc14db44c
> ->action(): f799c8dd
> ->action->handler(): dcf07981, bad_chained_irq+0x0/0x64
>      IRQ_LEVEL set
>    IRQ_NOPROBE set
>  IRQ_NOREQUEST set
>   IRQ_NOTHREAD set
> unexpected IRQ trap at vector 02
> generic_handle_domain_irq failed, error -22

[...]

News flash, your code is buggy. How about you debug it? The domain
name you use definitely indicates that you "know how" to do it...

	M.

-- 
Without deviation from the norm, progress is not possible.