The event queue allows to report asynchronous errors.
The translate function now injects faults when relevant.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v12 -> v13:
- return on virtio_error()
v11 -> v12:
- reporting the addr associated with the fault and set the
VIRTIO_IOMMU_FAULT_F_ADDRESS flag.
- added cpu_to_le<n>
v10 -> v11:
- change a virtio_error into an error_report_once
(no buffer available for output faults)
---
hw/virtio/trace-events | 1 +
hw/virtio/virtio-iommu.c | 73 +++++++++++++++++++++++++++++++++++++---
2 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 095aa8b509..e83500bee9 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -72,3 +72,4 @@ virtio_iommu_put_endpoint(uint32_t ep_id) "Free endpoint=%d"
virtio_iommu_get_domain(uint32_t domain_id) "Alloc domain=%d"
virtio_iommu_put_domain(uint32_t domain_id) "Free domain=%d"
virtio_iommu_translate_out(uint64_t virt_addr, uint64_t phys_addr, uint32_t sid) "0x%"PRIx64" -> 0x%"PRIx64 " for sid=%d"
+virtio_iommu_report_fault(uint8_t reason, uint32_t flags, uint32_t endpoint, uint64_t addr) "FAULT reason=%d flags=%d endpoint=%d address =0x%"PRIx64
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index cc0715284f..ec8f42e167 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -472,6 +472,51 @@ out:
}
}
+static void virtio_iommu_report_fault(VirtIOIOMMU *viommu, uint8_t reason,
+ int flags, uint32_t endpoint,
+ uint64_t address)
+{
+ VirtIODevice *vdev = &viommu->parent_obj;
+ VirtQueue *vq = viommu->event_vq;
+ struct virtio_iommu_fault fault;
+ VirtQueueElement *elem;
+ size_t sz;
+
+ memset(&fault, 0, sizeof(fault));
+ fault.reason = reason;
+ fault.flags = cpu_to_le32(flags);
+ fault.endpoint = cpu_to_le32(endpoint);
+ fault.address = cpu_to_le64(address);
+
+ for (;;) {
+ elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+
+ if (!elem) {
+ error_report_once(
+ "no buffer available in event queue to report event");
+ return;
+ }
+
+ if (iov_size(elem->in_sg, elem->in_num) < sizeof(fault)) {
+ virtio_error(vdev, "error buffer of wrong size");
+ virtqueue_detach_element(vq, elem, 0);
+ g_free(elem);
+ return;
+ }
+ break;
+ }
+ /* we have a buffer to fill in */
+ sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
+ &fault, sizeof(fault));
+ assert(sz == sizeof(fault));
+
+ trace_virtio_iommu_report_fault(reason, flags, endpoint, address);
+ virtqueue_push(vq, elem, sz);
+ virtio_notify(vdev, vq);
+ g_free(elem);
+
+}
+
static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
IOMMUAccessFlags flag,
int iommu_idx)
@@ -480,9 +525,10 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
VirtIOIOMMUInterval interval, *mapping_key;
VirtIOIOMMUMapping *mapping_value;
VirtIOIOMMU *s = sdev->viommu;
+ bool read_fault, write_fault;
VirtIOIOMMUEndpoint *ep;
+ uint32_t sid, flags;
bool bypass_allowed;
- uint32_t sid;
bool found;
interval.low = addr;
@@ -508,6 +554,9 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
if (!ep) {
if (!bypass_allowed) {
error_report_once("%s sid=%d is not known!!", __func__, sid);
+ virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_UNKNOWN,
+ VIRTIO_IOMMU_FAULT_F_ADDRESS,
+ sid, addr);
} else {
entry.perm = flag;
}
@@ -519,6 +568,9 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
error_report_once("%s %02x:%02x.%01x not attached to any domain",
__func__, PCI_BUS_NUM(sid),
PCI_SLOT(sid), PCI_FUNC(sid));
+ virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_DOMAIN,
+ VIRTIO_IOMMU_FAULT_F_ADDRESS,
+ sid, addr);
} else {
entry.perm = flag;
}
@@ -531,15 +583,26 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
if (!found) {
error_report_once("%s no mapping for 0x%"PRIx64" for sid=%d",
__func__, addr, sid);
+ virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_MAPPING,
+ VIRTIO_IOMMU_FAULT_F_ADDRESS,
+ sid, addr);
goto unlock;
}
- if (((flag & IOMMU_RO) &&
- !(mapping_value->flags & VIRTIO_IOMMU_MAP_F_READ)) ||
- ((flag & IOMMU_WO) &&
- !(mapping_value->flags & VIRTIO_IOMMU_MAP_F_WRITE))) {
+ read_fault = (flag & IOMMU_RO) &&
+ !(mapping_value->flags & VIRTIO_IOMMU_MAP_F_READ);
+ write_fault = (flag & IOMMU_WO) &&
+ !(mapping_value->flags & VIRTIO_IOMMU_MAP_F_WRITE);
+
+ flags = read_fault ? VIRTIO_IOMMU_FAULT_F_READ : 0;
+ flags |= write_fault ? VIRTIO_IOMMU_FAULT_F_WRITE : 0;
+ if (flags) {
error_report_once("%s permission error on 0x%"PRIx64"(%d): allowed=%d",
__func__, addr, flag, mapping_value->flags);
+ flags |= VIRTIO_IOMMU_FAULT_F_ADDRESS;
+ virtio_iommu_report_fault(s, VIRTIO_IOMMU_FAULT_R_MAPPING,
+ flags | VIRTIO_IOMMU_FAULT_F_ADDRESS,
+ sid, addr);
goto unlock;
}
entry.translated_addr = addr - mapping_key->low + mapping_value->phys_addr;
--
2.20.1
On Sat, Jan 25, 2020 at 06:19:51PM +0100, Eric Auger wrote:
[...]
> +static void virtio_iommu_report_fault(VirtIOIOMMU *viommu, uint8_t reason,
> + int flags, uint32_t endpoint,
> + uint64_t address)
> +{
> + VirtIODevice *vdev = &viommu->parent_obj;
> + VirtQueue *vq = viommu->event_vq;
> + struct virtio_iommu_fault fault;
> + VirtQueueElement *elem;
> + size_t sz;
> +
> + memset(&fault, 0, sizeof(fault));
> + fault.reason = reason;
> + fault.flags = cpu_to_le32(flags);
> + fault.endpoint = cpu_to_le32(endpoint);
> + fault.address = cpu_to_le64(address);
> +
> + for (;;) {
> + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> +
> + if (!elem) {
> + error_report_once(
> + "no buffer available in event queue to report event");
> + return;
> + }
> +
> + if (iov_size(elem->in_sg, elem->in_num) < sizeof(fault)) {
> + virtio_error(vdev, "error buffer of wrong size");
> + virtqueue_detach_element(vq, elem, 0);
> + g_free(elem);
> + return;
> + }
> + break;
This for loop is not needed any more? Other than that:
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
Hi Peter,
On 2/3/20 2:55 PM, Peter Xu wrote:
> On Sat, Jan 25, 2020 at 06:19:51PM +0100, Eric Auger wrote:
>
> [...]
>
>> +static void virtio_iommu_report_fault(VirtIOIOMMU *viommu, uint8_t reason,
>> + int flags, uint32_t endpoint,
>> + uint64_t address)
>> +{
>> + VirtIODevice *vdev = &viommu->parent_obj;
>> + VirtQueue *vq = viommu->event_vq;
>> + struct virtio_iommu_fault fault;
>> + VirtQueueElement *elem;
>> + size_t sz;
>> +
>> + memset(&fault, 0, sizeof(fault));
>> + fault.reason = reason;
>> + fault.flags = cpu_to_le32(flags);
>> + fault.endpoint = cpu_to_le32(endpoint);
>> + fault.address = cpu_to_le64(address);
>> +
>> + for (;;) {
>> + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>> +
>> + if (!elem) {
>> + error_report_once(
>> + "no buffer available in event queue to report event");
>> + return;
>> + }
>> +
>> + if (iov_size(elem->in_sg, elem->in_num) < sizeof(fault)) {
>> + virtio_error(vdev, "error buffer of wrong size");
>> + virtqueue_detach_element(vq, elem, 0);
>> + g_free(elem);
>> + return;
>> + }
>> + break;
>
> This for loop is not needed any more? Other than that:
hum yes indeed.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
Thanks!
Eric
>
© 2016 - 2026 Red Hat, Inc.