Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220803042728.3230346-1-chenhuacai@loongson.cn>
 <87r11xbxox.wl-maz@kernel.org> <CAAhV-H5d20zj+vnrEYRYW1h7FwRJpGYtJDc1q4hRdPh3KbXruw@mail.gmail.com>
 <97d33f58-6a15-3a94-f9c0-42ebd48cf562@loongson.cn>
In-Reply-To: <97d33f58-6a15-3a94-f9c0-42ebd48cf562@loongson.cn>
From:   Huacai Chen <chenhuacai@gmail.com>
Date:   Thu, 4 Aug 2022 10:46:00 +0800
Message-ID: <CAAhV-H6hYu5vdpPar_8nMKMWUgD3MLzNBFLkHJyHKfjvCRJJ4w@mail.gmail.com>
Subject: Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow
To:     Jianmin Lv <lvjianmin@loongson.cn>
Cc:     Marc Zyngier <maz@kernel.org>,
        Huacai Chen <chenhuacai@loongson.cn>,
        Thomas Gleixner <tglx@linutronix.de>,
        loongarch@lists.linux.dev, LKML <linux-kernel@vger.kernel.org>,
        Xuefeng Li <lixuefeng@loongson.cn>,
        Jiaxun Yang <jiaxun.yang@flygoat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Aug 3, 2022 at 7:59 PM Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
>
>
> On 2022/8/3 =E4=B8=8B=E5=8D=883:33, Huacai Chen wrote:
> > Hi, Jianmin,
> >
> > On Wed, Aug 3, 2022 at 3:20 PM Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> On Wed, 03 Aug 2022 05:27:27 +0100,
> >> Huacai Chen <chenhuacai@loongson.cn> wrote:
> >>>
> >>> Check hwirq overflow when allocate irq in eiointc domain.
> >>>
> >>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> >>> ---
> >>>   drivers/irqchip/irq-loongson-eiointc.c | 7 +++++--
> >>>   1 file changed, 5 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip=
/irq-loongson-eiointc.c
> >>> index 80d8ca6f2d46..f8060e58ee06 100644
> >>> --- a/drivers/irqchip/irq-loongson-eiointc.c
> >>> +++ b/drivers/irqchip/irq-loongson-eiointc.c
> >>> @@ -241,8 +241,11 @@ static int eiointc_domain_alloc(struct irq_domai=
n *domain, unsigned int virq,
> >>>        struct eiointc *priv =3D domain->host_data;
> >>>
> >>>        ret =3D irq_domain_translate_onecell(domain, arg, &hwirq, &typ=
e);
> >>> -     if (ret)
> >>> -             return ret;
> >>> +     if (ret < 0)
> >>> +             return -EINVAL;
> >>> +
> >>> +     if (hwirq >=3D IOCSR_EXTIOI_VECTOR_NUM)
> >>> +             return -EINVAL;
> >>
> >> How can this happen? Also, you're allocating a *range*. Surely the
> >> upper boundary should matter too?
> > Do you know the exact reason? Please give some information, thanks.
> >
>
> In our internal repo, we don't have middle domain in pch-msi driver, so
> no check for hwirq as in alloc of middle domain. When hwirq is assigned
> failed(negtive value), the wrong hwirq will be passed to parent
> domain(eio domain)'s alloc, so we add check in eio domain's alloc there.
>
>
> But here, it seems that the check is unnecessary, because in pch-msi driv=
er:
>
> static int pch_msi_middle_domain_alloc(struct irq_domain *domain,
>                                             unsigned int virq,
>                                             unsigned int nr_irqs, void
> *args)
> {
>          struct pch_msi_data *priv =3D domain->host_data;
>          int hwirq, err, i;
>
>
>          hwirq =3D pch_msi_allocate_hwirq(priv, nr_irqs);
>          if (hwirq < 0)
>                  return hwirq;
>
>
>          for (i =3D 0; i < nr_irqs; i++) {
>                  err =3D pch_msi_parent_domain_alloc(domain, virq + i,
> hwirq + i);
>                  [...]
>
>
> If pch_msi_allocate_hwirq failed, pch_msi_middle_domain_alloc will
> return, and pch_msi_parent_domain_alloc(will call eio domain's alloc)
> will not be called.
OK, then this patch can be removed and I only need to send the 2nd one. Tha=
nks.

Huacai

>
>
> >>
> >> And for the umpteenth time, please add a cover letter when sending
> >> multiple patches. This is a hard requirement for me.
> > OK, I will add a cover letter, even for simple fix patches. Sorry.
> >
> > Huacai
> >>
> >> Thanks,
> >>
> >>          M.
> >>
> >> --
> >> Without deviation from the norm, progress is not possible.
>
>