On Tue, 4 Mar 2025, Shyam Sundar S K wrote: > On 3/4/2025 19:16, Ilpo Järvinen wrote: > > On Tue, 18 Feb 2025, 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 > >> 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 | 50 +++++++++++++++++++-------- > >> 2 files changed, 40 insertions(+), 15 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..a81c661abd7e 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,8 +534,30 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) > >> goto error; > >> } > >> > >> - ret = amd_pmf_start_policy_engine(dev); > >> - if (ret) > >> + for (i = 0; i < ARRAY_SIZE(amd_pmf_ta_uuid); i++) { > >> + ret = amd_pmf_tee_init(dev, i); > > > > Any reason why you just pass the uuid pointer as it seems more obvious as > > a parameter than something as vague as "index"? > > > > The objective is to select the appropriate Trusted Application (TA) > binary from the /lib/firmware/amdtee/ directory. This selection is > determined by the UUIDs listed in the amd_pmf_ta_uuid[]. > > Typically, the most recent TA version should be located at the first > index, with the next most recent version at the second index, and so on. > > All these had to be done so that we don't end up in the version > compatibility issues we had encountered last time. I'm sorry, my editing messed the meaning of that comment up (I forgot to add the "not" word there). I only meant that you could pass uuid pointer instead of the index. > > I assume this change in general is to unbreak the case "3." from the link? > > Yes. that's right. Thanks. -- i.
