On Tue, 4 Mar 2025, Shyam Sundar S K wrote: > On 3/4/2025 21:02, Ilpo Järvinen wrote: > > 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. > > OK. I can respin, just to make sure are you expecting this? > > //ret = amd_pmf_tee_init(dev, i); > ret = amd_pmf_tee_init(dev, &amd_pmf_ta_uuid[i]); > > so that I propogate the uuid to the export_guid() rather than the index? Yes. -- i.
