| Message ID | 20230922175056.244940-11-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Introduce PMF Smart PC Solution Builder Feature | expand |
Hi Shyam, kernel test robot noticed the following build warnings: [auto build test WARNING on hid/for-next] [also build test WARNING on linus/master v6.6-rc3 next-20230921] [cannot apply to drm-misc/drm-misc-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Shyam-Sundar-S-K/platform-x86-amd-pmf-Add-PMF-TEE-interface/20230923-015418 base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next patch link: https://lore.kernel.org/r/20230922175056.244940-11-Shyam-sundar.S-k%40amd.com patch subject: [PATCH 10/15] platform/x86/amd/pmf: Add capability to sideload of policy binary config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230925/202309251031.awDDkRgS-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230925/202309251031.awDDkRgS-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309251031.awDDkRgS-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/platform/x86/amd/pmf/tee-if.c:305:5: warning: no previous prototype for 'amd_pmf_open_pb' [-Wmissing-prototypes] 305 | int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root) | ^~~~~~~~~~~~~~~ vim +/amd_pmf_open_pb +305 drivers/platform/x86/amd/pmf/tee-if.c 304 > 305 int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root) 306 { 307 struct dentry *file = NULL; 308 309 dev->esbin = debugfs_create_dir("pb", debugfs_root); 310 if (IS_ERR(dev->esbin)) 311 return -EINVAL; 312 313 file = debugfs_create_file("update_policy", 0644, dev->esbin, dev, &pb_fops); 314 if (!file) 315 return -EINVAL; 316 317 return 0; 318 } 319 #endif 320
On Fri, 22 Sep 2023, Shyam Sundar S K wrote: > A policy binary is OS agnostic, and the same policies are expected to work > across the OSes. At times it becomes difficult to debug when the policies > inside the policy binaries starts to misbehave. Add a way to sideload such > policies independently to debug them via a debugfs entry. > > 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 | 1 + > drivers/platform/x86/amd/pmf/tee-if.c | 60 +++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h > index 61a0f3225b62..780c442239e3 100644 > --- a/drivers/platform/x86/amd/pmf/pmf.h > +++ b/drivers/platform/x86/amd/pmf/pmf.h > @@ -215,6 +215,7 @@ struct amd_pmf_dev { > bool cnqf_supported; > struct notifier_block pwr_src_notifier; > /* Smart PC solution builder */ > + struct dentry *esbin; > unsigned char *policy_buf; > u32 policy_sz; > struct tee_context *tee_ctx; > diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c > index 4844782d93c7..fa37cfab2dc7 100644 > --- a/drivers/platform/x86/amd/pmf/tee-if.c > +++ b/drivers/platform/x86/amd/pmf/tee-if.c > @@ -8,6 +8,7 @@ > * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > */ > > +#include <linux/debugfs.h> > #include <linux/tee_drv.h> > #include <linux/uuid.h> > #include "pmf.h" > @@ -21,6 +22,13 @@ module_param(pb_actions_ms, int, 0644); > MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)"); > #endif > > +#ifdef CONFIG_AMD_PMF_DEBUG > +/* Sideload policy binaries to debug policy failures */ > +static bool pb_side_load; > +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); > > @@ -267,6 +275,49 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev) > return 0; > } > > +#ifdef CONFIG_AMD_PMF_DEBUG > +static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf, > + size_t length, loff_t *pos) > +{ > + struct amd_pmf_dev *dev = filp->private_data; > + int ret; > + > + /* policy binary size cannot exceed POLICY_BUF_MAX_SZ */ > + if (length > POLICY_BUF_MAX_SZ || length == 0) > + return -EINVAL; > + > + dev->policy_sz = length; > + if (copy_from_user(dev->policy_buf, buf, dev->policy_sz)) > + return -EFAULT; > + > + ret = amd_pmf_start_policy_engine(dev); > + if (ret) > + return -EINVAL; > + > + return length; > +} > + > +static const struct file_operations pb_fops = { > + .write = amd_pmf_get_pb_data, > + .open = simple_open, > +}; > + > +int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root) > +{ > + struct dentry *file = NULL; > + > + dev->esbin = debugfs_create_dir("pb", debugfs_root); > + if (IS_ERR(dev->esbin)) > + return -EINVAL; > + > + file = debugfs_create_file("update_policy", 0644, dev->esbin, dev, &pb_fops); > + if (!file) > + return -EINVAL; > + > + return 0; > +} > +#endif > + > static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev) > { > dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL); > @@ -279,6 +330,11 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev) > > memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz); > > +#ifdef CONFIG_AMD_PMF_DEBUG > + if (pb_side_load) Can't this go into amd_pmf_open_pb() as early return? > + amd_pmf_open_pb(dev, dev->dbgfs_dir); If you provide #else and empty amd_pmf_open_pb() above, you don't need to do ifdefs here. > +#endif > + > return amd_pmf_start_policy_engine(dev); > } > > @@ -381,6 +437,10 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) > > void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev) > { > +#ifdef CONFIG_AMD_PMF_DEBUG > + if (pb_side_load) > + debugfs_remove_recursive(dev->esbin); > +#endif Likewise here, if you add amd_pmf_remove_pb() into the above #ifdef + new #else block with empty body, you can just call it w/o #ifdefs here. > kfree(dev->prev_data); > kfree(dev->policy_buf); > cancel_delayed_work_sync(&dev->pb_work); >
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h index 61a0f3225b62..780c442239e3 100644 --- a/drivers/platform/x86/amd/pmf/pmf.h +++ b/drivers/platform/x86/amd/pmf/pmf.h @@ -215,6 +215,7 @@ struct amd_pmf_dev { bool cnqf_supported; struct notifier_block pwr_src_notifier; /* Smart PC solution builder */ + struct dentry *esbin; unsigned char *policy_buf; u32 policy_sz; struct tee_context *tee_ctx; diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c index 4844782d93c7..fa37cfab2dc7 100644 --- a/drivers/platform/x86/amd/pmf/tee-if.c +++ b/drivers/platform/x86/amd/pmf/tee-if.c @@ -8,6 +8,7 @@ * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> */ +#include <linux/debugfs.h> #include <linux/tee_drv.h> #include <linux/uuid.h> #include "pmf.h" @@ -21,6 +22,13 @@ module_param(pb_actions_ms, int, 0644); MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)"); #endif +#ifdef CONFIG_AMD_PMF_DEBUG +/* Sideload policy binaries to debug policy failures */ +static bool pb_side_load; +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); @@ -267,6 +275,49 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev) return 0; } +#ifdef CONFIG_AMD_PMF_DEBUG +static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf, + size_t length, loff_t *pos) +{ + struct amd_pmf_dev *dev = filp->private_data; + int ret; + + /* policy binary size cannot exceed POLICY_BUF_MAX_SZ */ + if (length > POLICY_BUF_MAX_SZ || length == 0) + return -EINVAL; + + dev->policy_sz = length; + if (copy_from_user(dev->policy_buf, buf, dev->policy_sz)) + return -EFAULT; + + ret = amd_pmf_start_policy_engine(dev); + if (ret) + return -EINVAL; + + return length; +} + +static const struct file_operations pb_fops = { + .write = amd_pmf_get_pb_data, + .open = simple_open, +}; + +int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root) +{ + struct dentry *file = NULL; + + dev->esbin = debugfs_create_dir("pb", debugfs_root); + if (IS_ERR(dev->esbin)) + return -EINVAL; + + file = debugfs_create_file("update_policy", 0644, dev->esbin, dev, &pb_fops); + if (!file) + return -EINVAL; + + return 0; +} +#endif + static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev) { dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL); @@ -279,6 +330,11 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev) memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz); +#ifdef CONFIG_AMD_PMF_DEBUG + if (pb_side_load) + amd_pmf_open_pb(dev, dev->dbgfs_dir); +#endif + return amd_pmf_start_policy_engine(dev); } @@ -381,6 +437,10 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev) { +#ifdef CONFIG_AMD_PMF_DEBUG + if (pb_side_load) + debugfs_remove_recursive(dev->esbin); +#endif kfree(dev->prev_data); kfree(dev->policy_buf); cancel_delayed_work_sync(&dev->pb_work);