2023-04-24 10:34:09

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH 0/2] irqchip/mips-gic fixes

Hi all,

This patchset fixes two issues in the MIPS GIC driver that may block
booting on some systems.

Please review.

Thanks

Jiaxun Yang (2):
irqchip/mips-gic: Don't touch vl_map if a local interrupt is not
routable
irqchip/mips-gic: Use raw spinlock for gic_lock

drivers/irqchip/irq-mips-gic.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)

--
2.34.1


2023-04-24 10:34:33

by Jiaxun Yang

[permalink] [raw]
Subject: [PATCH 1/2] irqchip/mips-gic: Don't touch vl_map if a local interrupt is not routable

When a GIC local interrupt is not routable, it's vl_map will be used
to control some internal states for core (providing IPTI, IPPCI, IPFDC
input signal for core). Overriding it will interfere core's intetrupt
controller.

Do not touch vl_map if a local interrupt is not routable, we are not
going to remap it.

Before dd098a0e0319 (" irqchip/mips-gic: Get rid of the reliance on
irq_cpu_online()"), if a local interrupt is not routable, then it won't
be requested from GIC Local domain, and thus gic_all_vpes_irq_cpu_online
won't be called for that particular interrupt.

Fixes: dd098a0e0319 (" irqchip/mips-gic: Get rid of the reliance on irq_cpu_online()")
Cc: [email protected]
Signed-off-by: Jiaxun Yang <[email protected]>
---
drivers/irqchip/irq-mips-gic.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 046c355e120b..b568d55ef7c5 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -399,6 +399,8 @@ static void gic_all_vpes_irq_cpu_online(void)
unsigned int intr = local_intrs[i];
struct gic_all_vpes_chip_data *cd;

+ if (!gic_local_irq_is_routable(intr))
+ continue;
cd = &gic_all_vpes_chip_data[intr];
write_gic_vl_map(mips_gic_vx_map_reg(intr), cd->map);
if (cd->mask)
--
2.34.1

2023-05-12 12:26:58

by Jiaxun Yang

[permalink] [raw]
Subject: Re: [PATCH 0/2] irqchip/mips-gic fixes



> 2023年4月24日 11:31,Jiaxun Yang <[email protected]> 写道:
>
> Hi all,
>
> This patchset fixes two issues in the MIPS GIC driver that may block
> booting on some systems.
>
> Please review.

Ping?

I expect this series go through irqchip fixes tree.

Thanks
- Jiaxun

>
> Thanks
>
> Jiaxun Yang (2):
> irqchip/mips-gic: Don't touch vl_map if a local interrupt is not
> routable
> irqchip/mips-gic: Use raw spinlock for gic_lock
>
> drivers/irqchip/irq-mips-gic.c | 32 +++++++++++++++++---------------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> --
> 2.34.1
>


2023-05-14 14:12:49

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH 1/2] irqchip/mips-gic: Don't touch vl_map if a local interrupt is not routable

Hello Jiaxun

On Mon, Apr 24, 2023 at 11:31:55AM +0100, Jiaxun Yang wrote:
> When a GIC local interrupt is not routable, it's vl_map will be used
> to control some internal states for core (providing IPTI, IPPCI, IPFDC

> input signal for core). Overriding it will interfere core's intetrupt

s/intetrupt/interrupt

