RE: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute

[Sethi Varun-B16395]

Hi Joerg,
In case of PAMU, geometry would be meaningless without specifying the number of subwindows. If the API user specifies a geometry but fails to specify the number of required subwindows, we would assume a default (max supported) value, which may be incorrect. It's going to be really ugly. I think we should add the number of sub windows to the domain geometry. This parameter would be used in case of a window based iommu. So, a separate set_window API wouldn't be required.

We can still implement the get_windows API for getting the maximum number of subwindows supported by the window based IOMMU.

Regards
Varun

> -----Original Message-----
> From: Joerg Roedel [mailto:joro@xxxxxxxxxx]
> Sent: Monday, February 04, 2013 6:49 PM
> To: Sethi Varun-B16395; Stuart Yoder
> Cc: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Joerg
> Roedel
> Subject: [PATCH 5/5] iommu: Add DOMAIN_ATTR_WINDOWS domain attribute
> 
> This attribute can be used to set and get the number of subwindows on
> IOMMUs that are window-based.
> 
> Signed-off-by: Joerg Roedel <joro@xxxxxxxxxx>
> ---
>  drivers/iommu/iommu.c |   33 ++++++++++++++++++++++++++++++---
>  include/linux/iommu.h |    5 +++++
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index
> 0fdb7db..66402f7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -891,6 +891,7 @@ int iommu_domain_get_attr(struct iommu_domain
> *domain,
>  	struct iommu_domain_geometry *geometry;
>  	bool *paging;
>  	int ret = 0;
> +	u32 *count;
> 
>  	switch (attr) {
>  	case DOMAIN_ATTR_GEOMETRY:
> @@ -902,6 +903,15 @@ int iommu_domain_get_attr(struct iommu_domain
> *domain,
>  		paging  = data;
>  		*paging = (domain->ops->pgsize_bitmap != 0UL);
>  		break;
> +	case DOMAIN_ATTR_WINDOWS:
> +		count = data;
> +
> +		if (domain->ops->domain_get_windows != NULL)
> +			*count = domain->ops->domain_get_windows(domain);
> +		else
> +			ret = -ENODEV;
> +
> +		break;
>  	default:
>  		if (!domain->ops->domain_get_attr)
>  			return -EINVAL;
> @@ -916,9 +926,26 @@ EXPORT_SYMBOL_GPL(iommu_domain_get_attr);
>  int iommu_domain_set_attr(struct iommu_domain *domain,
>  			  enum iommu_attr attr, void *data)
>  {
> -	if (!domain->ops->domain_set_attr)
> -		return -EINVAL;
> +	int ret = 0;
> +	u32 *count;
> +
> +	switch (attr) {
> +	case DOMAIN_ATTR_WINDOWS:
> +		count = data;
> +
> +		if (domain->ops->domain_set_windows != NULL)
> +			ret = domain->ops->domain_set_windows(domain, *count);
> +		else
> +			ret = -ENODEV;
> 
> -	return domain->ops->domain_set_attr(domain, attr, data);
> +		break;
> +	default:
> +		if (domain->ops->domain_set_attr == NULL)
> +			return -EINVAL;
> +
> +		ret = domain->ops->domain_set_attr(domain, attr, data);
> +	}
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h index
> 0cba2d8..8330df1 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -60,6 +60,7 @@ struct iommu_domain {
>  enum iommu_attr {
>  	DOMAIN_ATTR_GEOMETRY,
>  	DOMAIN_ATTR_PAGING,
> +	DOMAIN_ATTR_WINDOWS,
>  	DOMAIN_ATTR_MAX,
>  };
> 
> @@ -106,6 +107,10 @@ struct iommu_ops {
>  	int (*domain_window_enable)(struct iommu_domain *domain, u32
> wnd_nr,
>  				    phys_addr_t paddr, size_t size);
>  	void (*domain_window_disable)(struct iommu_domain *domain, u32
> wnd_nr);
> +	/* Set the numer of window per domain */
> +	int (*domain_set_windows)(struct iommu_domain *domain, u32
> w_count);
> +	/* Get the numer of window per domain */
> +	u32 (*domain_get_windows)(struct iommu_domain *domain);
> 
>  	unsigned long pgsize_bitmap;
>  };
> --
> 1.7.9.5
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/