All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow
@ 2022-08-03  4:27 Huacai Chen
  2022-08-03  4:27 ` [PATCH 2/2] irqchip/loongson-eiointc: Fix irq affinity setting Huacai Chen
  2022-08-03  7:20 ` [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow Marc Zyngier
  0 siblings, 2 replies; 6+ messages in thread
From: Huacai Chen @ 2022-08-03  4:27 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier
  Cc: loongarch, linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang,
	Huacai Chen

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_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;
 
 	for (i = 0; i < nr_irqs; i++) {
 		irq_domain_set_info(domain, virq + i, hwirq + i, &eiointc_irq_chip,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] irqchip/loongson-eiointc: Fix irq affinity setting
  2022-08-03  4:27 [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow Huacai Chen
@ 2022-08-03  4:27 ` Huacai Chen
  2022-08-03  7:20 ` [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow Marc Zyngier
  1 sibling, 0 replies; 6+ messages in thread
From: Huacai Chen @ 2022-08-03  4:27 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier
  Cc: loongarch, linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang,
	Huacai Chen

In multi-node case, csr_any_send() should set EIOINTC_REG_ENABLE of the
first core of target node, not the first core of the whole.

Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
 drivers/irqchip/irq-loongson-eiointc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index f8060e58ee06..58c1cf47b3d7 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -111,11 +111,15 @@ static int eiointc_set_irq_affinity(struct irq_data *d, const struct cpumask *af
 	regaddr = EIOINTC_REG_ENABLE + ((vector >> 5) << 2);
 
 	/* Mask target vector */
-	csr_any_send(regaddr, EIOINTC_ALL_ENABLE & (~BIT(vector & 0x1F)), 0x0, 0);
+	csr_any_send(regaddr, EIOINTC_ALL_ENABLE & (~BIT(vector & 0x1F)),
+			0x0, priv->node * CORES_PER_EIO_NODE);
+
 	/* Set route for target vector */
 	eiointc_set_irq_route(vector, cpu, priv->node, &priv->node_map);
+
 	/* Unmask target vector */
-	csr_any_send(regaddr, EIOINTC_ALL_ENABLE, 0x0, 0);
+	csr_any_send(regaddr, EIOINTC_ALL_ENABLE,
+			0x0, priv->node * CORES_PER_EIO_NODE);
 
 	irq_data_update_effective_affinity(d, cpumask_of(cpu));
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow
  2022-08-03  4:27 [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow Huacai Chen
  2022-08-03  4:27 ` [PATCH 2/2] irqchip/loongson-eiointc: Fix irq affinity setting Huacai Chen
@ 2022-08-03  7:20 ` Marc Zyngier
  2022-08-03  7:33   ` Huacai Chen
  1 sibling, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2022-08-03  7:20 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Thomas Gleixner, loongarch, linux-kernel, Xuefeng Li,
	Huacai Chen, Jiaxun Yang

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_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?

And for the umpteenth time, please add a cover letter when sending
multiple patches. This is a hard requirement for me.

Thanks,

	M.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow
  2022-08-03  7:20 ` [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow Marc Zyngier
@ 2022-08-03  7:33   ` Huacai Chen
  2022-08-03 11:58     ` Jianmin Lv
  0 siblings, 1 reply; 6+ messages in thread
From: Huacai Chen @ 2022-08-03  7:33 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Huacai Chen, Thomas Gleixner, loongarch, LKML, Xuefeng Li, Jiaxun Yang

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow
  2022-08-03  7:33   ` Huacai Chen
@ 2022-08-03 11:58     ` Jianmin Lv
  2022-08-04  2:46       ` Huacai Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Jianmin Lv @ 2022-08-03 11:58 UTC (permalink / raw)
  To: Huacai Chen, Marc Zyngier
  Cc: Huacai Chen, Thomas Gleixner, loongarch, LKML, Xuefeng Li, Jiaxun Yang



On 2022/8/3 下午3: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_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.


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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow
  2022-08-03 11:58     ` Jianmin Lv
@ 2022-08-04  2:46       ` Huacai Chen
  0 siblings, 0 replies; 6+ messages in thread
From: Huacai Chen @ 2022-08-04  2:46 UTC (permalink / raw)
  To: Jianmin Lv
  Cc: Marc Zyngier, Huacai Chen, Thomas Gleixner, loongarch, LKML,
	Xuefeng Li, Jiaxun Yang

On Wed, Aug 3, 2022 at 7:59 PM Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
>
>
> On 2022/8/3 下午3: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_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.
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-08-04  2:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03  4:27 [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow Huacai Chen
2022-08-03  4:27 ` [PATCH 2/2] irqchip/loongson-eiointc: Fix irq affinity setting Huacai Chen
2022-08-03  7:20 ` [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow Marc Zyngier
2022-08-03  7:33   ` Huacai Chen
2022-08-03 11:58     ` Jianmin Lv
2022-08-04  2:46       ` Huacai Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.