lkml.org 
[lkml]   [2021]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 2/4] fsi: occ: Store the SBEFIFO FFDC in the user response buffer
From
Date

On 10/15/21 12:05 AM, Joel Stanley wrote:
> On Mon, 27 Sept 2021 at 15:59, Eddie James <eajames@linux.ibm.com> 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?


The user's buffer now contains data in the event of a failure. No change
in the event of a successful transfer.


>
> Will existing userspace handle this?


Yes, unless a poorly-designed application is relying on the data being
the same after a failed transfer... In that case I would argue that the
application should be fixed.


>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> 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?


No, based on the sbefifo_parse_status function.


>
>> + size_t ffdc_len = (dh - 1) * 4;
>> + __be32 *ffdc = &resp[resp_len - dh];
> Should you be checking that this number is sensible?


Do you mean ffdc_len or (resp_len - dh)? I was basing all this on the
sbefifo_parse_status code, but I see that obviously:

resp_len - dh = resp_len - (resp_len - parsed_len) = parsed_len

So I will simplify.

As for ffdc_len, it is conceivable that dh is 0, so I will add a check
for that.



Thanks Joel!

Eddie


>
>> +
>> + 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;
>> +}

\
 
 \ /
  Last update: 2021-10-19 22:17    [W:0.055 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site