[PATCH v2] firmware/psci: Add debugfs support to ease debugging
Dmitry Baryshkov
dmitry.baryshkov at linaro.org
Mon Sep 19 05:00:45 PDT 2022
On 04/08/2022 18:26, Dmitry Baryshkov wrote:
> On Thu, 4 Aug 2022 at 17:49, Mark Rutland <mark.rutland at arm.com> wrote:
>>
>> On Thu, Aug 04, 2022 at 04:15:07PM +0300, Dmitry Baryshkov wrote:
>>> To ease debugging of PSCI supported features, add debugfs file called
>>> 'psci' describing PSCI and SMC CC versions, enabled features and
>>> options.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>> Changes since v1:
>>> - Extended the table to include MEM_PROTECT functions (noted by Mark
>>> Brown)
>>> - Switched to seq_puts where possible
>>> - Changed S_IRUGO to 0444
>>> ---
>>> drivers/firmware/psci/psci.c | 116 ++++++++++++++++++++++++++++++++++-
>>> include/uapi/linux/psci.h | 14 +++++
>>> 2 files changed, 129 insertions(+), 1 deletion(-)
>>
>> TBH I am really not keen on exposing this to userspace. AFAICT this is his is
>> incredibly niche, and is going to be very painful to maintain.
>>
>> Without a strong rationale, I do not thing we should do this.
>>
>> Who is going to use this, and when?
>
> I sketched this while trying to narrow down the particular issue on
> the Qualcomm platform (which started as an attempt to implement PSCI
> domains, but then led to understanding that while the platform claims
> supports OSI, it doesn't support SET_SUSPEND). We were going to use
> this to help to narrow down issues with the PSCI support on other
> platforms as well.
> Moreover during v1 review I saw at least a subtle request to include
> other information as well (e.g. the output from STAT_*),
>
>>
>> I'd be much happier logging stuff as it's probed into dmesg; I'm happy to add
>> more information there (and/or a command line parameter to be more verbose).
>
> Collecting logs is a nice thing, but asking the user to send the
> debugfs file is much easier compared to rebooting the kernel with
> different cmdline args (which in the Qualcomm case involves rebuilding
> the boot image).
Gentle ping for this patch for the review. It would be a really nice to
be able to ask users to provide a debugfs file rather than collecting
necessary data through the dmesg & other means.
>
>>
>> Thanks,
>> Mark.
>>
>>>
>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>> index 1628f1edef4a..42cae0ba10e2 100644
>>> --- a/drivers/firmware/psci/psci.c
>>> +++ b/drivers/firmware/psci/psci.c
>>> @@ -9,6 +9,7 @@
>>> #include <linux/acpi.h>
>>> #include <linux/arm-smccc.h>
>>> #include <linux/cpuidle.h>
>>> +#include <linux/debugfs.h>
>>> #include <linux/errno.h>
>>> #include <linux/linkage.h>
>>> #include <linux/of.h>
>>> @@ -326,12 +327,125 @@ static void psci_sys_poweroff(void)
>>> invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
>>> }
>>>
>>> -static int __init psci_features(u32 psci_func_id)
>>> +static int psci_features(u32 psci_func_id)
>>> {
>>> return invoke_psci_fn(PSCI_1_0_FN_PSCI_FEATURES,
>>> psci_func_id, 0, 0);
>>> }
>>>
>>> +#ifdef CONFIG_DEBUG_FS
>>> +
>>> +#define PSCI_ID(ver, _name) \
>>> + { .fn = PSCI_##ver##_FN_##_name, .name = #_name, }
>>> +#define PSCI_ID_NATIVE(ver, _name) \
>>> + { .fn = PSCI_FN_NATIVE(ver, _name), .name = #_name, }
>>> +
>>> +/* A table of all optional functions */
>>> +static const struct {
>>> + u32 fn;
>>> + const char *name;
>>> +} psci_fn_ids[] = {
>>> + PSCI_ID_NATIVE(0_2, MIGRATE),
>>> + PSCI_ID(0_2, MIGRATE_INFO_TYPE),
>>> + PSCI_ID_NATIVE(0_2, MIGRATE_INFO_UP_CPU),
>>> + PSCI_ID(1_0, CPU_FREEZE),
>>> + PSCI_ID_NATIVE(1_0, CPU_DEFAULT_SUSPEND),
>>> + PSCI_ID_NATIVE(1_0, NODE_HW_STATE),
>>> + PSCI_ID_NATIVE(1_0, SYSTEM_SUSPEND),
>>> + PSCI_ID(1_0, SET_SUSPEND_MODE),
>>> + PSCI_ID_NATIVE(1_0, STAT_RESIDENCY),
>>> + PSCI_ID_NATIVE(1_0, STAT_COUNT),
>>> + PSCI_ID_NATIVE(1_1, SYSTEM_RESET2),
>>> + PSCI_ID(1_1, MEM_PROTECT),
>>> + PSCI_ID_NATIVE(1_1, MEM_PROTECT_CHECK_RANGE),
>>> +};
>>> +
>>> +static int psci_debugfs_read(struct seq_file *s, void *data)
>>> +{
>>> + int feature, type, i;
>>> + u32 ver;
>>> +
>>> + ver = psci_ops.get_version();
>>> + seq_printf(s, "PSCIv%d.%d\n",
>>> + PSCI_VERSION_MAJOR(ver),
>>> + PSCI_VERSION_MINOR(ver));
>>> +
>>> + /* PSCI_FEATURES is available only starting from 1.0 */
>>> + if (PSCI_VERSION_MAJOR(ver) < 1)
>>> + return 0;
>>> +
>>> + feature = psci_features(ARM_SMCCC_VERSION_FUNC_ID);
>>> + if (feature != PSCI_RET_NOT_SUPPORTED) {
>>> + ver = invoke_psci_fn(ARM_SMCCC_VERSION_FUNC_ID, 0, 0, 0);
>>> + seq_printf(s, "SMC Calling Convention v%d.%d\n",
>>> + PSCI_VERSION_MAJOR(ver),
>>> + PSCI_VERSION_MINOR(ver));
>>> + } else {
>>> + seq_puts(s, "SMC Calling Convention v1.0 is assumed\n");
>>> + }
>>> +
>>> + feature = psci_features(PSCI_FN_NATIVE(0_2, CPU_SUSPEND));
>>> + if (feature < 0) {
>>> + seq_printf(s, "PSCI_FEATURES(CPU_SUSPEND) error (%d)\n", feature);
>>> + } else {
>>> + seq_printf(s, "OSI is %ssupported\n",
>>> + (feature & BIT(0)) ? "" : "not ");
>>> + seq_printf(s, "%s StateID format is used\n",
>>> + (feature & BIT(1)) ? "Extended" : "Original");
>>> + }
>>> +
>>> + type = psci_ops.migrate_info_type();
>>> + if (type == PSCI_0_2_TOS_UP_MIGRATE ||
>>> + type == PSCI_0_2_TOS_UP_NO_MIGRATE) {
>>> + unsigned long cpuid;
>>> +
>>> + seq_printf(s, "Trusted OS %smigrate capable\n",
>>> + type == PSCI_0_2_TOS_UP_NO_MIGRATE ? "not " : "");
>>> + cpuid = psci_migrate_info_up_cpu();
>>> + seq_printf(s, "Trusted OS resident on physical CPU 0x%lx (#%d)\n",
>>> + cpuid, resident_cpu);
>>> + } else if (type == PSCI_0_2_TOS_MP) {
>>> + seq_puts(s, "Trusted OS migration not required\n");
>>> + } else {
>>> + if (type != PSCI_RET_NOT_SUPPORTED)
>>> + seq_printf(s, "MIGRATE_INFO_TYPE returned unknown type (%d)\n", type);
>>> + }
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(psci_fn_ids); i++) {
>>> + feature = psci_features(psci_fn_ids[i].fn);
>>> + if (feature == PSCI_RET_NOT_SUPPORTED)
>>> + continue;
>>> + if (feature < 0)
>>> + seq_printf(s, "PSCI_FEATURES(%s) error (%d)\n",
>>> + psci_fn_ids[i].name, feature);
>>> + else
>>> + seq_printf(s, "%s is supported\n", psci_fn_ids[i].name);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int psci_debugfs_open(struct inode *inode, struct file *f)
>>> +{
>>> + return single_open(f, psci_debugfs_read, NULL);
>>> +}
>>> +
>>> +static const struct file_operations psci_debugfs_ops = {
>>> + .owner = THIS_MODULE,
>>> + .open = psci_debugfs_open,
>>> + .release = single_release,
>>> + .read = seq_read,
>>> + .llseek = seq_lseek
>>> +};
>>> +
>>> +static int __init psci_debugfs_init(void)
>>> +{
>>> + return PTR_ERR_OR_ZERO(debugfs_create_file("psci", 0444, NULL, NULL,
>>> + &psci_debugfs_ops));
>>> +}
>>> +late_initcall(psci_debugfs_init)
>>> +#endif
>>> +
>>> #ifdef CONFIG_CPU_IDLE
>>> static int psci_suspend_finisher(unsigned long state)
>>> {
>>> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
>>> index 2bf93c0d6354..3511095c2702 100644
>>> --- a/include/uapi/linux/psci.h
>>> +++ b/include/uapi/linux/psci.h
>>> @@ -48,12 +48,26 @@
>>> #define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU PSCI_0_2_FN64(7)
>>>
>>> #define PSCI_1_0_FN_PSCI_FEATURES PSCI_0_2_FN(10)
>>> +#define PSCI_1_0_FN_CPU_FREEZE PSCI_0_2_FN(11)
>>> +#define PSCI_1_0_FN_CPU_DEFAULT_SUSPEND PSCI_0_2_FN(12)
>>> +#define PSCI_1_0_FN_NODE_HW_STATE PSCI_0_2_FN(13)
>>> #define PSCI_1_0_FN_SYSTEM_SUSPEND PSCI_0_2_FN(14)
>>> #define PSCI_1_0_FN_SET_SUSPEND_MODE PSCI_0_2_FN(15)
>>> +#define PSCI_1_0_FN_STAT_RESIDENCY PSCI_0_2_FN(16)
>>> +#define PSCI_1_0_FN_STAT_COUNT PSCI_0_2_FN(17)
>>> +
>>> #define PSCI_1_1_FN_SYSTEM_RESET2 PSCI_0_2_FN(18)
>>> +#define PSCI_1_1_FN_MEM_PROTECT PSCI_0_2_FN(19)
>>> +#define PSCI_1_1_FN_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN(19)
>>>
>>> +#define PSCI_1_0_FN64_CPU_DEFAULT_SUSPEND PSCI_0_2_FN64(12)
>>> +#define PSCI_1_0_FN64_NODE_HW_STATE PSCI_0_2_FN64(13)
>>> #define PSCI_1_0_FN64_SYSTEM_SUSPEND PSCI_0_2_FN64(14)
>>> +#define PSCI_1_0_FN64_STAT_RESIDENCY PSCI_0_2_FN64(16)
>>> +#define PSCI_1_0_FN64_STAT_COUNT PSCI_0_2_FN64(17)
>>> +
>>> #define PSCI_1_1_FN64_SYSTEM_RESET2 PSCI_0_2_FN64(18)
>>> +#define PSCI_1_1_FN64_MEM_PROTECT_CHECK_RANGE PSCI_0_2_FN64(19)
>>>
>>> /* PSCI v0.2 power state encoding for CPU_SUSPEND function */
>>> #define PSCI_0_2_POWER_STATE_ID_MASK 0xffff
>>> --
>>> 2.35.1
>>>
>
>
>
--
With best wishes
Dmitry
More information about the linux-arm-kernel
mailing list