* [PATCH v2 0/2] iommu/virtio: Fixes
@ 2023-05-15 11:39 ` Jean-Philippe Brucker
0 siblings, 0 replies; 9+ messages in thread
From: Jean-Philippe Brucker @ 2023-05-15 11:39 UTC (permalink / raw)
To: joro, will
Cc: Jean-Philippe Brucker, akihiko.odaki, virtualization, eric.auger,
iommu, robin.murphy
One fix reported by Akihiko, and another found while going over the
driver.
Jean-Philippe Brucker (2):
iommu/virtio: Detach domain on endpoint release
iommu/virtio: Return size mapped for a detached domain
drivers/iommu/virtio-iommu.c | 57 ++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 16 deletions(-)
--
2.40.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] iommu/virtio: Detach domain on endpoint release
2023-05-15 11:39 ` Jean-Philippe Brucker
@ 2023-05-15 11:39 ` Jean-Philippe Brucker
-1 siblings, 0 replies; 9+ messages in thread
From: Jean-Philippe Brucker @ 2023-05-15 11:39 UTC (permalink / raw)
To: joro, will
Cc: robin.murphy, eric.auger, virtualization, iommu, akihiko.odaki,
Jean-Philippe Brucker
When an endpoint is released, for example a PCIe VF being destroyed or a
function hot-unplugged, it should be detached from its domain. Send a
DETACH request.
Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
Reported-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Link: https://lore.kernel.org/all/15bf1b00-3aa0-973a-3a86-3fa5c4d41d2c@daynix.com/
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Tested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
v1: https://lore.kernel.org/linux-iommu/20230414150744.562456-1-jean-philippe@linaro.org/
v2: fixed nr_endpoints count reported by Eric
---
drivers/iommu/virtio-iommu.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 5b8fe9bfa9a5b..fd316a37d7562 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -788,6 +788,29 @@ static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
return 0;
}
+static void viommu_detach_dev(struct viommu_endpoint *vdev)
+{
+ int i;
+ struct virtio_iommu_req_detach req;
+ struct viommu_domain *vdomain = vdev->vdomain;
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(vdev->dev);
+
+ if (!vdomain)
+ return;
+
+ req = (struct virtio_iommu_req_detach) {
+ .head.type = VIRTIO_IOMMU_T_DETACH,
+ .domain = cpu_to_le32(vdomain->id),
+ };
+
+ for (i = 0; i < fwspec->num_ids; i++) {
+ req.endpoint = cpu_to_le32(fwspec->ids[i]);
+ WARN_ON(viommu_send_req_sync(vdev->viommu, &req, sizeof(req)));
+ }
+ vdomain->nr_endpoints--;
+ vdev->vdomain = NULL;
+}
+
static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t pgsize, size_t pgcount,
int prot, gfp_t gfp, size_t *mapped)
@@ -990,6 +1013,7 @@ static void viommu_release_device(struct device *dev)
{
struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
+ viommu_detach_dev(vdev);
iommu_put_resv_regions(dev, &vdev->resv_regions);
kfree(vdev);
}
--
2.40.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 1/2] iommu/virtio: Detach domain on endpoint release
@ 2023-05-15 11:39 ` Jean-Philippe Brucker
0 siblings, 0 replies; 9+ messages in thread
From: Jean-Philippe Brucker @ 2023-05-15 11:39 UTC (permalink / raw)
To: joro, will
Cc: Jean-Philippe Brucker, akihiko.odaki, virtualization, eric.auger,
iommu, robin.murphy
When an endpoint is released, for example a PCIe VF being destroyed or a
function hot-unplugged, it should be detached from its domain. Send a
DETACH request.
Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
Reported-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Link: https://lore.kernel.org/all/15bf1b00-3aa0-973a-3a86-3fa5c4d41d2c@daynix.com/
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Tested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
v1: https://lore.kernel.org/linux-iommu/20230414150744.562456-1-jean-philippe@linaro.org/
v2: fixed nr_endpoints count reported by Eric
---
drivers/iommu/virtio-iommu.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 5b8fe9bfa9a5b..fd316a37d7562 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -788,6 +788,29 @@ static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
return 0;
}
+static void viommu_detach_dev(struct viommu_endpoint *vdev)
+{
+ int i;
+ struct virtio_iommu_req_detach req;
+ struct viommu_domain *vdomain = vdev->vdomain;
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(vdev->dev);
+
+ if (!vdomain)
+ return;
+
+ req = (struct virtio_iommu_req_detach) {
+ .head.type = VIRTIO_IOMMU_T_DETACH,
+ .domain = cpu_to_le32(vdomain->id),
+ };
+
+ for (i = 0; i < fwspec->num_ids; i++) {
+ req.endpoint = cpu_to_le32(fwspec->ids[i]);
+ WARN_ON(viommu_send_req_sync(vdev->viommu, &req, sizeof(req)));
+ }
+ vdomain->nr_endpoints--;
+ vdev->vdomain = NULL;
+}
+
static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t pgsize, size_t pgcount,
int prot, gfp_t gfp, size_t *mapped)
@@ -990,6 +1013,7 @@ static void viommu_release_device(struct device *dev)
{
struct viommu_endpoint *vdev = dev_iommu_priv_get(dev);
+ viommu_detach_dev(vdev);
iommu_put_resv_regions(dev, &vdev->resv_regions);
kfree(vdev);
}
--
2.40.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] iommu/virtio: Return size mapped for a detached domain
2023-05-15 11:39 ` Jean-Philippe Brucker
@ 2023-05-15 11:39 ` Jean-Philippe Brucker
-1 siblings, 0 replies; 9+ messages in thread
From: Jean-Philippe Brucker @ 2023-05-15 11:39 UTC (permalink / raw)
To: joro, will
Cc: robin.murphy, eric.auger, virtualization, iommu, akihiko.odaki,
Jean-Philippe Brucker
When map() is called on a detached domain, the domain does not exist in
the device so we do not send a MAP request, but we do update the
internal mapping tree, to be replayed on the next attach. Since this
constitutes a successful iommu_map() call, return *mapped in this case
too.
Fixes: 7e62edd7a33a ("iommu/virtio: Add map/unmap_pages() callbacks implementation")
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/virtio-iommu.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index fd316a37d7562..3551ed057774e 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -833,25 +833,26 @@ static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova,
if (ret)
return ret;
- map = (struct virtio_iommu_req_map) {
- .head.type = VIRTIO_IOMMU_T_MAP,
- .domain = cpu_to_le32(vdomain->id),
- .virt_start = cpu_to_le64(iova),
- .phys_start = cpu_to_le64(paddr),
- .virt_end = cpu_to_le64(end),
- .flags = cpu_to_le32(flags),
- };
+ if (vdomain->nr_endpoints) {
+ map = (struct virtio_iommu_req_map) {
+ .head.type = VIRTIO_IOMMU_T_MAP,
+ .domain = cpu_to_le32(vdomain->id),
+ .virt_start = cpu_to_le64(iova),
+ .phys_start = cpu_to_le64(paddr),
+ .virt_end = cpu_to_le64(end),
+ .flags = cpu_to_le32(flags),
+ };
- if (!vdomain->nr_endpoints)
- return 0;
-
- ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
- if (ret)
- viommu_del_mappings(vdomain, iova, end);
- else if (mapped)
+ ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
+ if (ret) {
+ viommu_del_mappings(vdomain, iova, end);
+ return ret;
+ }
+ }
+ if (mapped)
*mapped = size;
- return ret;
+ return 0;
}
static size_t viommu_unmap_pages(struct iommu_domain *domain, unsigned long iova,
--
2.40.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/2] iommu/virtio: Return size mapped for a detached domain
@ 2023-05-15 11:39 ` Jean-Philippe Brucker
0 siblings, 0 replies; 9+ messages in thread
From: Jean-Philippe Brucker @ 2023-05-15 11:39 UTC (permalink / raw)
To: joro, will
Cc: Jean-Philippe Brucker, akihiko.odaki, virtualization, eric.auger,
iommu, robin.murphy
When map() is called on a detached domain, the domain does not exist in
the device so we do not send a MAP request, but we do update the
internal mapping tree, to be replayed on the next attach. Since this
constitutes a successful iommu_map() call, return *mapped in this case
too.
Fixes: 7e62edd7a33a ("iommu/virtio: Add map/unmap_pages() callbacks implementation")
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/virtio-iommu.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index fd316a37d7562..3551ed057774e 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -833,25 +833,26 @@ static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova,
if (ret)
return ret;
- map = (struct virtio_iommu_req_map) {
- .head.type = VIRTIO_IOMMU_T_MAP,
- .domain = cpu_to_le32(vdomain->id),
- .virt_start = cpu_to_le64(iova),
- .phys_start = cpu_to_le64(paddr),
- .virt_end = cpu_to_le64(end),
- .flags = cpu_to_le32(flags),
- };
+ if (vdomain->nr_endpoints) {
+ map = (struct virtio_iommu_req_map) {
+ .head.type = VIRTIO_IOMMU_T_MAP,
+ .domain = cpu_to_le32(vdomain->id),
+ .virt_start = cpu_to_le64(iova),
+ .phys_start = cpu_to_le64(paddr),
+ .virt_end = cpu_to_le64(end),
+ .flags = cpu_to_le32(flags),
+ };
- if (!vdomain->nr_endpoints)
- return 0;
-
- ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
- if (ret)
- viommu_del_mappings(vdomain, iova, end);
- else if (mapped)
+ ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
+ if (ret) {
+ viommu_del_mappings(vdomain, iova, end);
+ return ret;
+ }
+ }
+ if (mapped)
*mapped = size;
- return ret;
+ return 0;
}
static size_t viommu_unmap_pages(struct iommu_domain *domain, unsigned long iova,
--
2.40.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] iommu/virtio: Return size mapped for a detached domain
2023-05-15 11:39 ` Jean-Philippe Brucker
(?)
@ 2023-05-17 16:41 ` Jason Gunthorpe
-1 siblings, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2023-05-17 16:41 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: joro, will, robin.murphy, eric.auger, virtualization, iommu,
akihiko.odaki
On Mon, May 15, 2023 at 12:39:50PM +0100, Jean-Philippe Brucker wrote:
> When map() is called on a detached domain, the domain does not exist in
> the device so we do not send a MAP request, but we do update the
> internal mapping tree, to be replayed on the next attach. Since this
> constitutes a successful iommu_map() call, return *mapped in this case
> too.
>
> Fixes: 7e62edd7a33a ("iommu/virtio: Add map/unmap_pages() callbacks implementation")
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> drivers/iommu/virtio-iommu.c | 33 +++++++++++++++++----------------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index fd316a37d7562..3551ed057774e 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -833,25 +833,26 @@ static int viommu_map_pages(struct iommu_domain *domain, unsigned long iova,
> if (ret)
> return ret;
>
> - map = (struct virtio_iommu_req_map) {
> - .head.type = VIRTIO_IOMMU_T_MAP,
> - .domain = cpu_to_le32(vdomain->id),
> - .virt_start = cpu_to_le64(iova),
> - .phys_start = cpu_to_le64(paddr),
> - .virt_end = cpu_to_le64(end),
> - .flags = cpu_to_le32(flags),
> - };
> + if (vdomain->nr_endpoints) {
> + map = (struct virtio_iommu_req_map) {
> + .head.type = VIRTIO_IOMMU_T_MAP,
> + .domain = cpu_to_le32(vdomain->id),
> + .virt_start = cpu_to_le64(iova),
> + .phys_start = cpu_to_le64(paddr),
> + .virt_end = cpu_to_le64(end),
> + .flags = cpu_to_le32(flags),
> + };
>
> - if (!vdomain->nr_endpoints)
> - return 0;
> -
> - ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
> - if (ret)
> - viommu_del_mappings(vdomain, iova, end);
> - else if (mapped)
> + ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
> + if (ret) {
> + viommu_del_mappings(vdomain, iova, end);
> + return ret;
> + }
> + }
> + if (mapped)
> *mapped = size;
This looks right
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
But your unmap looks troubled:
unmapped = viommu_del_mappings(vdomain, iova, iova + size - 1);
if (unmapped < size)
return 0;
It shouldn't unmap something and then return 0...
Also, these days this driver be much better to use a multi-order
xarray than in interval tree..
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] iommu/virtio: Fixes
2023-05-15 11:39 ` Jean-Philippe Brucker
@ 2023-05-23 6:19 ` Joerg Roedel
-1 siblings, 0 replies; 9+ messages in thread
From: Joerg Roedel @ 2023-05-23 6:19 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: will, robin.murphy, eric.auger, virtualization, iommu,
akihiko.odaki
On Mon, May 15, 2023 at 12:39:46PM +0100, Jean-Philippe Brucker wrote:
> One fix reported by Akihiko, and another found while going over the
> driver.
>
> Jean-Philippe Brucker (2):
> iommu/virtio: Detach domain on endpoint release
> iommu/virtio: Return size mapped for a detached domain
>
> drivers/iommu/virtio-iommu.c | 57 ++++++++++++++++++++++++++----------
> 1 file changed, 41 insertions(+), 16 deletions(-)
Applied, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] iommu/virtio: Fixes
@ 2023-05-23 6:19 ` Joerg Roedel
0 siblings, 0 replies; 9+ messages in thread
From: Joerg Roedel @ 2023-05-23 6:19 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: akihiko.odaki, will, virtualization, eric.auger, iommu,
robin.murphy
On Mon, May 15, 2023 at 12:39:46PM +0100, Jean-Philippe Brucker wrote:
> One fix reported by Akihiko, and another found while going over the
> driver.
>
> Jean-Philippe Brucker (2):
> iommu/virtio: Detach domain on endpoint release
> iommu/virtio: Return size mapped for a detached domain
>
> drivers/iommu/virtio-iommu.c | 57 ++++++++++++++++++++++++++----------
> 1 file changed, 41 insertions(+), 16 deletions(-)
Applied, thanks.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 9+ messages in thread