[PATCH 0/3] Updates in irqchip framework to remove irqchip

[PATCH 0/3] Updates in irqchip framework to remove irqchip

[Anirudha Sarangi]

Available irqchip framework does not fully support use cases where an
irqchip driver has to be loaded and unloaded as a module. 
Existing Xilinx INTC driver does not have a remove path which means the
INTC IP cannot be removed from a removable partition.

Anirudha Sarangi (3):
  irqchip: xilinx: Avoid __init macro usage for xilinx_intc_of_init
  irqchip: Add support to remove irqchip driver modules.
  irqchip: xilinx: Add support to remove the Xilinx INTC driver module.

 drivers/irqchip/irq-xilinx-intc.c | 53 ++++++++++++++++++++++++++++---
 drivers/irqchip/irqchip.c         | 38 ++++++++++++++++++++--
 include/linux/irq.h               | 15 ++++++++-
 include/linux/of_irq.h            |  1 +
 kernel/irq/handle.c               |  2 +-
 5 files changed, 100 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH 1/3] irqchip: xilinx: Avoid __init macro usage for xilinx_intc_of_init

[Anirudha Sarangi]

This patch ensures that xilinx_intc_of_init API is not allocated into
the .init.text section. Since this API calls the API set_handle_irq
which uses __init macro to be put into .init.text section, this patch
makes changes to ensure that set_handle_irq also does not stay in
.init.text section.
This patch is in preparation for the patch through which the xilinx
intc driver will be loaded and unloaded as a module.

