[PATCH v3 06/10] VAS: Define helpers to alloc/free windows

[Michael Neuling]

On Thu, 2017-03-16 at 20:33 -0700, Sukadev Bhattiprolu wrote:
> Define helpers to allocate/free VAS window objects. These will
> be used in follow-on patches when opening/closing windows.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev at linux.vnet.ibm.com>
> ---
>  drivers/misc/vas/vas-window.c | 74 +++++++++++++++++++++++++++++++++++++++++-
> -
>  1 file changed, 72 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/vas/vas-window.c b/drivers/misc/vas/vas-window.c
> index edf5c9f..9233bf5 100644
> --- a/drivers/misc/vas/vas-window.c
> +++ b/drivers/misc/vas/vas-window.c
> @@ -119,7 +119,7 @@ static void unmap_wc_mmio_bars(struct vas_window *window)
>   * OS/User Window Context (UWC) MMIO Base Address Region for the given
> window.
>   * Map these bus addresses and save the mapped kernel addresses in @window.
>   */
> -int map_wc_mmio_bars(struct vas_window *window)
> +static int map_wc_mmio_bars(struct vas_window *window)
>  {
>  	int len;
>  	uint64_t start;
> @@ -472,8 +472,78 @@ int init_winctx_regs(struct vas_window *window, struct
> vas_winctx *winctx)
>  	return 0;
>  }
>  
> -/* stub for now */
> +DEFINE_SPINLOCK(vas_ida_lock);
> +
> +void vas_release_window_id(struct ida *ida, int winid)
> +{
> +	spin_lock(&vas_ida_lock);
> +	ida_remove(ida, winid);
> +	spin_unlock(&vas_ida_lock);
> +}
> +
> +int vas_assign_window_id(struct ida *ida)
> +{
> +	int rc, winid;
> +
> +	rc = ida_pre_get(ida, GFP_KERNEL);
> +	if (!rc)
> +		return -EAGAIN;
> +
> +	spin_lock(&vas_ida_lock);
> +	rc = ida_get_new_above(ida, 0, &winid);
> +	spin_unlock(&vas_ida_lock);
> +
> +	if (rc)
> +		return rc;
> +
> +	if (winid > VAS_MAX_WINDOWS_PER_CHIP) {
> +		pr_err("VAS: Too many (%d) open windows\n", winid);
> +		vas_release_window_id(ida, winid);
> +		return -EAGAIN;
> +	}
> +
> +	return winid;
> +}
> +
> +static void vas_window_free(struct vas_window *window)
> +{
> +	unmap_wc_mmio_bars(window);
> +	kfree(window->paste_addr_name);
> +	kfree(window);
> +}
> +
> +static struct vas_window *vas_window_alloc(struct vas_instance *vinst, int
> id)
> +{
> +	struct vas_window *window;
> +
> +	window = kzalloc(sizeof(*window), GFP_KERNEL);
> +	if (!window)
> +		return NULL;
> +
> +	window->vinst = vinst;
> +	window->winid = id;
> +
> +	if (map_wc_mmio_bars(window))
> +		goto out_free;
> +
> +	return window;
> +
> +out_free:
> +	kfree(window);
> +	return NULL;
> +}
> +
>  int vas_window_reset(struct vas_instance *vinst, int winid)
> 

This interface seems a little weird to me. Needing an alloc in a hardware reset
path seems a bit strange.

Maybe the data structures are the issue.  A window is a hardware construct. 
Something that uses it should probably be called something else like a context. 
Something that references a window should just be the vas_instance + winid. 

You should be able to reset this hardware window by referencing structures
already allocated.  Something associated with the struct vas_instance.

Mikey

>  {
> +	struct vas_window *window;
> +
> +	window = vas_window_alloc(vinst, winid);
> +	if (!window)
> +		return -ENOMEM;
> +
> +	reset_window_regs(window);
> +
> +	vas_window_free(window);
> +
>  	return 0;
>  }