IOMMU Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint
       [not found] <20200323084108.1721-1-bbhushan2@marvell.com>
@ 2020-03-23  9:59 ` Jean-Philippe Brucker
  2020-03-23 17:25   ` [EXT] " Bharat Bhushan
  2020-03-23 18:04   ` Bharat Bhushan
  0 siblings, 2 replies; 5+ messages in thread
From: Jean-Philippe Brucker @ 2020-03-23  9:59 UTC (permalink / raw)
  To: Bharat Bhushan; +Cc: mst, virtualization, iommu, jasowang

Hi Bharat,

Please add the IOMMU list on your next posting

On Mon, Mar 23, 2020 at 02:11:08PM +0530, Bharat Bhushan wrote:
> Different endpoint can support different page size, probe
> endpoint if it supports specific page size otherwise use
> global page sizes.
> 
> Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> ---
>  drivers/iommu/virtio-iommu.c      | 24 ++++++++++++++++++++----
>  include/uapi/linux/virtio_iommu.h |  6 ++++++
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index cce329d71fba..e69347ca4ee6 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -78,6 +78,7 @@ struct viommu_endpoint {
>  	struct viommu_dev		*viommu;
>  	struct viommu_domain		*vdomain;
>  	struct list_head		resv_regions;
> +	u64				pgsize_bitmap;
>  };
>  
>  struct viommu_request {
> @@ -415,6 +416,14 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain)
>  	return ret;
>  }
>  
> +static int viommu_set_pgsize_bitmap(struct viommu_endpoint *vdev,
> +				    struct virtio_iommu_probe_pgsize_mask *mask)
> +
> +{
> +	vdev->pgsize_bitmap = mask->pgsize_bitmap;

We need to read this through le64_to_cpu(). Also check that the length of
the field provided by the device is >= sizeof(mask) (like
viommu_add_resv_mem() does)

> +	return 0;
> +}
> +
>  static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
>  			       struct virtio_iommu_probe_resv_mem *mem,
>  			       size_t len)
> @@ -494,11 +503,13 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
>  	while (type != VIRTIO_IOMMU_PROBE_T_NONE &&
>  	       cur < viommu->probe_size) {
>  		len = le16_to_cpu(prop->length) + sizeof(*prop);
> -
>  		switch (type) {
>  		case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
>  			ret = viommu_add_resv_mem(vdev, (void *)prop, len);
>  			break;
> +		case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK:
> +			ret = viommu_set_pgsize_bitmap(vdev, (void *)prop);
> +			break;
>  		default:
>  			dev_err(dev, "unknown viommu prop 0x%x\n", type);
>  		}
> @@ -607,16 +618,21 @@ static struct iommu_domain *viommu_domain_alloc(unsigned type)
>  	return &vdomain->domain;
>  }
>  
> -static int viommu_domain_finalise(struct viommu_dev *viommu,
> +static int viommu_domain_finalise(struct viommu_endpoint *vdev,
>  				  struct iommu_domain *domain)
>  {
>  	int ret;
>  	struct viommu_domain *vdomain = to_viommu_domain(domain);
> +	struct viommu_dev *viommu = vdev->viommu;
>  
>  	vdomain->viommu		= viommu;
>  	vdomain->map_flags	= viommu->map_flags;
>  
> -	domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> +	if (vdev->pgsize_bitmap)
> +		domain->pgsize_bitmap = vdev->pgsize_bitmap;
> +	else
> +		domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> +

nit: it could be nicer to initialize vdev->pgsize_bitmap in add_device(),
override it in probe_endpoint(), and just copy it here.

>  	domain->geometry	= viommu->geometry;
>  
>  	ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain,
> @@ -657,7 +673,7 @@ static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  		 * Properly initialize the domain now that we know which viommu
>  		 * owns it.
>  		 */
> -		ret = viommu_domain_finalise(vdev->viommu, domain);
> +		ret = viommu_domain_finalise(vdev, domain);

Attaching additional endpoints with different masks to the domain should
return an error

>  	} else if (vdomain->viommu != vdev->viommu) {
>  		dev_err(dev, "cannot attach to foreign vIOMMU\n");
>  		ret = -EXDEV;
> diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
> index 237e36a280cb..aff3db0ef54b 100644
> --- a/include/uapi/linux/virtio_iommu.h
> +++ b/include/uapi/linux/virtio_iommu.h
> @@ -111,6 +111,7 @@ struct virtio_iommu_req_unmap {
>  
>  #define VIRTIO_IOMMU_PROBE_T_NONE		0
>  #define VIRTIO_IOMMU_PROBE_T_RESV_MEM		1
> +#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK	2
>  
>  #define VIRTIO_IOMMU_PROBE_T_MASK		0xfff
>  
> @@ -119,6 +120,11 @@ struct virtio_iommu_probe_property {
>  	__le16					length;
>  };
>  
> +struct virtio_iommu_probe_pgsize_mask {
> +	struct virtio_iommu_probe_property	head;

Compilers will introduce 4 bytes of padding here, to align the next field.
We need to make them explicit by adding a 4-bytes 'reserved' field.

> +	uint64_t				pgsize_bitmap;

__le64

Thanks,
Jean

> +};
> +
>  #define VIRTIO_IOMMU_RESV_MEM_T_RESERVED	0
>  #define VIRTIO_IOMMU_RESV_MEM_T_MSI		1
>  
> -- 
> 2.17.1
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint
  2020-03-23  9:59 ` [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint Jean-Philippe Brucker
@ 2020-03-23 17:25   ` " Bharat Bhushan
  2020-03-24  7:58     ` Jean-Philippe Brucker
  2020-03-23 18:04   ` Bharat Bhushan
  1 sibling, 1 reply; 5+ messages in thread
From: Bharat Bhushan @ 2020-03-23 17:25 UTC (permalink / raw)
  To: Jean-Philippe Brucker; +Cc: mst, virtualization, iommu, jasowang

Hi Jean,

> -----Original Message-----
> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Sent: Monday, March 23, 2020 3:30 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: joro@8bytes.org; mst@redhat.com; jasowang@redhat.com;
> virtualization@lists.linux-foundation.org; iommu@lists.linux-foundation.org;
> eric.auger@redhat.com
> Subject: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by
> endpoint
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Bharat,
> 
> Please add the IOMMU list on your next posting

iommu@lists.linux-foundation.org is there, any other mailing list we need to add?

> 
> On Mon, Mar 23, 2020 at 02:11:08PM +0530, Bharat Bhushan wrote:
> > Different endpoint can support different page size, probe endpoint if
> > it supports specific page size otherwise use global page sizes.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > ---
> >  drivers/iommu/virtio-iommu.c      | 24 ++++++++++++++++++++----
> >  include/uapi/linux/virtio_iommu.h |  6 ++++++
> >  2 files changed, 26 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c
> > b/drivers/iommu/virtio-iommu.c index cce329d71fba..e69347ca4ee6 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -78,6 +78,7 @@ struct viommu_endpoint {
> >  	struct viommu_dev		*viommu;
> >  	struct viommu_domain		*vdomain;
> >  	struct list_head		resv_regions;
> > +	u64				pgsize_bitmap;
> >  };
> >
> >  struct viommu_request {
> > @@ -415,6 +416,14 @@ static int viommu_replay_mappings(struct
> viommu_domain *vdomain)
> >  	return ret;
> >  }
> >
> > +static int viommu_set_pgsize_bitmap(struct viommu_endpoint *vdev,
> > +				    struct virtio_iommu_probe_pgsize_mask *mask)
> > +
> > +{
> > +	vdev->pgsize_bitmap = mask->pgsize_bitmap;
> 
> We need to read this through le64_to_cpu(). Also check that the length of the field
> provided by the device is >= sizeof(mask) (like
> viommu_add_resv_mem() does)

Will take care of all the comments in next verions

Thank
-Bharat

> 
> > +	return 0;
> > +}
> > +
> >  static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
> >  			       struct virtio_iommu_probe_resv_mem *mem,
> >  			       size_t len)
> > @@ -494,11 +503,13 @@ static int viommu_probe_endpoint(struct viommu_dev
> *viommu, struct device *dev)
> >  	while (type != VIRTIO_IOMMU_PROBE_T_NONE &&
> >  	       cur < viommu->probe_size) {
> >  		len = le16_to_cpu(prop->length) + sizeof(*prop);
> > -
> >  		switch (type) {
> >  		case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
> >  			ret = viommu_add_resv_mem(vdev, (void *)prop, len);
> >  			break;
> > +		case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK:
> > +			ret = viommu_set_pgsize_bitmap(vdev, (void *)prop);
> > +			break;
> >  		default:
> >  			dev_err(dev, "unknown viommu prop 0x%x\n", type);
> >  		}
> > @@ -607,16 +618,21 @@ static struct iommu_domain
> *viommu_domain_alloc(unsigned type)
> >  	return &vdomain->domain;
> >  }
> >
> > -static int viommu_domain_finalise(struct viommu_dev *viommu,
> > +static int viommu_domain_finalise(struct viommu_endpoint *vdev,
> >  				  struct iommu_domain *domain)
> >  {
> >  	int ret;
> >  	struct viommu_domain *vdomain = to_viommu_domain(domain);
> > +	struct viommu_dev *viommu = vdev->viommu;
> >
> >  	vdomain->viommu		= viommu;
> >  	vdomain->map_flags	= viommu->map_flags;
> >
> > -	domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > +	if (vdev->pgsize_bitmap)
> > +		domain->pgsize_bitmap = vdev->pgsize_bitmap;
> > +	else
> > +		domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > +
> 
> nit: it could be nicer to initialize vdev->pgsize_bitmap in add_device(), override it
> in probe_endpoint(), and just copy it here.
> 
> >  	domain->geometry	= viommu->geometry;
> >
> >  	ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain, @@
> > -657,7 +673,7 @@ static int viommu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> >  		 * Properly initialize the domain now that we know which viommu
> >  		 * owns it.
> >  		 */
> > -		ret = viommu_domain_finalise(vdev->viommu, domain);
> > +		ret = viommu_domain_finalise(vdev, domain);
> 
> Attaching additional endpoints with different masks to the domain should return
> an error
> 
> >  	} else if (vdomain->viommu != vdev->viommu) {
> >  		dev_err(dev, "cannot attach to foreign vIOMMU\n");
> >  		ret = -EXDEV;
> > diff --git a/include/uapi/linux/virtio_iommu.h
> > b/include/uapi/linux/virtio_iommu.h
> > index 237e36a280cb..aff3db0ef54b 100644
> > --- a/include/uapi/linux/virtio_iommu.h
> > +++ b/include/uapi/linux/virtio_iommu.h
> > @@ -111,6 +111,7 @@ struct virtio_iommu_req_unmap {
> >
> >  #define VIRTIO_IOMMU_PROBE_T_NONE		0
> >  #define VIRTIO_IOMMU_PROBE_T_RESV_MEM		1
> > +#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK	2
> >
> >  #define VIRTIO_IOMMU_PROBE_T_MASK		0xfff
> >
> > @@ -119,6 +120,11 @@ struct virtio_iommu_probe_property {
> >  	__le16					length;
> >  };
> >
> > +struct virtio_iommu_probe_pgsize_mask {
> > +	struct virtio_iommu_probe_property	head;
> 
> Compilers will introduce 4 bytes of padding here, to align the next field.
> We need to make them explicit by adding a 4-bytes 'reserved' field.
> 
> > +	uint64_t				pgsize_bitmap;
> 
> __le64
> 
> Thanks,
> Jean
> 
> > +};
> > +
> >  #define VIRTIO_IOMMU_RESV_MEM_T_RESERVED	0
> >  #define VIRTIO_IOMMU_RESV_MEM_T_MSI		1
> >
> > --
> > 2.17.1
> >
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint
  2020-03-23  9:59 ` [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint Jean-Philippe Brucker
  2020-03-23 17:25   ` [EXT] " Bharat Bhushan
@ 2020-03-23 18:04   ` Bharat Bhushan
  2020-03-24  6:48     ` Jean-Philippe Brucker
  1 sibling, 1 reply; 5+ messages in thread
From: Bharat Bhushan @ 2020-03-23 18:04 UTC (permalink / raw)
  To: Jean-Philippe Brucker; +Cc: mst, virtualization, iommu, jasowang

Hi Jean,

> -----Original Message-----
> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Sent: Monday, March 23, 2020 3:30 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: joro@8bytes.org; mst@redhat.com; jasowang@redhat.com;
> virtualization@lists.linux-foundation.org; iommu@lists.linux-foundation.org;
> eric.auger@redhat.com
> Subject: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by
> endpoint
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Bharat,
> 
> Please add the IOMMU list on your next posting
> 
> On Mon, Mar 23, 2020 at 02:11:08PM +0530, Bharat Bhushan wrote:
> > Different endpoint can support different page size, probe endpoint if
> > it supports specific page size otherwise use global page sizes.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > ---
> >  drivers/iommu/virtio-iommu.c      | 24 ++++++++++++++++++++----
> >  include/uapi/linux/virtio_iommu.h |  6 ++++++
> >  2 files changed, 26 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c
> > b/drivers/iommu/virtio-iommu.c index cce329d71fba..e69347ca4ee6 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -78,6 +78,7 @@ struct viommu_endpoint {
> >  	struct viommu_dev		*viommu;
> >  	struct viommu_domain		*vdomain;
> >  	struct list_head		resv_regions;
> > +	u64				pgsize_bitmap;
> >  };
> >
> >  struct viommu_request {
> > @@ -415,6 +416,14 @@ static int viommu_replay_mappings(struct
> viommu_domain *vdomain)
> >  	return ret;
> >  }
> >
> > +static int viommu_set_pgsize_bitmap(struct viommu_endpoint *vdev,
> > +				    struct virtio_iommu_probe_pgsize_mask *mask)
> > +
> > +{
> > +	vdev->pgsize_bitmap = mask->pgsize_bitmap;
> 
> We need to read this through le64_to_cpu(). Also check that the length of the field
> provided by the device is >= sizeof(mask) (like
> viommu_add_resv_mem() does)
> 
> > +	return 0;
> > +}
> > +
> >  static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
> >  			       struct virtio_iommu_probe_resv_mem *mem,
> >  			       size_t len)
> > @@ -494,11 +503,13 @@ static int viommu_probe_endpoint(struct viommu_dev
> *viommu, struct device *dev)
> >  	while (type != VIRTIO_IOMMU_PROBE_T_NONE &&
> >  	       cur < viommu->probe_size) {
> >  		len = le16_to_cpu(prop->length) + sizeof(*prop);
> > -
> >  		switch (type) {
> >  		case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
> >  			ret = viommu_add_resv_mem(vdev, (void *)prop, len);
> >  			break;
> > +		case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK:
> > +			ret = viommu_set_pgsize_bitmap(vdev, (void *)prop);
> > +			break;
> >  		default:
> >  			dev_err(dev, "unknown viommu prop 0x%x\n", type);
> >  		}
> > @@ -607,16 +618,21 @@ static struct iommu_domain
> *viommu_domain_alloc(unsigned type)
> >  	return &vdomain->domain;
> >  }
> >
> > -static int viommu_domain_finalise(struct viommu_dev *viommu,
> > +static int viommu_domain_finalise(struct viommu_endpoint *vdev,
> >  				  struct iommu_domain *domain)
> >  {
> >  	int ret;
> >  	struct viommu_domain *vdomain = to_viommu_domain(domain);
> > +	struct viommu_dev *viommu = vdev->viommu;
> >
> >  	vdomain->viommu		= viommu;
> >  	vdomain->map_flags	= viommu->map_flags;
> >
> > -	domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > +	if (vdev->pgsize_bitmap)
> > +		domain->pgsize_bitmap = vdev->pgsize_bitmap;
> > +	else
> > +		domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > +
> 
> nit: it could be nicer to initialize vdev->pgsize_bitmap in add_device(),

To what size we should initialize in add_device, PAGE_SIZE?

Thanks
-Bharat

> override it
> in probe_endpoint(), and just copy it here.
> 
> >  	domain->geometry	= viommu->geometry;
> >
> >  	ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain, @@
> > -657,7 +673,7 @@ static int viommu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
> >  		 * Properly initialize the domain now that we know which viommu
> >  		 * owns it.
> >  		 */
> > -		ret = viommu_domain_finalise(vdev->viommu, domain);
> > +		ret = viommu_domain_finalise(vdev, domain);
> 
> Attaching additional endpoints with different masks to the domain should return
> an error
> 
> >  	} else if (vdomain->viommu != vdev->viommu) {
> >  		dev_err(dev, "cannot attach to foreign vIOMMU\n");
> >  		ret = -EXDEV;
> > diff --git a/include/uapi/linux/virtio_iommu.h
> > b/include/uapi/linux/virtio_iommu.h
> > index 237e36a280cb..aff3db0ef54b 100644
> > --- a/include/uapi/linux/virtio_iommu.h
> > +++ b/include/uapi/linux/virtio_iommu.h
> > @@ -111,6 +111,7 @@ struct virtio_iommu_req_unmap {
> >
> >  #define VIRTIO_IOMMU_PROBE_T_NONE		0
> >  #define VIRTIO_IOMMU_PROBE_T_RESV_MEM		1
> > +#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK	2
> >
> >  #define VIRTIO_IOMMU_PROBE_T_MASK		0xfff
> >
> > @@ -119,6 +120,11 @@ struct virtio_iommu_probe_property {
> >  	__le16					length;
> >  };
> >
> > +struct virtio_iommu_probe_pgsize_mask {
> > +	struct virtio_iommu_probe_property	head;
> 
> Compilers will introduce 4 bytes of padding here, to align the next field.
> We need to make them explicit by adding a 4-bytes 'reserved' field.
> 
> > +	uint64_t				pgsize_bitmap;
> 
> __le64
> 
> Thanks,
> Jean
> 
> > +};
> > +
> >  #define VIRTIO_IOMMU_RESV_MEM_T_RESERVED	0
> >  #define VIRTIO_IOMMU_RESV_MEM_T_MSI		1
> >
> > --
> > 2.17.1
> >
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint
  2020-03-23 18:04   ` Bharat Bhushan
@ 2020-03-24  6:48     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 5+ messages in thread
From: Jean-Philippe Brucker @ 2020-03-24  6:48 UTC (permalink / raw)
  To: Bharat Bhushan; +Cc: mst, virtualization, iommu, jasowang

On Mon, Mar 23, 2020 at 06:04:37PM +0000, Bharat Bhushan wrote:
> > > -static int viommu_domain_finalise(struct viommu_dev *viommu,
> > > +static int viommu_domain_finalise(struct viommu_endpoint *vdev,
> > >  				  struct iommu_domain *domain)
> > >  {
> > >  	int ret;
> > >  	struct viommu_domain *vdomain = to_viommu_domain(domain);
> > > +	struct viommu_dev *viommu = vdev->viommu;
> > >
> > >  	vdomain->viommu		= viommu;
> > >  	vdomain->map_flags	= viommu->map_flags;
> > >
> > > -	domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > > +	if (vdev->pgsize_bitmap)
> > > +		domain->pgsize_bitmap = vdev->pgsize_bitmap;
> > > +	else
> > > +		domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > > +
> > 
> > nit: it could be nicer to initialize vdev->pgsize_bitmap in add_device(),
> 
> To what size we should initialize in add_device, PAGE_SIZE?

No to viommu->pgsize_bitmap

Thanks,
Jean

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint
  2020-03-23 17:25   ` [EXT] " Bharat Bhushan
@ 2020-03-24  7:58     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 5+ messages in thread
From: Jean-Philippe Brucker @ 2020-03-24  7:58 UTC (permalink / raw)
  To: Bharat Bhushan; +Cc: mst, virtualization, iommu, jasowang

On Mon, Mar 23, 2020 at 05:25:17PM +0000, Bharat Bhushan wrote:
> Hi Jean,
> 
> > -----Original Message-----
> > From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > Sent: Monday, March 23, 2020 3:30 PM
> > To: Bharat Bhushan <bbhushan2@marvell.com>
> > Cc: joro@8bytes.org; mst@redhat.com; jasowang@redhat.com;
> > virtualization@lists.linux-foundation.org; iommu@lists.linux-foundation.org;
> > eric.auger@redhat.com
> > Subject: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by
> > endpoint
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > Hi Bharat,
> > 
> > Please add the IOMMU list on your next posting
> 
> iommu@lists.linux-foundation.org is there, any other mailing list we need to add?

I added that address when replying :) It doesn't look like your patch
reached the list:
https://lore.kernel.org/linux-iommu/20200323095943.GA2038940@myrica/T/
But virtualization and iommu lists are enough.

Thanks,
Jean

> 
> > 
> > On Mon, Mar 23, 2020 at 02:11:08PM +0530, Bharat Bhushan wrote:
> > > Different endpoint can support different page size, probe endpoint if
> > > it supports specific page size otherwise use global page sizes.
> > >
> > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > > ---
> > >  drivers/iommu/virtio-iommu.c      | 24 ++++++++++++++++++++----
> > >  include/uapi/linux/virtio_iommu.h |  6 ++++++
> > >  2 files changed, 26 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/iommu/virtio-iommu.c
> > > b/drivers/iommu/virtio-iommu.c index cce329d71fba..e69347ca4ee6 100644
> > > --- a/drivers/iommu/virtio-iommu.c
> > > +++ b/drivers/iommu/virtio-iommu.c
> > > @@ -78,6 +78,7 @@ struct viommu_endpoint {
> > >  	struct viommu_dev		*viommu;
> > >  	struct viommu_domain		*vdomain;
> > >  	struct list_head		resv_regions;
> > > +	u64				pgsize_bitmap;
> > >  };
> > >
> > >  struct viommu_request {
> > > @@ -415,6 +416,14 @@ static int viommu_replay_mappings(struct
> > viommu_domain *vdomain)
> > >  	return ret;
> > >  }
> > >
> > > +static int viommu_set_pgsize_bitmap(struct viommu_endpoint *vdev,
> > > +				    struct virtio_iommu_probe_pgsize_mask *mask)
> > > +
> > > +{
> > > +	vdev->pgsize_bitmap = mask->pgsize_bitmap;
> > 
> > We need to read this through le64_to_cpu(). Also check that the length of the field
> > provided by the device is >= sizeof(mask) (like
> > viommu_add_resv_mem() does)
> 
> Will take care of all the comments in next verions
> 
> Thank
> -Bharat
> 
> > 
> > > +	return 0;
> > > +}
> > > +
> > >  static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
> > >  			       struct virtio_iommu_probe_resv_mem *mem,
> > >  			       size_t len)
> > > @@ -494,11 +503,13 @@ static int viommu_probe_endpoint(struct viommu_dev
> > *viommu, struct device *dev)
> > >  	while (type != VIRTIO_IOMMU_PROBE_T_NONE &&
> > >  	       cur < viommu->probe_size) {
> > >  		len = le16_to_cpu(prop->length) + sizeof(*prop);
> > > -
> > >  		switch (type) {
> > >  		case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
> > >  			ret = viommu_add_resv_mem(vdev, (void *)prop, len);
> > >  			break;
> > > +		case VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK:
> > > +			ret = viommu_set_pgsize_bitmap(vdev, (void *)prop);
> > > +			break;
> > >  		default:
> > >  			dev_err(dev, "unknown viommu prop 0x%x\n", type);
> > >  		}
> > > @@ -607,16 +618,21 @@ static struct iommu_domain
> > *viommu_domain_alloc(unsigned type)
> > >  	return &vdomain->domain;
> > >  }
> > >
> > > -static int viommu_domain_finalise(struct viommu_dev *viommu,
> > > +static int viommu_domain_finalise(struct viommu_endpoint *vdev,
> > >  				  struct iommu_domain *domain)
> > >  {
> > >  	int ret;
> > >  	struct viommu_domain *vdomain = to_viommu_domain(domain);
> > > +	struct viommu_dev *viommu = vdev->viommu;
> > >
> > >  	vdomain->viommu		= viommu;
> > >  	vdomain->map_flags	= viommu->map_flags;
> > >
> > > -	domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > > +	if (vdev->pgsize_bitmap)
> > > +		domain->pgsize_bitmap = vdev->pgsize_bitmap;
> > > +	else
> > > +		domain->pgsize_bitmap	= viommu->pgsize_bitmap;
> > > +
> > 
> > nit: it could be nicer to initialize vdev->pgsize_bitmap in add_device(), override it
> > in probe_endpoint(), and just copy it here.
> > 
> > >  	domain->geometry	= viommu->geometry;
> > >
> > >  	ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain, @@
> > > -657,7 +673,7 @@ static int viommu_attach_dev(struct iommu_domain
> > *domain, struct device *dev)
> > >  		 * Properly initialize the domain now that we know which viommu
> > >  		 * owns it.
> > >  		 */
> > > -		ret = viommu_domain_finalise(vdev->viommu, domain);
> > > +		ret = viommu_domain_finalise(vdev, domain);
> > 
> > Attaching additional endpoints with different masks to the domain should return
> > an error
> > 
> > >  	} else if (vdomain->viommu != vdev->viommu) {
> > >  		dev_err(dev, "cannot attach to foreign vIOMMU\n");
> > >  		ret = -EXDEV;
> > > diff --git a/include/uapi/linux/virtio_iommu.h
> > > b/include/uapi/linux/virtio_iommu.h
> > > index 237e36a280cb..aff3db0ef54b 100644
> > > --- a/include/uapi/linux/virtio_iommu.h
> > > +++ b/include/uapi/linux/virtio_iommu.h
> > > @@ -111,6 +111,7 @@ struct virtio_iommu_req_unmap {
> > >
> > >  #define VIRTIO_IOMMU_PROBE_T_NONE		0
> > >  #define VIRTIO_IOMMU_PROBE_T_RESV_MEM		1
> > > +#define VIRTIO_IOMMU_PROBE_T_PAGE_SIZE_MASK	2
> > >
> > >  #define VIRTIO_IOMMU_PROBE_T_MASK		0xfff
> > >
> > > @@ -119,6 +120,11 @@ struct virtio_iommu_probe_property {
> > >  	__le16					length;
> > >  };
> > >
> > > +struct virtio_iommu_probe_pgsize_mask {
> > > +	struct virtio_iommu_probe_property	head;
> > 
> > Compilers will introduce 4 bytes of padding here, to align the next field.
> > We need to make them explicit by adding a 4-bytes 'reserved' field.
> > 
> > > +	uint64_t				pgsize_bitmap;
> > 
> > __le64
> > 
> > Thanks,
> > Jean
> > 
> > > +};
> > > +
> > >  #define VIRTIO_IOMMU_RESV_MEM_T_RESERVED	0
> > >  #define VIRTIO_IOMMU_RESV_MEM_T_MSI		1
> > >
> > > --
> > > 2.17.1
> > >
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200323084108.1721-1-bbhushan2@marvell.com>
2020-03-23  9:59 ` [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint Jean-Philippe Brucker
2020-03-23 17:25   ` [EXT] " Bharat Bhushan
2020-03-24  7:58     ` Jean-Philippe Brucker
2020-03-23 18:04   ` Bharat Bhushan
2020-03-24  6:48     ` Jean-Philippe Brucker

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org
	public-inbox-index linux-iommu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git