2023-04-20 12:11:50

by Corey Minyard

[permalink] [raw]
Subject: Re: [Openipmi-developer] [PATCH] ipmi: ipmi-bmc: Improve errno returned to userspace

Andrew, what do you think?

-corey

On Wed, Apr 19, 2023 at 05:00:32PM +0200, Govert Overgaauw via Openipmi-developer wrote:
> While the KCS driver is not in KCS_PHASE_WAIT_READ state it returns
> -EINVAL to userspace on a write call. change this to -EAGAIN to indicate
> that the error is related to the state and not the argument.
>
> Signed-off-by: Govert Overgaauw <[email protected]>
> ---
> drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
> index cf670e891966..4c7400faf333 100644
> --- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
> +++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
> @@ -405,7 +405,7 @@ static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
> kcs_bmc_write_data(priv->client.dev, priv->data_out[0]);
> ret = count;
> } else {
> - ret = -EINVAL;
> + ret = -EAGAIN;
> }
> spin_unlock_irq(&priv->lock);
>
> --
> 2.30.2
>
>
>
> _______________________________________________
> Openipmi-developer mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer


2023-04-21 02:00:40

by Andrew Jeffery

[permalink] [raw]
Subject: Re: [Openipmi-developer] [PATCH] ipmi: ipmi-bmc: Improve errno returned to userspace

Hi Corey,

On Thu, 20 Apr 2023, at 21:40, Corey Minyard wrote:
> Andrew, what do you think?

I'm a bit short on details of the IPMI KCS state machine and expectations of userspace in this exact case.

I've added Vernon who is one of the IPMI maintainers for OpenBMC.

I've also added Zev who's an interested party, and openbmc@ for good measure.

Andrew

>
> -corey
>
> On Wed, Apr 19, 2023 at 05:00:32PM +0200, Govert Overgaauw via
> Openipmi-developer wrote:
>> While the KCS driver is not in KCS_PHASE_WAIT_READ state it returns
>> -EINVAL to userspace on a write call. change this to -EAGAIN to indicate
>> that the error is related to the state and not the argument.
>>
>> Signed-off-by: Govert Overgaauw <[email protected]>
>> ---
>> drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>> index cf670e891966..4c7400faf333 100644
>> --- a/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>> +++ b/drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
>> @@ -405,7 +405,7 @@ static ssize_t kcs_bmc_ipmi_write(struct file *filp, const char __user *buf,
>> kcs_bmc_write_data(priv->client.dev, priv->data_out[0]);
>> ret = count;
>> } else {
>> - ret = -EINVAL;
>> + ret = -EAGAIN;
>> }
>> spin_unlock_irq(&priv->lock);
>>
>> --
>> 2.30.2
>>
>>
>>
>> _______________________________________________
>> Openipmi-developer mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/openipmi-developer