A lot of changes in v2. The distributor and redistributor saving and
restoring is left to the PSCI/firmware implementation after
discussions with ARM. This reduces the line changes by a lot and
removes now unneeded patches.
Patches are verified on an RK3399 platform with pending patches in the
ARM-Trusted-Firmware project.
Derek Basehore (5):
cpu_pm: add syscore_suspend error handling
irqchip/gic-v3-its: add ability to save/restore ITS state
DT/arm,gic-v3-its: add reset-on-suspend property
irqchip/gic-v3-its: add ability to resend MAPC on resume
DT/arm,gic-v3: add collections-reset-on-suspend property
.../bindings/interrupt-controller/arm,gic-v3.txt | 7 +
arch/arm64/Kconfig | 10 ++
drivers/irqchip/irq-gic-v3-its.c | 187 ++++++++++++++++-----
kernel/cpu_pm.c | 3 +
4 files changed, 169 insertions(+), 38 deletions(-)
--
2.16.0.rc1.238.g530d649a79-goog
This boolean property for the GIC-V3-ITS enables resending the MAP
COLLECTIONS commands when resuming for when the state is reset on
suspend.
Signed-off-by: Derek Basehore <[email protected]>
---
Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
index a470147d4f14..adb958e046d2 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
@@ -81,6 +81,10 @@ Optional:
- reset-on-suspend: Boolean property. Indicates that the ITS state is
reset on suspend. The state is then saved on suspend and restored on
resume.
+- collections-reset-on-suspend : Boolean property. If the collections for the
+ ITS are stored internally instead of externally, the state will be lost if the
+ GIC loses power. Setting this enables the kernel to reset the collections
+ state on resume for this ITS node.
The main GIC node must contain the appropriate #address-cells,
#size-cells and ranges properties for the reg property of all ITS
--
2.16.0.rc1.238.g530d649a79-goog
This adds documentation for the new reset-on-suspend property. This
property enables saving and restoring the ITS for when it loses state
in system suspend.
Signed-off-by: Derek Basehore <[email protected]>
---
Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
index 0a57f2f4167d..a470147d4f14 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
@@ -78,6 +78,9 @@ These nodes must have the following properties:
Optional:
- socionext,synquacer-pre-its: (u32, u32) tuple describing the untranslated
address and size of the pre-ITS window.
+- reset-on-suspend: Boolean property. Indicates that the ITS state is
+ reset on suspend. The state is then saved on suspend and restored on
+ resume.
The main GIC node must contain the appropriate #address-cells,
#size-cells and ranges properties for the reg property of all ITS
--
2.16.0.rc1.238.g530d649a79-goog
This adds functionality to resend the MAPC command to an ITS node on
resume. If the ITS is powered down during suspend and the collections
are not backed by memory, the ITS will lose that state. This just sets
up the known state for the collections after the ITS is restored.
This feature is enabled via Kconfig and the device tree.
Signed-off-by: Derek Basehore <[email protected]>
---
arch/arm64/Kconfig | 10 ++++
drivers/irqchip/irq-gic-v3-its.c | 101 ++++++++++++++++++++++++---------------
2 files changed, 73 insertions(+), 38 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 53612879fe56..f38f1a7b4266 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -571,6 +571,16 @@ config HISILICON_ERRATUM_161600802
If unsure, say Y.
+config ARM_GIC500_COLLECTIONS_RESET
+ bool "GIC-500 Collections: Workaround for GIC-500 Collections on suspend reset"
+ default y
+ help
+ The GIC-500 can store Collections state internally for the ITS. If
+ the ITS is reset on suspend (ie from power getting disabled), the
+ collections need to be reconfigured on resume.
+
+ If unsure, say Y.
+
config QCOM_FALKOR_ERRATUM_E1041
bool "Falkor E1041: Speculative instruction fetches might cause errant memory access"
default y
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4727b447610f..7633ea2b0a3c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -48,6 +48,7 @@
#define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1)
#define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
#define ITS_FLAGS_SAVE_SUSPEND_STATE (1ULL << 3)
+#define ITS_FLAGS_WORKAROUND_GIC500_MAPC (1ULL << 4)
#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
@@ -1950,52 +1951,53 @@ static void its_cpu_init_lpis(void)
dsb(sy);
}
-static void its_cpu_init_collection(void)
+static void its_cpu_init_collection(struct its_node *its)
{
- struct its_node *its;
- int cpu;
-
- spin_lock(&its_lock);
- cpu = smp_processor_id();
-
- list_for_each_entry(its, &its_nodes, entry) {
- u64 target;
+ int cpu = smp_processor_id();
+ u64 target;
- /* avoid cross node collections and its mapping */
- if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
- struct device_node *cpu_node;
+ /* avoid cross node collections and its mapping */
+ if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
+ struct device_node *cpu_node;
- cpu_node = of_get_cpu_node(cpu, NULL);
- if (its->numa_node != NUMA_NO_NODE &&
- its->numa_node != of_node_to_nid(cpu_node))
- continue;
- }
+ cpu_node = of_get_cpu_node(cpu, NULL);
+ if (its->numa_node != NUMA_NO_NODE &&
+ its->numa_node != of_node_to_nid(cpu_node))
+ return;
+ }
+ /*
+ * We now have to bind each collection to its target
+ * redistributor.
+ */
+ if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) {
/*
- * We now have to bind each collection to its target
+ * This ITS wants the physical address of the
* redistributor.
*/
- if (gic_read_typer(its->base + GITS_TYPER) & GITS_TYPER_PTA) {
- /*
- * This ITS wants the physical address of the
- * redistributor.
- */
- target = gic_data_rdist()->phys_base;
- } else {
- /*
- * This ITS wants a linear CPU number.
- */
- target = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER);
- target = GICR_TYPER_CPU_NUMBER(target) << 16;
- }
+ target = gic_data_rdist()->phys_base;
+ } else {
+ /* This ITS wants a linear CPU number. */
+ target = gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER);
+ target = GICR_TYPER_CPU_NUMBER(target) << 16;
+ }
- /* Perform collection mapping */
- its->collections[cpu].target_address = target;
- its->collections[cpu].col_id = cpu;
+ /* Perform collection mapping */
+ its->collections[cpu].target_address = target;
+ its->collections[cpu].col_id = cpu;
- its_send_mapc(its, &its->collections[cpu], 1);
- its_send_invall(its, &its->collections[cpu]);
- }
+ its_send_mapc(its, &its->collections[cpu], 1);
+ its_send_invall(its, &its->collections[cpu]);
+}
+
+static void its_cpu_init_collections(void)
+{
+ struct its_node *its;
+
+ spin_lock(&its_lock);
+
+ list_for_each_entry(its, &its_nodes, entry)
+ its_cpu_init_collection(its);
spin_unlock(&its_lock);
}
@@ -2997,6 +2999,18 @@ static bool __maybe_unused its_enable_quirk_hip07_161600802(void *data)
return true;
}
+static bool __maybe_unused its_enable_quirk_gic500_collections(void *data)
+{
+ struct its_node *its = data;
+
+ if (fwnode_property_present(its->fwnode_handle,
+ "collections-reset-on-suspend")) {
+ its->flags |= ITS_FLAGS_WORKAROUND_GIC500_MAPC;
+ return true;
+ }
+ return false;
+}
+
static const struct gic_quirk its_quirks[] = {
#ifdef CONFIG_CAVIUM_ERRATUM_22375
{
@@ -3042,6 +3056,14 @@ static const struct gic_quirk its_quirks[] = {
.mask = 0xffffffff,
.init = its_enable_quirk_hip07_161600802,
},
+#endif
+#ifdef CONFIG_ARM_GIC500_COLLECTIONS_RESET
+ {
+ .desc = "ITS: GIC-500 Collections Reset on Resume",
+ .iidr = 0x00000000,
+ .mask = 0xff000000,
+ .init = its_enable_quirk_gic500_collections,
+ },
#endif
{
}
@@ -3114,6 +3136,9 @@ void its_restore_enable(void)
}
writel_relaxed(ctx->ctlr, base + GITS_CTLR);
}
+
+ if (its->flags & ITS_FLAGS_WORKAROUND_GIC500_MAPC)
+ its_cpu_init_collection(its);
}
spin_unlock(&its_lock);
}
@@ -3380,7 +3405,7 @@ int its_cpu_init(void)
return -ENXIO;
}
its_cpu_init_lpis();
- its_cpu_init_collection();
+ its_cpu_init_collections();
}
return 0;
--
2.16.0.rc1.238.g530d649a79-goog
Some platforms power off GIC logic in suspend, so we need to
save/restore state. The distributor and redistributor registers need
to be handled in platform code due to access permissions on those
registers, but the ITS registers can be restored in the kernel.
Signed-off-by: Derek Basehore <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 86 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 06f025fd5726..4727b447610f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -33,6 +33,7 @@
#include <linux/of_platform.h>
#include <linux/percpu.h>
#include <linux/slab.h>
+#include <linux/syscore_ops.h>
#include <linux/irqchip.h>
#include <linux/irqchip/arm-gic-v3.h>
@@ -46,6 +47,7 @@
#define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0)
#define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1)
#define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
+#define ITS_FLAGS_SAVE_SUSPEND_STATE (1ULL << 3)
#define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
@@ -83,6 +85,15 @@ struct its_baser {
u32 psz;
};
+/*
+ * Saved ITS state - this is where saved state for the ITS is stored
+ * when it's disabled during system suspend.
+ */
+struct its_ctx {
+ u64 cbaser;
+ u32 ctlr;
+};
+
struct its_device;
/*
@@ -101,6 +112,7 @@ struct its_node {
struct its_collection *collections;
struct fwnode_handle *fwnode_handle;
u64 (*get_msi_base)(struct its_device *its_dev);
+ struct its_ctx its_ctx;
struct list_head its_device_list;
u64 flags;
unsigned long list_nr;
@@ -3042,6 +3054,75 @@ static void its_enable_quirks(struct its_node *its)
gic_enable_quirks(iidr, its_quirks, its);
}
+int its_save_disable(void)
+{
+ struct its_node *its;
+ int err = 0;
+
+ spin_lock(&its_lock);
+ list_for_each_entry(its, &its_nodes, entry) {
+ if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
+ struct its_ctx *ctx = &its->its_ctx;
+ void __iomem *base = its->base;
+
+ ctx->ctlr = readl_relaxed(base + GITS_CTLR);
+ err = its_force_quiescent(base);
+ if (err) {
+ writel_relaxed(ctx->ctlr, base + GITS_CTLR);
+ goto err;
+ }
+
+ ctx->cbaser = gits_read_cbaser(base + GITS_CBASER);
+ }
+ }
+
+err:
+ if (err) {
+ list_for_each_entry_continue_reverse(its, &its_nodes, entry) {
+ if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
+ struct its_ctx *ctx = &its->its_ctx;
+ void __iomem *base = its->base;
+
+ writel_relaxed(ctx->ctlr, base + GITS_CTLR);
+ }
+ }
+ }
+
+ spin_unlock(&its_lock);
+
+ return err;
+}
+
+void its_restore_enable(void)
+{
+ struct its_node *its;
+
+ spin_lock(&its_lock);
+ list_for_each_entry(its, &its_nodes, entry) {
+ struct its_ctx *ctx = &its->its_ctx;
+ struct its_baser *baser;
+ void __iomem *base;
+ int i;
+
+ if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
+ base = its->base;
+ gits_write_cbaser(ctx->cbaser, base + GITS_CBASER);
+ /* Restore GITS_BASER from the value cache. */
+ for (i = 0; i < GITS_BASER_NR_REGS; i++) {
+ baser = &its->tables[i];
+ its_write_baser(its, baser, baser->val);
+ }
+ writel_relaxed(ctx->ctlr, base + GITS_CTLR);
+ }
+ }
+ spin_unlock(&its_lock);
+}
+
+static struct syscore_ops its_syscore_ops = {
+ .suspend = its_save_disable,
+ .resume = its_restore_enable,
+};
+
static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
{
struct irq_domain *inner_domain;
@@ -3261,6 +3342,9 @@ static int __init its_probe_one(struct resource *res,
ctlr |= GITS_CTLR_ImDe;
writel_relaxed(ctlr, its->base + GITS_CTLR);
+ if (fwnode_property_present(handle, "reset-on-suspend"))
+ its->flags |= ITS_FLAGS_SAVE_SUSPEND_STATE;
+
err = its_init_domain(handle, its);
if (err)
goto out_free_tables;
@@ -3515,5 +3599,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
}
}
+ register_syscore_ops(&its_syscore_ops);
+
return 0;
}
--
2.16.0.rc1.238.g530d649a79-goog
If cpu_cluster_pm_enter() fails, cpu_pm_exit() should be called. This
will put the CPU in the correct state to resume from the failure.
Signed-off-by: Derek Basehore <[email protected]>
---
kernel/cpu_pm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/cpu_pm.c b/kernel/cpu_pm.c
index 67b02e138a47..03bcc0751a51 100644
--- a/kernel/cpu_pm.c
+++ b/kernel/cpu_pm.c
@@ -186,6 +186,9 @@ static int cpu_pm_suspend(void)
return ret;
ret = cpu_cluster_pm_enter();
+ if (ret)
+ cpu_pm_exit();
+
return ret;
}
--
2.16.0.rc1.238.g530d649a79-goog
One trivial comment:
On Thu, Jan 25, 2018 at 11:38:32PM -0800, Derek Basehore wrote:
> Some platforms power off GIC logic in suspend, so we need to
> save/restore state. The distributor and redistributor registers need
> to be handled in platform code due to access permissions on those
> registers, but the ITS registers can be restored in the kernel.
>
> Signed-off-by: Derek Basehore <[email protected]>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 86 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 06f025fd5726..4727b447610f 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
...
> @@ -3042,6 +3054,75 @@ static void its_enable_quirks(struct its_node *its)
> gic_enable_quirks(iidr, its_quirks, its);
> }
>
> +int its_save_disable(void)
This (and its_restore_enable()) should be static, now that you're only
using them in this file.
Brian
> +{
> + struct its_node *its;
> + int err = 0;
> +
> + spin_lock(&its_lock);
> + list_for_each_entry(its, &its_nodes, entry) {
> + if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
> + struct its_ctx *ctx = &its->its_ctx;
> + void __iomem *base = its->base;
> +
> + ctx->ctlr = readl_relaxed(base + GITS_CTLR);
> + err = its_force_quiescent(base);
> + if (err) {
> + writel_relaxed(ctx->ctlr, base + GITS_CTLR);
> + goto err;
> + }
> +
> + ctx->cbaser = gits_read_cbaser(base + GITS_CBASER);
> + }
> + }
> +
> +err:
> + if (err) {
> + list_for_each_entry_continue_reverse(its, &its_nodes, entry) {
> + if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
> + struct its_ctx *ctx = &its->its_ctx;
> + void __iomem *base = its->base;
> +
> + writel_relaxed(ctx->ctlr, base + GITS_CTLR);
> + }
> + }
> + }
> +
> + spin_unlock(&its_lock);
> +
> + return err;
> +}
> +
> +void its_restore_enable(void)
> +{
> + struct its_node *its;
> +
> + spin_lock(&its_lock);
> + list_for_each_entry(its, &its_nodes, entry) {
> + struct its_ctx *ctx = &its->its_ctx;
> + struct its_baser *baser;
> + void __iomem *base;
> + int i;
> +
> + if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
> + base = its->base;
> + gits_write_cbaser(ctx->cbaser, base + GITS_CBASER);
> + /* Restore GITS_BASER from the value cache. */
> + for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> + baser = &its->tables[i];
> + its_write_baser(its, baser, baser->val);
> + }
> + writel_relaxed(ctx->ctlr, base + GITS_CTLR);
> + }
> + }
> + spin_unlock(&its_lock);
> +}
> +
> +static struct syscore_ops its_syscore_ops = {
> + .suspend = its_save_disable,
> + .resume = its_restore_enable,
> +};
> +
> static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
> {
> struct irq_domain *inner_domain;
On Fri, Jan 26, 2018 at 12:59 PM, Brian Norris <[email protected]> wrote:
> One trivial comment:
>
> On Thu, Jan 25, 2018 at 11:38:32PM -0800, Derek Basehore wrote:
>> Some platforms power off GIC logic in suspend, so we need to
>> save/restore state. The distributor and redistributor registers need
>> to be handled in platform code due to access permissions on those
>> registers, but the ITS registers can be restored in the kernel.
>>
>> Signed-off-by: Derek Basehore <[email protected]>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 86 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 86 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 06f025fd5726..4727b447610f 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>
> ...
>
>> @@ -3042,6 +3054,75 @@ static void its_enable_quirks(struct its_node *its)
>> gic_enable_quirks(iidr, its_quirks, its);
>> }
>>
>> +int its_save_disable(void)
>
> This (and its_restore_enable()) should be static, now that you're only
> using them in this file.
Oops. I'll change that and add a pr_err in the its_force_quiescent
error case in the next patch set.
>
> Brian
>
>> +{
>> + struct its_node *its;
>> + int err = 0;
>> +
>> + spin_lock(&its_lock);
>> + list_for_each_entry(its, &its_nodes, entry) {
>> + if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
>> + struct its_ctx *ctx = &its->its_ctx;
>> + void __iomem *base = its->base;
>> +
>> + ctx->ctlr = readl_relaxed(base + GITS_CTLR);
>> + err = its_force_quiescent(base);
>> + if (err) {
>> + writel_relaxed(ctx->ctlr, base + GITS_CTLR);
>> + goto err;
>> + }
>> +
>> + ctx->cbaser = gits_read_cbaser(base + GITS_CBASER);
>> + }
>> + }
>> +
>> +err:
>> + if (err) {
>> + list_for_each_entry_continue_reverse(its, &its_nodes, entry) {
>> + if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
>> + struct its_ctx *ctx = &its->its_ctx;
>> + void __iomem *base = its->base;
>> +
>> + writel_relaxed(ctx->ctlr, base + GITS_CTLR);
>> + }
>> + }
>> + }
>> +
>> + spin_unlock(&its_lock);
>> +
>> + return err;
>> +}
>> +
>> +void its_restore_enable(void)
>> +{
>> + struct its_node *its;
>> +
>> + spin_lock(&its_lock);
>> + list_for_each_entry(its, &its_nodes, entry) {
>> + struct its_ctx *ctx = &its->its_ctx;
>> + struct its_baser *baser;
>> + void __iomem *base;
>> + int i;
>> +
>> + if (its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE) {
>> + base = its->base;
>> + gits_write_cbaser(ctx->cbaser, base + GITS_CBASER);
>> + /* Restore GITS_BASER from the value cache. */
>> + for (i = 0; i < GITS_BASER_NR_REGS; i++) {
>> + baser = &its->tables[i];
>> + its_write_baser(its, baser, baser->val);
>> + }
>> + writel_relaxed(ctx->ctlr, base + GITS_CTLR);
>> + }
>> + }
>> + spin_unlock(&its_lock);
>> +}
>> +
>> +static struct syscore_ops its_syscore_ops = {
>> + .suspend = its_save_disable,
>> + .resume = its_restore_enable,
>> +};
>> +
>> static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
>> {
>> struct irq_domain *inner_domain;