Signed-off-by: Anirudha Sarangi <anirudha.sarangi@xilinx.com>
---
 drivers/irqchip/irq-xilinx-intc.c | 4 ++--
 include/linux/irq.h               | 2 +-
 kernel/irq/handle.c               | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 1d3d273309bd..642733a6cbdf 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -177,8 +177,8 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-static int __init xilinx_intc_of_init(struct device_node *intc,
-					     struct device_node *parent)
+static int xilinx_intc_of_init(struct device_node *intc,
+			       struct device_node *parent)
 {
 	struct xintc_irq_chip *irqc;
 	int ret, irq;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 2efde6a79b7e..252fab8074de 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1250,7 +1250,7 @@ int ipi_send_mask(unsigned int virq, const struct cpumask *dest);
  * Returns 0 on success, or -EBUSY if an IRQ handler has already been
  * registered.
  */
-int __init set_handle_irq(void (*handle_irq)(struct pt_regs *));
+int set_handle_irq(void (*handle_irq)(struct pt_regs *));
 
 /*
  * Allows interrupt handlers to find the irqchip that's been registered as the
diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 762a928e18f9..a0b18e8f5af0 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -218,7 +218,7 @@ irqreturn_t handle_irq_event(struct irq_desc *desc)
 }
 
 #ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
-int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
+int set_handle_irq(void (*handle_irq)(struct pt_regs *))
 {
 	if (handle_arch_irq)
 		return -EBUSY;
-- 
2.17.1

[PATCH 2/3] irqchip: Add support to remove irqchip driver modules.

[Anirudha Sarangi]

The existing irqchip implementation does not fully support use cases
where an irqchip driver has to be used as a module. In particular there
is no support to remove an irqchip driver module.
The use cases where an irqchip driver has to be loaded and then removed
as a module are really relevant in fpga world. A user can decide to
have a irqchip as part of a removable partial fpga region. In such cases
not only the corresponding irqchip driver has to be loaded as a module,
but must also be removed when the removable partial region is removed.

The existing implementation updates the existing framework to achieve
the above said goal.

Signed-off-by: Anirudha Sarangi <anirudha.sarangi@xilinx.com>
---
 drivers/irqchip/irqchip.c | 38 +++++++++++++++++++++++++++++++++++---
 include/linux/irq.h       | 13 +++++++++++++
 include/linux/of_irq.h    |  1 +
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irqchip.c b/drivers/irqchip/irqchip.c
index 3570f0a588c4..8687131e7268 100644
--- a/drivers/irqchip/irqchip.c
+++ b/drivers/irqchip/irqchip.c
@@ -15,6 +15,11 @@
 #include <linux/irqchip.h>
 #include <linux/platform_device.h>
 
+struct platform_irqchip_instance {
+	of_irq_init_cb_t irq_init_cb;
+	of_irq_remove_cb_t irq_remove_cb;
+};
+
 /*
  * This special of_device_id is the sentinel at the end of the
  * of_device_id[] array of all irqchips. It is automatically placed at
@@ -34,11 +39,22 @@ void __init irqchip_init(void)
 
 int platform_irqchip_probe(struct platform_device *pdev)
 {
+	struct platform_irqchip_instance *irqchip;
+	const struct irqc_init_remove_funps *irqchip_funps;
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *par_np = of_irq_find_parent(np);
-	of_irq_init_cb_t irq_init_cb = of_device_get_match_data(&pdev->dev);
 
-	if (!irq_init_cb)
+	irqchip = devm_kzalloc(&pdev->dev, sizeof(*irqchip), GFP_KERNEL);
+	if (!irqchip)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, irqchip);
+
+	irqchip_funps = of_device_get_match_data(&pdev->dev);
+	irqchip->irq_init_cb =	irqchip_funps->irqchip_initp;
+	irqchip->irq_remove_cb = irqchip_funps->irqchip_removep;
+
+	if (!irqchip->irq_init_cb)
 		return -EINVAL;
 
 	if (par_np == np)
@@ -55,6 +71,22 @@ int platform_irqchip_probe(struct platform_device *pdev)
 	if (par_np && !irq_find_matching_host(par_np, DOMAIN_BUS_ANY))
 		return -EPROBE_DEFER;
 
-	return irq_init_cb(np, par_np);
+	return irqchip->irq_init_cb(np, par_np);
 }
 EXPORT_SYMBOL_GPL(platform_irqchip_probe);
+
+int platform_irqchip_remove(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *par_np = of_irq_find_parent(np);
+	struct platform_irqchip_instance *irqchip = platform_get_drvdata(pdev);
+
+	if (!irqchip->irq_remove_cb)
+		return -EINVAL;
+
+	if (par_np == np)
+		par_np = NULL;
+
+	return irqchip->irq_remove_cb(np, par_np);
+}
+EXPORT_SYMBOL_GPL(platform_irqchip_remove);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 252fab8074de..d46d28f56bc0 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1098,6 +1098,19 @@ struct irq_domain_chip_generic {
 	struct irq_chip_generic	*gc[];
 };
 
+/**
+ * struct irqc_init_remove_funps - Stores function pointers for irqc init
+ * and remove APIs. Used when the irqchip driver is to be used as a module.
+ * @irqchip_initp:	Function pointer for init/entry point of a irqchip driver.
+ * @irqchip_removep:Function pointer for irqchip driver remove function.
+ */
+struct irqc_init_remove_funps {
+	int (*irqchip_initp)(struct device_node *irqc,
+			     struct device_node *parent);
+	int (*irqchip_removep)(struct device_node *irqc,
+			       struct device_node *parent);
+};
+
 /* Generic chip callback functions */
 void irq_gc_noop(struct irq_data *d);
 void irq_gc_mask_disable_reg(struct irq_data *d);
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index aaf219bd0354..4210ac64956f 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -10,6 +10,7 @@
 #include <linux/of.h>
 
 typedef int (*of_irq_init_cb_t)(struct device_node *, struct device_node *);
+typedef int (*of_irq_remove_cb_t)(struct device_node *, struct device_node *);
 
 /*
  * Workarounds only applied to 32bit powermac machines
-- 
2.17.1

[PATCH 3/3] irqchip: xilinx: Add support to remove the Xilinx INTC driver module.

[Anirudha Sarangi]

The existing Xilinx INTC driver does not have mechanism to remove the
driver when it is being used as a module. For example, if the Xilinx
INTC IP is part of a removable fpga partition and a user intends to
replace the removable partition with a new one, the existing
implementation will not support the same.
The enhancement is done by adding a new remove API and making other
necessary changes.

Signed-off-by: Anirudha Sarangi <anirudha.sarangi@xilinx.com>
---
 drivers/irqchip/irq-xilinx-intc.c | 49 +++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 642733a6cbdf..2c5b76efabf6 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -39,6 +39,7 @@ struct xintc_irq_chip {
 	struct		irq_domain *root_domain;
 	u32		intr_mask;
 	u32		nr_irq;
+	int		irq;
 };
 
 static struct xintc_irq_chip *primary_intc;
@@ -234,6 +235,8 @@ static int xilinx_intc_of_init(struct device_node *intc,
 
 	if (parent) {
 		irq = irq_of_parse_and_map(intc, 0);
+		irqc->irq = irq;
+		intc->data = irqc;
 		if (irq) {
 			irq_set_chained_handler_and_data(irq,
 							 xil_intc_irq_handler,
@@ -257,5 +260,47 @@ static int xilinx_intc_of_init(struct device_node *intc,
 
 }
 
-IRQCHIP_DECLARE(xilinx_intc_xps, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
-IRQCHIP_DECLARE(xilinx_intc_opb, "xlnx,opb-intc-1.00.c", xilinx_intc_of_init);
+static int xilinx_intc_of_remove(struct device_node *intc,
+				 struct device_node *parent)
+{
+	int irq;
+	struct xintc_irq_chip *irqc;
+
+	if (!parent)
+		return 0;
+
+	irqc = intc->data;
+	irq = irqc->irq;
+	irq_set_chained_handler_and_data(irq, NULL, NULL);
+	if (irqc->domain) {
+		irq_dispose_mapping(irq);
+		irq_domain_remove(irqc->domain);
+	}
+	/*
+	 * Disable all external interrupts until they are
+	 * explicity requested.
+	 */
+	xintc_write(irqc, IER, 0);
+	/* Acknowledge any pending interrupts just in case. */
+	xintc_write(irqc, IAR, 0xffffffff);
+	/* Turn off the Master Enable. */
+	xintc_write(irqc, MER, 0x0);
+
+	iounmap(irqc->base);
+	kfree(irqc);
+
+	return 0;
+}
+
+static struct irqc_init_remove_funps intc_funps = {
+		.irqchip_initp = xilinx_intc_of_init,
+		.irqchip_removep = xilinx_intc_of_remove,
+};
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(xilinx_intc_xps)
+IRQCHIP_MATCH("xlnx,xps-intc-1.00.a", &intc_funps)
+IRQCHIP_PLATFORM_DRIVER_END(xilinx_intc_xps)
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(xilinx_intc_opb)
+IRQCHIP_MATCH("xlnx,opb-intc-1.00.c", &intc_funps)
+IRQCHIP_PLATFORM_DRIVER_END(xilinx_intc_opb)
-- 
2.17.1

