Re: [PATCH v3 08/16] platform/x86/amd/pmf: Add support to update system state

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 10/10/2023 9:33 PM, Mario Limonciello wrote:
> On 10/10/2023 07:59, Shyam Sundar S K wrote:
>> PMF driver based on the output actions from the TA can request to
>> update
>> the system states like entering s0i3, lock screen etc. by generating
>> an uevent. Based on the udev rules set in the userspace the event id
>> matching the uevent shall get updated accordingly using the systemctl.
>>
>> Sample udev rules under Documentation/admin-guide/pmf.rst.
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> 
> One minor nit below.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> 
>> ---
>>   Documentation/admin-guide/index.rst   |  1 +
>>   Documentation/admin-guide/pmf.rst     | 25 +++++++++++++++++++
>>   drivers/platform/x86/amd/pmf/pmf.h    |  9 +++++++
>>   drivers/platform/x86/amd/pmf/tee-if.c | 36
>> ++++++++++++++++++++++++++-
>>   4 files changed, 70 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/admin-guide/pmf.rst
>>
>> diff --git a/Documentation/admin-guide/index.rst
>> b/Documentation/admin-guide/index.rst
>> index 43ea35613dfc..fb40a1f6f79e 100644
>> --- a/Documentation/admin-guide/index.rst
>> +++ b/Documentation/admin-guide/index.rst
>> @@ -119,6 +119,7 @@ configure specific aspects of kernel behavior to
>> your liking.
>>      parport
>>      perf-security
>>      pm/index
>> +   pmf
>>      pnp
>>      rapidio
>>      ras
>> diff --git a/Documentation/admin-guide/pmf.rst
>> b/Documentation/admin-guide/pmf.rst
>> new file mode 100644
>> index 000000000000..6985bb5b9452
>> --- /dev/null
>> +++ b/Documentation/admin-guide/pmf.rst
>> @@ -0,0 +1,25 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +Set udev rules for PMF Smart PC Builder
>> +---------------------------------------
>> +
>> +AMD PMF(Platform Management Framework) Smart PC Solution builder
>> has to set the system states
>> +like S0i3, Screen lock, hibernate etc, based on the output actions
>> provided by the PMF
>> +TA (Trusted Application).
>> +
>> +In order for this to work the PMF driver generates a uevent for
>> userspace to react to. Below are
>> +sample udev rules that can facilitate this experience when a
>> machine has PMF Smart PC solution builder
>> +enabled.
>> +
>> +Please add the following line(s) to
>> +``/etc/udev/rules.d/99-local.rules``::
>> +
>> +        DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="0",
>> RUN+="/usr/bin/systemctl suspend"
>> +        DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="1",
>> RUN+="/usr/bin/systemctl hibernate"
>> +        DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="2",
>> RUN+="/bin/loginctl lock-sessions"
>> +
>> +EVENT_ID values:
>> +0= Put the system to S0i3/S2Idle
>> +1= Put the system to hibernate
>> +2= Lock the screen
>> +
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>> b/drivers/platform/x86/amd/pmf/pmf.h
>> index 20f3e16b0a32..67f11113d5a7 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -73,6 +73,7 @@
>>   #define PMF_POLICY_STT_MIN                    6
>>   #define PMF_POLICY_STT_SKINTEMP_APU                7
>>   #define PMF_POLICY_STT_SKINTEMP_HS2                8
>> +#define PMF_POLICY_SYSTEM_STATE                    9
>>   #define PMF_POLICY_P3T                        38
>>     /* TA macros */
>> @@ -440,6 +441,13 @@ struct apmf_dyn_slider_output {
>>   } __packed;
>>     /* Smart PC - TA internals */
> 
> I know that Ilpo had a comment about this in an earlier version that
> there is a "__" instead of "_".  I know this is intended behavior for
> consistency with internal usage, but maybe it's worth having a comment
> somewhere mentioning it's intended behavior?  I'm not sure where.
> 

I missed to change here.  I have changed at other places too.

>> +enum system_state {
>> +    SYSTEM_STATE__S0i3,
>> +    SYSTEM_STATE__S4,
>> +    SYSTEM_STATE__SCREEN_LOCK,
>> +    SYSTEM_STATE__MAX
>> +};
>> +
>>   enum ta_slider {
>>       TA_BEST_BATTERY,    /* Best Battery */
>>       TA_BETTER_BATTERY,    /* Better Battery */
>> @@ -471,6 +479,7 @@ enum ta_pmf_error_type {
>>   };
>>     struct pmf_action_table {
>> +    enum system_state system_state;
>>       u32 spl;        /* in mW */
>>       u32 sppt;        /* in mW */
>>       u32 sppt_apuonly;    /* in mW */
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>> b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 92879ae4f8f0..c08ef13a1494 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -24,6 +24,20 @@ MODULE_PARM_DESC(pb_actions_ms, "Policy binary
>> actions sampling frequency (defau
>>   static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77,
>> 0x3fb8, 0x524d,
>>                           0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85,
>> 0x43);
>>   +static const char *amd_pmf_uevent_as_str(unsigned int state)
>> +{
>> +    switch (state) {
>> +    case SYSTEM_STATE__S0i3:
>> +        return "S0i3";
>> +    case SYSTEM_STATE__S4:
>> +        return "S4";
>> +    case SYSTEM_STATE__SCREEN_LOCK:
>> +        return "SCREEN_LOCK";
>> +    default:
>> +        return "Unknown Smart PC event";
>> +    }
>> +}
>> +
>>   static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
>>                    struct tee_ioctl_invoke_arg *arg,
>>                    struct tee_param *param)
>> @@ -42,9 +56,23 @@ static void amd_pmf_prepare_args(struct
>> amd_pmf_dev *dev, int cmd,
>>       param[0].u.memref.shm_offs = 0;
>>   }
>>   +static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16
>> event)
>> +{
>> +    char *envp[2] = {};
>> +
>> +    envp[0] = kasprintf(GFP_KERNEL, "EVENT_ID=%d", event);
>> +    if (!envp[0])
>> +        return -EINVAL;
>> +
>> +    kobject_uevent_env(&dev->dev->kobj, KOBJ_CHANGE, envp);
>> +
>> +    kfree(envp[0]);
>> +    return 0;
>> +}
>> +
>>   static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>> ta_pmf_enact_result *out)
>>   {
>> -    u32 val;
>> +    u32 val, event = 0;
>>       int idx;
>>         for (idx = 0; idx < out->actions_count; idx++) {
>> @@ -113,6 +141,12 @@ static void amd_pmf_apply_policies(struct
>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>                   dev->prev_data->p3t_limit = val;
>>               }
>>               break;
>> +
>> +        case PMF_POLICY_SYSTEM_STATE:
>> +            amd_pmf_update_uevents(dev, event);
>> +            dev_dbg(dev->dev, "update SYSTEM_STATE : %s\n",
>> +                amd_pmf_uevent_as_str(event));
>> +            break;
>>           }
>>       }
>>   }
>