[RFC PATCH v2 3/5] iommu/virtio-iommu: Add event queue
The event queue offers a way for the device to report access faults from
devices. It is implemented on virtqueue #1, whenever the host needs to
signal a fault it fills one of the buffers offered by the guest and
interrupts it.
Signed-off-by: Jean-Philippe Brucker
---
drivers/iommu/virtio-iommu.c | 138 ++
include/uapi/linux/virtio_iommu.h | 18 +
2 files changed, 142 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 79e0add94e05..fe0d449bf489 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -30,6 +30,12 @@
#define MSI_IOVA_BASE 0x800
#define MSI_IOVA_LENGTH0x10
+enum viommu_vq_idx {
+ VIOMMU_REQUEST_VQ = 0,
+ VIOMMU_EVENT_VQ = 1,
+ VIOMMU_NUM_VQS = 2,
+};
+
struct viommu_dev {
struct iommu_device iommu;
struct device *dev;
@@ -37,7 +43,7 @@ struct viommu_dev {
struct ida domain_ids;
- struct virtqueue*vq;
+ struct virtqueue*vqs[VIOMMU_NUM_VQS];
/* Serialize anything touching the request queue */
spinlock_t request_lock;
@@ -84,6 +90,15 @@ struct viommu_request {
struct list_headlist;
};
+#define VIOMMU_FAULT_RESV_MASK 0xff00
+
+struct viommu_event {
+ union {
+ u32 head;
+ struct virtio_iommu_fault fault;
+ };
+};
+
#define to_viommu_domain(domain) container_of(domain, struct viommu_domain,
domain)
/* Virtio transport */
@@ -160,12 +175,13 @@ static int viommu_receive_resp(struct viommu_dev *viommu,
int nr_sent,
unsigned int len;
int nr_received = 0;
struct viommu_request *req, *pending;
+ struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
pending = list_first_entry_or_null(sent, struct viommu_request, list);
if (WARN_ON(!pending))
return 0;
- while ((req = virtqueue_get_buf(viommu->vq, &len)) != NULL) {
+ while ((req = virtqueue_get_buf(vq, &len)) != NULL) {
if (req != pending) {
dev_warn(viommu->dev, "discarding stale request\n");
continue;
@@ -202,6 +218,7 @@ static int _viommu_send_reqs_sync(struct viommu_dev *viommu,
* dies.
*/
unsigned long timeout_ms = 1000;
+ struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
*nr_sent = 0;
@@ -211,15 +228,14 @@ static int _viommu_send_reqs_sync(struct viommu_dev
*viommu,
sg[0] = &req->top;
sg[1] = &req->bottom;
- ret = virtqueue_add_sgs(viommu->vq, sg, 1, 1, req,
- GFP_ATOMIC);
+ ret = virtqueue_add_sgs(vq, sg, 1, 1, req, GFP_ATOMIC);
if (ret)
break;
list_add_tail(&req->list, &pending);
}
- if (i && !virtqueue_kick(viommu->vq))
+ if (i && !virtqueue_kick(vq))
return -EPIPE;
timeout = ktime_add_ms(ktime_get(), timeout_ms * i);
@@ -554,6 +570,70 @@ static int viommu_probe_endpoint(struct viommu_dev
*viommu, struct device *dev)
return 0;
}
+static int viommu_fault_handler(struct viommu_dev *viommu,
+ struct virtio_iommu_fault *fault)
+{
+ char *reason_str;
+
+ u8 reason = fault->reason;
+ u32 flags = le32_to_cpu(fault->flags);
+ u32 endpoint= le32_to_cpu(fault->endpoint);
+ u64 address = le64_to_cpu(fault->address);
+
+ switch (reason) {
+ case VIRTIO_IOMMU_FAULT_R_DOMAIN:
+ reason_str = "domain";
+ break;
+ case VIRTIO_IOMMU_FAULT_R_MAPPING:
+ reason_str = "page";
+ break;
+ case VIRTIO_IOMMU_FAULT_R_UNKNOWN:
+ default:
+ reason_str = "unknown";
+ break;
+ }
+
+ /* TODO: find EP by ID and report_iommu_fault */
+ if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
+ dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx
[%s%s%s]\n",
+ reason_str, endpoint, address,
+ flags & VIRTIO_IOMMU_FAULT_F_READ ? "R" :
"",
+ flags & VIRTIO_IOMMU_FAULT_F_WRITE ? "W" :
"",
+ flags & VIRTIO_IOMMU_FAULT_F_EXEC ? "X" :
"");
+ else
+ dev_err_ratelimited(viommu->dev, "%s fault from EP %u\n",
+ reason_str, endpoint);
+
+ return 0;
+}
+
+static void viommu_event_handler(struct virtqueue *vq)
+{
+ int ret;
+ unsigned int len;
+ struct scatterlist sg[1];
+
Re: [RFC PATCH v2 3/5] iommu/virtio-iommu: Add event queue
Hi,
On 17/11/17 19:52, Jean-Philippe Brucker wrote:
> The event queue offers a way for the device to report access faults from
> devices.
end points?
It is implemented on virtqueue #1, whenever the host needs to
> signal a fault it fills one of the buffers offered by the guest and
> interrupts it.
>
> Signed-off-by: Jean-Philippe Brucker
> ---
> drivers/iommu/virtio-iommu.c | 138
> ++
> include/uapi/linux/virtio_iommu.h | 18 +
> 2 files changed, 142 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 79e0add94e05..fe0d449bf489 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -30,6 +30,12 @@
> #define MSI_IOVA_BASE0x800
> #define MSI_IOVA_LENGTH 0x10
>
> +enum viommu_vq_idx {
> + VIOMMU_REQUEST_VQ = 0,
> + VIOMMU_EVENT_VQ = 1,
> + VIOMMU_NUM_VQS = 2,
> +};
> +
> struct viommu_dev {
> struct iommu_device iommu;
> struct device *dev;
> @@ -37,7 +43,7 @@ struct viommu_dev {
>
> struct ida domain_ids;
>
> - struct virtqueue*vq;
> + struct virtqueue*vqs[VIOMMU_NUM_VQS];
> /* Serialize anything touching the request queue */
> spinlock_t request_lock;
>
> @@ -84,6 +90,15 @@ struct viommu_request {
> struct list_headlist;
> };
>
> +#define VIOMMU_FAULT_RESV_MASK 0xff00
> +
> +struct viommu_event {
> + union {
> + u32 head;
> + struct virtio_iommu_fault fault;
> + };
> +};
> +
> #define to_viommu_domain(domain) container_of(domain, struct viommu_domain,
> domain)
>
> /* Virtio transport */
> @@ -160,12 +175,13 @@ static int viommu_receive_resp(struct viommu_dev
> *viommu, int nr_sent,
> unsigned int len;
> int nr_received = 0;
> struct viommu_request *req, *pending;
> + struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
>
> pending = list_first_entry_or_null(sent, struct viommu_request, list);
> if (WARN_ON(!pending))
> return 0;
>
> - while ((req = virtqueue_get_buf(viommu->vq, &len)) != NULL) {
> + while ((req = virtqueue_get_buf(vq, &len)) != NULL) {
> if (req != pending) {
> dev_warn(viommu->dev, "discarding stale request\n");
> continue;
> @@ -202,6 +218,7 @@ static int _viommu_send_reqs_sync(struct viommu_dev
> *viommu,
>* dies.
>*/
> unsigned long timeout_ms = 1000;
> + struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
>
> *nr_sent = 0;
>
> @@ -211,15 +228,14 @@ static int _viommu_send_reqs_sync(struct viommu_dev
> *viommu,
> sg[0] = &req->top;
> sg[1] = &req->bottom;
>
> - ret = virtqueue_add_sgs(viommu->vq, sg, 1, 1, req,
> - GFP_ATOMIC);
> + ret = virtqueue_add_sgs(vq, sg, 1, 1, req, GFP_ATOMIC);
> if (ret)
> break;
>
> list_add_tail(&req->list, &pending);
> }
>
> - if (i && !virtqueue_kick(viommu->vq))
> + if (i && !virtqueue_kick(vq))
> return -EPIPE;
>
> timeout = ktime_add_ms(ktime_get(), timeout_ms * i);
> @@ -554,6 +570,70 @@ static int viommu_probe_endpoint(struct viommu_dev
> *viommu, struct device *dev)
> return 0;
> }
>
> +static int viommu_fault_handler(struct viommu_dev *viommu,
> + struct virtio_iommu_fault *fault)
> +{
> + char *reason_str;
> +
> + u8 reason = fault->reason;
> + u32 flags = le32_to_cpu(fault->flags);
> + u32 endpoint= le32_to_cpu(fault->endpoint);
> + u64 address = le64_to_cpu(fault->address);
> +
> + switch (reason) {
> + case VIRTIO_IOMMU_FAULT_R_DOMAIN:
> + reason_str = "domain";
> + break;
> + case VIRTIO_IOMMU_FAULT_R_MAPPING:
> + reason_str = "page";
> + break;
> + case VIRTIO_IOMMU_FAULT_R_UNKNOWN:
> + default:
> + reason_str = "unknown";
> + break;
> + }
> +
> + /* TODO: find EP by ID and report_iommu_fault */
> + if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
> + dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx
> [%s%s%s]\n",
> + reason_str, endpoint, address,
> + flags & VIRTIO_IOMMU_FAULT_F_READ ? "R" :
> "",
> + flags & VIRTIO_IOMMU_FAULT_F_WRITE ? "W" :
> "",
> + flags & VIRTIO_IOMMU_FAULT_F_EXEC ? "X" :
> "");
> + else
> + dev_err_ratelimited(viommu->dev, "%s fault from EP %u\n",
> +
Re: [RFC PATCH v2 3/5] iommu/virtio-iommu: Add event queue
On 16/01/18 10:10, Auger Eric wrote:
> Hi,
>
> On 17/11/17 19:52, Jean-Philippe Brucker wrote:
>> The event queue offers a way for the device to report access faults from
>> devices.
> end points?
Yes
[...]
>> +static void viommu_event_handler(struct virtqueue *vq)
>> +{
>> +int ret;
>> +unsigned int len;
>> +struct scatterlist sg[1];
>> +struct viommu_event *evt;
>> +struct viommu_dev *viommu = vq->vdev->priv;
>> +
>> +while ((evt = virtqueue_get_buf(vq, &len)) != NULL) {
>> +if (len > sizeof(*evt)) {
>> +dev_err(viommu->dev,
>> +"invalid event buffer (len %u != %zu)\n",
>> +len, sizeof(*evt));
>> +} else if (!(evt->head & VIOMMU_FAULT_RESV_MASK)) {
>> +viommu_fault_handler(viommu, &evt->fault);
>> +}
>> +
>> +sg_init_one(sg, evt, sizeof(*evt));
> in case of above error case, ie. len > sizeof(*evt), is it safe to push
> evt again?
I think it is, len is just what the device reports as written. The above
error would be a device bug, very unlikely and not worth a special
treatment (freeing the buffer), maybe not even worth the dev_err()
actually.
>> +ret = virtqueue_add_inbuf(vq, sg, 1, evt, GFP_ATOMIC);
>> +if (ret)
>> +dev_err(viommu->dev, "could not add event buffer\n");
>> +}
>> +
>> +if (!virtqueue_kick(vq))
>> +dev_err(viommu->dev, "kick failed\n");
>> +}
>> +
>> /* IOMMU API */
>>
>> static bool viommu_capable(enum iommu_cap cap)
>> @@ -938,19 +1018,44 @@ static struct iommu_ops viommu_ops = {
>> .put_resv_regions = viommu_put_resv_regions,
>> };
>>
>> -static int viommu_init_vq(struct viommu_dev *viommu)
>> +static int viommu_init_vqs(struct viommu_dev *viommu)
>> {
>> struct virtio_device *vdev = dev_to_virtio(viommu->dev);
>> -const char *name = "request";
>> -void *ret;
>> +const char *names[] = { "request", "event" };
>> +vq_callback_t *callbacks[] = {
>> +NULL, /* No async requests */
>> +viommu_event_handler,
>> +};
>> +
>> +return virtio_find_vqs(vdev, VIOMMU_NUM_VQS, viommu->vqs, callbacks,
>> + names, NULL);
>> +}
>>
>> -ret = virtio_find_single_vq(vdev, NULL, name);
>> -if (IS_ERR(ret)) {
>> -dev_err(viommu->dev, "cannot find VQ\n");
>> -return PTR_ERR(ret);
>> +static int viommu_fill_evtq(struct viommu_dev *viommu)
>> +{
>> +int i, ret;
>> +struct scatterlist sg[1];
>> +struct viommu_event *evts;
>> +struct virtqueue *vq = viommu->vqs[VIOMMU_EVENT_VQ];
>> +size_t nr_evts = min_t(size_t, PAGE_SIZE / sizeof(struct viommu_event),
>> + viommu->vqs[VIOMMU_EVENT_VQ]->num_free);
>> +
>> +evts = devm_kmalloc_array(viommu->dev, nr_evts, sizeof(*evts),
>> + GFP_KERNEL);
>> +if (!evts)
>> +return -ENOMEM;
>> +
>> +for (i = 0; i < nr_evts; i++) {
>> +sg_init_one(sg, &evts[i], sizeof(*evts));
>> +ret = virtqueue_add_inbuf(vq, sg, 1, &evts[i], GFP_KERNEL);
> who does the deallocation of evts?
I think it should be viommu_remove, which should also remove the
virtqueues. I'll add this.
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
