lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 7 Nov 2022 09:15:49 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Alexander Usyskin <alexander.usyskin@...el.com>
Cc:     Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Tvrtko Ursulin <tvrtko.ursulin@...ux.intel.com>,
        Tomas Winkler <tomas.winkler@...el.com>,
        Vitaly Lubart <vitaly.lubart@...el.com>,
        intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mei: add timeout to send

On Thu, Nov 03, 2022 at 05:55:34PM +0200, Alexander Usyskin wrote:
> When driver wakes up the firmware from the low power stand,
> it is sending a memory ready message.
> The send is done via synchronous/blocking function to ensure
> that firmware is in ready state. However firmware might be
> in unstable state and send might be block forever.
> To address this issue a timeout is added to blocking write command on
> the internal bus.
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
> ---
>  drivers/misc/mei/bus-fixup.c | 19 +++++++++++--------
>  drivers/misc/mei/bus.c       |  9 +++++----
>  drivers/misc/mei/client.c    | 21 +++++++++++++++++----
>  drivers/misc/mei/client.h    |  2 +-
>  drivers/misc/mei/main.c      |  2 +-
>  drivers/misc/mei/mei_dev.h   |  2 +-
>  6 files changed, 36 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 71fbf0bc8453..3174cad8a5cc 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -128,7 +128,7 @@ static int mei_osver(struct mei_cl_device *cldev)
>  	os_ver = (struct mei_os_ver *)fwcaps->data;
>  	os_ver->os_type = OSTYPE_LINUX;
>  
> -	return __mei_cl_send(cldev->cl, buf, size, 0, mode);
> +	return __mei_cl_send(cldev->cl, buf, size, 0, mode, 0);

Ick, adding another parameter to a function is not helpful here, what
does 0 mean unless you look it up?

> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -29,11 +29,12 @@
>   * @length: buffer length
>   * @vtag: virtual tag
>   * @mode: sending mode
> + * @timeout: send timeout for blocking writes, 0 for infinite timeout

What units are these in?  Jiffies?  seconds?  hours?  Don't make us
guess :(

Why not write a function that is mei_cl_send_timeout() that has the new
option?  That way it's obvious that the other ones are blocking, right?

thanks,

greg k-h

Powered by blists - more mailing lists