| Message ID | 20220307101550.95538-3-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: at91: add support for secure suspend on sama5d2 | expand |
Hi Clément, On 07.03.2022 12:15, Clément Léger wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > When running with OP-TEE, the suspend control is handled securely. > Suspend can be entered using PSCI support. Since the sama5d2 supports > multiple suspend modes, add a new CONFIG_ATMEL_SECURE_PM which will > send a SMC call to select the suspend mode at init time. > > "atmel.pm_modes" boot argument is still supported for compatibility > purposes but the standby value is actually ignored since PSCI suspend > is used and it only support one mode (suspend). > > Signed-off-by: Clément Léger <clement.leger@bootlin.com> > --- > arch/arm/mach-at91/Kconfig | 12 ++++++++++- > arch/arm/mach-at91/pm.c | 38 +++++++++++++++++++++++++++++++++ > arch/arm/mach-at91/sam_secure.h | 4 ++++ > 3 files changed, 53 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig > index 02f6b108fd5d..5a6ca38d6303 100644 > --- a/arch/arm/mach-at91/Kconfig > +++ b/arch/arm/mach-at91/Kconfig > @@ -208,7 +208,17 @@ config SOC_SAMA5 > select SRAM if PM > > config ATMEL_PM > - bool > + bool "Atmel PM support" > + > +config ATMEL_SECURE_PM > + bool "Atmel Secure PM support" > + depends on SOC_SAMA5D2 && ATMEL_PM > + select ARM_PSCI > + help > + When running under a TEE, the suspend mode must be requested to be set > + at TEE level. When enable, this option will use secure monitor calls > + to set the suspend level. PSCI is then used to enter suspend. > + NOTE: This support is mutually exclusive with CONFIG_ATMEL_PM This note may be a bit confusing as ATMEL_SECURE_PM depends on CONFIG_ATMEL_PM. > > config SOC_SAMA7 > bool > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c > index dd6f4ce3f766..e40515691540 100644 > --- a/arch/arm/mach-at91/pm.c > +++ b/arch/arm/mach-at91/pm.c > @@ -27,6 +27,7 @@ > > #include "generic.h" > #include "pm.h" > +#include "sam_secure.h" > > #define BACKUP_DDR_PHY_CALIBRATION (9) > > @@ -856,6 +857,35 @@ static int __init at91_pm_backup_init(void) > return ret; > } > > +static void at91_pm_secure_init(void) > +{ > + int suspend_mode; > + struct arm_smccc_res res; > + > + suspend_mode = soc_pm.data.suspend_mode; > + > + res = sam_smccc_call(SAMA5_SMC_SIP_SET_SUSPEND_MODE, > + suspend_mode, 0); > + if (res.a0 == 0) { > + pr_info("AT91: Secure PM: suspend mode set to %s\n", > + pm_modes[suspend_mode].pattern); > + return; > + } > + > + pr_warn("AT91: Secure PM: %s mode not supported !\n", > + pm_modes[suspend_mode].pattern); > + > + res = sam_smccc_call(SAMA5_SMC_SIP_GET_SUSPEND_MODE, 0, 0); > + if (res.a0 == 0) { > + pr_warn("AT91: Secure PM: failed to get default mode\n"); > + return; > + } > + > + pr_info("AT91: Secure PM: using default suspend mode %s\n", > + pm_modes[suspend_mode].pattern); > + > + soc_pm.data.suspend_mode = res.a1; > +} > static const struct of_device_id atmel_shdwc_ids[] = { > { .compatible = "atmel,sama5d2-shdwc" }, > { .compatible = "microchip,sam9x60-shdwc" }, > @@ -1188,6 +1218,11 @@ void __init sama5d2_pm_init(void) > if (!IS_ENABLED(CONFIG_SOC_SAMA5D2)) > return; > > + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) { > + at91_pm_secure_init(); > + return; > + } > + > at91_pm_modes_validate(modes, ARRAY_SIZE(modes)); > at91_pm_modes_init(iomaps, ARRAY_SIZE(iomaps)); > ret = at91_dt_ramc(false); > @@ -1262,6 +1297,9 @@ static int __init at91_pm_modes_select(char *str) > soc_pm.data.standby_mode = standby; > soc_pm.data.suspend_mode = suspend; > > + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) > + pr_warn("AT91: Secure PM: ignoring standby mode\n"); What happens with soc_pm.data.standby_mode? Can Linux still suspend to it? If that's the case then the proper data for standby mode is not initialized as sama5d2_pm_init() returns if CONFIG_ATMEL_SECURE_PM is enabled, thus system may hang in such a case. Is this intended? Otherwise, maybe this pr_warn() could be moved in sama5d2_pm_init() as sama5d2 is the only user at the moment. Thank you, Claudiu Beznea > + > return 0; > } > early_param("atmel.pm_modes", at91_pm_modes_select); > diff --git a/arch/arm/mach-at91/sam_secure.h b/arch/arm/mach-at91/sam_secure.h > index 360036672f52..1e7d8b20ba1e 100644 > --- a/arch/arm/mach-at91/sam_secure.h > +++ b/arch/arm/mach-at91/sam_secure.h > @@ -8,6 +8,10 @@ > > #include <linux/arm-smccc.h> > > +/* Secure Monitor mode APIs */ > +#define SAMA5_SMC_SIP_SET_SUSPEND_MODE 0x400 > +#define SAMA5_SMC_SIP_GET_SUSPEND_MODE 0x401 > + > void __init sam_secure_init(void); > struct arm_smccc_res sam_smccc_call(u32 fn, u32 arg0, u32 arg1); > > -- > 2.34.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Le Tue, 8 Mar 2022 08:21:01 +0000, <Claudiu.Beznea@microchip.com> a écrit : [...] > > static const struct of_device_id atmel_shdwc_ids[] = { > > { .compatible = "atmel,sama5d2-shdwc" }, > > { .compatible = "microchip,sam9x60-shdwc" }, > > @@ -1188,6 +1218,11 @@ void __init sama5d2_pm_init(void) > > if (!IS_ENABLED(CONFIG_SOC_SAMA5D2)) > > return; > > > > + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) { > > + at91_pm_secure_init(); > > + return; > > + } > > + > > at91_pm_modes_validate(modes, ARRAY_SIZE(modes)); > > at91_pm_modes_init(iomaps, ARRAY_SIZE(iomaps)); > > ret = at91_dt_ramc(false); > > @@ -1262,6 +1297,9 @@ static int __init at91_pm_modes_select(char *str) > > soc_pm.data.standby_mode = standby; > > soc_pm.data.suspend_mode = suspend; > > > > + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) > > + pr_warn("AT91: Secure PM: ignoring standby mode\n"); > > What happens with soc_pm.data.standby_mode? Can Linux still suspend to it? Unfortunately, no. PSCI suspend support only allows one mode (SUSPEND_MEM, cf psci_suspend_ops) which should be the deepest suspend mode supported by the platform. Since the sama5 family support more than one suspend mode, we added this mechanism to keep the existing support for atmel,pm_modes support. We could potentially add a suspend support for sama5d2 that call the SMC to set the suspend mode and then enters PSCI system suspend by calling psci_system_suspend_enter(). But I'm not sure it is the idea behind the PSCI system suspend call. > If that's the case then the proper data for standby mode is not initialized > as sama5d2_pm_init() returns if CONFIG_ATMEL_SECURE_PM is enabled, thus > system may hang in such a case. Is this intended? Since the platform_suspend_ops won't be registered, there should be no call to enter sama5d2 suspend (either mem or standby) from this code. > > Otherwise, maybe this pr_warn() could be moved in sama5d2_pm_init() as > sama5d2 is the only user at the moment. Ok, I will move the print. Thanks, Clément > > Thank you, > Claudiu Beznea > > > + > > return 0; > > } > > early_param("atmel.pm_modes", at91_pm_modes_select); > > diff --git a/arch/arm/mach-at91/sam_secure.h b/arch/arm/mach-at91/sam_secure.h > > index 360036672f52..1e7d8b20ba1e 100644 > > --- a/arch/arm/mach-at91/sam_secure.h > > +++ b/arch/arm/mach-at91/sam_secure.h > > @@ -8,6 +8,10 @@ > > > > #include <linux/arm-smccc.h> > > > > +/* Secure Monitor mode APIs */ > > +#define SAMA5_SMC_SIP_SET_SUSPEND_MODE 0x400 > > +#define SAMA5_SMC_SIP_GET_SUSPEND_MODE 0x401 > > + > > void __init sam_secure_init(void); > > struct arm_smccc_res sam_smccc_call(u32 fn, u32 arg0, u32 arg1); > > > > -- > > 2.34.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig index 02f6b108fd5d..5a6ca38d6303 100644 --- a/arch/arm/mach-at91/Kconfig +++ b/arch/arm/mach-at91/Kconfig @@ -208,7 +208,17 @@ config SOC_SAMA5 select SRAM if PM config ATMEL_PM - bool + bool "Atmel PM support" + +config ATMEL_SECURE_PM + bool "Atmel Secure PM support" + depends on SOC_SAMA5D2 && ATMEL_PM + select ARM_PSCI + help + When running under a TEE, the suspend mode must be requested to be set + at TEE level. When enable, this option will use secure monitor calls + to set the suspend level. PSCI is then used to enter suspend. + NOTE: This support is mutually exclusive with CONFIG_ATMEL_PM config SOC_SAMA7 bool diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c index dd6f4ce3f766..e40515691540 100644 --- a/arch/arm/mach-at91/pm.c +++ b/arch/arm/mach-at91/pm.c @@ -27,6 +27,7 @@ #include "generic.h" #include "pm.h" +#include "sam_secure.h" #define BACKUP_DDR_PHY_CALIBRATION (9) @@ -856,6 +857,35 @@ static int __init at91_pm_backup_init(void) return ret; } +static void at91_pm_secure_init(void) +{ + int suspend_mode; + struct arm_smccc_res res; + + suspend_mode = soc_pm.data.suspend_mode; + + res = sam_smccc_call(SAMA5_SMC_SIP_SET_SUSPEND_MODE, + suspend_mode, 0); + if (res.a0 == 0) { + pr_info("AT91: Secure PM: suspend mode set to %s\n", + pm_modes[suspend_mode].pattern); + return; + } + + pr_warn("AT91: Secure PM: %s mode not supported !\n", + pm_modes[suspend_mode].pattern); + + res = sam_smccc_call(SAMA5_SMC_SIP_GET_SUSPEND_MODE, 0, 0); + if (res.a0 == 0) { + pr_warn("AT91: Secure PM: failed to get default mode\n"); + return; + } + + pr_info("AT91: Secure PM: using default suspend mode %s\n", + pm_modes[suspend_mode].pattern); + + soc_pm.data.suspend_mode = res.a1; +} static const struct of_device_id atmel_shdwc_ids[] = { { .compatible = "atmel,sama5d2-shdwc" }, { .compatible = "microchip,sam9x60-shdwc" }, @@ -1188,6 +1218,11 @@ void __init sama5d2_pm_init(void) if (!IS_ENABLED(CONFIG_SOC_SAMA5D2)) return; + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) { + at91_pm_secure_init(); + return; + } + at91_pm_modes_validate(modes, ARRAY_SIZE(modes)); at91_pm_modes_init(iomaps, ARRAY_SIZE(iomaps)); ret = at91_dt_ramc(false); @@ -1262,6 +1297,9 @@ static int __init at91_pm_modes_select(char *str) soc_pm.data.standby_mode = standby; soc_pm.data.suspend_mode = suspend; + if (IS_ENABLED(CONFIG_ATMEL_SECURE_PM)) + pr_warn("AT91: Secure PM: ignoring standby mode\n"); + return 0; } early_param("atmel.pm_modes", at91_pm_modes_select); diff --git a/arch/arm/mach-at91/sam_secure.h b/arch/arm/mach-at91/sam_secure.h index 360036672f52..1e7d8b20ba1e 100644 --- a/arch/arm/mach-at91/sam_secure.h +++ b/arch/arm/mach-at91/sam_secure.h @@ -8,6 +8,10 @@ #include <linux/arm-smccc.h> +/* Secure Monitor mode APIs */ +#define SAMA5_SMC_SIP_SET_SUSPEND_MODE 0x400 +#define SAMA5_SMC_SIP_GET_SUSPEND_MODE 0x401 + void __init sam_secure_init(void); struct arm_smccc_res sam_smccc_call(u32 fn, u32 arg0, u32 arg1);
When running with OP-TEE, the suspend control is handled securely. Suspend can be entered using PSCI support. Since the sama5d2 supports multiple suspend modes, add a new CONFIG_ATMEL_SECURE_PM which will send a SMC call to select the suspend mode at init time. "atmel.pm_modes" boot argument is still supported for compatibility purposes but the standby value is actually ignored since PSCI suspend is used and it only support one mode (suspend). Signed-off-by: Clément Léger <clement.leger@bootlin.com> --- arch/arm/mach-at91/Kconfig | 12 ++++++++++- arch/arm/mach-at91/pm.c | 38 +++++++++++++++++++++++++++++++++ arch/arm/mach-at91/sam_secure.h | 4 ++++ 3 files changed, 53 insertions(+), 1 deletion(-)