Re: [PATCH] powerpc/vas: Fix IRQ name allocation

[Haren Myneni <haren@linux.ibm.com>]

On Sat, 2020-12-12 at 15:27 +0100, Cédric Le Goater wrote:
> The VAS device allocates a generic interrupt to handle page faults
> but
> the IRQ name doesn't show under /proc. This is because it's on
> stack. Allocate the name.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Thanks for fixing.

Acked-by: Haren Myneni <haren@linux.ibm.com>

> ---
> 
>  I didn't understand this part in init_vas_instance() :
> 
> 	if (vinst->virq) {
> 		rc = vas_irq_fault_window_setup(vinst);
> 		/*
> 		 * Fault window is used only for user space send
> windows.
> 		 * So if vinst->virq is NULL, tx_win_open returns
> -ENODEV
> 		 * for user space.
> 		 */
> 		if (rc)
> 			vinst->virq = 0;
> 	}
> 
>  If the IRQ cannot be requested, the device probing should fail but
>  it's not today. The use of 'vinst->virq' is suspicious.

VAS raises an interrupt only when NX sees fault on request buffers and
faults can happen only for user space requests. So Fault window setup
is needed for user space requests. For kernel requests, continue even
if IRQ / fault_window_setup is failed. 

When window open request is issued from user space, kernel returns
-ENODEV if vinst->virq = 0 (means fault window setup is failed). 


> 
>  arch/powerpc/platforms/powernv/vas.h |  1 +
>  arch/powerpc/platforms/powernv/vas.c | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/vas.h
> b/arch/powerpc/platforms/powernv/vas.h
> index 70f793e8f6cc..c7db3190baca 100644
> --- a/arch/powerpc/platforms/powernv/vas.h
> +++ b/arch/powerpc/platforms/powernv/vas.h
> @@ -340,6 +340,7 @@ struct vas_instance {
>  	struct vas_window *rxwin[VAS_COP_TYPE_MAX];
>  	struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
>  
> +	char *name;
>  	char *dbgname;
>  	struct dentry *dbgdir;
>  };
> diff --git a/arch/powerpc/platforms/powernv/vas.c
> b/arch/powerpc/platforms/powernv/vas.c
> index 598e4cd563fb..b65256a63e87 100644
> --- a/arch/powerpc/platforms/powernv/vas.c
> +++ b/arch/powerpc/platforms/powernv/vas.c
> @@ -28,12 +28,10 @@ static DEFINE_PER_CPU(int, cpu_vas_id);
>  
>  static int vas_irq_fault_window_setup(struct vas_instance *vinst)
>  {
> -	char devname[64];
>  	int rc = 0;
>  
> -	snprintf(devname, sizeof(devname), "vas-%d", vinst->vas_id);
>  	rc = request_threaded_irq(vinst->virq, vas_fault_handler,
> -				vas_fault_thread_fn, 0, devname,
> vinst);
> +				vas_fault_thread_fn, 0, vinst->name,
> vinst);
>  
>  	if (rc) {
>  		pr_err("VAS[%d]: Request IRQ(%d) failed with %d\n",
> @@ -80,6 +78,12 @@ static int init_vas_instance(struct
> platform_device *pdev)
>  	if (!vinst)
>  		return -ENOMEM;
>  
> +	vinst->name = kasprintf(GFP_KERNEL, "vas-%d", vasid);
> +	if (!vinst->name) {
> +		kfree(vinst);
> +		return -ENOMEM;
> +	}
> +
>  	INIT_LIST_HEAD(&vinst->node);
>  	ida_init(&vinst->ida);
>  	mutex_init(&vinst->mutex);
> @@ -162,6 +166,7 @@ static int init_vas_instance(struct
> platform_device *pdev)
>  	return 0;
>  
>  free_vinst:
> +	kfree(vinst->name);
>  	kfree(vinst);
>  	return -ENODEV;
>