* [PATCH 1/9] ARM: enable secure platform-only erratas
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-10-05 15:53 ` Dmitry Osipenko
2017-10-05 18:16 ` Dmitry Osipenko
2017-07-20 0:29 ` [PATCH 3/9] arm: cache-l2x0: share l2x0_base Michał Mirosław
` (9 subsequent siblings)
10 siblings, 2 replies; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
Allow secure-only erratas to be used in multiarch kernel.
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/Kconfig | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a208bfe367b5..a1eff866550b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -696,6 +696,14 @@ config ARCH_MULTI_CPU_AUTO
endmenu
+config ARCH_ASSUME_SECURE_PLATFORM
+ bool "Enable ERRATAs using secure-only registers"
+ default !ARCH_MULTIPLATFORM
+ help
+ Allow erratas that need access to secure-only registers.
+
+ Beware: Resulting kernel won't boot on a CPU in non-Secure mode.
+
config ARCH_VIRT
bool "Dummy Virtual Machine"
depends on ARCH_MULTI_V7
@@ -984,7 +992,7 @@ config ARM_ERRATA_430973
config ARM_ERRATA_458693
bool "ARM errata: Processor deadlock when a false hazard is created"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 458693 Cortex-A8 (r2p0)
erratum. For very specific sequences of memory operations, it is
@@ -998,7 +1006,7 @@ config ARM_ERRATA_458693
config ARM_ERRATA_460075
bool "ARM errata: Data written to the L2 cache can be overwritten with stale data"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 460075 Cortex-A8 (r2p0)
erratum. Any asynchronous access to the L2 cache may encounter a
@@ -1011,7 +1019,7 @@ config ARM_ERRATA_460075
config ARM_ERRATA_742230
bool "ARM errata: DMB operation may be faulty"
depends on CPU_V7 && SMP
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 742230 Cortex-A9
(r1p0..r2p2) erratum. Under rare circumstances, a DMB instruction
@@ -1024,7 +1032,7 @@ config ARM_ERRATA_742230
config ARM_ERRATA_742231
bool "ARM errata: Incorrect hazard handling in the SCU may lead to data corruption"
depends on CPU_V7 && SMP
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 742231 Cortex-A9
(r2p0..r2p2) erratum. Under certain conditions, specific to the
@@ -1062,7 +1070,7 @@ config ARM_ERRATA_720789
config ARM_ERRATA_743622
bool "ARM errata: Faulty hazard checking in the Store Buffer may lead to data corruption"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 743622 Cortex-A9
(r2p*) erratum. Under very rare conditions, a faulty
@@ -1076,7 +1084,7 @@ config ARM_ERRATA_743622
config ARM_ERRATA_751472
bool "ARM errata: Interrupted ICIALLUIS may prevent completion of broadcasted operation"
depends on CPU_V7
- depends on !ARCH_MULTIPLATFORM
+ depends on ARCH_ASSUME_SECURE_PLATFORM
help
This option enables the workaround for the 751472 Cortex-A9 (prior
to r3p0) erratum. An interrupted ICIALLUIS operation may prevent the
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 1/9] ARM: enable secure platform-only erratas
2017-07-20 0:29 ` [PATCH 1/9] ARM: enable secure platform-only erratas Michał Mirosław
@ 2017-10-05 15:53 ` Dmitry Osipenko
2017-10-05 18:16 ` Dmitry Osipenko
1 sibling, 0 replies; 29+ messages in thread
From: Dmitry Osipenko @ 2017-10-05 15:53 UTC (permalink / raw)
To: linux-arm-kernel
On 20.07.2017 03:29, Micha? Miros?aw wrote:
> Allow secure-only erratas to be used in multiarch kernel.
>
> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> ---
> arch/arm/Kconfig | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a208bfe367b5..a1eff866550b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -696,6 +696,14 @@ config ARCH_MULTI_CPU_AUTO
>
> endmenu
>
> +config ARCH_ASSUME_SECURE_PLATFORM
What about ARCH_SECURE_PLATFORM?
> + bool "Enable ERRATAs using secure-only registers"
> + default !ARCH_MULTIPLATFORM
> + help
> + Allow erratas that need access to secure-only registers.
> +
> + Beware: Resulting kernel won't boot on a CPU in non-Secure mode.
> +
> config ARCH_VIRT
> bool "Dummy Virtual Machine"
> depends on ARCH_MULTI_V7
> @@ -984,7 +992,7 @@ config ARM_ERRATA_430973
> config ARM_ERRATA_458693
> bool "ARM errata: Processor deadlock when a false hazard is created"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 458693 Cortex-A8 (r2p0)
> erratum. For very specific sequences of memory operations, it is
> @@ -998,7 +1006,7 @@ config ARM_ERRATA_458693
> config ARM_ERRATA_460075
> bool "ARM errata: Data written to the L2 cache can be overwritten with stale data"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 460075 Cortex-A8 (r2p0)
> erratum. Any asynchronous access to the L2 cache may encounter a
> @@ -1011,7 +1019,7 @@ config ARM_ERRATA_460075
> config ARM_ERRATA_742230
> bool "ARM errata: DMB operation may be faulty"
> depends on CPU_V7 && SMP
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 742230 Cortex-A9
> (r1p0..r2p2) erratum. Under rare circumstances, a DMB instruction
> @@ -1024,7 +1032,7 @@ config ARM_ERRATA_742230
> config ARM_ERRATA_742231
> bool "ARM errata: Incorrect hazard handling in the SCU may lead to data corruption"
> depends on CPU_V7 && SMP
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 742231 Cortex-A9
> (r2p0..r2p2) erratum. Under certain conditions, specific to the
> @@ -1062,7 +1070,7 @@ config ARM_ERRATA_720789
> config ARM_ERRATA_743622
> bool "ARM errata: Faulty hazard checking in the Store Buffer may lead to data corruption"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 743622 Cortex-A9
> (r2p*) erratum. Under very rare conditions, a faulty
> @@ -1076,7 +1084,7 @@ config ARM_ERRATA_743622
> config ARM_ERRATA_751472
> bool "ARM errata: Interrupted ICIALLUIS may prevent completion of broadcasted operation"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 751472 Cortex-A9 (prior
> to r3p0) erratum. An interrupted ICIALLUIS operation may prevent the
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/9] ARM: enable secure platform-only erratas
2017-07-20 0:29 ` [PATCH 1/9] ARM: enable secure platform-only erratas Michał Mirosław
2017-10-05 15:53 ` Dmitry Osipenko
@ 2017-10-05 18:16 ` Dmitry Osipenko
2017-12-19 23:28 ` Russell King - ARM Linux
1 sibling, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2017-10-05 18:16 UTC (permalink / raw)
To: linux-arm-kernel
On 20.07.2017 03:29, Micha? Miros?aw wrote:
> Allow secure-only erratas to be used in multiarch kernel.
>
> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> ---
> arch/arm/Kconfig | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a208bfe367b5..a1eff866550b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -696,6 +696,14 @@ config ARCH_MULTI_CPU_AUTO
>
> endmenu
>
> +config ARCH_ASSUME_SECURE_PLATFORM
> + bool "Enable ERRATAs using secure-only registers"
> + default !ARCH_MULTIPLATFORM
I think default should always be "Yes" and this option shouldn't affect
multiplatform kernels. So a multiplatform kernel wouldn't be an option for your
device.
> + help
> + Allow erratas that need access to secure-only registers.
> +
> + Beware: Resulting kernel won't boot on a CPU in non-Secure mode.
> +
Probably better to say here something like that: Disable this option to avoid
CPU erratas that require access to secure-only registers. This might be needed
in a case of running under a secure monitor.
> config ARCH_VIRT
> bool "Dummy Virtual Machine"
> depends on ARCH_MULTI_V7
> @@ -984,7 +992,7 @@ config ARM_ERRATA_430973
> config ARM_ERRATA_458693
> bool "ARM errata: Processor deadlock when a false hazard is created"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 458693 Cortex-A8 (r2p0)
> erratum. For very specific sequences of memory operations, it is
> @@ -998,7 +1006,7 @@ config ARM_ERRATA_458693
> config ARM_ERRATA_460075
> bool "ARM errata: Data written to the L2 cache can be overwritten with stale data"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 460075 Cortex-A8 (r2p0)
> erratum. Any asynchronous access to the L2 cache may encounter a
> @@ -1011,7 +1019,7 @@ config ARM_ERRATA_460075
> config ARM_ERRATA_742230
> bool "ARM errata: DMB operation may be faulty"
> depends on CPU_V7 && SMP
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 742230 Cortex-A9
> (r1p0..r2p2) erratum. Under rare circumstances, a DMB instruction
> @@ -1024,7 +1032,7 @@ config ARM_ERRATA_742230
> config ARM_ERRATA_742231
> bool "ARM errata: Incorrect hazard handling in the SCU may lead to data corruption"
> depends on CPU_V7 && SMP
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 742231 Cortex-A9
> (r2p0..r2p2) erratum. Under certain conditions, specific to the
> @@ -1062,7 +1070,7 @@ config ARM_ERRATA_720789
> config ARM_ERRATA_743622
> bool "ARM errata: Faulty hazard checking in the Store Buffer may lead to data corruption"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 743622 Cortex-A9
> (r2p*) erratum. Under very rare conditions, a faulty
> @@ -1076,7 +1084,7 @@ config ARM_ERRATA_743622
> config ARM_ERRATA_751472
> bool "ARM errata: Interrupted ICIALLUIS may prevent completion of broadcasted operation"
> depends on CPU_V7
> - depends on !ARCH_MULTIPLATFORM
> + depends on ARCH_ASSUME_SECURE_PLATFORM
> help
> This option enables the workaround for the 751472 Cortex-A9 (prior
> to r3p0) erratum. An interrupted ICIALLUIS operation may prevent the
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/9] ARM: enable secure platform-only erratas
2017-10-05 18:16 ` Dmitry Osipenko
@ 2017-12-19 23:28 ` Russell King - ARM Linux
2017-12-20 12:49 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Russell King - ARM Linux @ 2017-12-19 23:28 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 05, 2017 at 09:16:12PM +0300, Dmitry Osipenko wrote:
> On 20.07.2017 03:29, Micha? Miros?aw wrote:
> > Allow secure-only erratas to be used in multiarch kernel.
> >
> > Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> > ---
> > arch/arm/Kconfig | 20 ++++++++++++++------
> > 1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index a208bfe367b5..a1eff866550b 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -696,6 +696,14 @@ config ARCH_MULTI_CPU_AUTO
> >
> > endmenu
> >
> > +config ARCH_ASSUME_SECURE_PLATFORM
> > + bool "Enable ERRATAs using secure-only registers"
> > + default !ARCH_MULTIPLATFORM
>
> I think default should always be "Yes" and this option shouldn't affect
> multiplatform kernels. So a multiplatform kernel wouldn't be an option
> for your device.
No, that changes the current behaviour.
Current behaviour is for the secure-only errata to be disabled when the
multi-platform option is enabled, because multi-platform kernels have to
be able to run in the non-secure world. Defaulting this option to "yes"
means that these errata become visible.
I have to wonder why you need this though - your patches seem to be
targetting a platform that runs in non-secure world, and enabling these
errata would make the kernel non-bootable on your platform.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/9] ARM: enable secure platform-only erratas
2017-12-19 23:28 ` Russell King - ARM Linux
@ 2017-12-20 12:49 ` Dmitry Osipenko
0 siblings, 0 replies; 29+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 12:49 UTC (permalink / raw)
To: linux-arm-kernel
On 20.12.2017 02:28, Russell King - ARM Linux wrote:
> On Thu, Oct 05, 2017 at 09:16:12PM +0300, Dmitry Osipenko wrote:
>> On 20.07.2017 03:29, Micha? Miros?aw wrote:
>>> Allow secure-only erratas to be used in multiarch kernel.
>>>
>>> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
>>> ---
>>> arch/arm/Kconfig | 20 ++++++++++++++------
>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index a208bfe367b5..a1eff866550b 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -696,6 +696,14 @@ config ARCH_MULTI_CPU_AUTO
>>>
>>> endmenu
>>>
>>> +config ARCH_ASSUME_SECURE_PLATFORM
>>> + bool "Enable ERRATAs using secure-only registers"
>>> + default !ARCH_MULTIPLATFORM
>>
>> I think default should always be "Yes" and this option shouldn't affect
>> multiplatform kernels. So a multiplatform kernel wouldn't be an option
>> for your device.
>
> No, that changes the current behaviour.
>
> Current behaviour is for the secure-only errata to be disabled when the
> multi-platform option is enabled, because multi-platform kernels have to
> be able to run in the non-secure world. Defaulting this option to "yes"
> means that these errata become visible.
Indeed, I got it inverted.
> I have to wonder why you need this though - your patches seem to be
> targetting a platform that runs in non-secure world, and enabling these
> errata would make the kernel non-bootable on your platform.
Perhaps because Micha? made the Tegra's CPU reset handler hardcoded to either
secure or to non-secure case based on the kernels configuration. I've showed how
we can get rid of that inflexibility in [0], maybe Micha? could pick up the idea
in the next iteration of the patches.
[0] https://marc.info/?l=linux-tegra&m=151371042522835
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/9] arm: cache-l2x0: share l2x0_base
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
2017-07-20 0:29 ` [PATCH 1/9] ARM: enable secure platform-only erratas Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-10-05 16:48 ` Mark Rutland
2017-07-20 0:29 ` [PATCH 2/9] arm: cache-l2x0: remove duplicate warning Michał Mirosław
` (8 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
Share l2x0_base between cache-l2x0 and pmu drivers. They are duplicates.
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/include/asm/hardware/cache-l2x0.h | 5 +++--
arch/arm/mm/cache-l2x0-pmu.c | 8 ++------
arch/arm/mm/cache-l2x0.c | 4 ++--
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 736292b42fca..492de655e4f3 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -167,11 +167,11 @@ static inline int l2x0_of_init(u32 aux_val, u32 aux_mask)
#endif
#ifdef CONFIG_CACHE_L2X0_PMU
-void l2x0_pmu_register(void __iomem *base, u32 part);
+void l2x0_pmu_register(u32 part);
void l2x0_pmu_suspend(void);
void l2x0_pmu_resume(void);
#else
-static inline void l2x0_pmu_register(void __iomem *base, u32 part) {}
+static inline void l2x0_pmu_register(u32 part) {}
static inline void l2x0_pmu_suspend(void) {}
static inline void l2x0_pmu_resume(void) {}
#endif
@@ -193,6 +193,7 @@ struct l2x0_regs {
unsigned long aux2_ctrl;
};
+extern void __iomem *l2x0_base;
extern struct l2x0_regs l2x0_saved_regs;
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm/mm/cache-l2x0-pmu.c b/arch/arm/mm/cache-l2x0-pmu.c
index 0a1e2280141f..1f36801d337f 100644
--- a/arch/arm/mm/cache-l2x0-pmu.c
+++ b/arch/arm/mm/cache-l2x0-pmu.c
@@ -29,7 +29,6 @@
#define PMU_NR_COUNTERS 2
-static void __iomem *l2x0_base;
static struct pmu *l2x0_pmu;
static cpumask_t pmu_cpu;
@@ -491,7 +490,7 @@ void l2x0_pmu_resume(void)
l2x0_pmu_enable(l2x0_pmu);
}
-void __init l2x0_pmu_register(void __iomem *base, u32 part)
+void __init l2x0_pmu_register(u32 part)
{
/*
* Determine whether we support the PMU, and choose the name for sysfs.
@@ -503,8 +502,7 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
* supported by this driver.
*
* We must defer registering the PMU until the perf subsystem is up and
- * running, so just stash the name and base, and leave that to another
- * initcall.
+ * running, so just stash the name, and leave that to another initcall.
*/
switch (part & L2X0_CACHE_ID_PART_MASK) {
case L2X0_CACHE_ID_PART_L220:
@@ -516,8 +514,6 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
default:
return;
}
-
- l2x0_base = base;
}
static __init int l2x0_pmu_init(void)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index ea1e70ff4568..bbfbc18399f9 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -48,13 +48,13 @@ struct l2c_init_data {
#define CACHE_LINE_SIZE 32
-static void __iomem *l2x0_base;
static const struct l2c_init_data *l2x0_data;
static DEFINE_RAW_SPINLOCK(l2x0_lock);
static u32 l2x0_way_mask; /* Bitmask of active ways */
static u32 l2x0_size;
static unsigned long sync_reg_offset = L2X0_CACHE_SYNC;
+void __iomem *l2x0_base;
struct l2x0_regs l2x0_saved_regs;
static bool l2x0_bresp_disable;
@@ -900,7 +900,7 @@ static int __init __l2c_init(const struct l2c_init_data *data,
pr_info("%s: CACHE_ID 0x%08x, AUX_CTRL 0x%08x\n",
data->type, cache_id, aux);
- l2x0_pmu_register(l2x0_base, cache_id);
+ l2x0_pmu_register(cache_id);
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 3/9] arm: cache-l2x0: share l2x0_base
2017-07-20 0:29 ` [PATCH 3/9] arm: cache-l2x0: share l2x0_base Michał Mirosław
@ 2017-10-05 16:48 ` Mark Rutland
2017-12-19 23:10 ` Russell King - ARM Linux
0 siblings, 1 reply; 29+ messages in thread
From: Mark Rutland @ 2017-10-05 16:48 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 20, 2017 at 02:29:23AM +0200, Micha? Miros?aw wrote:
> Share l2x0_base between cache-l2x0 and pmu drivers. They are duplicates.
This is not quite true, and this change introduces bugs.
The l2x0 pmu driver only sets its copy of the base for an l2x0 variant
it recognises:
> -void __init l2x0_pmu_register(void __iomem *base, u32 part)
> +void __init l2x0_pmu_register(u32 part)
> {
> /*
> * Determine whether we support the PMU, and choose the name for sysfs.
> @@ -503,8 +502,7 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
> * supported by this driver.
> *
> * We must defer registering the PMU until the perf subsystem is up and
> - * running, so just stash the name and base, and leave that to another
> - * initcall.
> + * running, so just stash the name, and leave that to another initcall.
> */
> switch (part & L2X0_CACHE_ID_PART_MASK) {
> case L2X0_CACHE_ID_PART_L220:
> @@ -516,8 +514,6 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
> default:
> return;
> }
> -
> - l2x0_base = base;
... note that we may return *before* setting l2x0_base.
This (deliberately) prevents us from erroneously registering a PMU for
l2x0 variants for which we don't know how to drive any IMPLEMENTATION
DEFINED PMU functionality.
This cahnge breaks that, and will result in l2x0_pmu_init() trying to
register a PMU with a NULL name.
So NAK to this change.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 3/9] arm: cache-l2x0: share l2x0_base
2017-10-05 16:48 ` Mark Rutland
@ 2017-12-19 23:10 ` Russell King - ARM Linux
0 siblings, 0 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2017-12-19 23:10 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 05, 2017 at 05:48:32PM +0100, Mark Rutland wrote:
> On Thu, Jul 20, 2017 at 02:29:23AM +0200, Micha? Miros?aw wrote:
> > Share l2x0_base between cache-l2x0 and pmu drivers. They are duplicates.
>
> This is not quite true, and this change introduces bugs.
>
> The l2x0 pmu driver only sets its copy of the base for an l2x0 variant
> it recognises:
>
> > -void __init l2x0_pmu_register(void __iomem *base, u32 part)
> > +void __init l2x0_pmu_register(u32 part)
> > {
> > /*
> > * Determine whether we support the PMU, and choose the name for sysfs.
> > @@ -503,8 +502,7 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
> > * supported by this driver.
> > *
> > * We must defer registering the PMU until the perf subsystem is up and
> > - * running, so just stash the name and base, and leave that to another
> > - * initcall.
> > + * running, so just stash the name, and leave that to another initcall.
> > */
> > switch (part & L2X0_CACHE_ID_PART_MASK) {
> > case L2X0_CACHE_ID_PART_L220:
> > @@ -516,8 +514,6 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
> > default:
> > return;
> > }
> > -
> > - l2x0_base = base;
>
> ... note that we may return *before* setting l2x0_base.
>
> This (deliberately) prevents us from erroneously registering a PMU for
> l2x0 variants for which we don't know how to drive any IMPLEMENTATION
> DEFINED PMU functionality.
>
> This cahnge breaks that, and will result in l2x0_pmu_init() trying to
> register a PMU with a NULL name.
>
> So NAK to this change.
Also I don't want the virtual base address of the L2 cache exposed to
the world, that's an invitation for people to do what they did years
ago, and throw code left right and centre into the tree that accesses
the L2 cache directly from all sorts of places - the result was very
much a mess that took a big patch series to sort out.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/9] arm: cache-l2x0: remove duplicate warning
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
2017-07-20 0:29 ` [PATCH 1/9] ARM: enable secure platform-only erratas Michał Mirosław
2017-07-20 0:29 ` [PATCH 3/9] arm: cache-l2x0: share l2x0_base Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-07-20 0:29 ` [PATCH 5/9] ARM: trusted_foundations: announce firmware version Michał Mirosław
` (7 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
Warning abound changing of AUX register in l2x0_of_init() is later
repeated in __l2c_init(). Move whole thing over to __l2c_init().
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/mm/cache-l2x0.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 808efbb89b88..ea1e70ff4568 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -819,6 +819,8 @@ static int __init __l2c_init(const struct l2c_init_data *data,
if (old_aux != aux)
pr_warn("L2C: DT/platform modifies aux control register: 0x%08x -> 0x%08x\n",
old_aux, aux);
+ else if (aux_mask != ~0U && aux_val != 0)
+ pr_alert("L2C: platform provided aux values match the hardware, so have no effect. Please remove them.\n");
/* Determine the number of ways */
switch (cache_id & L2X0_CACHE_ID_PART_MASK) {
@@ -1755,7 +1757,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
const struct l2c_init_data *data;
struct device_node *np;
struct resource res;
- u32 cache_id, old_aux;
+ u32 cache_id;
u32 cache_level = 2;
bool nosync = false;
@@ -1778,14 +1780,6 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
of_property_read_bool(np, "arm,io-coherent"))
data = &of_l2c310_coherent_data;
- old_aux = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
- if (old_aux != ((old_aux & aux_mask) | aux_val)) {
- pr_warn("L2C: platform modifies aux control register: 0x%08x -> 0x%08x\n",
- old_aux, (old_aux & aux_mask) | aux_val);
- } else if (aux_mask != ~0U && aux_val != 0) {
- pr_alert("L2C: platform provided aux values match the hardware, so have no effect. Please remove them.\n");
- }
-
/* All L2 caches are unified, so this property should be specified */
if (!of_property_read_bool(np, "cache-unified"))
pr_err("L2C: device tree omits to specify unified cache\n");
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 5/9] ARM: trusted_foundations: announce firmware version
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (2 preceding siblings ...)
2017-07-20 0:29 ` [PATCH 2/9] arm: cache-l2x0: remove duplicate warning Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-07-20 0:29 ` [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops Michał Mirosław
` (6 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
Announce Trusted Foundations version for debugging and documentation.
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/firmware/trusted_foundations.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index 81ff71b87438..105655802dfd 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -30,6 +30,8 @@
#define TF_CACHE_FLUSH 2
#define TF_CACHE_REENABLE 4
+#define TF_GET_PROTOCOL_VERSION 0xfffffffb
+
#define TF_CPU_PM 0xfffffffc
#define TF_CPU_PM_S3 0xffffffe3
#define TF_CPU_PM_S2 0xffffffe6
@@ -39,7 +41,7 @@
static unsigned long cpu_boot_addr;
-static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
+static u64 __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
{
asm volatile(
".arch_extension sec\n\t"
@@ -54,6 +56,7 @@ static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
:
: "r" (type), "r" (arg1), "r" (arg2)
: "memory");
+ return 0; /* silence gcc warning */
}
static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
@@ -106,6 +109,11 @@ static int tf_init_cache(void)
}
#endif /* CONFIG_CACHE_L2X0 */
+static u32 tf_get_protocol_version(void)
+{
+ return tf_generic_smc(TF_GET_PROTOCOL_VERSION, 0, 0) >> 48;
+}
+
static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
.prepare_idle = tf_prepare_idle,
@@ -116,6 +124,12 @@ static const struct firmware_ops trusted_foundations_ops = {
void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
{
+ unsigned int protocol = tf_get_protocol_version();
+
+ pr_info("TF: version %u.%u, protocol %u.%u\n",
+ pd->version_major, pd->version_minor,
+ protocol >> 8, protocol & 0xFF);
+
/*
* we are not using version information for now since currently
* supported SMCs are compatible with all TF releases
@@ -141,5 +155,6 @@ void of_register_trusted_foundations(void)
&pdata.version_minor);
if (err != 0)
panic("Trusted Foundation: missing version-minor property\n");
+
register_trusted_foundations(&pdata);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (3 preceding siblings ...)
2017-07-20 0:29 ` [PATCH 5/9] ARM: trusted_foundations: announce firmware version Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-10-05 16:24 ` Dmitry Osipenko
2017-12-19 18:56 ` Dmitry Osipenko
2017-07-20 0:29 ` [PATCH 6/9] ARM: init: update secondary_data register documentation Michał Mirosław
` (5 subsequent siblings)
10 siblings, 2 replies; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
Use firmware_ops to provide hook for cache initialization through
Trusted Foundations firmware, as some writes need Secure mode.
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/firmware/trusted_foundations.c | 46 ++++++++++++++++++++++++++++++
arch/arm/include/asm/hardware/cache-l2x0.h | 1 +
arch/arm/mm/cache-l2x0.c | 10 ++++++-
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index 3fb1b5a1dce9..81ff71b87438 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -17,11 +17,19 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/of.h>
+#include <asm/io.h>
#include <asm/firmware.h>
+#include <asm/outercache.h>
+#include <asm/hardware/cache-l2x0.h>
#include <asm/trusted_foundations.h>
+#define TF_CACHE_MAINT 0xfffff100
#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+#define TF_CACHE_INIT 1
+#define TF_CACHE_FLUSH 2
+#define TF_CACHE_REENABLE 4
+
#define TF_CPU_PM 0xfffffffc
#define TF_CPU_PM_S3 0xffffffe3
#define TF_CPU_PM_S2 0xffffffe6
@@ -63,9 +71,47 @@ static int tf_prepare_idle(void)
return 0;
}
+#ifdef CONFIG_CACHE_L2X0
+static void tf_write_sec(unsigned long val, unsigned reg)
+{
+ unsigned long cur = readl_relaxed(l2x0_base + reg);
+
+ pr_warn("TF: ignoring write_sec[0x%x]: 0x%08lx -> 0x%08lx\n", reg, cur, val);
+}
+
+static void tf_disable_cache(void)
+{
+ tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_FLUSH, l2x0_way_mask);
+}
+
+static void tf_resume_cache(void)
+{
+ unsigned long aux_val = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
+ tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_REENABLE, aux_val);
+}
+
+static void tf_configure_cache(const struct l2x0_regs *regs)
+{
+ outer_cache.disable = tf_disable_cache;
+ outer_cache.resume = tf_resume_cache;
+}
+
+static int tf_init_cache(void)
+{
+ tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_INIT, 0);
+
+ outer_cache.write_sec = tf_write_sec;
+ outer_cache.configure = tf_configure_cache;
+ return 0;
+}
+#endif /* CONFIG_CACHE_L2X0 */
+
static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
.prepare_idle = tf_prepare_idle,
+#ifdef CONFIG_CACHE_L2X0
+ .l2x0_init = tf_init_cache,
+#endif
};
void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
index 492de655e4f3..665eb0758417 100644
--- a/arch/arm/include/asm/hardware/cache-l2x0.h
+++ b/arch/arm/include/asm/hardware/cache-l2x0.h
@@ -194,6 +194,7 @@ struct l2x0_regs {
};
extern void __iomem *l2x0_base;
+extern u32 l2x0_way_mask; /* Bitmask of active ways */
extern struct l2x0_regs l2x0_saved_regs;
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index bbfbc18399f9..f1268e9b35f0 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -30,6 +30,7 @@
#include <asm/cp15.h>
#include <asm/cputype.h>
#include <asm/hardware/cache-l2x0.h>
+#include <asm/firmware.h>
#include "cache-tauros3.h"
#include "cache-aurora-l2.h"
@@ -37,6 +38,7 @@ struct l2c_init_data {
const char *type;
unsigned way_size_0;
unsigned num_lock;
+ void (*init)(void __iomem *, u32 *, u32 *);
void (*of_parse)(const struct device_node *, u32 *, u32 *);
void (*enable)(void __iomem *, unsigned);
void (*fixup)(void __iomem *, u32, struct outer_cache_fns *);
@@ -50,11 +52,11 @@ struct l2c_init_data {
static const struct l2c_init_data *l2x0_data;
static DEFINE_RAW_SPINLOCK(l2x0_lock);
-static u32 l2x0_way_mask; /* Bitmask of active ways */
static u32 l2x0_size;
static unsigned long sync_reg_offset = L2X0_CACHE_SYNC;
void __iomem *l2x0_base;
+u32 l2x0_way_mask; /* Bitmask of active ways */
struct l2x0_regs l2x0_saved_regs;
static bool l2x0_bresp_disable;
@@ -1760,6 +1762,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
u32 cache_id;
u32 cache_level = 2;
bool nosync = false;
+ int err;
np = of_find_matching_node(NULL, l2x0_ids);
if (!np)
@@ -1792,6 +1795,11 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
nosync = of_property_read_bool(np, "arm,outer-sync-disable");
+ /* Call firmware init */
+ err = call_firmware_op(l2x0_init);
+ if (err && err != -ENOSYS)
+ return err;
+
/* Read back current (default) hardware configuration */
if (data->save)
data->save(l2x0_base);
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops
2017-07-20 0:29 ` [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops Michał Mirosław
@ 2017-10-05 16:24 ` Dmitry Osipenko
2017-12-19 18:56 ` Dmitry Osipenko
1 sibling, 0 replies; 29+ messages in thread
From: Dmitry Osipenko @ 2017-10-05 16:24 UTC (permalink / raw)
To: linux-arm-kernel
On 20.07.2017 03:29, Micha? Miros?aw wrote:
> Use firmware_ops to provide hook for cache initialization through
> Trusted Foundations firmware, as some writes need Secure mode.
>
> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> ---
> arch/arm/firmware/trusted_foundations.c | 46 ++++++++++++++++++++++++++++++
> arch/arm/include/asm/hardware/cache-l2x0.h | 1 +
> arch/arm/mm/cache-l2x0.c | 10 ++++++-
> 3 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
> index 3fb1b5a1dce9..81ff71b87438 100644
> --- a/arch/arm/firmware/trusted_foundations.c
> +++ b/arch/arm/firmware/trusted_foundations.c
> @@ -17,11 +17,19 @@
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/of.h>
> +#include <asm/io.h>
> #include <asm/firmware.h>
> +#include <asm/outercache.h>
> +#include <asm/hardware/cache-l2x0.h>
> #include <asm/trusted_foundations.h>
>
> +#define TF_CACHE_MAINT 0xfffff100
> #define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
>
> +#define TF_CACHE_INIT 1
> +#define TF_CACHE_FLUSH 2
> +#define TF_CACHE_REENABLE 4
> +
> #define TF_CPU_PM 0xfffffffc
> #define TF_CPU_PM_S3 0xffffffe3
> #define TF_CPU_PM_S2 0xffffffe6
> @@ -63,9 +71,47 @@ static int tf_prepare_idle(void)
> return 0;
> }
>
> +#ifdef CONFIG_CACHE_L2X0
> +static void tf_write_sec(unsigned long val, unsigned reg)
> +{
> + unsigned long cur = readl_relaxed(l2x0_base + reg);
> +
> + pr_warn("TF: ignoring write_sec[0x%x]: 0x%08lx -> 0x%08lx\n", reg, cur, val);
> +}
> +
> +static void tf_disable_cache(void)
> +{
> + tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_FLUSH, l2x0_way_mask);
> +}
> +
> +static void tf_resume_cache(void)
> +{
> + unsigned long aux_val = readl_relaxed(l2x0_base + L2X0_AUX_CTRL);
> + tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_REENABLE, aux_val);
> +}
> +
> +static void tf_configure_cache(const struct l2x0_regs *regs)
> +{
> + outer_cache.disable = tf_disable_cache;
> + outer_cache.resume = tf_resume_cache;
> +}
> +
> +static int tf_init_cache(void)
> +{
> + tf_generic_smc(TF_CACHE_MAINT, TF_CACHE_INIT, 0);
> +
> + outer_cache.write_sec = tf_write_sec;
> + outer_cache.configure = tf_configure_cache;
> + return 0;
> +}
> +#endif /* CONFIG_CACHE_L2X0 */
> +
> static const struct firmware_ops trusted_foundations_ops = {
> .set_cpu_boot_addr = tf_set_cpu_boot_addr,
> .prepare_idle = tf_prepare_idle,
> +#ifdef CONFIG_CACHE_L2X0
> + .l2x0_init = tf_init_cache,
> +#endif
> };
>
> void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
> diff --git a/arch/arm/include/asm/hardware/cache-l2x0.h b/arch/arm/include/asm/hardware/cache-l2x0.h
> index 492de655e4f3..665eb0758417 100644
> --- a/arch/arm/include/asm/hardware/cache-l2x0.h
> +++ b/arch/arm/include/asm/hardware/cache-l2x0.h
> @@ -194,6 +194,7 @@ struct l2x0_regs {
> };
>
> extern void __iomem *l2x0_base;
> +extern u32 l2x0_way_mask; /* Bitmask of active ways */
> extern struct l2x0_regs l2x0_saved_regs;
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index bbfbc18399f9..f1268e9b35f0 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -30,6 +30,7 @@
> #include <asm/cp15.h>
> #include <asm/cputype.h>
> #include <asm/hardware/cache-l2x0.h>
> +#include <asm/firmware.h>
> #include "cache-tauros3.h"
> #include "cache-aurora-l2.h"
>
> @@ -37,6 +38,7 @@ struct l2c_init_data {
> const char *type;
> unsigned way_size_0;
> unsigned num_lock;
> + void (*init)(void __iomem *, u32 *, u32 *);
> void (*of_parse)(const struct device_node *, u32 *, u32 *);
> void (*enable)(void __iomem *, unsigned);
> void (*fixup)(void __iomem *, u32, struct outer_cache_fns *);
> @@ -50,11 +52,11 @@ struct l2c_init_data {
>
> static const struct l2c_init_data *l2x0_data;
> static DEFINE_RAW_SPINLOCK(l2x0_lock);
> -static u32 l2x0_way_mask; /* Bitmask of active ways */
> static u32 l2x0_size;
> static unsigned long sync_reg_offset = L2X0_CACHE_SYNC;
>
> void __iomem *l2x0_base;
> +u32 l2x0_way_mask; /* Bitmask of active ways */
> struct l2x0_regs l2x0_saved_regs;
>
> static bool l2x0_bresp_disable;
> @@ -1760,6 +1762,7 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
> u32 cache_id;
> u32 cache_level = 2;
> bool nosync = false;
> + int err;
>
> np = of_find_matching_node(NULL, l2x0_ids);
> if (!np)
> @@ -1792,6 +1795,11 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>
> nosync = of_property_read_bool(np, "arm,outer-sync-disable");
>
> + /* Call firmware init */
What about to make this comment a bit more explanatory? Add couple words about
what firmware initializes, maybe it enables access from CPU.. of course if you
have any clue.
> + err = call_firmware_op(l2x0_init);
> + if (err && err != -ENOSYS)
> + return err;
> +
> /* Read back current (default) hardware configuration */
> if (data->save)
> data->save(l2x0_base);
>
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops
2017-07-20 0:29 ` [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops Michał Mirosław
2017-10-05 16:24 ` Dmitry Osipenko
@ 2017-12-19 18:56 ` Dmitry Osipenko
1 sibling, 0 replies; 29+ messages in thread
From: Dmitry Osipenko @ 2017-12-19 18:56 UTC (permalink / raw)
To: linux-arm-kernel
On 20.07.2017 03:29, Micha? Miros?aw wrote:
> Use firmware_ops to provide hook for cache initialization through
> Trusted Foundations firmware, as some writes need Secure mode.
>
> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> ---
I've tested this patch on my T30, which I ported to current linux-next, and it
works excellent. Micha?, are you going to re-spin this patch?
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/9] ARM: init: update secondary_data register documentation
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (4 preceding siblings ...)
2017-07-20 0:29 ` [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-12-19 23:16 ` Russell King - ARM Linux
2017-07-20 0:29 ` [PATCH 8/9] ARM: tegra: avoid touching Secure registers in reset handler Michał Mirosław
` (4 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
Fix secondary_data pointer register and document other
occurrences in SMP boot code.
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/kernel/head.S | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 04286fd9e09c..68b13db86bef 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -413,7 +413,7 @@ ENDPROC(secondary_startup)
ENDPROC(secondary_startup_arm)
/*
- * r6 = &secondary_data
+ * r7 = &secondary_data
*/
ENTRY(__secondary_switched)
ldr sp, [r7, #12] @ get secondary_data.stack
@@ -443,6 +443,7 @@ __secondary_data:
* r2 = atags or dtb pointer
* r4 = TTBR pointer (low word)
* r5 = TTBR pointer (high word if LPAE)
+ * r7 = secondary_data pointer (SMP)
* r9 = processor ID
* r13 = *virtual* address to jump to upon completion
*/
@@ -480,6 +481,7 @@ ENDPROC(__enable_mmu)
* r0 = cp#15 control register
* r1 = machine ID
* r2 = atags or dtb pointer
+ * r7 = secondary_data pointer (SMP)
* r9 = processor ID
* r13 = *virtual* address to jump to upon completion
*
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 8/9] ARM: tegra: avoid touching Secure registers in reset handler
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (5 preceding siblings ...)
2017-07-20 0:29 ` [PATCH 6/9] ARM: init: update secondary_data register documentation Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-07-20 0:29 ` [PATCH 7/9] ARM: tegra: enable cache via TF Michał Mirosław
` (3 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
This allows secondary CPUs to boot with Trusted Foundations firmware.
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/mach-tegra/reset-handler.S | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index aae7f5961563..f642032a5a08 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -143,6 +143,8 @@ ENTRY(__tegra_cpu_reset_handler)
cpsid aif, 0x13 @ SVC mode, interrupts disabled
tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
+
+#ifdef CONFIG_ARCH_ASSUME_SECURE_PLATFORM
#ifdef CONFIG_ARCH_TEGRA_2x_SOC
t20_check:
cmp r6, #TEGRA20
@@ -172,6 +174,7 @@ t30_errata:
b after_errata
after_t30_check:
#endif
+#endif /* CONFIG_ARCH_ASSUME_SECURE_PLATFORM */
after_errata:
mrc p15, 0, r10, c0, c0, 5 @ MPIDR
and r10, r10, #0x3 @ R10 = CPU number
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 7/9] ARM: tegra: enable cache via TF
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (6 preceding siblings ...)
2017-07-20 0:29 ` [PATCH 8/9] ARM: tegra: avoid touching Secure registers in reset handler Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-12-19 19:07 ` Dmitry Osipenko
2017-12-19 23:21 ` Russell King - ARM Linux
2017-07-20 0:29 ` [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Michał Mirosław
` (2 subsequent siblings)
10 siblings, 2 replies; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
Cache enable needs to go via firmware call with TF running.
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
arch/arm/mach-tegra/reset-handler.S | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index 805f306fa6f7..aae7f5961563 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -78,8 +78,20 @@ ENTRY(tegra_resume)
orr r1, r1, #1
str r1, [r0]
#endif
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+ adr r3, __tegra_smc_stack
+ stmia r3, {r4-r12, sp, lr}
-#ifdef CONFIG_CACHE_L2X0
+ mov r0, #3 // local wake
+ mov r3, #0
+ mov r4, #0
+ dsb
+ .arch_extension sec
+ smc #0
+
+ adr r3, __tegra_smc_stack
+ ldmia r3, {r4-r12, sp, pc}
+#elif defined(CONFIG_CACHE_L2X0)
/* L2 cache resume & re-enable */
bl l2c310_early_resume
#endif
@@ -92,6 +104,16 @@ end_ca9_scu_l2_resume:
ENDPROC(tegra_resume)
#endif
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+ .align L1_CACHE_SHIFT
+ .type __tegra_smc_stack, %object
+__tegra_smc_stack:
+ .rept 11
+ .long 0
+ .endr
+ .size __tegra_smc_stack, . - __tegra_smc_stack
+#endif /* CONFIG_TRUSTED_FOUNDATIONS */
+
.align L1_CACHE_SHIFT
ENTRY(__tegra_cpu_reset_handler_start)
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 7/9] ARM: tegra: enable cache via TF
2017-07-20 0:29 ` [PATCH 7/9] ARM: tegra: enable cache via TF Michał Mirosław
@ 2017-12-19 19:07 ` Dmitry Osipenko
2017-12-19 23:21 ` Russell King - ARM Linux
1 sibling, 0 replies; 29+ messages in thread
From: Dmitry Osipenko @ 2017-12-19 19:07 UTC (permalink / raw)
To: linux-arm-kernel
On 20.07.2017 03:29, Micha? Miros?aw wrote:
> Cache enable needs to go via firmware call with TF running.
>
> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> ---
Perhaps we can unify secure and non-secure modes. The code below works on both
secure-t30 and nonsecure-t20, all CPU's booted and working fine.
The "ARM: enable secure platform-only erratas" patch isn't needed in this case.
diff --git a/arch/arm/mach-tegra/reset-handler.S
b/arch/arm/mach-tegra/reset-handler.S
index 805f306fa6f7..9a92bbf8b5b0 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -80,6 +80,28 @@ ENTRY(tegra_resume)
#endif
#ifdef CONFIG_CACHE_L2X0
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+ adr r3, __tegra_cpu_reset_handler_data
+ ldr r0, [r3, #RESET_DATA(TF_PRESENT)]
+ cmp r0, #0
+ beq ca9_scu_l2_resume
+
+ adr r3, __tegra_smc_stack
+ stmia r3, {r4-r12, sp, lr}
+
+ mov r0, #3 // local wake
+ mov r3, #0
+ mov r4, #0
+ dsb
+ .arch_extension sec
+ smc #0
+
+ adr r3, __tegra_smc_stack
+ ldmia r3, {r4-r12, sp, pc}
+
+ b end_ca9_scu_l2_resume
+ca9_scu_l2_resume:
+#endif
/* L2 cache resume & re-enable */
bl l2c310_early_resume
#endif
@@ -92,6 +114,16 @@ end_ca9_scu_l2_resume:
ENDPROC(tegra_resume)
#endif
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+ .align L1_CACHE_SHIFT
+ .type __tegra_smc_stack, %object
+__tegra_smc_stack:
+ .rept 11
+ .long 0
+ .endr
+ .size __tegra_smc_stack, . - __tegra_smc_stack
+#endif /* CONFIG_TRUSTED_FOUNDATIONS */
+
.align L1_CACHE_SHIFT
ENTRY(__tegra_cpu_reset_handler_start)
@@ -121,6 +153,12 @@ ENTRY(__tegra_cpu_reset_handler)
cpsid aif, 0x13 @ SVC mode, interrupts disabled
tegra_get_soc_id TEGRA_APB_MISC_BASE, r6
+
+ adr r5, __tegra_cpu_reset_handler_data
+ ldr r0, [r5, #RESET_DATA(TF_PRESENT)]
+ cmp r0, #0
+ bne after_errata
+
#ifdef CONFIG_ARCH_TEGRA_2x_SOC
t20_check:
cmp r6, #TEGRA20
@@ -285,6 +323,10 @@ __tegra_cpu_reset_handler_data:
.equ __tegra20_cpu1_resettable_status_offset, \
. - __tegra_cpu_reset_handler_start
.byte 0
+ .align 4
+ .globl __tegra_tf_present
+ .equ __tegra_tf_present, . - __tegra_cpu_reset_handler_start
+ .long 0
.align L1_CACHE_SHIFT
ENTRY(__tegra_cpu_reset_handler_end)
diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index dc558892753c..9b6558a69308 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -18,6 +18,7 @@
#include <linux/cpumask.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/of.h>
#include <soc/tegra/fuse.h>
@@ -89,6 +90,15 @@ static void __init tegra_cpu_reset_handler_enable(void)
void __init tegra_cpu_reset_handler_init(void)
{
+#ifdef CONFIG_TRUSTED_FOUNDATIONS
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "tlm,trusted-foundations");
+ if (np) {
+ __tegra_cpu_reset_handler_data[TEGRA_RESET_TF_PRESENT] = true;
+ of_node_put(np);
+ }
+#endif
#ifdef CONFIG_SMP
__tegra_cpu_reset_handler_data[TEGRA_RESET_MASK_PRESENT] =
diff --git a/arch/arm/mach-tegra/reset.h b/arch/arm/mach-tegra/reset.h
index 9c479c7925b8..0d9ddc022ece 100644
--- a/arch/arm/mach-tegra/reset.h
+++ b/arch/arm/mach-tegra/reset.h
@@ -25,7 +25,9 @@
#define TEGRA_RESET_STARTUP_SECONDARY 3
#define TEGRA_RESET_STARTUP_LP2 4
#define TEGRA_RESET_STARTUP_LP1 5
-#define TEGRA_RESET_DATA_SIZE 6
+#define TEGRA_RESET_RESETTABLE_STATUS 6
+#define TEGRA_RESET_TF_PRESENT 7
+#define TEGRA_RESET_DATA_SIZE 8
#ifndef __ASSEMBLY__
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 7/9] ARM: tegra: enable cache via TF
2017-07-20 0:29 ` [PATCH 7/9] ARM: tegra: enable cache via TF Michał Mirosław
2017-12-19 19:07 ` Dmitry Osipenko
@ 2017-12-19 23:21 ` Russell King - ARM Linux
1 sibling, 0 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2017-12-19 23:21 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 20, 2017 at 02:29:25AM +0200, Micha? Miros?aw wrote:
> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> index 805f306fa6f7..aae7f5961563 100644
> --- a/arch/arm/mach-tegra/reset-handler.S
> +++ b/arch/arm/mach-tegra/reset-handler.S
> @@ -78,8 +78,20 @@ ENTRY(tegra_resume)
> orr r1, r1, #1
> str r1, [r0]
> #endif
> +#ifdef CONFIG_TRUSTED_FOUNDATIONS
> + adr r3, __tegra_smc_stack
> + stmia r3, {r4-r12, sp, lr}
>
> -#ifdef CONFIG_CACHE_L2X0
> + mov r0, #3 // local wake
> + mov r3, #0
> + mov r4, #0
> + dsb
> + .arch_extension sec
> + smc #0
> +
> + adr r3, __tegra_smc_stack
> + ldmia r3, {r4-r12, sp, pc}
You don't want to jump to the 'lr' value previously stacked here. I
also wonder whether you need all this stacking, or whether you're
just doing it because you don't know whether its necessary. From
what I can see, the only register that this code cares about is r8,
although it would be wise to place a comment in the code if the smc
call corrupts the other registers.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (7 preceding siblings ...)
2017-07-20 0:29 ` [PATCH 7/9] ARM: tegra: enable cache via TF Michał Mirosław
@ 2017-07-20 0:29 ` Michał Mirosław
2017-07-20 12:45 ` Jon Hunter
2017-07-20 7:48 ` [PATCH 0/9] Bringing Asus TF300T support to mainline Mikko Perttunen
2017-10-04 21:25 ` Michał Mirosław
10 siblings, 1 reply; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 0:29 UTC (permalink / raw)
To: linux-arm-kernel
This removes unnecessary lock causing following BUG splat:
BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
no locks held by swapper/0/0.
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
[<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
[<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
[<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
[<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
[<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
[<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
[<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
[<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
[<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
[<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
[<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)
Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
---
drivers/soc/tegra/pmc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..3e6ec9fdba41 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
if (!tegra_powergate_is_valid(id))
return -EINVAL;
- mutex_lock(&pmc->powergates_lock);
status = tegra_powergate_state(id);
- mutex_unlock(&pmc->powergates_lock);
return status;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle
2017-07-20 0:29 ` [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Michał Mirosław
@ 2017-07-20 12:45 ` Jon Hunter
2017-07-20 16:28 ` Michał Mirosław
0 siblings, 1 reply; 29+ messages in thread
From: Jon Hunter @ 2017-07-20 12:45 UTC (permalink / raw)
To: linux-arm-kernel
On 20/07/17 01:29, Micha? Miros?aw wrote:
> This removes unnecessary lock causing following BUG splat:
>
> BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
> in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
> no locks held by swapper/0/0.
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> [<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
> [<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
> [<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
> [<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
> [<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
> [<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
> [<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
> [<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
> [<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
> [<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
> [<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
> [<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)
>
> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> ---
> drivers/soc/tegra/pmc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index e233dd5dcab3..3e6ec9fdba41 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
> if (!tegra_powergate_is_valid(id))
> return -EINVAL;
>
> - mutex_lock(&pmc->powergates_lock);
> status = tegra_powergate_state(id);
> - mutex_unlock(&pmc->powergates_lock);
>
> return status;
> }
Thanks for the fix. However, I would prefer that we fix this the following
way ...
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index a2d163f759b4..546d2b121c39 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void)
cpu_rst_status = readl(clk_base +
TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
- cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) ||
- tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3);
+ cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
+ tegra_pmc_cpu_is_powered(2) ||
+ tegra_pmc_cpu_is_powered(3);
if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
return false;
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3..f61371ea3fe0 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid)
if (id < 0)
return false;
- return tegra_powergate_is_powered(id);
+ return tegra_powergate_state(id);
}
Could you try the above and make sure that this works?
I would prefer to leave the mutex in tegra_powergate_is_powered() so it could
be used by any driver. Or maybe we could even get rid of
tegra_powergate_is_powered() if we don't really need it.
Cheers
Jon
--
nvpublic
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle
2017-07-20 12:45 ` Jon Hunter
@ 2017-07-20 16:28 ` Michał Mirosław
2017-07-21 8:15 ` Jon Hunter
0 siblings, 1 reply; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 16:28 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 20, 2017 at 01:45:12PM +0100, Jon Hunter wrote:
>
> On 20/07/17 01:29, Micha? Miros?aw wrote:
> > This removes unnecessary lock causing following BUG splat:
> >
> > BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
> > in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
> > no locks held by swapper/0/0.
> > CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
> > Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> > [<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
> > [<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
> > [<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
> > [<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
> > [<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
> > [<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
> > [<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
> > [<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
> > [<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
> > [<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
> > [<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
> > [<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)
> >
> > Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
> > ---
> > drivers/soc/tegra/pmc.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> > index e233dd5dcab3..3e6ec9fdba41 100644
> > --- a/drivers/soc/tegra/pmc.c
> > +++ b/drivers/soc/tegra/pmc.c
> > @@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
> > if (!tegra_powergate_is_valid(id))
> > return -EINVAL;
> >
> > - mutex_lock(&pmc->powergates_lock);
> > status = tegra_powergate_state(id);
> > - mutex_unlock(&pmc->powergates_lock);
> >
> > return status;
> > }
>
> Thanks for the fix. However, I would prefer that we fix this the following
> way ...
>
> diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
> index a2d163f759b4..546d2b121c39 100644
> --- a/drivers/clk/tegra/clk-tegra30.c
> +++ b/drivers/clk/tegra/clk-tegra30.c
> @@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void)
>
> cpu_rst_status = readl(clk_base +
> TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
> - cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) ||
> - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) ||
> - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3);
> + cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
> + tegra_pmc_cpu_is_powered(2) ||
> + tegra_pmc_cpu_is_powered(3);
>
> if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
> return false;
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index e233dd5dcab3..f61371ea3fe0 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid)
> if (id < 0)
> return false;
>
> - return tegra_powergate_is_powered(id);
> + return tegra_powergate_state(id);
> }
>
>
> Could you try the above and make sure that this works?
Something ate TABs in your patch, but yes, it works.
> I would prefer to leave the mutex in tegra_powergate_is_powered() so it could
> be used by any driver. Or maybe we could even get rid of
> tegra_powergate_is_powered() if we don't really need it.
This mutex does not protect anything here, though - there is only one register
read inside the locked section, and after unlock other CPU might change
it before the read value gets returned.
Code size-wise it would be better to remove tegra_pmc_cpu_is_powered()
instead.
Best Regards,
Micha??Miros?aw
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle
2017-07-20 16:28 ` Michał Mirosław
@ 2017-07-21 8:15 ` Jon Hunter
0 siblings, 0 replies; 29+ messages in thread
From: Jon Hunter @ 2017-07-21 8:15 UTC (permalink / raw)
To: linux-arm-kernel
On 20/07/17 17:28, Micha? Miros?aw wrote:
> On Thu, Jul 20, 2017 at 01:45:12PM +0100, Jon Hunter wrote:
>>
>> On 20/07/17 01:29, Micha? Miros?aw wrote:
>>> This removes unnecessary lock causing following BUG splat:
>>>
>>> BUG: sleeping function called from invalid context at /linux/kernel/locking/mutex.c:747
>>> in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
>>> no locks held by swapper/0/0.
>>> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.13.0-rc1mq-00016-gbf3f627c6b2b #1
>>> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
>>> [<b0110a14>] (unwind_backtrace) from [<b010c710>] (show_stack+0x18/0x1c)
>>> [<b010c710>] (show_stack) from [<b068ada8>] (dump_stack+0x84/0x98)
>>> [<b068ada8>] (dump_stack) from [<b014ff54>] (___might_sleep+0x158/0x17c)
>>> [<b014ff54>] (___might_sleep) from [<b06a15c4>] (__mutex_lock+0x34/0x9a0)
>>> [<b06a15c4>] (__mutex_lock) from [<b06a30dc>] (mutex_lock_nested+0x24/0x2c)
>>> [<b06a30dc>] (mutex_lock_nested) from [<b041f6d8>] (tegra_powergate_is_powered+0x40/0xa4)
>>> [<b041f6d8>] (tegra_powergate_is_powered) from [<b04197e8>] (tegra30_cpu_rail_off_ready+0x28/0x6c)
>>> [<b04197e8>] (tegra30_cpu_rail_off_ready) from [<b011c73c>] (tegra30_idle_lp2+0x70/0x114)
>>> [<b011c73c>] (tegra30_idle_lp2) from [<b0516080>] (cpuidle_enter_state+0x12c/0x47c)
>>> [<b0516080>] (cpuidle_enter_state) from [<b0163394>] (do_idle+0x1b4/0x204)
>>> [<b0163394>] (do_idle) from [<b016368c>] (cpu_startup_entry+0x20/0x24)
>>> [<b016368c>] (cpu_startup_entry) from [<b0900dd8>] (start_kernel+0x39c/0x3a8)
>>>
>>> Signed-off-by: Micha? Miros?aw <mirq-linux@rere.qmqm.pl>
>>> ---
>>> drivers/soc/tegra/pmc.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>> index e233dd5dcab3..3e6ec9fdba41 100644
>>> --- a/drivers/soc/tegra/pmc.c
>>> +++ b/drivers/soc/tegra/pmc.c
>>> @@ -526,9 +526,7 @@ int tegra_powergate_is_powered(unsigned int id)
>>> if (!tegra_powergate_is_valid(id))
>>> return -EINVAL;
>>>
>>> - mutex_lock(&pmc->powergates_lock);
>>> status = tegra_powergate_state(id);
>>> - mutex_unlock(&pmc->powergates_lock);
>>>
>>> return status;
>>> }
>>
>> Thanks for the fix. However, I would prefer that we fix this the following
>> way ...
>>
>> diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
>> index a2d163f759b4..546d2b121c39 100644
>> --- a/drivers/clk/tegra/clk-tegra30.c
>> +++ b/drivers/clk/tegra/clk-tegra30.c
>> @@ -1152,9 +1152,9 @@ static bool tegra30_cpu_rail_off_ready(void)
>>
>> cpu_rst_status = readl(clk_base +
>> TEGRA30_CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
>> - cpu_pwr_status = tegra_powergate_is_powered(TEGRA_POWERGATE_CPU1) ||
>> - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU2) ||
>> - tegra_powergate_is_powered(TEGRA_POWERGATE_CPU3);
>> + cpu_pwr_status = tegra_pmc_cpu_is_powered(1) ||
>> + tegra_pmc_cpu_is_powered(2) ||
>> + tegra_pmc_cpu_is_powered(3);
>>
>> if (((cpu_rst_status & 0xE) != 0xE) || cpu_pwr_status)
>> return false;
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index e233dd5dcab3..f61371ea3fe0 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -605,7 +605,7 @@ bool tegra_pmc_cpu_is_powered(unsigned int cpuid)
>> if (id < 0)
>> return false;
>>
>> - return tegra_powergate_is_powered(id);
>> + return tegra_powergate_state(id);
>> }
>>
>>
>> Could you try the above and make sure that this works?
>
> Something ate TABs in your patch, but yes, it works.
>
>> I would prefer to leave the mutex in tegra_powergate_is_powered() so it could
>> be used by any driver. Or maybe we could even get rid of
>> tegra_powergate_is_powered() if we don't really need it.
>
> This mutex does not protect anything here, though - there is only one register
> read inside the locked section, and after unlock other CPU might change
> it before the read value gets returned.
>
> Code size-wise it would be better to remove tegra_pmc_cpu_is_powered()
> instead.
Maybe, but tegra_pmc_cpu_is_powered() is needed by
tegra30_boot_secondary().
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/9] Bringing Asus TF300T support to mainline
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (8 preceding siblings ...)
2017-07-20 0:29 ` [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Michał Mirosław
@ 2017-07-20 7:48 ` Mikko Perttunen
2017-07-20 15:07 ` Michał Mirosław
2017-10-04 21:25 ` Michał Mirosław
10 siblings, 1 reply; 29+ messages in thread
From: Mikko Perttunen @ 2017-07-20 7:48 UTC (permalink / raw)
To: linux-arm-kernel
On 20.07.2017 03:29, Micha? Miros?aw wrote:
> Here is a suprisingly small set of patches that enable Asus TF300T tablet
> to boot and have all cores available. TF300T is one of a consumer devices
> based on NVidia's Cardhu reference tablet.
This is very cool! How did you debug it, or did it work on the first
try? :) I have a TF201 myself, probably quite similar to this, might be
worth a try.
>
> This series is an arch-dependent part. TF parts were extracted from ASUS's
> GPL code dump. The rest being driver code - a work in progress - is
> available in branch tf300t at:
>
> https://rere.qmqm.pl/git/linux
This URL doesn't work for me at least in the browser, but
https://rere.qmqm.pl/git/?p=linux;a=summary works.
Regarding the USB device mode - yes, there are several drivers for
roughly the same hardware. It would be good to get both host and device
mode with the same driver so that we could have OTG support.
Cheers,
Mikko
>
> Best Regards,
> Micha? Miros?aw
>
> ---
>
> Micha? Miros?aw (9):
> ARM: enable secure platform-only erratas
> arm: cache-l2x0: remove duplicate warning
> arm: cache-l2x0: share l2x0_base
> ARM: trusted_foundations: enable L2x0 cache via firmware_ops
> ARM: trusted_foundations: announce firmware version
> ARM: init: update secondary_data register documentation
> ARM: tegra: enable cache via TF
> ARM: tegra: avoid touching Secure registers in reset handler
> ARM: tegra: fix sleeping while atomic in CPU idle
>
> arch/arm/Kconfig | 20 +++++++---
> arch/arm/firmware/trusted_foundations.c | 63 +++++++++++++++++++++++++++++-
> arch/arm/include/asm/hardware/cache-l2x0.h | 6 ++-
> arch/arm/kernel/head.S | 4 +-
> arch/arm/mach-tegra/reset-handler.S | 27 ++++++++++++-
> arch/arm/mm/cache-l2x0-pmu.c | 8 +---
> arch/arm/mm/cache-l2x0.c | 26 ++++++------
> drivers/soc/tegra/pmc.c | 2 -
> 8 files changed, 125 insertions(+), 31 deletions(-)
>
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 0/9] Bringing Asus TF300T support to mainline
2017-07-20 7:48 ` [PATCH 0/9] Bringing Asus TF300T support to mainline Mikko Perttunen
@ 2017-07-20 15:07 ` Michał Mirosław
0 siblings, 0 replies; 29+ messages in thread
From: Michał Mirosław @ 2017-07-20 15:07 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 20, 2017 at 10:48:31AM +0300, Mikko Perttunen wrote:
> On 20.07.2017 03:29, Micha? Miros?aw wrote:
> > Here is a suprisingly small set of patches that enable Asus TF300T tablet
> > to boot and have all cores available. TF300T is one of a consumer devices
> > based on NVidia's Cardhu reference tablet.
>
> This is very cool! How did you debug it, or did it work on the first try? :)
> I have a TF201 myself, probably quite similar to this, might be worth a try.
Unfortunatelly it worked only after about 300 compile-upload-boot cycles. ;-)
I hacked an early console using bootloader framebuffer. The code for it is
a spaghetti monster, but if you would like to try it, I can add a patch.
> > This series is an arch-dependent part. TF parts were extracted from ASUS's
> > GPL code dump. The rest being driver code - a work in progress - is
> > available in branch tf300t at:
> >
> > https://rere.qmqm.pl/git/linux
> This URL doesn't work for me at least in the browser, but
> https://rere.qmqm.pl/git/?p=linux;a=summary works.
[...]
Ah, I should have been more precise: the url is for git clone (read-only).
Best Regards,
Micha? Miros?aw
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/9] Bringing Asus TF300T support to mainline
2017-07-20 0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
` (9 preceding siblings ...)
2017-07-20 7:48 ` [PATCH 0/9] Bringing Asus TF300T support to mainline Mikko Perttunen
@ 2017-10-04 21:25 ` Michał Mirosław
2017-10-05 15:52 ` Dmitry Osipenko
10 siblings, 1 reply; 29+ messages in thread
From: Michał Mirosław @ 2017-10-04 21:25 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 20, 2017 at 02:29:22AM +0200, Micha? Miros?aw wrote:
> Here is a suprisingly small set of patches that enable Asus TF300T tablet
> to boot and have all cores available. TF300T is one of a consumer devices
> based on NVidia's Cardhu reference tablet.
>
> This series is an arch-dependent part. TF parts were extracted from ASUS's
> GPL code dump. The rest being driver code - a work in progress - is
> available in branch tf300t at:
>
> https://rere.qmqm.pl/git/linux
Just a quick note, that I've rebased the branch on v4.14-rc3 and added
some more patches on top. This includes updated defconfig and devicetree
with working eMMC and an attempt at making TF300T's dock hot-pluggable.
Best Regards,
Micha? Miros?aw
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 0/9] Bringing Asus TF300T support to mainline
2017-10-04 21:25 ` Michał Mirosław
@ 2017-10-05 15:52 ` Dmitry Osipenko
2017-10-05 16:07 ` Dmitry Osipenko
0 siblings, 1 reply; 29+ messages in thread
From: Dmitry Osipenko @ 2017-10-05 15:52 UTC (permalink / raw)
To: linux-arm-kernel
On 05.10.2017 00:25, Micha? Miros?aw wrote:
> On Thu, Jul 20, 2017 at 02:29:22AM +0200, Micha? Miros?aw wrote:
>> Here is a suprisingly small set of patches that enable Asus TF300T tablet
>> to boot and have all cores available. TF300T is one of a consumer devices
>> based on NVidia's Cardhu reference tablet.
>>
>> This series is an arch-dependent part. TF parts were extracted from ASUS's
>> GPL code dump. The rest being driver code - a work in progress - is
>> available in branch tf300t at:
>>
>> https://rere.qmqm.pl/git/linux
>
> Just a quick note, that I've rebased the branch on v4.14-rc3 and added
> some more patches on top. This includes updated defconfig and devicetree
> with working eMMC and an attempt at making TF300T's dock hot-pluggable.
>
Just a small note.. I think you should develop on top of linux-next, since your
patches would be applied to linux-next first.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/9] Bringing Asus TF300T support to mainline
2017-10-05 15:52 ` Dmitry Osipenko
@ 2017-10-05 16:07 ` Dmitry Osipenko
0 siblings, 0 replies; 29+ messages in thread
From: Dmitry Osipenko @ 2017-10-05 16:07 UTC (permalink / raw)
To: linux-arm-kernel
On 05.10.2017 18:52, Dmitry Osipenko wrote:
> On 05.10.2017 00:25, Micha? Miros?aw wrote:
>> On Thu, Jul 20, 2017 at 02:29:22AM +0200, Micha? Miros?aw wrote:
>>> Here is a suprisingly small set of patches that enable Asus TF300T tablet
>>> to boot and have all cores available. TF300T is one of a consumer devices
>>> based on NVidia's Cardhu reference tablet.
>>>
>>> This series is an arch-dependent part. TF parts were extracted from ASUS's
>>> GPL code dump. The rest being driver code - a work in progress - is
>>> available in branch tf300t at:
>>>
>>> https://rere.qmqm.pl/git/linux
>>
>> Just a quick note, that I've rebased the branch on v4.14-rc3 and added
>> some more patches on top. This includes updated defconfig and devicetree
>> with working eMMC and an attempt at making TF300T's dock hot-pluggable.
>>
>
> Just a small note.. I think you should develop on top of linux-next, since your
> patches would be applied to linux-next first.
>
Also I think you should split the series into logically separated patchsets.
Like each of "cleanup" patches should be standalone, arm: / l2x0: / soc: patches
factored out into individual patchsets.
^ permalink raw reply [flat|nested] 29+ messages in thread