Re: [PATCH v6 2/3] platform/x86/amd/pmc: Handle overflow cases where the num_samples range is higher

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 10/9/23 15:43, Shyam Sundar S K wrote:
> In amd_pmc_stb_debugfs_open_v2(), the stb buffer is created based on the
> num_samples and the read/write pointer offset. This holds good when the
> num_samples reported by PMFW is less than S2D_TELEMETRY_BYTES_MAX; where
> the stb buffer gets filled from 0th position until
> S2D_TELEMETRY_BYTES_MAX - 1 based on the read/write pointer offset.
> 
> But when the num_samples exceeds the S2D_TELEMETRY_BYTES_MAX, the current
> code does not handle it well as it does not account for the cases where
> the stb buffer has to filled up as a circular buffer.
> 
> Handle this scenario into two cases, where first memcpy will have the
> samples from location:
> (num_samples % S2D_TELEMETRY_BYTES_MAX) - (S2D_TELEMETRY_BYTES_MAX - 1)
> and next memcpy will have the newest ones i.e.
> 0 - (num_samples % S2D_TELEMETRY_BYTES_MAX - 1)
> 
> Suggested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> ---
> v5->v6:
>  - Make changes as per Hans on v5
>  - based on review-ilpo branch


Thanks, patch looks good to me now:

Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Regards,

Hans





> 
> v4->v5:
>  - Fix exisiting code problems when reading stb buffer as a circular data
>  - 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 | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index fdc1e104c437..e0b5d9de473a 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -276,16 +276,23 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
>  
>  	flex_arr->size = fsize;
>  
> -	/* Start capturing data from the last push location */
> +	/*
> +	 * Start capturing data from the last push location.
> +	 * This is for general cases, where the stb limits
> +	 * are meant for standard usage.
> +	 */
>  	if (num_samples > S2D_TELEMETRY_BYTES_MAX) {
> -		fsize  = S2D_TELEMETRY_BYTES_MAX;
> -		stb_rdptr_offset = num_samples - fsize;
> +		/* First read oldest data starting 1 behind last write till end of ringbuffer */
> +		stb_rdptr_offset = num_samples % S2D_TELEMETRY_BYTES_MAX;
> +		fsize = S2D_TELEMETRY_BYTES_MAX - stb_rdptr_offset;
> +
> +		memcpy_fromio(flex_arr->data, dev->stb_virt_addr + stb_rdptr_offset, fsize);
> +		/* Second copy the newer samples from offset 0 - last write */
> +		memcpy_fromio(flex_arr->data + fsize, dev->stb_virt_addr, stb_rdptr_offset);
>  	} else {
> -		fsize = num_samples;
> -		stb_rdptr_offset = 0;
> +		memcpy_fromio(flex_arr->data, dev->stb_virt_addr, fsize);
>  	}
>  
> -	memcpy_fromio(flex_arr->data, dev->stb_virt_addr + stb_rdptr_offset, fsize);
>  	filp->private_data = flex_arr;
>  
>  	return 0;




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux