On Mon, 27 Sept 2021 at 15:59, Eddie James <eajames@xxxxxxxxxxxxx> wrote:
>
> If the SBEFIFO response indicates an error, store the response in the
> user buffer and return an error. Previously, the user had no way of
> obtaining the SBEFIFO FFDC.
How does this look for userspace?
Will existing userspace handle this?
>
> Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> ---
> Changes since v1:
> - Don't store any magic value; only return non-zero resp_len in the error
> case if there is FFDC
>
> drivers/fsi/fsi-occ.c | 66 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
> index ace3ec7767e5..1d5f6fdc2a34 100644
> --- a/drivers/fsi/fsi-occ.c
> +++ b/drivers/fsi/fsi-occ.c
> @@ -55,6 +55,9 @@ struct occ {
> int idx;
> u8 sequence_number;
> void *buffer;
> + void *client_buffer;
> + size_t client_buffer_size;
> + size_t client_response_size;
> enum versions version;
> struct miscdevice mdev;
> struct mutex occ_lock;
> @@ -217,6 +220,20 @@ static const struct file_operations occ_fops = {
> .release = occ_release,
> };
>
> +static void occ_save_ffdc(struct occ *occ, __be32 *resp, size_t parsed_len,
> + size_t resp_len)
> +{
> + size_t dh = resp_len - parsed_len;
Is there any chance that parsed_len is larger than resp_len?
> + size_t ffdc_len = (dh - 1) * 4;
> + __be32 *ffdc = &resp[resp_len - dh];
Should you be checking that this number is sensible?
> +
> + if (ffdc_len > occ->client_buffer_size)
> + ffdc_len = occ->client_buffer_size;
> +
> + memcpy(occ->client_buffer, ffdc, ffdc_len);
> + occ->client_response_size = ffdc_len;
> +}
