On Sun, 2018-12-16 at 22:28 +0100, Greg Kurz wrote:
> All fields in the PE are big-endian. Use cpu_to_be32() like
> everywhere
> else something is written to the PE. Otherwise a wrong TID will be
> used
> by the NPU. If this TID happens to point to an existing thread
> sharing
> the same mm, it could be woken up by error. This is highly improbable
> though. The likely outcome of this is the NPU not finding the target
> thread and forcing the AFU into sending an interrupt, which userspace
> is supposed to handle anyway.
>
> Fixes: e948e06fc63a ("ocxl: Expose the thread_id needed for wait on
> POWER9")
> Cc: stable@xxxxxxxxxxxxxxx # v4.18
> Signed-off-by: Greg Kurz <groug@xxxxxxxx>
> ---
>
> This bug remained unnoticed so far because the current OCXL test
> suite
> happens to call OCXL_IOCTL_ENABLE_P9_WAIT before attaching a context.
> This causes ocxl_link_update_pe() to be called before
> ocxl_link_add_pe()
> which re-writes the TID in the PE with the appropriate endianness.
>
> I have some patches that change the behavior of the OCXL test suite
> so that
> it can catch the issue:
>
> https://github.com/gkurz/libocxl/commits/wake-host-thread-rework
> ---
> drivers/misc/ocxl/link.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index 31695a078485..646d16450066 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -566,7 +566,7 @@ int ocxl_link_update_pe(void *link_handle, int
> pasid, __u16 tid)
>
> mutex_lock(&spa->spa_lock);
>
> - pe->tid = tid;
> + pe->tid = cpu_to_be32(tid);
>
> /*
> * The barrier makes sure the PE is updated
>
Good catch, thanks.
Reviewed-by: Alastair D'Silva <alastair@xxxxxxxxxxx>
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
