Re: [PATCH v2 4/5] powerpc/pseries: Implement RTAS error injection via pseries_eeh_err_inject

[Narayana Murty N <nnmlinux@linux.ibm.com>]

Hi Sourabh,

Thanks for the review.


On 07/06/26 7:05 PM, Sourabh Jain wrote:
>>
>
> session_token 0 is considered valid here. Where as it was considered 
> invalid in other
> function above.
Agreed. |rtas_open_errinjct_session()|is expected to return a positive 
session token on
success, while zero is not considered a valid token by 
|rtas_close_errinjct_session()|.
>
>> +        return session_token;
>> +
>> +    /* get errinjct token */
>> +    errinjct_token = rtas_function_token(RTAS_FN_IBM_ERRINJCT);
>> +    if (errinjct_token == RTAS_UNKNOWN_SERVICE) {
> How about checking this before getting the session token?
Agreed. I will obtain and validate the |ibm,errinjct|RTAS token before
opening the error-injection session. This avoids opening a session when 
the injection service is unavailable.
>
>> +        pr_err("RTAS: ibm,errinjct not available\n");
>> +        rc = -ENODEV;
>> +        goto out_close;
>> +    }
>> +
>> +    /* prepare shared buffer while holding lock */
>> +    spin_lock(&rtas_errinjct_buf_lock);
>> +    rc = prepare_errinjct_buffer(pe, type, func, addr, mask);
>> +    if (rc) {
>> +        spin_unlock(&rtas_errinjct_buf_lock);
>> +        goto out_close;
>> +    }
>> +
>> +    /* perform the errinjct RTAS call */
>> +    rc = do_errinjct_call(errinjct_token, type, session_token);
>> +    spin_unlock(&rtas_errinjct_buf_lock);
>> +
>> +out_close:
>> +    /* always attempt close if we opened a session */
>> +    rtas_close_errinjct_session(session_token);
>> +    return rc;
>>   }
>>   +
>
> This new line seems unnecessary.

Agreed. I will remove the extra blank line.

Thanks,
Narayana

>
>
>>   static struct eeh_ops pseries_eeh_ops = {
>>       .name            = "pseries",
>>       .probe            = pseries_eeh_probe,