[PATCH 0/8] irqdomain/debugfs: Fix uses of irq_domain_alloc_fwnode

[PATCH 0/8] irqdomain/debugfs: Fix uses of irq_domain_alloc_fwnode

[Marc Zyngier]

I recently noticed that all irq_domain_alloc_fwnode were passing a VA
to it, which is unfortunate as this is designed to appear in debugfs
(and we don't like to leak VAs). Disaster was avoided thanks to our
____ptrval____ friend, but it remains that the whole thing is pretty
useless if you have more than a single domain (they all have the same
name and creation fails).

In order to sort it out, change all users of irq_domain_alloc_fwnode
to pass the PA of the irqchip the domain will be associated with. One
notable exception is the HyperV PCI controller driver which has no PA
to associate with. This is solved by using a named fwnode instead,
using the device GUID.

Finally, irq_domain_alloc_fwnode() is changed to pa a pionter to a PA,
which can be safely advertised in debugfs.

Marc Zyngier (8):
  irqchip/gic-v3: Register the distributor's PA instead of its VA in
    fwnode
  irqchip/gic-v3-its: Register the ITS' PA instead of its VA in fwnode
  irqchip/gic: Register the distributor's PA instead of its VA in fwnode
  irqchip/gic-v2m: Register the frame's PA instead of its VA in fwnode
  irqchip/ixp4xx: Register the base PA instead of its VA in fwnode
  gpio/ixp4xx: Register the base PA instead of its VA in fwnode
  PCI: hv: Allocate a named fwnode instead of an address-based one
  irqdomain/debugfs: Use PAs to generate fwnode names

 drivers/gpio/gpio-ixp4xx.c          |  2 +-
 drivers/irqchip/irq-gic-v2m.c       |  2 +-
 drivers/irqchip/irq-gic-v3-its.c    |  2 +-
 drivers/irqchip/irq-gic-v3.c        |  2 +-
 drivers/irqchip/irq-gic.c           |  2 +-
 drivers/irqchip/irq-ixp4xx.c        |  2 +-
 drivers/pci/controller/pci-hyperv.c | 10 +++++++++-
 include/linux/irqdomain.h           |  6 +++---
 kernel/irq/irqdomain.c              |  9 +++++----
 9 files changed, 23 insertions(+), 14 deletions(-)

-- 
2.20.1

[PATCH 1/8] irqchip/gic-v3: Register the distributor's PA instead of its VA in fwnode

[Marc Zyngier]

Do not expose the distributor's VA (it appears in debugfs). Instead,
record the PA, which at least can be used to precisely identify
the associated irqchip and domain.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 96d927f0f91a..be961c261093 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1845,7 +1845,7 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
 	if (err)
 		goto out_redist_unmap;
 
-	domain_handle = irq_domain_alloc_fwnode(acpi_data.dist_base);
+	domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
 	if (!domain_handle) {
 		err = -ENOMEM;
 		goto out_redist_unmap;
-- 
2.20.1

[PATCH 2/8] irqchip/gic-v3-its: Register the ITS' PA instead of its VA in fwnode

[Marc Zyngier]

Do not expose the ITS' VA (it appears in debugfs). Instead, record
the PA, which at least can be used to precisely identify the associated
irqchip and domain.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b5c3672aea2..8eeb0e284896 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3921,7 +3921,7 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
 	res.end = its_entry->base_address + ACPI_GICV3_ITS_MEM_SIZE - 1;
 	res.flags = IORESOURCE_MEM;
 
-	dom_handle = irq_domain_alloc_fwnode((void *)its_entry->base_address);
+	dom_handle = irq_domain_alloc_fwnode(&res.start);
 	if (!dom_handle) {
 		pr_err("ITS@%pa: Unable to allocate GICv3 ITS domain token\n",
 		       &res.start);
-- 
2.20.1

[PATCH 3/8] irqchip/gic: Register the distributor's PA instead of its VA in fwnode

[Marc Zyngier]

Do not expose the distributor's VA (it appears in debugfs). Instead,
record the PA, which at least can be used to precisely identify
the associated irqchip and domain.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index e45f45e68720..b6b857377763 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1627,7 +1627,7 @@ static int __init gic_v2_acpi_init(struct acpi_subtable_header *header,
 	/*
 	 * Initialize GIC instance zero (no multi-GIC support).
 	 */
-	domain_handle = irq_domain_alloc_fwnode(gic->raw_dist_base);
+	domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
 	if (!domain_handle) {
 		pr_err("Unable to allocate domain handle\n");
 		gic_teardown(gic);
-- 
2.20.1

[PATCH 4/8] irqchip/gic-v2m: Register the frame's PA instead of its VA in fwnode

[Marc Zyngier]

Do not expose the frame's VA (it appears in debugfs). Instead,
record the PA, which at least can be used to precisely identify
the associated irqchip and domain.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 7338f90b2f9e..e88e75c22b6a 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -525,7 +525,7 @@ acpi_parse_madt_msi(union acpi_subtable_headers *header,
 			spi_start, nr_spis);
 	}
 
-	fwnode = irq_domain_alloc_fwnode((void *)m->base_address);
+	fwnode = irq_domain_alloc_fwnode(&res.start);
 	if (!fwnode) {
 		pr_err("Unable to allocate GICv2m domain token\n");
 		return -EINVAL;
-- 
2.20.1

[PATCH 5/8] irqchip/ixp4xx: Register the base PA instead of its VA in fwnode

[Marc Zyngier]

Do not expose the base VA (it appears in debugfs). Instead,
record the PA, which at least can be used to precisely identify
the associated irqchip and domain.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-ixp4xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-ixp4xx.c b/drivers/irqchip/irq-ixp4xx.c
index 6751c35b7e1d..37e0749215c7 100644
--- a/drivers/irqchip/irq-ixp4xx.c
+++ b/drivers/irqchip/irq-ixp4xx.c
@@ -319,7 +319,7 @@ void __init ixp4xx_irq_init(resource_size_t irqbase,
 		pr_crit("IXP4XX: could not ioremap interrupt controller\n");
 		return;
 	}
-	fwnode = irq_domain_alloc_fwnode(base);
+	fwnode = irq_domain_alloc_fwnode(&irqbase);
 	if (!fwnode) {
 		pr_crit("IXP4XX: no domain handle\n");
 		return;
-- 
2.20.1

[PATCH 6/8] gpio/ixp4xx: Register the base PA instead of its VA in fwnode

[Marc Zyngier]

Do not expose the base VA (it appears in debugfs). Instead,
record the PA, which at least can be used to precisely identify
the associated irqchip and domain.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/gpio/gpio-ixp4xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-ixp4xx.c b/drivers/gpio/gpio-ixp4xx.c
index 670c2a85a35b..cc72c9aca5a1 100644
--- a/drivers/gpio/gpio-ixp4xx.c
+++ b/drivers/gpio/gpio-ixp4xx.c
@@ -400,7 +400,7 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
 		g->fwnode = of_node_to_fwnode(np);
 	} else {
 		parent = ixp4xx_get_irq_domain();
-		g->fwnode = irq_domain_alloc_fwnode(g->base);
+		g->fwnode = irq_domain_alloc_fwnode(&res->start);
 		if (!g->fwnode) {
 			dev_err(dev, "no domain base\n");
 			return -ENODEV;
-- 
2.20.1

[PATCH 7/8] PCI: hv: Allocate a named fwnode instead of an address-based one

[Marc Zyngier]

To allocate its fwnode that is then used to allocate an irqdomain,
the driver uses irq_domain_alloc_fwnode(), passing it a VA as an
identifier. This is a rather bad idea, as this address ends up
published in debugfs (and we want to move away from VAs there
anyway).

Instead, let's allocate a named fwnode by using the device GUID as
an identifier. It is allegedly unique, and can be traced back to
the original device.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/pci/controller/pci-hyperv.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 40b625458afa..f6ed2583167a 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2521,6 +2521,7 @@ static int hv_pci_probe(struct hv_device *hdev,
 			const struct hv_vmbus_device_id *dev_id)
 {
 	struct hv_pcibus_device *hbus;
+	char *name;
 	int ret;
 
 	/*
@@ -2589,7 +2590,14 @@ static int hv_pci_probe(struct hv_device *hdev,
 		goto free_config;
 	}
 
-	hbus->sysdata.fwnode = irq_domain_alloc_fwnode(hbus);
+	name = kasprintf("%pUL", &hdev->dev_instance);
+	if (!name) {
+		ret = -ENOMEM;
+		goto unmap;
+	}
+
+	hbus->sysdata.fwnode = irq_domain_alloc_named_fwnode(name);
+	kfree(name);
 	if (!hbus->sysdata.fwnode) {
 		ret = -ENOMEM;
 		goto unmap;
-- 
2.20.1

[PATCH 8/8] irqdomain/debugfs: Use PAs to generate fwnode names

[Marc Zyngier]

Booting a large arm64 server (HiSi D05) leads to the following
shouting at boot time:

[   20.722132] debugfs: File 'irqchip@(____ptrval____)-3' in directory 'domains' already present!
[   20.730851] debugfs: File 'irqchip@(____ptrval____)-3' in directory 'domains' already present!
[   20.739560] debugfs: File 'irqchip@(____ptrval____)-3' in directory 'domains' already present!
[   20.748267] debugfs: File 'irqchip@(____ptrval____)-3' in directory 'domains' already present!
[   20.756975] debugfs: File 'irqchip@(____ptrval____)-3' in directory 'domains' already present!
[   20.765683] debugfs: File 'irqchip@(____ptrval____)-3' in directory 'domains' already present!
[   20.774391] debugfs: File 'irqchip@(____ptrval____)-3' in directory 'domains' already present!

and many more... Evidently, we expect something a bit more informative
than ____ptrval____, and certainly we want all of our domains, not just
the first one.

For that, turn the %p used to generate the fwnode name into something
that won't be repainted (%pa). Given that we've now fixed all users to
pass a pointer to a PA, it will actually do the right thing.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/irqdomain.h | 6 +++---
 kernel/irq/irqdomain.c    | 9 +++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 07ec8b390161..583e7abd07f9 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -220,7 +220,7 @@ static inline struct device_node *irq_domain_get_of_node(struct irq_domain *d)
 
 #ifdef CONFIG_IRQ_DOMAIN
 struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id,
-						const char *name, void *data);
+						const char *name, phys_addr_t *pa);
 
 enum {
 	IRQCHIP_FWNODE_REAL,
@@ -241,9 +241,9 @@ struct fwnode_handle *irq_domain_alloc_named_id_fwnode(const char *name, int id)
 					 NULL);
 }
 
-static inline struct fwnode_handle *irq_domain_alloc_fwnode(void *data)
+static inline struct fwnode_handle *irq_domain_alloc_fwnode(phys_addr_t *pa)
 {
-	return __irq_domain_alloc_fwnode(IRQCHIP_FWNODE_REAL, 0, NULL, data);
+	return __irq_domain_alloc_fwnode(IRQCHIP_FWNODE_REAL, 0, NULL, pa);
 }
 
 void irq_domain_free_fwnode(struct fwnode_handle *fwnode);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 3078d0e48bba..e7bbab149750 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -31,7 +31,7 @@ struct irqchip_fwid {
 	struct fwnode_handle	fwnode;
 	unsigned int		type;
 	char			*name;
-	void *data;
+	phys_addr_t		*pa;
 };
 
 #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
@@ -62,7 +62,8 @@ EXPORT_SYMBOL_GPL(irqchip_fwnode_ops);
  * domain struct.
  */
 struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id,
-						const char *name, void *data)
+						const char *name,
+						phys_addr_t *pa)
 {
 	struct irqchip_fwid *fwid;
 	char *n;
@@ -77,7 +78,7 @@ struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id,
 		n = kasprintf(GFP_KERNEL, "%s-%d", name, id);
 		break;
 	default:
-		n = kasprintf(GFP_KERNEL, "irqchip@%p", data);
+		n = kasprintf(GFP_KERNEL, "irqchip@%pa", pa);
 		break;
 	}
 
@@ -89,7 +90,7 @@ struct fwnode_handle *__irq_domain_alloc_fwnode(unsigned int type, int id,
 
 	fwid->type = type;
 	fwid->name = n;
-	fwid->data = data;
+	fwid->pa = pa;
 	fwid->fwnode.ops = &irqchip_fwnode_ops;
 	return &fwid->fwnode;
 }
-- 
2.20.1

Re: [PATCH 6/8] gpio/ixp4xx: Register the base PA instead of its VA in fwnode

[Linus Walleij]

On Tue, Aug 6, 2019 at 4:57 PM Marc Zyngier <maz@kernel.org> wrote:

> Do not expose the base VA (it appears in debugfs). Instead,
> record the PA, which at least can be used to precisely identify
> the associated irqchip and domain.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Please apply this directly to the irq subsystem tree with
the rest of the fixes.

Yours,
Linus Walleij

Re: [PATCH 7/8] PCI: hv: Allocate a named fwnode instead of an address-based one

[Marc Zyngier]

On 06/08/2019 15:57, Marc Zyngier wrote:
> To allocate its fwnode that is then used to allocate an irqdomain,
> the driver uses irq_domain_alloc_fwnode(), passing it a VA as an
> identifier. This is a rather bad idea, as this address ends up
> published in debugfs (and we want to move away from VAs there
> anyway).
> 
> Instead, let's allocate a named fwnode by using the device GUID as
> an identifier. It is allegedly unique, and can be traced back to
> the original device.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/pci/controller/pci-hyperv.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 40b625458afa..f6ed2583167a 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2521,6 +2521,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>  			const struct hv_vmbus_device_id *dev_id)
>  {
>  	struct hv_pcibus_device *hbus;
> +	char *name;
>  	int ret;
>  
>  	/*
> @@ -2589,7 +2590,14 @@ static int hv_pci_probe(struct hv_device *hdev,
>  		goto free_config;
>  	}
>  
> -	hbus->sysdata.fwnode = irq_domain_alloc_fwnode(hbus);
> +	name = kasprintf("%pUL", &hdev->dev_instance);

Of course, this is bogus. It needs a GFP_KERNEL as the first parameter.

/me adds HYPERV to .config, and fixes it locally.

	M.
-- 
Jazz is not dead, it just smells funny...

Re: [PATCH 0/8] irqdomain/debugfs: Fix uses of irq_domain_alloc_fwnode

[Thomas Gleixner]

On Tue, 6 Aug 2019, Marc Zyngier wrote:

> I recently noticed that all irq_domain_alloc_fwnode were passing a VA
> to it, which is unfortunate as this is designed to appear in debugfs
> (and we don't like to leak VAs). Disaster was avoided thanks to our
> ____ptrval____ friend, but it remains that the whole thing is pretty
> useless if you have more than a single domain (they all have the same
> name and creation fails).
> 
> In order to sort it out, change all users of irq_domain_alloc_fwnode
> to pass the PA of the irqchip the domain will be associated with. One
> notable exception is the HyperV PCI controller driver which has no PA
> to associate with. This is solved by using a named fwnode instead,
> using the device GUID.
> 
> Finally, irq_domain_alloc_fwnode() is changed to pa a pionter to a PA,
> which can be safely advertised in debugfs.

Acked-by: Thomas Gleixner <tglx@linutronix.de>