On 10/9/2023 8:36 PM, Ilpo Järvinen wrote: > On Mon, 9 Oct 2023, Shyam Sundar S K wrote: > >> There have been instances when the default size (1M) of the STB is not >> sufficient to get the complete traces of the failure. In such scenarios >> we can use a module_param to enable full trace that shall contain more >> debugging data. This is not a regular case and hence not enabling this >> capability by default. >> >> With this change, there will be two cases on how the driver fetches the >> stb data: >> 1) A special case (proposed now) - which is required only for certain >> platforms. Here, a new module param will be supplied to the driver that >> will have a special PMFW supporting enhanced dram sizes for getting >> the stb data. Without the special PMFW support, just setting the module >> param will not help to get the enhanced stb data. >> >> To adapt to this change, we will have a new amd_pmc_stb_handle_efr() to >> handle enhanced firmware reporting mechanism. Note that, since num_samples >> based r/w pointer offset calculation is not required for enhanced firmware >> reporting we will have this mailbox command sent only in case of regular >> STB cases. >> >> 2) Current code branch which fetches the stb data based on the parameters >> like the num_samples, fsize and the r/w pointer. >> >> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> Co-developed-by: Harsh Jain <Harsh.Jain@xxxxxxx> >> Signed-off-by: Harsh Jain <Harsh.Jain@xxxxxxx> >> Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> >> --- >> v6->v7: >> - Code simplication >> >> v5->v6: >> - No change >> >> v4->v5: >> - create a new function amd_pmc_stb_handle_efr() to handle enhanced firmware reporting mechanism >> - based on review-ilpo branch >> >> v3->v4: >> - Update code branches and commit-msg as per Ilpo's remark. >> >> v2->v3: >> - no change >> >> v1->v2: >> - rebase to 'review-hans' branch >> - drop 2/4 of v1 >> (https://patchwork.kernel.org/project/platform-driver-x86/list/?series=775324&state=%2A&archive=both) >> >> drivers/platform/x86/amd/pmc/pmc.c | 32 ++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c >> index e0b5d9de473a..af6d400193ff 100644 >> --- a/drivers/platform/x86/amd/pmc/pmc.c >> +++ b/drivers/platform/x86/amd/pmc/pmc.c >> @@ -53,6 +53,7 @@ >> >> /* STB Spill to DRAM Parameters */ >> #define S2D_TELEMETRY_BYTES_MAX 0x100000 >> +#define S2D_RSVD_RAM_SPACE 0x100000 >> #define S2D_TELEMETRY_DRAMBYTES_MAX 0x1000000 >> >> /* STB Spill to DRAM Message Definition */ >> @@ -165,6 +166,10 @@ static bool disable_workarounds; >> module_param(disable_workarounds, bool, 0644); >> MODULE_PARM_DESC(disable_workarounds, "Disable workarounds for platform bugs"); >> >> +static bool dump_custom_stb; >> +module_param(dump_custom_stb, bool, 0644); >> +MODULE_PARM_DESC(dump_custom_stb, "Enable to dump full STB buffer"); >> + >> static struct amd_pmc_dev pmc; >> static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret); >> static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf); >> @@ -241,6 +246,25 @@ static const struct file_operations amd_pmc_stb_debugfs_fops = { >> .release = amd_pmc_stb_debugfs_release, >> }; >> >> +/* Enhanced STB Firmware Reporting Mechanism */ >> +static int amd_pmc_stb_handle_efr(struct file *filp) >> +{ >> + struct amd_pmc_dev *dev = filp->f_inode->i_private; >> + struct amd_pmc_stb_v2_data *flex_arr; >> + u32 fsize; >> + >> + fsize = dev->dram_size - S2D_RSVD_RAM_SPACE; >> + flex_arr = kmalloc(struct_size(flex_arr, data, fsize), GFP_KERNEL); >> + if (!flex_arr) >> + return -ENOMEM; >> + >> + flex_arr->size = fsize; >> + memcpy_fromio(flex_arr->data, dev->stb_virt_addr, fsize); >> + filp->private_data = flex_arr; >> + >> + return 0; > > Thanks, this make much more sense than the early versions! > > Just one confirmation, is dev->dram_size >= S2D_RSVD_RAM_SPACE always > guaranteed so that the fsize never underflows? > Yes, that's right. Thanks, Shyam