> controller.
>
> Do not touch vl_map if a local interrupt is not routable, we are not
> going to remap it.
>
> Before dd098a0e0319 (" irqchip/mips-gic: Get rid of the reliance on
> irq_cpu_online()"), if a local interrupt is not routable, then it won't
> be requested from GIC Local domain, and thus gic_all_vpes_irq_cpu_online
> won't be called for that particular interrupt.
>
> Fixes: dd098a0e0319 (" irqchip/mips-gic: Get rid of the reliance on irq_cpu_online()")
> Cc: [email protected]
> Signed-off-by: Jiaxun Yang <[email protected]>

My system doesn't have VPEs but 2x MIPS32 P5600 cores with GIC enabled as
EIC device so I can't fully test this change, but at the very least it
looks reasonable. Indeed performing the local IRQs routing setups for
the non-routable IRQs looks invalid. A similar change can be spotted
in the gic_irq_domain_map() method implementation.

> ---
> drivers/irqchip/irq-mips-gic.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
> index 046c355e120b..b568d55ef7c5 100644
> --- a/drivers/irqchip/irq-mips-gic.c
> +++ b/drivers/irqchip/irq-mips-gic.c
> @@ -399,6 +399,8 @@ static void gic_all_vpes_irq_cpu_online(void)
> unsigned int intr = local_intrs[i];
> struct gic_all_vpes_chip_data *cd;
>
> + if (!gic_local_irq_is_routable(intr))
> + continue;

Please add newline here to distinguish the skip-step code chunk and
the setup code so the look would look a tiny bit more readable.

> cd = &gic_all_vpes_chip_data[intr];
> write_gic_vl_map(mips_gic_vx_map_reg(intr), cd->map);
> if (cd->mask)

Other than that the change looks good. Thanks.

Reviewed-by: Serge Semin <[email protected]>

-Serge(y)

> --
> 2.34.1
>

2023-05-14 14:34:25

by Serge Semin

[permalink] [raw]
Subject: Re: [PATCH 0/2] irqchip/mips-gic fixes

Hi Jiaxun

On Mon, Apr 24, 2023 at 11:31:54AM +0100, Jiaxun Yang wrote:
> Hi all,
>
> This patchset fixes two issues in the MIPS GIC driver that may block
> booting on some systems.
>
> Please review.

Thanks for the series. Please find some comments sent in the framework
of the corresponding patches which don't though prevent from having
the patchset tested. No problems were found on the Baikal-T1 SoC with
2x MIPS32 P5600 core on board. So feel free to add for the entire
series:

Tested-by: Serge Semin <[email protected]>

-Serge(y)

>
> Thanks
>
> Jiaxun Yang (2):
> irqchip/mips-gic: Don't touch vl_map if a local interrupt is not
> routable
> irqchip/mips-gic: Use raw spinlock for gic_lock
>
> drivers/irqchip/irq-mips-gic.c | 32 +++++++++++++++++---------------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> --
> 2.34.1
>

Subject: [irqchip: irq/irqchip-fixes] irqchip/mips-gic: Don't touch vl_map if a local interrupt is not routable

The following commit has been merged into the irq/irqchip-fixes branch of irqchip:

Commit-ID: 2c6c9c049510163090b979ea5f92a68ae8d93c45
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/2c6c9c049510163090b979ea5f92a68ae8d93c45
Author: Jiaxun Yang <[email protected]>
AuthorDate: Mon, 24 Apr 2023 11:31:55 +01:00
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 16 May 2023 10:59:28 +01:00

irqchip/mips-gic: Don't touch vl_map if a local interrupt is not routable

When a GIC local interrupt is not routable, it's vl_map will be used
to control some internal states for core (providing IPTI, IPPCI, IPFDC
input signal for core). Overriding it will interfere core's intetrupt
controller.

Do not touch vl_map if a local interrupt is not routable, we are not
going to remap it.

Before dd098a0e0319 (" irqchip/mips-gic: Get rid of the reliance on
irq_cpu_online()"), if a local interrupt is not routable, then it won't
be requested from GIC Local domain, and thus gic_all_vpes_irq_cpu_online
won't be called for that particular interrupt.

Fixes: dd098a0e0319 (" irqchip/mips-gic: Get rid of the reliance on irq_cpu_online()")
Cc: [email protected]
Signed-off-by: Jiaxun Yang <[email protected]>
Reviewed-by: Serge Semin <[email protected]>
Tested-by: Serge Semin <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-mips-gic.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 046c355..b568d55 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -399,6 +399,8 @@ static void gic_all_vpes_irq_cpu_online(void)
unsigned int intr = local_intrs[i];
struct gic_all_vpes_chip_data *cd;

+ if (!gic_local_irq_is_routable(intr))
+ continue;
cd = &gic_all_vpes_chip_data[intr];
write_gic_vl_map(mips_gic_vx_map_reg(intr), cd->map);
if (cd->mask)