[PATCH v6] iommu/virtio: Use page size bitmap supported by endpoint
Different endpoint can support different page size, probe
endpoint if it supports specific page size otherwise use
global page sizes.
Device attached to domain should support a minimum of
domain supported page sizes. If device supports more
than domain supported page sizes then device is limited
to use domain supported page sizes only.
Signed-off-by: Bharat Bhushan
---
v5->v6
- property length before dereference
- Error out on no supported page sizes (page-size-mask is zero)
- Allow device to attach to domain even it supports
minimum of domain supported page sizes. In that case device
will use domain page sizes only.
- added format of pgsize_bitmap
v4->v5:
- Rebase to Linux v5.7-rc4
v3->v4:
- Fix whitespace error
v2->v3:
- Fixed error return for incompatible endpoint
- __u64 changed to __le64 in header file
drivers/iommu/virtio-iommu.c | 63 ---
include/uapi/linux/virtio_iommu.h | 14 ++-
2 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 4e1d11af23c8..cbac3047a781 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_headresv_regions;
+ u64 pgsize_bitmap;
};
struct viommu_request {
@@ -415,6 +416,23 @@ 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,
+ size_t len)
+{
+ u64 pgsize_bitmap;
+
+ if (len < sizeof(*mask))
+ return -EINVAL;
+
+ pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
+ if (!pgsize_bitmap)
+ return -EINVAL;
+
+ vdev->pgsize_bitmap = pgsize_bitmap;
+ return 0;
+}
+
static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
struct virtio_iommu_probe_resv_mem *mem,
size_t len)
@@ -499,6 +517,9 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu,
struct device *dev)
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, len);
+ break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
}
@@ -615,7 +636,7 @@ static int viommu_domain_finalise(struct viommu_endpoint
*vdev,
struct viommu_dev *viommu = vdev->viommu;
struct viommu_domain *vdomain = to_viommu_domain(domain);
- viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);
+ viommu_page_size = 1UL << __ffs(vdev->pgsize_bitmap);
if (viommu_page_size > PAGE_SIZE) {
dev_err(vdev->dev,
"granule 0x%lx larger than system page size 0x%lx\n",
@@ -630,7 +651,7 @@ static int viommu_domain_finalise(struct viommu_endpoint
*vdev,
vdomain->id = (unsigned int)ret;
- domain->pgsize_bitmap = viommu->pgsize_bitmap;
+ domain->pgsize_bitmap = vdev->pgsize_bitmap;
domain->geometry= viommu->geometry;
vdomain->map_flags = viommu->map_flags;
@@ -654,6 +675,38 @@ static void viommu_domain_free(struct iommu_domain *domain)
kfree(vdomain);
}
+/*
+ * Check whether the endpoint's capabilities are compatible with other
+ * endpoints in the domain. Report any inconsistency.
+ */
+static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
+ struct viommu_domain *vdomain)
+{
+ struct device *dev = vdev->dev;
+ u64 pgsize_bitmap;
+
+ if (vdomain->viommu != vdev->viommu) {
+ dev_err(dev, "cannot attach to foreign vIOMMU\n");
+ return false;
+ }
+
+ pgsize_bitmap = vdomain->domain.pgsize_bitmap & vdev->pgsize_bitmap;
+
+ if (pgsize_bitmap != vdomain->domain.pgsize_bitmap) {
+ dev_err(dev, "incompatible domain bitmap 0x%lx != 0x%llx\n",
+ vdomain->domain.pgsize_bitmap, vdev->pgsize_bitmap);
+ return false;
+ }
+
+ /* Domain pagesize bitmap is subset of device pagesize bitmap */
+ if (pgsize_bitmap != vdev->pgsize_bitmap) {
+ dev_info(dev, "page size bitmap used %llx, supported %llx\n",
+p
[PATCH v6] iommu/virtio: Use page size bitmap supported by endpoint
Different endpoint can support different page size, probe
endpoint if it supports specific page size otherwise use
global page sizes.
Device attached to domain should support a minimum of
domain supported page sizes. If device supports more
than domain supported page sizes then device is limited
to use domain supported page sizes only.
Signed-off-by: Bharat Bhushan
---
v5->v6
- property length before dereference
- Error out on no supported page sizes (page-size-mask is zero)
- Allow device to attach to domain even it supports
minimum of domain supported page sizes. In that case device
will use domain page sizes only.
- added format of pgsize_bitmap
v4->v5:
- Rebase to Linux v5.7-rc4
v3->v4:
- Fix whitespace error
v2->v3:
- Fixed error return for incompatible endpoint
- __u64 changed to __le64 in header file
drivers/iommu/virtio-iommu.c | 63 ---
include/uapi/linux/virtio_iommu.h | 14 ++-
2 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 4e1d11af23c8..cbac3047a781 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_headresv_regions;
+ u64 pgsize_bitmap;
};
struct viommu_request {
@@ -415,6 +416,23 @@ 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,
+ size_t len)
+{
+ u64 pgsize_bitmap;
+
+ if (len < sizeof(*mask))
+ return -EINVAL;
+
+ pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
+ if (!pgsize_bitmap)
+ return -EINVAL;
+
+ vdev->pgsize_bitmap = pgsize_bitmap;
+ return 0;
+}
+
static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
struct virtio_iommu_probe_resv_mem *mem,
size_t len)
@@ -499,6 +517,9 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu,
struct device *dev)
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, len);
+ break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
}
@@ -615,7 +636,7 @@ static int viommu_domain_finalise(struct viommu_endpoint
*vdev,
struct viommu_dev *viommu = vdev->viommu;
struct viommu_domain *vdomain = to_viommu_domain(domain);
- viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);
+ viommu_page_size = 1UL << __ffs(vdev->pgsize_bitmap);
if (viommu_page_size > PAGE_SIZE) {
dev_err(vdev->dev,
"granule 0x%lx larger than system page size 0x%lx\n",
@@ -630,7 +651,7 @@ static int viommu_domain_finalise(struct viommu_endpoint
*vdev,
vdomain->id = (unsigned int)ret;
- domain->pgsize_bitmap = viommu->pgsize_bitmap;
+ domain->pgsize_bitmap = vdev->pgsize_bitmap;
domain->geometry= viommu->geometry;
vdomain->map_flags = viommu->map_flags;
@@ -654,6 +675,38 @@ static void viommu_domain_free(struct iommu_domain *domain)
kfree(vdomain);
}
+/*
+ * Check whether the endpoint's capabilities are compatible with other
+ * endpoints in the domain. Report any inconsistency.
+ */
+static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
+ struct viommu_domain *vdomain)
+{
+ struct device *dev = vdev->dev;
+ u64 pgsize_bitmap;
+
+ if (vdomain->viommu != vdev->viommu) {
+ dev_err(dev, "cannot attach to foreign vIOMMU\n");
+ return false;
+ }
+
+ pgsize_bitmap = vdomain->domain.pgsize_bitmap & vdev->pgsize_bitmap;
+
+ if (pgsize_bitmap != vdomain->domain.pgsize_bitmap) {
+ dev_err(dev, "incompatible domain bitmap 0x%lx != 0x%llx\n",
+ vdomain->domain.pgsize_bitmap, vdev->pgsize_bitmap);
+ return false;
+ }
+
+ /* Domain pagesize bitmap is subset of device pagesize bitmap */
+ if (pgsize_bitmap != vdev->pgsize_bitmap) {
+ dev_info(dev, "page size bitmap used %llx, supported %llx\n",
+p
RE: [EXT] Re: [PATCH v5] iommu/virtio: Use page size bitmap supported by endpoint
Hi Jean,
> -Original Message-
> From: Michael S. Tsirkin
> Sent: Wednesday, May 6, 2020 5:53 AM
> To: Bharat Bhushan
> Cc: jean-phili...@linaro.org; j...@8bytes.org; jasow...@redhat.com;
> virtualizat...@lists.linux-foundation.org; iommu@lists.linux-foundation.org;
> linux-ker...@vger.kernel.org; eric.auger@gmail.com; eric.au...@redhat.com
> Subject: [EXT] Re: [PATCH v5] iommu/virtio: Use page size bitmap supported by
> endpoint
>
> External Email
>
> --
> On Tue, May 05, 2020 at 03:00:04PM +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
> > ---
> > v4->v5:
> > - Rebase to Linux v5.7-rc4
> >
> > v3->v4:
> > - Fix whitespace error
> >
> > v2->v3:
> > - Fixed error return for incompatible endpoint
> > - __u64 changed to __le64 in header file
> >
> > drivers/iommu/virtio-iommu.c | 48 ---
> > include/uapi/linux/virtio_iommu.h | 7 +
> > 2 files changed, 51 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c
> > b/drivers/iommu/virtio-iommu.c index d5cac4f46ca5..9513d2ab819e 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_headresv_regions;
> > + u64 pgsize_bitmap;
> > };
> >
> > struct viommu_request {
> > @@ -415,6 +416,19 @@ 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,
> > + size_t len)
> > +{
> > + u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
> > +
> > + if (len < sizeof(*mask))
>
> This is too late to validate length, you have dereferenced it already.
> do it before the read pls.
>
> > + return -EINVAL;
>
> OK but note that guest will then just proceed to ignore the property. Is that
> really
> OK? Wouldn't host want to know?
>
>
> > +
> > + vdev->pgsize_bitmap = pgsize_bitmap;
>
> what if bitmap is 0? Is that a valid size? I see a bunch of BUG_ON with that
> value ...
>
> I also see a bunch of code like e.g. this:
>
> pg_size = 1UL << __ffs(pgsize_bitmap);
>
> which probably won't DTRT on a 32 bit guest if the bitmap has bits set in the
> high
> word.
>
>
>
> > + return 0;
> > +}
> > +
> > static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
> >struct virtio_iommu_probe_resv_mem *mem,
> >size_t len)
> > @@ -499,6 +513,9 @@ static int viommu_probe_endpoint(struct viommu_dev
> *viommu, struct device *dev)
> > 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, len);
> > + break;
> > default:
> > dev_err(dev, "unknown viommu prop 0x%x\n", type);
> > }
> > @@ -630,7 +647,7 @@ static int viommu_domain_finalise(struct
> > viommu_endpoint *vdev,
> >
> > vdomain->id = (unsigned int)ret;
> >
> > - domain->pgsize_bitmap = viommu->pgsize_bitmap;
> > + domain->pgsize_bitmap = vdev->pgsize_bitmap;
> > domain->geometry= viommu->geometry;
> >
> > vdomain->map_flags = viommu->map_flags;
> > @@ -654,6 +671,29 @@ static void viommu_domain_free(struct iommu_domain
> *domain)
> > kfree(vdomain);
> > }
> >
> > +/*
> > + * Check whether the endpoint's capabilities are compatible with
> > +other
> > + * endpoints in the domain. Report any inconsistency.
> > + */
> > +static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
> > + struct viommu_domain *vdoma
RE: [EXT] Re: [PATCH v5] iommu/virtio: Use page size bitmap supported by endpoint
> -Original Message-
> From: Michael S. Tsirkin
> Sent: Wednesday, May 6, 2020 5:53 AM
> To: Bharat Bhushan
> Cc: jean-phili...@linaro.org; j...@8bytes.org; jasow...@redhat.com;
> virtualizat...@lists.linux-foundation.org; iommu@lists.linux-foundation.org;
> linux-ker...@vger.kernel.org; eric.auger@gmail.com; eric.au...@redhat.com
> Subject: [EXT] Re: [PATCH v5] iommu/virtio: Use page size bitmap supported by
> endpoint
>
> External Email
>
> --
> On Tue, May 05, 2020 at 03:00:04PM +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
> > ---
> > v4->v5:
> > - Rebase to Linux v5.7-rc4
> >
> > v3->v4:
> > - Fix whitespace error
> >
> > v2->v3:
> > - Fixed error return for incompatible endpoint
> > - __u64 changed to __le64 in header file
> >
> > drivers/iommu/virtio-iommu.c | 48 ---
> > include/uapi/linux/virtio_iommu.h | 7 +
> > 2 files changed, 51 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c
> > b/drivers/iommu/virtio-iommu.c index d5cac4f46ca5..9513d2ab819e 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_headresv_regions;
> > + u64 pgsize_bitmap;
> > };
> >
> > struct viommu_request {
> > @@ -415,6 +416,19 @@ 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,
> > + size_t len)
> > +{
> > + u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
> > +
> > + if (len < sizeof(*mask))
>
> This is too late to validate length, you have dereferenced it already.
> do it before the read pls.
Yes, Will change here and other places as well
>
> > + return -EINVAL;
>
> OK but note that guest will then just proceed to ignore the property. Is that
> really
> OK? Wouldn't host want to know?
Guest need to be in sync with device, so yes seems like guest need to tell
device which page-size-mask it is using.
Corresponding spec change patch
(https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg06214.html)
Would like Jean/Eric to comment here as well.
>
>
> > +
> > + vdev->pgsize_bitmap = pgsize_bitmap;
>
> what if bitmap is 0? Is that a valid size? I see a bunch of BUG_ON with that
> value ...
As per spec proposed device is supposed to set at-least one bit.
Will add a bug_on her.
Should we add bug_on or switch to global config page-size mask if this is zero
(notify device which page-size-mask it is using).
>
> I also see a bunch of code like e.g. this:
>
> pg_size = 1UL << __ffs(pgsize_bitmap);
>
> which probably won't DTRT on a 32 bit guest if the bitmap has bits set in the
> high
> word.
>
My thought is that in that case viommu_domain_finalise() will fail, do not
proceed.
>
>
> > + return 0;
> > +}
> > +
> > static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
> >struct virtio_iommu_probe_resv_mem *mem,
> >size_t len)
> > @@ -499,6 +513,9 @@ static int viommu_probe_endpoint(struct viommu_dev
> *viommu, struct device *dev)
> > 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, len);
> > + break;
> > default:
> > dev_err(dev, "unknown viommu prop 0x%x\n", type);
> > }
> > @@ -630,7 +647,7 @@ static int viommu_domain_finalise(struct
> > viommu_endpoint *vdev,
> >
> > vdomain->id = (unsigned int)ret;
> >
> > - domain->pgsize_bitmap = viommu->pgsize_bitmap;
> > + domain->pgsize_bitmap = vdev->pgsize_bitmap;
> >
[PATCH v5] iommu/virtio: Use page size bitmap supported by endpoint
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
---
v4->v5:
- Rebase to Linux v5.7-rc4
v3->v4:
- Fix whitespace error
v2->v3:
- Fixed error return for incompatible endpoint
- __u64 changed to __le64 in header file
drivers/iommu/virtio-iommu.c | 48 ---
include/uapi/linux/virtio_iommu.h | 7 +
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index d5cac4f46ca5..9513d2ab819e 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_headresv_regions;
+ u64 pgsize_bitmap;
};
struct viommu_request {
@@ -415,6 +416,19 @@ 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,
+ size_t len)
+{
+ u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
+
+ if (len < sizeof(*mask))
+ return -EINVAL;
+
+ vdev->pgsize_bitmap = pgsize_bitmap;
+ return 0;
+}
+
static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
struct virtio_iommu_probe_resv_mem *mem,
size_t len)
@@ -499,6 +513,9 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu,
struct device *dev)
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, len);
+ break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
}
@@ -630,7 +647,7 @@ static int viommu_domain_finalise(struct viommu_endpoint
*vdev,
vdomain->id = (unsigned int)ret;
- domain->pgsize_bitmap = viommu->pgsize_bitmap;
+ domain->pgsize_bitmap = vdev->pgsize_bitmap;
domain->geometry= viommu->geometry;
vdomain->map_flags = viommu->map_flags;
@@ -654,6 +671,29 @@ static void viommu_domain_free(struct iommu_domain *domain)
kfree(vdomain);
}
+/*
+ * Check whether the endpoint's capabilities are compatible with other
+ * endpoints in the domain. Report any inconsistency.
+ */
+static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
+ struct viommu_domain *vdomain)
+{
+ struct device *dev = vdev->dev;
+
+ if (vdomain->viommu != vdev->viommu) {
+ dev_err(dev, "cannot attach to foreign vIOMMU\n");
+ return false;
+ }
+
+ if (vdomain->domain.pgsize_bitmap != vdev->pgsize_bitmap) {
+ dev_err(dev, "incompatible domain bitmap 0x%lx != 0x%llx\n",
+ vdomain->domain.pgsize_bitmap, vdev->pgsize_bitmap);
+ return false;
+ }
+
+ return true;
+}
+
static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int i;
@@ -670,9 +710,8 @@ static int viommu_attach_dev(struct iommu_domain *domain,
struct device *dev)
* owns it.
*/
ret = viommu_domain_finalise(vdev, domain);
- } else if (vdomain->viommu != vdev->viommu) {
- dev_err(dev, "cannot attach to foreign vIOMMU\n");
- ret = -EXDEV;
+ } else if (!viommu_endpoint_is_compatible(vdev, vdomain)) {
+ ret = -EINVAL;
}
mutex_unlock(&vdomain->mutex);
@@ -886,6 +925,7 @@ static int viommu_add_device(struct device *dev)
vdev->dev = dev;
vdev->viommu = viommu;
+ vdev->pgsize_bitmap = viommu->pgsize_bitmap;
INIT_LIST_HEAD(&vdev->resv_regions);
dev_iommu_priv_set(dev, vdev);
diff --git a/include/uapi/linux/virtio_iommu.h
b/include/uapi/linux/virtio_iommu.h
index 48e3c29223b5..2cced7accc99 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_MASK2
#define VIRTIO_IOMMU_PROBE_T_MASK 0xfff
@@ -119,6 +120,12 @@ struct virtio_iommu_probe_property {
[RFC PATCH v4] iommu/virtio: Use page size bitmap supported by endpoint
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
---
v3->v4:
- Fix whitespace error
v2->v3:
- Fixed error return for incompatible endpoint
- __u64 changed to __le64 in header file
drivers/iommu/virtio-iommu.c | 53 +++
include/uapi/linux/virtio_iommu.h | 7
2 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index cce329d71fba..ab09c04b1702 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_headresv_regions;
+ u64 pgsize_bitmap;
};
struct viommu_request {
@@ -415,6 +416,19 @@ 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,
+ size_t len)
+{
+ u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
+
+ if (len < sizeof(*mask))
+ return -EINVAL;
+
+ vdev->pgsize_bitmap = pgsize_bitmap;
+ return 0;
+}
+
static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
struct virtio_iommu_probe_resv_mem *mem,
size_t len)
@@ -499,6 +513,9 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu,
struct device *dev)
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, len);
+ break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
}
@@ -607,16 +624,17 @@ 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;
+ domain->pgsize_bitmap = vdev->pgsize_bitmap;
domain->geometry= viommu->geometry;
ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain,
@@ -642,6 +660,29 @@ static void viommu_domain_free(struct iommu_domain *domain)
kfree(vdomain);
}
+/*
+ * Check whether the endpoint's capabilities are compatible with other
+ * endpoints in the domain. Report any inconsistency.
+ */
+static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
+ struct viommu_domain *vdomain)
+{
+ struct device *dev = vdev->dev;
+
+ if (vdomain->viommu != vdev->viommu) {
+ dev_err(dev, "cannot attach to foreign vIOMMU\n");
+ return false;
+ }
+
+ if (vdomain->domain.pgsize_bitmap != vdev->pgsize_bitmap) {
+ dev_err(dev, "incompatible domain bitmap 0x%lx != 0x%llx\n",
+ vdomain->domain.pgsize_bitmap, vdev->pgsize_bitmap);
+ return false;
+ }
+
+ return true;
+}
+
static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int i;
@@ -657,10 +698,9 @@ 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);
- } else if (vdomain->viommu != vdev->viommu) {
- dev_err(dev, "cannot attach to foreign vIOMMU\n");
- ret = -EXDEV;
+ ret = viommu_domain_finalise(vdev, domain);
+ } else if (!viommu_endpoint_is_compatible(vdev, vdomain)) {
+ ret = -EINVAL;
}
mutex_unlock(&vdomain->mutex);
@@ -875,6 +915,7 @@ static int viommu_add_device(struct device *dev)
vdev->dev = dev;
vdev->viommu = viommu;
+ vdev->pgsize_bitmap = viommu->pgsize_bitmap;
INIT_LIST_HEAD(&vdev->resv_regions);
fws
[RFC PATCH v3] iommu/virtio: Use page size bitmap supported by endpoint
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
---
drivers/iommu/virtio-iommu.c | 54 +++
include/uapi/linux/virtio_iommu.h | 7
2 files changed, 55 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index cce329d71fba..2ea9e143d95c 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_headresv_regions;
+ u64 pgsize_bitmap;
};
struct viommu_request {
@@ -415,6 +416,20 @@ 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,
+ size_t len)
+
+{
+ u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
+
+ if (len < sizeof(*mask))
+ return -EINVAL;
+
+ vdev->pgsize_bitmap = pgsize_bitmap;
+ return 0;
+}
+
static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
struct virtio_iommu_probe_resv_mem *mem,
size_t len)
@@ -499,6 +514,9 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu,
struct device *dev)
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, len);
+ break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
}
@@ -607,16 +625,17 @@ 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;
+ domain->pgsize_bitmap = vdev->pgsize_bitmap;
domain->geometry= viommu->geometry;
ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain,
@@ -642,6 +661,29 @@ static void viommu_domain_free(struct iommu_domain *domain)
kfree(vdomain);
}
+/*
+ * Check whether the endpoint's capabilities are compatible with other
+ * endpoints in the domain. Report any inconsistency.
+ */
+static bool viommu_endpoint_is_compatible(struct viommu_endpoint *vdev,
+ struct viommu_domain *vdomain)
+{
+ struct device *dev = vdev->dev;
+
+ if (vdomain->viommu != vdev->viommu) {
+ dev_err(dev, "cannot attach to foreign vIOMMU\n");
+ return false;
+ }
+
+ if (vdomain->domain.pgsize_bitmap != vdev->pgsize_bitmap) {
+ dev_err(dev, "incompatible domain bitmap 0x%lx != 0x%llx\n",
+ vdomain->domain.pgsize_bitmap, vdev->pgsize_bitmap);
+ return false;
+ }
+
+ return true;
+}
+
static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int i;
@@ -657,10 +699,9 @@ 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);
- } else if (vdomain->viommu != vdev->viommu) {
- dev_err(dev, "cannot attach to foreign vIOMMU\n");
- ret = -EXDEV;
+ ret = viommu_domain_finalise(vdev, domain);
+ } else if (!viommu_endpoint_is_compatible(vdev, vdomain)) {
+ ret = -EINVAL;
}
mutex_unlock(&vdomain->mutex);
@@ -875,6 +916,7 @@ static int viommu_add_device(struct device *dev)
vdev->dev = dev;
vdev->viommu = viommu;
+ vdev->pgsize_bitmap = viommu->pgsize_bitmap;
INIT_LIST_HEAD(&vdev->resv_regions);
fwspec->iommu_priv = vdev;
diff --git a/include/uapi/linux/virtio_iommu.h
b/include/uapi/linux/virtio_iommu.h
index 237e36a280cb..bd
RE: [EXT] Re: [RFC PATCH v2] iommu/virtio: Use page size bitmap supported by endpoint
> -Original Message-
> From: Jean-Philippe Brucker
> Sent: Wednesday, April 1, 2020 9:20 PM
> To: Robin Murphy
> Cc: Bharat Bhushan ; j...@8bytes.org;
> m...@redhat.com; jasow...@redhat.com; virtualization@lists.linux-
> foundation.org; iommu@lists.linux-foundation.org;
> linux-ker...@vger.kernel.org;
> eric.auger@gmail.com; eric.au...@redhat.com
> Subject: [EXT] Re: [RFC PATCH v2] iommu/virtio: Use page size bitmap
> supported by
> endpoint
>
> External Email
>
> --
> On Wed, Apr 01, 2020 at 02:00:13PM +0100, Robin Murphy wrote:
> > On 2020-04-01 12:38 pm, 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
> > > ---
> > > drivers/iommu/virtio-iommu.c | 33 +++
> > > include/uapi/linux/virtio_iommu.h | 7 +++
> > > 2 files changed, 36 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/iommu/virtio-iommu.c
> > > b/drivers/iommu/virtio-iommu.c index cce329d71fba..c794cb5b7b3e
> > > 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_headresv_regions;
> > > + u64 pgsize_bitmap;
> > > };
> > > struct viommu_request {
> > > @@ -415,6 +416,20 @@ 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,
> > > + size_t len)
> > > +
> > > +{
> > > + u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
> > > +
> > > + if (len < sizeof(*mask))
> > > + return -EINVAL;
> > > +
> > > + vdev->pgsize_bitmap = pgsize_bitmap;
> > > + return 0;
> > > +}
> > > +
> > > static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
> > > struct virtio_iommu_probe_resv_mem *mem,
> > > size_t len)
> > > @@ -494,11 +509,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);
> > > -
>
> Whitespace change
>
> > > 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, len);
> > > + break;
> > > default:
> > > dev_err(dev, "unknown viommu prop 0x%x\n",
> > > type);
> > > }
> > > @@ -607,16 +624,23 @@ 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;
> > > + /* Devices in same domain must support same size pages */
> >
> > AFAICS what the code appears to do is enforce that the first endpoint
> > attached to any domain has the same pgsize_bitmap as the most recently
> > probed viommu_d
[RFC PATCH v2] iommu/virtio: Use page size bitmap supported by endpoint
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
---
drivers/iommu/virtio-iommu.c | 33 +++
include/uapi/linux/virtio_iommu.h | 7 +++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index cce329d71fba..c794cb5b7b3e 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_headresv_regions;
+ u64 pgsize_bitmap;
};
struct viommu_request {
@@ -415,6 +416,20 @@ 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,
+ size_t len)
+
+{
+ u64 pgsize_bitmap = le64_to_cpu(mask->pgsize_bitmap);
+
+ if (len < sizeof(*mask))
+ return -EINVAL;
+
+ vdev->pgsize_bitmap = pgsize_bitmap;
+ return 0;
+}
+
static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
struct virtio_iommu_probe_resv_mem *mem,
size_t len)
@@ -494,11 +509,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, len);
+ break;
default:
dev_err(dev, "unknown viommu prop 0x%x\n", type);
}
@@ -607,16 +624,23 @@ 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;
+ /* Devices in same domain must support same size pages */
+ if ((domain->pgsize_bitmap != viommu->pgsize_bitmap) &&
+ (domain->pgsize_bitmap != vdev->pgsize_bitmap))
+ return -EINVAL;
+
+ domain->pgsize_bitmap = vdev->pgsize_bitmap;
+
domain->geometry= viommu->geometry;
ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain,
@@ -657,7 +681,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);
} else if (vdomain->viommu != vdev->viommu) {
dev_err(dev, "cannot attach to foreign vIOMMU\n");
ret = -EXDEV;
@@ -875,6 +899,7 @@ static int viommu_add_device(struct device *dev)
vdev->dev = dev;
vdev->viommu = viommu;
+ vdev->pgsize_bitmap = viommu->pgsize_bitmap;
INIT_LIST_HEAD(&vdev->resv_regions);
fwspec->iommu_priv = vdev;
diff --git a/include/uapi/linux/virtio_iommu.h
b/include/uapi/linux/virtio_iommu.h
index 237e36a280cb..dc9d3f40bcd8 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_MASK2
#define VIRTIO_IOMMU_PROBE_T_MASK 0xfff
@@ -119,6 +120,12 @@ struct virtio_iommu_probe_property {
__le16 length;
};
+struct virtio_iommu_probe_pgsize_mask {
+ struct virtio_iommu_probe_property head;
+ __u8reserved[4];
+ __u64 pgsize_bitmap;
+};
+
#define VIRTIO_IOMMU_R
RE: [EXT] Re: [PATCH v2 3/3] iommu/virtio: Reject IOMMU page granule larger than PAGE_SIZE
Hi Jean,
> -Original Message-
> From: Auger Eric
> Sent: Thursday, March 26, 2020 11:11 PM
> To: Jean-Philippe Brucker ; iommu@lists.linux-
> foundation.org
> Cc: virtualizat...@lists.linux-foundation.org; j...@8bytes.org;
> m...@redhat.com;
> jasow...@redhat.com; Bharat Bhushan
> Subject: [EXT] Re: [PATCH v2 3/3] iommu/virtio: Reject IOMMU page granule
> larger
> than PAGE_SIZE
>
> External Email
>
> --
> Hi Jean,
>
> On 3/26/20 10:35 AM, Jean-Philippe Brucker wrote:
> > We don't currently support IOMMUs with a page granule larger than the
> > system page size. The IOVA allocator has a BUG_ON() in this case, and
> > VFIO has a WARN_ON().
> >
> > Removing these obstacles ranges doesn't seem possible without major
> > changes to the DMA API and VFIO. Some callers of iommu_map(), for
> > example, want to map multiple page-aligned regions adjacent to each
> > others for scatter-gather purposes. Even in simple DMA API uses, a
> > call to dma_map_page() would let the endpoint access neighbouring
> > memory. And VFIO users cannot ensure that their virtual address buffer
> > is physically contiguous at the IOMMU granule.
> >
> > Rather than triggering the IOVA BUG_ON() on mismatched page sizes,
> > abort the vdomain finalise() with an error message. We could simply
> > abort the viommu probe(), but an upcoming extension to virtio-iommu
> > will allow setting different page masks for each endpoint.
> >
> > Reported-by: Bharat Bhushan
> > Signed-off-by: Jean-Philippe Brucker
> Reviewed-by: Eric Auger
Reviewed-by: Eric Auger
Thanks
-Bharat
>
> Thanks
>
> Eric
> > ---
> > v1->v2: Move to vdomain_finalise(), improve commit message
> > ---
> > drivers/iommu/virtio-iommu.c | 14 --
> > 1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/virtio-iommu.c
> > b/drivers/iommu/virtio-iommu.c index 5eed75cd121f..750f69c49b95 100644
> > --- a/drivers/iommu/virtio-iommu.c
> > +++ b/drivers/iommu/virtio-iommu.c
> > @@ -607,12 +607,22 @@ 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;
> > + unsigned long viommu_page_size;
> > + struct viommu_dev *viommu = vdev->viommu;
> > struct viommu_domain *vdomain = to_viommu_domain(domain);
> >
> > + viommu_page_size = 1UL << __ffs(viommu->pgsize_bitmap);
> > + if (viommu_page_size > PAGE_SIZE) {
> > + dev_err(vdev->dev,
> > + "granule 0x%lx larger than system page size 0x%lx\n",
> > + viommu_page_size, PAGE_SIZE);
> > + return -EINVAL;
> > + }
> > +
> > ret = ida_alloc_range(&viommu->domain_ids, viommu->first_domain,
> > viommu->last_domain, GFP_KERNEL);
> > if (ret < 0)
> > @@ -659,7 +669,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);
> > } else if (vdomain->viommu != vdev->viommu) {
> > dev_err(dev, "cannot attach to foreign vIOMMU\n");
> > ret = -EXDEV;
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint
Hi Jean,
> -Original Message-
> From: Jean-Philippe Brucker
> Sent: Monday, March 23, 2020 3:30 PM
> To: Bharat Bhushan
> Cc: j...@8bytes.org; m...@redhat.com; jasow...@redhat.com;
> virtualizat...@lists.linux-foundation.org; iommu@lists.linux-foundation.org;
> eric.au...@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
> > ---
> > 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_headresv_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
> > * o
RE: [EXT] Re: [PATCH RFC] iommu/virtio: Use page size bitmap supported by endpoint
Hi Jean,
> -Original Message-
> From: Jean-Philippe Brucker
> Sent: Monday, March 23, 2020 3:30 PM
> To: Bharat Bhushan
> Cc: j...@8bytes.org; m...@redhat.com; jasow...@redhat.com;
> virtualizat...@lists.linux-foundation.org; iommu@lists.linux-foundation.org;
> eric.au...@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
> > ---
> > 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_headresv_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
RE: [PATCH v5 0/7] Add virtio-iommu driver
Hi Jean, > -Original Message- > From: Auger Eric > Sent: Friday, November 23, 2018 1:59 PM > To: Jean-Philippe Brucker ; > iommu@lists.linux-foundation.org; linux-...@vger.kernel.org; > devicet...@vger.kernel.org; virtualizat...@lists.linux-foundation.org; virtio- > d...@lists.oasis-open.org; j...@8bytes.org; m...@redhat.com > Cc: jasow...@redhat.com; robh...@kernel.org; mark.rutl...@arm.com; > bhelg...@google.com; frowand.l...@gmail.com; > kvm...@lists.cs.columbia.edu; tnowi...@caviumnetworks.com; > kevin.t...@intel.com; marc.zyng...@arm.com; robin.mur...@arm.com; > will.dea...@arm.com; lorenzo.pieral...@arm.com; Bharat Bhushan > > Subject: Re: [PATCH v5 0/7] Add virtio-iommu driver > > Hi Jean, > > On 11/22/18 8:37 PM, Jean-Philippe Brucker wrote: > > Implement the virtio-iommu driver, following specification v0.9 [1]. > > > > Since v4 [2] I fixed the issues reported by Eric, and added > > Reviewed-by from Eric and Rob. Thanks! > > > > I changed the specification to fix one inconsistency discussed in v4. > > That the device fills the probe buffer with zeroes is now a "SHOULD" > > instead of a "MAY", since it's the only way for the driver to know if > > the device wrote the status. Existing devices already do this. In > > addition the device now needs to fill the three padding bytes at the > > tail with zeroes. > > > > You can find Linux driver and kvmtool device on branches > > virtio-iommu/v0.9 [3]. I also lightly tested with Eric's latest QEMU > > device [4]. > > > > [1] Virtio-iommu specification v0.9, sources, pdf and diff from v0.8 > > git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9 > > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fjpbr > ucker.net%2Fvirtio-iommu%2Fspec%2Fv0.9%2Fvirtio-iommu- > v0.9.pdf&data=02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7 > df8e41943d4108d6511db8ed%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% > 7C1%7C636785585424990803&sdata=la0tSTLcOI5HkQ65a%2BCHKeI3H5iu > qZ%2F8r6Q5YF8tfsU%3D&reserved=0 > > > > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fjpbr > > ucker.net%2Fvirtio-iommu%2Fspec%2Fdiffs%2Fvirtio-iommu-pdf-diff-v0.8- > v > > > 0.9.pdf&data=02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7d > f8e4194 > > > 3d4108d6511db8ed%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C6 > 3678558 > > > 5424990803&sdata=AEXEib4lihcpfE6O6wLf%2BMElPtA7ZLGYE2mj0288PZ > k%3D& > > amp;reserved=0 > > > > [2] [PATCH v4 0/7] Add virtio-iommu driver > > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis > > ts.linuxfoundation.org%2Fpipermail%2Fiommu%2F2018- > November%2F031074.ht > > > ml&data=02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7df8e4 > 1943d410 > > > 8d6511db8ed%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636785 > 5854249 > > > 90803&sdata=mUUSBQ%2FjEeRGaisGBK20G9WmfXPwlERKDaeeRqHW4 > 08%3D&r > > eserved=0 > > > > [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9 > > git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9 > > > > [4] [RFC v9 00/17] VIRTIO-IOMMU device > > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw > ww > > .mail-archive.com%2Fqemu- > devel%40nongnu.org%2Fmsg575578.html&data= > > > 02%7C01%7Cbharat.bhushan%40nxp.com%7C6e7180e7df8e41943d4108d651 > 1db8ed% > > > 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636785585424990803& > amp;sd > > > ata=fo9WKE33Nm%2FdW2C2XcSVmv9itWjEyRN1irgEZgOWtZI%3D&rese > rved=0 > > > > Jean-Philippe Brucker (7): > > dt-bindings: virtio-mmio: Add IOMMU description > > dt-bindings: virtio: Add virtio-pci-iommu node > > of: Allow the iommu-map property to omit untranslated devices > > PCI: OF: Initialize dev->fwnode appropriately > > iommu: Add virtio-iommu driver > > iommu/virtio: Add probe request > > iommu/virtio: Add event queue > > > > .../devicetree/bindings/virtio/iommu.txt | 66 + > > .../devicetree/bindings/virtio/mmio.txt | 30 + > > MAINTAINERS |7 + > > drivers/iommu/Kconfig | 11 + > > drivers/iommu/Makefile|1 + > > drivers/iommu/virtio-iommu.c | 1157 + > > drivers/of/base.c | 10 +- > > drivers/pci/of.c |7 + > > include/uapi/linux/virtio_ids.h |1 + > > include/uapi/linux/virtio_iommu.h | 161 +++ > > 10 files changed, 1448 insertions(+), 3 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/virtio/iommu.txt > > create mode 100644 drivers/iommu/virtio-iommu.c create mode 100644 > > include/uapi/linux/virtio_iommu.h > > > for the whole series > Tested-by: Eric Auger I have tested this series with virtio/vfio both Tested-by: Bharat Bhushan Thanks -Bharat > > Thanks > > Eric ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/6 v2] iommu: of: make of_pci_map_rid() available for other devices too
> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: Tuesday, April 17, 2018 10:23 PM
> To: Nipun Gupta ; robh...@kernel.org;
> frowand.l...@gmail.com
> Cc: will.dea...@arm.com; mark.rutl...@arm.com; catalin.mari...@arm.com;
> h...@lst.de; gre...@linuxfoundation.org; j...@8bytes.org;
> m.szyprow...@samsung.com; shawn...@kernel.org; bhelg...@google.com;
> iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linuxppc-
> d...@lists.ozlabs.org; linux-...@vger.kernel.org; Bharat Bhushan
> ; stuyo...@gmail.com; Laurentiu Tudor
> ; Leo Li
> Subject: Re: [PATCH 2/6 v2] iommu: of: make of_pci_map_rid() available for
> other devices too
>
> On 17/04/18 11:21, Nipun Gupta wrote:
> > iommu-map property is also used by devices with fsl-mc. This patch
> > moves the of_pci_map_rid to generic location, so that it can be used
> > by other busses too.
Nipun, please clarify that only function name is changed and rest of body is
same.
> >
> > Signed-off-by: Nipun Gupta
> > ---
> > drivers/iommu/of_iommu.c | 106
> > +--
>
> Doesn't this break "msi-parent" parsing for !CONFIG_OF_IOMMU?
Yes, this will be a problem with MSI
> I guess you
> don't want fsl-mc to have to depend on PCI, but this looks like a step in the
> wrong direction.
>
> I'm not entirely sure where of_map_rid() fits best, but from a quick look
> around
> the least-worst option might be drivers/of/of_address.c, unless Rob and Frank
> have a better idea of where generic DT-based ID translation routines could
> live?
drivers/of/address.c may be proper place to move until someone have better idea.
Thanks
-Bharat
>
> > drivers/of/irq.c | 6 +--
> > drivers/pci/of.c | 101
> >
> > include/linux/of_iommu.h | 11 +
> > include/linux/of_pci.h | 10 -
> > 5 files changed, 117 insertions(+), 117 deletions(-)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index
> > 5c36a8b..4e7712f 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -138,6 +138,106 @@ static int of_iommu_xlate(struct device *dev,
> > return ops->of_xlate(dev, iommu_spec);
> > }
> >
> > +/**
> > + * of_map_rid - Translate a requester ID through a downstream mapping.
> > + * @np: root complex device node.
> > + * @rid: device requester ID to map.
> > + * @map_name: property name of the map to use.
> > + * @map_mask_name: optional property name of the mask to use.
> > + * @target: optional pointer to a target device node.
> > + * @id_out: optional pointer to receive the translated ID.
> > + *
> > + * Given a device requester ID, look up the appropriate
> > +implementation-defined
> > + * platform ID and/or the target device which receives transactions
> > +on that
> > + * ID, as per the "iommu-map" and "msi-map" bindings. Either of
> > +@target or
> > + * @id_out may be NULL if only the other is required. If @target
> > +points to
> > + * a non-NULL device node pointer, only entries targeting that node
> > +will be
> > + * matched; if it points to a NULL value, it will receive the device
> > +node of
> > + * the first matching target phandle, with a reference held.
> > + *
> > + * Return: 0 on success or a standard error code on failure.
> > + */
> > +int of_map_rid(struct device_node *np, u32 rid,
> > + const char *map_name, const char *map_mask_name,
> > + struct device_node **target, u32 *id_out) {
> > + u32 map_mask, masked_rid;
> > + int map_len;
> > + const __be32 *map = NULL;
> > +
> > + if (!np || !map_name || (!target && !id_out))
> > + return -EINVAL;
> > +
> > + map = of_get_property(np, map_name, &map_len);
> > + if (!map) {
> > + if (target)
> > + return -ENODEV;
> > + /* Otherwise, no map implies no translation */
> > + *id_out = rid;
> > + return 0;
> > + }
> > +
> > + if (!map_len || map_len % (4 * sizeof(*map))) {
> > + pr_err("%pOF: Error: Bad %s length: %d\n", np,
> > + map_name, map_len);
> > + return -EINVAL;
> > + }
> > +
> > + /* The default is to select all bits. */
> > + map_mask = 0x;
>
RE: [PATCH v2 1/2] dma-mapping: move dma configuration to bus infrastructure
> -Original Message-
> From: Nipun Gupta
> Sent: Wednesday, March 21, 2018 12:25 PM
> To: robin.mur...@arm.com; h...@lst.de; li...@armlinux.org.uk;
> gre...@linuxfoundation.org; m.szyprow...@samsung.com
> Cc: bhelg...@google.com; zaj...@gmail.com; andy.gr...@linaro.org;
> david.br...@linaro.org; dan.j.willi...@intel.com; vinod.k...@intel.com;
> thierry.red...@gmail.com; robh...@kernel.org; frowand.l...@gmail.com;
> jarkko.sakki...@linux.intel.com; rafael.j.wyso...@intel.com;
> dmitry.torok...@gmail.com; jo...@kernel.org; msucha...@suse.de; linux-
> ker...@vger.kernel.org; iommu@lists.linux-foundation.org; linux-
> wirel...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> s...@vger.kernel.org; dmaeng...@vger.kernel.org; dri-
> de...@lists.freedesktop.org; linux-te...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-...@vger.kernel.org; Bharat Bhushan
> ; Leo Li ; Nipun Gupta
>
> Subject: [PATCH v2 1/2] dma-mapping: move dma configuration to bus
> infrastructure
>
> It's bus specific aspect to map a given device on the bus and relevant
> firmware
> description of its DMA configuration.
> So, this change introduces '/dma_configure/' as bus callback giving
> flexibility to
> busses for implementing its own dma configuration function.
>
> The change eases the addition of new busses w.r.t. adding the dma
> configuration functionality.
>
> This patch also updates the PCI, Platform, ACPI and host1x bus to use new
> introduced callbacks.
>
> Suggested-by: Christoph Hellwig
> Signed-off-by: Nipun Gupta
> ---
> - The patches are based on the comments on:
>https://patchwork.kernel.org/patch/10259087/
>
> Changes in v2:
> - Do not have dma_deconfigure callback
> - Have '/dma_common_configure/' API to provide a common DMA
> configuration which can be used by busses if it suits them.
> - Platform and ACPI bus to use '/dma_common_configure/' in
> '/dma_configure/' callback.
> - Updated commit message
> - Updated pci_dma_configure API with changes suggested by Robin
>
> drivers/amba/bus.c | 7 +++
> drivers/base/dma-mapping.c | 35 +++
> drivers/base/platform.c | 6 ++
> drivers/gpu/host1x/bus.c| 9 +
> drivers/pci/pci-driver.c| 32
> include/linux/device.h | 4
> include/linux/dma-mapping.h | 1 +
> 7 files changed, 74 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c index 594c228..2fa1e8b
> 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -171,6 +172,11 @@ static int amba_pm_runtime_resume(struct device
> *dev) } #endif /* CONFIG_PM */
>
> +static int amba_dma_configure(struct device *dev) {
> + return dma_common_configure(dev);
> +}
> +
> static const struct dev_pm_ops amba_pm = {
> .suspend= pm_generic_suspend,
> .resume = pm_generic_resume,
> @@ -194,6 +200,7 @@ struct bus_type amba_bustype = {
> .dev_groups = amba_dev_groups,
> .match = amba_match,
> .uevent = amba_uevent,
> + .dma_configure = amba_dma_configure,
> .pm = &amba_pm,
> .force_dma = true,
> };
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c index
> 3b11835..48f9af0 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -331,38 +331,33 @@ void dma_common_free_remap(void *cpu_addr,
> size_t size, unsigned long vm_flags) #endif
>
> /*
> - * Common configuration to enable DMA API use for a device
> + * Common configuration to enable DMA API use for a device.
> + * A bus can use this function in its 'dma_configure' callback, if
> + * suitable for the bus.
> */
> -#include
> -
> -int dma_configure(struct device *dev)
> +int dma_common_configure(struct device *dev)
> {
> - struct device *bridge = NULL, *dma_dev = dev;
> enum dev_dma_attr attr;
> int ret = 0;
>
> - if (dev_is_pci(dev)) {
> - bridge = pci_get_host_bridge_device(to_pci_dev(dev));
> - dma_dev = bridge;
> - if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&
> - dma_dev->parent->of_node)
> - dma_dev = dma_dev->parent;
> - }
> -
> - if (dma_dev->of_node) {
> - ret = of_dma_configure(dev, dma_dev->of_node);
> - } else if (has_acpi_comp
RE: [RFC] virtio-iommu version 0.4
Hi Eric, > -Original Message- > From: Auger Eric [mailto:eric.au...@redhat.com] > Sent: Tuesday, September 12, 2017 10:43 PM > To: Jean-Philippe Brucker ; > iommu@lists.linux-foundation.org; k...@vger.kernel.org; > virtualizat...@lists.linux-foundation.org; virtio-...@lists.oasis-open.org > Cc: will.dea...@arm.com; robin.mur...@arm.com; > lorenzo.pieral...@arm.com; m...@redhat.com; jasow...@redhat.com; > marc.zyng...@arm.com; eric.auger....@gmail.com; Bharat Bhushan > ; pet...@redhat.com; kevin.t...@intel.com > Subject: Re: [RFC] virtio-iommu version 0.4 > > Hi jean, > > On 04/08/2017 20:19, Jean-Philippe Brucker wrote: > > This is the continuation of my proposal for virtio-iommu, the para- > > virtualized IOMMU. Here is a summary of the changes since last time [1]: > > > > * The virtio-iommu document now resembles an actual specification. It is > > split into a formal description of the virtio device, and implementation > > notes. Please find sources and binaries at [2]. > > > > * Added a probe request to describe to the guest different properties that > > do not fit in firmware or in the virtio config space. This is a > > necessary stepping stone for extending the virtio-iommu. > > > > * There is a working Qemu prototype [3], thanks to Eric Auger and Bharat > > Bhushan. > > > > You can find the Linux driver and kvmtool device at [4] and [5]. I > > plan to rework driver and kvmtool device slightly before sending the > > patches. > > > > To understand the virtio-iommu, I advise to first read introduction > > and motivation, then skim through implementation notes and finally > > look at the device specification. > > > > I wasn't sure how to organize the review. For those who prefer to > > comment inline, I attached v0.4 of device-operations.tex and > > topology.tex+MSI.tex to this thread. They are the biggest chunks of > > the document. But LaTeX isn't very pleasant to read, so you can simply > > send a list of comments in relation to section numbers and a few words of > context, we'll manage. > > > > --- > > Version numbers 0.1-0.4 are arbitrary. I'm hoping they allow to > > compare more easily differences since the RFC (see [6]), but haven't > > been made public so far. This is the first public posting since > > initial proposal [1], and the following describes all changes. > > > > ## v0.1 ## > > > > Content is the same as the RFC, but formatted to LaTeX. 'make' > > generates one PDF and one HTML document. > > > > ## v0.2 ## > > > > Add introductions, improve topology example and firmware description > > based on feedback and a number of useful discussions. > > > > ## v0.3 ## > > > > Add normative sections (MUST, SHOULD, etc). Clarify some things, > > tighten the device and driver behaviour. Unmap semantics are > > consolidated; they are now closer to VFIO Type1 v2 semantics. > > > > ## v0.4 ## > > > > Introduce PROBE requests. They provide per-endpoint information to the > > driver that couldn't be described otherwise. > > > > For the moment, they allow to handle MSIs on x86 virtual platforms > > (see 3.2). To do that we communicate reserved IOVA regions, that will > > also be useful for describing regions that cannot be mapped for a > > given endpoint, for instance addresses that correspond to a PCI bridge > window. > > > > Introducing such a large framework for this tiny feature may seem > > overkill, but it is needed for future extensions of the virtio-iommu > > and I believe it really is worth the effort. > > 2.6.7 > - As I am currently integrating v0.4 in QEMU here are some other comments: > At the moment struct virtio_iommu_req_probe flags is missing in your > header. As such I understood the ACK protocol was not implemented by the > driver in your branch. > - VIRTIO_IOMMU_PROBE_PROPERTY_TYPE_MASK is > VIRTIO_IOMMU_T_MASK in your header too. > 2.6.8.2: > - I am really confused about what the device should report as resv regions > depending on the PE nature (VFIO or not VFIO) > > In other iommu drivers, the resv regions are populated by the iommu driver > through its get_resv_regions callback. They are usually composed of an > iommu specific MSI region (mapped or bypassed) and non IOMMU specific > (device specific) reserved regions: > iommu_dma_get_resv_regions(). In the case of virtio-iommu driver, those > are the guest reserved regions. > > First in the current virtio-iommu driver I don't see the > iommu_dma_get_resv_regions call. Do you agree th
RE: [virtio-dev] [RFC] virtio-iommu version 0.4
Hi, > -Original Message- > From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com] > Sent: Thursday, August 17, 2017 3:42 PM > To: Adam Tao > Cc: iommu@lists.linux-foundation.org; k...@vger.kernel.org; > virtualizat...@lists.linux-foundation.org; virtio-...@lists.oasis-open.org; > will.dea...@arm.com; robin.mur...@arm.com; lorenzo.pieral...@arm.com; > m...@redhat.com; jasow...@redhat.com; marc.zyng...@arm.com; > eric.au...@redhat.com; eric.auger....@gmail.com; Bharat Bhushan > ; pet...@redhat.com; kevin.t...@intel.com > Subject: Re: [virtio-dev] [RFC] virtio-iommu version 0.4 > > Hi Adam, > > On 16/08/17 05:08, Adam Tao wrote: > >> * There is a working Qemu prototype [3], thanks to Eric Auger and Bharat > >> Bhushan. > > Hi, Brucker > > I read the related spec for virtio IOMMU, I am wondering if we support > > both the virtual and physical devices in the guest to use the virtio > > IOMMU, how can we config the related address translation for the > > different type of devices. > > e.g. > > 1. for Virtio devs(e.g. virtio-net), we can change the memmap table > > used by the vhost device. > > 2. for physical devices we can directly config the IOMMU/SMMU on the > > host. > > > > So do you know are there any RFCs for the back-end implementation for > > the virtio-IOMMU thanks Adam > > Eric's series [3] adds support for virtual devices (I'v only had time to test > virtio-net so far, but as vhost-iotlb seems to be supported by Qemu, it should > work with vhost-net as well). Bharat is working on the VFIO backend for > physical devices. As far as I'm aware, his latest version is: > > [7] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg04094.html I am working on newer version which will be based on Eric-v3. Actually I wanted to include the fix for the issue reported by Eric, it fails if we assign more than one interface. I reproduced the issues on my side and know the root cause. I am working on fixing this. Thanks -Bharat > > Thanks, > Jean > > > >> [1] http://www.spinics.net/lists/kvm/msg147990.html > >> [2] git://linux-arm.org/virtio-iommu.git branch viommu/v0.4 > >> http://www.linux-arm.org/git?p=virtio- > iommu.git;a=blob;f=dist/v0.4/virtio-iommu-v0.4.pdf > >> I reiterate the disclaimers: don't use this document as a reference, > >> it's a draft. It's also not an OASIS document yet. It may be riddled > >> with mistakes. As this is a working draft, it is unstable and I do not > >> guarantee backward compatibility of future versions. > >> [3] https://lists.gnu.org/archive/html/qemu-arm/2017-08/msg4.html > >> [4] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.4 > >> Warning: UAPI headers have changed! They didn't follow the spec, > >> please update. (Use branch v0.1, that has the old headers, for the > >> Qemu prototype [3]) > >> [5] git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.4 > >> Warning: command-line has changed! Use --viommu vfio[,opts] and > >> --viommu virtio[,opts] to instantiate a device. > >> [6] > >> http://www.linux-arm.org/git?p=virtio-iommu.git;a=tree;f=dist/diffs ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [virtio-dev] [RFC PATCH linux] iommu: Add virtio-iommu driver
Hi Jean
> -Original Message-
> From: virtio-...@lists.oasis-open.org [mailto:virtio-dev@lists.oasis-
> open.org] On Behalf Of Jean-Philippe Brucker
> Sent: Saturday, April 08, 2017 12:53 AM
> To: iommu@lists.linux-foundation.org; k...@vger.kernel.org;
> virtualizat...@lists.linux-foundation.org; virtio-...@lists.oasis-open.org
> Cc: cd...@linaro.org; will.dea...@arm.com; robin.mur...@arm.com;
> lorenzo.pieral...@arm.com; j...@8bytes.org; m...@redhat.com;
> jasow...@redhat.com; alex.william...@redhat.com;
> marc.zyng...@arm.com
> Subject: [virtio-dev] [RFC PATCH linux] iommu: Add virtio-iommu driver
>
> The virtio IOMMU is a para-virtualized device, allowing to send IOMMU
> requests such as map/unmap over virtio-mmio transport. This driver should
> illustrate the initial proposal for virtio-iommu, that you hopefully received
> with it. It handle attach, detach, map and unmap requests.
>
> The bulk of the code is to create requests and send them through virtio.
> Implementing the IOMMU API is fairly straightforward since the virtio-iommu
> MAP/UNMAP interface is almost identical. I threw in a custom
> map_sg() function which takes up some space, but is optional. The core
> function would send a sequence of map requests, waiting for a reply
> between each mapping. This optimization avoids yielding to the host after
> each map, and instead prepares a batch of requests in the virtio ring and
> kicks the host once.
>
> It must be applied on top of the probe deferral work for IOMMU, currently
> under discussion. This allows to dissociate early driver detection and device
> probing: device-tree or ACPI is parsed early to find which devices are
> translated by the IOMMU, but the IOMMU itself cannot be probed until the
> core virtio module is loaded.
>
> Enabling DEBUG makes it extremely verbose at the moment, but it should be
> calmer in next versions.
>
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/iommu/Kconfig | 11 +
> drivers/iommu/Makefile| 1 +
> drivers/iommu/virtio-iommu.c | 980
> ++
> include/uapi/linux/Kbuild | 1 +
> include/uapi/linux/virtio_ids.h | 1 +
> include/uapi/linux/virtio_iommu.h | 142 ++
> 6 files changed, 1136 insertions(+)
> create mode 100644 drivers/iommu/virtio-iommu.c create mode 100644
> include/uapi/linux/virtio_iommu.h
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index
> 37e204f3d9be..8cd56ee9a93a 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -359,4 +359,15 @@ config MTK_IOMMU_V1
>
> if unsure, say N here.
>
> +config VIRTIO_IOMMU
> + tristate "Virtio IOMMU driver"
> + depends on VIRTIO_MMIO
> + select IOMMU_API
> + select INTERVAL_TREE
> + select ARM_DMA_USE_IOMMU if ARM
> + help
> + Para-virtualised IOMMU driver with virtio.
> +
> + Say Y here if you intend to run this kernel as a guest.
> +
> endif # IOMMU_SUPPORT
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile index
> 195f7b997d8e..1199d8475802 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -27,3 +27,4 @@ obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-
> smmu.o
> obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
> obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
> obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> new file mode 100644 index ..1cf4f57b7817
> --- /dev/null
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -0,0 +1,980 @@
> +/*
> + * Virtio driver for the paravirtualized IOMMU
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307,
> USA.
> + *
> + * Copyright (C) 2017 ARM Limited
> + *
> + * Author: Jean-Philippe Brucker */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +struct viommu_dev {
> + struct iommu_device iommu;
> + struct device *dev;
> + struct virtio_device*vdev;
> +
> + struct virtqueue*vq;
> + struct list_headpending_reque
RE: [RFC PATCH kvmtool 00/15] Add virtio-iommu
Hi Jean, I am trying to run and review on my side but I see Linux patches are not with latest kernel version. Will it be possible for you to share your Linux and kvmtool git repository reference? Thanks -Bharat > -Original Message- > From: virtualization-boun...@lists.linux-foundation.org > [mailto:virtualization-boun...@lists.linux-foundation.org] On Behalf Of Jean- > Philippe Brucker > Sent: Saturday, April 08, 2017 12:55 AM > To: iommu@lists.linux-foundation.org; k...@vger.kernel.org; > virtualizat...@lists.linux-foundation.org; virtio-...@lists.oasis-open.org > Cc: cd...@linaro.org; lorenzo.pieral...@arm.com; m...@redhat.com; > marc.zyng...@arm.com; j...@8bytes.org; will.dea...@arm.com; > robin.mur...@arm.com > Subject: [RFC PATCH kvmtool 00/15] Add virtio-iommu > > Implement a virtio-iommu device and translate DMA traffic from vfio and > virtio devices. Virtio needed some rework to support scatter-gather accesses > to vring and buffers at page granularity. Patch 3 implements the actual > virtio- > iommu device. > > Adding --viommu on the command-line now inserts a virtual IOMMU in front > of all virtio and vfio devices: > > $ lkvm run -k Image --console virtio -p console=hvc0 \ > --viommu --vfio 0 --vfio 4 --irqchip gicv3-its > ... > [2.998949] virtio_iommu virtio0: probe successful > [3.007739] virtio_iommu virtio1: probe successful > ... > [3.165023] iommu: Adding device :00:00.0 to group 0 > [3.536480] iommu: Adding device 10200.virtio to group 1 > [3.553643] iommu: Adding device 10600.virtio to group 2 > [3.570687] iommu: Adding device 10800.virtio to group 3 > [3.627425] iommu: Adding device 10a00.virtio to group 4 > [7.823689] iommu: Adding device :00:01.0 to group 5 > ... > > Patches 13 and 14 add debug facilities. Some statistics are gathered for each > address space and can be queried via the debug builtin: > > $ lkvm debug -n guest-1210 --iommu stats > iommu 0 "viommu-vfio" > kicks 1255 > requests 1256 > ioas 1 > maps7 > unmaps 4 > resident2101248 > ioas 6 > maps623 > unmaps 620 > resident16384 > iommu 1 "viommu-virtio" > kicks 11426 > requests 11431 > ioas 2 > maps2836 > unmaps 2835 > resident8192 > accesses2836 > ... > > This is based on the VFIO patchset[1], itself based on Andre's ITS work. > The VFIO bits have only been tested on a software model and are unlikely to > work on actual hardware, but I also tested virtio on an ARM Juno. > > [1] http://www.spinics.net/lists/kvm/msg147624.html > > Jean-Philippe Brucker (15): > virtio: synchronize virtio-iommu headers with Linux > FDT: (re)introduce a dynamic phandle allocator > virtio: add virtio-iommu > Add a simple IOMMU > iommu: describe IOMMU topology in device-trees > irq: register MSI doorbell addresses > virtio: factor virtqueue initialization > virtio: add vIOMMU instance for virtio devices > virtio: access vring and buffers through IOMMU mappings > virtio-pci: translate MSIs with the virtual IOMMU > virtio: set VIRTIO_F_IOMMU_PLATFORM when necessary > vfio: add support for virtual IOMMU > virtio-iommu: debug via IPC > virtio-iommu: implement basic debug commands > virtio: use virtio-iommu when available > > Makefile | 3 + > arm/gic.c | 4 + > arm/include/arm-common/fdt-arch.h | 2 +- > arm/pci.c | 49 ++- > builtin-debug.c | 8 +- > builtin-run.c | 2 + > fdt.c | 35 ++ > include/kvm/builtin-debug.h | 6 + > include/kvm/devices.h | 4 + > include/kvm/fdt.h | 20 + > include/kvm/iommu.h | 105 + > include/kvm/irq.h | 3 + > include/kvm/kvm-config.h | 1 + > include/kvm/vfio.h| 2 + > include/kvm/virtio-iommu.h| 15 + > include/kvm/virtio-mmio.h | 1 + > include/kvm/virtio-pci.h | 2 + > include/kvm/virtio.h | 137 +- > include/linux/virtio_config.h | 74 > include/linux/virtio_ids.h| 4 + > include/linux/virtio_iommu.h | 142 ++ > iommu.c | 240 ++ > irq.c | 35 ++ > kvm-ipc.c | 43 +- > mips/include/kvm/fdt-arch.h | 2 +- > powerpc/include/kvm/fdt-arch.h| 2 +- > vfio.c| 281 +++- > virtio/9p.c | 7 +- >
RE: [PATCH] iommu/dma: Setup iova_domain granule for IOMMU_DMA_MSI cookies
Hi Robin,
I faced same issue on our platform and debugged to get the root cause of the
issue. Also fixed in somewhat similar way, cleared iova_off and not change size
for if cookie->type is IOMMU_DMA_MSI_COOKIE. Anyways that was not as good as
below changes.
While just now I saw this was already discussed and solved. Should have looked
earlier to save time, but the debugging was some learning experience 😊
I have tested your below change and it works fine on NXP platform.
Thanks
-Bharat
> -Original Message-
> From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
> boun...@lists.linux-foundation.org] On Behalf Of Shanker Donthineni
> Sent: Thursday, April 13, 2017 8:00 PM
> To: Robin Murphy ; Nate Watterson
> ; Joerg Roedel ;
> iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] iommu/dma: Setup iova_domain granule for
> IOMMU_DMA_MSI cookies
>
> Hi Robin,
>
> I tested your changes and the device pass-through feature works fine on
> QDF2400 server platform. Maybe Nate comments on the patch contents but
> it fixes the problem.
>
>
> @@ -317,13 +317,13 @@ static void iommu_dma_free_iova(struct
> iommu_dma_cookie *cookie,
> dma_addr_t iova, size_t size) {
> struct iova_domain *iovad = &cookie->iovad;
> - unsigned long shift = iova_shift(iovad);
>
> /* The MSI case is only ever cleaning up its most recent allocation */
> if (cookie->type == IOMMU_DMA_MSI_COOKIE)
> cookie->msi_iova -= size;
> else
> - free_iova_fast(iovad, iova >> shift, size >> shift);
> + free_iova_fast(iovad, iova_pfn(iovad, iova),
> + size >> iova_shift(iovad));
> }
>
> static void __iommu_dma_unmap(struct iommu_domain *domain,
> dma_addr_t dma_addr, @@ -538,11 +538,14 @@ static dma_addr_t
> __iommu_dma_map(struct device *dev, phys_addr_t phys, {
> struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> struct iommu_dma_cookie *cookie = domain->iova_cookie;
> - struct iova_domain *iovad = &cookie->iovad;
> - size_t iova_off = iova_offset(iovad, phys);
> + size_t iova_off = 0;
> dma_addr_t iova;
>
> - size = iova_align(iovad, size + iova_off);
> + if (cookie->type == IOMMU_DMA_IOVA_COOKIE) {
> + iova_off = iova_offset(&cookie->iovad, phys);
> + size = iova_align(&cookie->iovad, size + iova_off);
> + }
>
>
> On 04/13/2017 06:21 AM, Robin Murphy wrote:
> > Hi Nate,
> >
> > On 13/04/17 09:55, Nate Watterson wrote:
> >> Currently, the __iommu_dma_{map/free} functions call
> >> iova_{offset/align} making them unsuitable for use with iommu_domains
> >> having an IOMMU_DMA_MSI cookie since the cookie's iova_domain
> member, iovad, is uninitialized.
> >>
> >> Now that iommu_dma_get_msi_page() calls __iommu_dma_map()
> regardless
> >> of cookie type, failures are being seen when mapping MSI target
> >> addresses for devices attached to UNMANAGED domains. To work
> around
> >> this issue, the iova_domain granule for IOMMU_DMA_MSI cookies is
> >> initialized to the value returned by cookie_msi_granule().
> > Oh bum. Thanks for the report.
> >
> > However, I really don't like bodging around it with deliberate
> > undefined behaviour. Fixing things properly doesn't seem too hard:
> >
> > ->8-
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index 8348f366ddd1..62618e77bedc 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -396,13 +396,13 @@ static void iommu_dma_free_iova(struct
> > iommu_dma_cookie *cookie,
> > dma_addr_t iova, size_t size) {
> > struct iova_domain *iovad = &cookie->iovad;
> > - unsigned long shift = iova_shift(iovad);
> >
> > /* The MSI case is only ever cleaning up its most recent
> > allocation */
> > if (cookie->type == IOMMU_DMA_MSI_COOKIE)
> > cookie->msi_iova -= size;
> > else
> > - free_iova_fast(iovad, iova >> shift, size >> shift);
> > + free_iova_fast(iovad, iova_pfn(iovad, iova),
> > + size >> iova_shift(iovad));
> > }
> >
> > static void __iommu_dma_unmap(struct iommu_domain *domain,
> dma_addr_t
> > dma_addr, @@ -617,11 +617,14 @@ static dma_addr_t
> > __iommu_dma_map(struct device *dev, phys_addr_t phys, {
> > struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> > struct iommu_dma_cookie *cookie = domain->iova_cookie;
> > - struct iova_domain *iovad = &cookie->iovad;
> > - size_t iova_off = iova_offset(iovad, phys);
> > + size_t iova_off = 0;
> > dma_addr_t iova;
> >
> > - size = iova_align(iovad, size + iova_off);
> > + if (cookie->type == IOMMU_DMA_IOVA_COOKIE) {
> > + iova_off = iova_offset(&cookie->iovad, phys);
> > + size = iova_align(&cook
RE: [PATCH v8 00/18] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
> -Original Message- > From: Eric Auger [mailto:eric.au...@redhat.com] > Sent: Wednesday, January 11, 2017 3:12 PM > To: eric.au...@redhat.com; eric.auger@gmail.com; > christoffer.d...@linaro.org; marc.zyng...@arm.com; > robin.mur...@arm.com; alex.william...@redhat.com; > will.dea...@arm.com; j...@8bytes.org; t...@linutronix.de; > ja...@lakedaemon.net; linux-arm-ker...@lists.infradead.org > Cc: k...@vger.kernel.org; drjo...@redhat.com; linux- > ker...@vger.kernel.org; pranav.sawargaon...@gmail.com; > iommu@lists.linux-foundation.org; punit.agra...@arm.com; Diana Madalina > Craciun ; gpkulka...@gmail.com; > shank...@codeaurora.org; Bharat Bhushan ; > geethasowjanya.ak...@gmail.com > Subject: [PATCH v8 00/18] KVM PCIe/MSI passthrough on ARM/ARM64 and > IOVA reserved regions > > Following LPC discussions, we now report reserved regions through the > iommu-group sysfs reserved_regions attribute file. > > Reserved regions are populated through the IOMMU get_resv_region > callback (former get_dm_regions), now implemented by amd-iommu, intel- > iommu and arm-smmu: > - the intel-iommu reports the [0xfee0 - 0xfeef] MSI window > as a reserved region and RMRR regions as direct-mapped regions. > - the amd-iommu reports device direct mapped regions, the MSI region > and HT regions. > - the arm-smmu reports the MSI window (arbitrarily located at > 0x800 and 1MB large). > > Unsafe interrupt assignment is tested by enumerating all MSI irq domains > and checking MSI remapping is supported in the above hierarchy. > This check is done in case we detect the iommu translates MSI (an > IOMMU_RESV_MSI window exists). Otherwise the IRQ remapping capability > is checked at IOMMU level. Obviously this is a defensive IRQ safety > assessment: Assuming there are several MSI controllers in the system and at > least one does not implement IRQ remapping, the assignment will be > considered as unsafe (even if this controller is not acessible from the > assigned devices). > > The series first patch stems from Robin's branch: > http://linux-arm.org/git?p=linux- > rm.git;a=shortlog;h=refs/heads/iommu/misc > > Best Regards > > Eric > > Git: complete series available at > https://github.com/eauger/linux/tree/v4.10-rc3-reserved-v8 This series is tested on NXP platform, if you want you can add my tested by Tested-by: Bharat Bhushan Thanks -Bharat > > istory: > > PATCHv7 -> PATCHv8 > - take into account Marc's comments and apply his R-b > - remove iommu_group_remove_file call in iommu_group_release > - add Will's A-b > - removed [PATCH v7 01/19] iommu/dma: Implement PCI allocation > optimisation and updated iommu/dma: Allow MSI-only cookies > as per Robin's indications > > PATCHv6 -> PATCHv7: > - iommu/dma: Implement PCI allocation optimisation was added to apply > iommu/dma: Allow MSI-only cookies > - report Intel RMRR as direct-mapped regions > - report the type in the iommu group sysfs reserved_regions file > - do not merge regions of different types when building the list > of reserved regions > - intgeration Robin's "iommu/dma: Allow MSI-only cookies" last > version > - update Documentation/ABI/testing/sysfs-kernel-iommu_groups > - rename IOMMU_RESV_NOMAP into IOMMU_RESV_RESERVED > > PATCHv5 -> PATCHv6 > - Introduce IRQ_DOMAIN_FLAG_MSI as suggested by Marc > - irq_domain_is_msi, irq_domain_is_msi_remap, > irq_domain_hierarchical_is_msi_remap, > - set IRQ_DOMAIN_FLAG_MSI in msi_create_irq_domain > - fix compil issue on i386 > - rework test at VFIO level > > RFCv4 -> PATCHv5 > - fix IRQ security assessment by looking at irq domain parents > - check DOMAIN_BUS_FSL_MC_MSI irq domains > - AMD MSI and HT regions are exposed in iommu group sysfs > > RFCv3 -> RFCv4: > - arm-smmu driver does not register PCI host bridge windows as > reserved regions anymore > - Implement reserved region get/put callbacks also in arm-smmuv3 > - take the iommu_group lock on iommu_get_group_resv_regions > - add a type field in iommu_resv_region instead of using prot > - init the region list_head in iommu_alloc_resv_region, also > add type parameter > - iommu_insert_resv_region manage overlaps and sort reserved > windows > - address IRQ safety assessment by enumerating all the MSI irq > domains and checking the MSI_REMAP flag > - update Documentation/ABI/testing/sysfs-kernel-iommu_groups > > RFC v2 -> v3: > - switch to an iommu-group sysfs API > - use new dummy allocator provided by Robin > - dummy allocator initialized by vfio-iommu-type1 after enumerating > the reserved regions > - at the moment ARM MSI base address/size is lef
RE: [RFC PATCH] iommu/arm-smmu: Add global SMR masking property
Hi Robin,
> -Original Message-
> From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
> boun...@lists.linux-foundation.org] On Behalf Of Nipun Gupta
> Sent: Sunday, December 18, 2016 2:37 AM
> To: Robin Murphy ; iommu@lists.linux-
> foundation.org; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org
> Cc: mark.rutl...@arm.com; will.dea...@arm.com; Stuart Yoder
>
> Subject: RE: [RFC PATCH] iommu/arm-smmu: Add global SMR masking
> property
>
>
>
> > -Original Message-
> > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
> > boun...@lists.linux-foundation.org] On Behalf Of Robin Murphy
> > Sent: Friday, December 16, 2016 18:49
> > To: iommu@lists.linux-foundation.org; devicet...@vger.kernel.org;
> > linux-arm- ker...@lists.infradead.org
> > Cc: mark.rutl...@arm.com; will.dea...@arm.com; Stuart Yoder
> >
> > Subject: [RFC PATCH] iommu/arm-smmu: Add global SMR masking
> property
> >
> > The current SMR masking support using a 2-cell iommu-specifier is
> > primarily intended to handle individual masters with large and/or
> > complex Stream ID assignments; it quickly gets a bit clunky in other
> > SMR use-cases where we just want to consistently mask out the same
> > part of every Stream ID (e.g. for MMU-500 configurations where the
> > appended TBU number gets in the way unnecessarily). Let's add a new
> > property to allow a single global mask value to better fit the latter
> > situation.
> >
> > CC: Stuart Yoder
>
> Tested-by: Nipun Gupta
We have verified this patches with PCI and FSL-MC bus devices.
I do not see any comment on this patch, I know there were holidays around, can
we assume this as accepted and we can develop u-boot patches.
Thanks
-Bharat
>
> > Signed-off-by: Robin Murphy
> > ---
> >
> > Compile-tested only...
> >
> > Documentation/devicetree/bindings/iommu/arm,smmu.txt | 8
> > drivers/iommu/arm-smmu.c | 4 +++-
> > 2 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > index e862d1485205..98f5cbe5fdb4 100644
> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> > @@ -60,6 +60,14 @@ conditions.
> >aliases of secure registers have to be used during
> >SMMU configuration.
> >
> > +- stream-match-mask : Specifies a fixed SMR mask value to combine with
> > + the Stream ID value from every iommu-specifier. This
> > + may be used instead of an "#iommu-cells" value of 2
> > + when there is no need for per-master SMR masks, but
> > + it is still desired to mask some portion of every
> > + Stream ID (e.g. for certain MMU-500 configurations
> > + given globally unique external IDs).
> > +
> > ** Deprecated properties:
> >
> > - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index
> > 8f7281444551..f1abcb7dde36 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -1534,13 +1534,15 @@ static int arm_smmu_domain_set_attr(struct
> > iommu_domain *domain,
> >
> > static int arm_smmu_of_xlate(struct device *dev, struct
> > of_phandle_args *args) {
> > - u32 fwid = 0;
> > + u32 mask, fwid = 0;
> >
> > if (args->args_count > 0)
> > fwid |= (u16)args->args[0];
> >
> > if (args->args_count > 1)
> > fwid |= (u16)args->args[1] << SMR_MASK_SHIFT;
> > + else if (!of_property_read_u32(args->np, "stream-match-mask",
> > &mask))
> > + fwid |= (u16)mask << SMR_MASK_SHIFT;
> >
> > return iommu_fwspec_add_ids(dev, &fwid, 1); }
> > --
> > 2.10.2.dirty
> >
> > ___
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v6 17/18] vfio/type1: Check MSI remapping at irq domain level
> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Friday, January 06, 2017 2:50 PM
> To: Auger Eric ; Bharat Bhushan
> ; eric.auger@gmail.com;
> christoffer.d...@linaro.org; robin.mur...@arm.com;
> alex.william...@redhat.com; will.dea...@arm.com; j...@8bytes.org;
> t...@linutronix.de; ja...@lakedaemon.net; linux-arm-
> ker...@lists.infradead.org
> Cc: k...@vger.kernel.org; drjo...@redhat.com; linux-
> ker...@vger.kernel.org; pranav.sawargaon...@gmail.com;
> iommu@lists.linux-foundation.org; punit.agra...@arm.com; Diana Madalina
> Craciun ; gpkulka...@gmail.com;
> shank...@codeaurora.org; geethasowjanya.ak...@gmail.com
> Subject: Re: [PATCH v6 17/18] vfio/type1: Check MSI remapping at irq
> domain level
>
> On 06/01/17 09:08, Auger Eric wrote:
> > Hi Bharat
> >
> > On 06/01/2017 09:50, Bharat Bhushan wrote:
> >> Hi Eric,
> >>
> >>> -Original Message-
> >>> From: Eric Auger [mailto:eric.au...@redhat.com]
> >>> Sent: Friday, January 06, 2017 12:35 AM
> >>> To: eric.au...@redhat.com; eric.auger@gmail.com;
> >>> christoffer.d...@linaro.org; marc.zyng...@arm.com;
> >>> robin.mur...@arm.com; alex.william...@redhat.com;
> >>> will.dea...@arm.com; j...@8bytes.org; t...@linutronix.de;
> >>> ja...@lakedaemon.net; linux-arm-ker...@lists.infradead.org
> >>> Cc: k...@vger.kernel.org; drjo...@redhat.com; linux-
> >>> ker...@vger.kernel.org; pranav.sawargaon...@gmail.com;
> >>> iommu@lists.linux-foundation.org; punit.agra...@arm.com; Diana
> >>> Madalina Craciun ; gpkulka...@gmail.com;
> >>> shank...@codeaurora.org; Bharat Bhushan
> ;
> >>> geethasowjanya.ak...@gmail.com
> >>> Subject: [PATCH v6 17/18] vfio/type1: Check MSI remapping at irq
> >>> domain level
> >>>
> >>> In case the IOMMU translates MSI transactions (typical case on ARM),
> >>> we check MSI remapping capability at IRQ domain level. Otherwise it
> >>> is checked at IOMMU level.
> >>>
> >>> At this stage the arm-smmu-(v3) still advertise the
> >>> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> removed
> >>> in subsequent patches.
> >>>
> >>> Signed-off-by: Eric Auger
> >>>
> >>> ---
> >>>
> >>> v6: rewrite test
> >>> ---
> >>> drivers/vfio/vfio_iommu_type1.c | 9 ++---
> >>> 1 file changed, 6 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/vfio/vfio_iommu_type1.c
> >>> b/drivers/vfio/vfio_iommu_type1.c index b473ef80..fa0b5c4 100644
> >>> --- a/drivers/vfio/vfio_iommu_type1.c
> >>> +++ b/drivers/vfio/vfio_iommu_type1.c
> >>> @@ -40,6 +40,7 @@
> >>> #include
> >>> #include
> >>> #include
> >>> +#include
> >>>
> >>> #define DRIVER_VERSION "0.2"
> >>> #define DRIVER_AUTHOR "Alex Williamson
> >>> "
> >>> @@ -1208,7 +1209,7 @@ static int
> vfio_iommu_type1_attach_group(void
> >>> *iommu_data,
> >>> struct vfio_domain *domain, *d;
> >>> struct bus_type *bus = NULL, *mdev_bus;
> >>> int ret;
> >>> - bool resv_msi;
> >>> + bool resv_msi, msi_remap;
> >>> phys_addr_t resv_msi_base;
> >>>
> >>> mutex_lock(&iommu->lock);
> >>> @@ -1284,8 +1285,10 @@ static int
> vfio_iommu_type1_attach_group(void
> >>> *iommu_data,
> >>> INIT_LIST_HEAD(&domain->group_list);
> >>> list_add(&group->next, &domain->group_list);
> >>>
> >>> - if (!allow_unsafe_interrupts &&
> >>> - !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> >>> + msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> >>
> >> There can be multiple interrupt-controller, at-least theoretically it is
> possible and not sure practically it exists and supported, where not all may
> support IRQ_REMAP. If that is the case be then should we check for IRQ-
> REMAP for that device-bus irq-domain?
> >>
> > I mentioned in the cover letter that the approach was defensive and
> > rough today. As soon as we detect an MSI controller in the platform
> > that has no support for MSI remapping we flag the assignment as
> > unsafe. I think this approach was agreed on the ML. Such rough
> > assessment was used in the past on x86.
> >
> > I am reluctant to add more complexity at that stage. This can be
> > improved latter I think when such platforms show up.
>
> I don't think this is worth it. If the system is so broken that the designer
> cannot make up their mind about device isolation, too bad.
> People will either disable the non-isolating MSI controller altogether, or
> force
> the unsafe flag.
Understand, just want to be sure that this is a known limitation. It will be
good if we have some comment around this function.
Thanks
-Bharat
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v6 17/18] vfio/type1: Check MSI remapping at irq domain level
Hi Eric,
> -Original Message-
> From: Eric Auger [mailto:eric.au...@redhat.com]
> Sent: Friday, January 06, 2017 12:35 AM
> To: eric.au...@redhat.com; eric.auger@gmail.com;
> christoffer.d...@linaro.org; marc.zyng...@arm.com;
> robin.mur...@arm.com; alex.william...@redhat.com;
> will.dea...@arm.com; j...@8bytes.org; t...@linutronix.de;
> ja...@lakedaemon.net; linux-arm-ker...@lists.infradead.org
> Cc: k...@vger.kernel.org; drjo...@redhat.com; linux-
> ker...@vger.kernel.org; pranav.sawargaon...@gmail.com;
> iommu@lists.linux-foundation.org; punit.agra...@arm.com; Diana Madalina
> Craciun ; gpkulka...@gmail.com;
> shank...@codeaurora.org; Bharat Bhushan ;
> geethasowjanya.ak...@gmail.com
> Subject: [PATCH v6 17/18] vfio/type1: Check MSI remapping at irq domain
> level
>
> In case the IOMMU translates MSI transactions (typical case on ARM), we
> check MSI remapping capability at IRQ domain level. Otherwise it is checked
> at IOMMU level.
>
> At this stage the arm-smmu-(v3) still advertise the
> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be removed
> in subsequent patches.
>
> Signed-off-by: Eric Auger
>
> ---
>
> v6: rewrite test
> ---
> drivers/vfio/vfio_iommu_type1.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c
> b/drivers/vfio/vfio_iommu_type1.c index b473ef80..fa0b5c4 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -40,6 +40,7 @@
> #include
> #include
> #include
> +#include
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson
> "
> @@ -1208,7 +1209,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> struct vfio_domain *domain, *d;
> struct bus_type *bus = NULL, *mdev_bus;
> int ret;
> - bool resv_msi;
> + bool resv_msi, msi_remap;
> phys_addr_t resv_msi_base;
>
> mutex_lock(&iommu->lock);
> @@ -1284,8 +1285,10 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> INIT_LIST_HEAD(&domain->group_list);
> list_add(&group->next, &domain->group_list);
>
> - if (!allow_unsafe_interrupts &&
> - !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> + msi_remap = resv_msi ? irq_domain_check_msi_remap() :
There can be multiple interrupt-controller, at-least theoretically it is
possible and not sure practically it exists and supported, where not all may
support IRQ_REMAP. If that is the case be then should we check for IRQ-REMAP
for that device-bus irq-domain?
Thanks
-Bharat
> + iommu_capable(bus,
> IOMMU_CAP_INTR_REMAP);
> +
> + if (!allow_unsafe_interrupts && !msi_remap) {
> pr_warn("%s: No interrupt remapping support. Use the
> module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support
> on this platform\n",
> __func__);
> ret = -EPERM;
> --
> 1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap
Hi Mark, > -Original Message- > From: Auger Eric [mailto:eric.au...@redhat.com] > Sent: Thursday, January 05, 2017 5:39 PM > To: Marc Zyngier ; eric.auger@gmail.com; > christoffer.d...@linaro.org; robin.mur...@arm.com; > alex.william...@redhat.com; will.dea...@arm.com; j...@8bytes.org; > t...@linutronix.de; ja...@lakedaemon.net; linux-arm- > ker...@lists.infradead.org > Cc: drjo...@redhat.com; k...@vger.kernel.org; punit.agra...@arm.com; > linux-ker...@vger.kernel.org; geethasowjanya.ak...@gmail.com; Diana > Madalina Craciun ; iommu@lists.linux- > foundation.org; pranav.sawargaon...@gmail.com; Bharat Bhushan > ; shank...@codeaurora.org; > gpkulka...@gmail.com > Subject: Re: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap > > Hi Marc, > > On 05/01/2017 12:57, Marc Zyngier wrote: > > On 05/01/17 11:29, Auger Eric wrote: > >> Hi Marc, > >> > >> On 05/01/2017 12:25, Marc Zyngier wrote: > >>> On 05/01/17 10:45, Auger Eric wrote: > >>>> Hi Marc, > >>>> > >>>> On 04/01/2017 16:27, Marc Zyngier wrote: > >>>>> On 04/01/17 14:11, Auger Eric wrote: > >>>>>> Hi Marc, > >>>>>> > >>>>>> On 04/01/2017 14:46, Marc Zyngier wrote: > >>>>>>> Hi Eric, > >>>>>>> > >>>>>>> On 04/01/17 13:32, Eric Auger wrote: > >>>>>>>> This new function checks whether all platform and PCI MSI > >>>>>>>> domains implement IRQ remapping. This is useful to understand > >>>>>>>> whether VFIO passthrough is safe with respect to interrupts. > >>>>>>>> > >>>>>>>> On ARM typically an MSI controller can sit downstream to the > >>>>>>>> IOMMU without preventing VFIO passthrough. > >>>>>>>> As such any assigned device can write into the MSI doorbell. > >>>>>>>> In case the MSI controller implements IRQ remapping, assigned > >>>>>>>> devices will not be able to trigger interrupts towards the > >>>>>>>> host. On the contrary, the assignment must be emphasized as > >>>>>>>> unsafe with respect to interrupts. > >>>>>>>> > >>>>>>>> Signed-off-by: Eric Auger > >>>>>>>> > >>>>>>>> --- > >>>>>>>> > >>>>>>>> v4 -> v5: > >>>>>>>> - Handle DOMAIN_BUS_FSL_MC_MSI domains > >>>>>>>> - Check parents > >>>>>>>> --- > >>>>>>>> include/linux/irqdomain.h | 1 + > >>>>>>>> kernel/irq/irqdomain.c| 41 > + > >>>>>>>> 2 files changed, 42 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/include/linux/irqdomain.h > >>>>>>>> b/include/linux/irqdomain.h index ab017b2..281a40f 100644 > >>>>>>>> --- a/include/linux/irqdomain.h > >>>>>>>> +++ b/include/linux/irqdomain.h > >>>>>>>> @@ -219,6 +219,7 @@ struct irq_domain > *irq_domain_add_legacy(struct device_node *of_node, > >>>>>>>> void *host_data); > >>>>>>>> extern struct irq_domain *irq_find_matching_fwspec(struct > irq_fwspec *fwspec, > >>>>>>>> enum > irq_domain_bus_token bus_token); > >>>>>>>> +extern bool irq_domain_check_msi_remap(void); > >>>>>>>> extern void irq_set_default_host(struct irq_domain *host); > >>>>>>>> extern int irq_domain_alloc_descs(int virq, unsigned int nr_irqs, > >>>>>>>>irq_hw_number_t hwirq, int node, > diff --git > >>>>>>>> a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index > >>>>>>>> 8c0a0ae..700caea 100644 > >>>>>>>> --- a/kernel/irq/irqdomain.c > >>>>>>>> +++ b/kernel/irq/irqdomain.c > >>>>>>>> @@ -278,6 +278,47 @@ struct irq_domain > >>>>>>>> *irq_find_matching_fwspec(struct irq_fwspec *fwspec, > >>>>>>>> EXPORT_SYMBOL_GPL(irq_find_matching_fwspec); &
RE: [RFC v4 00/16] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
Hi Eric, Tested this series on NXP ARMv8 platform. Thanks -Bharat > -Original Message- > From: Eric Auger [mailto:eric.au...@redhat.com] > Sent: Wednesday, December 14, 2016 2:00 AM > To: eric.au...@redhat.com; eric.auger@gmail.com; > christoffer.d...@linaro.org; marc.zyng...@arm.com; > robin.mur...@arm.com; alex.william...@redhat.com; > will.dea...@arm.com; j...@8bytes.org; t...@linutronix.de; > ja...@lakedaemon.net; linux-arm-ker...@lists.infradead.org > Cc: k...@vger.kernel.org; drjo...@redhat.com; linux- > ker...@vger.kernel.org; pranav.sawargaon...@gmail.com; > iommu@lists.linux-foundation.org; punit.agra...@arm.com; Diana Madalina > Craciun ; gpkulka...@gmail.com; > shank...@codeaurora.org; Bharat Bhushan > Subject: [RFC v4 00/16] KVM PCIe/MSI passthrough on ARM/ARM64 and > IOVA reserved regions > > Following LPC discussions, we now report reserved regions through iommu- > group sysfs reserved_regions attribute file. > > Reserved regions are populated through the IOMMU get_resv_region > callback (former get_dm_regions), now implemented by amd-iommu, intel- > iommu and arm-smmu: > - the amd-iommu reports device direct mapped regions. > - the intel-iommu reports the [0xfee0 - 0xfeef] MSI window > as an IOMMU_RESV_NOMAP reserved region. > - the arm-smmu reports the MSI window (arbitrarily located at > 0x800 and 1MB large). > > Unsafe interrupt assignment is tested by enumerating all MSI irq domains > and checking they support MSI remapping. This check is done in case we > detect the iommu translates MSI (an IOMMU_RESV_MSI window exists). > Otherwise the IRQ remapping capability is checked at IOMMU level. > Obviously this is a defensive IRQ safety assessment. > Assuming there are several MSI controllers in the system and at least one > does not implement IRQ remapping, the assignment will be considered as > unsafe (even if this controller is not acessible from the assigned devices). > > The series integrates a not officially posted patch from Robin: > "iommu/dma: Allow MSI-only cookies". > > Best Regards > > Eric > > Git: complete series available at > https://github.com/eauger/linux/tree/v4.9-reserved-v4 > > History: > > RFCv3 -> RFCv4: > - arm-smmu driver does not register PCI host bridge windows as > reserved regions anymore > - Implement reserved region get/put callbacks also in arm-smmuv3 > - take the iommu_group lock on iommu_get_group_resv_regions > - add a type field in iommu_resv_region instead of using prot > - init the region list_head in iommu_alloc_resv_region, also > add type parameter > - iommu_insert_resv_region manage overlaps and sort reserved > windows > - address IRQ safety assessment by enumerating all the MSI irq > domains and checking the MSI_REMAP flag > - update Documentation/ABI/testing/sysfs-kernel-iommu_groups > - Did not add T-b since the code has significantly changed > > RFCv2 -> RFCv3: > - switch to an iommu-group sysfs API > - use new dummy allocator provided by Robin > - dummy allocator initialized by vfio-iommu-type1 after enumerating > the reserved regions > - at the moment ARM MSI base address/size is left unchanged compared > to v2 > - we currently report reserved regions and not usable IOVA regions as > requested by Alex > > RFC v1 -> v2: > - fix intel_add_reserved_regions > - add mutex lock/unlock in vfio_iommu_type1 > > > Eric Auger (16): > iommu/dma: Allow MSI-only cookies > iommu: Rename iommu_dm_regions into iommu_resv_regions > iommu: Add a new type field in iommu_resv_region > iommu: iommu_alloc_resv_region > iommu: Only map direct mapped regions > iommu: iommu_get_group_resv_regions > iommu: Implement reserved_regions iommu-group sysfs file > iommu/vt-d: Implement reserved region get/put callbacks > iommu/arm-smmu: Implement reserved region get/put callbacks > iommu/arm-smmu-v3: Implement reserved region get/put callbacks > irqdomain: Add IRQ_DOMAIN_FLAG_MSI_REMAP value > irqdomain: irq_domain_check_msi_remap > irqchip/gicv3-its: Sets IRQ_DOMAIN_FLAG_MSI_REMAP > vfio/type1: Allow transparent MSI IOVA allocation > vfio/type1: Check MSI remapping at irq domain level > iommu/arm-smmu: Do not advertise IOMMU_CAP_INTR_REMAP anymore > > .../ABI/testing/sysfs-kernel-iommu_groups | 9 ++ > drivers/iommu/amd_iommu.c | 21 +-- > drivers/iommu/arm-smmu-v3.c| 30 +++- > drivers/iommu/arm-smmu.c | 30 +++- > drivers/iommu/dma-iommu.c | 116 +--- > drivers/iommu/intel-iommu.c| 50 +-- >
RE: RFC: extend iommu-map binding to support #iommu-cells > 1
> -Original Message- > From: Robin Murphy [mailto:robin.mur...@arm.com] > Sent: Friday, December 16, 2016 8:21 PM > To: Stuart Yoder ; Mark Rutland > > Cc: will.dea...@arm.com; robh...@kernel.org; Bharat Bhushan > ; Nipun Gupta ; Diana > Madalina Craciun ; devicet...@vger.kernel.org; > iommu@lists.linux-foundation.org > Subject: Re: RFC: extend iommu-map binding to support #iommu-cells > 1 > > On 16/12/16 14:21, Stuart Yoder wrote: > > > > > >> -Original Message- > >> From: Mark Rutland [mailto:mark.rutl...@arm.com] > >> Sent: Friday, December 16, 2016 5:39 AM > >> To: Stuart Yoder > >> Cc: robin.mur...@arm.com; will.dea...@arm.com; > robh...@kernel.org; > >> Bharat Bhushan ; Nipun Gupta > >> ; Diana Madalina Craciun > >> ; devicet...@vger.kernel.org; > >> iommu@lists.linux-foundation.org > >> Subject: Re: RFC: extend iommu-map binding to support #iommu-cells > > >> 1 > >> > >> On Fri, Dec 16, 2016 at 02:36:57AM +, Stuart Yoder wrote: > >>> For context, please see the thread: > >>> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw > >>> ww.spinics.net%2Flists%2Farm- > >> > kernel%2Fmsg539066.html&data=01%7C01%7Cstuart.yoder%40nxp.com%7C > 0464e > >> 12bddfd42e0f0a508d425a847cb%7C686ea > >> > 1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=KnzeOlbts271GwD1cpx2FLMsKn > xw%2F > >> OdiGVTp6%2BKFrbM%3D&reserved=0 > >>> > >>> The existing iommu-map binding did not account for the situation > >>> where #iommu-cells == 2, as permitted in the ARM SMMU binding. The > >>> 2nd cell of the IOMMU specifier being the SMR mask. The existing > >>> binding defines the mapping as: > >>>Any RID r in the interval [rid-base, rid-base + length) is associated > >>> with > >>>the listed IOMMU, with the iommu-specifier (r - rid-base + iommu- > base). > >>> > >>> ...and that does not work if iommu-base is 2 cells, the second being > >>> the SMR mask. > >>> > >>> While this can be worked around by always having length=1, it seems > >>> we can get this cleaned up by updating the binding definition for iommu- > map. > >> > >> To reiterate, I'm very much not keen on the pci-iommu binding having > >> knowledge of the decomposition of iommu-specifiers from other > bindings. > > > > With the current definition of iommu-map we already have baked in an > > assumption that an iommu-specifier is a value that can be incremented > > by 1 to get to the next sequential specifier. So the binding already > > has "knowledge" that applies in most, but not all cases. > > > > The generic iommu binding also defines a case where #iommu-cells=4 for > > some IOMMUs. > > > > Since the ARM SMMU is a special case, perhaps the intepretation of an > > iommu-specifier in the context of iommu-map should be moved into the > > SMMU binding. > > > >> As mentioned previously, there's an intended interpretation [1] that > >> we need to fix up the pci-iommu binding with. With that, I don't > >> think it's even necessary to bodge iommu-cells = <1> AFAICT. > > > > You had said in the previous thread: > > > > > I had intended that the offset was added to the final cell of the > > > iommu-specifier (i.e. that the iommu-specifier was treated as a large > > > number). > > > > > You can handle this case by adding additional entries in the map table, > > > with a single entry length. > > > > I understand that, and it works, but I don't see why the definition > > has to be that the offset is added to the "final cell". > > Because the cells of a specifier form a single opaque big-endian value. > Were DT little-endian it would be the "first cell". To be pedantic, both of > those descriptions are technically wrong because they fail to account for > overflow and carry up into the next cell (in whichever direction). > > > Why can't it be > > an iommu specific definition that makes sense for a given IOMMU? > > Because the implementation would then become a nightmarish rabbit- > warren of special-cases, largely defeating the point of a *generic* binding. > At > the very least it'd have to chase every phandle and determine its compatible > just to work out what to do with any given specifier - merely thinking about > the complexity of the error handling for the number of additional failure > modes that introduces is enough to put me off. Currently if iommu-cells = 2 then SMMU treats the 2 cells as stream-id + stream-id mask. While DT binding is silent on how 2 cells, which means that actually stream-id could be more than 32bit, but SMMU uses it in the way it wants to, which is not correct. If we be more explicit on defining the meaning of two cells than it will avoid confusion and implementation will match the DT binding definition. Thanks -Bharat > > Robin. > > > > > Stuart > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: RFC: extend iommu-map binding to support #iommu-cells > 1
> -Original Message- > From: Stuart Yoder > Sent: Friday, December 16, 2016 8:07 AM > To: Mark Rutland ; robin.mur...@arm.com; > will.dea...@arm.com > Cc: robh...@kernel.org; Bharat Bhushan ; > Nipun Gupta ; Diana Madalina Craciun > ; devicet...@vger.kernel.org; iommu@lists.linux- > foundation.org > Subject: RFC: extend iommu-map binding to support #iommu-cells > 1 > > For context, please see the thread: > https://www.spinics.net/lists/arm-kernel/msg539066.html > > The existing iommu-map binding did not account for the situation where > #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell of > the IOMMU specifier being the SMR mask. The existing binding defines the > mapping as: >Any RID r in the interval [rid-base, rid-base + length) is associated with >the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base). > > ...and that does not work if iommu-base is 2 cells, the second being the SMR > mask. > > While this can be worked around by always having length=1, it seems we can > get this cleaned up by updating the binding definition for iommu-map. > > See patch below. Thoughts? > > Thanks, > Stuart > > - > > diff --git a/Documentation/devicetree/bindings/pci/pci-iommu.txt > b/Documentation/devicetree/bindings/pci/pci-iommu.txt > index 56c8296..e81b461 100644 > --- a/Documentation/devicetree/bindings/pci/pci-iommu.txt > +++ b/Documentation/devicetree/bindings/pci/pci-iommu.txt > @@ -38,8 +38,20 @@ Optional properties >The property is an arbitrary number of tuples of >(rid-base,iommu,iommu-base,length). > > - Any RID r in the interval [rid-base, rid-base + length) is associated with > - the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base). > + If the associated IOMMU has an #iommu-cells value of 1, any RID r in > + the interval [rid-base, rid-base + length) is associated with the > + listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base). > + > + ARM SMMU Note: > +The ARM SMMU binding permits an #iommu-cells value of 2 and in this > +case defines an IOMMU specifier to be: (stream-id,smr-mask) > + > +In an iommu-map this means the iommu-base consists of 2 cells: > +(rid-base,iommu,[stream-id,smr-mask],length). > + > +In this case the RID to IOMMU specifier mapping is defined to be: > +any RID r in the interval [rid-base, rid-base + length) is associated > +with the listed IOMMU, with the iommu-specifier (r - rid-base + stream- > id). Should not this be (r - rid-base + (stream-id & smr-mask)) ? So basically stream-id ranges from (stream-id & smr-mask) - (stream-id & smr-mask + (length - 1) ) Thanks -Bharat > > - iommu-map-mask: A mask to be applied to each Requester ID prior to > being >mapped to an iommu-specifier per the iommu-map property. > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
Hi Eric, I have tested this series on NXP platform. Thanks -Bharat > -Original Message- > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > boun...@lists.linux-foundation.org] On Behalf Of Eric Auger > Sent: Tuesday, November 15, 2016 6:39 PM > To: eric.au...@redhat.com; eric.auger@gmail.com; > christoffer.d...@linaro.org; marc.zyng...@arm.com; > robin.mur...@arm.com; alex.william...@redhat.com; > will.dea...@arm.com; j...@8bytes.org; t...@linutronix.de; > ja...@lakedaemon.net; linux-arm-ker...@lists.infradead.org > Cc: drjo...@redhat.com; k...@vger.kernel.org; punit.agra...@arm.com; > linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org; > pranav.sawargaon...@gmail.com > Subject: [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and > IOVA reserved regions > > Following LPC discussions, we now report reserved regions through iommu- > group sysfs reserved_regions attribute file. > > Reserved regions are populated through the IOMMU get_resv_region > callback (former get_dm_regions), now implemented by amd-iommu, intel- > iommu and arm-smmu. > > The intel-iommu reports the [FEE0_h - FEF0_000h] MSI window as an > IOMMU_RESV_NOMAP reserved region. > > arm-smmu reports the MSI window (arbitrarily located at 0x800 and 1MB > large) and the PCI host bridge windows. > > The series integrates a not officially posted patch from Robin: > "iommu/dma: Allow MSI-only cookies". > > This series currently does not address IRQ safety assessment. > > Best Regards > > Eric > > Git: complete series available at > https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3 > > History: > RFC v2 -> v3: > - switch to an iommu-group sysfs API > - use new dummy allocator provided by Robin > - dummy allocator initialized by vfio-iommu-type1 after enumerating > the reserved regions > - at the moment ARM MSI base address/size is left unchanged compared > to v2 > - we currently report reserved regions and not usable IOVA regions as > requested by Alex > > RFC v1 -> v2: > - fix intel_add_reserved_regions > - add mutex lock/unlock in vfio_iommu_type1 > > > Eric Auger (10): > iommu/dma: Allow MSI-only cookies > iommu: Rename iommu_dm_regions into iommu_resv_regions > iommu: Add new reserved IOMMU attributes > iommu: iommu_alloc_resv_region > iommu: Do not map reserved regions > iommu: iommu_get_group_resv_regions > iommu: Implement reserved_regions iommu-group sysfs file > iommu/vt-d: Implement reserved region get/put callbacks > iommu/arm-smmu: Implement reserved region get/put callbacks > vfio/type1: Get MSI cookie > > drivers/iommu/amd_iommu.c | 20 +++--- > drivers/iommu/arm-smmu.c| 52 +++ > drivers/iommu/dma-iommu.c | 116 ++ > --- > drivers/iommu/intel-iommu.c | 50 ++ > drivers/iommu/iommu.c | 141 > > drivers/vfio/vfio_iommu_type1.c | 26 > include/linux/dma-iommu.h | 7 ++ > include/linux/iommu.h | 49 ++ > 8 files changed, 391 insertions(+), 70 deletions(-) > > -- > 1.9.1 > > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
Hi Eric, Have you sent out QEMU side patches based on this new approach? In case I missed please point me the patches? Thanks -Bharat > -Original Message- > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu- > boun...@lists.linux-foundation.org] On Behalf Of Eric Auger > Sent: Tuesday, November 15, 2016 6:39 PM > To: eric.au...@redhat.com; eric.auger@gmail.com; > christoffer.d...@linaro.org; marc.zyng...@arm.com; > robin.mur...@arm.com; alex.william...@redhat.com; > will.dea...@arm.com; j...@8bytes.org; t...@linutronix.de; > ja...@lakedaemon.net; linux-arm-ker...@lists.infradead.org > Cc: drjo...@redhat.com; k...@vger.kernel.org; punit.agra...@arm.com; > linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org; > pranav.sawargaon...@gmail.com > Subject: [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and > IOVA reserved regions > > Following LPC discussions, we now report reserved regions through iommu- > group sysfs reserved_regions attribute file. > > Reserved regions are populated through the IOMMU get_resv_region > callback (former get_dm_regions), now implemented by amd-iommu, intel- > iommu and arm-smmu. > > The intel-iommu reports the [FEE0_h - FEF0_000h] MSI window as an > IOMMU_RESV_NOMAP reserved region. > > arm-smmu reports the MSI window (arbitrarily located at 0x800 and 1MB > large) and the PCI host bridge windows. > > The series integrates a not officially posted patch from Robin: > "iommu/dma: Allow MSI-only cookies". > > This series currently does not address IRQ safety assessment. > > Best Regards > > Eric > > Git: complete series available at > https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3 > > History: > RFC v2 -> v3: > - switch to an iommu-group sysfs API > - use new dummy allocator provided by Robin > - dummy allocator initialized by vfio-iommu-type1 after enumerating > the reserved regions > - at the moment ARM MSI base address/size is left unchanged compared > to v2 > - we currently report reserved regions and not usable IOVA regions as > requested by Alex > > RFC v1 -> v2: > - fix intel_add_reserved_regions > - add mutex lock/unlock in vfio_iommu_type1 > > > Eric Auger (10): > iommu/dma: Allow MSI-only cookies > iommu: Rename iommu_dm_regions into iommu_resv_regions > iommu: Add new reserved IOMMU attributes > iommu: iommu_alloc_resv_region > iommu: Do not map reserved regions > iommu: iommu_get_group_resv_regions > iommu: Implement reserved_regions iommu-group sysfs file > iommu/vt-d: Implement reserved region get/put callbacks > iommu/arm-smmu: Implement reserved region get/put callbacks > vfio/type1: Get MSI cookie > > drivers/iommu/amd_iommu.c | 20 +++--- > drivers/iommu/arm-smmu.c| 52 +++ > drivers/iommu/dma-iommu.c | 116 ++ > --- > drivers/iommu/intel-iommu.c | 50 ++ > drivers/iommu/iommu.c | 141 > > drivers/vfio/vfio_iommu_type1.c | 26 > include/linux/dma-iommu.h | 7 ++ > include/linux/iommu.h | 49 ++ > 8 files changed, 391 insertions(+), 70 deletions(-) > > -- > 1.9.1 > > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
> -Original Message- > From: Wood Scott-B07421 > Sent: Friday, December 06, 2013 5:31 AM > To: Bhushan Bharat-R65777 > Cc: Alex Williamson; linux-...@vger.kernel.org; ag...@suse.de; Yoder Stuart- > B08248; iommu@lists.linux-foundation.org; bhelg...@google.com; linuxppc- > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU) > > On Sun, 2013-11-24 at 23:33 -0600, Bharat Bhushan wrote: > > > > > -Original Message- > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > Sent: Friday, November 22, 2013 2:31 AM > > > To: Wood Scott-B07421 > > > Cc: Bhushan Bharat-R65777; linux-...@vger.kernel.org; ag...@suse.de; > > > Yoder Stuart-B08248; iommu@lists.linux-foundation.org; > > > bhelg...@google.com; linuxppc- d...@lists.ozlabs.org; > > > linux-ker...@vger.kernel.org > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale > > > IOMMU (PAMU) > > > > > > On Thu, 2013-11-21 at 14:47 -0600, Scott Wood wrote: > > > > On Thu, 2013-11-21 at 13:43 -0700, Alex Williamson wrote: > > > > > On Thu, 2013-11-21 at 11:20 +, Bharat Bhushan wrote: > > > > > > > > > > > > > -Original Message- > > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > > Sent: Thursday, November 21, 2013 12:17 AM > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > Cc: j...@8bytes.org; bhelg...@google.com; ag...@suse.de; > > > > > > > Wood Scott-B07421; Yoder Stuart-B08248; > > > > > > > iommu@lists.linux-foundation.org; linux- > > > > > > > p...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- > > > > > > > ker...@vger.kernel.org; Bhushan Bharat-R65777 > > > > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for > > > > > > > Freescale IOMMU (PAMU) > > > > > > > > > > > > > > Is VFIO_IOMMU_PAMU_GET_MSI_BANK_COUNT per aperture (ie. each > > > > > > > vfio user has $COUNT regions at their disposal exclusively)? > > > > > > > > > > > > Number of msi-bank count is system wide and not per aperture, > > > > > > But will be > > > setting windows for banks in the device aperture. > > > > > > So say if we are direct assigning 2 pci device (both have > > > > > > different iommu > > > group, so 2 aperture in iommu) to VM. > > > > > > Now qemu can make only one call to know how many msi-banks are > > > > > > there but > > > it must set sub-windows for all banks for both pci device in its > > > respective aperture. > > > > > > > > > > I'm still confused. What I want to make sure of is that the > > > > > banks are independent per aperture. For instance, if we have > > > > > two separate userspace processes operating independently and > > > > > they both chose to use msi bank zero for their device, that's > > > > > bank zero within each aperture and doesn't interfere. Or > > > > > another way to ask is can a malicious user interfere with other users > > > > > by > using the wrong bank. > > > > > Thanks, > > > > > > > > They can interfere. > > > > Want to be sure of how they can interfere? > > If more than one VFIO user shares the same MSI group, one of the users can > send > MSIs to another user, by using the wrong interrupt within the bank. > Unexpected > MSIs could cause misbehavior or denial of service. > > > >> With this hardware, the only way to prevent that > > > > is to make sure that a bank is not shared by multiple protection > > > > contexts. > > > > For some of our users, though, I believe preventing this is less > > > > important than the performance benefit. > > > > So should we let this patch series in without protection? > > No, there should be some sort of opt-in mechanism similar to IOMMU-less VFIO > -- > but not the same exact one, since one is a much more serious loss of isolation > than the other. Can you please elaborate "opt-in mechanism"? > > > > I think we need some sort of ownership model around the msi banks then. > > > Otherwise there's nothing preventing another userspace from > > > attempting an MSI based attack on other users, or perhaps even on > > > the host. VFIO can't allow that. Thanks, > > > > We have very few (3 MSI bank on most of chips), so we can not assign > > one to each userspace. > > That depends on how many users there are. What I think we can do is: - Reserve one MSI region for host. Host will not share MSI region with Guest. - For upto 2 Guest (MAX msi with host - 1) give then separate MSI sub regions - Additional Guest will share MSI region with other guest. Any better suggestion are most welcome. Thanks -Bharat > > -Scott > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
> -Original Message- > From: Wood Scott-B07421 > Sent: Friday, December 06, 2013 5:52 AM > To: Bhushan Bharat-R65777 > Cc: Alex Williamson; linux-...@vger.kernel.org; ag...@suse.de; Yoder Stuart- > B08248; iommu@lists.linux-foundation.org; bhelg...@google.com; linuxppc- > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU) > > On Thu, 2013-11-28 at 03:19 -0600, Bharat Bhushan wrote: > > > > > -Original Message- > > > From: Bhushan Bharat-R65777 > > > Sent: Wednesday, November 27, 2013 9:39 PM > > > To: 'Alex Williamson' > > > Cc: Wood Scott-B07421; linux-...@vger.kernel.org; ag...@suse.de; > > > Yoder Stuart- B08248; iommu@lists.linux-foundation.org; > > > bhelg...@google.com; linuxppc- d...@lists.ozlabs.org; > > > linux-ker...@vger.kernel.org > > > Subject: RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale > > > IOMMU (PAMU) > > > > > > > > > > > > > -Original Message- > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > Sent: Monday, November 25, 2013 10:08 PM > > > > To: Bhushan Bharat-R65777 > > > > Cc: Wood Scott-B07421; linux-...@vger.kernel.org; ag...@suse.de; > > > > Yoder > > > > Stuart- B08248; iommu@lists.linux-foundation.org; > > > > bhelg...@google.com; > > > > linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale > > > > IOMMU > > > > (PAMU) > > > > > > > > On Mon, 2013-11-25 at 05:33 +, Bharat Bhushan wrote: > > > > > > > > > > > -Original Message- > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > Sent: Friday, November 22, 2013 2:31 AM > > > > > > To: Wood Scott-B07421 > > > > > > Cc: Bhushan Bharat-R65777; linux-...@vger.kernel.org; > > > > > > ag...@suse.de; Yoder Stuart-B08248; > > > > > > iommu@lists.linux-foundation.org; bhelg...@google.com; > > > > > > linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > > > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for > > > > > > Freescale IOMMU (PAMU) > > > > > > > > > > > > On Thu, 2013-11-21 at 14:47 -0600, Scott Wood wrote: > > > > > > > On Thu, 2013-11-21 at 13:43 -0700, Alex Williamson wrote: > > > > > > > > On Thu, 2013-11-21 at 11:20 +, Bharat Bhushan wrote: > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > > > From: Alex Williamson > > > > > > > > > > [mailto:alex.william...@redhat.com] > > > > > > > > > > Sent: Thursday, November 21, 2013 12:17 AM > > > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > > > Cc: j...@8bytes.org; bhelg...@google.com; > > > > > > > > > > ag...@suse.de; Wood Scott-B07421; Yoder Stuart-B08248; > > > > > > > > > > iommu@lists.linux-foundation.org; linux- > > > > > > > > > > p...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; > > > > > > > > > > linux- ker...@vger.kernel.org; Bhushan Bharat-R65777 > > > > > > > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for > > > > > > > > > > Freescale IOMMU (PAMU) > > > > > > > > > > > > > > > > > > > > Is VFIO_IOMMU_PAMU_GET_MSI_BANK_COUNT per aperture (ie. > > > > > > > > > > each vfio user has $COUNT regions at their disposal > exclusively)? > > > > > > > > > > > > > > > > > > Number of msi-bank count is system wide and not per > > > > > > > > > aperture, But will be > > > > > > setting windows for banks in the device aperture. > > > > > > > > > So say if we are direct assigning 2 pci device (both > > > > > > > > > have different iommu > > > > > > group, so 2 aperture in iommu) to VM. > > > > > > > > > Now qemu can make only one call to know how many > > > > > > &g
RE: [PATCH 1/9 v2] pci:msi: add weak function for returning msi region info
> -Original Message-
> From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci-ow...@vger.kernel.org]
> On Behalf Of Bjorn Helgaas
> Sent: Tuesday, November 26, 2013 5:06 AM
> To: Bhushan Bharat-R65777
> Cc: alex.william...@redhat.com; j...@8bytes.org; ag...@suse.de; Wood Scott-
> B07421; Yoder Stuart-B08248; iommu@lists.linux-foundation.org; linux-
> p...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH 1/9 v2] pci:msi: add weak function for returning msi
> region
> info
>
> On Tue, Nov 19, 2013 at 10:47:05AM +0530, Bharat Bhushan wrote:
> > In Aperture type of IOMMU (like FSL PAMU), VFIO-iommu system need to
> > know the MSI region to map its window in h/w. This patch just defines
> > the required weak functions only and will be used by followup patches.
> >
> > Signed-off-by: Bharat Bhushan
> > ---
> > v1->v2
> > - Added description on "struct msi_region"
> >
> > drivers/pci/msi.c | 22 ++
> > include/linux/msi.h | 14 ++
> > 2 files changed, 36 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index
> > d5f90d6..2643a29 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -67,6 +67,28 @@ int __weak arch_msi_check_device(struct pci_dev *dev, int
> nvec, int type)
> > return chip->check_device(chip, dev, nvec, type); }
> >
> > +int __weak arch_msi_get_region_count(void) {
> > + return 0;
> > +}
> > +
> > +int __weak arch_msi_get_region(int region_num, struct msi_region
> > +*region) {
> > + return 0;
> > +}
> > +
> > +int msi_get_region_count(void)
> > +{
> > + return arch_msi_get_region_count();
> > +}
> > +EXPORT_SYMBOL(msi_get_region_count);
> > +
> > +int msi_get_region(int region_num, struct msi_region *region) {
> > + return arch_msi_get_region(region_num, region); }
> > +EXPORT_SYMBOL(msi_get_region);
> > +
> > int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int
> > type) {
> > struct msi_desc *entry;
> > diff --git a/include/linux/msi.h b/include/linux/msi.h index
> > b17ead8..ade1480 100644
> > --- a/include/linux/msi.h
> > +++ b/include/linux/msi.h
> > @@ -51,6 +51,18 @@ struct msi_desc {
> > };
> >
> > /*
> > + * This structure is used to get
> > + * - physical address
> > + * - size
> > + * of a msi region
> > + */
> > +struct msi_region {
> > + int region_num; /* MSI region number */
> > + dma_addr_t addr; /* Address of MSI region */
> > + size_t size; /* Size of MSI region */ };
> > +
> > +/*
> > * The arch hooks to setup up msi irqs. Those functions are
> > * implemented as weak symbols so that they /can/ be overriden by
> > * architecture specific code if needed.
> > @@ -64,6 +76,8 @@ void arch_restore_msi_irqs(struct pci_dev *dev, int
> > irq);
> >
> > void default_teardown_msi_irqs(struct pci_dev *dev); void
> > default_restore_msi_irqs(struct pci_dev *dev, int irq);
> > +int arch_msi_get_region_count(void);
> > +int arch_msi_get_region(int region_num, struct msi_region *region);
>
> It doesn't look like any of this (struct msi_region, msi_get_region(),
> msi_get_region_count()) is actually used by drivers/pci/msi.c, so I don't
> think
> it needs to be declared in generic code. It looks like it's only used in
> drivers/vfio/vfio_iommu_fsl_pamu.c, where you already know you have an FSL
> IOMMU, and you can just call FSL-specific interfaces directly.
Thanks Bjorn,
Want to be sure of what you are suggesting.
What I understood is that we define these (struct msi_region, msi_get_region(),
msi_get_region_count()) in arch/powerpc/include/fsl_msi.h (a new file). Include
this header file directly in driver/vfio/vfio_iommu_fsl_pamu.c
Same also applies for msi_set_iova() in patch-5 ?
-Bharat
>
> Bjorn
>
> >
> > struct msi_chip {
> > struct module *owner;
> > --
> > 1.7.0.4
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in the
> body
> of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
> -Original Message- > From: Bhushan Bharat-R65777 > Sent: Wednesday, November 27, 2013 9:39 PM > To: 'Alex Williamson' > Cc: Wood Scott-B07421; linux-...@vger.kernel.org; ag...@suse.de; Yoder Stuart- > B08248; iommu@lists.linux-foundation.org; bhelg...@google.com; linuxppc- > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > Subject: RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU) > > > > > -Original Message- > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > Sent: Monday, November 25, 2013 10:08 PM > > To: Bhushan Bharat-R65777 > > Cc: Wood Scott-B07421; linux-...@vger.kernel.org; ag...@suse.de; Yoder > > Stuart- B08248; iommu@lists.linux-foundation.org; bhelg...@google.com; > > linuxppc- d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU > > (PAMU) > > > > On Mon, 2013-11-25 at 05:33 +, Bharat Bhushan wrote: > > > > > > > -Original Message- > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > Sent: Friday, November 22, 2013 2:31 AM > > > > To: Wood Scott-B07421 > > > > Cc: Bhushan Bharat-R65777; linux-...@vger.kernel.org; > > > > ag...@suse.de; Yoder Stuart-B08248; > > > > iommu@lists.linux-foundation.org; bhelg...@google.com; linuxppc- > > > > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale > > > > IOMMU (PAMU) > > > > > > > > On Thu, 2013-11-21 at 14:47 -0600, Scott Wood wrote: > > > > > On Thu, 2013-11-21 at 13:43 -0700, Alex Williamson wrote: > > > > > > On Thu, 2013-11-21 at 11:20 +, Bharat Bhushan wrote: > > > > > > > > > > > > > > > -Original Message- > > > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > > > Sent: Thursday, November 21, 2013 12:17 AM > > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > > Cc: j...@8bytes.org; bhelg...@google.com; ag...@suse.de; > > > > > > > > Wood Scott-B07421; Yoder Stuart-B08248; > > > > > > > > iommu@lists.linux-foundation.org; linux- > > > > > > > > p...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- > > > > > > > > ker...@vger.kernel.org; Bhushan Bharat-R65777 > > > > > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for > > > > > > > > Freescale IOMMU (PAMU) > > > > > > > > > > > > > > > > Is VFIO_IOMMU_PAMU_GET_MSI_BANK_COUNT per aperture (ie. > > > > > > > > each vfio user has $COUNT regions at their disposal > > > > > > > > exclusively)? > > > > > > > > > > > > > > Number of msi-bank count is system wide and not per > > > > > > > aperture, But will be > > > > setting windows for banks in the device aperture. > > > > > > > So say if we are direct assigning 2 pci device (both have > > > > > > > different iommu > > > > group, so 2 aperture in iommu) to VM. > > > > > > > Now qemu can make only one call to know how many msi-banks > > > > > > > are there but > > > > it must set sub-windows for all banks for both pci device in its > > > > respective aperture. > > > > > > > > > > > > I'm still confused. What I want to make sure of is that the > > > > > > banks are independent per aperture. For instance, if we have > > > > > > two separate userspace processes operating independently and > > > > > > they both chose to use msi bank zero for their device, that's > > > > > > bank zero within each aperture and doesn't interfere. Or > > > > > > another way to ask is can a malicious user interfere with > > > > > > other users by > > using the wrong bank. > > > > > > Thanks, > > > > > > > > > > They can interfere. > > > > > > Want to be sure of how they can interfere? > > > > What happens if more than one user selects the same MSI bank? > > Minimally, wouldn't that result in the IOMMU blocking transactions > > from the previous user once the new user
RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
> -Original Message- > From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Monday, November 25, 2013 10:08 PM > To: Bhushan Bharat-R65777 > Cc: Wood Scott-B07421; linux-...@vger.kernel.org; ag...@suse.de; Yoder Stuart- > B08248; iommu@lists.linux-foundation.org; bhelg...@google.com; linuxppc- > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU) > > On Mon, 2013-11-25 at 05:33 +, Bharat Bhushan wrote: > > > > > -Original Message- > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > Sent: Friday, November 22, 2013 2:31 AM > > > To: Wood Scott-B07421 > > > Cc: Bhushan Bharat-R65777; linux-...@vger.kernel.org; ag...@suse.de; > > > Yoder Stuart-B08248; iommu@lists.linux-foundation.org; > > > bhelg...@google.com; linuxppc- d...@lists.ozlabs.org; > > > linux-ker...@vger.kernel.org > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale > > > IOMMU (PAMU) > > > > > > On Thu, 2013-11-21 at 14:47 -0600, Scott Wood wrote: > > > > On Thu, 2013-11-21 at 13:43 -0700, Alex Williamson wrote: > > > > > On Thu, 2013-11-21 at 11:20 +, Bharat Bhushan wrote: > > > > > > > > > > > > > -Original Message- > > > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > > > Sent: Thursday, November 21, 2013 12:17 AM > > > > > > > To: Bhushan Bharat-R65777 > > > > > > > Cc: j...@8bytes.org; bhelg...@google.com; ag...@suse.de; > > > > > > > Wood Scott-B07421; Yoder Stuart-B08248; > > > > > > > iommu@lists.linux-foundation.org; linux- > > > > > > > p...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- > > > > > > > ker...@vger.kernel.org; Bhushan Bharat-R65777 > > > > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for > > > > > > > Freescale IOMMU (PAMU) > > > > > > > > > > > > > > Is VFIO_IOMMU_PAMU_GET_MSI_BANK_COUNT per aperture (ie. each > > > > > > > vfio user has $COUNT regions at their disposal exclusively)? > > > > > > > > > > > > Number of msi-bank count is system wide and not per aperture, > > > > > > But will be > > > setting windows for banks in the device aperture. > > > > > > So say if we are direct assigning 2 pci device (both have > > > > > > different iommu > > > group, so 2 aperture in iommu) to VM. > > > > > > Now qemu can make only one call to know how many msi-banks are > > > > > > there but > > > it must set sub-windows for all banks for both pci device in its > > > respective aperture. > > > > > > > > > > I'm still confused. What I want to make sure of is that the > > > > > banks are independent per aperture. For instance, if we have > > > > > two separate userspace processes operating independently and > > > > > they both chose to use msi bank zero for their device, that's > > > > > bank zero within each aperture and doesn't interfere. Or > > > > > another way to ask is can a malicious user interfere with other users > > > > > by > using the wrong bank. > > > > > Thanks, > > > > > > > > They can interfere. > > > > Want to be sure of how they can interfere? > > What happens if more than one user selects the same MSI bank? > Minimally, wouldn't that result in the IOMMU blocking transactions from the > previous user once the new user activates their mapping? Yes and no; With current implementation yes but with a minor change no. Later in this response I will explain how. > > > >> With this hardware, the only way to prevent that > > > > is to make sure that a bank is not shared by multiple protection > > > > contexts. > > > > For some of our users, though, I believe preventing this is less > > > > important than the performance benefit. > > > > So should we let this patch series in without protection? > > No. > > > > > > > I think we need some sort of ownership model around the msi banks then. > > > Otherwise there's nothing preventing another userspace from > > > attempting an MSI based attack on other users, or perhaps even on > > > the host
RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
> -Original Message- > From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Friday, November 22, 2013 2:31 AM > To: Wood Scott-B07421 > Cc: Bhushan Bharat-R65777; linux-...@vger.kernel.org; ag...@suse.de; Yoder > Stuart-B08248; iommu@lists.linux-foundation.org; bhelg...@google.com; > linuxppc- > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU) > > On Thu, 2013-11-21 at 14:47 -0600, Scott Wood wrote: > > On Thu, 2013-11-21 at 13:43 -0700, Alex Williamson wrote: > > > On Thu, 2013-11-21 at 11:20 +, Bharat Bhushan wrote: > > > > > > > > > -Original Message- > > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > > > Sent: Thursday, November 21, 2013 12:17 AM > > > > > To: Bhushan Bharat-R65777 > > > > > Cc: j...@8bytes.org; bhelg...@google.com; ag...@suse.de; Wood > > > > > Scott-B07421; Yoder Stuart-B08248; > > > > > iommu@lists.linux-foundation.org; linux- p...@vger.kernel.org; > > > > > linuxppc-...@lists.ozlabs.org; linux- ker...@vger.kernel.org; > > > > > Bhushan Bharat-R65777 > > > > > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale > > > > > IOMMU (PAMU) > > > > > > > > > > Is VFIO_IOMMU_PAMU_GET_MSI_BANK_COUNT per aperture (ie. each > > > > > vfio user has $COUNT regions at their disposal exclusively)? > > > > > > > > Number of msi-bank count is system wide and not per aperture, But will > > > > be > setting windows for banks in the device aperture. > > > > So say if we are direct assigning 2 pci device (both have different > > > > iommu > group, so 2 aperture in iommu) to VM. > > > > Now qemu can make only one call to know how many msi-banks are there but > it must set sub-windows for all banks for both pci device in its respective > aperture. > > > > > > I'm still confused. What I want to make sure of is that the banks > > > are independent per aperture. For instance, if we have two separate > > > userspace processes operating independently and they both chose to > > > use msi bank zero for their device, that's bank zero within each > > > aperture and doesn't interfere. Or another way to ask is can a > > > malicious user interfere with other users by using the wrong bank. > > > Thanks, > > > > They can interfere. Want to be sure of how they can interfere? >> With this hardware, the only way to prevent that > > is to make sure that a bank is not shared by multiple protection contexts. > > For some of our users, though, I believe preventing this is less > > important than the performance benefit. So should we let this patch series in without protection? > > I think we need some sort of ownership model around the msi banks then. > Otherwise there's nothing preventing another userspace from attempting an MSI > based attack on other users, or perhaps even on the host. VFIO can't allow > that. Thanks, We have very few (3 MSI bank on most of chips), so we can not assign one to each userspace. What we can do is host and userspace does not share a MSI bank while userspace will share a MSI bank. Thanks -Bharat > > Alex > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
> -Original Message- > From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Thursday, November 21, 2013 12:17 AM > To: Bhushan Bharat-R65777 > Cc: j...@8bytes.org; bhelg...@google.com; ag...@suse.de; Wood Scott-B07421; > Yoder Stuart-B08248; iommu@lists.linux-foundation.org; linux- > p...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; linux- > ker...@vger.kernel.org; Bhushan Bharat-R65777 > Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU) > > On Tue, 2013-11-19 at 10:47 +0530, Bharat Bhushan wrote: > > From: Bharat Bhushan > > > > PAMU (FSL IOMMU) has a concept of primary window and subwindows. > > Primary window corresponds to the complete guest iova address space > > (including MSI space), with respect to IOMMU_API this is termed as > > geometry. IOVA Base of subwindow is determined from the number of > > subwindows (configurable using iommu API). > > MSI I/O page must be within the geometry and maximum supported > > subwindows, so MSI IO-page is setup just after guest memory iova space. > > > > So patch 1/9-4/9(inclusive) are for defining the interface to get: > > - Number of MSI regions (which is number of MSI banks for powerpc) > > - MSI-region address range: Physical page which have the > > address/addresses used for generating MSI interrupt > > and size of the page. > > > > Patch 5/9-7/9(inclusive) is defining the interface of setting up MSI > > iova-base for a msi region(bank) for a device. so that when > > msi-message will be composed then this configured iova will be used. > > Earlier we were using iommu interface for getting the configured iova > > which was not currect and Alex Williamson suggeested this type of interface. > > > > patch 8/9 moves some common functions in a separate file so that these > > can be used by FSL_PAMU implementation (next patch uses this). > > These will be used later for iommu-none implementation. I believe we > > can do more of this but will take step by step. > > > > Finally last patch actually adds the support for FSL-PAMU :) > > Patches 1-3: msi_get_region needs to return an error an error (probably > -EINVAL) if called on a path where there's no backend implementation. > Otherwise the caller doesn't know that the data in the region pointer isn't > valid. will correct. > > Patches 5&6: same as above for msi_set_iova, return an error if no backend > implementation. Ok > > Patch 7: Why does fsl_msi_del_iova_device bother to return anything if it's > always zero? Return -ENODEV when not found? Will make -ENODEV. > > Patch 9: > > vfio_handle_get_attr() passes random kernel data back to userspace in the > event > of iommu_domain_get_attr() error. Will correct. > > vfio_handle_set_attr(): I don't see any data validation happening, is > iommu_domain_set_attr() really that safe? We do not need any data validation here and iommu driver does whatever needed. So yes, iommu_domain_set_attr() is safe. > > For both of those, drop the pr_err on unknown attribute, it's sufficient to > return error. ok > > Is VFIO_IOMMU_PAMU_GET_MSI_BANK_COUNT per aperture (ie. each vfio user has > $COUNT regions at their disposal exclusively)? Number of msi-bank count is system wide and not per aperture, But will be setting windows for banks in the device aperture. So say if we are direct assigning 2 pci device (both have different iommu group, so 2 aperture in iommu) to VM. Now qemu can make only one call to know how many msi-banks are there but it must set sub-windows for all banks for both pci device in its respective aperture. Thanks -Bharat > Thanks, > > Alex > > > v1->v2 > > - Added interface for setting msi iova for a msi region for a device. > >Earlier I added iommu interface for same but as per comment that is > >removed and now created a direct interface between vfio and msi. > > - Incorporated review comments (details is in individual patch) > > > > Bharat Bhushan (9): > > pci:msi: add weak function for returning msi region info > > pci: msi: expose msi region information functions > > powerpc: pci: Add arch specific msi region interface > > powerpc: msi: Extend the msi region interface to get info from > > fsl_msi > > pci/msi: interface to set an iova for a msi region > > powerpc: pci: Extend msi iova page setup to arch specific > > pci: msi: Extend msi iova setting interface to powerpc arch > > vfio: moving some functions in common file > > vfio pci: Add vfio iommu implementation for FSL_PAMU > > > > arch/p
[PATCH 9/9 v2] vfio pci: Add vfio iommu implementation for FSL_PAMU
This patch adds vfio iommu support for Freescale IOMMU (PAMU -
Peripheral Access Management Unit).
The Freescale PAMU is an aperture-based IOMMU with the following
characteristics. Each device has an entry in a table in memory
describing the iova->phys mapping. The mapping has:
-an overall aperture that is power of 2 sized, and has a start iova that
is naturally aligned
-has 1 or more windows within the aperture
-number of windows must be power of 2, max is 256
-size of each window is determined by aperture size / # of windows
-iova of each window is determined by aperture start iova / # of windows
-the mapped region in each window can be different than
the window size...mapping must power of 2
-physical address of the mapping must be naturally aligned
with the mapping size
Some of the code is derived from TYPE1 iommu (driver/vfio/vfio_iommu_type1.c).
Signed-off-by: Bharat Bhushan
---
v1->v2
- Use lock around msi-dma list
- check for overlap between dma and msi-dma pages
- Some code cleanup as per various comments
drivers/vfio/Kconfig |6 +
drivers/vfio/Makefile |1 +
drivers/vfio/vfio_iommu_fsl_pamu.c | 1003
include/uapi/linux/vfio.h | 100
4 files changed, 1110 insertions(+), 0 deletions(-)
create mode 100644 drivers/vfio/vfio_iommu_fsl_pamu.c
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 26b3d9d..7d1da26 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -8,11 +8,17 @@ config VFIO_IOMMU_SPAPR_TCE
depends on VFIO && SPAPR_TCE_IOMMU
default n
+config VFIO_IOMMU_FSL_PAMU
+ tristate
+ depends on VFIO
+ default n
+
menuconfig VFIO
tristate "VFIO Non-Privileged userspace driver framework"
depends on IOMMU_API
select VFIO_IOMMU_TYPE1 if X86
select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES)
+ select VFIO_IOMMU_FSL_PAMU if FSL_PAMU
help
VFIO provides a framework for secure userspace device drivers.
See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index c5792ec..7461350 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_VFIO) += vfio.o
obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_common.o vfio_iommu_type1.o
obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_common.o
vfio_iommu_spapr_tce.o
+obj-$(CONFIG_VFIO_IOMMU_FSL_PAMU) += vfio_iommu_common.o vfio_iommu_fsl_pamu.o
obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_iommu_fsl_pamu.c
b/drivers/vfio/vfio_iommu_fsl_pamu.c
new file mode 100644
index 000..66efc84
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_fsl_pamu.c
@@ -0,0 +1,1003 @@
+/*
+ * VFIO: IOMMU DMA mapping support for FSL PAMU IOMMU
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * Author: Bharat Bhushan
+ *
+ * This file is derived from driver/vfio/vfio_iommu_type1.c
+ *
+ * The Freescale PAMU is an aperture-based IOMMU with the following
+ * characteristics. Each device has an entry in a table in memory
+ * describing the iova->phys mapping. The mapping has:
+ * -an overall aperture that is power of 2 sized, and has a start iova that
+ * is naturally aligned
+ * -has 1 or more windows within the aperture
+ * -number of windows must be power of 2, max is 256
+ * -size of each window is determined by aperture size / # of windows
+ * -iova of each window is determined by aperture start iova / # of windows
+ * -the mapped region in each window can be different than
+ * the window size...mapping must power of 2
+ * -physical address of the mapping must be naturally aligned
+ * with the mapping size
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include /* pci_bus_type */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "vfio_iommu_common.h"
+
+#define DRIVER_VERSION "0.1"
+#define DRIVER_AUTHOR "Bharat Bhushan "
+#define DRIVER_DESC "FSL PAMU IOMMU driver for VFIO"
+
+struct vfio_iommu {
+ struct iommu_domain *domain;
+ struct mutexlock;
[PATCH 8/9 v2] vfio: moving some functions in common file
Some function defined in vfio_iommu_type1.c are generic (not specific
or type1 iommu) and we want to use these for FSL IOMMU (PAMU) and
going forward in iommu-none driver.
So I have created a new file naming vfio_iommu_common.c and moved some
of generic functions into this file.
I Agree (with Alex Williamson and myself :-)) that some more functions
can be moved to this new common file (with some changes in type1/fsl_pamu
and others). But in this patch i avoided doing these changes and
just moved functions which are straight forward and allow me to
get fsl-powerpc vfio framework in place.
Signed-off-by: Bharat Bhushan
---
v1->v2
- removed un-necessary header file inclusion
- mark static function which are internal to *common.c
drivers/vfio/Makefile|4 +-
drivers/vfio/vfio_iommu_common.c | 227 ++
drivers/vfio/vfio_iommu_common.h | 27 +
drivers/vfio/vfio_iommu_type1.c | 206 +--
4 files changed, 257 insertions(+), 207 deletions(-)
create mode 100644 drivers/vfio/vfio_iommu_common.c
create mode 100644 drivers/vfio/vfio_iommu_common.h
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 72bfabc..c5792ec 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_VFIO) += vfio.o
-obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
-obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
+obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_common.o vfio_iommu_type1.o
+obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_common.o
vfio_iommu_spapr_tce.o
obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_iommu_common.c b/drivers/vfio/vfio_iommu_common.c
new file mode 100644
index 000..08eea71
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_common.c
@@ -0,0 +1,227 @@
+/*
+ * VFIO: Common code for vfio IOMMU support
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All rights reserved.
+ * Author: Alex Williamson
+ * Author: Bharat Bhushan
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio:
+ * Copyright 2010 Cisco Systems, Inc. All rights reserved.
+ * Author: Tom Lyon, p...@cisco.com
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+static bool disable_hugepages;
+module_param_named(disable_hugepages,
+ disable_hugepages, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_hugepages,
+"Disable VFIO IOMMU support for IOMMU hugepages.");
+
+struct vwork {
+ struct mm_struct*mm;
+ longnpage;
+ struct work_struct work;
+};
+
+/* delayed decrement/increment for locked_vm */
+static void vfio_lock_acct_bg(struct work_struct *work)
+{
+ struct vwork *vwork = container_of(work, struct vwork, work);
+ struct mm_struct *mm;
+
+ mm = vwork->mm;
+ down_write(&mm->mmap_sem);
+ mm->locked_vm += vwork->npage;
+ up_write(&mm->mmap_sem);
+ mmput(mm);
+ kfree(vwork);
+}
+
+void vfio_lock_acct(long npage)
+{
+ struct vwork *vwork;
+ struct mm_struct *mm;
+
+ if (!current->mm || !npage)
+ return; /* process exited or nothing to do */
+
+ if (down_write_trylock(¤t->mm->mmap_sem)) {
+ current->mm->locked_vm += npage;
+ up_write(¤t->mm->mmap_sem);
+ return;
+ }
+
+ /*
+* Couldn't get mmap_sem lock, so must setup to update
+* mm->locked_vm later. If locked_vm were atomic, we
+* wouldn't need this silliness
+*/
+ vwork = kmalloc(sizeof(struct vwork), GFP_KERNEL);
+ if (!vwork)
+ return;
+ mm = get_task_mm(current);
+ if (!mm) {
+ kfree(vwork);
+ return;
+ }
+ INIT_WORK(&vwork->work, vfio_lock_acct_bg);
+ vwork->mm = mm;
+ vwork->npage = npage;
+ schedule_work(&vwork->work);
+}
+
+/*
+ * Some mappings aren't backed by a struct page, for example an mmap'd
+ * MMIO range for our own or another device. These use a different
+ * pfn conversion and shouldn't be tracked as locked pages.
+ */
+static bool is_invalid_reserved_pfn(unsigned long pfn)
+{
+ if (pfn_valid(pfn)) {
+ bool reserved;
+ struct page *tail = pfn_to_page(pfn);
+ struct page *head = compound_trans_head(tail);
+ reserved = !!(PageReserved(head));
+ if (head != tail) {
+ /*
+* "head" is not a dangling pointer
+* (compound_trans_head takes care of that)
+* but the hugepage may have been split
+
[PATCH 7/9 v2] pci: msi: Extend msi iova setting interface to powerpc arch
Now we Keep track of devices which have msi page mapping to specific
iova page for all msi bank. When composing MSI address and data then
this list will be traversed. If device found in the list then use
configured iova page otherwise iova page will be taken as before.
Signed-off-by: Bharat Bhushan
---
v2
- new patch
arch/powerpc/sysdev/fsl_msi.c | 90 +
arch/powerpc/sysdev/fsl_msi.h | 16 ++-
2 files changed, 104 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index eeebbf0..52d2beb 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -137,6 +137,75 @@ static int fsl_msi_get_region(int region_num, struct
msi_region *region)
return -ENODEV;
}
+/* Add device to the list which have iova page mapping */
+static int fsl_msi_add_iova_device(struct fsl_msi *msi_data,
+ struct pci_dev *pdev, dma_addr_t iova)
+{
+ struct fsl_msi_device *device;
+
+ mutex_lock(&msi_data->lock);
+ list_for_each_entry(device, &msi_data->device_list, list) {
+ /* If mapping already exits then update with new page mapping */
+ if (device->dev == pdev) {
+ device->iova = iova;
+ mutex_unlock(&msi_data->lock);
+ return 0;
+ }
+ }
+
+ device = kzalloc(sizeof(struct fsl_msi_device), GFP_KERNEL);
+ if (!device) {
+ pr_err("%s: Memory allocation failed\n", __func__);
+ mutex_unlock(&msi_data->lock);
+ return -ENOMEM;
+ }
+
+ device->dev = pdev;
+ device->iova = iova;
+ list_add_tail(&device->list, &msi_data->device_list);
+ mutex_unlock(&msi_data->lock);
+ return 0;
+}
+
+/* Remove device to the list which have iova page mapping */
+static int fsl_msi_del_iova_device(struct fsl_msi *msi_data,
+ struct pci_dev *pdev)
+{
+ struct fsl_msi_device *device;
+
+ mutex_lock(&msi_data->lock);
+ list_for_each_entry(device, &msi_data->device_list, list) {
+ if (device->dev == pdev) {
+ list_del(&device->list);
+ kfree(device);
+ break;
+ }
+ }
+ mutex_unlock(&msi_data->lock);
+ return 0;
+}
+
+/* set/clear device iova mapping for the requested msi region */
+static int fsl_msi_set_iova(struct pci_dev *pdev, int region_num,
+ dma_addr_t iova, bool set)
+{
+ struct fsl_msi *msi_data;
+ int ret = -EINVAL;
+
+ list_for_each_entry(msi_data, &msi_head, list) {
+ if (msi_data->bank_index != region_num)
+ continue;
+
+ if (set)
+ ret = fsl_msi_add_iova_device(msi_data, pdev, iova);
+ else
+ ret = fsl_msi_del_iova_device(msi_data, pdev);
+
+ break;
+ }
+ return ret;
+}
+
static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type)
{
if (type == PCI_CAP_ID_MSIX)
@@ -167,6 +236,7 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int
hwirq,
struct msi_msg *msg,
struct fsl_msi *fsl_msi_data)
{
+ struct fsl_msi_device *device;
struct fsl_msi *msi_data = fsl_msi_data;
struct pci_controller *hose = pci_bus_to_host(pdev->bus);
u64 address; /* Physical address of the MSIIR */
@@ -181,6 +251,15 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int
hwirq,
address = fsl_pci_immrbar_base(hose) +
(msi_data->msiir & 0xf);
+ mutex_lock(&msi_data->lock);
+ list_for_each_entry(device, &msi_data->device_list, list) {
+ if (device->dev == pdev) {
+ address = device->iova | (msi_data->msiir & 0xfff);
+ break;
+ }
+ }
+ mutex_unlock(&msi_data->lock);
+
msg->address_lo = lower_32_bits(address);
msg->address_hi = upper_32_bits(address);
@@ -356,6 +435,7 @@ static int fsl_of_msi_remove(struct platform_device *ofdev)
struct fsl_msi *msi = platform_get_drvdata(ofdev);
int virq, i;
struct fsl_msi_cascade_data *cascade_data;
+ struct fsl_msi_device *device;
if (msi->list.prev != NULL)
list_del(&msi->list);
@@ -371,6 +451,13 @@ static int fsl_of_msi_remove(struct platform_device *ofdev)
msi_bitmap_free(&msi->bitmap);
if ((msi->feature & FSL_PIC_IP_MASK) != FSL_PIC_IP_VMPIC)
iounmap
[PATCH 6/9 v2] powerpc: pci: Extend msi iova page setup to arch specific
This patch extend the interface to arch specific code for setting
msi iova address for a msi page. Machine specific code is not yet
implemented.
Signed-off-by: Bharat Bhushan
---
v2
- new patch
arch/powerpc/include/asm/machdep.h |2 ++
arch/powerpc/kernel/msi.c | 10 ++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/machdep.h
b/arch/powerpc/include/asm/machdep.h
index 8d1b787..e87b806 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -132,6 +132,8 @@ struct machdep_calls {
/* Returns the requested region's address and size */
int (*msi_get_region)(int region_num,
struct msi_region *region);
+ int (*msi_set_iova)(struct pci_dev *pdev, int region_num,
+ dma_addr_t iova, bool set);
#endif
void(*restart)(char *cmd);
diff --git a/arch/powerpc/kernel/msi.c b/arch/powerpc/kernel/msi.c
index 1a67787..e2bd555 100644
--- a/arch/powerpc/kernel/msi.c
+++ b/arch/powerpc/kernel/msi.c
@@ -13,6 +13,16 @@
#include
+int arch_msi_set_iova(struct pci_dev *pdev, int region_num,
+ dma_addr_t iova, bool set)
+{
+ if (ppc_md.msi_set_iova) {
+ pr_debug("msi: Using platform get_region_count routine.\n");
+ return ppc_md.msi_set_iova(pdev, region_num, iova, set);
+ }
+ return 0;
+}
+
int arch_msi_get_region_count(void)
{
if (ppc_md.msi_get_region_count) {
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/9 v2] powerpc: pci: Add arch specific msi region interface
This patch adds the interface to get the msi region information from arch
specific code. The machine spicific code is not yet defined.
Signed-off-by: Bharat Bhushan
---
v1->v2
- None
arch/powerpc/include/asm/machdep.h |8
arch/powerpc/kernel/msi.c | 18 ++
2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/machdep.h
b/arch/powerpc/include/asm/machdep.h
index 8b48090..8d1b787 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -30,6 +30,7 @@ struct file;
struct pci_controller;
struct kimage;
struct pci_host_bridge;
+struct msi_region;
struct machdep_calls {
char*name;
@@ -124,6 +125,13 @@ struct machdep_calls {
int (*setup_msi_irqs)(struct pci_dev *dev,
int nvec, int type);
void(*teardown_msi_irqs)(struct pci_dev *dev);
+
+ /* Returns the number of MSI regions (banks) */
+ int (*msi_get_region_count)(void);
+
+ /* Returns the requested region's address and size */
+ int (*msi_get_region)(int region_num,
+ struct msi_region *region);
#endif
void(*restart)(char *cmd);
diff --git a/arch/powerpc/kernel/msi.c b/arch/powerpc/kernel/msi.c
index 8bbc12d..1a67787 100644
--- a/arch/powerpc/kernel/msi.c
+++ b/arch/powerpc/kernel/msi.c
@@ -13,6 +13,24 @@
#include
+int arch_msi_get_region_count(void)
+{
+ if (ppc_md.msi_get_region_count) {
+ pr_debug("msi: Using platform get_region_count routine.\n");
+ return ppc_md.msi_get_region_count();
+ }
+ return 0;
+}
+
+int arch_msi_get_region(int region_num, struct msi_region *region)
+{
+ if (ppc_md.msi_get_region) {
+ pr_debug("msi: Using platform get_region routine.\n");
+ return ppc_md.msi_get_region(region_num, region);
+ }
+ return 0;
+}
+
int arch_msi_check_device(struct pci_dev* dev, int nvec, int type)
{
if (!ppc_md.setup_msi_irqs || !ppc_md.teardown_msi_irqs) {
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
From: Bharat Bhushan PAMU (FSL IOMMU) has a concept of primary window and subwindows. Primary window corresponds to the complete guest iova address space (including MSI space), with respect to IOMMU_API this is termed as geometry. IOVA Base of subwindow is determined from the number of subwindows (configurable using iommu API). MSI I/O page must be within the geometry and maximum supported subwindows, so MSI IO-page is setup just after guest memory iova space. So patch 1/9-4/9(inclusive) are for defining the interface to get: - Number of MSI regions (which is number of MSI banks for powerpc) - MSI-region address range: Physical page which have the address/addresses used for generating MSI interrupt and size of the page. Patch 5/9-7/9(inclusive) is defining the interface of setting up MSI iova-base for a msi region(bank) for a device. so that when msi-message will be composed then this configured iova will be used. Earlier we were using iommu interface for getting the configured iova which was not currect and Alex Williamson suggeested this type of interface. patch 8/9 moves some common functions in a separate file so that these can be used by FSL_PAMU implementation (next patch uses this). These will be used later for iommu-none implementation. I believe we can do more of this but will take step by step. Finally last patch actually adds the support for FSL-PAMU :) v1->v2 - Added interface for setting msi iova for a msi region for a device. Earlier I added iommu interface for same but as per comment that is removed and now created a direct interface between vfio and msi. - Incorporated review comments (details is in individual patch) Bharat Bhushan (9): pci:msi: add weak function for returning msi region info pci: msi: expose msi region information functions powerpc: pci: Add arch specific msi region interface powerpc: msi: Extend the msi region interface to get info from fsl_msi pci/msi: interface to set an iova for a msi region powerpc: pci: Extend msi iova page setup to arch specific pci: msi: Extend msi iova setting interface to powerpc arch vfio: moving some functions in common file vfio pci: Add vfio iommu implementation for FSL_PAMU arch/powerpc/include/asm/machdep.h | 10 + arch/powerpc/kernel/msi.c | 28 + arch/powerpc/sysdev/fsl_msi.c | 132 +- arch/powerpc/sysdev/fsl_msi.h | 25 +- drivers/pci/msi.c | 35 ++ drivers/vfio/Kconfig |6 + drivers/vfio/Makefile |5 +- drivers/vfio/vfio_iommu_common.c | 227 drivers/vfio/vfio_iommu_common.h | 27 + drivers/vfio/vfio_iommu_fsl_pamu.c | 1003 drivers/vfio/vfio_iommu_type1.c| 206 + include/linux/msi.h| 14 + include/linux/pci.h| 21 + include/uapi/linux/vfio.h | 100 14 files changed, 1623 insertions(+), 216 deletions(-) create mode 100644 drivers/vfio/vfio_iommu_common.c create mode 100644 drivers/vfio/vfio_iommu_common.h create mode 100644 drivers/vfio/vfio_iommu_fsl_pamu.c ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/9 v2] pci/msi: interface to set an iova for a msi region
This patch defines an interface by which a msi page
can be mapped to a specific iova page.
This is a requirement in aperture type of IOMMUs (like Freescale PAMU),
where we map msi iova page just after guest memory iova address.
Signed-off-by: Bharat Bhushan
---
v2
- new patch
drivers/pci/msi.c | 13 +
include/linux/pci.h |8
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 2643a29..040609f 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -77,6 +77,19 @@ int __weak arch_msi_get_region(int region_num, struct
msi_region *region)
return 0;
}
+int __weak arch_msi_set_iova(struct pci_dev *pdev, int region_num,
+dma_addr_t iova, bool set)
+{
+ return 0;
+}
+
+int msi_set_iova(struct pci_dev *pdev, int region_num,
+dma_addr_t iova, bool set)
+{
+ return arch_msi_set_iova(pdev, region_num, iova, set);
+}
+EXPORT_SYMBOL(msi_set_iova);
+
int msi_get_region_count(void)
{
return arch_msi_get_region_count();
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c587034..c6d3e58 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1195,6 +1195,12 @@ static inline int msi_get_region(int region_num, struct
msi_region *region)
{
return 0;
}
+
+static inline int msi_set_iova(struct pci_dev *pdev, int region_num,
+ dma_addr_t iova, bool set)
+{
+ return 0;
+}
#else
int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec);
int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
@@ -1209,6 +1215,8 @@ void pci_restore_msi_state(struct pci_dev *dev);
int pci_msi_enabled(void);
int msi_get_region_count(void);
int msi_get_region(int region_num, struct msi_region *region);
+int msi_set_iova(struct pci_dev *pdev, int region_num,
+dma_addr_t iova, bool set);
#endif
#ifdef CONFIG_PCIEPORTBUS
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/9 v2] powerpc: msi: Extend the msi region interface to get info from fsl_msi
The FSL MSI will provide the interface to get:
- Number of MSI regions (which is number of MSI banks for powerpc)
- Get the region address range: Physical page which have the
address/addresses used for generating MSI interrupt
and size of the page.
These are required to create IOMMU (Freescale PAMU) mapping for
devices which are directly assigned using VFIO.
Signed-off-by: Bharat Bhushan
---
v1->v2
- Atomic increment of bank index for parallel probe of msi node
arch/powerpc/sysdev/fsl_msi.c | 42 +++-
arch/powerpc/sysdev/fsl_msi.h | 11 -
2 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 77efbae..eeebbf0 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -109,6 +109,34 @@ static int fsl_msi_init_allocator(struct fsl_msi *msi_data)
return 0;
}
+static int fsl_msi_get_region_count(void)
+{
+ int count = 0;
+ struct fsl_msi *msi_data;
+
+ list_for_each_entry(msi_data, &msi_head, list)
+ count++;
+
+ return count;
+}
+
+static int fsl_msi_get_region(int region_num, struct msi_region *region)
+{
+ struct fsl_msi *msi_data;
+
+ list_for_each_entry(msi_data, &msi_head, list) {
+ if (msi_data->bank_index == region_num) {
+ region->region_num = msi_data->bank_index;
+ /* Setting PAGE_SIZE as MSIIR is a 4 byte register */
+ region->size = PAGE_SIZE;
+ region->addr = msi_data->msiir & ~(region->size - 1);
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type)
{
if (type == PCI_CAP_ID_MSIX)
@@ -150,7 +178,8 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int
hwirq,
if (reg && (len == sizeof(u64)))
address = be64_to_cpup(reg);
else
- address = fsl_pci_immrbar_base(hose) + msi_data->msiir_offset;
+ address = fsl_pci_immrbar_base(hose) +
+ (msi_data->msiir & 0xf);
msg->address_lo = lower_32_bits(address);
msg->address_hi = upper_32_bits(address);
@@ -393,6 +422,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
const struct fsl_msi_feature *features;
int len;
u32 offset;
+ static atomic_t bank_index = ATOMIC_INIT(-1);
match = of_match_device(fsl_of_msi_ids, &dev->dev);
if (!match)
@@ -436,18 +466,15 @@ static int fsl_of_msi_probe(struct platform_device *dev)
dev->dev.of_node->full_name);
goto error_out;
}
- msi->msiir_offset =
- features->msiir_offset + (res.start & 0xf);
/*
* First read the MSIIR/MSIIR1 offset from dts
* On failure use the hardcode MSIIR offset
*/
if (of_address_to_resource(dev->dev.of_node, 1, &msiir))
- msi->msiir_offset = features->msiir_offset +
- (res.start & MSIIR_OFFSET_MASK);
+ msi->msiir = res.start + features->msiir_offset;
else
- msi->msiir_offset = msiir.start & MSIIR_OFFSET_MASK;
+ msi->msiir = msiir.start;
}
msi->feature = features->fsl_pic_ip;
@@ -521,6 +548,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
}
}
+ msi->bank_index = atomic_inc_return(&bank_index);
list_add_tail(&msi->list, &msi_head);
/* The multiple setting ppc_md.setup_msi_irqs will not harm things */
@@ -528,6 +556,8 @@ static int fsl_of_msi_probe(struct platform_device *dev)
ppc_md.setup_msi_irqs = fsl_setup_msi_irqs;
ppc_md.teardown_msi_irqs = fsl_teardown_msi_irqs;
ppc_md.msi_check_device = fsl_msi_check_device;
+ ppc_md.msi_get_region_count = fsl_msi_get_region_count;
+ ppc_md.msi_get_region = fsl_msi_get_region;
} else if (ppc_md.setup_msi_irqs != fsl_setup_msi_irqs) {
dev_err(&dev->dev, "Different MSI driver already installed!\n");
err = -ENODEV;
diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
index df9aa9f..a2cc5a2 100644
--- a/arch/powerpc/sysdev/fsl_msi.h
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -31,14 +31,21 @@ struct fsl_msi {
struct irq_domain *irqhost;
unsigned long cascade_irq;
-
- u32 msiir_offset; /* Offset of MSIIR, relative to start of CCS
[PATCH 1/9 v2] pci:msi: add weak function for returning msi region info
In Aperture type of IOMMU (like FSL PAMU), VFIO-iommu system need to know
the MSI region to map its window in h/w. This patch just defines the
required weak functions only and will be used by followup patches.
Signed-off-by: Bharat Bhushan
---
v1->v2
- Added description on "struct msi_region"
drivers/pci/msi.c | 22 ++
include/linux/msi.h | 14 ++
2 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d5f90d6..2643a29 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -67,6 +67,28 @@ int __weak arch_msi_check_device(struct pci_dev *dev, int
nvec, int type)
return chip->check_device(chip, dev, nvec, type);
}
+int __weak arch_msi_get_region_count(void)
+{
+ return 0;
+}
+
+int __weak arch_msi_get_region(int region_num, struct msi_region *region)
+{
+ return 0;
+}
+
+int msi_get_region_count(void)
+{
+ return arch_msi_get_region_count();
+}
+EXPORT_SYMBOL(msi_get_region_count);
+
+int msi_get_region(int region_num, struct msi_region *region)
+{
+ return arch_msi_get_region(region_num, region);
+}
+EXPORT_SYMBOL(msi_get_region);
+
int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
{
struct msi_desc *entry;
diff --git a/include/linux/msi.h b/include/linux/msi.h
index b17ead8..ade1480 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -51,6 +51,18 @@ struct msi_desc {
};
/*
+ * This structure is used to get
+ * - physical address
+ * - size
+ * of a msi region
+ */
+struct msi_region {
+ int region_num; /* MSI region number */
+ dma_addr_t addr; /* Address of MSI region */
+ size_t size; /* Size of MSI region */
+};
+
+/*
* The arch hooks to setup up msi irqs. Those functions are
* implemented as weak symbols so that they /can/ be overriden by
* architecture specific code if needed.
@@ -64,6 +76,8 @@ void arch_restore_msi_irqs(struct pci_dev *dev, int irq);
void default_teardown_msi_irqs(struct pci_dev *dev);
void default_restore_msi_irqs(struct pci_dev *dev, int irq);
+int arch_msi_get_region_count(void);
+int arch_msi_get_region(int region_num, struct msi_region *region);
struct msi_chip {
struct module *owner;
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/9 v2] pci: msi: expose msi region information functions
So by now we have defined all the interfaces for getting the msi region,
this patch expose the interface to linux subsystem. These will be used by
vfio subsystem for setting up iommu for MSI interrupt of direct assignment
devices.
Signed-off-by: Bharat Bhushan
---
v1->v2
- None
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index da172f9..c587034 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1142,6 +1142,7 @@ struct msix_entry {
u16 entry; /* driver uses to specify entry, OS writes */
};
+struct msi_region;
#ifndef CONFIG_PCI_MSI
static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
@@ -1184,6 +1185,16 @@ static inline int pci_msi_enabled(void)
{
return 0;
}
+
+static inline int msi_get_region_count(void)
+{
+ return 0;
+}
+
+static inline int msi_get_region(int region_num, struct msi_region *region)
+{
+ return 0;
+}
#else
int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec);
int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
@@ -1196,6 +1207,8 @@ void pci_disable_msix(struct pci_dev *dev);
void msi_remove_pci_irq_vectors(struct pci_dev *dev);
void pci_restore_msi_state(struct pci_dev *dev);
int pci_msi_enabled(void);
+int msi_get_region_count(void);
+int msi_get_region(int region_num, struct msi_region *region);
#endif
#ifdef CONFIG_PCIEPORTBUS
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 7/7] vfio pci: Add vfio iommu implementation for FSL_PAMU
This patch adds vfio iommu support for Freescale IOMMU
(PAMU - Peripheral Access Management Unit).
The Freescale PAMU is an aperture-based IOMMU with the following
characteristics. Each device has an entry in a table in memory
describing the iova->phys mapping. The mapping has:
-an overall aperture that is power of 2 sized, and has a start iova that
is naturally aligned
-has 1 or more windows within the aperture
-number of windows must be power of 2, max is 256
-size of each window is determined by aperture size / # of windows
-iova of each window is determined by aperture start iova / # of windows
-the mapped region in each window can be different than
the window size...mapping must power of 2
-physical address of the mapping must be naturally aligned
with the mapping size
Some of the code is derived from TYPE1 iommu (driver/vfio/vfio_iommu_type1.c).
Signed-off-by: Bharat Bhushan
---
drivers/vfio/Kconfig |6 +
drivers/vfio/Makefile |1 +
drivers/vfio/vfio_iommu_fsl_pamu.c | 952
include/uapi/linux/vfio.h | 100
4 files changed, 1059 insertions(+), 0 deletions(-)
create mode 100644 drivers/vfio/vfio_iommu_fsl_pamu.c
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 26b3d9d..7d1da26 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -8,11 +8,17 @@ config VFIO_IOMMU_SPAPR_TCE
depends on VFIO && SPAPR_TCE_IOMMU
default n
+config VFIO_IOMMU_FSL_PAMU
+ tristate
+ depends on VFIO
+ default n
+
menuconfig VFIO
tristate "VFIO Non-Privileged userspace driver framework"
depends on IOMMU_API
select VFIO_IOMMU_TYPE1 if X86
select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES)
+ select VFIO_IOMMU_FSL_PAMU if FSL_PAMU
help
VFIO provides a framework for secure userspace device drivers.
See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index c5792ec..7461350 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_VFIO) += vfio.o
obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_common.o vfio_iommu_type1.o
obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_common.o
vfio_iommu_spapr_tce.o
+obj-$(CONFIG_VFIO_IOMMU_FSL_PAMU) += vfio_iommu_common.o vfio_iommu_fsl_pamu.o
obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_iommu_fsl_pamu.c
b/drivers/vfio/vfio_iommu_fsl_pamu.c
new file mode 100644
index 000..b29365f
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_fsl_pamu.c
@@ -0,0 +1,952 @@
+/*
+ * VFIO: IOMMU DMA mapping support for FSL PAMU IOMMU
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * Author: Bharat Bhushan
+ *
+ * This file is derived from driver/vfio/vfio_iommu_type1.c
+ *
+ * The Freescale PAMU is an aperture-based IOMMU with the following
+ * characteristics. Each device has an entry in a table in memory
+ * describing the iova->phys mapping. The mapping has:
+ * -an overall aperture that is power of 2 sized, and has a start iova that
+ * is naturally aligned
+ * -has 1 or more windows within the aperture
+ * -number of windows must be power of 2, max is 256
+ * -size of each window is determined by aperture size / # of windows
+ * -iova of each window is determined by aperture start iova / # of windows
+ * -the mapped region in each window can be different than
+ * the window size...mapping must power of 2
+ * -physical address of the mapping must be naturally aligned
+ * with the mapping size
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include /* pci_bus_type */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "vfio_iommu_common.h"
+
+#define DRIVER_VERSION "0.1"
+#define DRIVER_AUTHOR "Bharat Bhushan "
+#define DRIVER_DESC "FSL PAMU IOMMU driver for VFIO"
+
+struct vfio_iommu {
+ struct iommu_domain *domain;
+ struct mutexlock;
+ dma_addr_t aperture_start;
+ dma_addr_t aperture_end;
+ dma_addr_t
[PATCH 6/7] vfio: moving some functions in common file
Some function defined in vfio_iommu_type1.c were common and
we want to use these for FSL IOMMU (PAMU) and iommu-none driver.
So some of them are moved to vfio_iommu_common.c
I think we can do more of that but we will take this step by step.
Signed-off-by: Bharat Bhushan
---
drivers/vfio/Makefile|4 +-
drivers/vfio/vfio_iommu_common.c | 235 ++
drivers/vfio/vfio_iommu_common.h | 30 +
drivers/vfio/vfio_iommu_type1.c | 206 +-
4 files changed, 268 insertions(+), 207 deletions(-)
create mode 100644 drivers/vfio/vfio_iommu_common.c
create mode 100644 drivers/vfio/vfio_iommu_common.h
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 72bfabc..c5792ec 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_VFIO) += vfio.o
-obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
-obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
+obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_common.o vfio_iommu_type1.o
+obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_common.o
vfio_iommu_spapr_tce.o
obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_iommu_common.c b/drivers/vfio/vfio_iommu_common.c
new file mode 100644
index 000..8bdc0ea
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_common.c
@@ -0,0 +1,235 @@
+/*
+ * VFIO: Common code for vfio IOMMU support
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All rights reserved.
+ * Author: Alex Williamson
+ * Author: Bharat Bhushan
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio:
+ * Copyright 2010 Cisco Systems, Inc. All rights reserved.
+ * Author: Tom Lyon, p...@cisco.com
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include /* pci_bus_type */
+#include
+#include
+#include
+#include
+#include
+#include
+
+static bool disable_hugepages;
+module_param_named(disable_hugepages,
+ disable_hugepages, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_hugepages,
+"Disable VFIO IOMMU support for IOMMU hugepages.");
+
+struct vwork {
+ struct mm_struct*mm;
+ longnpage;
+ struct work_struct work;
+};
+
+/* delayed decrement/increment for locked_vm */
+void vfio_lock_acct_bg(struct work_struct *work)
+{
+ struct vwork *vwork = container_of(work, struct vwork, work);
+ struct mm_struct *mm;
+
+ mm = vwork->mm;
+ down_write(&mm->mmap_sem);
+ mm->locked_vm += vwork->npage;
+ up_write(&mm->mmap_sem);
+ mmput(mm);
+ kfree(vwork);
+}
+
+void vfio_lock_acct(long npage)
+{
+ struct vwork *vwork;
+ struct mm_struct *mm;
+
+ if (!current->mm || !npage)
+ return; /* process exited or nothing to do */
+
+ if (down_write_trylock(¤t->mm->mmap_sem)) {
+ current->mm->locked_vm += npage;
+ up_write(¤t->mm->mmap_sem);
+ return;
+ }
+
+ /*
+* Couldn't get mmap_sem lock, so must setup to update
+* mm->locked_vm later. If locked_vm were atomic, we
+* wouldn't need this silliness
+*/
+ vwork = kmalloc(sizeof(struct vwork), GFP_KERNEL);
+ if (!vwork)
+ return;
+ mm = get_task_mm(current);
+ if (!mm) {
+ kfree(vwork);
+ return;
+ }
+ INIT_WORK(&vwork->work, vfio_lock_acct_bg);
+ vwork->mm = mm;
+ vwork->npage = npage;
+ schedule_work(&vwork->work);
+}
+
+/*
+ * Some mappings aren't backed by a struct page, for example an mmap'd
+ * MMIO range for our own or another device. These use a different
+ * pfn conversion and shouldn't be tracked as locked pages.
+ */
+bool is_invalid_reserved_pfn(unsigned long pfn)
+{
+ if (pfn_valid(pfn)) {
+ bool reserved;
+ struct page *tail = pfn_to_page(pfn);
+ struct page *head = compound_trans_head(tail);
+ reserved = !!(PageReserved(head));
+ if (head != tail) {
+ /*
+* "head" is not a dangling pointer
+* (compound_trans_head takes care of that)
+* but the hugepage may have been split
+* from under us (and we may not hold a
+* reference count on the head page so it can
+* be reused before we run PageReferenced), so
+* we've to check PageTail before returning
+* what we just read.
+*/
+
[PATCH 2/7] iommu: add api to get iommu_domain of a device
This api return the iommu domain to which the device is attached.
The iommu_domain is required for making API calls related to iommu.
Follow up patches which use this API to know iommu maping.
Signed-off-by: Bharat Bhushan
---
drivers/iommu/iommu.c | 10 ++
include/linux/iommu.h |7 +++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index fbe9ca7..6ac5f50 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -696,6 +696,16 @@ void iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_detach_device);
+struct iommu_domain *iommu_get_dev_domain(struct device *dev)
+{
+ struct iommu_ops *ops = dev->bus->iommu_ops;
+
+ if (unlikely(ops == NULL || ops->get_dev_iommu_domain == NULL))
+ return NULL;
+
+ return ops->get_dev_iommu_domain(dev);
+}
+EXPORT_SYMBOL_GPL(iommu_get_dev_domain);
/*
* IOMMU groups are really the natrual working unit of the IOMMU, but
* the IOMMU API works on domains and devices. Bridge that gap by
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7ea319e..fa046bd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -127,6 +127,7 @@ struct iommu_ops {
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);
+ struct iommu_domain *(*get_dev_iommu_domain)(struct device *dev);
unsigned long pgsize_bitmap;
};
@@ -190,6 +191,7 @@ extern int iommu_domain_window_enable(struct iommu_domain
*domain, u32 wnd_nr,
phys_addr_t offset, u64 size,
int prot);
extern void iommu_domain_window_disable(struct iommu_domain *domain, u32
wnd_nr);
+extern struct iommu_domain *iommu_get_dev_domain(struct device *dev);
/**
* report_iommu_fault() - report about an IOMMU fault to the IOMMU framework
* @domain: the iommu domain where the fault has happened
@@ -284,6 +286,11 @@ static inline void iommu_domain_window_disable(struct
iommu_domain *domain,
{
}
+static inline struct iommu_domain *iommu_get_dev_domain(struct device *dev)
+{
+ return NULL;
+}
+
static inline phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
dma_addr_t iova)
{
return 0;
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/7] powerpc: Add interface to get msi region information
This patch adds interface to get following information
- Number of MSI regions (which is number of MSI banks for powerpc).
- Get the region address range: Physical page which have the
address/addresses used for generating MSI interrupt
and size of the page.
These are required to create IOMMU (Freescale PAMU) mapping for
devices which are directly assigned using VFIO.
Signed-off-by: Bharat Bhushan
---
arch/powerpc/include/asm/machdep.h |8 +++
arch/powerpc/include/asm/pci.h |2 +
arch/powerpc/kernel/msi.c | 18
arch/powerpc/sysdev/fsl_msi.c | 39 +--
arch/powerpc/sysdev/fsl_msi.h | 11 -
drivers/pci/msi.c | 26
include/linux/msi.h|8 +++
include/linux/pci.h| 13
8 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/machdep.h
b/arch/powerpc/include/asm/machdep.h
index 8b48090..8d1b787 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -30,6 +30,7 @@ struct file;
struct pci_controller;
struct kimage;
struct pci_host_bridge;
+struct msi_region;
struct machdep_calls {
char*name;
@@ -124,6 +125,13 @@ struct machdep_calls {
int (*setup_msi_irqs)(struct pci_dev *dev,
int nvec, int type);
void(*teardown_msi_irqs)(struct pci_dev *dev);
+
+ /* Returns the number of MSI regions (banks) */
+ int (*msi_get_region_count)(void);
+
+ /* Returns the requested region's address and size */
+ int (*msi_get_region)(int region_num,
+ struct msi_region *region);
#endif
void(*restart)(char *cmd);
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 6653f27..e575349 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -117,6 +117,8 @@ extern int pci_proc_domain(struct pci_bus *bus);
#define arch_setup_msi_irqs arch_setup_msi_irqs
#define arch_teardown_msi_irqs arch_teardown_msi_irqs
#define arch_msi_check_device arch_msi_check_device
+#define arch_msi_get_region_count arch_msi_get_region_count
+#define arch_msi_get_region arch_msi_get_region
struct vm_area_struct;
/* Map a range of PCI memory or I/O space for a device into user space */
diff --git a/arch/powerpc/kernel/msi.c b/arch/powerpc/kernel/msi.c
index 8bbc12d..1a67787 100644
--- a/arch/powerpc/kernel/msi.c
+++ b/arch/powerpc/kernel/msi.c
@@ -13,6 +13,24 @@
#include
+int arch_msi_get_region_count(void)
+{
+ if (ppc_md.msi_get_region_count) {
+ pr_debug("msi: Using platform get_region_count routine.\n");
+ return ppc_md.msi_get_region_count();
+ }
+ return 0;
+}
+
+int arch_msi_get_region(int region_num, struct msi_region *region)
+{
+ if (ppc_md.msi_get_region) {
+ pr_debug("msi: Using platform get_region routine.\n");
+ return ppc_md.msi_get_region(region_num, region);
+ }
+ return 0;
+}
+
int arch_msi_check_device(struct pci_dev* dev, int nvec, int type)
{
if (!ppc_md.setup_msi_irqs || !ppc_md.teardown_msi_irqs) {
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index ab02db3..ed045cb 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -96,6 +96,34 @@ static int fsl_msi_init_allocator(struct fsl_msi *msi_data)
return 0;
}
+static int fsl_msi_get_region_count(void)
+{
+ int count = 0;
+ struct fsl_msi *msi_data;
+
+ list_for_each_entry(msi_data, &msi_head, list)
+ count++;
+
+ return count;
+}
+
+static int fsl_msi_get_region(int region_num, struct msi_region *region)
+{
+ struct fsl_msi *msi_data;
+
+ list_for_each_entry(msi_data, &msi_head, list) {
+ if (msi_data->bank_index == region_num) {
+ region->region_num = msi_data->bank_index;
+ /* Setting PAGE_SIZE as MSIIR is a 4 byte register */
+ region->size = PAGE_SIZE;
+ region->addr = msi_data->msiir & ~(region->size - 1);
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type)
{
if (type == PCI_CAP_ID_MSIX)
@@ -137,7 +165,8 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int
hwirq,
if (reg && (len == sizeof(u64)))
address = be64_to_cpup(reg);
else
- address = fsl_pci_immrbar_base(hose) + msi_data->msiir_offset;
+ address = fsl_pci_immrbar_base(hose) +
+
[PATCH 0/7] vfio-pci: add support for Freescale IOMMU (PAMU)
From: Bharat Bhushan This patchset adds support for vfio-pci with Freescale IOMMU (PAMU- Peripheral Access Management Unit) The Freescale PAMU is an aperture-based IOMMU with the following characteristics. Each device has an entry in a table in memory describing the iova->phys mapping. The mapping has: -an overall aperture that is power of 2 sized, and has a start iova that is naturally aligned -has 1 or more windows within the aperture -number of windows must be power of 2, max is 256 -size of each window is determined by aperture size / # of windows -iova of each window is determined by aperture start iova / # of windows -the mapped region in each window can be different than the window size...mapping must power of 2 -physical address of the mapping must be naturally aligned with the mapping size Because of some of above said limitations we need to set limited aperture window which will have space for MSI address mapping. So we create space for MSI windows just after the IOVA (guest memory). First 4 patches in this patchset are for setting up MSI window and MSI address at device accordingly. Fifth patch resolves compilation error. Sixth patch moves some common functions in a separate file so that they can be used by FSL_PAMU implementation (next patch uses this). These will be used later for iommu-none implementation. I believe we can do more of this but will take step by step. Finally the seventh patch actually adds the support for FSL-PAMU :) Bharat Bhushan (7): powerpc: Add interface to get msi region information iommu: add api to get iommu_domain of a device fsl iommu: add get_dev_iommu_domain powerpc: translate msi addr to iova if iommu is in use iommu: supress loff_t compilation error on powerpc vfio: moving some functions in common file vfio pci: Add vfio iommu implementation for FSL_PAMU arch/powerpc/include/asm/machdep.h |8 + arch/powerpc/include/asm/pci.h |2 + arch/powerpc/kernel/msi.c | 18 + arch/powerpc/sysdev/fsl_msi.c | 95 - arch/powerpc/sysdev/fsl_msi.h | 11 +- drivers/iommu/fsl_pamu_domain.c| 30 ++ drivers/iommu/iommu.c | 10 + drivers/pci/msi.c | 26 + drivers/vfio/Kconfig |6 + drivers/vfio/Makefile |5 +- drivers/vfio/pci/vfio_pci_rdwr.c |3 +- drivers/vfio/vfio_iommu_common.c | 235 + drivers/vfio/vfio_iommu_common.h | 30 ++ drivers/vfio/vfio_iommu_fsl_pamu.c | 952 drivers/vfio/vfio_iommu_type1.c| 206 + include/linux/iommu.h |7 + include/linux/msi.h|8 + include/linux/pci.h| 13 + include/uapi/linux/vfio.h | 100 19 files changed, 1550 insertions(+), 215 deletions(-) create mode 100644 drivers/vfio/vfio_iommu_common.c create mode 100644 drivers/vfio/vfio_iommu_common.h create mode 100644 drivers/vfio/vfio_iommu_fsl_pamu.c ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/7] fsl iommu: add get_dev_iommu_domain
From: Bharat Bhushan
returns the iommu_domain of the requested device for fsl pamu.
Use PCI controller dev struct for pci devices as current LIODN schema
assign LIODN to PCI controller not PCI device. This will be corrected
with proper LIODN schema.
Signed-off-by: Bharat Bhushan
---
drivers/iommu/fsl_pamu_domain.c | 30 ++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 14d803a..1d0dfe3 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -1140,6 +1140,35 @@ static u32 fsl_pamu_get_windows(struct iommu_domain
*domain)
return dma_domain->win_cnt;
}
+static struct iommu_domain *fsl_get_dev_domain(struct device *dev)
+{
+ struct pci_controller *pci_ctl;
+ struct device_domain_info *info;
+ struct pci_dev *pdev;
+
+ /*
+* Use PCI controller dev struct for pci devices as current
+* LIODN schema assign LIODN to PCI controller not PCI device
+* This should get corrected with proper LIODN schema.
+*/
+ if (dev->bus == &pci_bus_type) {
+ pdev = to_pci_dev(dev);
+ pci_ctl = pci_bus_to_host(pdev->bus);
+ /*
+* make dev point to pci controller device
+* so we can get the LIODN programmed by
+* u-boot.
+*/
+ dev = pci_ctl->parent;
+ }
+
+ info = dev->archdata.iommu_domain;
+ if (info && info->domain)
+ return info->domain->iommu_domain;
+
+ return NULL;
+}
+
static struct iommu_ops fsl_pamu_ops = {
.domain_init= fsl_pamu_domain_init,
.domain_destroy = fsl_pamu_domain_destroy,
@@ -1155,6 +1184,7 @@ static struct iommu_ops fsl_pamu_ops = {
.domain_get_attr = fsl_pamu_get_domain_attr,
.add_device = fsl_pamu_add_device,
.remove_device = fsl_pamu_remove_device,
+ .get_dev_iommu_domain = fsl_get_dev_domain,
};
int pamu_domain_init()
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/7] iommu: supress loff_t compilation error on powerpc
Signed-off-by: Bharat Bhushan
---
drivers/vfio/pci/vfio_pci_rdwr.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c b/drivers/vfio/pci/vfio_pci_rdwr.c
index 210db24..8a8156a 100644
--- a/drivers/vfio/pci/vfio_pci_rdwr.c
+++ b/drivers/vfio/pci/vfio_pci_rdwr.c
@@ -181,7 +181,8 @@ ssize_t vfio_pci_vga_rw(struct vfio_pci_device *vdev, char
__user *buf,
size_t count, loff_t *ppos, bool iswrite)
{
int ret;
- loff_t off, pos = *ppos & VFIO_PCI_OFFSET_MASK;
+ loff_t off;
+ u64 pos = (u64 )(*ppos & VFIO_PCI_OFFSET_MASK);
void __iomem *iomem = NULL;
unsigned int rsrc;
bool is_ioport;
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/7] powerpc: translate msi addr to iova if iommu is in use
If the device is attached with iommu domain then set MSI address
to the iova configured in PAMU.
Signed-off-by: Bharat Bhushan
---
arch/powerpc/sysdev/fsl_msi.c | 56 +++-
1 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index ed045cb..c7cf018 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -150,7 +151,40 @@ static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
return;
}
-static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
+static uint64_t fsl_iommu_get_iova(struct pci_dev *pdev, dma_addr_t msi_phys)
+{
+ struct iommu_domain *domain;
+ struct iommu_domain_geometry geometry;
+ u32 wins = 0;
+ uint64_t iova, size;
+ int ret, i;
+
+ domain = iommu_get_dev_domain(&pdev->dev);
+ if (!domain)
+ return 0;
+
+ ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_WINDOWS, &wins);
+ if (ret)
+ return 0;
+
+ ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_GEOMETRY, &geometry);
+ if (ret)
+ return 0;
+
+ iova = geometry.aperture_start;
+ size = geometry.aperture_end - geometry.aperture_start + 1;
+ do_div(size, wins);
+ for (i = 0; i < wins; i++) {
+ phys_addr_t phys;
+ phys = iommu_iova_to_phys(domain, iova);
+ if (phys == (msi_phys & ~(PAGE_SIZE - 1)))
+ return (iova + (msi_phys & (PAGE_SIZE - 1)));
+ iova += size;
+ }
+ return 0;
+}
+
+static int fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
struct msi_msg *msg,
struct fsl_msi *fsl_msi_data)
{
@@ -168,6 +202,16 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int
hwirq,
address = fsl_pci_immrbar_base(hose) +
(msi_data->msiir & 0xf);
+ /*
+* If the device is attached with iommu domain then set MSI address
+* to the iova configured in PAMU.
+*/
+ if (iommu_get_dev_domain(&pdev->dev)) {
+ address = fsl_iommu_get_iova(pdev, msi_data->msiir);
+ if (!address)
+ return -ENODEV;
+ }
+
msg->address_lo = lower_32_bits(address);
msg->address_hi = upper_32_bits(address);
@@ -175,6 +219,8 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int
hwirq,
pr_debug("%s: allocated srs: %d, ibs: %d\n",
__func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG);
+
+ return 0;
}
static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
@@ -244,7 +290,13 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int
nvec, int type)
/* chip_data is msi_data via host->hostdata in host->map() */
irq_set_msi_desc(virq, entry);
- fsl_compose_msi_msg(pdev, hwirq, &msg, msi_data);
+ if (fsl_compose_msi_msg(pdev, hwirq, &msg, msi_data)) {
+ dev_err(&pdev->dev, "Fail to set MSI for hwirq %i\n",
+ hwirq);
+ msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, 1);
+ rc = -ENODEV;
+ goto out_free;
+ }
write_msi_msg(virq, &msg);
}
return 0;
--
1.7.0.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
