Hello!
gic_irq_domain_translate()'s GIC_IRQ_TYPE_PARTITION code knows about EPPI, and
gic_populate_ppi_partitions() sets them up, but gic_irq_domain_select() and
partition_domain_translate() didn't get the memo, meaning partitioned EPPI
don't work.
I'm not aware of a platform affected by this, so I don't think its stable
material.
Based on rc1, available here:
git://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git irqchip/ppi_partition/eppi_fixes/v1
Thanks,
James Morse (2):
irqchip/gic-v3: Add __gic_get_ppi_index() to find the PPI number from
hwirq
irqchip/gic-v3: Fix selection of partition domain for EPPIs
drivers/irqchip/irq-gic-v3.c | 61 +++++++++++++++++++++++++++++-------
1 file changed, 50 insertions(+), 11 deletions(-)
--
2.30.2
commit 5f51f803826e ("irqchip/gic-v3: Add EPPI range support") added
GIC_IRQ_TYPE_PARTITION support for EPPI to gic_irq_domain_translate(),
and commit 52085d3f2028 ("irqchip/gic-v3: Dynamically allocate PPI
partition descriptors") made the gic_data.ppi_descs array big enough for
EPPI, but neither gic_irq_domain_select() nor partition_domain_translate()
were updated.
This means partitions are created by partition_create_desc() for the
EPPI range, but can't be registered as they will always match the root
domain and map to the summary interrupt.
Update gic_irq_domain_select() to match PPI and EPPI. The fwspec for
PPI and EPPI both start from 0. Use gic_irq_domain_translate() to find
the hwirq from the fwspec, then convert this to a ppi index.
Reported-by: Valentin Schneider <[email protected]>
Signed-off-by: James Morse <[email protected]>
---
I'm not aware of a platform affected by this. If wanted as a fix, the
tag would be:
Fixes: 5f51f803826e ("irqchip/gic-v3: Add EPPI range support")
(and merge both patches)
---
drivers/irqchip/irq-gic-v3.c | 48 ++++++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index b24f0a9d2876..8b6e9b2fc621 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1472,10 +1472,34 @@ static void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
}
}
+static bool fwspec_is_partitioned_ppi(struct irq_fwspec *fwspec,
+ irq_hw_number_t hwirq)
+{
+ enum gic_intid_range range;
+
+ if (!gic_data.ppi_descs)
+ return false;
+
+ if (!is_of_node(fwspec->fwnode))
+ return false;
+
+ if (fwspec->param_count < 4 || !fwspec->param[3])
+ return false;
+
+ range = __get_intid_range(hwirq);
+ if (range != PPI_RANGE && range != EPPI_RANGE)
+ return false;
+
+ return true;
+}
+
static int gic_irq_domain_select(struct irq_domain *d,
struct irq_fwspec *fwspec,
enum irq_domain_bus_token bus_token)
{
+ unsigned int type, ret, ppi_idx;
+ irq_hw_number_t hwirq;
+
/* Not for us */
if (fwspec->fwnode != d->fwnode)
return 0;
@@ -1484,16 +1508,19 @@ static int gic_irq_domain_select(struct irq_domain *d,
if (!is_of_node(fwspec->fwnode))
return 1;
+ ret = gic_irq_domain_translate(d, fwspec, &hwirq, &type);
+ if (WARN_ON_ONCE(ret))
+ return 0;
+
+ if (!fwspec_is_partitioned_ppi(fwspec, hwirq))
+ return d == gic_data.domain;
+
/*
* If this is a PPI and we have a 4th (non-null) parameter,
* then we need to match the partition domain.
*/
- if (fwspec->param_count >= 4 &&
- fwspec->param[0] == 1 && fwspec->param[3] != 0 &&
- gic_data.ppi_descs)
- return d == partition_get_domain(gic_data.ppi_descs[fwspec->param[1]]);
-
- return d == gic_data.domain;
+ ppi_idx = __gic_get_ppi_index(hwirq);
+ return d == partition_get_domain(gic_data.ppi_descs[ppi_idx]);
}
static const struct irq_domain_ops gic_irq_domain_ops = {
@@ -1508,7 +1535,9 @@ static int partition_domain_translate(struct irq_domain *d,
unsigned long *hwirq,
unsigned int *type)
{
+ unsigned long ppi_intid;
struct device_node *np;
+ unsigned int ppi_idx;
int ret;
if (!gic_data.ppi_descs)
@@ -1518,7 +1547,12 @@ static int partition_domain_translate(struct irq_domain *d,
if (WARN_ON(!np))
return -EINVAL;
- ret = partition_translate_id(gic_data.ppi_descs[fwspec->param[1]],
+ ret = gic_irq_domain_translate(d, fwspec, &ppi_intid, type);
+ if (WARN_ON_ONCE(ret))
+ return 0;
+
+ ppi_idx = __gic_get_ppi_index(ppi_intid);
+ ret = partition_translate_id(gic_data.ppi_descs[ppi_idx],
of_node_to_fwnode(np));
if (ret < 0)
return ret;
--
2.30.2
gic_get_ppi_index() is a useful concept for ppi partitions, as the GIC
has two PPI ranges but needs mapping to a single range when used as an
index in the gic_data.ppi_descs[] array.
Add a double-underscore version which takes just the intid. This will
be used in the partition domain select and translate helpers to enable
partition support for the EPPI range.
Signed-off-by: James Morse <[email protected]>
---
drivers/irqchip/irq-gic-v3.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e0f4debe64e1..b24f0a9d2876 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -446,18 +446,23 @@ static void gic_irq_set_prio(struct irq_data *d, u8 prio)
writeb_relaxed(prio, base + offset + index);
}
-static u32 gic_get_ppi_index(struct irq_data *d)
+static u32 __gic_get_ppi_index(irq_hw_number_t hwirq)
{
- switch (get_intid_range(d)) {
+ switch (__get_intid_range(hwirq)) {
case PPI_RANGE:
- return d->hwirq - 16;
+ return hwirq - 16;
case EPPI_RANGE:
- return d->hwirq - EPPI_BASE_INTID + 16;
+ return hwirq - EPPI_BASE_INTID + 16;
default:
unreachable();
}
}
+static u32 gic_get_ppi_index(struct irq_data *d)
+{
+ return __gic_get_ppi_index(d->hwirq);
+}
+
static int gic_irq_nmi_setup(struct irq_data *d)
{
struct irq_desc *desc = irq_to_desc(d->irq);
--
2.30.2
On 29/07/21 17:27, James Morse wrote:
> gic_get_ppi_index() is a useful concept for ppi partitions, as the GIC
> has two PPI ranges but needs mapping to a single range when used as an
> index in the gic_data.ppi_descs[] array.
>
> Add a double-underscore version which takes just the intid. This will
> be used in the partition domain select and translate helpers to enable
> partition support for the EPPI range.
>
> Signed-off-by: James Morse <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
On 29/07/21 17:27, James Morse wrote:
> commit 5f51f803826e ("irqchip/gic-v3: Add EPPI range support") added
> GIC_IRQ_TYPE_PARTITION support for EPPI to gic_irq_domain_translate(),
> and commit 52085d3f2028 ("irqchip/gic-v3: Dynamically allocate PPI
> partition descriptors") made the gic_data.ppi_descs array big enough for
> EPPI, but neither gic_irq_domain_select() nor partition_domain_translate()
> were updated.
>
> This means partitions are created by partition_create_desc() for the
> EPPI range, but can't be registered as they will always match the root
> domain and map to the summary interrupt.
>
> Update gic_irq_domain_select() to match PPI and EPPI. The fwspec for
> PPI and EPPI both start from 0. Use gic_irq_domain_translate() to find
> the hwirq from the fwspec, then convert this to a ppi index.
>
> Reported-by: Valentin Schneider <[email protected]>
> Signed-off-by: James Morse <[email protected]>
Tiny nit below, regardless:
Reviewed-by: Valentin Schneider <[email protected]>
> @@ -1518,7 +1547,12 @@ static int partition_domain_translate(struct irq_domain *d,
> if (WARN_ON(!np))
> return -EINVAL;
>
> - ret = partition_translate_id(gic_data.ppi_descs[fwspec->param[1]],
> + ret = gic_irq_domain_translate(d, fwspec, &ppi_intid, type);
This assigns @type for us, so the @type assignment at the tail of
partition_domain_translate() becomes redundant.
> + if (WARN_ON_ONCE(ret))
> + return 0;
> +
> + ppi_idx = __gic_get_ppi_index(ppi_intid);
> + ret = partition_translate_id(gic_data.ppi_descs[ppi_idx],
> of_node_to_fwnode(np));
> if (ret < 0)
> return ret;
> --
> 2.30.2
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: d753f849bf487faffd05898e6a8e5aa9d146cb50
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d753f849bf487faffd05898e6a8e5aa9d146cb50
Author: James Morse <[email protected]>
AuthorDate: Thu, 29 Jul 2021 17:27:48
Committer: Marc Zyngier <[email protected]>
CommitterDate: Thu, 12 Aug 2021 08:11:03 +01:00
irqchip/gic-v3: Fix selection of partition domain for EPPIs
commit 5f51f803826e ("irqchip/gic-v3: Add EPPI range support") added
GIC_IRQ_TYPE_PARTITION support for EPPI to gic_irq_domain_translate(),
and commit 52085d3f2028 ("irqchip/gic-v3: Dynamically allocate PPI
partition descriptors") made the gic_data.ppi_descs array big enough for
EPPI, but neither gic_irq_domain_select() nor partition_domain_translate()
were updated.
This means partitions are created by partition_create_desc() for the
EPPI range, but can't be registered as they will always match the root
domain and map to the summary interrupt.
Update gic_irq_domain_select() to match PPI and EPPI. The fwspec for
PPI and EPPI both start from 0. Use gic_irq_domain_translate() to find
the hwirq from the fwspec, then convert this to a ppi index.
Reported-by: Valentin Schneider <[email protected]>
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-gic-v3.c | 48 +++++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index b24f0a9..8b6e9b2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1472,10 +1472,34 @@ static void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
}
}
+static bool fwspec_is_partitioned_ppi(struct irq_fwspec *fwspec,
+ irq_hw_number_t hwirq)
+{
+ enum gic_intid_range range;
+
+ if (!gic_data.ppi_descs)
+ return false;
+
+ if (!is_of_node(fwspec->fwnode))
+ return false;
+
+ if (fwspec->param_count < 4 || !fwspec->param[3])
+ return false;
+
+ range = __get_intid_range(hwirq);
+ if (range != PPI_RANGE && range != EPPI_RANGE)
+ return false;
+
+ return true;
+}
+
static int gic_irq_domain_select(struct irq_domain *d,
struct irq_fwspec *fwspec,
enum irq_domain_bus_token bus_token)
{
+ unsigned int type, ret, ppi_idx;
+ irq_hw_number_t hwirq;
+
/* Not for us */
if (fwspec->fwnode != d->fwnode)
return 0;
@@ -1484,16 +1508,19 @@ static int gic_irq_domain_select(struct irq_domain *d,
if (!is_of_node(fwspec->fwnode))
return 1;
+ ret = gic_irq_domain_translate(d, fwspec, &hwirq, &type);
+ if (WARN_ON_ONCE(ret))
+ return 0;
+
+ if (!fwspec_is_partitioned_ppi(fwspec, hwirq))
+ return d == gic_data.domain;
+
/*
* If this is a PPI and we have a 4th (non-null) parameter,
* then we need to match the partition domain.
*/
- if (fwspec->param_count >= 4 &&
- fwspec->param[0] == 1 && fwspec->param[3] != 0 &&
- gic_data.ppi_descs)
- return d == partition_get_domain(gic_data.ppi_descs[fwspec->param[1]]);
-
- return d == gic_data.domain;
+ ppi_idx = __gic_get_ppi_index(hwirq);
+ return d == partition_get_domain(gic_data.ppi_descs[ppi_idx]);
}
static const struct irq_domain_ops gic_irq_domain_ops = {
@@ -1508,7 +1535,9 @@ static int partition_domain_translate(struct irq_domain *d,
unsigned long *hwirq,
unsigned int *type)
{
+ unsigned long ppi_intid;
struct device_node *np;
+ unsigned int ppi_idx;
int ret;
if (!gic_data.ppi_descs)
@@ -1518,7 +1547,12 @@ static int partition_domain_translate(struct irq_domain *d,
if (WARN_ON(!np))
return -EINVAL;
- ret = partition_translate_id(gic_data.ppi_descs[fwspec->param[1]],
+ ret = gic_irq_domain_translate(d, fwspec, &ppi_intid, type);
+ if (WARN_ON_ONCE(ret))
+ return 0;
+
+ ppi_idx = __gic_get_ppi_index(ppi_intid);
+ ret = partition_translate_id(gic_data.ppi_descs[ppi_idx],
of_node_to_fwnode(np));
if (ret < 0)
return ret;
The following commit has been merged into the irq/irqchip-next branch of irqchip:
Commit-ID: bfa80ee9ce6e2f18da76459c3dd7b0ad57fb2c20
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/bfa80ee9ce6e2f18da76459c3dd7b0ad57fb2c20
Author: James Morse <[email protected]>
AuthorDate: Thu, 29 Jul 2021 17:27:47
Committer: Marc Zyngier <[email protected]>
CommitterDate: Thu, 12 Aug 2021 08:11:03 +01:00
irqchip/gic-v3: Add __gic_get_ppi_index() to find the PPI number from hwirq
gic_get_ppi_index() is a useful concept for ppi partitions, as the GIC
has two PPI ranges but needs mapping to a single range when used as an
index in the gic_data.ppi_descs[] array.
Add a double-underscore version which takes just the intid. This will
be used in the partition domain select and translate helpers to enable
partition support for the EPPI range.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-gic-v3.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e0f4deb..b24f0a9 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -446,18 +446,23 @@ static void gic_irq_set_prio(struct irq_data *d, u8 prio)
writeb_relaxed(prio, base + offset + index);
}
-static u32 gic_get_ppi_index(struct irq_data *d)
+static u32 __gic_get_ppi_index(irq_hw_number_t hwirq)
{
- switch (get_intid_range(d)) {
+ switch (__get_intid_range(hwirq)) {
case PPI_RANGE:
- return d->hwirq - 16;
+ return hwirq - 16;
case EPPI_RANGE:
- return d->hwirq - EPPI_BASE_INTID + 16;
+ return hwirq - EPPI_BASE_INTID + 16;
default:
unreachable();
}
}
+static u32 gic_get_ppi_index(struct irq_data *d)
+{
+ return __gic_get_ppi_index(d->hwirq);
+}
+
static int gic_irq_nmi_setup(struct irq_data *d)
{
struct irq_desc *desc = irq_to_desc(d->irq);