Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow

[Huacai Chen]

On Wed, Aug 3, 2022 at 7:59 PM Jianmin Lv <lvjianmin@xxxxxxxxxxx> wrote:
>
>
>
> On 2022/8/3 下午3:33, Huacai Chen wrote:
> > Hi, Jianmin,
> >
> > On Wed, Aug 3, 2022 at 3:20 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> >>
> >> On Wed, 03 Aug 2022 05:27:27 +0100,
> >> Huacai Chen <chenhuacai@xxxxxxxxxxx> wrote:
> >>>
> >>> Check hwirq overflow when allocate irq in eiointc domain.
> >>>
> >>> Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
> >>> ---
> >>>   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_domain *domain, unsigned int virq,
> >>>        struct eiointc *priv = domain->host_data;
> >>>
> >>>        ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
> >>> -     if (ret)
> >>> -             return ret;
> >>> +     if (ret < 0)
> >>> +             return -EINVAL;
> >>> +
> >>> +     if (hwirq >= 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 driver:
>
> 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 = domain->host_data;
>          int hwirq, err, i;
>
>
>          hwirq = pch_msi_allocate_hwirq(priv, nr_irqs);
>          if (hwirq < 0)
>                  return hwirq;
>
>
>          for (i = 0; i < nr_irqs; i++) {
>                  err = 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. Thanks.

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.
>
>