The GIC architecture specification defines a set of registers
for redistributors and ITSes that control the sharebility and
cacheability attributes of redistributors/ITSes initiator ports
on the interconnect (GICR_[V]PROPBASER, GICR_[V]PENDBASER,
GITS_BASER<n>).
Architecturally the GIC provides a means to drive shareability
and cacheability attributes signals and related IWB/OWB/ISH barriers
but it is not mandatory for designs to wire up the corresponding
interconnect signals that control the cacheability/shareability
of transactions.
Redistributors and ITSes interconnect ports can be connected to
non-coherent interconnects that are not able to manage the
shareability/cacheability attributes; this implicitly makes
the redistributors and ITSes non-coherent observers.
So far, the GIC driver on probe executes a write to "probe" for
the redistributors and ITSes registers shareability bitfields
by writing a value (ie InnerShareable - the shareability domain the
CPUs are in) and check it back to detect whether the value sticks or
not; this hinges on a GIC programming model behaviour that predates the
current specifications, that just define shareability bits as writeable
but do not guarantee that writing certain shareability values
enable the expected behaviour for the redistributors/ITSes
memory interconnect ports.
To enable non-coherent GIC designs, introduce the "dma-noncoherent"
device tree property to allow firmware to describe redistributors and
ITSes as non-coherent observers on the memory interconnect and use the
property to force the shareability attributes to be programmed into the
redistributors and ITSes registers.
Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e0c2b10d154d..758ea3092305 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5056,7 +5056,8 @@ static int __init its_compute_its_list_map(struct resource *res,
}
static int __init its_probe_one(struct resource *res,
- struct fwnode_handle *handle, int numa_node)
+ struct fwnode_handle *handle, int numa_node,
+ bool non_coherent)
{
struct its_node *its;
void __iomem *its_base;
@@ -5148,7 +5149,7 @@ static int __init its_probe_one(struct resource *res,
gits_write_cbaser(baser, its->base + GITS_CBASER);
tmp = gits_read_cbaser(its->base + GITS_CBASER);
- if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE)
+ if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE || non_coherent)
tmp &= ~GITS_CBASER_SHAREABILITY_MASK;
if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
@@ -5356,11 +5357,19 @@ static const struct of_device_id its_device_id[] = {
{},
};
+static void of_check_rdists_coherent(struct device_node *node)
+{
+ if (of_property_read_bool(node, "dma-noncoherent"))
+ gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
+}
+
static int __init its_of_probe(struct device_node *node)
{
struct device_node *np;
struct resource res;
+ of_check_rdists_coherent(node);
+
/*
* Make sure *all* the ITS are reset before we probe any, as
* they may be sharing memory. If any of the ITS fails to
@@ -5396,7 +5405,8 @@ static int __init its_of_probe(struct device_node *node)
continue;
}
- its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
+ its_probe_one(&res, &np->fwnode, of_node_to_nid(np),
+ of_property_read_bool(np, "dma-noncoherent"));
}
return 0;
}
@@ -5533,7 +5543,8 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
}
err = its_probe_one(&res, dom_handle,
- acpi_get_its_numa_node(its_entry->translation_id));
+ acpi_get_its_numa_node(its_entry->translation_id),
+ false);
if (!err)
return 0;
--
2.34.1
On Tue, 05 Sep 2023 15:24:51 +0100,
Lorenzo Pieralisi <[email protected]> wrote:
>
> On Tue, Sep 05, 2023 at 12:34:58PM +0100, Marc Zyngier wrote:
> > @@ -4808,6 +4812,11 @@ static const struct gic_quirk its_quirks[] = {
> > .init = its_enable_rk3588001,
> > },
> > #endif
> > + {
> > + .desc = "ITS: non-coherent attribute",
> > + .property = "dma-noncoherent",
> > + .init = its_set_non_coherent,
> > + },
>
> For the records, that requires adding a gic_enable_of_quirks() call for the
> ITS DT node that at the moment is not needed, something like this:
>
> -- >8 --
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 25a12b46ce35..3ae3cb9aadd9 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -4826,6 +4826,10 @@ static void its_enable_quirks(struct its_node *its)
> u32 iidr = readl_relaxed(its->base + GITS_IIDR);
>
> gic_enable_quirks(iidr, its_quirks, its);
> +
> + if (is_of_node(its->fwnode_handle))
> + gic_enable_of_quirks(to_of_node(its->fwnode_handle),
> + its_quirks, its);
> }
>
Ah, well spotted. Yes, please fold this in.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
On Tue, 05 Sep 2023 11:47:21 +0100,
Lorenzo Pieralisi <[email protected]> wrote:
>
> The GIC architecture specification defines a set of registers
> for redistributors and ITSes that control the sharebility and
> cacheability attributes of redistributors/ITSes initiator ports
> on the interconnect (GICR_[V]PROPBASER, GICR_[V]PENDBASER,
> GITS_BASER<n>).
>
> Architecturally the GIC provides a means to drive shareability
> and cacheability attributes signals and related IWB/OWB/ISH barriers
> but it is not mandatory for designs to wire up the corresponding
> interconnect signals that control the cacheability/shareability
> of transactions.
>
> Redistributors and ITSes interconnect ports can be connected to
> non-coherent interconnects that are not able to manage the
> shareability/cacheability attributes; this implicitly makes
> the redistributors and ITSes non-coherent observers.
>
> So far, the GIC driver on probe executes a write to "probe" for
> the redistributors and ITSes registers shareability bitfields
> by writing a value (ie InnerShareable - the shareability domain the
> CPUs are in) and check it back to detect whether the value sticks or
> not; this hinges on a GIC programming model behaviour that predates the
> current specifications, that just define shareability bits as writeable
> but do not guarantee that writing certain shareability values
> enable the expected behaviour for the redistributors/ITSes
> memory interconnect ports.
>
> To enable non-coherent GIC designs, introduce the "dma-noncoherent"
> device tree property to allow firmware to describe redistributors and
> ITSes as non-coherent observers on the memory interconnect and use the
> property to force the shareability attributes to be programmed into the
> redistributors and ITSes registers.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index e0c2b10d154d..758ea3092305 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -5056,7 +5056,8 @@ static int __init its_compute_its_list_map(struct resource *res,
> }
>
> static int __init its_probe_one(struct resource *res,
> - struct fwnode_handle *handle, int numa_node)
> + struct fwnode_handle *handle, int numa_node,
> + bool non_coherent)
> {
> struct its_node *its;
> void __iomem *its_base;
> @@ -5148,7 +5149,7 @@ static int __init its_probe_one(struct resource *res,
> gits_write_cbaser(baser, its->base + GITS_CBASER);
> tmp = gits_read_cbaser(its->base + GITS_CBASER);
>
> - if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE)
> + if (its->flags & ITS_FLAGS_FORCE_NON_SHAREABLE || non_coherent)
> tmp &= ~GITS_CBASER_SHAREABILITY_MASK;
Please use the non_coherent attribute to set the flag, instead of
using it as some sideband signalling. Not having this information
stored in the its_node structure makes it harder to debug.
We have an over-engineered quirk framework, and it should be put to a
good use.
>
> if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
> @@ -5356,11 +5357,19 @@ static const struct of_device_id its_device_id[] = {
> {},
> };
>
> +static void of_check_rdists_coherent(struct device_node *node)
> +{
> + if (of_property_read_bool(node, "dma-noncoherent"))
> + gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
> +}
> +
> static int __init its_of_probe(struct device_node *node)
> {
> struct device_node *np;
> struct resource res;
>
> + of_check_rdists_coherent(node);
It really feels that the flag should instead be communicated by the
base GIC driver, as it readily communicates the whole rdists structure
already.
> +
> /*
> * Make sure *all* the ITS are reset before we probe any, as
> * they may be sharing memory. If any of the ITS fails to
> @@ -5396,7 +5405,8 @@ static int __init its_of_probe(struct device_node *node)
> continue;
> }
>
> - its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
> + its_probe_one(&res, &np->fwnode, of_node_to_nid(np),
> + of_property_read_bool(np, "dma-noncoherent"));
> }
> return 0;
> }
> @@ -5533,7 +5543,8 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
> }
>
> err = its_probe_one(&res, dom_handle,
> - acpi_get_its_numa_node(its_entry->translation_id));
> + acpi_get_its_numa_node(its_entry->translation_id),
> + false);
I came up with the following alternative approach, which is as usual
completely untested. It is entirely based on the quirk infrastructure,
and doesn't touch the ACPI path at all.
Thanks,
M.
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index 3db4592cda1c..00641e88aa38 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -29,4 +29,8 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
void gic_enable_of_quirks(const struct device_node *np,
const struct gic_quirk *quirks, void *data);
+#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
+#define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
+#define RDIST_FLAGS_FORCE_NON_SHAREABLE (1 << 2)
+
#endif /* _IRQ_GIC_COMMON_H */
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e0c2b10d154d..6edf59af757b 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -44,10 +44,6 @@
#define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
#define ITS_FLAGS_FORCE_NON_SHAREABLE (1ULL << 3)
-#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
-#define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
-#define RDIST_FLAGS_FORCE_NON_SHAREABLE (1 << 2)
-
#define RD_LOCAL_LPI_ENABLED BIT(0)
#define RD_LOCAL_PENDTABLE_PREALLOCATED BIT(1)
#define RD_LOCAL_MEMRESERVE_DONE BIT(2)
@@ -4754,6 +4750,14 @@ static bool __maybe_unused its_enable_rk3588001(void *data)
return true;
}
+static bool its_set_non_coherent(void *data)
+{
+ struct its_node *its = data;
+
+ its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
+ return true;
+}
+
static const struct gic_quirk its_quirks[] = {
#ifdef CONFIG_CAVIUM_ERRATUM_22375
{
@@ -4808,6 +4812,11 @@ static const struct gic_quirk its_quirks[] = {
.init = its_enable_rk3588001,
},
#endif
+ {
+ .desc = "ITS: non-coherent attribute",
+ .property = "dma-noncoherent",
+ .init = its_set_non_coherent,
+ },
{
}
};
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index eedfa8e9f077..7f518c0ae723 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1857,6 +1857,14 @@ static bool gic_enable_quirk_arm64_2941627(void *data)
return true;
}
+static bool rd_set_non_coherent(void *data)
+{
+ struct gic_chip_data *d = data;
+
+ d->rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
+ return true;
+}
+
static const struct gic_quirk gic_quirks[] = {
{
.desc = "GICv3: Qualcomm MSM8996 broken firmware",
@@ -1923,6 +1931,11 @@ static const struct gic_quirk gic_quirks[] = {
.mask = 0xff0f0fff,
.init = gic_enable_quirk_arm64_2941627,
},
+ {
+ .desc = "GICv3: non-coherent attribute",
+ .property = "dma-noncoherent",
+ .init = rd_set_non_coherent,
+ },
{
}
};
--
Without deviation from the norm, progress is not possible.
On Tue, Sep 05, 2023 at 12:34:58PM +0100, Marc Zyngier wrote:
[...]
> > * Make sure *all* the ITS are reset before we probe any, as
> > * they may be sharing memory. If any of the ITS fails to
> > @@ -5396,7 +5405,8 @@ static int __init its_of_probe(struct device_node *node)
> > continue;
> > }
> >
> > - its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
> > + its_probe_one(&res, &np->fwnode, of_node_to_nid(np),
> > + of_property_read_bool(np, "dma-noncoherent"));
> > }
> > return 0;
> > }
> > @@ -5533,7 +5543,8 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
> > }
> >
> > err = its_probe_one(&res, dom_handle,
> > - acpi_get_its_numa_node(its_entry->translation_id));
> > + acpi_get_its_numa_node(its_entry->translation_id),
> > + false);
>
> I came up with the following alternative approach, which is as usual
> completely untested. It is entirely based on the quirk infrastructure,
> and doesn't touch the ACPI path at all.
Writing the ACPI bits. We can't use the quirks framework for ACPI (we
don't have "properties" and I don't think we want to attach any to the
fwnode_handle) that's why I generalized its_probe_one() above with an
extra param, that would have simplified ACPI parsing:
- we alloc struct its_node in its_probe_one() but at that stage
ACPI parsing was already done. If we have to parse the MADT(ITS) again
just to scan for non-coherent we then have to match the MADT entries
to the *current* struct its_node* we are handling (MADT parsing
callbacks don't even take a param - we have to resort to global
variables - definitely doable but it is a bit ugly).
I will post the resulting code (or something else if I find anything
nicer) but I thought I would mention the reasoning behind the additional
bool flag above.
Lorenzo
>
> Thanks,
>
> M.
>
> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> index 3db4592cda1c..00641e88aa38 100644
> --- a/drivers/irqchip/irq-gic-common.h
> +++ b/drivers/irqchip/irq-gic-common.h
> @@ -29,4 +29,8 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
> void gic_enable_of_quirks(const struct device_node *np,
> const struct gic_quirk *quirks, void *data);
>
> +#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
> +#define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
> +#define RDIST_FLAGS_FORCE_NON_SHAREABLE (1 << 2)
> +
> #endif /* _IRQ_GIC_COMMON_H */
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index e0c2b10d154d..6edf59af757b 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -44,10 +44,6 @@
> #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
> #define ITS_FLAGS_FORCE_NON_SHAREABLE (1ULL << 3)
>
> -#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
> -#define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
> -#define RDIST_FLAGS_FORCE_NON_SHAREABLE (1 << 2)
> -
> #define RD_LOCAL_LPI_ENABLED BIT(0)
> #define RD_LOCAL_PENDTABLE_PREALLOCATED BIT(1)
> #define RD_LOCAL_MEMRESERVE_DONE BIT(2)
> @@ -4754,6 +4750,14 @@ static bool __maybe_unused its_enable_rk3588001(void *data)
> return true;
> }
>
> +static bool its_set_non_coherent(void *data)
> +{
> + struct its_node *its = data;
> +
> + its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
> + return true;
> +}
> +
> static const struct gic_quirk its_quirks[] = {
> #ifdef CONFIG_CAVIUM_ERRATUM_22375
> {
> @@ -4808,6 +4812,11 @@ static const struct gic_quirk its_quirks[] = {
> .init = its_enable_rk3588001,
> },
> #endif
> + {
> + .desc = "ITS: non-coherent attribute",
> + .property = "dma-noncoherent",
> + .init = its_set_non_coherent,
> + },
> {
> }
> };
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index eedfa8e9f077..7f518c0ae723 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -1857,6 +1857,14 @@ static bool gic_enable_quirk_arm64_2941627(void *data)
> return true;
> }
>
> +static bool rd_set_non_coherent(void *data)
> +{
> + struct gic_chip_data *d = data;
> +
> + d->rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
> + return true;
> +}
> +
> static const struct gic_quirk gic_quirks[] = {
> {
> .desc = "GICv3: Qualcomm MSM8996 broken firmware",
> @@ -1923,6 +1931,11 @@ static const struct gic_quirk gic_quirks[] = {
> .mask = 0xff0f0fff,
> .init = gic_enable_quirk_arm64_2941627,
> },
> + {
> + .desc = "GICv3: non-coherent attribute",
> + .property = "dma-noncoherent",
> + .init = rd_set_non_coherent,
> + },
> {
> }
> };
>
> --
> Without deviation from the norm, progress is not possible.
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 03/10/2023 3:43 pm, Lorenzo Pieralisi wrote:
> On Tue, Sep 05, 2023 at 12:34:58PM +0100, Marc Zyngier wrote:
>
> [...]
>
>>> * Make sure *all* the ITS are reset before we probe any, as
>>> * they may be sharing memory. If any of the ITS fails to
>>> @@ -5396,7 +5405,8 @@ static int __init its_of_probe(struct device_node *node)
>>> continue;
>>> }
>>>
>>> - its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
>>> + its_probe_one(&res, &np->fwnode, of_node_to_nid(np),
>>> + of_property_read_bool(np, "dma-noncoherent"));
>>> }
>>> return 0;
>>> }
>>> @@ -5533,7 +5543,8 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
>>> }
>>>
>>> err = its_probe_one(&res, dom_handle,
>>> - acpi_get_its_numa_node(its_entry->translation_id));
>>> + acpi_get_its_numa_node(its_entry->translation_id),
>>> + false);
>>
>> I came up with the following alternative approach, which is as usual
>> completely untested. It is entirely based on the quirk infrastructure,
>> and doesn't touch the ACPI path at all.
>
> Writing the ACPI bits. We can't use the quirks framework for ACPI (we
> don't have "properties" and I don't think we want to attach any to the
> fwnode_handle) that's why I generalized its_probe_one() above with an
> extra param, that would have simplified ACPI parsing:
>
> - we alloc struct its_node in its_probe_one() but at that stage
> ACPI parsing was already done. If we have to parse the MADT(ITS) again
> just to scan for non-coherent we then have to match the MADT entries
> to the *current* struct its_node* we are handling (MADT parsing
> callbacks don't even take a param - we have to resort to global
> variables - definitely doable but it is a bit ugly).
How about a compromise of passing a whole MADT flags field into
its_probe_one() (where its_of_probe() can just pass 0), to pass through
to its_enable_quirks() to then match against an madt_flags field in the
gic_quirk? gic_acpi_init() could then do something similar for the
redistributor quirk, although I guess it would then need to distinguish
GICC and GICR-based quirks cases since the respective flags are in
different formats.
Thanks,
Robin.
On Tue, 03 Oct 2023 15:43:40 +0100,
Lorenzo Pieralisi <[email protected]> wrote:
>
> On Tue, Sep 05, 2023 at 12:34:58PM +0100, Marc Zyngier wrote:
>
> [...]
>
> > > * Make sure *all* the ITS are reset before we probe any, as
> > > * they may be sharing memory. If any of the ITS fails to
> > > @@ -5396,7 +5405,8 @@ static int __init its_of_probe(struct device_node *node)
> > > continue;
> > > }
> > >
> > > - its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
> > > + its_probe_one(&res, &np->fwnode, of_node_to_nid(np),
> > > + of_property_read_bool(np, "dma-noncoherent"));
> > > }
> > > return 0;
> > > }
> > > @@ -5533,7 +5543,8 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
> > > }
> > >
> > > err = its_probe_one(&res, dom_handle,
> > > - acpi_get_its_numa_node(its_entry->translation_id));
> > > + acpi_get_its_numa_node(its_entry->translation_id),
> > > + false);
> >
> > I came up with the following alternative approach, which is as usual
> > completely untested. It is entirely based on the quirk infrastructure,
> > and doesn't touch the ACPI path at all.
>
> Writing the ACPI bits. We can't use the quirks framework for ACPI (we
> don't have "properties" and I don't think we want to attach any to the
> fwnode_handle) that's why I generalized its_probe_one() above with an
> extra param, that would have simplified ACPI parsing:
>
> - we alloc struct its_node in its_probe_one() but at that stage
> ACPI parsing was already done. If we have to parse the MADT(ITS) again
> just to scan for non-coherent we then have to match the MADT entries
> to the *current* struct its_node* we are handling (MADT parsing
> callbacks don't even take a param - we have to resort to global
> variables - definitely doable but it is a bit ugly).
Well, a more acceptable approach would be for its_probe_one() to take
an allocated and possibly pre-populated its_node structure (crucially,
with the quirk flags set), which itself results in a bunch of low
hanging cleanups, see the patch below.
I have boot tested it in a DT guest, so it is obviously perfect.
M.
From 978f654d4459adf0b8f3f8e896ca37035b3b114c Mon Sep 17 00:00:00 2001
From: Marc Zyngier <[email protected]>
Date: Tue, 3 Oct 2023 17:35:27 +0100
Subject: [PATCH] irqchip/gic-v3-its: Split allocation from initialisation of
its_node
In order to pave the way for more fancy quirk handling without making
more of a mess of this terrible driver, split the allocation of the
ITS descriptor (its_node) from the actual probing.
This will allow firmware-specific hooks to be added between these
two points.
Signed-off-by: Marc Zyngier <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 151 +++++++++++++++++++------------
1 file changed, 91 insertions(+), 60 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e0c2b10d154d..bf21383b714e 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4952,7 +4952,7 @@ static void __init __iomem *its_map_one(struct resource *res, int *err)
return NULL;
}
-static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
+static int its_init_domain(struct its_node *its)
{
struct irq_domain *inner_domain;
struct msi_domain_info *info;
@@ -4966,7 +4966,7 @@ static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
inner_domain = irq_domain_create_hierarchy(its_parent,
its->msi_domain_flags, 0,
- handle, &its_domain_ops,
+ its->fwnode_handle, &its_domain_ops,
info);
if (!inner_domain) {
kfree(info);
@@ -5017,8 +5017,7 @@ static int its_init_vpe_domain(void)
return 0;
}
-static int __init its_compute_its_list_map(struct resource *res,
- void __iomem *its_base)
+static int __init its_compute_its_list_map(struct its_node *its)
{
int its_number;
u32 ctlr;
@@ -5032,15 +5031,15 @@ static int __init its_compute_its_list_map(struct resource *res,
its_number = find_first_zero_bit(&its_list_map, GICv4_ITS_LIST_MAX);
if (its_number >= GICv4_ITS_LIST_MAX) {
pr_err("ITS@%pa: No ITSList entry available!\n",
- &res->start);
+ &its->phys_base);
return -EINVAL;
}
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
ctlr &= ~GITS_CTLR_ITS_NUMBER;
ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
- writel_relaxed(ctlr, its_base + GITS_CTLR);
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ writel_relaxed(ctlr, its->base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number << GITS_CTLR_ITS_NUMBER_SHIFT)) {
its_number = ctlr & GITS_CTLR_ITS_NUMBER;
its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
@@ -5048,75 +5047,50 @@ static int __init its_compute_its_list_map(struct resource *res,
if (test_and_set_bit(its_number, &its_list_map)) {
pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
- &res->start, its_number);
+ &its->phys_base, its_number);
return -EINVAL;
}
return its_number;
}
-static int __init its_probe_one(struct resource *res,
- struct fwnode_handle *handle, int numa_node)
+static int __init its_probe_one(struct its_node *its)
{
- struct its_node *its;
- void __iomem *its_base;
- u64 baser, tmp, typer;
+ u64 baser, tmp;
struct page *page;
u32 ctlr;
int err;
- its_base = its_map_one(res, &err);
- if (!its_base)
- return err;
-
- pr_info("ITS %pR\n", res);
-
- its = kzalloc(sizeof(*its), GFP_KERNEL);
- if (!its) {
- err = -ENOMEM;
- goto out_unmap;
- }
-
- raw_spin_lock_init(&its->lock);
- mutex_init(&its->dev_alloc_lock);
- INIT_LIST_HEAD(&its->entry);
- INIT_LIST_HEAD(&its->its_device_list);
- typer = gic_read_typer(its_base + GITS_TYPER);
- its->typer = typer;
- its->base = its_base;
- its->phys_base = res->start;
if (is_v4(its)) {
- if (!(typer & GITS_TYPER_VMOVP)) {
- err = its_compute_its_list_map(res, its_base);
+ if (!(its->typer & GITS_TYPER_VMOVP)) {
+ err = its_compute_its_list_map(its);
if (err < 0)
- goto out_free_its;
+ goto out;
its->list_nr = err;
pr_info("ITS@%pa: Using ITS number %d\n",
- &res->start, err);
+ &its->phys_base, err);
} else {
- pr_info("ITS@%pa: Single VMOVP capable\n", &res->start);
+ pr_info("ITS@%pa: Single VMOVP capable\n", &its->phys_base);
}
if (is_v4_1(its)) {
- u32 svpet = FIELD_GET(GITS_TYPER_SVPET, typer);
+ u32 svpet = FIELD_GET(GITS_TYPER_SVPET, its->typer);
- its->sgir_base = ioremap(res->start + SZ_128K, SZ_64K);
+ its->sgir_base = ioremap(its->phys_base + SZ_128K, SZ_64K);
if (!its->sgir_base) {
err = -ENOMEM;
- goto out_free_its;
+ goto out;
}
- its->mpidr = readl_relaxed(its_base + GITS_MPIDR);
+ its->mpidr = readl_relaxed(its->base + GITS_MPIDR);
pr_info("ITS@%pa: Using GICv4.1 mode %08x %08x\n",
- &res->start, its->mpidr, svpet);
+ &its->phys_base, its->mpidr, svpet);
}
}
- its->numa_node = numa_node;
-
page = alloc_pages_node(its->numa_node, GFP_KERNEL | __GFP_ZERO,
get_order(ITS_CMD_QUEUE_SZ));
if (!page) {
@@ -5125,12 +5099,9 @@ static int __init its_probe_one(struct resource *res,
}
its->cmd_base = (void *)page_address(page);
its->cmd_write = its->cmd_base;
- its->fwnode_handle = handle;
its->get_msi_base = its_irq_get_msi_base;
its->msi_domain_flags = IRQ_DOMAIN_FLAG_ISOLATED_MSI;
- its_enable_quirks(its);
-
err = its_alloc_tables(its);
if (err)
goto out_free_cmd;
@@ -5174,7 +5145,7 @@ static int __init its_probe_one(struct resource *res,
ctlr |= GITS_CTLR_ImDe;
writel_relaxed(ctlr, its->base + GITS_CTLR);
- err = its_init_domain(handle, its);
+ err = its_init_domain(its);
if (err)
goto out_free_tables;
@@ -5191,11 +5162,8 @@ static int __init its_probe_one(struct resource *res,
out_unmap_sgir:
if (its->sgir_base)
iounmap(its->sgir_base);
-out_free_its:
- kfree(its);
-out_unmap:
- iounmap(its_base);
- pr_err("ITS@%pa: failed probing (%d)\n", &res->start, err);
+out:
+ pr_err("ITS@%pa: failed probing (%d)\n", &its->phys_base, err);
return err;
}
@@ -5356,10 +5324,53 @@ static const struct of_device_id its_device_id[] = {
{},
};
+static struct its_node __init *its_node_init(struct resource *res,
+ struct fwnode_handle *handle, int numa_node)
+{
+ void __iomem *its_base;
+ struct its_node *its;
+ int err;
+
+ its_base = its_map_one(res, &err);
+ if (!its_base)
+ return NULL;
+
+ pr_info("ITS %pR\n", res);
+
+ its = kzalloc(sizeof(*its), GFP_KERNEL);
+ if (!its)
+ goto out_unmap;
+
+ raw_spin_lock_init(&its->lock);
+ mutex_init(&its->dev_alloc_lock);
+ INIT_LIST_HEAD(&its->entry);
+ INIT_LIST_HEAD(&its->its_device_list);
+
+ its->typer = gic_read_typer(its_base + GITS_TYPER);
+ its->base = its_base;
+ its->phys_base = res->start;
+
+ its->numa_node = numa_node;
+ its->fwnode_handle = handle;
+
+ return its;
+
+out_unmap:
+ iounmap(its_base);
+ return NULL;
+}
+
+static void its_node_destroy(struct its_node *its)
+{
+ iounmap(its->base);
+ kfree(its);
+}
+
static int __init its_of_probe(struct device_node *node)
{
struct device_node *np;
struct resource res;
+ int err;
/*
* Make sure *all* the ITS are reset before we probe any, as
@@ -5369,8 +5380,6 @@ static int __init its_of_probe(struct device_node *node)
*/
for (np = of_find_matching_node(node, its_device_id); np;
np = of_find_matching_node(np, its_device_id)) {
- int err;
-
if (!of_device_is_available(np) ||
!of_property_read_bool(np, "msi-controller") ||
of_address_to_resource(np, 0, &res))
@@ -5383,6 +5392,8 @@ static int __init its_of_probe(struct device_node *node)
for (np = of_find_matching_node(node, its_device_id); np;
np = of_find_matching_node(np, its_device_id)) {
+ struct its_node *its;
+
if (!of_device_is_available(np))
continue;
if (!of_property_read_bool(np, "msi-controller")) {
@@ -5396,7 +5407,17 @@ static int __init its_of_probe(struct device_node *node)
continue;
}
- its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
+
+ its = its_node_init(&res, &np->fwnode, of_node_to_nid(np));
+ if (!its)
+ return -ENOMEM;
+
+ its_enable_quirks(its);
+ err = its_probe_one(its);
+ if (err) {
+ its_node_destroy(its);
+ return err;
+ }
}
return 0;
}
@@ -5508,6 +5529,7 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
{
struct acpi_madt_generic_translator *its_entry;
struct fwnode_handle *dom_handle;
+ struct its_node *its;
struct resource res;
int err;
@@ -5532,11 +5554,20 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
goto dom_err;
}
- err = its_probe_one(&res, dom_handle,
- acpi_get_its_numa_node(its_entry->translation_id));
+ its = its_node_init(&res, dom_handle,
+ acpi_get_its_numa_node(its_entry->translation_id));
+ if (!its) {
+ err = -ENOMEM;
+ goto node_err;
+ }
+
+ /* Stick ACPI quirk handling here */
+
+ err = its_probe_one(its);
if (!err)
return 0;
+node_err:
iort_deregister_domain_token(its_entry->translation_id);
dom_err:
irq_domain_free_fwnode(dom_handle);
--
2.34.1
--
Without deviation from the norm, progress is not possible.
On Tue, Oct 03, 2023 at 05:44:27PM +0100, Marc Zyngier wrote:
> On Tue, 03 Oct 2023 15:43:40 +0100,
> Lorenzo Pieralisi <[email protected]> wrote:
> >
> > On Tue, Sep 05, 2023 at 12:34:58PM +0100, Marc Zyngier wrote:
> >
> > [...]
> >
> > > > * Make sure *all* the ITS are reset before we probe any, as
> > > > * they may be sharing memory. If any of the ITS fails to
> > > > @@ -5396,7 +5405,8 @@ static int __init its_of_probe(struct device_node *node)
> > > > continue;
> > > > }
> > > >
> > > > - its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
> > > > + its_probe_one(&res, &np->fwnode, of_node_to_nid(np),
> > > > + of_property_read_bool(np, "dma-noncoherent"));
> > > > }
> > > > return 0;
> > > > }
> > > > @@ -5533,7 +5543,8 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
> > > > }
> > > >
> > > > err = its_probe_one(&res, dom_handle,
> > > > - acpi_get_its_numa_node(its_entry->translation_id));
> > > > + acpi_get_its_numa_node(its_entry->translation_id),
> > > > + false);
> > >
> > > I came up with the following alternative approach, which is as usual
> > > completely untested. It is entirely based on the quirk infrastructure,
> > > and doesn't touch the ACPI path at all.
> >
> > Writing the ACPI bits. We can't use the quirks framework for ACPI (we
> > don't have "properties" and I don't think we want to attach any to the
> > fwnode_handle) that's why I generalized its_probe_one() above with an
> > extra param, that would have simplified ACPI parsing:
> >
> > - we alloc struct its_node in its_probe_one() but at that stage
> > ACPI parsing was already done. If we have to parse the MADT(ITS) again
> > just to scan for non-coherent we then have to match the MADT entries
> > to the *current* struct its_node* we are handling (MADT parsing
> > callbacks don't even take a param - we have to resort to global
> > variables - definitely doable but it is a bit ugly).
>
> Well, a more acceptable approach would be for its_probe_one() to take
> an allocated and possibly pre-populated its_node structure (crucially,
> with the quirk flags set), which itself results in a bunch of low
> hanging cleanups, see the patch below.
>
> I have boot tested it in a DT guest, so it is obviously perfect.
Thanks Marc I will test it and add the ACPI parsing bits.
Lorenzo
>
> M.
>
> From 978f654d4459adf0b8f3f8e896ca37035b3b114c Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <[email protected]>
> Date: Tue, 3 Oct 2023 17:35:27 +0100
> Subject: [PATCH] irqchip/gic-v3-its: Split allocation from initialisation of
> its_node
>
> In order to pave the way for more fancy quirk handling without making
> more of a mess of this terrible driver, split the allocation of the
> ITS descriptor (its_node) from the actual probing.
>
> This will allow firmware-specific hooks to be added between these
> two points.
>
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 151 +++++++++++++++++++------------
> 1 file changed, 91 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index e0c2b10d154d..bf21383b714e 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -4952,7 +4952,7 @@ static void __init __iomem *its_map_one(struct resource *res, int *err)
> return NULL;
> }
>
> -static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
> +static int its_init_domain(struct its_node *its)
> {
> struct irq_domain *inner_domain;
> struct msi_domain_info *info;
> @@ -4966,7 +4966,7 @@ static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
>
> inner_domain = irq_domain_create_hierarchy(its_parent,
> its->msi_domain_flags, 0,
> - handle, &its_domain_ops,
> + its->fwnode_handle, &its_domain_ops,
> info);
> if (!inner_domain) {
> kfree(info);
> @@ -5017,8 +5017,7 @@ static int its_init_vpe_domain(void)
> return 0;
> }
>
> -static int __init its_compute_its_list_map(struct resource *res,
> - void __iomem *its_base)
> +static int __init its_compute_its_list_map(struct its_node *its)
> {
> int its_number;
> u32 ctlr;
> @@ -5032,15 +5031,15 @@ static int __init its_compute_its_list_map(struct resource *res,
> its_number = find_first_zero_bit(&its_list_map, GICv4_ITS_LIST_MAX);
> if (its_number >= GICv4_ITS_LIST_MAX) {
> pr_err("ITS@%pa: No ITSList entry available!\n",
> - &res->start);
> + &its->phys_base);
> return -EINVAL;
> }
>
> - ctlr = readl_relaxed(its_base + GITS_CTLR);
> + ctlr = readl_relaxed(its->base + GITS_CTLR);
> ctlr &= ~GITS_CTLR_ITS_NUMBER;
> ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
> - writel_relaxed(ctlr, its_base + GITS_CTLR);
> - ctlr = readl_relaxed(its_base + GITS_CTLR);
> + writel_relaxed(ctlr, its->base + GITS_CTLR);
> + ctlr = readl_relaxed(its->base + GITS_CTLR);
> if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number << GITS_CTLR_ITS_NUMBER_SHIFT)) {
> its_number = ctlr & GITS_CTLR_ITS_NUMBER;
> its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
> @@ -5048,75 +5047,50 @@ static int __init its_compute_its_list_map(struct resource *res,
>
> if (test_and_set_bit(its_number, &its_list_map)) {
> pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
> - &res->start, its_number);
> + &its->phys_base, its_number);
> return -EINVAL;
> }
>
> return its_number;
> }
>
> -static int __init its_probe_one(struct resource *res,
> - struct fwnode_handle *handle, int numa_node)
> +static int __init its_probe_one(struct its_node *its)
> {
> - struct its_node *its;
> - void __iomem *its_base;
> - u64 baser, tmp, typer;
> + u64 baser, tmp;
> struct page *page;
> u32 ctlr;
> int err;
>
> - its_base = its_map_one(res, &err);
> - if (!its_base)
> - return err;
> -
> - pr_info("ITS %pR\n", res);
> -
> - its = kzalloc(sizeof(*its), GFP_KERNEL);
> - if (!its) {
> - err = -ENOMEM;
> - goto out_unmap;
> - }
> -
> - raw_spin_lock_init(&its->lock);
> - mutex_init(&its->dev_alloc_lock);
> - INIT_LIST_HEAD(&its->entry);
> - INIT_LIST_HEAD(&its->its_device_list);
> - typer = gic_read_typer(its_base + GITS_TYPER);
> - its->typer = typer;
> - its->base = its_base;
> - its->phys_base = res->start;
> if (is_v4(its)) {
> - if (!(typer & GITS_TYPER_VMOVP)) {
> - err = its_compute_its_list_map(res, its_base);
> + if (!(its->typer & GITS_TYPER_VMOVP)) {
> + err = its_compute_its_list_map(its);
> if (err < 0)
> - goto out_free_its;
> + goto out;
>
> its->list_nr = err;
>
> pr_info("ITS@%pa: Using ITS number %d\n",
> - &res->start, err);
> + &its->phys_base, err);
> } else {
> - pr_info("ITS@%pa: Single VMOVP capable\n", &res->start);
> + pr_info("ITS@%pa: Single VMOVP capable\n", &its->phys_base);
> }
>
> if (is_v4_1(its)) {
> - u32 svpet = FIELD_GET(GITS_TYPER_SVPET, typer);
> + u32 svpet = FIELD_GET(GITS_TYPER_SVPET, its->typer);
>
> - its->sgir_base = ioremap(res->start + SZ_128K, SZ_64K);
> + its->sgir_base = ioremap(its->phys_base + SZ_128K, SZ_64K);
> if (!its->sgir_base) {
> err = -ENOMEM;
> - goto out_free_its;
> + goto out;
> }
>
> - its->mpidr = readl_relaxed(its_base + GITS_MPIDR);
> + its->mpidr = readl_relaxed(its->base + GITS_MPIDR);
>
> pr_info("ITS@%pa: Using GICv4.1 mode %08x %08x\n",
> - &res->start, its->mpidr, svpet);
> + &its->phys_base, its->mpidr, svpet);
> }
> }
>
> - its->numa_node = numa_node;
> -
> page = alloc_pages_node(its->numa_node, GFP_KERNEL | __GFP_ZERO,
> get_order(ITS_CMD_QUEUE_SZ));
> if (!page) {
> @@ -5125,12 +5099,9 @@ static int __init its_probe_one(struct resource *res,
> }
> its->cmd_base = (void *)page_address(page);
> its->cmd_write = its->cmd_base;
> - its->fwnode_handle = handle;
> its->get_msi_base = its_irq_get_msi_base;
> its->msi_domain_flags = IRQ_DOMAIN_FLAG_ISOLATED_MSI;
>
> - its_enable_quirks(its);
> -
> err = its_alloc_tables(its);
> if (err)
> goto out_free_cmd;
> @@ -5174,7 +5145,7 @@ static int __init its_probe_one(struct resource *res,
> ctlr |= GITS_CTLR_ImDe;
> writel_relaxed(ctlr, its->base + GITS_CTLR);
>
> - err = its_init_domain(handle, its);
> + err = its_init_domain(its);
> if (err)
> goto out_free_tables;
>
> @@ -5191,11 +5162,8 @@ static int __init its_probe_one(struct resource *res,
> out_unmap_sgir:
> if (its->sgir_base)
> iounmap(its->sgir_base);
> -out_free_its:
> - kfree(its);
> -out_unmap:
> - iounmap(its_base);
> - pr_err("ITS@%pa: failed probing (%d)\n", &res->start, err);
> +out:
> + pr_err("ITS@%pa: failed probing (%d)\n", &its->phys_base, err);
> return err;
> }
>
> @@ -5356,10 +5324,53 @@ static const struct of_device_id its_device_id[] = {
> {},
> };
>
> +static struct its_node __init *its_node_init(struct resource *res,
> + struct fwnode_handle *handle, int numa_node)
> +{
> + void __iomem *its_base;
> + struct its_node *its;
> + int err;
> +
> + its_base = its_map_one(res, &err);
> + if (!its_base)
> + return NULL;
> +
> + pr_info("ITS %pR\n", res);
> +
> + its = kzalloc(sizeof(*its), GFP_KERNEL);
> + if (!its)
> + goto out_unmap;
> +
> + raw_spin_lock_init(&its->lock);
> + mutex_init(&its->dev_alloc_lock);
> + INIT_LIST_HEAD(&its->entry);
> + INIT_LIST_HEAD(&its->its_device_list);
> +
> + its->typer = gic_read_typer(its_base + GITS_TYPER);
> + its->base = its_base;
> + its->phys_base = res->start;
> +
> + its->numa_node = numa_node;
> + its->fwnode_handle = handle;
> +
> + return its;
> +
> +out_unmap:
> + iounmap(its_base);
> + return NULL;
> +}
> +
> +static void its_node_destroy(struct its_node *its)
> +{
> + iounmap(its->base);
> + kfree(its);
> +}
> +
> static int __init its_of_probe(struct device_node *node)
> {
> struct device_node *np;
> struct resource res;
> + int err;
>
> /*
> * Make sure *all* the ITS are reset before we probe any, as
> @@ -5369,8 +5380,6 @@ static int __init its_of_probe(struct device_node *node)
> */
> for (np = of_find_matching_node(node, its_device_id); np;
> np = of_find_matching_node(np, its_device_id)) {
> - int err;
> -
> if (!of_device_is_available(np) ||
> !of_property_read_bool(np, "msi-controller") ||
> of_address_to_resource(np, 0, &res))
> @@ -5383,6 +5392,8 @@ static int __init its_of_probe(struct device_node *node)
>
> for (np = of_find_matching_node(node, its_device_id); np;
> np = of_find_matching_node(np, its_device_id)) {
> + struct its_node *its;
> +
> if (!of_device_is_available(np))
> continue;
> if (!of_property_read_bool(np, "msi-controller")) {
> @@ -5396,7 +5407,17 @@ static int __init its_of_probe(struct device_node *node)
> continue;
> }
>
> - its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
> +
> + its = its_node_init(&res, &np->fwnode, of_node_to_nid(np));
> + if (!its)
> + return -ENOMEM;
> +
> + its_enable_quirks(its);
> + err = its_probe_one(its);
> + if (err) {
> + its_node_destroy(its);
> + return err;
> + }
> }
> return 0;
> }
> @@ -5508,6 +5529,7 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
> {
> struct acpi_madt_generic_translator *its_entry;
> struct fwnode_handle *dom_handle;
> + struct its_node *its;
> struct resource res;
> int err;
>
> @@ -5532,11 +5554,20 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
> goto dom_err;
> }
>
> - err = its_probe_one(&res, dom_handle,
> - acpi_get_its_numa_node(its_entry->translation_id));
> + its = its_node_init(&res, dom_handle,
> + acpi_get_its_numa_node(its_entry->translation_id));
> + if (!its) {
> + err = -ENOMEM;
> + goto node_err;
> + }
> +
> + /* Stick ACPI quirk handling here */
> +
> + err = its_probe_one(its);
> if (!err)
> return 0;
>
> +node_err:
> iort_deregister_domain_token(its_entry->translation_id);
> dom_err:
> irq_domain_free_fwnode(dom_handle);
> --
> 2.34.1
>
> --
> Without deviation from the norm, progress is not possible.