Re: [PATCH V5] cxl: Add support for ASB_Notify on POWER9

[Frederic Barrat <fbarrat@linux.vnet.ibm.com>]

> --- a/drivers/misc/cxl/file.c
> +++ b/drivers/misc/cxl/file.c
> @@ -173,7 +173,7 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
>   	 * flags are set it's invalid
>   	 */
>   	if (work.reserved1 || work.reserved2 || work.reserved3 ||
> -	    work.reserved4 || work.reserved5 || work.reserved6 ||
> +	    work.reserved4 || work.reserved5 ||
>   	    (work.flags & ~CXL_START_WORK_ALL)) {
>   		rc = -EINVAL;
>   		goto out;
> @@ -248,7 +248,19 @@ static long afu_ioctl_start_work(struct cxl_context *ctx,
>   	 */
>   	smp_mb();
> 
> -	trace_cxl_attach(ctx, work.work_element_descriptor, work.num_interrupts, amr);
> +	/* Assign a unique TIDR (thread id) for the current thread */
> +	if (work.flags & CXL_START_WORK_TID) {
> +		rc = cxl_context_thread_tidr(ctx);
> +		if (rc)

We're already pretty deep and have allocated quite a few resources, we 
we'd need to unwind (see error path below when the attach fails).

However, we cannot clear the thread TIDR register, so we need to be 
careful that a user process cannot exhaust our limited pool of TIDs by 
calling the attach ioctl with bogus arguments. Which should be easy to 
do: attach the max number of contexts, and keep calling attach!
So we're going to need to figure out something to prevent that (define a 
max allocation per context? with a value of 1 for now?)


> diff --git a/include/uapi/misc/cxl.h b/include/uapi/misc/cxl.h
> index 49e8fd0..3ea2d4b4 100644
> --- a/include/uapi/misc/cxl.h
> +++ b/include/uapi/misc/cxl.h
> @@ -20,20 +20,22 @@ struct cxl_ioctl_start_work {
>   	__u64 work_element_descriptor;
>   	__u64 amr;
>   	__s16 num_interrupts;
> -	__s16 reserved1;
> -	__s32 reserved2;
> +	__s16 tid;

Should probably be unsigned.

   Fred


> +	__s32 reserved1;
> +	__u64 reserved2;
>   	__u64 reserved3;
>   	__u64 reserved4;
>   	__u64 reserved5;
> -	__u64 reserved6;
>   };
> 
>   #define CXL_START_WORK_AMR		0x0000000000000001ULL
>   #define CXL_START_WORK_NUM_IRQS		0x0000000000000002ULL
>   #define CXL_START_WORK_ERR_FF		0x0000000000000004ULL
> +#define CXL_START_WORK_TID		0x0000000000000008ULL
>   #define CXL_START_WORK_ALL		(CXL_START_WORK_AMR |\
>   					 CXL_START_WORK_NUM_IRQS |\
> -					 CXL_START_WORK_ERR_FF)
> +					 CXL_START_WORK_ERR_FF |\
> +					 CXL_START_WORK_TID)
> 
> 
>   /* Possible modes that an afu can be in */
>