Re: [PATCH 02/15] platform/x86/amd/pmf: Add support PMF-TA interaction

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Fri, 22 Sep 2023, Shyam Sundar S K wrote:

> PMF TA (Trusted Application) loads via the TEE environment into the
> AMD ASP.
> 
> PMF-TA supports two commands:
> 1) Init: Initialize the TA with the PMF Smart PC policy binary and
> start the policy engine. A policy is a combination of inputs and
> outputs, where;
>  - the inputs are the changing dynamics of the system like the user
>    behaviour, system heuristics etc.
>  - the outputs, which are the actions to be set on the system which
>    lead to better power management and enhanced user experience.
> 
> PMF driver acts as a central manager in this case to supply the
> inputs required to the TA (either by getting the information from
> the other kernel subsystems or from userland)
> 
> 2) Enact: Enact the output actions from the TA. The action could be
> applying a new thermal limit to boost/throttle the power limits or
> change system behavior.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/pmf.h    | 10 +++
>  drivers/platform/x86/amd/pmf/tee-if.c | 97 ++++++++++++++++++++++++++-
>  2 files changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 02460c2a31ea..a9333ff6c0a7 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -59,6 +59,9 @@
>  #define ARG_NONE 0
>  #define AVG_SAMPLE_SIZE 3
>  
> +/* TA macros */
> +#define PMF_TA_IF_VERSION__MAJOR				1

I suppose double _ is not intentional?

> +
>  /* AMD PMF BIOS interfaces */
>  struct apmf_verify_interface {
>  	u16 size;
> @@ -184,6 +187,7 @@ struct amd_pmf_dev {
>  	struct tee_shm *fw_shm_pool;
>  	u32 session_id;
>  	void *shbuf;
> +	struct delayed_work pb_work;
>  	bool smart_pc_enabled;
>  };
>  
> @@ -395,6 +399,12 @@ struct apmf_dyn_slider_output {
>  	struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
>  } __packed;
>  
> +/* cmd ids for TA communication */
> +enum ta_pmf_command {
> +	TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE,
> +	TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES

Add comma to the second line too.

Did you mean to have double _?

> +};
> +
>  struct ta_pmf_shared_memory {
>  	int command_id;
>  	int resp_id;
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index b48340edbf44..1fce04beacb3 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -13,9 +13,96 @@
>  #include "pmf.h"
>  
>  #define MAX_TEE_PARAM	4
> +
> +/* Policy binary actions sampling frequency (in ms) */
> +static int pb_actions_ms = 1000;
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +module_param(pb_actions_ms, int, 0644);
> +MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
> +#endif
> +
>  static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
>  						0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
>  
> +static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
> +				 struct tee_ioctl_invoke_arg *arg,
> +				 struct tee_param *param)
> +{
> +	memset(arg, 0, sizeof(*arg));
> +	memset(param, 0, MAX_TEE_PARAM * sizeof(*param));
> +
> +	arg->func = cmd;
> +	arg->session = dev->session_id;
> +	arg->num_params = MAX_TEE_PARAM;
> +
> +	/* Fill invoke cmd params */
> +	param[0].u.memref.size = sizeof(struct ta_pmf_shared_memory);
> +	param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
> +	param[0].u.memref.shm = dev->fw_shm_pool;
> +	param[0].u.memref.shm_offs = 0;
> +}
> +
> +static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
> +{
> +	struct ta_pmf_shared_memory *ta_sm = NULL;
> +	struct tee_param param[MAX_TEE_PARAM];
> +	struct tee_ioctl_invoke_arg arg;
> +	int ret = 0;
> +
> +	if (!dev->tee_ctx)
> +		return -ENODEV;
> +
> +	ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
> +	memset(ta_sm, 0, sizeof(struct ta_pmf_shared_memory));
> +	ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES;
> +	ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
> +
> +	amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES, &arg, param);
> +
> +	ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
> +	if (ret < 0 || arg.ret != 0) {
> +		dev_err(dev->dev, "%s failed TEE err: %x, ret:%x\n", __func__, arg.ret, ret);

No __func__s please.

> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
> +{
> +	struct ta_pmf_shared_memory *ta_sm = NULL;
> +	struct tee_param param[MAX_TEE_PARAM];
> +	struct tee_ioctl_invoke_arg arg;
> +	int ret = 0;
> +
> +	if (!dev->tee_ctx) {
> +		dev_err(dev->dev, "%s tee_ctx no context\n", __func__);
> +		return -ENODEV;
> +	}
> +
> +	ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
> +	ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE;
> +	ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
> +
> +	amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE, &arg, param);
> +
> +	ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
> +	if (ret < 0 || arg.ret != 0) {
> +		dev_err(dev->dev, "%s failed TEE err: %x, ret:%x\n", __func__, arg.ret, ret);
> +		return -EINVAL;
> +	}
> +
> +	return ta_sm->pmf_result;
> +}
> +
> +static void amd_pmf_invoke_cmd(struct work_struct *work)
> +{
> +	struct amd_pmf_dev *dev = container_of(work, struct amd_pmf_dev, pb_work.work);
> +
> +	amd_pmf_invoke_cmd_enact(dev);
> +	schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
> +}
> +
>  static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
>  {
>  	return ver->impl_id == TEE_IMPL_ID_AMDTEE;
> @@ -97,10 +184,18 @@ static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
>  
>  int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>  {
> -	return amd_pmf_tee_init(dev);
> +	int ret;
> +
> +	ret = amd_pmf_tee_init(dev);
> +	if (ret)
> +		return ret;
> +
> +	INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
> +	return 0;
>  }
>  
>  void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
>  {
> +	cancel_delayed_work_sync(&dev->pb_work);
>  	amd_pmf_tee_deinit(dev);
>  }
> 

-- 
 i.