* [PATCH] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts
@ 2016-08-16 10:19 Sudeep Holla
2016-08-16 19:21 ` Christopher Covington
2016-08-17 12:49 ` [PATCH v2] " Sudeep Holla
0 siblings, 2 replies; 5+ messages in thread
From: Sudeep Holla @ 2016-08-16 10:19 UTC (permalink / raw)
To: linux-kernel, Christopher Covington
Cc: Sudeep Holla, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Lorenzo Pieralisi
As per the GICv3 specification, to power down a processor using GICv3
and allow automatic power-on if an interrupt must be sent to a processor,
software must set Enable to zero for all interrupt groups(by writing
to GICC_CTLR or ICC_IGRPEN{0,1}_EL1/3 as appropriate.
When commit 3708d52fc6bb ("irqchip: gic-v3: Implement CPU PM notifier")
was introduced there were no firmware implementations(in particular PSCI)
handling this.
Linux kernel may not be aware of the CPU power state details and might
fail to identify the power states that require quiescing the CPU
interface. Even if it can be aware of those details, it can't determine
which CPU power state have been triggered at the platform level and how
the power control is implemented.
This patch make disabling redistributor and group1 non-secure interrupts
in the power down path and re-enabling of redistributor in the power-up
path conditional. It will be handled in the kernel if and only if the
non-secure accesses are permitted to access and modify control registers.
It is left to the platform implementation otherwise.
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/irqchip/irq-gic-v3.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
Hi Christopher,
Can you check if ACPI processor idle works with this patch on QDF2432 ?
PSCI implementation much now deal with Grp1 interrupts when CPU is being
powered down during suspend/resume. I have pushed changes to ARM TF[1]
Regards,
Sudeep
[1] https://github.com/sudeep-holla/arm-trusted-firmware/commit/65d68ca64d12a4ce5b05a96808dd6f638451940d
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 6fc56c3466b0..a8db96540ca1 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -119,6 +119,12 @@ static void gic_redist_wait_for_rwp(void)
gic_do_wait_for_rwp(gic_data_rdist_rd_base());
}
+/* Check whether it's single security state view */
+static bool gic_dist_security_disabled(void)
+{
+ return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
+}
+
#ifdef CONFIG_ARM64
static DEFINE_STATIC_KEY_FALSE(is_cavium_thunderx);
@@ -671,9 +677,10 @@ static int gic_cpu_pm_notifier(struct notifier_block *self,
unsigned long cmd, void *v)
{
if (cmd == CPU_PM_EXIT) {
- gic_enable_redist(true);
+ if (gic_dist_security_disabled())
+ gic_enable_redist(true);
gic_cpu_sys_reg_init();
- } else if (cmd == CPU_PM_ENTER) {
+ } else if (cmd == CPU_PM_ENTER && gic_dist_security_disabled()) {
gic_write_grpen1(0);
gic_enable_redist(false);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts
2016-08-16 10:19 [PATCH] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts Sudeep Holla
@ 2016-08-16 19:21 ` Christopher Covington
2016-08-17 12:39 ` Sudeep Holla
2016-08-17 12:49 ` [PATCH v2] " Sudeep Holla
1 sibling, 1 reply; 5+ messages in thread
From: Christopher Covington @ 2016-08-16 19:21 UTC (permalink / raw)
To: Sudeep Holla, linux-kernel
Cc: Thomas Gleixner, Jason Cooper, Marc Zyngier, Lorenzo Pieralisi,
Prashanth Prakash
Thanks Sudeep!
On 08/16/2016 06:19 AM, Sudeep Holla wrote:
> As per the GICv3 specification, to power down a processor using GICv3
> and allow automatic power-on if an interrupt must be sent to a processor,
> software must set Enable to zero for all interrupt groups(by writing
> to GICC_CTLR or ICC_IGRPEN{0,1}_EL1/3 as appropriate.
>
> When commit 3708d52fc6bb ("irqchip: gic-v3: Implement CPU PM notifier")
> was introduced there were no firmware implementations(in particular PSCI)
> handling this.
>
> Linux kernel may not be aware of the CPU power state details and might
> fail to identify the power states that require quiescing the CPU
> interface. Even if it can be aware of those details, it can't determine
> which CPU power state have been triggered at the platform level and how
> the power control is implemented.
>
> This patch make disabling redistributor and group1 non-secure interrupts
> in the power down path and re-enabling of redistributor in the power-up
> path conditional. It will be handled in the kernel if and only if the
> non-secure accesses are permitted to access and modify control registers.
> It is left to the platform implementation otherwise.
>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/irqchip/irq-gic-v3.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> Hi Christopher,
>
> Can you check if ACPI processor idle works with this patch on QDF2432 ?
This fixes the boot hang, and I see the usage and time files in cpuidle
sysfs increasing on an idle system.
Tested-by: Christopher Covington <cov@codeaurora.org>
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts
2016-08-16 19:21 ` Christopher Covington
@ 2016-08-17 12:39 ` Sudeep Holla
0 siblings, 0 replies; 5+ messages in thread
From: Sudeep Holla @ 2016-08-17 12:39 UTC (permalink / raw)
To: Christopher Covington, linux-kernel
Cc: Sudeep Holla, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Lorenzo Pieralisi, Prashanth Prakash
Hi Christopher,
On 16/08/16 20:21, Christopher Covington wrote:
> Thanks Sudeep!
>
> On 08/16/2016 06:19 AM, Sudeep Holla wrote:
>> As per the GICv3 specification, to power down a processor using GICv3
>> and allow automatic power-on if an interrupt must be sent to a processor,
>> software must set Enable to zero for all interrupt groups(by writing
>> to GICC_CTLR or ICC_IGRPEN{0,1}_EL1/3 as appropriate.
>>
>> When commit 3708d52fc6bb ("irqchip: gic-v3: Implement CPU PM notifier")
>> was introduced there were no firmware implementations(in particular PSCI)
>> handling this.
>>
>> Linux kernel may not be aware of the CPU power state details and might
>> fail to identify the power states that require quiescing the CPU
>> interface. Even if it can be aware of those details, it can't determine
>> which CPU power state have been triggered at the platform level and how
>> the power control is implemented.
>>
>> This patch make disabling redistributor and group1 non-secure interrupts
>> in the power down path and re-enabling of redistributor in the power-up
>> path conditional. It will be handled in the kernel if and only if the
>> non-secure accesses are permitted to access and modify control registers.
>> It is left to the platform implementation otherwise.
>>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>> drivers/irqchip/irq-gic-v3.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> Hi Christopher,
>>
>> Can you check if ACPI processor idle works with this patch on QDF2432 ?
>
> This fixes the boot hang, and I see the usage and time files in cpuidle
> sysfs increasing on an idle system.
>
> Tested-by: Christopher Covington <cov@codeaurora.org>
>
Thanks for testing.
I found the when CPU_PM is disabled, build triggers a warning. I will
move gic_dist_security_disabled inside the ifdef CPU_PM and post v2.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts
2016-08-16 10:19 [PATCH] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts Sudeep Holla
2016-08-16 19:21 ` Christopher Covington
@ 2016-08-17 12:49 ` Sudeep Holla
2016-08-17 13:02 ` Marc Zyngier
1 sibling, 1 reply; 5+ messages in thread
From: Sudeep Holla @ 2016-08-17 12:49 UTC (permalink / raw)
To: linux-kernel
Cc: Sudeep Holla, Christopher Covington, Prashanth Prakash,
Marc Zyngier, Lorenzo Pieralisi, Thomas Gleixner, Jason Cooper
As per the GICv3 specification, to power down a processor using GICv3
and allow automatic power-on if an interrupt must be sent to a processor,
software must set Enable to zero for all interrupt groups(by writing
to GICC_CTLR or ICC_IGRPEN{0,1}_EL1/3 as appropriate.
When commit 3708d52fc6bb ("irqchip: gic-v3: Implement CPU PM notifier")
was introduced there were no firmware implementations(in particular PSCI)
handling this.
Linux kernel may not be aware of the CPU power state details and might
fail to identify the power states that require quiescing the CPU
interface. Even if it can be aware of those details, it can't determine
which CPU power state have been triggered at the platform level and how
the power control is implemented.
This patch make disabling redistributor and group1 non-secure interrupts
in the power down path and re-enabling of redistributor in the power-up
path conditional. It will be handled in the kernel if and only if the
non-secure accesses are permitted to access and modify control registers.
It is left to the platform implementation otherwise.
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Tested-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/irqchip/irq-gic-v3.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
v1->v2:
- Moved gic_dist_security_disabled inside CONFIG_CPU_PM to fix
the build warning triggered otherwise.
Hi Marc,
Consider this as a fix for v4.8 as it fixes CPUIdle related boot hang on
Qualcomm QDF2432 platform.
Regards,
Sudeep
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 6fc56c3466b0..ede5672ab34d 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -667,13 +667,20 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
#endif
#ifdef CONFIG_CPU_PM
+/* Check whether it's single security state view */
+static bool gic_dist_security_disabled(void)
+{
+ return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
+}
+
static int gic_cpu_pm_notifier(struct notifier_block *self,
unsigned long cmd, void *v)
{
if (cmd == CPU_PM_EXIT) {
- gic_enable_redist(true);
+ if (gic_dist_security_disabled())
+ gic_enable_redist(true);
gic_cpu_sys_reg_init();
- } else if (cmd == CPU_PM_ENTER) {
+ } else if (cmd == CPU_PM_ENTER && gic_dist_security_disabled()) {
gic_write_grpen1(0);
gic_enable_redist(false);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts
2016-08-17 12:49 ` [PATCH v2] " Sudeep Holla
@ 2016-08-17 13:02 ` Marc Zyngier
0 siblings, 0 replies; 5+ messages in thread
From: Marc Zyngier @ 2016-08-17 13:02 UTC (permalink / raw)
To: Sudeep Holla, linux-kernel
Cc: Christopher Covington, Prashanth Prakash, Lorenzo Pieralisi,
Thomas Gleixner, Jason Cooper
On 17/08/16 13:49, Sudeep Holla wrote:
> As per the GICv3 specification, to power down a processor using GICv3
> and allow automatic power-on if an interrupt must be sent to a processor,
> software must set Enable to zero for all interrupt groups(by writing
> to GICC_CTLR or ICC_IGRPEN{0,1}_EL1/3 as appropriate.
>
> When commit 3708d52fc6bb ("irqchip: gic-v3: Implement CPU PM notifier")
> was introduced there were no firmware implementations(in particular PSCI)
> handling this.
>
> Linux kernel may not be aware of the CPU power state details and might
> fail to identify the power states that require quiescing the CPU
> interface. Even if it can be aware of those details, it can't determine
> which CPU power state have been triggered at the platform level and how
> the power control is implemented.
>
> This patch make disabling redistributor and group1 non-secure interrupts
> in the power down path and re-enabling of redistributor in the power-up
> path conditional. It will be handled in the kernel if and only if the
> non-secure accesses are permitted to access and modify control registers.
> It is left to the platform implementation otherwise.
>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Tested-by: Christopher Covington <cov@codeaurora.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/irqchip/irq-gic-v3.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> v1->v2:
> - Moved gic_dist_security_disabled inside CONFIG_CPU_PM to fix
> the build warning triggered otherwise.
>
> Hi Marc,
>
> Consider this as a fix for v4.8 as it fixes CPUIdle related boot hang on
> Qualcomm QDF2432 platform.
Applied, thanks.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-08-17 13:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-16 10:19 [PATCH] irqchip/gicv3: remove disabling redistributor and group1 non-secure interrupts Sudeep Holla
2016-08-16 19:21 ` Christopher Covington
2016-08-17 12:39 ` Sudeep Holla
2016-08-17 12:49 ` [PATCH v2] " Sudeep Holla
2016-08-17 13:02 ` Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).