This patch implements the PROBE request. At the moment,
no reserved regions are returned as none are registered
per device. Only a NONE property is returned.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v8 -> v9:
- fix filling of properties (changes induced by v0.7 -> v0.8 spec
evolution)
- return VIRTIO_IOMMU_S_INVAL in case of error
v7 -> v8:
- adapt to removal of value filed in virtio_iommu_probe_property
v6 -> v7:
- adapt to the change in virtio_iommu_probe_resv_mem fields
- use get_endpoint() instead of directly checking the EP
was registered.
v4 -> v5:
- initialize bufstate.error to false
- add cpu_to_le64(size)
---
hw/virtio/trace-events | 2 +
hw/virtio/virtio-iommu.c | 168 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 168 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8257065159..2e557dffb4 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -75,3 +75,5 @@ virtio_iommu_unmap_left_interval(uint64_t low, uint64_t high, uint64_t next_low,
virtio_iommu_unmap_right_interval(uint64_t low, uint64_t high, uint64_t next_low, uint64_t next_high) "Unmap right [0x%"PRIx64",0x%"PRIx64"], new interval=[0x%"PRIx64",0x%"PRIx64"]"
virtio_iommu_unmap_inc_interval(uint64_t low, uint64_t high) "Unmap inc [0x%"PRIx64",0x%"PRIx64"]"
virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
+virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t start, uint64_t end, uint32_t flags, size_t filled) "dev= %d, subtype=%d start=0x%"PRIx64" end=0x%"PRIx64" flags=%d filled=0x%lx"
+virtio_iommu_fill_none_property(uint32_t devid) "devid=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index a8de583f9a..66be9a4627 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -37,6 +37,10 @@
/* Max size */
#define VIOMMU_DEFAULT_QUEUE_SIZE 256
+#define VIOMMU_PROBE_SIZE 512
+
+#define SUPPORTED_PROBE_PROPERTIES (\
+ 1 << VIRTIO_IOMMU_PROBE_T_RESV_MEM)
typedef struct viommu_domain {
uint32_t id;
@@ -49,6 +53,7 @@ typedef struct viommu_endpoint {
viommu_domain *domain;
QLIST_ENTRY(viommu_endpoint) next;
VirtIOIOMMU *viommu;
+ GTree *reserved_regions;
} viommu_endpoint;
typedef struct viommu_interval {
@@ -63,6 +68,13 @@ typedef struct viommu_mapping {
uint32_t flags;
} viommu_mapping;
+typedef struct viommu_property_buffer {
+ viommu_endpoint *endpoint;
+ size_t filled;
+ uint8_t *start;
+ bool error;
+} viommu_property_buffer;
+
static inline uint16_t virtio_iommu_get_sid(IOMMUDevice *dev)
{
return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
@@ -102,6 +114,9 @@ static viommu_endpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
ep->viommu = s;
trace_virtio_iommu_get_endpoint(ep_id);
g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
+ ep->reserved_regions = g_tree_new_full((GCompareDataFunc)interval_cmp,
+ NULL, (GDestroyNotify)g_free,
+ (GDestroyNotify)g_free);
return ep;
}
@@ -115,6 +130,7 @@ static void virtio_iommu_put_endpoint(gpointer data)
}
trace_virtio_iommu_put_endpoint(ep->id);
+ g_tree_destroy(ep->reserved_regions);
g_free(ep);
}
@@ -348,6 +364,125 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
return VIRTIO_IOMMU_S_INVAL;
}
+/**
+ * virtio_iommu_fill_resv_mem_prop - Add a RESV_MEM probe
+ * property into the probe request buffer
+ *
+ * @key: interval handle
+ * @value: handle to the reserved memory region
+ * @data: handle to the probe request buffer state
+ */
+static gboolean virtio_iommu_fill_resv_mem_prop(gpointer key,
+ gpointer value,
+ gpointer data)
+{
+ struct virtio_iommu_probe_resv_mem *resv =
+ (struct virtio_iommu_probe_resv_mem *)value;
+ struct virtio_iommu_probe_resv_mem *buf_prop;
+ viommu_property_buffer *bufstate = (viommu_property_buffer *)data;
+ size_t prop_size = sizeof(*resv);
+
+ if (bufstate->filled + prop_size >= VIOMMU_PROBE_SIZE) {
+ bufstate->error = true;
+ /* get the traversal stopped by returning true */
+ return true;
+ }
+ buf_prop = (struct virtio_iommu_probe_resv_mem *)
+ (bufstate->start + bufstate->filled);
+ *buf_prop = *resv;
+
+ bufstate->filled += prop_size;
+ trace_virtio_iommu_fill_resv_property(bufstate->endpoint->id,
+ resv->subtype, resv->start,
+ resv->end, resv->subtype,
+ bufstate->filled);
+ return false;
+}
+
+static int virtio_iommu_fill_none_prop(viommu_property_buffer *bufstate)
+{
+ struct virtio_iommu_probe_property *prop;
+
+ prop = (struct virtio_iommu_probe_property *)
+ (bufstate->start + bufstate->filled);
+ prop->type = 0;
+ prop->length = 0;
+ bufstate->filled += sizeof(*prop);
+ trace_virtio_iommu_fill_none_property(bufstate->endpoint->id);
+ return 0;
+}
+
+/* Fill the properties[] buffer with properties of type @type */
+static int virtio_iommu_fill_property(int type,
+ viommu_property_buffer *bufstate)
+{
+ int ret = -ENOSPC;
+
+ if (bufstate->filled + sizeof(struct virtio_iommu_probe_property)
+ >= VIOMMU_PROBE_SIZE) {
+ /* no space left for the header */
+ bufstate->error = true;
+ goto out;
+ }
+
+ switch (type) {
+ case VIRTIO_IOMMU_PROBE_T_NONE:
+ ret = virtio_iommu_fill_none_prop(bufstate);
+ break;
+ case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
+ {
+ viommu_endpoint *ep = bufstate->endpoint;
+
+ g_tree_foreach(ep->reserved_regions,
+ virtio_iommu_fill_resv_mem_prop,
+ bufstate);
+ if (!bufstate->error) {
+ ret = 0;
+ }
+ break;
+ }
+ default:
+ ret = -ENOENT;
+ break;
+ }
+out:
+ if (ret) {
+ error_report("%s property of type=%d could not be filled (%d),"
+ " remaining size = 0x%lx",
+ __func__, type, ret, bufstate->filled);
+ }
+ return ret;
+}
+
+/**
+ * virtio_iommu_probe - Fill the probe request buffer with all
+ * the properties the device is able to return and add a NONE
+ * property at the end. @buf points to properties[].
+ */
+static int virtio_iommu_probe(VirtIOIOMMU *s,
+ struct virtio_iommu_req_probe *req,
+ uint8_t *buf)
+{
+ uint32_t ep_id = le32_to_cpu(req->endpoint);
+ viommu_endpoint *ep = virtio_iommu_get_endpoint(s, ep_id);
+ int16_t prop_types = SUPPORTED_PROBE_PROPERTIES, type;
+ viommu_property_buffer bufstate = {.start = buf, .filled = 0,
+ .error = false, .endpoint = ep};
+
+ while ((type = ctz32(prop_types)) != 32) {
+ if (virtio_iommu_fill_property(type, &bufstate)) {
+ goto failure;
+ }
+ prop_types &= ~(1 << type);
+ }
+ if (virtio_iommu_fill_property(VIRTIO_IOMMU_PROBE_T_NONE, &bufstate)) {
+ goto failure;
+ }
+ return VIRTIO_IOMMU_S_OK;
+failure:
+ return VIRTIO_IOMMU_S_INVAL;
+}
+
static int virtio_iommu_iov_to_req(struct iovec *iov,
unsigned int iov_cnt,
void *req, size_t req_sz)
@@ -398,6 +533,17 @@ static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
return ret ? ret : virtio_iommu_unmap(s, &req);
}
+static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
+ struct iovec *iov,
+ unsigned int iov_cnt,
+ uint8_t *buf)
+{
+ struct virtio_iommu_req_probe req;
+ int ret = virtio_iommu_iov_to_req(iov, iov_cnt, &req, sizeof(req));
+
+ return ret ? ret : virtio_iommu_probe(s, &req, buf);
+}
+
static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
@@ -443,17 +589,33 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
case VIRTIO_IOMMU_T_UNMAP:
tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt);
break;
+ case VIRTIO_IOMMU_T_PROBE:
+ {
+ struct virtio_iommu_req_tail *ptail;
+ uint8_t *buf = g_malloc0(s->config.probe_size + sizeof(tail));
+
+ ptail = (struct virtio_iommu_req_tail *)
+ (buf + s->config.probe_size);
+ ptail->status = virtio_iommu_handle_probe(s, iov, iov_cnt, buf);
+
+ sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
+ buf, s->config.probe_size + sizeof(tail));
+ g_free(buf);
+ assert(sz == s->config.probe_size + sizeof(tail));
+ goto push;
+ }
default:
tail.status = VIRTIO_IOMMU_S_UNSUPP;
}
- qemu_mutex_unlock(&s->mutex);
out:
sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
&tail, sizeof(tail));
assert(sz == sizeof(tail));
- virtqueue_push(vq, elem, sizeof(tail));
+push:
+ qemu_mutex_unlock(&s->mutex);
+ virtqueue_push(vq, elem, sz);
virtio_notify(vdev, vq);
g_free(elem);
}
@@ -608,6 +770,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
s->config.input_range.end = -1UL;
s->config.domain_range.start = 0;
s->config.domain_range.end = 32;
+ s->config.probe_size = VIOMMU_PROBE_SIZE;
virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX);
virtio_add_feature(&s->features, VIRTIO_RING_F_INDIRECT_DESC);
@@ -617,6 +780,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MAP_UNMAP);
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_BYPASS);
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MMIO);
+ virtio_add_feature(&s->features, VIRTIO_IOMMU_F_PROBE);
qemu_mutex_init(&s->mutex);
--
2.20.1
On Tue, Jul 30, 2019 at 07:21:32PM +0200, Eric Auger wrote:
[...]
> +/* Fill the properties[] buffer with properties of type @type */
> +static int virtio_iommu_fill_property(int type,
> + viommu_property_buffer *bufstate)
> +{
> + int ret = -ENOSPC;
> +
> + if (bufstate->filled + sizeof(struct virtio_iommu_probe_property)
> + >= VIOMMU_PROBE_SIZE) {
> + /* no space left for the header */
> + bufstate->error = true;
> + goto out;
> + }
> +
> + switch (type) {
> + case VIRTIO_IOMMU_PROBE_T_NONE:
> + ret = virtio_iommu_fill_none_prop(bufstate);
> + break;
> + case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
> + {
> + viommu_endpoint *ep = bufstate->endpoint;
> +
> + g_tree_foreach(ep->reserved_regions,
> + virtio_iommu_fill_resv_mem_prop,
> + bufstate);
> + if (!bufstate->error) {
> + ret = 0;
> + }
> + break;
> + }
> + default:
> + ret = -ENOENT;
> + break;
> + }
> +out:
> + if (ret) {
> + error_report("%s property of type=%d could not be filled (%d),"
> + " remaining size = 0x%lx",
> + __func__, type, ret, bufstate->filled);
Nit: If this can really be triggered then we might still change it to
error_report_once()? If it's not (which it seems to), maybe assert
directly?
Other than that it looks good to me:
Reviewed-by: Peter Xu <peterx@redhat.com>
Regards,
--
Peter Xu
Hi Peter,
On 8/19/19 2:08 PM, Peter Xu wrote:
> On Tue, Jul 30, 2019 at 07:21:32PM +0200, Eric Auger wrote:
>
> [...]
>
>> +/* Fill the properties[] buffer with properties of type @type */
>> +static int virtio_iommu_fill_property(int type,
>> + viommu_property_buffer *bufstate)
>> +{
>> + int ret = -ENOSPC;
>> +
>> + if (bufstate->filled + sizeof(struct virtio_iommu_probe_property)
>> + >= VIOMMU_PROBE_SIZE) {
>> + /* no space left for the header */
>> + bufstate->error = true;
>> + goto out;
>> + }
>> +
>> + switch (type) {
>> + case VIRTIO_IOMMU_PROBE_T_NONE:
>> + ret = virtio_iommu_fill_none_prop(bufstate);
>> + break;
>> + case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
>> + {
>> + viommu_endpoint *ep = bufstate->endpoint;
>> +
>> + g_tree_foreach(ep->reserved_regions,
>> + virtio_iommu_fill_resv_mem_prop,
>> + bufstate);
>> + if (!bufstate->error) {
>> + ret = 0;
>> + }
>> + break;
>> + }
>> + default:
>> + ret = -ENOENT;
>> + break;
>> + }
>> +out:
>> + if (ret) {
>> + error_report("%s property of type=%d could not be filled (%d),"
>> + " remaining size = 0x%lx",
>> + __func__, type, ret, bufstate->filled);
>
> Nit: If this can really be triggered then we might still change it to
> error_report_once()? If it's not (which it seems to), maybe assert
> directly?
I put error_report_once() at the moment. The reserved regions may be
passed through cfg or device properties. I think it may happen that
their size get larger than the size set in the device config.
>
> Other than that it looks good to me:
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
Thank you for the review!
Best Regards
Eric
>
> Regards,
>
© 2016 - 2026 Red Hat, Inc.