Re: [PATCH 1/3] irqchip: xilinx: Avoid __init macro usage for xilinx_intc_of_init

[Marc Zyngier]

On Tue, 27 Apr 2021 12:31:34 +0100,
Anirudha Sarangi <anirudha.sarangi@xilinx.com> wrote:
> 
> This patch ensures that xilinx_intc_of_init API is not allocated into
> the .init.text section. Since this API calls the API set_handle_irq
> which uses __init macro to be put into .init.text section, this patch
> makes changes to ensure that set_handle_irq also does not stay in
> .init.text section.
> This patch is in preparation for the patch through which the xilinx
> intc driver will be loaded and unloaded as a module.
> 
> Signed-off-by: Anirudha Sarangi <anirudha.sarangi@xilinx.com>
> ---
>  drivers/irqchip/irq-xilinx-intc.c | 4 ++--
>  include/linux/irq.h               | 2 +-
>  kernel/irq/handle.c               | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 1d3d273309bd..642733a6cbdf 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -177,8 +177,8 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
>  	chained_irq_exit(chip, desc);
>  }
>  
> -static int __init xilinx_intc_of_init(struct device_node *intc,
> -					     struct device_node *parent)
> +static int xilinx_intc_of_init(struct device_node *intc,
> +			       struct device_node *parent)
>  {
>  	struct xintc_irq_chip *irqc;
>  	int ret, irq;
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 2efde6a79b7e..252fab8074de 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -1250,7 +1250,7 @@ int ipi_send_mask(unsigned int virq, const struct cpumask *dest);
>   * Returns 0 on success, or -EBUSY if an IRQ handler has already been
>   * registered.
>   */
> -int __init set_handle_irq(void (*handle_irq)(struct pt_regs *));
> +int set_handle_irq(void (*handle_irq)(struct pt_regs *));
>  
>  /*
>   * Allows interrupt handlers to find the irqchip that's been registered as the
> diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
> index 762a928e18f9..a0b18e8f5af0 100644
> --- a/kernel/irq/handle.c
> +++ b/kernel/irq/handle.c
> @@ -218,7 +218,7 @@ irqreturn_t handle_irq_event(struct irq_desc *desc)
>  }
>  
>  #ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
> -int __init set_handle_irq(void (*handle_irq)(struct pt_regs *))
> +int set_handle_irq(void (*handle_irq)(struct pt_regs *))
>  {
>  	if (handle_arch_irq)
>  		return -EBUSY;

No, never.

This function is for a root interrupt controller. You will never be
able to use this from a module. If you are calling this from something
that isn't marked __init, this is a terrible bug.

	M.

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

Re: [PATCH 2/3] irqchip: Add support to remove irqchip driver modules.

[Marc Zyngier]

On Tue, 27 Apr 2021 12:31:35 +0100,
Anirudha Sarangi <anirudha.sarangi@xilinx.com> wrote:
> 
> The existing irqchip implementation does not fully support use cases
> where an irqchip driver has to be used as a module. In particular there
> is no support to remove an irqchip driver module.
> The use cases where an irqchip driver has to be loaded and then removed
> as a module are really relevant in fpga world. A user can decide to
> have a irqchip as part of a removable partial fpga region. In such cases
> not only the corresponding irqchip driver has to be loaded as a module,
> but must also be removed when the removable partial region is removed.
> 
> The existing implementation updates the existing framework to achieve
> the above said goal.
> 
> Signed-off-by: Anirudha Sarangi <anirudha.sarangi@xilinx.com>

There is absolutely no way we can entertain the removal of an
interrupt controller based on *this*.

What happen to the irqdesc structures? What happen when a client
driver decides to do a disable_irq(), or any other interaction with
the interrupt controller that now has dangling pointers everywhere (if
your third patch is supposed to be an example of how to use this
functionality)?

So no, you can't do that until you figure out all the dependencies
that need to be accounted for to safely remove an interrupt
controller.

	M.

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

Re: [PATCH 2/3] irqchip: Add support to remove irqchip driver modules.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]

Hi Anirudha,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/irq/core]
[also build test WARNING on robh/for-next linus/master v5.12 next-20210427]
[cannot apply to xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anirudha-Sarangi/Updates-in-irqchip-framework-to-remove-irqchip/20210427-193255
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 765822e1569a37aab5e69736c52d4ad4a289eba6
config: mips-randconfig-r033-20210427 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d7308da4a5aaded897a7e0c06e7e88d81fc64879)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/70da936a840d5e21d42f09996e9fc8c56e6a44ce
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anirudha-Sarangi/Updates-in-irqchip-framework-to-remove-irqchip/20210427-193255
        git checkout 70da936a840d5e21d42f09996e9fc8c56e6a44ce
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/irqchip/irqchip.c:78:5: warning: no previous prototype for function 'platform_irqchip_remove' [-Wmissing-prototypes]
   int platform_irqchip_remove(struct platform_device *pdev)
       ^
   drivers/irqchip/irqchip.c:78:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int platform_irqchip_remove(struct platform_device *pdev)
   ^
   static 
   1 warning generated.


vim +/platform_irqchip_remove +78 drivers/irqchip/irqchip.c

    77	
  > 78	int platform_irqchip_remove(struct platform_device *pdev)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19852 bytes --]

Re: [PATCH 3/3] irqchip: xilinx: Add support to remove the Xilinx INTC driver module.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3148 bytes --]

Hi Anirudha,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/irq/core]
[also build test ERROR on robh/for-next linus/master v5.12 next-20210427]
[cannot apply to xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anirudha-Sarangi/Updates-in-irqchip-framework-to-remove-irqchip/20210427-193255
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 765822e1569a37aab5e69736c52d4ad4a289eba6
config: microblaze-randconfig-s031-20210426 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/5ca8cc6e787f51feed9a979547906dded222b51b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anirudha-Sarangi/Updates-in-irqchip-framework-to-remove-irqchip/20210427-193255
        git checkout 5ca8cc6e787f51feed9a979547906dded222b51b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=microblaze 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/irqchip/irq-xilinx-intc.c: In function 'xilinx_intc_of_remove':
>> drivers/irqchip/irq-xilinx-intc.c:275:10: error: 'struct xintc_irq_chip' has no member named 'domain'
     275 |  if (irqc->domain) {
         |          ^~
   drivers/irqchip/irq-xilinx-intc.c:277:25: error: 'struct xintc_irq_chip' has no member named 'domain'
     277 |   irq_domain_remove(irqc->domain);
         |                         ^~


vim +275 drivers/irqchip/irq-xilinx-intc.c

   262	
   263	static int xilinx_intc_of_remove(struct device_node *intc,
   264					 struct device_node *parent)
   265	{
   266		int irq;
   267		struct xintc_irq_chip *irqc;
   268	
   269		if (!parent)
   270			return 0;
   271	
   272		irqc = intc->data;
   273		irq = irqc->irq;
   274		irq_set_chained_handler_and_data(irq, NULL, NULL);
 > 275		if (irqc->domain) {
   276			irq_dispose_mapping(irq);
   277			irq_domain_remove(irqc->domain);
   278		}
   279		/*
   280		 * Disable all external interrupts until they are
   281		 * explicity requested.
   282		 */
   283		xintc_write(irqc, IER, 0);
   284		/* Acknowledge any pending interrupts just in case. */
   285		xintc_write(irqc, IAR, 0xffffffff);
   286		/* Turn off the Master Enable. */
   287		xintc_write(irqc, MER, 0x0);
   288	
   289		iounmap(irqc->base);
   290		kfree(irqc);
   291	
   292		return 0;
   293	}
   294	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24139 bytes --]