On 2/17/2025 23:00, Mario Limonciello wrote: > On 2/17/2025 00:49, Shyam Sundar S K wrote: >> The PMF driver allocates a shared memory buffer using >> tee_shm_alloc_kernel_buf() for communication with the PMF-TA. >> >> The latest PMF-TA version introduces new structures with OEM debug >> information and additional policy input conditions for evaluating the >> policy binary. Consequently, the shared memory size must be >> increased to >> ensure compatibility between the PMF driver and the updated PMF-TA. >> >> To do so, Introduce the new PMF-TA UUID and update the PMF shared >> memory > > Small nit that this should be s/Introduce/introduce/. I wouldn't > change it if nothing else is raised. > Ack. >> configuration to ensure compatibility with the latest PMF-TA version. >> Additionally, export the TA UUID. >> >> These updates will result in modifications to the prototypes of >> amd_pmf_tee_init() and amd_pmf_ta_open_session(). >> >> Link: https://lore.kernel.org/all/55ac865f-b1c7-fa81-51c4- >> d211c7963e7e@xxxxxxxxxxxxxxx/ >> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx> >> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> > >> --- >> drivers/platform/x86/amd/pmf/pmf.h | 5 ++- >> drivers/platform/x86/amd/pmf/tee-if.c | 49 ++++++++++++++++++ >> +-------- >> 2 files changed, 38 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/ >> x86/amd/pmf/pmf.h >> index 41b2b91b8fdc..e6bdee68ccf3 100644 >> --- a/drivers/platform/x86/amd/pmf/pmf.h >> +++ b/drivers/platform/x86/amd/pmf/pmf.h >> @@ -106,9 +106,12 @@ struct cookie_header { >> #define PMF_TA_IF_VERSION_MAJOR 1 >> #define TA_PMF_ACTION_MAX 32 >> #define TA_PMF_UNDO_MAX 8 >> -#define TA_OUTPUT_RESERVED_MEM 906 >> +#define TA_OUTPUT_RESERVED_MEM 922 >> #define MAX_OPERATION_PARAMS 4 >> +#define TA_ERROR_CRYPTO_INVALID_PARAM 0x20002 >> +#define TA_ERROR_CRYPTO_BIN_TOO_LARGE 0x2000d >> + >> #define PMF_IF_V1 1 >> #define PMF_IF_V2 2 >> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/ >> platform/x86/amd/pmf/tee-if.c >> index b404764550c4..43bda6f98a11 100644 >> --- a/drivers/platform/x86/amd/pmf/tee-if.c >> +++ b/drivers/platform/x86/amd/pmf/tee-if.c >> @@ -27,8 +27,11 @@ module_param(pb_side_load, bool, 0444); >> MODULE_PARM_DESC(pb_side_load, "Sideload policy binaries debug >> policy failures"); >> #endif >> -static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, >> 0x3fb8, 0x524d, >> - 0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, >> 0x43); >> +static const uuid_t amd_pmf_ta_uuid[] = { UUID_INIT(0xd9b39bf2, >> 0x66bd, 0x4154, 0xaf, 0xb8, 0x8a, >> + 0xcc, 0x2b, 0x2b, 0x60, 0xd6), >> + UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d, 0xb1, >> 0x2d, 0xc5, >> + 0x29, 0xb1, 0x3d, 0x85, 0x43), >> + }; >> static const char *amd_pmf_uevent_as_str(unsigned int state) >> { >> @@ -321,7 +324,7 @@ static int amd_pmf_start_policy_engine(struct >> amd_pmf_dev *dev) >> */ >> schedule_delayed_work(&dev->pb_work, >> msecs_to_jiffies(pb_actions_ms * 3)); >> } else { >> - dev_err(dev->dev, "ta invoke cmd init failed err: %x\n", res); >> + dev_dbg(dev->dev, "ta invoke cmd init failed err: %x\n", res); >> dev->smart_pc_enabled = false; >> return res; >> } >> @@ -390,12 +393,12 @@ static int amd_pmf_amdtee_ta_match(struct >> tee_ioctl_version_data *ver, const voi >> return ver->impl_id == TEE_IMPL_ID_AMDTEE; >> } >> -static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 *id) >> +static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 >> *id, int index) >> { >> struct tee_ioctl_open_session_arg sess_arg = {}; >> int rc; >> - export_uuid(sess_arg.uuid, &amd_pmf_ta_uuid); >> + export_uuid(sess_arg.uuid, &amd_pmf_ta_uuid[index]); >> sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC; >> sess_arg.num_params = 0; >> @@ -434,7 +437,7 @@ static int >> amd_pmf_register_input_device(struct amd_pmf_dev *dev) >> return 0; >> } >> -static int amd_pmf_tee_init(struct amd_pmf_dev *dev) >> +static int amd_pmf_tee_init(struct amd_pmf_dev *dev, int index) >> { >> u32 size; >> int ret; >> @@ -445,7 +448,7 @@ static int amd_pmf_tee_init(struct amd_pmf_dev >> *dev) >> return PTR_ERR(dev->tee_ctx); >> } >> - ret = amd_pmf_ta_open_session(dev->tee_ctx, &dev->session_id); >> + ret = amd_pmf_ta_open_session(dev->tee_ctx, &dev->session_id, >> index); >> if (ret) { >> dev_err(dev->dev, "Failed to open TA session (%d)\n", ret); >> ret = -EINVAL; >> @@ -489,7 +492,8 @@ static void amd_pmf_tee_deinit(struct >> amd_pmf_dev *dev) >> int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) >> { >> - int ret; >> + bool status; >> + int ret, i; >> ret = apmf_check_smart_pc(dev); >> if (ret) { >> @@ -502,10 +506,6 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) >> return -ENODEV; >> } >> - ret = amd_pmf_tee_init(dev); >> - if (ret) >> - return ret; >> - >> INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd); >> ret = amd_pmf_set_dram_addr(dev, true); >> @@ -534,9 +534,28 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) >> goto error; >> } >> - ret = amd_pmf_start_policy_engine(dev); >> - if (ret) >> - goto error; >> + for (i = 0; i < ARRAY_SIZE(amd_pmf_ta_uuid); i++) { >> + ret = amd_pmf_tee_init(dev, i); >> + if (ret) >> + return ret; >> + >> + ret = amd_pmf_start_policy_engine(dev); >> + switch (ret) { >> + case TA_PMF_TYPE_SUCCESS: >> + status = true; >> + break; >> + case TA_ERROR_CRYPTO_INVALID_PARAM: >> + case TA_ERROR_CRYPTO_BIN_TOO_LARGE: >> + amd_pmf_tee_deinit(dev); >> + status = false; >> + break; >> + default: >> + goto error; >> + } >> + >> + if (status) >> + break; >> + } >> > > After the loop I think you're missing one more case of > > if (!status) > goto error; > > Otherwise can't you potentially end up with both attempts returning an > error code? > > If you think it makes sense to still be able to "sideload" a PB in > this case perhaps it would be best to change it to > > if (!status && !pb_side_load) > goto error; > >> if (pb_side_load) >> amd_pmf_open_pb(dev, dev->dbgfs_dir); > I thought sideload will take a different path. Let me respin a new version based on your thoughts. Thanks, Shyam
