* [PATCH v13 00/10] VIRTIO-IOMMU device
@ 2020-01-25 17:19 Eric Auger
2020-01-25 17:19 ` [PATCH v13 01/10] virtio-iommu: Add skeleton Eric Auger
` (10 more replies)
0 siblings, 11 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This series implements the QEMU virtio-iommu device.
This matches the v0.12 spec (voted) and the corresponding
virtio-iommu driver upstreamed in 5.3. All kernel dependencies
are resolved for DT integration. The virtio-iommu can be
instantiated in ARM virt using "-device virtio-iommu-pci".
Non DT mode is not yet supported as it has non resolved kernel
dependencies [1].
This feature targets 5.0.
Integration with vhost devices and vfio devices is not part
of this series. Please follow Bharat's respins [2].
Best Regards
Eric
This series can be found at:
https://github.com/eauger/qemu/tree/v4.2-virtio-iommu-v13
References:
[1] [RFC 00/13] virtio-iommu on non-devicetree platforms
[2] [PATCH RFC v5 0/5] virtio-iommu: VFIO integration
Testing:
- tested with guest using virtio-net-pci
(,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on)
and virtio-blk-pci
- migration
History:
v12 -> v13:
- Take into account Peter's comments
- fix qtest error and accomodate for directory changes in
test
- remove "[PATCH v12 01/13] migration: Support QLIST migration"
which is now upstream
- fix iommu_find_iommu_pcibus()
- squash commits as requested by Peter
- remove spurious guest log
v11 -> v12:
- took into account Peter and Jean's comments
- use guest features
- restore as_by_bus_num and when attaching devices, check they are
actually protected by the IOMMU. Updated the tests accordingly.
- fix the mapping ref counting and make sure mappings are properly
cleaned.
- Use CamelCase for data types
- simplify postload callback as suggested by Peter
- add R-bs
- fix mingw compilation issue
- add IOMMU migration priority
- qlist migration load simplified following Juan's suggestion
v10 -> v11:
- introduce virtio_iommu_handle_req macro
- migration support
- introduce DEFINE_PROP_INTERVAL and pass reserved regions
through an array of those
- domain gtree simplification
v9 -> v10:
- rebase on 4.1.0-rc2, compliance with 0.12 spec
- removed ACPI part
- cleanup (see individual change logs)
- moved to a PATCH series
v8 -> v9:
- virtio-iommu-pci device needs to be instantiated from the command
line (RID is not imposed anymore).
- tail structure properly initialized
v7 -> v8:
- virtio-iommu-pci added
- virt instantiation modified
- DT and ACPI modified to exclude the iommu RID from the mapping
- VIRTIO_IOMMU_F_BYPASS, VIRTIO_F_VERSION_1 features exposed
v6 -> v7:
- rebase on qemu 3.0.0-rc3
- minor update against v0.7
- fix issue with EP not on pci.0 and ACPI probing
- change the instantiation method
v5 -> v6:
- minor update against v0.6 spec
- fix g_hash_table_lookup in virtio_iommu_find_add_as
- replace some error_reports by qemu_log_mask(LOG_GUEST_ERROR, ...)
v4 -> v5:
- event queue and fault reporting
- we now return the IOAPIC MSI region if the virtio-iommu is instantiated
in a PC machine.
- we bypass transactions on MSI HW region and fault on reserved ones.
- We support ACPI boot with mach-virt (based on IORT proposal)
- We moved to the new driver naming conventions
- simplified mach-virt instantiation
- worked around the disappearing of pci_find_primary_bus
- in virtio_iommu_translate, check the dev->as is not NULL
- initialize as->device_list in virtio_iommu_get_as
- initialize bufstate.error to false in virtio_iommu_probe
v3 -> v4:
- probe request support although no reserved region is returned at
the moment
- unmap semantics less strict, as specified in v0.4
- device registration, attach/detach revisited
- split into smaller patches to ease review
- propose a way to inform the IOMMU mr about the page_size_mask
of underlying HW IOMMU, if any
- remove warning associated with the translation of the MSI doorbell
v2 -> v3:
- rebase on top of 2.10-rc0 and especially
[PATCH qemu v9 0/2] memory/iommu: QOM'fy IOMMU MemoryRegion
- add mutex init
- fix as->mappings deletion using g_tree_ref/unref
- when a dev is attached whereas it is already attached to
another address space, first detach it
- fix some error values
- page_sizes = TARGET_PAGE_MASK;
- I haven't changed the unmap() semantics yet, waiting for the
next virtio-iommu spec revision.
v1 -> v2:
- fix redefinition of viommu_as typedef
Eric Auger (10):
virtio-iommu: Add skeleton
virtio-iommu: Decode the command payload
virtio-iommu: Implement attach/detach command
virtio-iommu: Implement map/unmap
virtio-iommu: Implement translate
virtio-iommu: Implement fault reporting
virtio-iommu-pci: Add virtio iommu pci support
hw/arm/virt: Add the virtio-iommu device tree mappings
virtio-iommu: Support migration
tests: Add virtio-iommu test
hw/arm/virt.c | 54 +-
hw/virtio/Kconfig | 5 +
hw/virtio/Makefile.objs | 2 +
hw/virtio/trace-events | 20 +
hw/virtio/virtio-iommu-pci.c | 88 +++
hw/virtio/virtio-iommu.c | 897 ++++++++++++++++++++++++++++++
include/hw/arm/virt.h | 2 +
include/hw/pci/pci.h | 1 +
include/hw/virtio/virtio-iommu.h | 61 ++
qdev-monitor.c | 1 +
tests/qtest/Makefile.include | 2 +
tests/qtest/libqos/virtio-iommu.c | 177 ++++++
tests/qtest/libqos/virtio-iommu.h | 45 ++
tests/qtest/virtio-iommu-test.c | 306 ++++++++++
14 files changed, 1653 insertions(+), 8 deletions(-)
create mode 100644 hw/virtio/virtio-iommu-pci.c
create mode 100644 hw/virtio/virtio-iommu.c
create mode 100644 include/hw/virtio/virtio-iommu.h
create mode 100644 tests/qtest/libqos/virtio-iommu.c
create mode 100644 tests/qtest/libqos/virtio-iommu.h
create mode 100644 tests/qtest/virtio-iommu-test.c
--
2.20.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v13 01/10] virtio-iommu: Add skeleton
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-01-25 17:19 ` [PATCH v13 02/10] virtio-iommu: Decode the command payload Eric Auger
` (9 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This patchs adds the skeleton for the virtio-iommu device.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
v12 -> v13
- removed IOMMU_PCI_BUS_MAX and IOMMU_PCI_DEVFN_MAX
v11 -> v12:
- remove s_by_bus_num
- drop set_features (rely on default implementation) and
acked_features
v9 -> v10:
- mutex initialized here
- initialize tail
- included hw/qdev-properties.h
- removed g_memdup
- removed s->config.domain_range.start = 0;
v9 -> v10:
- expose VIRTIO_IOMMU_F_MMIO feature
- s/domain_bits/domain_range struct
- change error codes
- enforce unmigratable
- Kconfig
v7 -> v8:
- expose VIRTIO_IOMMU_F_BYPASS and VIRTIO_F_VERSION_1
features
- set_config dummy implementation + tracing
- add trace in get_features
- set the features on realize() and store the acked ones
- remove inclusion of linux/virtio_iommu.h
v6 -> v7:
- removed qapi-event.h include
- add primary_bus and associated property
v4 -> v5:
- use the new v0.5 terminology (domain, endpoint)
- add the event virtqueue
v3 -> v4:
- use page_size_mask instead of page_sizes
- added set_features()
- added some traces (reset, set_status, set_features)
- empty virtio_iommu_set_config() as the driver MUST NOT
write to device configuration fields
- add get_config trace
v2 -> v3:
- rebase on 2.10-rc0, ie. use IOMMUMemoryRegion and remove
iommu_ops.
- advertise VIRTIO_IOMMU_F_MAP_UNMAP feature
- page_sizes set to TARGET_PAGE_SIZE
Conflicts:
hw/virtio/trace-events
---
hw/virtio/Kconfig | 5 +
hw/virtio/Makefile.objs | 1 +
hw/virtio/trace-events | 7 +
hw/virtio/virtio-iommu.c | 265 +++++++++++++++++++++++++++++++
include/hw/virtio/virtio-iommu.h | 57 +++++++
5 files changed, 335 insertions(+)
create mode 100644 hw/virtio/virtio-iommu.c
create mode 100644 include/hw/virtio/virtio-iommu.h
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index f87def27a6..d29525b36f 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -9,6 +9,11 @@ config VIRTIO_RNG
default y
depends on VIRTIO
+config VIRTIO_IOMMU
+ bool
+ default y
+ depends on VIRTIO
+
config VIRTIO_PCI
bool
default y if PCI_DEVICES
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index de0f5fc39b..2fd9da7410 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -16,6 +16,7 @@ obj-$(call land,$(CONFIG_VIRTIO_CRYPTO),$(CONFIG_VIRTIO_PCI)) += virtio-crypto-p
obj-$(CONFIG_VIRTIO_PMEM) += virtio-pmem.o
common-obj-$(call land,$(CONFIG_VIRTIO_PMEM),$(CONFIG_VIRTIO_PCI)) += virtio-pmem-pci.o
obj-$(call land,$(CONFIG_VHOST_USER_FS),$(CONFIG_VIRTIO_PCI)) += vhost-user-fs-pci.o
+obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
ifeq ($(CONFIG_VIRTIO_PCI),y)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index e28ba48da6..02d93d7f63 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -53,3 +53,10 @@ virtio_mmio_write_offset(uint64_t offset, uint64_t value) "virtio_mmio_write off
virtio_mmio_guest_page(uint64_t size, int shift) "guest page size 0x%" PRIx64 " shift %d"
virtio_mmio_queue_write(uint64_t value, int max_size) "mmio_queue write 0x%" PRIx64 " max %d"
virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
+
+# hw/virtio/virtio-iommu.c
+virtio_iommu_device_reset(void) "reset!"
+virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
+virtio_iommu_device_status(uint8_t status) "driver status = %d"
+virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x"
+virtio_iommu_set_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
new file mode 100644
index 0000000000..7e55eda67e
--- /dev/null
+++ b/hw/virtio/virtio-iommu.c
@@ -0,0 +1,265 @@
+/*
+ * virtio-iommu device
+ *
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "qemu-common.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/virtio.h"
+#include "sysemu/kvm.h"
+#include "trace.h"
+
+#include "standard-headers/linux/virtio_ids.h"
+
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/virtio-iommu.h"
+
+/* Max size */
+#define VIOMMU_DEFAULT_QUEUE_SIZE 256
+
+static int virtio_iommu_handle_attach(VirtIOIOMMU *s,
+ struct iovec *iov,
+ unsigned int iov_cnt)
+{
+ return VIRTIO_IOMMU_S_UNSUPP;
+}
+static int virtio_iommu_handle_detach(VirtIOIOMMU *s,
+ struct iovec *iov,
+ unsigned int iov_cnt)
+{
+ return VIRTIO_IOMMU_S_UNSUPP;
+}
+static int virtio_iommu_handle_map(VirtIOIOMMU *s,
+ struct iovec *iov,
+ unsigned int iov_cnt)
+{
+ return VIRTIO_IOMMU_S_UNSUPP;
+}
+static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
+ struct iovec *iov,
+ unsigned int iov_cnt)
+{
+ return VIRTIO_IOMMU_S_UNSUPP;
+}
+
+static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
+{
+ VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
+ struct virtio_iommu_req_head head;
+ struct virtio_iommu_req_tail tail = {};
+ VirtQueueElement *elem;
+ unsigned int iov_cnt;
+ struct iovec *iov;
+ size_t sz;
+
+ for (;;) {
+ elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+ if (!elem) {
+ return;
+ }
+
+ if (iov_size(elem->in_sg, elem->in_num) < sizeof(tail) ||
+ iov_size(elem->out_sg, elem->out_num) < sizeof(head)) {
+ virtio_error(vdev, "virtio-iommu bad head/tail size");
+ virtqueue_detach_element(vq, elem, 0);
+ g_free(elem);
+ break;
+ }
+
+ iov_cnt = elem->out_num;
+ iov = elem->out_sg;
+ sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
+ if (unlikely(sz != sizeof(head))) {
+ tail.status = VIRTIO_IOMMU_S_DEVERR;
+ goto out;
+ }
+ qemu_mutex_lock(&s->mutex);
+ switch (head.type) {
+ case VIRTIO_IOMMU_T_ATTACH:
+ tail.status = virtio_iommu_handle_attach(s, iov, iov_cnt);
+ break;
+ case VIRTIO_IOMMU_T_DETACH:
+ tail.status = virtio_iommu_handle_detach(s, iov, iov_cnt);
+ break;
+ case VIRTIO_IOMMU_T_MAP:
+ tail.status = virtio_iommu_handle_map(s, iov, iov_cnt);
+ break;
+ case VIRTIO_IOMMU_T_UNMAP:
+ tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt);
+ break;
+ 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));
+ virtio_notify(vdev, vq);
+ g_free(elem);
+ }
+}
+
+static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
+{
+ VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
+ struct virtio_iommu_config *config = &dev->config;
+
+ trace_virtio_iommu_get_config(config->page_size_mask,
+ config->input_range.start,
+ config->input_range.end,
+ config->domain_range.end,
+ config->probe_size);
+ memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
+}
+
+static void virtio_iommu_set_config(VirtIODevice *vdev,
+ const uint8_t *config_data)
+{
+ struct virtio_iommu_config config;
+
+ memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
+ trace_virtio_iommu_set_config(config.page_size_mask,
+ config.input_range.start,
+ config.input_range.end,
+ config.domain_range.end,
+ config.probe_size);
+}
+
+static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
+ Error **errp)
+{
+ VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
+
+ f |= dev->features;
+ trace_virtio_iommu_get_features(f);
+ return f;
+}
+
+/*
+ * Migration is not yet supported: most of the state consists
+ * of balanced binary trees which are not yet ready for getting
+ * migrated
+ */
+static const VMStateDescription vmstate_virtio_iommu_device = {
+ .name = "virtio-iommu-device",
+ .unmigratable = 1,
+};
+
+static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
+
+ virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
+ sizeof(struct virtio_iommu_config));
+
+ s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
+ virtio_iommu_handle_command);
+ s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
+
+ s->config.page_size_mask = TARGET_PAGE_MASK;
+ s->config.input_range.end = -1UL;
+ s->config.domain_range.end = 32;
+
+ virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX);
+ virtio_add_feature(&s->features, VIRTIO_RING_F_INDIRECT_DESC);
+ virtio_add_feature(&s->features, VIRTIO_F_VERSION_1);
+ virtio_add_feature(&s->features, VIRTIO_IOMMU_F_INPUT_RANGE);
+ virtio_add_feature(&s->features, VIRTIO_IOMMU_F_DOMAIN_RANGE);
+ 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);
+
+ qemu_mutex_init(&s->mutex);
+}
+
+static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+ virtio_cleanup(vdev);
+}
+
+static void virtio_iommu_device_reset(VirtIODevice *vdev)
+{
+ trace_virtio_iommu_device_reset();
+}
+
+static void virtio_iommu_set_status(VirtIODevice *vdev, uint8_t status)
+{
+ trace_virtio_iommu_device_status(status);
+}
+
+static void virtio_iommu_instance_init(Object *obj)
+{
+}
+
+static const VMStateDescription vmstate_virtio_iommu = {
+ .name = "virtio-iommu",
+ .minimum_version_id = 1,
+ .version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_VIRTIO_DEVICE,
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static Property virtio_iommu_properties[] = {
+ DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus, "PCI", PCIBus *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_iommu_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+ dc->props = virtio_iommu_properties;
+ dc->vmsd = &vmstate_virtio_iommu;
+
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+ vdc->realize = virtio_iommu_device_realize;
+ vdc->unrealize = virtio_iommu_device_unrealize;
+ vdc->reset = virtio_iommu_device_reset;
+ vdc->get_config = virtio_iommu_get_config;
+ vdc->set_config = virtio_iommu_set_config;
+ vdc->get_features = virtio_iommu_get_features;
+ vdc->set_status = virtio_iommu_set_status;
+ vdc->vmsd = &vmstate_virtio_iommu_device;
+}
+
+static const TypeInfo virtio_iommu_info = {
+ .name = TYPE_VIRTIO_IOMMU,
+ .parent = TYPE_VIRTIO_DEVICE,
+ .instance_size = sizeof(VirtIOIOMMU),
+ .instance_init = virtio_iommu_instance_init,
+ .class_init = virtio_iommu_class_init,
+};
+
+static void virtio_register_types(void)
+{
+ type_register_static(&virtio_iommu_info);
+}
+
+type_init(virtio_register_types)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
new file mode 100644
index 0000000000..041f2c9390
--- /dev/null
+++ b/include/hw/virtio/virtio-iommu.h
@@ -0,0 +1,57 @@
+/*
+ * virtio-iommu device
+ *
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QEMU_VIRTIO_IOMMU_H
+#define QEMU_VIRTIO_IOMMU_H
+
+#include "standard-headers/linux/virtio_iommu.h"
+#include "hw/virtio/virtio.h"
+#include "hw/pci/pci.h"
+
+#define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
+#define VIRTIO_IOMMU(obj) \
+ OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
+
+typedef struct IOMMUDevice {
+ void *viommu;
+ PCIBus *bus;
+ int devfn;
+ IOMMUMemoryRegion iommu_mr;
+ AddressSpace as;
+} IOMMUDevice;
+
+typedef struct IOMMUPciBus {
+ PCIBus *bus;
+ IOMMUDevice *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
+} IOMMUPciBus;
+
+typedef struct VirtIOIOMMU {
+ VirtIODevice parent_obj;
+ VirtQueue *req_vq;
+ VirtQueue *event_vq;
+ struct virtio_iommu_config config;
+ uint64_t features;
+ GHashTable *as_by_busptr;
+ PCIBus *primary_bus;
+ GTree *domains;
+ QemuMutex mutex;
+ GTree *endpoints;
+} VirtIOIOMMU;
+
+#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 02/10] virtio-iommu: Decode the command payload
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
2020-01-25 17:19 ` [PATCH v13 01/10] virtio-iommu: Add skeleton Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-01-25 17:19 ` [PATCH v13 03/10] virtio-iommu: Implement attach/detach command Eric Auger
` (8 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This patch adds the command payload decoding and
introduces the functions that will do the actual
command handling. Those functions are not yet implemented.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
v11 -> v12:
- ADded Jean and Peter's R-b
v10 -> v11:
- use a macro for handle command functions
v9 -> v10:
- make virtio_iommu_handle_* more compact and
remove get_payload_size
v7 -> v8:
- handle new domain parameter in detach
- remove reserved checks
v5 -> v6:
- change map/unmap semantics (remove size)
v4 -> v5:
- adopt new v0.5 terminology
v3 -> v4:
- no flags field anymore in struct virtio_iommu_req_unmap
- test reserved on attach/detach, change trace proto
- rebase on v2.10.0.
---
hw/virtio/trace-events | 4 +++
hw/virtio/virtio-iommu.c | 76 +++++++++++++++++++++++++++++++++-------
2 files changed, 68 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 02d93d7f63..f7141aa2f6 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -60,3 +60,7 @@ virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx6
virtio_iommu_device_status(uint8_t status) "driver status = %d"
virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x"
virtio_iommu_set_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x"
+virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
+virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
+virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
+virtio_iommu_unmap(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 7e55eda67e..9d1b997df7 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -34,31 +34,83 @@
/* Max size */
#define VIOMMU_DEFAULT_QUEUE_SIZE 256
-static int virtio_iommu_handle_attach(VirtIOIOMMU *s,
- struct iovec *iov,
- unsigned int iov_cnt)
+static int virtio_iommu_attach(VirtIOIOMMU *s,
+ struct virtio_iommu_req_attach *req)
{
+ uint32_t domain_id = le32_to_cpu(req->domain);
+ uint32_t ep_id = le32_to_cpu(req->endpoint);
+
+ trace_virtio_iommu_attach(domain_id, ep_id);
+
return VIRTIO_IOMMU_S_UNSUPP;
}
-static int virtio_iommu_handle_detach(VirtIOIOMMU *s,
- struct iovec *iov,
- unsigned int iov_cnt)
+
+static int virtio_iommu_detach(VirtIOIOMMU *s,
+ struct virtio_iommu_req_detach *req)
{
+ uint32_t domain_id = le32_to_cpu(req->domain);
+ uint32_t ep_id = le32_to_cpu(req->endpoint);
+
+ trace_virtio_iommu_detach(domain_id, ep_id);
+
return VIRTIO_IOMMU_S_UNSUPP;
}
-static int virtio_iommu_handle_map(VirtIOIOMMU *s,
- struct iovec *iov,
- unsigned int iov_cnt)
+
+static int virtio_iommu_map(VirtIOIOMMU *s,
+ struct virtio_iommu_req_map *req)
{
+ uint32_t domain_id = le32_to_cpu(req->domain);
+ uint64_t phys_start = le64_to_cpu(req->phys_start);
+ uint64_t virt_start = le64_to_cpu(req->virt_start);
+ uint64_t virt_end = le64_to_cpu(req->virt_end);
+ uint32_t flags = le32_to_cpu(req->flags);
+
+ trace_virtio_iommu_map(domain_id, virt_start, virt_end, phys_start, flags);
+
return VIRTIO_IOMMU_S_UNSUPP;
}
-static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
- struct iovec *iov,
- unsigned int iov_cnt)
+
+static int virtio_iommu_unmap(VirtIOIOMMU *s,
+ struct virtio_iommu_req_unmap *req)
{
+ uint32_t domain_id = le32_to_cpu(req->domain);
+ uint64_t virt_start = le64_to_cpu(req->virt_start);
+ uint64_t virt_end = le64_to_cpu(req->virt_end);
+
+ trace_virtio_iommu_unmap(domain_id, virt_start, virt_end);
+
return VIRTIO_IOMMU_S_UNSUPP;
}
+static int virtio_iommu_iov_to_req(struct iovec *iov,
+ unsigned int iov_cnt,
+ void *req, size_t req_sz)
+{
+ size_t sz, payload_sz = req_sz - sizeof(struct virtio_iommu_req_tail);
+
+ sz = iov_to_buf(iov, iov_cnt, 0, req, payload_sz);
+ if (unlikely(sz != payload_sz)) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+ return 0;
+}
+
+#define virtio_iommu_handle_req(__req) \
+static int virtio_iommu_handle_ ## __req(VirtIOIOMMU *s, \
+ struct iovec *iov, \
+ unsigned int iov_cnt) \
+{ \
+ struct virtio_iommu_req_ ## __req req; \
+ int ret = virtio_iommu_iov_to_req(iov, iov_cnt, &req, sizeof(req)); \
+ \
+ return ret ? ret : virtio_iommu_ ## __req(s, &req); \
+}
+
+virtio_iommu_handle_req(attach)
+virtio_iommu_handle_req(detach)
+virtio_iommu_handle_req(map)
+virtio_iommu_handle_req(unmap)
+
static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
2020-01-25 17:19 ` [PATCH v13 01/10] virtio-iommu: Add skeleton Eric Auger
2020-01-25 17:19 ` [PATCH v13 02/10] virtio-iommu: Decode the command payload Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-02-03 13:49 ` Peter Xu
2020-01-25 17:19 ` [PATCH v13 04/10] virtio-iommu: Implement map/unmap Eric Auger
` (7 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This patch implements the endpoint attach/detach to/from
a domain.
Domain and endpoint internal datatypes are introduced.
Both are stored in RB trees. The domain owns a list of
endpoints attached to it. Also helpers to get/put
end points and domains are introduced.
As for the IOMMU memory regions, a callback is called on
PCI bus enumeration that initializes for a given device
on the bus hierarchy an IOMMU memory region. The PCI bus
hierarchy is stored locally in IOMMUPciBus and IOMMUDevice
objects.
At the time of the enumeration, the bus number may not be
computed yet.
So operations that will need to retrieve the IOMMUdevice
and its IOMMU memory region from the bus number and devfn,
once the bus number is garanteed to be frozen, use an array
of IOMMUPciBus, lazily populated.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v12 -> v13:
- squashed v12 4, 5, 6 into this patch
- rename virtio_iommu_get_sid into virtio_iommu_get_bdf
v11 -> v12:
- check the device is protected by the iommu on attach
- on detach, check the domain id the device is attached to matches
the one used in the detach command
- fix mapping ref counter and destroy the domain when no end-points
are attached anymore
---
hw/virtio/trace-events | 6 +
hw/virtio/virtio-iommu.c | 315 ++++++++++++++++++++++++++++++-
include/hw/virtio/virtio-iommu.h | 3 +
3 files changed, 322 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index f7141aa2f6..15595f8cd7 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -64,3 +64,9 @@ virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
virtio_iommu_unmap(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
+virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
+virtio_iommu_init_iommu_mr(char *iommu_mr) "init %s"
+virtio_iommu_get_endpoint(uint32_t ep_id) "Alloc endpoint=%d"
+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"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 9d1b997df7..e5cc94138b 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -23,6 +23,8 @@
#include "hw/qdev-properties.h"
#include "hw/virtio/virtio.h"
#include "sysemu/kvm.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
#include "trace.h"
#include "standard-headers/linux/virtio_ids.h"
@@ -30,19 +32,234 @@
#include "hw/virtio/virtio-bus.h"
#include "hw/virtio/virtio-access.h"
#include "hw/virtio/virtio-iommu.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci.h"
/* Max size */
#define VIOMMU_DEFAULT_QUEUE_SIZE 256
+typedef struct VirtIOIOMMUDomain {
+ uint32_t id;
+ GTree *mappings;
+ QLIST_HEAD(, VirtIOIOMMUEndpoint) endpoint_list;
+} VirtIOIOMMUDomain;
+
+typedef struct VirtIOIOMMUEndpoint {
+ uint32_t id;
+ VirtIOIOMMUDomain *domain;
+ QLIST_ENTRY(VirtIOIOMMUEndpoint) next;
+} VirtIOIOMMUEndpoint;
+
+typedef struct VirtIOIOMMUInterval {
+ uint64_t low;
+ uint64_t high;
+} VirtIOIOMMUInterval;
+
+static inline uint16_t virtio_iommu_get_bdf(IOMMUDevice *dev)
+{
+ return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
+}
+
+/**
+ * The bus number is used for lookup when SID based operations occur.
+ * In that case we lazily populate the IOMMUPciBus array from the bus hash
+ * table. At the time the IOMMUPciBus is created (iommu_find_add_as), the bus
+ * numbers may not be always initialized yet.
+ */
+static IOMMUPciBus *iommu_find_iommu_pcibus(VirtIOIOMMU *s, uint8_t bus_num)
+{
+ IOMMUPciBus *iommu_pci_bus = s->iommu_pcibus_by_bus_num[bus_num];
+
+ if (!iommu_pci_bus) {
+ GHashTableIter iter;
+
+ g_hash_table_iter_init(&iter, s->as_by_busptr);
+ while (g_hash_table_iter_next(&iter, NULL, (void **)&iommu_pci_bus)) {
+ if (pci_bus_num(iommu_pci_bus->bus) == bus_num) {
+ s->iommu_pcibus_by_bus_num[bus_num] = iommu_pci_bus;
+ return iommu_pci_bus;
+ }
+ }
+ return NULL;
+ }
+ return iommu_pci_bus;
+}
+
+static IOMMUMemoryRegion *virtio_iommu_mr(VirtIOIOMMU *s, uint32_t sid)
+{
+ uint8_t bus_n, devfn;
+ IOMMUPciBus *iommu_pci_bus;
+ IOMMUDevice *dev;
+
+ bus_n = PCI_BUS_NUM(sid);
+ iommu_pci_bus = iommu_find_iommu_pcibus(s, bus_n);
+ if (iommu_pci_bus) {
+ devfn = sid & PCI_DEVFN_MAX;
+ dev = iommu_pci_bus->pbdev[devfn];
+ if (dev) {
+ return &dev->iommu_mr;
+ }
+ }
+ return NULL;
+}
+
+static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+ VirtIOIOMMUInterval *inta = (VirtIOIOMMUInterval *)a;
+ VirtIOIOMMUInterval *intb = (VirtIOIOMMUInterval *)b;
+
+ if (inta->high < intb->low) {
+ return -1;
+ } else if (intb->high < inta->low) {
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
+static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
+{
+ QLIST_REMOVE(ep, next);
+ g_tree_unref(ep->domain->mappings);
+ ep->domain = NULL;
+}
+
+static VirtIOIOMMUEndpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
+ uint32_t ep_id)
+{
+ VirtIOIOMMUEndpoint *ep;
+
+ ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
+ if (ep) {
+ return ep;
+ }
+ if (!virtio_iommu_mr(s, ep_id)) {
+ return NULL;
+ }
+ ep = g_malloc0(sizeof(*ep));
+ ep->id = ep_id;
+ trace_virtio_iommu_get_endpoint(ep_id);
+ g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
+ return ep;
+}
+
+static void virtio_iommu_put_endpoint(gpointer data)
+{
+ VirtIOIOMMUEndpoint *ep = (VirtIOIOMMUEndpoint *)data;
+
+ assert(!ep->domain);
+
+ trace_virtio_iommu_put_endpoint(ep->id);
+ g_free(ep);
+}
+
+static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
+ uint32_t domain_id)
+{
+ VirtIOIOMMUDomain *domain;
+
+ domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
+ if (domain) {
+ return domain;
+ }
+ domain = g_malloc0(sizeof(*domain));
+ domain->id = domain_id;
+ domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
+ NULL, (GDestroyNotify)g_free,
+ (GDestroyNotify)g_free);
+ g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain);
+ QLIST_INIT(&domain->endpoint_list);
+ trace_virtio_iommu_get_domain(domain_id);
+ return domain;
+}
+
+static void virtio_iommu_put_domain(gpointer data)
+{
+ VirtIOIOMMUDomain *domain = (VirtIOIOMMUDomain *)data;
+ VirtIOIOMMUEndpoint *iter, *tmp;
+
+ QLIST_FOREACH_SAFE(iter, &domain->endpoint_list, next, tmp) {
+ virtio_iommu_detach_endpoint_from_domain(iter);
+ }
+ trace_virtio_iommu_put_domain(domain->id);
+ g_free(domain);
+}
+
+static AddressSpace *virtio_iommu_find_add_as(PCIBus *bus, void *opaque,
+ int devfn)
+{
+ VirtIOIOMMU *s = opaque;
+ IOMMUPciBus *sbus = g_hash_table_lookup(s->as_by_busptr, bus);
+ static uint32_t mr_index;
+ IOMMUDevice *sdev;
+
+ if (!sbus) {
+ sbus = g_malloc0(sizeof(IOMMUPciBus) +
+ sizeof(IOMMUDevice *) * PCI_DEVFN_MAX);
+ sbus->bus = bus;
+ g_hash_table_insert(s->as_by_busptr, bus, sbus);
+ }
+
+ sdev = sbus->pbdev[devfn];
+ if (!sdev) {
+ char *name = g_strdup_printf("%s-%d-%d",
+ TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+ mr_index++, devfn);
+ sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
+
+ sdev->viommu = s;
+ sdev->bus = bus;
+ sdev->devfn = devfn;
+
+ trace_virtio_iommu_init_iommu_mr(name);
+
+ memory_region_init_iommu(&sdev->iommu_mr, sizeof(sdev->iommu_mr),
+ TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+ OBJECT(s), name,
+ UINT64_MAX);
+ address_space_init(&sdev->as,
+ MEMORY_REGION(&sdev->iommu_mr), TYPE_VIRTIO_IOMMU);
+ g_free(name);
+ }
+ return &sdev->as;
+}
+
static int virtio_iommu_attach(VirtIOIOMMU *s,
struct virtio_iommu_req_attach *req)
{
uint32_t domain_id = le32_to_cpu(req->domain);
uint32_t ep_id = le32_to_cpu(req->endpoint);
+ VirtIOIOMMUDomain *domain;
+ VirtIOIOMMUEndpoint *ep;
trace_virtio_iommu_attach(domain_id, ep_id);
- return VIRTIO_IOMMU_S_UNSUPP;
+ ep = virtio_iommu_get_endpoint(s, ep_id);
+ if (!ep) {
+ return VIRTIO_IOMMU_S_NOENT;
+ }
+
+ if (ep->domain) {
+ VirtIOIOMMUDomain *previous_domain = ep->domain;
+ /*
+ * the device is already attached to a domain,
+ * detach it first
+ */
+ virtio_iommu_detach_endpoint_from_domain(ep);
+ if (QLIST_EMPTY(&previous_domain->endpoint_list)) {
+ g_tree_remove(s->domains, GUINT_TO_POINTER(previous_domain->id));
+ }
+ }
+
+ domain = virtio_iommu_get_domain(s, domain_id);
+ if (!QLIST_EMPTY(&domain->endpoint_list)) {
+ g_tree_ref(domain->mappings);
+ }
+ QLIST_INSERT_HEAD(&domain->endpoint_list, ep, next);
+
+ ep->domain = domain;
+
+ return VIRTIO_IOMMU_S_OK;
}
static int virtio_iommu_detach(VirtIOIOMMU *s,
@@ -50,10 +267,34 @@ static int virtio_iommu_detach(VirtIOIOMMU *s,
{
uint32_t domain_id = le32_to_cpu(req->domain);
uint32_t ep_id = le32_to_cpu(req->endpoint);
+ VirtIOIOMMUDomain *domain;
+ VirtIOIOMMUEndpoint *ep;
trace_virtio_iommu_detach(domain_id, ep_id);
- return VIRTIO_IOMMU_S_UNSUPP;
+ ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
+ if (!ep) {
+ return VIRTIO_IOMMU_S_NOENT;
+ }
+
+ domain = ep->domain;
+
+ if (!domain || domain->id != domain_id) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
+ virtio_iommu_detach_endpoint_from_domain(ep);
+
+ /*
+ * when the last EP is detached, simply remove the domain for
+ * the domain list and destroy it. Note its mappings were already
+ * freed by the ref count mechanism. Next operation involving
+ * the same domain id will re-create one domain object.
+ */
+ if (QLIST_EMPTY(&domain->endpoint_list)) {
+ g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
+ }
+ return VIRTIO_IOMMU_S_OK;
}
static int virtio_iommu_map(VirtIOIOMMU *s,
@@ -172,6 +413,27 @@ out:
}
}
+static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
+ IOMMUAccessFlags flag,
+ int iommu_idx)
+{
+ IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+ uint32_t sid;
+
+ IOMMUTLBEntry entry = {
+ .target_as = &address_space_memory,
+ .iova = addr,
+ .translated_addr = addr,
+ .addr_mask = ~(hwaddr)0,
+ .perm = IOMMU_NONE,
+ };
+
+ sid = virtio_iommu_get_bdf(sdev);
+
+ trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
+ return entry;
+}
+
static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
{
VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
@@ -218,6 +480,13 @@ static const VMStateDescription vmstate_virtio_iommu_device = {
.unmigratable = 1,
};
+static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
+{
+ guint ua = GPOINTER_TO_UINT(a);
+ guint ub = GPOINTER_TO_UINT(b);
+ return (ua > ub) - (ua < ub);
+}
+
static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -226,6 +495,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
sizeof(struct virtio_iommu_config));
+ memset(s->iommu_pcibus_by_bus_num, 0, sizeof(s->iommu_pcibus_by_bus_num));
+
s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
virtio_iommu_handle_command);
s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
@@ -244,18 +515,43 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MMIO);
qemu_mutex_init(&s->mutex);
+
+ s->as_by_busptr = g_hash_table_new_full(NULL, NULL, NULL, g_free);
+
+ if (s->primary_bus) {
+ pci_setup_iommu(s->primary_bus, virtio_iommu_find_add_as, s);
+ } else {
+ error_setg(errp, "VIRTIO-IOMMU is not attached to any PCI bus!");
+ }
}
static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
+
+ g_tree_destroy(s->domains);
+ g_tree_destroy(s->endpoints);
virtio_cleanup(vdev);
}
static void virtio_iommu_device_reset(VirtIODevice *vdev)
{
+ VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
+
trace_virtio_iommu_device_reset();
+
+ if (s->domains) {
+ g_tree_destroy(s->domains);
+ }
+ if (s->endpoints) {
+ g_tree_destroy(s->endpoints);
+ }
+ s->domains = g_tree_new_full((GCompareDataFunc)int_cmp,
+ NULL, NULL, virtio_iommu_put_domain);
+ s->endpoints = g_tree_new_full((GCompareDataFunc)int_cmp,
+ NULL, NULL, virtio_iommu_put_endpoint);
}
static void virtio_iommu_set_status(VirtIODevice *vdev, uint8_t status)
@@ -301,6 +597,14 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
vdc->vmsd = &vmstate_virtio_iommu_device;
}
+static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
+ void *data)
+{
+ IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
+
+ imrc->translate = virtio_iommu_translate;
+}
+
static const TypeInfo virtio_iommu_info = {
.name = TYPE_VIRTIO_IOMMU,
.parent = TYPE_VIRTIO_DEVICE,
@@ -309,9 +613,16 @@ static const TypeInfo virtio_iommu_info = {
.class_init = virtio_iommu_class_init,
};
+static const TypeInfo virtio_iommu_memory_region_info = {
+ .parent = TYPE_IOMMU_MEMORY_REGION,
+ .name = TYPE_VIRTIO_IOMMU_MEMORY_REGION,
+ .class_init = virtio_iommu_memory_region_class_init,
+};
+
static void virtio_register_types(void)
{
type_register_static(&virtio_iommu_info);
+ type_register_static(&virtio_iommu_memory_region_info);
}
type_init(virtio_register_types)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 041f2c9390..2a2c2ecf83 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -28,6 +28,8 @@
#define VIRTIO_IOMMU(obj) \
OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
+#define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region"
+
typedef struct IOMMUDevice {
void *viommu;
PCIBus *bus;
@@ -48,6 +50,7 @@ typedef struct VirtIOIOMMU {
struct virtio_iommu_config config;
uint64_t features;
GHashTable *as_by_busptr;
+ IOMMUPciBus *iommu_pcibus_by_bus_num[PCI_BUS_MAX];
PCIBus *primary_bus;
GTree *domains;
QemuMutex mutex;
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 04/10] virtio-iommu: Implement map/unmap
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (2 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 03/10] virtio-iommu: Implement attach/detach command Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-01-25 17:19 ` [PATCH v13 05/10] virtio-iommu: Implement translate Eric Auger
` (6 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This patch implements virtio_iommu_map/unmap.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
v11 -> v12:
- check unmanaged managed flags on map
- removed 2 qemu_log_mask in unmap()
- fix leak
v10 -> v11:
- revisit the implementation of unmap according to Peter's suggestion
- removed virt_addr and size from viommu_mapping struct
- use g_tree_lookup_extended()
- return VIRTIO_IOMMU_S_RANGE in case a mapping were
to be split on unmap (instead of INVAL)
v5 -> v6:
- use new v0.6 fields
- replace error_report by qemu_log_mask
v3 -> v4:
- implement unmap semantics as specified in v0.4
---
hw/virtio/trace-events | 1 +
hw/virtio/virtio-iommu.c | 63 ++++++++++++++++++++++++++++++++++++++--
2 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 15595f8cd7..22162d6583 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -64,6 +64,7 @@ virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
virtio_iommu_unmap(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
+virtio_iommu_unmap_done(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
virtio_iommu_init_iommu_mr(char *iommu_mr) "init %s"
virtio_iommu_get_endpoint(uint32_t ep_id) "Alloc endpoint=%d"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index e5cc94138b..e1e0c69473 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/log.h"
#include "qemu/iov.h"
#include "qemu-common.h"
#include "hw/qdev-properties.h"
@@ -55,6 +56,11 @@ typedef struct VirtIOIOMMUInterval {
uint64_t high;
} VirtIOIOMMUInterval;
+typedef struct VirtIOIOMMUMapping {
+ uint64_t phys_addr;
+ uint32_t flags;
+} VirtIOIOMMUMapping;
+
static inline uint16_t virtio_iommu_get_bdf(IOMMUDevice *dev)
{
return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
@@ -305,10 +311,39 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
uint64_t virt_start = le64_to_cpu(req->virt_start);
uint64_t virt_end = le64_to_cpu(req->virt_end);
uint32_t flags = le32_to_cpu(req->flags);
+ VirtIOIOMMUDomain *domain;
+ VirtIOIOMMUInterval *interval;
+ VirtIOIOMMUMapping *mapping;
+
+ if (flags & ~VIRTIO_IOMMU_MAP_F_MASK) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
+ domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
+ if (!domain) {
+ return VIRTIO_IOMMU_S_NOENT;
+ }
+
+ interval = g_malloc0(sizeof(*interval));
+
+ interval->low = virt_start;
+ interval->high = virt_end;
+
+ mapping = g_tree_lookup(domain->mappings, (gpointer)interval);
+ if (mapping) {
+ g_free(interval);
+ return VIRTIO_IOMMU_S_INVAL;
+ }
trace_virtio_iommu_map(domain_id, virt_start, virt_end, phys_start, flags);
- return VIRTIO_IOMMU_S_UNSUPP;
+ mapping = g_malloc0(sizeof(*mapping));
+ mapping->phys_addr = phys_start;
+ mapping->flags = flags;
+
+ g_tree_insert(domain->mappings, interval, mapping);
+
+ return VIRTIO_IOMMU_S_OK;
}
static int virtio_iommu_unmap(VirtIOIOMMU *s,
@@ -317,10 +352,34 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
uint32_t domain_id = le32_to_cpu(req->domain);
uint64_t virt_start = le64_to_cpu(req->virt_start);
uint64_t virt_end = le64_to_cpu(req->virt_end);
+ VirtIOIOMMUMapping *iter_val;
+ VirtIOIOMMUInterval interval, *iter_key;
+ VirtIOIOMMUDomain *domain;
+ int ret = VIRTIO_IOMMU_S_OK;
trace_virtio_iommu_unmap(domain_id, virt_start, virt_end);
- return VIRTIO_IOMMU_S_UNSUPP;
+ domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
+ if (!domain) {
+ return VIRTIO_IOMMU_S_NOENT;
+ }
+ interval.low = virt_start;
+ interval.high = virt_end;
+
+ while (g_tree_lookup_extended(domain->mappings, &interval,
+ (void **)&iter_key, (void**)&iter_val)) {
+ uint64_t current_low = iter_key->low;
+ uint64_t current_high = iter_key->high;
+
+ if (interval.low <= current_low && interval.high >= current_high) {
+ g_tree_remove(domain->mappings, iter_key);
+ trace_virtio_iommu_unmap_done(domain_id, current_low, current_high);
+ } else {
+ ret = VIRTIO_IOMMU_S_RANGE;
+ break;
+ }
+ }
+ return ret;
}
static int virtio_iommu_iov_to_req(struct iovec *iov,
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 05/10] virtio-iommu: Implement translate
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (3 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 04/10] virtio-iommu: Implement map/unmap Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-01-25 17:19 ` [PATCH v13 06/10] virtio-iommu: Implement fault reporting Eric Auger
` (5 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This patch implements the translate callback
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
v11 -> v12:
- Added Jean's R-b
- s/qemu_log_mask/error_report_once
v10 -> v11:
- take into account the new value struct and use
g_tree_lookup_extended
- switched to error_report_once
v6 -> v7:
- implemented bypass-mode
v5 -> v6:
- replace error_report by qemu_log_mask
v4 -> v5:
- check the device domain is not NULL
- s/printf/error_report
- set flags to IOMMU_NONE in case of all translation faults
---
hw/virtio/trace-events | 1 +
hw/virtio/virtio-iommu.c | 60 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 22162d6583..095aa8b509 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -71,3 +71,4 @@ virtio_iommu_get_endpoint(uint32_t ep_id) "Alloc endpoint=%d"
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"
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index e1e0c69473..cc0715284f 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -477,19 +477,77 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
int iommu_idx)
{
IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
+ VirtIOIOMMUInterval interval, *mapping_key;
+ VirtIOIOMMUMapping *mapping_value;
+ VirtIOIOMMU *s = sdev->viommu;
+ VirtIOIOMMUEndpoint *ep;
+ bool bypass_allowed;
uint32_t sid;
+ bool found;
+
+ interval.low = addr;
+ interval.high = addr + 1;
IOMMUTLBEntry entry = {
.target_as = &address_space_memory,
.iova = addr,
.translated_addr = addr,
- .addr_mask = ~(hwaddr)0,
+ .addr_mask = (1 << ctz32(s->config.page_size_mask)) - 1,
.perm = IOMMU_NONE,
};
+ bypass_allowed = virtio_vdev_has_feature(&s->parent_obj,
+ VIRTIO_IOMMU_F_BYPASS);
+
sid = virtio_iommu_get_bdf(sdev);
trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
+ qemu_mutex_lock(&s->mutex);
+
+ ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(sid));
+ if (!ep) {
+ if (!bypass_allowed) {
+ error_report_once("%s sid=%d is not known!!", __func__, sid);
+ } else {
+ entry.perm = flag;
+ }
+ goto unlock;
+ }
+
+ if (!ep->domain) {
+ if (!bypass_allowed) {
+ error_report_once("%s %02x:%02x.%01x not attached to any domain",
+ __func__, PCI_BUS_NUM(sid),
+ PCI_SLOT(sid), PCI_FUNC(sid));
+ } else {
+ entry.perm = flag;
+ }
+ goto unlock;
+ }
+
+ found = g_tree_lookup_extended(ep->domain->mappings, (gpointer)(&interval),
+ (void **)&mapping_key,
+ (void **)&mapping_value);
+ if (!found) {
+ error_report_once("%s no mapping for 0x%"PRIx64" for sid=%d",
+ __func__, addr, sid);
+ 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))) {
+ error_report_once("%s permission error on 0x%"PRIx64"(%d): allowed=%d",
+ __func__, addr, flag, mapping_value->flags);
+ goto unlock;
+ }
+ entry.translated_addr = addr - mapping_key->low + mapping_value->phys_addr;
+ entry.perm = flag;
+ trace_virtio_iommu_translate_out(addr, entry.translated_addr, sid);
+
+unlock:
+ qemu_mutex_unlock(&s->mutex);
return entry;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 06/10] virtio-iommu: Implement fault reporting
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (4 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 05/10] virtio-iommu: Implement translate Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-02-03 13:55 ` Peter Xu
2020-01-25 17:19 ` [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support Eric Auger
` (4 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (5 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 06/10] virtio-iommu: Implement fault reporting Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-02-03 13:03 ` Michael S. Tsirkin
2020-01-25 17:19 ` [PATCH v13 08/10] hw/arm/virt: Add the virtio-iommu device tree mappings Eric Auger
` (3 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This patch adds virtio-iommu-pci, which is the pci proxy for
the virtio-iommu device.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
v11 -> v12:
- added Jean's R-b
- remove the array of intervals. Will be introduced later?
v10 -> v11:
- add the reserved_regions array property
v9 -> v10:
- include "hw/qdev-properties.h" header
v8 -> v9:
- add the msi-bypass property
- create virtio-iommu-pci.c
---
hw/virtio/Makefile.objs | 1 +
hw/virtio/virtio-iommu-pci.c | 88 ++++++++++++++++++++++++++++++++
include/hw/pci/pci.h | 1 +
include/hw/virtio/virtio-iommu.h | 1 +
qdev-monitor.c | 1 +
5 files changed, 92 insertions(+)
create mode 100644 hw/virtio/virtio-iommu-pci.c
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 2fd9da7410..4e4d39a0a4 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -29,6 +29,7 @@ obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
obj-$(CONFIG_VIRTIO_BALLOON) += virtio-balloon-pci.o
+obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu-pci.o
obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
new file mode 100644
index 0000000000..4cfae1f9df
--- /dev/null
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -0,0 +1,88 @@
+/*
+ * Virtio IOMMU PCI Bindings
+ *
+ * Copyright (c) 2019 Red Hat, Inc.
+ * Written by Eric Auger
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+
+#include "virtio-pci.h"
+#include "hw/virtio/virtio-iommu.h"
+#include "hw/qdev-properties.h"
+
+typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
+
+/*
+ * virtio-iommu-pci: This extends VirtioPCIProxy.
+ *
+ */
+#define VIRTIO_IOMMU_PCI(obj) \
+ OBJECT_CHECK(VirtIOIOMMUPCI, (obj), TYPE_VIRTIO_IOMMU_PCI)
+
+struct VirtIOIOMMUPCI {
+ VirtIOPCIProxy parent_obj;
+ VirtIOIOMMU vdev;
+};
+
+static Property virtio_iommu_pci_properties[] = {
+ DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+ VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
+ DeviceState *vdev = DEVICE(&dev->vdev);
+
+ qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+ object_property_set_link(OBJECT(dev),
+ OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
+ "primary-bus", errp);
+ object_property_set_bool(OBJECT(vdev), true, "realized", errp);
+}
+
+static void virtio_iommu_pci_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+ PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+ k->realize = virtio_iommu_pci_realize;
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+ dc->props = virtio_iommu_pci_properties;
+ pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+ pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
+ pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
+ pcidev_k->class_id = PCI_CLASS_OTHERS;
+}
+
+static void virtio_iommu_pci_instance_init(Object *obj)
+{
+ VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
+
+ virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+ TYPE_VIRTIO_IOMMU);
+}
+
+static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
+ .base_name = TYPE_VIRTIO_IOMMU_PCI,
+ .generic_name = "virtio-iommu-pci",
+ .transitional_name = "virtio-iommu-pci-transitional",
+ .non_transitional_name = "virtio-iommu-pci-non-transitional",
+ .instance_size = sizeof(VirtIOIOMMUPCI),
+ .instance_init = virtio_iommu_pci_instance_init,
+ .class_init = virtio_iommu_pci_class_init,
+};
+
+static void virtio_iommu_pci_register(void)
+{
+ virtio_pci_types_register(&virtio_iommu_pci_info);
+}
+
+type_init(virtio_iommu_pci_register)
+
+
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 2acd8321af..cfedf5a995 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -86,6 +86,7 @@ extern bool pci_available;
#define PCI_DEVICE_ID_VIRTIO_9P 0x1009
#define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
#define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013
+#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014
#define PCI_VENDOR_ID_REDHAT 0x1b36
#define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 2a2c2ecf83..f39aa0fbb4 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -25,6 +25,7 @@
#include "hw/pci/pci.h"
#define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
+#define TYPE_VIRTIO_IOMMU_PCI "virtio-iommu-device-base"
#define VIRTIO_IOMMU(obj) \
OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 3465a1e2d0..97f4022b51 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -66,6 +66,7 @@ static const QDevAlias qdev_alias_table[] = {
{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
{ "virtio-input-host-pci", "virtio-input-host",
QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+ { "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
{ "virtio-keyboard-pci", "virtio-keyboard",
QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 08/10] hw/arm/virt: Add the virtio-iommu device tree mappings
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (6 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-01-25 17:19 ` [PATCH v13 09/10] virtio-iommu: Support migration Eric Auger
` (2 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
Adds the "virtio,pci-iommu" node in the host bridge node and
the RID mapping, excluding the IOMMU RID.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
v11 -> v12:
- Added Jean's R-b
v10 -> v11:
- remove msi_bypass
v8 -> v9:
- disable msi-bypass property
- addition of the subnode is handled is the hotplug handler
and IOMMU RID is notimposed anymore
v6 -> v7:
- align to the smmu instantiation code
v4 -> v5:
- VirtMachineClass no_iommu added in this patch
- Use object_resolve_path_type
---
hw/arm/virt.c | 54 ++++++++++++++++++++++++++++++++++++-------
include/hw/arm/virt.h | 2 ++
2 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 656b0081c2..6cffae7024 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -32,6 +32,7 @@
#include "qemu-common.h"
#include "qemu/units.h"
#include "qemu/option.h"
+#include "monitor/qdev.h"
#include "qapi/error.h"
#include "hw/sysbus.h"
#include "hw/boards.h"
@@ -54,6 +55,7 @@
#include "qemu/error-report.h"
#include "qemu/module.h"
#include "hw/pci-host/gpex.h"
+#include "hw/virtio/virtio-pci.h"
#include "hw/arm/sysbus-fdt.h"
#include "hw/platform-bus.h"
#include "hw/qdev-properties.h"
@@ -71,6 +73,7 @@
#include "hw/mem/pc-dimm.h"
#include "hw/mem/nvdimm.h"
#include "hw/acpi/generic_event_device.h"
+#include "hw/virtio/virtio-iommu.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1180,6 +1183,30 @@ static void create_smmu(const VirtMachineState *vms,
g_free(node);
}
+static void create_virtio_iommu(VirtMachineState *vms, Error **errp)
+{
+ const char compat[] = "virtio,pci-iommu";
+ uint16_t bdf = vms->virtio_iommu_bdf;
+ char *node;
+
+ vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
+
+ node = g_strdup_printf("%s/virtio_iommu@%d", vms->pciehb_nodename, bdf);
+ qemu_fdt_add_subnode(vms->fdt, node);
+ qemu_fdt_setprop(vms->fdt, node, "compatible", compat, sizeof(compat));
+ qemu_fdt_setprop_sized_cells(vms->fdt, node, "reg",
+ 1, bdf << 8, 1, 0, 1, 0,
+ 1, 0, 1, 0);
+
+ qemu_fdt_setprop_cell(vms->fdt, node, "#iommu-cells", 1);
+ qemu_fdt_setprop_cell(vms->fdt, node, "phandle", vms->iommu_phandle);
+ g_free(node);
+
+ qemu_fdt_setprop_cells(vms->fdt, vms->pciehb_nodename, "iommu-map",
+ 0x0, vms->iommu_phandle, 0x0, bdf,
+ bdf + 1, vms->iommu_phandle, bdf + 1, 0xffff - bdf);
+}
+
static void create_pcie(VirtMachineState *vms)
{
hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base;
@@ -1258,7 +1285,7 @@ static void create_pcie(VirtMachineState *vms)
}
}
- nodename = g_strdup_printf("/pcie@%" PRIx64, base);
+ nodename = vms->pciehb_nodename = g_strdup_printf("/pcie@%" PRIx64, base);
qemu_fdt_add_subnode(vms->fdt, nodename);
qemu_fdt_setprop_string(vms->fdt, nodename,
"compatible", "pci-host-ecam-generic");
@@ -1301,13 +1328,16 @@ static void create_pcie(VirtMachineState *vms)
if (vms->iommu) {
vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
- create_smmu(vms, pci->bus);
-
- qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
- 0x0, vms->iommu_phandle, 0x0, 0x10000);
+ switch (vms->iommu) {
+ case VIRT_IOMMU_SMMUV3:
+ create_smmu(vms, pci->bus);
+ qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
+ 0x0, vms->iommu_phandle, 0x0, 0x10000);
+ break;
+ default:
+ g_assert_not_reached();
+ }
}
-
- g_free(nodename);
}
static void create_platform_bus(VirtMachineState *vms)
@@ -1971,6 +2001,13 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
virt_memory_plug(hotplug_dev, dev, errp);
}
+ if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+ PCIDevice *pdev = PCI_DEVICE(dev);
+
+ vms->iommu = VIRT_IOMMU_VIRTIO;
+ vms->virtio_iommu_bdf = pci_get_bdf(pdev);
+ create_virtio_iommu(vms, errp);
+ }
}
static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
@@ -1984,7 +2021,8 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
DeviceState *dev)
{
if (object_dynamic_cast(OBJECT(dev), TYPE_SYS_BUS_DEVICE) ||
- (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM))) {
+ (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) ||
+ (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI))) {
return HOTPLUG_HANDLER(machine);
}
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 38f0c33c77..165035fa8f 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -124,8 +124,10 @@ typedef struct {
bool virt;
int32_t gic_version;
VirtIOMMUType iommu;
+ uint16_t virtio_iommu_bdf;
struct arm_boot_info bootinfo;
MemMapEntry *memmap;
+ char *pciehb_nodename;
const int *irqmap;
int smp_cpus;
void *fdt;
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 09/10] virtio-iommu: Support migration
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (7 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 08/10] hw/arm/virt: Add the virtio-iommu device tree mappings Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-01-25 17:19 ` [PATCH v13 10/10] tests: Add virtio-iommu test Eric Auger
2020-02-03 12:58 ` [PATCH v13 00/10] VIRTIO-IOMMU device Auger Eric
10 siblings, 0 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
Add Migration support. We rely on recently added gtree and qlist
migration. We only migrate the domain gtree. The endpoint gtree
is re-constructed in a post-load operation.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
---
v11 -> v12:
- do not migrate the endpoint gtree but reconstruct it from the
domain gtree (Peter's suggestion)
- add MIG_PRI_IOMMU
---
hw/virtio/virtio-iommu.c | 109 +++++++++++++++++++++++++++++++++++----
1 file changed, 99 insertions(+), 10 deletions(-)
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index ec8f42e167..8f5c6f17df 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -650,16 +650,6 @@ static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
return f;
}
-/*
- * Migration is not yet supported: most of the state consists
- * of balanced binary trees which are not yet ready for getting
- * migrated
- */
-static const VMStateDescription vmstate_virtio_iommu_device = {
- .name = "virtio-iommu-device",
- .unmigratable = 1,
-};
-
static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
{
guint ua = GPOINTER_TO_UINT(a);
@@ -743,9 +733,108 @@ static void virtio_iommu_instance_init(Object *obj)
{
}
+#define VMSTATE_INTERVAL \
+{ \
+ .name = "interval", \
+ .version_id = 1, \
+ .minimum_version_id = 1, \
+ .fields = (VMStateField[]) { \
+ VMSTATE_UINT64(low, VirtIOIOMMUInterval), \
+ VMSTATE_UINT64(high, VirtIOIOMMUInterval), \
+ VMSTATE_END_OF_LIST() \
+ } \
+}
+
+#define VMSTATE_MAPPING \
+{ \
+ .name = "mapping", \
+ .version_id = 1, \
+ .minimum_version_id = 1, \
+ .fields = (VMStateField[]) { \
+ VMSTATE_UINT64(phys_addr, VirtIOIOMMUMapping),\
+ VMSTATE_UINT32(flags, VirtIOIOMMUMapping), \
+ VMSTATE_END_OF_LIST() \
+ }, \
+}
+
+static const VMStateDescription vmstate_interval_mapping[2] = {
+ VMSTATE_MAPPING, /* value */
+ VMSTATE_INTERVAL /* key */
+};
+
+static int domain_preload(void *opaque)
+{
+ VirtIOIOMMUDomain *domain = opaque;
+
+ domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
+ NULL, g_free, g_free);
+ return 0;
+}
+
+static const VMStateDescription vmstate_endpoint = {
+ .name = "endpoint",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(id, VirtIOIOMMUEndpoint),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static const VMStateDescription vmstate_domain = {
+ .name = "domain",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .pre_load = domain_preload,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(id, VirtIOIOMMUDomain),
+ VMSTATE_GTREE_V(mappings, VirtIOIOMMUDomain, 1,
+ vmstate_interval_mapping,
+ VirtIOIOMMUInterval, VirtIOIOMMUMapping),
+ VMSTATE_QLIST_V(endpoint_list, VirtIOIOMMUDomain, 1,
+ vmstate_endpoint, VirtIOIOMMUEndpoint, next),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static gboolean reconstruct_endpoints(gpointer key, gpointer value,
+ gpointer data)
+{
+ VirtIOIOMMU *s = (VirtIOIOMMU *)data;
+ VirtIOIOMMUDomain *d = (VirtIOIOMMUDomain *)value;
+ VirtIOIOMMUEndpoint *iter;
+
+ QLIST_FOREACH(iter, &d->endpoint_list, next) {
+ iter->domain = d;
+ g_tree_insert(s->endpoints, GUINT_TO_POINTER(iter->id), iter);
+ }
+ return false; /* continue the domain traversal */
+}
+
+static int iommu_post_load(void *opaque, int version_id)
+{
+ VirtIOIOMMU *s = opaque;
+
+ g_tree_foreach(s->domains, reconstruct_endpoints, s);
+ return 0;
+}
+
+static const VMStateDescription vmstate_virtio_iommu_device = {
+ .name = "virtio-iommu-device",
+ .minimum_version_id = 1,
+ .version_id = 1,
+ .post_load = iommu_post_load,
+ .fields = (VMStateField[]) {
+ VMSTATE_GTREE_DIRECT_KEY_V(domains, VirtIOIOMMU, 1,
+ &vmstate_domain, VirtIOIOMMUDomain),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
static const VMStateDescription vmstate_virtio_iommu = {
.name = "virtio-iommu",
.minimum_version_id = 1,
+ .priority = MIG_PRI_IOMMU,
.version_id = 1,
.fields = (VMStateField[]) {
VMSTATE_VIRTIO_DEVICE,
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v13 10/10] tests: Add virtio-iommu test
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (8 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 09/10] virtio-iommu: Support migration Eric Auger
@ 2020-01-25 17:19 ` Eric Auger
2020-02-03 12:58 ` [PATCH v13 00/10] VIRTIO-IOMMU device Auger Eric
10 siblings, 0 replies; 28+ messages in thread
From: Eric Auger @ 2020-01-25 17:19 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
This adds the framework to test the virtio-iommu-pci device
and tests exercising the attach/detach, map/unmap API.
To run the tests:
make tests/qtest/qos-test
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/qtest/qos-test V=1
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v12 -> v13
- remove probe_size test
- move to qtest directory
---
tests/qtest/Makefile.include | 2 +
tests/qtest/libqos/virtio-iommu.c | 177 +++++++++++++++++
tests/qtest/libqos/virtio-iommu.h | 45 +++++
tests/qtest/virtio-iommu-test.c | 306 ++++++++++++++++++++++++++++++
4 files changed, 530 insertions(+)
create mode 100644 tests/qtest/libqos/virtio-iommu.c
create mode 100644 tests/qtest/libqos/virtio-iommu.h
create mode 100644 tests/qtest/virtio-iommu-test.c
diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index e6bb4ab28c..fd6e8f43f7 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -189,6 +189,7 @@ qos-test-obj-y += tests/qtest/libqos/virtio-pci-modern.o
qos-test-obj-y += tests/qtest/libqos/virtio-rng.o
qos-test-obj-y += tests/qtest/libqos/virtio-scsi.o
qos-test-obj-y += tests/qtest/libqos/virtio-serial.o
+qos-test-obj-y += tests/qtest/libqos/virtio-iommu.o
# qos machines:
qos-test-obj-y += tests/qtest/libqos/aarch64-xlnx-zcu102-machine.o
@@ -228,6 +229,7 @@ qos-test-obj-y += tests/qtest/virtio-net-test.o
qos-test-obj-y += tests/qtest/virtio-rng-test.o
qos-test-obj-y += tests/qtest/virtio-scsi-test.o
qos-test-obj-y += tests/qtest/virtio-serial-test.o
+qos-test-obj-y += tests/qtest/virtio-iommu-test.o
qos-test-obj-y += tests/qtest/vmxnet3-test.o
check-unit-y += tests/test-qgraph$(EXESUF)
diff --git a/tests/qtest/libqos/virtio-iommu.c b/tests/qtest/libqos/virtio-iommu.c
new file mode 100644
index 0000000000..b4e9ea44fb
--- /dev/null
+++ b/tests/qtest/libqos/virtio-iommu.c
@@ -0,0 +1,177 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+#include "libqos/qgraph.h"
+#include "libqos/virtio-iommu.h"
+#include "hw/virtio/virtio-iommu.h"
+
+static QGuestAllocator *alloc;
+
+/* virtio-iommu-device */
+static void *qvirtio_iommu_get_driver(QVirtioIOMMU *v_iommu,
+ const char *interface)
+{
+ if (!g_strcmp0(interface, "virtio-iommu")) {
+ return v_iommu;
+ }
+ if (!g_strcmp0(interface, "virtio")) {
+ return v_iommu->vdev;
+ }
+
+ fprintf(stderr, "%s not present in virtio-iommu-device\n", interface);
+ g_assert_not_reached();
+}
+
+static void *qvirtio_iommu_device_get_driver(void *object,
+ const char *interface)
+{
+ QVirtioIOMMUDevice *v_iommu = object;
+ return qvirtio_iommu_get_driver(&v_iommu->iommu, interface);
+}
+
+static void virtio_iommu_cleanup(QVirtioIOMMU *interface)
+{
+ qvirtqueue_cleanup(interface->vdev->bus, interface->vq, alloc);
+}
+
+static void virtio_iommu_setup(QVirtioIOMMU *interface)
+{
+ QVirtioDevice *vdev = interface->vdev;
+ uint64_t features;
+
+ features = qvirtio_get_features(vdev);
+ features &= ~(QVIRTIO_F_BAD_FEATURE |
+ (1ull << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1ull << VIRTIO_RING_F_EVENT_IDX) |
+ (1ull << VIRTIO_IOMMU_F_BYPASS));
+ qvirtio_set_features(vdev, features);
+ interface->vq = qvirtqueue_setup(interface->vdev, alloc, 0);
+ qvirtio_set_driver_ok(interface->vdev);
+}
+
+static void qvirtio_iommu_device_destructor(QOSGraphObject *obj)
+{
+ QVirtioIOMMUDevice *v_iommu = (QVirtioIOMMUDevice *) obj;
+ QVirtioIOMMU *iommu = &v_iommu->iommu;
+
+ virtio_iommu_cleanup(iommu);
+}
+
+static void qvirtio_iommu_device_start_hw(QOSGraphObject *obj)
+{
+ QVirtioIOMMUDevice *v_iommu = (QVirtioIOMMUDevice *) obj;
+ QVirtioIOMMU *iommu = &v_iommu->iommu;
+
+ virtio_iommu_setup(iommu);
+}
+
+static void *virtio_iommu_device_create(void *virtio_dev,
+ QGuestAllocator *t_alloc,
+ void *addr)
+{
+ QVirtioIOMMUDevice *virtio_rdevice = g_new0(QVirtioIOMMUDevice, 1);
+ QVirtioIOMMU *interface = &virtio_rdevice->iommu;
+
+ interface->vdev = virtio_dev;
+ alloc = t_alloc;
+
+ virtio_rdevice->obj.get_driver = qvirtio_iommu_device_get_driver;
+ virtio_rdevice->obj.destructor = qvirtio_iommu_device_destructor;
+ virtio_rdevice->obj.start_hw = qvirtio_iommu_device_start_hw;
+
+ return &virtio_rdevice->obj;
+}
+
+/* virtio-iommu-pci */
+static void *qvirtio_iommu_pci_get_driver(void *object, const char *interface)
+{
+ QVirtioIOMMUPCI *v_iommu = object;
+ if (!g_strcmp0(interface, "pci-device")) {
+ return v_iommu->pci_vdev.pdev;
+ }
+ return qvirtio_iommu_get_driver(&v_iommu->iommu, interface);
+}
+
+static void qvirtio_iommu_pci_destructor(QOSGraphObject *obj)
+{
+ QVirtioIOMMUPCI *iommu_pci = (QVirtioIOMMUPCI *) obj;
+ QVirtioIOMMU *interface = &iommu_pci->iommu;
+ QOSGraphObject *pci_vobj = &iommu_pci->pci_vdev.obj;
+
+ virtio_iommu_cleanup(interface);
+ qvirtio_pci_destructor(pci_vobj);
+}
+
+static void qvirtio_iommu_pci_start_hw(QOSGraphObject *obj)
+{
+ QVirtioIOMMUPCI *iommu_pci = (QVirtioIOMMUPCI *) obj;
+ QVirtioIOMMU *interface = &iommu_pci->iommu;
+ QOSGraphObject *pci_vobj = &iommu_pci->pci_vdev.obj;
+
+ qvirtio_pci_start_hw(pci_vobj);
+ virtio_iommu_setup(interface);
+}
+
+
+static void *virtio_iommu_pci_create(void *pci_bus, QGuestAllocator *t_alloc,
+ void *addr)
+{
+ QVirtioIOMMUPCI *virtio_rpci = g_new0(QVirtioIOMMUPCI, 1);
+ QVirtioIOMMU *interface = &virtio_rpci->iommu;
+ QOSGraphObject *obj = &virtio_rpci->pci_vdev.obj;
+
+ virtio_pci_init(&virtio_rpci->pci_vdev, pci_bus, addr);
+ interface->vdev = &virtio_rpci->pci_vdev.vdev;
+ alloc = t_alloc;
+
+ obj->get_driver = qvirtio_iommu_pci_get_driver;
+ obj->start_hw = qvirtio_iommu_pci_start_hw;
+ obj->destructor = qvirtio_iommu_pci_destructor;
+
+ return obj;
+}
+
+static void virtio_iommu_register_nodes(void)
+{
+ QPCIAddress addr = {
+ .devfn = QPCI_DEVFN(4, 0),
+ };
+
+ QOSGraphEdgeOptions opts = {
+ .extra_device_opts = "addr=04.0",
+ };
+
+ /* virtio-iommu-device */
+ qos_node_create_driver("virtio-iommu-device", virtio_iommu_device_create);
+ qos_node_consumes("virtio-iommu-device", "virtio-bus", NULL);
+ qos_node_produces("virtio-iommu-device", "virtio");
+ qos_node_produces("virtio-iommu-device", "virtio-iommu");
+
+ /* virtio-iommu-pci */
+ add_qpci_address(&opts, &addr);
+ qos_node_create_driver("virtio-iommu-pci", virtio_iommu_pci_create);
+ qos_node_consumes("virtio-iommu-pci", "pci-bus", &opts);
+ qos_node_produces("virtio-iommu-pci", "pci-device");
+ qos_node_produces("virtio-iommu-pci", "virtio");
+ qos_node_produces("virtio-iommu-pci", "virtio-iommu");
+}
+
+libqos_init(virtio_iommu_register_nodes);
diff --git a/tests/qtest/libqos/virtio-iommu.h b/tests/qtest/libqos/virtio-iommu.h
new file mode 100644
index 0000000000..6970b45a01
--- /dev/null
+++ b/tests/qtest/libqos/virtio-iommu.h
@@ -0,0 +1,45 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef TESTS_LIBQOS_VIRTIO_IOMMU_H
+#define TESTS_LIBQOS_VIRTIO_IOMMU_H
+
+#include "libqos/qgraph.h"
+#include "libqos/virtio.h"
+#include "libqos/virtio-pci.h"
+
+typedef struct QVirtioIOMMU QVirtioIOMMU;
+typedef struct QVirtioIOMMUPCI QVirtioIOMMUPCI;
+typedef struct QVirtioIOMMUDevice QVirtioIOMMUDevice;
+
+struct QVirtioIOMMU {
+ QVirtioDevice *vdev;
+ QVirtQueue *vq;
+};
+
+struct QVirtioIOMMUPCI {
+ QVirtioPCIDevice pci_vdev;
+ QVirtioIOMMU iommu;
+};
+
+struct QVirtioIOMMUDevice {
+ QOSGraphObject obj;
+ QVirtioIOMMU iommu;
+};
+
+#endif
diff --git a/tests/qtest/virtio-iommu-test.c b/tests/qtest/virtio-iommu-test.c
new file mode 100644
index 0000000000..6948082075
--- /dev/null
+++ b/tests/qtest/virtio-iommu-test.c
@@ -0,0 +1,306 @@
+/*
+ * QTest testcase for VirtIO IOMMU
+ *
+ * Copyright (c) 2014 SUSE LINUX Products GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+#include "qemu/module.h"
+#include "libqos/qgraph.h"
+#include "libqos/virtio-iommu.h"
+#include "hw/virtio/virtio-iommu.h"
+
+#define PCI_SLOT_HP 0x06
+#define QVIRTIO_IOMMU_TIMEOUT_US (30 * 1000 * 1000)
+
+static QGuestAllocator *alloc;
+
+static void iommu_hotplug(void *obj, void *data, QGuestAllocator *alloc)
+{
+ QVirtioPCIDevice *dev = obj;
+ QTestState *qts = dev->pdev->bus->qts;
+
+ qtest_qmp_device_add(qts, "virtio-iommu-pci", "iommu1",
+ "{'addr': %s}", stringify(PCI_SLOT_HP));
+
+}
+
+static void pci_config(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+ QVirtioIOMMU *v_iommu = obj;
+ QVirtioDevice *dev = v_iommu->vdev;
+ uint64_t input_range_start = qvirtio_config_readq(dev, 8);
+ uint64_t input_range_end = qvirtio_config_readq(dev, 16);
+ uint32_t domain_range_start = qvirtio_config_readl(dev, 24);
+ uint32_t domain_range_end = qvirtio_config_readl(dev, 28);
+
+ g_assert_cmpint(input_range_start, ==, 0);
+ g_assert_cmphex(input_range_end, ==, 0xFFFFFFFFFFFFFFFF);
+ g_assert_cmpint(domain_range_start, ==, 0);
+ g_assert_cmpint(domain_range_end, ==, 32);
+}
+
+/**
+ * send_attach_detach - Send an attach/detach command to the device
+ * @type: VIRTIO_IOMMU_T_ATTACH/VIRTIO_IOMMU_T_DETACH
+ * @domain: domain the end point is attached to
+ * @ep: end-point
+ */
+static int send_attach_detach(QTestState *qts, QVirtioIOMMU *v_iommu,
+ uint8_t type, uint32_t domain, uint32_t ep)
+{
+ QVirtioDevice *dev = v_iommu->vdev;
+ QVirtQueue *vq = v_iommu->vq;
+ uint64_t ro_addr, wr_addr;
+ uint32_t free_head;
+ struct virtio_iommu_req_attach req; /* same layout as detach */
+ size_t ro_size = sizeof(req) - sizeof(struct virtio_iommu_req_tail);
+ size_t wr_size = sizeof(struct virtio_iommu_req_tail);
+ char buffer[64];
+ int ret;
+
+ req.head.type = type;
+ req.domain = domain;
+ req.endpoint = ep;
+
+ ro_addr = guest_alloc(alloc, ro_size);
+ wr_addr = guest_alloc(alloc, wr_size);
+
+ qtest_memwrite(qts, ro_addr, &req, ro_size);
+ free_head = qvirtqueue_add(qts, vq, ro_addr, ro_size, false, true);
+ qvirtqueue_add(qts, vq, wr_addr, wr_size, true, false);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_IOMMU_TIMEOUT_US);
+ qtest_memread(qts, wr_addr, buffer, wr_size);
+ ret = ((struct virtio_iommu_req_tail *)buffer)->status;
+ guest_free(alloc, ro_addr);
+ guest_free(alloc, wr_addr);
+ return ret;
+}
+
+/**
+ * send_map - Send a map command to the device
+ * @domain: domain the new binding is attached to
+ * @virt_start: iova start
+ * @virt_end: iova end
+ * @phys_start: base physical address
+ * @flags: mapping flags
+ */
+static int send_map(QTestState *qts, QVirtioIOMMU *v_iommu,
+ uint32_t domain, uint64_t virt_start, uint64_t virt_end,
+ uint64_t phys_start, uint32_t flags)
+{
+ QVirtioDevice *dev = v_iommu->vdev;
+ QVirtQueue *vq = v_iommu->vq;
+ uint64_t ro_addr, wr_addr;
+ uint32_t free_head;
+ struct virtio_iommu_req_map req;
+ size_t ro_size = sizeof(req) - sizeof(struct virtio_iommu_req_tail);
+ size_t wr_size = sizeof(struct virtio_iommu_req_tail);
+ char buffer[64];
+ int ret;
+
+ req.head.type = VIRTIO_IOMMU_T_MAP;
+ req.domain = domain;
+ req.virt_start = virt_start;
+ req.virt_end = virt_end;
+ req.phys_start = phys_start;
+ req.flags = flags;
+
+ ro_addr = guest_alloc(alloc, ro_size);
+ wr_addr = guest_alloc(alloc, wr_size);
+
+ qtest_memwrite(qts, ro_addr, &req, ro_size);
+ free_head = qvirtqueue_add(qts, vq, ro_addr, ro_size, false, true);
+ qvirtqueue_add(qts, vq, wr_addr, wr_size, true, false);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_IOMMU_TIMEOUT_US);
+ memread(wr_addr, buffer, wr_size);
+ ret = ((struct virtio_iommu_req_tail *)buffer)->status;
+ guest_free(alloc, ro_addr);
+ guest_free(alloc, wr_addr);
+ return ret;
+}
+
+/**
+ * send_unmap - Send an unmap command to the device
+ * @domain: domain the new binding is attached to
+ * @virt_start: iova start
+ * @virt_end: iova end
+ */
+static int send_unmap(QTestState *qts, QVirtioIOMMU *v_iommu,
+ uint32_t domain, uint64_t virt_start, uint64_t virt_end)
+{
+ QVirtioDevice *dev = v_iommu->vdev;
+ QVirtQueue *vq = v_iommu->vq;
+ uint64_t ro_addr, wr_addr;
+ uint32_t free_head;
+ struct virtio_iommu_req_unmap req;
+ size_t ro_size = sizeof(req) - sizeof(struct virtio_iommu_req_tail);
+ size_t wr_size = sizeof(struct virtio_iommu_req_tail);
+ char buffer[64];
+ int ret;
+
+ req.head.type = VIRTIO_IOMMU_T_UNMAP;
+ req.domain = domain;
+ req.virt_start = virt_start;
+ req.virt_end = virt_end;
+
+ ro_addr = guest_alloc(alloc, ro_size);
+ wr_addr = guest_alloc(alloc, wr_size);
+
+ qtest_memwrite(qts, ro_addr, &req, ro_size);
+ free_head = qvirtqueue_add(qts, vq, ro_addr, ro_size, false, true);
+ qvirtqueue_add(qts, vq, wr_addr, wr_size, true, false);
+ qvirtqueue_kick(qts, dev, vq, free_head);
+ qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+ QVIRTIO_IOMMU_TIMEOUT_US);
+ memread(wr_addr, buffer, wr_size);
+ ret = ((struct virtio_iommu_req_tail *)buffer)->status;
+ guest_free(alloc, ro_addr);
+ guest_free(alloc, wr_addr);
+ return ret;
+}
+
+/* Test unmap scenari documented in the spec v0.12 */
+static void test_attach_detach(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+ QVirtioIOMMU *v_iommu = obj;
+ QTestState *qts = global_qtest;
+ int ret;
+
+ alloc = t_alloc;
+
+ /* type, domain, ep */
+
+ /* attach ep0 to domain 0 */
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 0, 0);
+ g_assert_cmpint(ret, ==, 0);
+
+ /* attach a non existing device (1) */
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 0, 444);
+ g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+ /* detach a non existing device (1) */
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 0, 1);
+ g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+ /* move ep0 from domain 0 to domain 1 */
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 1, 0);
+ g_assert_cmpint(ret, ==, 0);
+
+ /* detach ep0 to domain 0 */
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 0, 0);
+ g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_INVAL);
+
+ /* detach ep0 from domain 1 */
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 1, 0);
+ g_assert_cmpint(ret, ==, 0);
+
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 1, 0);
+ g_assert_cmpint(ret, ==, 0);
+ ret = send_map(qts, v_iommu, 1, 0x0, 0xFFF, 0xa1000,
+ VIRTIO_IOMMU_MAP_F_READ);
+ g_assert_cmpint(ret, ==, 0);
+ ret = send_map(qts, v_iommu, 1, 0x2000, 0x2FFF, 0xb1000,
+ VIRTIO_IOMMU_MAP_F_READ);
+ g_assert_cmpint(ret, ==, 0);
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_DETACH, 1, 0);
+ g_assert_cmpint(ret, ==, 0);
+}
+
+static void test_map_unmap(void *obj, void *data, QGuestAllocator *t_alloc)
+{
+ QVirtioIOMMU *v_iommu = obj;
+ QTestState *qts = global_qtest;
+ int ret;
+
+ alloc = t_alloc;
+
+ /* attach ep0 to domain 1 */
+ ret = send_attach_detach(qts, v_iommu, VIRTIO_IOMMU_T_ATTACH, 1, 0);
+ g_assert_cmpint(ret, ==, 0);
+
+ ret = send_map(qts, v_iommu, 0, 0, 0xFFF, 0xa1000, VIRTIO_IOMMU_MAP_F_READ);
+ g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+ /* domain, virt start, virt end, phys start, flags */
+ ret = send_map(qts, v_iommu, 1, 0, 0xFFF, 0xa1000, VIRTIO_IOMMU_MAP_F_READ);
+ g_assert_cmpint(ret, ==, 0);
+
+ ret = send_unmap(qts, v_iommu, 4, 0x10, 0xFFF);
+ g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_NOENT);
+
+ ret = send_unmap(qts, v_iommu, 1, 0x10, 0xFFF);
+ g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_RANGE);
+
+ ret = send_unmap(qts, v_iommu, 1, 0, 0x1000);
+ g_assert_cmpint(ret, ==, 0); /* unmap everything */
+
+ /* Spec example sequence */
+
+ /* 1 */
+ ret = send_unmap(qts, v_iommu, 1, 0, 4);
+ g_assert_cmpint(ret, ==, 0); /* doesn't unmap anything */
+
+ /* 2 */
+ send_map(qts, v_iommu, 1, 0, 9, 0xa1000, VIRTIO_IOMMU_MAP_F_READ);
+ ret = send_unmap(qts, v_iommu, 1, 0, 9);
+ g_assert_cmpint(ret, ==, 0); /* unmaps [0,9] */
+
+ /* 3 */
+ send_map(qts, v_iommu, 1, 0, 4, 0xb1000, VIRTIO_IOMMU_MAP_F_READ);
+ send_map(qts, v_iommu, 1, 5, 9, 0xb2000, VIRTIO_IOMMU_MAP_F_READ);
+ ret = send_unmap(qts, v_iommu, 1, 0, 9);
+ g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] and [5,9] */
+
+ /* 4 */
+ send_map(qts, v_iommu, 1, 0, 9, 0xc1000, VIRTIO_IOMMU_MAP_F_READ);
+ ret = send_unmap(qts, v_iommu, 1, 0, 4);
+ g_assert_cmpint(ret, ==, VIRTIO_IOMMU_S_RANGE); /* doesn't unmap anything */
+
+ ret = send_unmap(qts, v_iommu, 1, 0, 10);
+ g_assert_cmpint(ret, ==, 0);
+
+ /* 5 */
+ send_map(qts, v_iommu, 1, 0, 4, 0xd1000, VIRTIO_IOMMU_MAP_F_READ);
+ send_map(qts, v_iommu, 1, 5, 9, 0xd2000, VIRTIO_IOMMU_MAP_F_READ);
+ ret = send_unmap(qts, v_iommu, 1, 0, 4);
+ g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] */
+
+ ret = send_unmap(qts, v_iommu, 1, 5, 9);
+ g_assert_cmpint(ret, ==, 0);
+
+ /* 6 */
+ send_map(qts, v_iommu, 1, 0, 4, 0xe2000, VIRTIO_IOMMU_MAP_F_READ);
+ ret = send_unmap(qts, v_iommu, 1, 0, 9);
+ g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] */
+
+ /* 7 */
+ send_map(qts, v_iommu, 1, 0, 4, 0xf2000, VIRTIO_IOMMU_MAP_F_READ);
+ send_map(qts, v_iommu, 1, 10, 14, 0xf3000, VIRTIO_IOMMU_MAP_F_READ);
+ ret = send_unmap(qts, v_iommu, 1, 0, 14);
+ g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] and [10,14] */
+
+ send_unmap(qts, v_iommu, 1, 0, 100);
+ send_map(qts, v_iommu, 1, 10, 14, 0xf3000, VIRTIO_IOMMU_MAP_F_READ);
+ send_map(qts, v_iommu, 1, 0, 4, 0xf2000, VIRTIO_IOMMU_MAP_F_READ);
+ ret = send_unmap(qts, v_iommu, 1, 0, 4);
+ g_assert_cmpint(ret, ==, 0); /* unmaps [0,4] and [10,14] */
+}
+
+static void register_virtio_iommu_test(void)
+{
+ qos_add_test("hotplug", "virtio-iommu-pci", iommu_hotplug, NULL);
+ qos_add_test("config", "virtio-iommu", pci_config, NULL);
+ qos_add_test("attach_detach", "virtio-iommu", test_attach_detach, NULL);
+ qos_add_test("map_unmap", "virtio-iommu", test_map_unmap, NULL);
+}
+
+libqos_init(register_virtio_iommu_test);
--
2.20.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v13 00/10] VIRTIO-IOMMU device
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
` (9 preceding siblings ...)
2020-01-25 17:19 ` [PATCH v13 10/10] tests: Add virtio-iommu test Eric Auger
@ 2020-02-03 12:58 ` Auger Eric
2020-02-03 13:49 ` Peter Xu
10 siblings, 1 reply; 28+ messages in thread
From: Auger Eric @ 2020-02-03 12:58 UTC (permalink / raw)
To: eric.auger.pro, qemu-devel, qemu-arm, peter.maydell,
jean-philippe, dgilbert, quintela, mst, peterx
Cc: kevin.tian, bharatb.linux, tnowicki
Hi,
On 1/25/20 6:19 PM, Eric Auger wrote:
> This series implements the QEMU virtio-iommu device.
>
> This matches the v0.12 spec (voted) and the corresponding
> virtio-iommu driver upstreamed in 5.3. All kernel dependencies
> are resolved for DT integration. The virtio-iommu can be
> instantiated in ARM virt using "-device virtio-iommu-pci".
>
> Non DT mode is not yet supported as it has non resolved kernel
> dependencies [1].
>
> This feature targets 5.0.
If possible I would like to make this feature upstream in 5.0. Do you
guys have other comments/objections?
Thanks
Eric
>
> Integration with vhost devices and vfio devices is not part
> of this series. Please follow Bharat's respins [2].
>
> Best Regards
>
> Eric
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/v4.2-virtio-iommu-v13
>
> References:
> [1] [RFC 00/13] virtio-iommu on non-devicetree platforms
> [2] [PATCH RFC v5 0/5] virtio-iommu: VFIO integration
>
> Testing:
> - tested with guest using virtio-net-pci
> (,vhost=off,iommu_platform,disable-modern=off,disable-legacy=on)
> and virtio-blk-pci
> - migration
>
> History:
>
> v12 -> v13:
> - Take into account Peter's comments
> - fix qtest error and accomodate for directory changes in
> test
> - remove "[PATCH v12 01/13] migration: Support QLIST migration"
> which is now upstream
> - fix iommu_find_iommu_pcibus()
> - squash commits as requested by Peter
> - remove spurious guest log
>
> v11 -> v12:
> - took into account Peter and Jean's comments
> - use guest features
> - restore as_by_bus_num and when attaching devices, check they are
> actually protected by the IOMMU. Updated the tests accordingly.
> - fix the mapping ref counting and make sure mappings are properly
> cleaned.
> - Use CamelCase for data types
> - simplify postload callback as suggested by Peter
> - add R-bs
> - fix mingw compilation issue
> - add IOMMU migration priority
> - qlist migration load simplified following Juan's suggestion
>
> v10 -> v11:
> - introduce virtio_iommu_handle_req macro
> - migration support
> - introduce DEFINE_PROP_INTERVAL and pass reserved regions
> through an array of those
> - domain gtree simplification
>
> v9 -> v10:
> - rebase on 4.1.0-rc2, compliance with 0.12 spec
> - removed ACPI part
> - cleanup (see individual change logs)
> - moved to a PATCH series
>
> v8 -> v9:
> - virtio-iommu-pci device needs to be instantiated from the command
> line (RID is not imposed anymore).
> - tail structure properly initialized
>
> v7 -> v8:
> - virtio-iommu-pci added
> - virt instantiation modified
> - DT and ACPI modified to exclude the iommu RID from the mapping
> - VIRTIO_IOMMU_F_BYPASS, VIRTIO_F_VERSION_1 features exposed
>
> v6 -> v7:
> - rebase on qemu 3.0.0-rc3
> - minor update against v0.7
> - fix issue with EP not on pci.0 and ACPI probing
> - change the instantiation method
>
> v5 -> v6:
> - minor update against v0.6 spec
> - fix g_hash_table_lookup in virtio_iommu_find_add_as
> - replace some error_reports by qemu_log_mask(LOG_GUEST_ERROR, ...)
>
> v4 -> v5:
> - event queue and fault reporting
> - we now return the IOAPIC MSI region if the virtio-iommu is instantiated
> in a PC machine.
> - we bypass transactions on MSI HW region and fault on reserved ones.
> - We support ACPI boot with mach-virt (based on IORT proposal)
> - We moved to the new driver naming conventions
> - simplified mach-virt instantiation
> - worked around the disappearing of pci_find_primary_bus
> - in virtio_iommu_translate, check the dev->as is not NULL
> - initialize as->device_list in virtio_iommu_get_as
> - initialize bufstate.error to false in virtio_iommu_probe
>
> v3 -> v4:
> - probe request support although no reserved region is returned at
> the moment
> - unmap semantics less strict, as specified in v0.4
> - device registration, attach/detach revisited
> - split into smaller patches to ease review
> - propose a way to inform the IOMMU mr about the page_size_mask
> of underlying HW IOMMU, if any
> - remove warning associated with the translation of the MSI doorbell
>
> v2 -> v3:
> - rebase on top of 2.10-rc0 and especially
> [PATCH qemu v9 0/2] memory/iommu: QOM'fy IOMMU MemoryRegion
> - add mutex init
> - fix as->mappings deletion using g_tree_ref/unref
> - when a dev is attached whereas it is already attached to
> another address space, first detach it
> - fix some error values
> - page_sizes = TARGET_PAGE_MASK;
> - I haven't changed the unmap() semantics yet, waiting for the
> next virtio-iommu spec revision.
>
> v1 -> v2:
> - fix redefinition of viommu_as typedef
>
>
> Eric Auger (10):
> virtio-iommu: Add skeleton
> virtio-iommu: Decode the command payload
> virtio-iommu: Implement attach/detach command
> virtio-iommu: Implement map/unmap
> virtio-iommu: Implement translate
> virtio-iommu: Implement fault reporting
> virtio-iommu-pci: Add virtio iommu pci support
> hw/arm/virt: Add the virtio-iommu device tree mappings
> virtio-iommu: Support migration
> tests: Add virtio-iommu test
>
> hw/arm/virt.c | 54 +-
> hw/virtio/Kconfig | 5 +
> hw/virtio/Makefile.objs | 2 +
> hw/virtio/trace-events | 20 +
> hw/virtio/virtio-iommu-pci.c | 88 +++
> hw/virtio/virtio-iommu.c | 897 ++++++++++++++++++++++++++++++
> include/hw/arm/virt.h | 2 +
> include/hw/pci/pci.h | 1 +
> include/hw/virtio/virtio-iommu.h | 61 ++
> qdev-monitor.c | 1 +
> tests/qtest/Makefile.include | 2 +
> tests/qtest/libqos/virtio-iommu.c | 177 ++++++
> tests/qtest/libqos/virtio-iommu.h | 45 ++
> tests/qtest/virtio-iommu-test.c | 306 ++++++++++
> 14 files changed, 1653 insertions(+), 8 deletions(-)
> create mode 100644 hw/virtio/virtio-iommu-pci.c
> create mode 100644 hw/virtio/virtio-iommu.c
> create mode 100644 include/hw/virtio/virtio-iommu.h
> create mode 100644 tests/qtest/libqos/virtio-iommu.c
> create mode 100644 tests/qtest/libqos/virtio-iommu.h
> create mode 100644 tests/qtest/virtio-iommu-test.c
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support
2020-01-25 17:19 ` [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support Eric Auger
@ 2020-02-03 13:03 ` Michael S. Tsirkin
2020-02-03 13:20 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2020-02-03 13:03 UTC (permalink / raw)
To: Eric Auger
Cc: peter.maydell, kevin.tian, tnowicki, jean-philippe, quintela,
qemu-devel, peterx, dgilbert, bharatb.linux, qemu-arm,
eric.auger.pro
On Sat, Jan 25, 2020 at 06:19:52PM +0100, Eric Auger wrote:
> This patch adds virtio-iommu-pci, which is the pci proxy for
> the virtio-iommu device.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
I commented on v11 of this patch:
> > Could you send a smaller patchset without pci/acpi bits for now?
And you answered:
> Yes I am about to send v12.
I guess this patch is here by mistake then?
I think PCI devices should always have config space so guests are
not tempted to find work-arounds. Right?
> ---
>
> v11 -> v12:
> - added Jean's R-b
> - remove the array of intervals. Will be introduced later?
>
> v10 -> v11:
> - add the reserved_regions array property
>
> v9 -> v10:
> - include "hw/qdev-properties.h" header
>
> v8 -> v9:
> - add the msi-bypass property
> - create virtio-iommu-pci.c
> ---
> hw/virtio/Makefile.objs | 1 +
> hw/virtio/virtio-iommu-pci.c | 88 ++++++++++++++++++++++++++++++++
> include/hw/pci/pci.h | 1 +
> include/hw/virtio/virtio-iommu.h | 1 +
> qdev-monitor.c | 1 +
> 5 files changed, 92 insertions(+)
> create mode 100644 hw/virtio/virtio-iommu-pci.c
>
> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> index 2fd9da7410..4e4d39a0a4 100644
> --- a/hw/virtio/Makefile.objs
> +++ b/hw/virtio/Makefile.objs
> @@ -29,6 +29,7 @@ obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
> obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
> obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
> obj-$(CONFIG_VIRTIO_BALLOON) += virtio-balloon-pci.o
> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu-pci.o
> obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
> obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
> obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> new file mode 100644
> index 0000000000..4cfae1f9df
> --- /dev/null
> +++ b/hw/virtio/virtio-iommu-pci.c
> @@ -0,0 +1,88 @@
> +/*
> + * Virtio IOMMU PCI Bindings
> + *
> + * Copyright (c) 2019 Red Hat, Inc.
> + * Written by Eric Auger
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 or
> + * (at your option) any later version.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "virtio-pci.h"
> +#include "hw/virtio/virtio-iommu.h"
> +#include "hw/qdev-properties.h"
> +
> +typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
> +
> +/*
> + * virtio-iommu-pci: This extends VirtioPCIProxy.
> + *
> + */
> +#define VIRTIO_IOMMU_PCI(obj) \
> + OBJECT_CHECK(VirtIOIOMMUPCI, (obj), TYPE_VIRTIO_IOMMU_PCI)
> +
> +struct VirtIOIOMMUPCI {
> + VirtIOPCIProxy parent_obj;
> + VirtIOIOMMU vdev;
> +};
> +
> +static Property virtio_iommu_pci_properties[] = {
> + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> +{
> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
> + DeviceState *vdev = DEVICE(&dev->vdev);
> +
> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> + object_property_set_link(OBJECT(dev),
> + OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
> + "primary-bus", errp);
> + object_property_set_bool(OBJECT(vdev), true, "realized", errp);
> +}
> +
> +static void virtio_iommu_pci_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> + k->realize = virtio_iommu_pci_realize;
> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> + dc->props = virtio_iommu_pci_properties;
> + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
> + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> + pcidev_k->class_id = PCI_CLASS_OTHERS;
> +}
> +
> +static void virtio_iommu_pci_instance_init(Object *obj)
> +{
> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
> +
> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> + TYPE_VIRTIO_IOMMU);
> +}
> +
> +static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
> + .base_name = TYPE_VIRTIO_IOMMU_PCI,
> + .generic_name = "virtio-iommu-pci",
> + .transitional_name = "virtio-iommu-pci-transitional",
> + .non_transitional_name = "virtio-iommu-pci-non-transitional",
> + .instance_size = sizeof(VirtIOIOMMUPCI),
> + .instance_init = virtio_iommu_pci_instance_init,
> + .class_init = virtio_iommu_pci_class_init,
> +};
> +
> +static void virtio_iommu_pci_register(void)
> +{
> + virtio_pci_types_register(&virtio_iommu_pci_info);
> +}
> +
> +type_init(virtio_iommu_pci_register)
> +
> +
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 2acd8321af..cfedf5a995 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -86,6 +86,7 @@ extern bool pci_available;
> #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
> #define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
> #define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013
> +#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014
>
> #define PCI_VENDOR_ID_REDHAT 0x1b36
> #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 2a2c2ecf83..f39aa0fbb4 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -25,6 +25,7 @@
> #include "hw/pci/pci.h"
>
> #define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
> +#define TYPE_VIRTIO_IOMMU_PCI "virtio-iommu-device-base"
> #define VIRTIO_IOMMU(obj) \
> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
>
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 3465a1e2d0..97f4022b51 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -66,6 +66,7 @@ static const QDevAlias qdev_alias_table[] = {
> { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
> { "virtio-input-host-pci", "virtio-input-host",
> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> + { "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> { "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
> { "virtio-keyboard-pci", "virtio-keyboard",
> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> --
> 2.20.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support
2020-02-03 13:03 ` Michael S. Tsirkin
@ 2020-02-03 13:20 ` Auger Eric
2020-02-03 13:28 ` Michael S. Tsirkin
0 siblings, 1 reply; 28+ messages in thread
From: Auger Eric @ 2020-02-03 13:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: peter.maydell, kevin.tian, tnowicki, quintela, qemu-devel,
peterx, dgilbert, bharatb.linux, qemu-arm, jean-philippe,
eric.auger.pro
Hi Michael,
On 2/3/20 2:03 PM, Michael S. Tsirkin wrote:
> On Sat, Jan 25, 2020 at 06:19:52PM +0100, Eric Auger wrote:
>> This patch adds virtio-iommu-pci, which is the pci proxy for
>> the virtio-iommu device.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>
> I commented on v11 of this patch:
>>> Could you send a smaller patchset without pci/acpi bits for now?
> And you answered:
>> Yes I am about to send v12.
>
> I guess this patch is here by mistake then?
>
> I think PCI devices should always have config space so guests are
> not tempted to find work-arounds. Right?
No it is not here by mistake. I removed everything related non DT
integration as we discussed.
DT support is fully upstream even for virtio-iommu-pci.
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/iommu.txt
So what's wrong implementing that at the moment. As we discussed we
would use the PCIe config space integration for non DT.
If I use the MMIO based device, I am forced to lock an MMIO region for
it in the machvirt memory map:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/virtio/mmio.txt
I guess Peter (Maydell) will not be happy with this situation either.
Thanks
Eric
>
>> ---
>>
>> v11 -> v12:
>> - added Jean's R-b
>> - remove the array of intervals. Will be introduced later?
>>
>> v10 -> v11:
>> - add the reserved_regions array property
>>
>> v9 -> v10:
>> - include "hw/qdev-properties.h" header
>>
>> v8 -> v9:
>> - add the msi-bypass property
>> - create virtio-iommu-pci.c
>> ---
>> hw/virtio/Makefile.objs | 1 +
>> hw/virtio/virtio-iommu-pci.c | 88 ++++++++++++++++++++++++++++++++
>> include/hw/pci/pci.h | 1 +
>> include/hw/virtio/virtio-iommu.h | 1 +
>> qdev-monitor.c | 1 +
>> 5 files changed, 92 insertions(+)
>> create mode 100644 hw/virtio/virtio-iommu-pci.c
>>
>> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
>> index 2fd9da7410..4e4d39a0a4 100644
>> --- a/hw/virtio/Makefile.objs
>> +++ b/hw/virtio/Makefile.objs
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
>> obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
>> obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
>> obj-$(CONFIG_VIRTIO_BALLOON) += virtio-balloon-pci.o
>> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu-pci.o
>> obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
>> obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
>> obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
>> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
>> new file mode 100644
>> index 0000000000..4cfae1f9df
>> --- /dev/null
>> +++ b/hw/virtio/virtio-iommu-pci.c
>> @@ -0,0 +1,88 @@
>> +/*
>> + * Virtio IOMMU PCI Bindings
>> + *
>> + * Copyright (c) 2019 Red Hat, Inc.
>> + * Written by Eric Auger
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "virtio-pci.h"
>> +#include "hw/virtio/virtio-iommu.h"
>> +#include "hw/qdev-properties.h"
>> +
>> +typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
>> +
>> +/*
>> + * virtio-iommu-pci: This extends VirtioPCIProxy.
>> + *
>> + */
>> +#define VIRTIO_IOMMU_PCI(obj) \
>> + OBJECT_CHECK(VirtIOIOMMUPCI, (obj), TYPE_VIRTIO_IOMMU_PCI)
>> +
>> +struct VirtIOIOMMUPCI {
>> + VirtIOPCIProxy parent_obj;
>> + VirtIOIOMMU vdev;
>> +};
>> +
>> +static Property virtio_iommu_pci_properties[] = {
>> + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
>> + DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>> +{
>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
>> + DeviceState *vdev = DEVICE(&dev->vdev);
>> +
>> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>> + object_property_set_link(OBJECT(dev),
>> + OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
>> + "primary-bus", errp);
>> + object_property_set_bool(OBJECT(vdev), true, "realized", errp);
>> +}
>> +
>> +static void virtio_iommu_pci_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
>> + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
>> + k->realize = virtio_iommu_pci_realize;
>> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>> + dc->props = virtio_iommu_pci_properties;
>> + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
>> + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
>> + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
>> + pcidev_k->class_id = PCI_CLASS_OTHERS;
>> +}
>> +
>> +static void virtio_iommu_pci_instance_init(Object *obj)
>> +{
>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
>> +
>> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>> + TYPE_VIRTIO_IOMMU);
>> +}
>> +
>> +static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
>> + .base_name = TYPE_VIRTIO_IOMMU_PCI,
>> + .generic_name = "virtio-iommu-pci",
>> + .transitional_name = "virtio-iommu-pci-transitional",
>> + .non_transitional_name = "virtio-iommu-pci-non-transitional",
>> + .instance_size = sizeof(VirtIOIOMMUPCI),
>> + .instance_init = virtio_iommu_pci_instance_init,
>> + .class_init = virtio_iommu_pci_class_init,
>> +};
>> +
>> +static void virtio_iommu_pci_register(void)
>> +{
>> + virtio_pci_types_register(&virtio_iommu_pci_info);
>> +}
>> +
>> +type_init(virtio_iommu_pci_register)
>> +
>> +
>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>> index 2acd8321af..cfedf5a995 100644
>> --- a/include/hw/pci/pci.h
>> +++ b/include/hw/pci/pci.h
>> @@ -86,6 +86,7 @@ extern bool pci_available;
>> #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
>> #define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
>> #define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013
>> +#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014
>>
>> #define PCI_VENDOR_ID_REDHAT 0x1b36
>> #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>> index 2a2c2ecf83..f39aa0fbb4 100644
>> --- a/include/hw/virtio/virtio-iommu.h
>> +++ b/include/hw/virtio/virtio-iommu.h
>> @@ -25,6 +25,7 @@
>> #include "hw/pci/pci.h"
>>
>> #define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
>> +#define TYPE_VIRTIO_IOMMU_PCI "virtio-iommu-device-base"
>> #define VIRTIO_IOMMU(obj) \
>> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
>>
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index 3465a1e2d0..97f4022b51 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -66,6 +66,7 @@ static const QDevAlias qdev_alias_table[] = {
>> { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
>> { "virtio-input-host-pci", "virtio-input-host",
>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>> + { "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>> { "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
>> { "virtio-keyboard-pci", "virtio-keyboard",
>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>> --
>> 2.20.1
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support
2020-02-03 13:20 ` Auger Eric
@ 2020-02-03 13:28 ` Michael S. Tsirkin
2020-02-03 13:38 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2020-02-03 13:28 UTC (permalink / raw)
To: Auger Eric
Cc: peter.maydell, kevin.tian, tnowicki, quintela, qemu-devel,
peterx, dgilbert, bharatb.linux, qemu-arm, jean-philippe,
eric.auger.pro
On Mon, Feb 03, 2020 at 02:20:55PM +0100, Auger Eric wrote:
> Hi Michael,
>
> On 2/3/20 2:03 PM, Michael S. Tsirkin wrote:
> > On Sat, Jan 25, 2020 at 06:19:52PM +0100, Eric Auger wrote:
> >> This patch adds virtio-iommu-pci, which is the pci proxy for
> >> the virtio-iommu device.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> >
> > I commented on v11 of this patch:
> >>> Could you send a smaller patchset without pci/acpi bits for now?
> > And you answered:
> >> Yes I am about to send v12.
> >
> > I guess this patch is here by mistake then?
> >
> > I think PCI devices should always have config space so guests are
> > not tempted to find work-arounds. Right?
> No it is not here by mistake. I removed everything related non DT
> integration as we discussed.
>
> DT support is fully upstream even for virtio-iommu-pci.
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/iommu.txt
>
> So what's wrong implementing that at the moment. As we discussed we
> would use the PCIe config space integration for non DT.
>
> If I use the MMIO based device, I am forced to lock an MMIO region for
> it in the machvirt memory map:
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/virtio/mmio.txt
>
> I guess Peter (Maydell) will not be happy with this situation either.
>
> Thanks
>
> Eric
I see. Can't we limit this to DT platforms for now then?
> >
> >> ---
> >>
> >> v11 -> v12:
> >> - added Jean's R-b
> >> - remove the array of intervals. Will be introduced later?
> >>
> >> v10 -> v11:
> >> - add the reserved_regions array property
> >>
> >> v9 -> v10:
> >> - include "hw/qdev-properties.h" header
> >>
> >> v8 -> v9:
> >> - add the msi-bypass property
> >> - create virtio-iommu-pci.c
> >> ---
> >> hw/virtio/Makefile.objs | 1 +
> >> hw/virtio/virtio-iommu-pci.c | 88 ++++++++++++++++++++++++++++++++
> >> include/hw/pci/pci.h | 1 +
> >> include/hw/virtio/virtio-iommu.h | 1 +
> >> qdev-monitor.c | 1 +
> >> 5 files changed, 92 insertions(+)
> >> create mode 100644 hw/virtio/virtio-iommu-pci.c
> >>
> >> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> >> index 2fd9da7410..4e4d39a0a4 100644
> >> --- a/hw/virtio/Makefile.objs
> >> +++ b/hw/virtio/Makefile.objs
> >> @@ -29,6 +29,7 @@ obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
> >> obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
> >> obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
> >> obj-$(CONFIG_VIRTIO_BALLOON) += virtio-balloon-pci.o
> >> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu-pci.o
> >> obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
> >> obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
> >> obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
> >> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> >> new file mode 100644
> >> index 0000000000..4cfae1f9df
> >> --- /dev/null
> >> +++ b/hw/virtio/virtio-iommu-pci.c
> >> @@ -0,0 +1,88 @@
> >> +/*
> >> + * Virtio IOMMU PCI Bindings
> >> + *
> >> + * Copyright (c) 2019 Red Hat, Inc.
> >> + * Written by Eric Auger
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 or
> >> + * (at your option) any later version.
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +
> >> +#include "virtio-pci.h"
> >> +#include "hw/virtio/virtio-iommu.h"
> >> +#include "hw/qdev-properties.h"
> >> +
> >> +typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
> >> +
> >> +/*
> >> + * virtio-iommu-pci: This extends VirtioPCIProxy.
> >> + *
> >> + */
> >> +#define VIRTIO_IOMMU_PCI(obj) \
> >> + OBJECT_CHECK(VirtIOIOMMUPCI, (obj), TYPE_VIRTIO_IOMMU_PCI)
> >> +
> >> +struct VirtIOIOMMUPCI {
> >> + VirtIOPCIProxy parent_obj;
> >> + VirtIOIOMMU vdev;
> >> +};
> >> +
> >> +static Property virtio_iommu_pci_properties[] = {
> >> + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
> >> + DEFINE_PROP_END_OF_LIST(),
> >> +};
> >> +
> >> +static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> >> +{
> >> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
> >> + DeviceState *vdev = DEVICE(&dev->vdev);
> >> +
> >> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> >> + object_property_set_link(OBJECT(dev),
> >> + OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
> >> + "primary-bus", errp);
> >> + object_property_set_bool(OBJECT(vdev), true, "realized", errp);
> >> +}
> >> +
> >> +static void virtio_iommu_pci_class_init(ObjectClass *klass, void *data)
> >> +{
> >> + DeviceClass *dc = DEVICE_CLASS(klass);
> >> + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> >> + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> >> + k->realize = virtio_iommu_pci_realize;
> >> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> >> + dc->props = virtio_iommu_pci_properties;
> >> + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> >> + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
> >> + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> >> + pcidev_k->class_id = PCI_CLASS_OTHERS;
> >> +}
> >> +
> >> +static void virtio_iommu_pci_instance_init(Object *obj)
> >> +{
> >> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
> >> +
> >> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> >> + TYPE_VIRTIO_IOMMU);
> >> +}
> >> +
> >> +static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
> >> + .base_name = TYPE_VIRTIO_IOMMU_PCI,
> >> + .generic_name = "virtio-iommu-pci",
> >> + .transitional_name = "virtio-iommu-pci-transitional",
> >> + .non_transitional_name = "virtio-iommu-pci-non-transitional",
> >> + .instance_size = sizeof(VirtIOIOMMUPCI),
> >> + .instance_init = virtio_iommu_pci_instance_init,
> >> + .class_init = virtio_iommu_pci_class_init,
> >> +};
> >> +
> >> +static void virtio_iommu_pci_register(void)
> >> +{
> >> + virtio_pci_types_register(&virtio_iommu_pci_info);
> >> +}
> >> +
> >> +type_init(virtio_iommu_pci_register)
> >> +
> >> +
> >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> >> index 2acd8321af..cfedf5a995 100644
> >> --- a/include/hw/pci/pci.h
> >> +++ b/include/hw/pci/pci.h
> >> @@ -86,6 +86,7 @@ extern bool pci_available;
> >> #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
> >> #define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
> >> #define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013
> >> +#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014
> >>
> >> #define PCI_VENDOR_ID_REDHAT 0x1b36
> >> #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
> >> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> >> index 2a2c2ecf83..f39aa0fbb4 100644
> >> --- a/include/hw/virtio/virtio-iommu.h
> >> +++ b/include/hw/virtio/virtio-iommu.h
> >> @@ -25,6 +25,7 @@
> >> #include "hw/pci/pci.h"
> >>
> >> #define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
> >> +#define TYPE_VIRTIO_IOMMU_PCI "virtio-iommu-device-base"
> >> #define VIRTIO_IOMMU(obj) \
> >> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
> >>
> >> diff --git a/qdev-monitor.c b/qdev-monitor.c
> >> index 3465a1e2d0..97f4022b51 100644
> >> --- a/qdev-monitor.c
> >> +++ b/qdev-monitor.c
> >> @@ -66,6 +66,7 @@ static const QDevAlias qdev_alias_table[] = {
> >> { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
> >> { "virtio-input-host-pci", "virtio-input-host",
> >> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> >> + { "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> >> { "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
> >> { "virtio-keyboard-pci", "virtio-keyboard",
> >> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> >> --
> >> 2.20.1
> >
> >
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support
2020-02-03 13:28 ` Michael S. Tsirkin
@ 2020-02-03 13:38 ` Auger Eric
2020-02-03 13:46 ` Michael S. Tsirkin
0 siblings, 1 reply; 28+ messages in thread
From: Auger Eric @ 2020-02-03 13:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: peter.maydell, kevin.tian, tnowicki, quintela, qemu-devel,
peterx, dgilbert, bharatb.linux, qemu-arm, jean-philippe,
eric.auger.pro
Hi Michael,
On 2/3/20 2:28 PM, Michael S. Tsirkin wrote:
> On Mon, Feb 03, 2020 at 02:20:55PM +0100, Auger Eric wrote:
>> Hi Michael,
>>
>> On 2/3/20 2:03 PM, Michael S. Tsirkin wrote:
>>> On Sat, Jan 25, 2020 at 06:19:52PM +0100, Eric Auger wrote:
>>>> This patch adds virtio-iommu-pci, which is the pci proxy for
>>>> the virtio-iommu device.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>>
>>> I commented on v11 of this patch:
>>>>> Could you send a smaller patchset without pci/acpi bits for now?
>>> And you answered:
>>>> Yes I am about to send v12.
>>>
>>> I guess this patch is here by mistake then?
>>>
>>> I think PCI devices should always have config space so guests are
>>> not tempted to find work-arounds. Right?
>> No it is not here by mistake. I removed everything related non DT
>> integration as we discussed.
>>
>> DT support is fully upstream even for virtio-iommu-pci.
>> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/iommu.txt
>>
>> So what's wrong implementing that at the moment. As we discussed we
>> would use the PCIe config space integration for non DT.
>>
>> If I use the MMIO based device, I am forced to lock an MMIO region for
>> it in the machvirt memory map:
>> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/virtio/mmio.txt
>>
>> I guess Peter (Maydell) will not be happy with this situation either.
>>
>> Thanks
>>
>> Eric
>
> I see. Can't we limit this to DT platforms for now then?
That is the case. virtio-iommu does not work with ACPI.
For non DT the plan is to use what you suggested, ie. pass the binding
info through the PCIe config space.
In that prospect I am waiting for Jean-Philippe's [RFC 00/13]
virtio-iommu on non-devicetree platforms respin
(https://lore.kernel.org/linux-iommu/20191203190136.00007171@intel.com/T/)
and especially patches 12 and 13 of the series, ie. binding info through
the PCIe config space.
Thanks
Eric
>
>
>
>>>
>>>> ---
>>>>
>>>> v11 -> v12:
>>>> - added Jean's R-b
>>>> - remove the array of intervals. Will be introduced later?
>>>>
>>>> v10 -> v11:
>>>> - add the reserved_regions array property
>>>>
>>>> v9 -> v10:
>>>> - include "hw/qdev-properties.h" header
>>>>
>>>> v8 -> v9:
>>>> - add the msi-bypass property
>>>> - create virtio-iommu-pci.c
>>>> ---
>>>> hw/virtio/Makefile.objs | 1 +
>>>> hw/virtio/virtio-iommu-pci.c | 88 ++++++++++++++++++++++++++++++++
>>>> include/hw/pci/pci.h | 1 +
>>>> include/hw/virtio/virtio-iommu.h | 1 +
>>>> qdev-monitor.c | 1 +
>>>> 5 files changed, 92 insertions(+)
>>>> create mode 100644 hw/virtio/virtio-iommu-pci.c
>>>>
>>>> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
>>>> index 2fd9da7410..4e4d39a0a4 100644
>>>> --- a/hw/virtio/Makefile.objs
>>>> +++ b/hw/virtio/Makefile.objs
>>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
>>>> obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
>>>> obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
>>>> obj-$(CONFIG_VIRTIO_BALLOON) += virtio-balloon-pci.o
>>>> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu-pci.o
>>>> obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
>>>> obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
>>>> obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
>>>> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
>>>> new file mode 100644
>>>> index 0000000000..4cfae1f9df
>>>> --- /dev/null
>>>> +++ b/hw/virtio/virtio-iommu-pci.c
>>>> @@ -0,0 +1,88 @@
>>>> +/*
>>>> + * Virtio IOMMU PCI Bindings
>>>> + *
>>>> + * Copyright (c) 2019 Red Hat, Inc.
>>>> + * Written by Eric Auger
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 or
>>>> + * (at your option) any later version.
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +
>>>> +#include "virtio-pci.h"
>>>> +#include "hw/virtio/virtio-iommu.h"
>>>> +#include "hw/qdev-properties.h"
>>>> +
>>>> +typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
>>>> +
>>>> +/*
>>>> + * virtio-iommu-pci: This extends VirtioPCIProxy.
>>>> + *
>>>> + */
>>>> +#define VIRTIO_IOMMU_PCI(obj) \
>>>> + OBJECT_CHECK(VirtIOIOMMUPCI, (obj), TYPE_VIRTIO_IOMMU_PCI)
>>>> +
>>>> +struct VirtIOIOMMUPCI {
>>>> + VirtIOPCIProxy parent_obj;
>>>> + VirtIOIOMMU vdev;
>>>> +};
>>>> +
>>>> +static Property virtio_iommu_pci_properties[] = {
>>>> + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
>>>> + DEFINE_PROP_END_OF_LIST(),
>>>> +};
>>>> +
>>>> +static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>>>> +{
>>>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
>>>> + DeviceState *vdev = DEVICE(&dev->vdev);
>>>> +
>>>> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>>>> + object_property_set_link(OBJECT(dev),
>>>> + OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
>>>> + "primary-bus", errp);
>>>> + object_property_set_bool(OBJECT(vdev), true, "realized", errp);
>>>> +}
>>>> +
>>>> +static void virtio_iommu_pci_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>>> + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
>>>> + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
>>>> + k->realize = virtio_iommu_pci_realize;
>>>> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>>> + dc->props = virtio_iommu_pci_properties;
>>>> + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
>>>> + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
>>>> + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
>>>> + pcidev_k->class_id = PCI_CLASS_OTHERS;
>>>> +}
>>>> +
>>>> +static void virtio_iommu_pci_instance_init(Object *obj)
>>>> +{
>>>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
>>>> +
>>>> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>>>> + TYPE_VIRTIO_IOMMU);
>>>> +}
>>>> +
>>>> +static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
>>>> + .base_name = TYPE_VIRTIO_IOMMU_PCI,
>>>> + .generic_name = "virtio-iommu-pci",
>>>> + .transitional_name = "virtio-iommu-pci-transitional",
>>>> + .non_transitional_name = "virtio-iommu-pci-non-transitional",
>>>> + .instance_size = sizeof(VirtIOIOMMUPCI),
>>>> + .instance_init = virtio_iommu_pci_instance_init,
>>>> + .class_init = virtio_iommu_pci_class_init,
>>>> +};
>>>> +
>>>> +static void virtio_iommu_pci_register(void)
>>>> +{
>>>> + virtio_pci_types_register(&virtio_iommu_pci_info);
>>>> +}
>>>> +
>>>> +type_init(virtio_iommu_pci_register)
>>>> +
>>>> +
>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>>> index 2acd8321af..cfedf5a995 100644
>>>> --- a/include/hw/pci/pci.h
>>>> +++ b/include/hw/pci/pci.h
>>>> @@ -86,6 +86,7 @@ extern bool pci_available;
>>>> #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
>>>> #define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
>>>> #define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013
>>>> +#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014
>>>>
>>>> #define PCI_VENDOR_ID_REDHAT 0x1b36
>>>> #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
>>>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>>>> index 2a2c2ecf83..f39aa0fbb4 100644
>>>> --- a/include/hw/virtio/virtio-iommu.h
>>>> +++ b/include/hw/virtio/virtio-iommu.h
>>>> @@ -25,6 +25,7 @@
>>>> #include "hw/pci/pci.h"
>>>>
>>>> #define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
>>>> +#define TYPE_VIRTIO_IOMMU_PCI "virtio-iommu-device-base"
>>>> #define VIRTIO_IOMMU(obj) \
>>>> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
>>>>
>>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>>>> index 3465a1e2d0..97f4022b51 100644
>>>> --- a/qdev-monitor.c
>>>> +++ b/qdev-monitor.c
>>>> @@ -66,6 +66,7 @@ static const QDevAlias qdev_alias_table[] = {
>>>> { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
>>>> { "virtio-input-host-pci", "virtio-input-host",
>>>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>>>> + { "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>>>> { "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
>>>> { "virtio-keyboard-pci", "virtio-keyboard",
>>>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>>>> --
>>>> 2.20.1
>>>
>>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support
2020-02-03 13:38 ` Auger Eric
@ 2020-02-03 13:46 ` Michael S. Tsirkin
2020-02-03 13:52 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Michael S. Tsirkin @ 2020-02-03 13:46 UTC (permalink / raw)
To: Auger Eric
Cc: peter.maydell, kevin.tian, tnowicki, quintela, qemu-devel,
peterx, dgilbert, bharatb.linux, qemu-arm, jean-philippe,
eric.auger.pro
On Mon, Feb 03, 2020 at 02:38:22PM +0100, Auger Eric wrote:
> Hi Michael,
>
> On 2/3/20 2:28 PM, Michael S. Tsirkin wrote:
> > On Mon, Feb 03, 2020 at 02:20:55PM +0100, Auger Eric wrote:
> >> Hi Michael,
> >>
> >> On 2/3/20 2:03 PM, Michael S. Tsirkin wrote:
> >>> On Sat, Jan 25, 2020 at 06:19:52PM +0100, Eric Auger wrote:
> >>>> This patch adds virtio-iommu-pci, which is the pci proxy for
> >>>> the virtio-iommu device.
> >>>>
> >>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> >>>
> >>> I commented on v11 of this patch:
> >>>>> Could you send a smaller patchset without pci/acpi bits for now?
> >>> And you answered:
> >>>> Yes I am about to send v12.
> >>>
> >>> I guess this patch is here by mistake then?
> >>>
> >>> I think PCI devices should always have config space so guests are
> >>> not tempted to find work-arounds. Right?
> >> No it is not here by mistake. I removed everything related non DT
> >> integration as we discussed.
> >>
> >> DT support is fully upstream even for virtio-iommu-pci.
> >> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/iommu.txt
> >>
> >> So what's wrong implementing that at the moment. As we discussed we
> >> would use the PCIe config space integration for non DT.
> >>
> >> If I use the MMIO based device, I am forced to lock an MMIO region for
> >> it in the machvirt memory map:
> >> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/virtio/mmio.txt
> >>
> >> I guess Peter (Maydell) will not be happy with this situation either.
> >>
> >> Thanks
> >>
> >> Eric
> >
> > I see. Can't we limit this to DT platforms for now then?
> That is the case. virtio-iommu does not work with ACPI.
Right. But nothing seems to prevent users from creating it
on systems without dt, and there won't be an easy way for
management to detect it when qemu added this support down the road.
> For non DT the plan is to use what you suggested, ie. pass the binding
> info through the PCIe config space.
>
> In that prospect I am waiting for Jean-Philippe's [RFC 00/13]
> virtio-iommu on non-devicetree platforms respin
> (https://lore.kernel.org/linux-iommu/20191203190136.00007171@intel.com/T/)
> and especially patches 12 and 13 of the series, ie. binding info through
> the PCIe config space.
>
> Thanks
>
> Eric
> >
> >
> >
> >>>
> >>>> ---
> >>>>
> >>>> v11 -> v12:
> >>>> - added Jean's R-b
> >>>> - remove the array of intervals. Will be introduced later?
> >>>>
> >>>> v10 -> v11:
> >>>> - add the reserved_regions array property
> >>>>
> >>>> v9 -> v10:
> >>>> - include "hw/qdev-properties.h" header
> >>>>
> >>>> v8 -> v9:
> >>>> - add the msi-bypass property
> >>>> - create virtio-iommu-pci.c
> >>>> ---
> >>>> hw/virtio/Makefile.objs | 1 +
> >>>> hw/virtio/virtio-iommu-pci.c | 88 ++++++++++++++++++++++++++++++++
> >>>> include/hw/pci/pci.h | 1 +
> >>>> include/hw/virtio/virtio-iommu.h | 1 +
> >>>> qdev-monitor.c | 1 +
> >>>> 5 files changed, 92 insertions(+)
> >>>> create mode 100644 hw/virtio/virtio-iommu-pci.c
> >>>>
> >>>> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> >>>> index 2fd9da7410..4e4d39a0a4 100644
> >>>> --- a/hw/virtio/Makefile.objs
> >>>> +++ b/hw/virtio/Makefile.objs
> >>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
> >>>> obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
> >>>> obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
> >>>> obj-$(CONFIG_VIRTIO_BALLOON) += virtio-balloon-pci.o
> >>>> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu-pci.o
> >>>> obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
> >>>> obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
> >>>> obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
> >>>> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> >>>> new file mode 100644
> >>>> index 0000000000..4cfae1f9df
> >>>> --- /dev/null
> >>>> +++ b/hw/virtio/virtio-iommu-pci.c
> >>>> @@ -0,0 +1,88 @@
> >>>> +/*
> >>>> + * Virtio IOMMU PCI Bindings
> >>>> + *
> >>>> + * Copyright (c) 2019 Red Hat, Inc.
> >>>> + * Written by Eric Auger
> >>>> + *
> >>>> + * This program is free software; you can redistribute it and/or modify
> >>>> + * it under the terms of the GNU General Public License version 2 or
> >>>> + * (at your option) any later version.
> >>>> + */
> >>>> +
> >>>> +#include "qemu/osdep.h"
> >>>> +
> >>>> +#include "virtio-pci.h"
> >>>> +#include "hw/virtio/virtio-iommu.h"
> >>>> +#include "hw/qdev-properties.h"
> >>>> +
> >>>> +typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
> >>>> +
> >>>> +/*
> >>>> + * virtio-iommu-pci: This extends VirtioPCIProxy.
> >>>> + *
> >>>> + */
> >>>> +#define VIRTIO_IOMMU_PCI(obj) \
> >>>> + OBJECT_CHECK(VirtIOIOMMUPCI, (obj), TYPE_VIRTIO_IOMMU_PCI)
> >>>> +
> >>>> +struct VirtIOIOMMUPCI {
> >>>> + VirtIOPCIProxy parent_obj;
> >>>> + VirtIOIOMMU vdev;
> >>>> +};
> >>>> +
> >>>> +static Property virtio_iommu_pci_properties[] = {
> >>>> + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
> >>>> + DEFINE_PROP_END_OF_LIST(),
> >>>> +};
> >>>> +
> >>>> +static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> >>>> +{
> >>>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
> >>>> + DeviceState *vdev = DEVICE(&dev->vdev);
> >>>> +
> >>>> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> >>>> + object_property_set_link(OBJECT(dev),
> >>>> + OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
> >>>> + "primary-bus", errp);
> >>>> + object_property_set_bool(OBJECT(vdev), true, "realized", errp);
> >>>> +}
> >>>> +
> >>>> +static void virtio_iommu_pci_class_init(ObjectClass *klass, void *data)
> >>>> +{
> >>>> + DeviceClass *dc = DEVICE_CLASS(klass);
> >>>> + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> >>>> + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> >>>> + k->realize = virtio_iommu_pci_realize;
> >>>> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> >>>> + dc->props = virtio_iommu_pci_properties;
> >>>> + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> >>>> + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
> >>>> + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> >>>> + pcidev_k->class_id = PCI_CLASS_OTHERS;
> >>>> +}
> >>>> +
> >>>> +static void virtio_iommu_pci_instance_init(Object *obj)
> >>>> +{
> >>>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
> >>>> +
> >>>> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> >>>> + TYPE_VIRTIO_IOMMU);
> >>>> +}
> >>>> +
> >>>> +static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
> >>>> + .base_name = TYPE_VIRTIO_IOMMU_PCI,
> >>>> + .generic_name = "virtio-iommu-pci",
> >>>> + .transitional_name = "virtio-iommu-pci-transitional",
> >>>> + .non_transitional_name = "virtio-iommu-pci-non-transitional",
> >>>> + .instance_size = sizeof(VirtIOIOMMUPCI),
> >>>> + .instance_init = virtio_iommu_pci_instance_init,
> >>>> + .class_init = virtio_iommu_pci_class_init,
> >>>> +};
> >>>> +
> >>>> +static void virtio_iommu_pci_register(void)
> >>>> +{
> >>>> + virtio_pci_types_register(&virtio_iommu_pci_info);
> >>>> +}
> >>>> +
> >>>> +type_init(virtio_iommu_pci_register)
> >>>> +
> >>>> +
> >>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> >>>> index 2acd8321af..cfedf5a995 100644
> >>>> --- a/include/hw/pci/pci.h
> >>>> +++ b/include/hw/pci/pci.h
> >>>> @@ -86,6 +86,7 @@ extern bool pci_available;
> >>>> #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
> >>>> #define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
> >>>> #define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013
> >>>> +#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014
> >>>>
> >>>> #define PCI_VENDOR_ID_REDHAT 0x1b36
> >>>> #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
> >>>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> >>>> index 2a2c2ecf83..f39aa0fbb4 100644
> >>>> --- a/include/hw/virtio/virtio-iommu.h
> >>>> +++ b/include/hw/virtio/virtio-iommu.h
> >>>> @@ -25,6 +25,7 @@
> >>>> #include "hw/pci/pci.h"
> >>>>
> >>>> #define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
> >>>> +#define TYPE_VIRTIO_IOMMU_PCI "virtio-iommu-device-base"
> >>>> #define VIRTIO_IOMMU(obj) \
> >>>> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
> >>>>
> >>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
> >>>> index 3465a1e2d0..97f4022b51 100644
> >>>> --- a/qdev-monitor.c
> >>>> +++ b/qdev-monitor.c
> >>>> @@ -66,6 +66,7 @@ static const QDevAlias qdev_alias_table[] = {
> >>>> { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
> >>>> { "virtio-input-host-pci", "virtio-input-host",
> >>>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> >>>> + { "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> >>>> { "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
> >>>> { "virtio-keyboard-pci", "virtio-keyboard",
> >>>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> >>>> --
> >>>> 2.20.1
> >>>
> >>>
> >
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
2020-01-25 17:19 ` [PATCH v13 03/10] virtio-iommu: Implement attach/detach command Eric Auger
@ 2020-02-03 13:49 ` Peter Xu
2020-02-03 14:59 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Peter Xu @ 2020-02-03 13:49 UTC (permalink / raw)
To: Eric Auger
Cc: peter.maydell, kevin.tian, tnowicki, jean-philippe, quintela,
qemu-devel, dgilbert, bharatb.linux, qemu-arm, mst,
eric.auger.pro
On Sat, Jan 25, 2020 at 06:19:48PM +0100, Eric Auger wrote:
> This patch implements the endpoint attach/detach to/from
> a domain.
>
> Domain and endpoint internal datatypes are introduced.
> Both are stored in RB trees. The domain owns a list of
> endpoints attached to it. Also helpers to get/put
> end points and domains are introduced.
>
> As for the IOMMU memory regions, a callback is called on
> PCI bus enumeration that initializes for a given device
> on the bus hierarchy an IOMMU memory region. The PCI bus
> hierarchy is stored locally in IOMMUPciBus and IOMMUDevice
> objects.
>
> At the time of the enumeration, the bus number may not be
> computed yet.
>
> So operations that will need to retrieve the IOMMUdevice
> and its IOMMU memory region from the bus number and devfn,
> once the bus number is garanteed to be frozen, use an array
> of IOMMUPciBus, lazily populated.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v12 -> v13:
> - squashed v12 4, 5, 6 into this patch
> - rename virtio_iommu_get_sid into virtio_iommu_get_bdf
>
> v11 -> v12:
> - check the device is protected by the iommu on attach
> - on detach, check the domain id the device is attached to matches
> the one used in the detach command
> - fix mapping ref counter and destroy the domain when no end-points
> are attached anymore
> ---
> hw/virtio/trace-events | 6 +
> hw/virtio/virtio-iommu.c | 315 ++++++++++++++++++++++++++++++-
> include/hw/virtio/virtio-iommu.h | 3 +
> 3 files changed, 322 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index f7141aa2f6..15595f8cd7 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -64,3 +64,9 @@ virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
> virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
> virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
> virtio_iommu_unmap(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
> +virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
> +virtio_iommu_init_iommu_mr(char *iommu_mr) "init %s"
> +virtio_iommu_get_endpoint(uint32_t ep_id) "Alloc endpoint=%d"
> +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"
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 9d1b997df7..e5cc94138b 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -23,6 +23,8 @@
> #include "hw/qdev-properties.h"
> #include "hw/virtio/virtio.h"
> #include "sysemu/kvm.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> #include "trace.h"
>
> #include "standard-headers/linux/virtio_ids.h"
> @@ -30,19 +32,234 @@
> #include "hw/virtio/virtio-bus.h"
> #include "hw/virtio/virtio-access.h"
> #include "hw/virtio/virtio-iommu.h"
> +#include "hw/pci/pci_bus.h"
> +#include "hw/pci/pci.h"
>
> /* Max size */
> #define VIOMMU_DEFAULT_QUEUE_SIZE 256
>
> +typedef struct VirtIOIOMMUDomain {
> + uint32_t id;
> + GTree *mappings;
> + QLIST_HEAD(, VirtIOIOMMUEndpoint) endpoint_list;
> +} VirtIOIOMMUDomain;
> +
> +typedef struct VirtIOIOMMUEndpoint {
> + uint32_t id;
> + VirtIOIOMMUDomain *domain;
> + QLIST_ENTRY(VirtIOIOMMUEndpoint) next;
> +} VirtIOIOMMUEndpoint;
> +
> +typedef struct VirtIOIOMMUInterval {
> + uint64_t low;
> + uint64_t high;
> +} VirtIOIOMMUInterval;
> +
> +static inline uint16_t virtio_iommu_get_bdf(IOMMUDevice *dev)
> +{
> + return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
> +}
> +
> +/**
> + * The bus number is used for lookup when SID based operations occur.
> + * In that case we lazily populate the IOMMUPciBus array from the bus hash
> + * table. At the time the IOMMUPciBus is created (iommu_find_add_as), the bus
> + * numbers may not be always initialized yet.
> + */
> +static IOMMUPciBus *iommu_find_iommu_pcibus(VirtIOIOMMU *s, uint8_t bus_num)
> +{
> + IOMMUPciBus *iommu_pci_bus = s->iommu_pcibus_by_bus_num[bus_num];
> +
> + if (!iommu_pci_bus) {
> + GHashTableIter iter;
> +
> + g_hash_table_iter_init(&iter, s->as_by_busptr);
> + while (g_hash_table_iter_next(&iter, NULL, (void **)&iommu_pci_bus)) {
> + if (pci_bus_num(iommu_pci_bus->bus) == bus_num) {
> + s->iommu_pcibus_by_bus_num[bus_num] = iommu_pci_bus;
> + return iommu_pci_bus;
> + }
> + }
> + return NULL;
> + }
> + return iommu_pci_bus;
> +}
> +
> +static IOMMUMemoryRegion *virtio_iommu_mr(VirtIOIOMMU *s, uint32_t sid)
> +{
> + uint8_t bus_n, devfn;
> + IOMMUPciBus *iommu_pci_bus;
> + IOMMUDevice *dev;
> +
> + bus_n = PCI_BUS_NUM(sid);
> + iommu_pci_bus = iommu_find_iommu_pcibus(s, bus_n);
> + if (iommu_pci_bus) {
> + devfn = sid & PCI_DEVFN_MAX;
> + dev = iommu_pci_bus->pbdev[devfn];
> + if (dev) {
> + return &dev->iommu_mr;
> + }
> + }
> + return NULL;
> +}
> +
> +static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> +{
> + VirtIOIOMMUInterval *inta = (VirtIOIOMMUInterval *)a;
> + VirtIOIOMMUInterval *intb = (VirtIOIOMMUInterval *)b;
> +
> + if (inta->high < intb->low) {
> + return -1;
> + } else if (intb->high < inta->low) {
> + return 1;
> + } else {
> + return 0;
> + }
> +}
> +
> +static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
> +{
> + QLIST_REMOVE(ep, next);
> + g_tree_unref(ep->domain->mappings);
Here domain->mapping is unreferenced for each endpoint, while at [1]
below you only reference the domain->mappings if it's the first
endpoint. Is that problematic?
> + ep->domain = NULL;
> +}
> +
> +static VirtIOIOMMUEndpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
> + uint32_t ep_id)
> +{
> + VirtIOIOMMUEndpoint *ep;
> +
> + ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
> + if (ep) {
> + return ep;
> + }
> + if (!virtio_iommu_mr(s, ep_id)) {
> + return NULL;
> + }
> + ep = g_malloc0(sizeof(*ep));
> + ep->id = ep_id;
> + trace_virtio_iommu_get_endpoint(ep_id);
> + g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
> + return ep;
> +}
> +
> +static void virtio_iommu_put_endpoint(gpointer data)
> +{
> + VirtIOIOMMUEndpoint *ep = (VirtIOIOMMUEndpoint *)data;
> +
> + assert(!ep->domain);
> +
> + trace_virtio_iommu_put_endpoint(ep->id);
> + g_free(ep);
> +}
> +
> +static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
> + uint32_t domain_id)
> +{
> + VirtIOIOMMUDomain *domain;
> +
> + domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
> + if (domain) {
> + return domain;
> + }
> + domain = g_malloc0(sizeof(*domain));
> + domain->id = domain_id;
> + domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
> + NULL, (GDestroyNotify)g_free,
> + (GDestroyNotify)g_free);
> + g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain);
> + QLIST_INIT(&domain->endpoint_list);
> + trace_virtio_iommu_get_domain(domain_id);
> + return domain;
> +}
> +
> +static void virtio_iommu_put_domain(gpointer data)
> +{
> + VirtIOIOMMUDomain *domain = (VirtIOIOMMUDomain *)data;
> + VirtIOIOMMUEndpoint *iter, *tmp;
> +
> + QLIST_FOREACH_SAFE(iter, &domain->endpoint_list, next, tmp) {
> + virtio_iommu_detach_endpoint_from_domain(iter);
> + }
Do you need to destroy the domain->mappings here? Is it leaked?
> + trace_virtio_iommu_put_domain(domain->id);
> + g_free(domain);
> +}
> +
> +static AddressSpace *virtio_iommu_find_add_as(PCIBus *bus, void *opaque,
> + int devfn)
> +{
> + VirtIOIOMMU *s = opaque;
> + IOMMUPciBus *sbus = g_hash_table_lookup(s->as_by_busptr, bus);
> + static uint32_t mr_index;
> + IOMMUDevice *sdev;
> +
> + if (!sbus) {
> + sbus = g_malloc0(sizeof(IOMMUPciBus) +
> + sizeof(IOMMUDevice *) * PCI_DEVFN_MAX);
> + sbus->bus = bus;
> + g_hash_table_insert(s->as_by_busptr, bus, sbus);
> + }
> +
> + sdev = sbus->pbdev[devfn];
> + if (!sdev) {
> + char *name = g_strdup_printf("%s-%d-%d",
> + TYPE_VIRTIO_IOMMU_MEMORY_REGION,
> + mr_index++, devfn);
> + sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
> +
> + sdev->viommu = s;
> + sdev->bus = bus;
> + sdev->devfn = devfn;
> +
> + trace_virtio_iommu_init_iommu_mr(name);
> +
> + memory_region_init_iommu(&sdev->iommu_mr, sizeof(sdev->iommu_mr),
> + TYPE_VIRTIO_IOMMU_MEMORY_REGION,
> + OBJECT(s), name,
> + UINT64_MAX);
> + address_space_init(&sdev->as,
> + MEMORY_REGION(&sdev->iommu_mr), TYPE_VIRTIO_IOMMU);
> + g_free(name);
> + }
> + return &sdev->as;
> +}
> +
> static int virtio_iommu_attach(VirtIOIOMMU *s,
> struct virtio_iommu_req_attach *req)
> {
> uint32_t domain_id = le32_to_cpu(req->domain);
> uint32_t ep_id = le32_to_cpu(req->endpoint);
> + VirtIOIOMMUDomain *domain;
> + VirtIOIOMMUEndpoint *ep;
>
> trace_virtio_iommu_attach(domain_id, ep_id);
>
> - return VIRTIO_IOMMU_S_UNSUPP;
> + ep = virtio_iommu_get_endpoint(s, ep_id);
> + if (!ep) {
> + return VIRTIO_IOMMU_S_NOENT;
> + }
> +
> + if (ep->domain) {
> + VirtIOIOMMUDomain *previous_domain = ep->domain;
> + /*
> + * the device is already attached to a domain,
> + * detach it first
> + */
> + virtio_iommu_detach_endpoint_from_domain(ep);
> + if (QLIST_EMPTY(&previous_domain->endpoint_list)) {
> + g_tree_remove(s->domains, GUINT_TO_POINTER(previous_domain->id));
> + }
> + }
> +
> + domain = virtio_iommu_get_domain(s, domain_id);
> + if (!QLIST_EMPTY(&domain->endpoint_list)) {
> + g_tree_ref(domain->mappings);
[1]
> + }
> + QLIST_INSERT_HEAD(&domain->endpoint_list, ep, next);
> +
> + ep->domain = domain;
> +
> + return VIRTIO_IOMMU_S_OK;
> }
>
> static int virtio_iommu_detach(VirtIOIOMMU *s,
> @@ -50,10 +267,34 @@ static int virtio_iommu_detach(VirtIOIOMMU *s,
> {
> uint32_t domain_id = le32_to_cpu(req->domain);
> uint32_t ep_id = le32_to_cpu(req->endpoint);
> + VirtIOIOMMUDomain *domain;
> + VirtIOIOMMUEndpoint *ep;
>
> trace_virtio_iommu_detach(domain_id, ep_id);
>
> - return VIRTIO_IOMMU_S_UNSUPP;
> + ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
> + if (!ep) {
> + return VIRTIO_IOMMU_S_NOENT;
> + }
> +
> + domain = ep->domain;
> +
> + if (!domain || domain->id != domain_id) {
> + return VIRTIO_IOMMU_S_INVAL;
> + }
> +
> + virtio_iommu_detach_endpoint_from_domain(ep);
> +
> + /*
> + * when the last EP is detached, simply remove the domain for
> + * the domain list and destroy it. Note its mappings were already
> + * freed by the ref count mechanism. Next operation involving
> + * the same domain id will re-create one domain object.
> + */
> + if (QLIST_EMPTY(&domain->endpoint_list)) {
> + g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
> + }
> + return VIRTIO_IOMMU_S_OK;
> }
>
> static int virtio_iommu_map(VirtIOIOMMU *s,
> @@ -172,6 +413,27 @@ out:
> }
> }
>
> +static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> + IOMMUAccessFlags flag,
> + int iommu_idx)
> +{
> + IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
> + uint32_t sid;
> +
> + IOMMUTLBEntry entry = {
> + .target_as = &address_space_memory,
> + .iova = addr,
> + .translated_addr = addr,
> + .addr_mask = ~(hwaddr)0,
> + .perm = IOMMU_NONE,
> + };
> +
> + sid = virtio_iommu_get_bdf(sdev);
> +
> + trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
> + return entry;
> +}
> +
> static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
> {
> VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
> @@ -218,6 +480,13 @@ static const VMStateDescription vmstate_virtio_iommu_device = {
> .unmigratable = 1,
> };
>
> +static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
> +{
> + guint ua = GPOINTER_TO_UINT(a);
> + guint ub = GPOINTER_TO_UINT(b);
> + return (ua > ub) - (ua < ub);
> +}
> +
> static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> @@ -226,6 +495,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
> sizeof(struct virtio_iommu_config));
>
> + memset(s->iommu_pcibus_by_bus_num, 0, sizeof(s->iommu_pcibus_by_bus_num));
> +
> s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
> virtio_iommu_handle_command);
> s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
> @@ -244,18 +515,43 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MMIO);
>
> qemu_mutex_init(&s->mutex);
> +
> + s->as_by_busptr = g_hash_table_new_full(NULL, NULL, NULL, g_free);
> +
> + if (s->primary_bus) {
> + pci_setup_iommu(s->primary_bus, virtio_iommu_find_add_as, s);
> + } else {
> + error_setg(errp, "VIRTIO-IOMMU is not attached to any PCI bus!");
> + }
> }
>
> static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
> +
> + g_tree_destroy(s->domains);
> + g_tree_destroy(s->endpoints);
>
> virtio_cleanup(vdev);
> }
>
> static void virtio_iommu_device_reset(VirtIODevice *vdev)
> {
> + VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> +
> trace_virtio_iommu_device_reset();
> +
> + if (s->domains) {
> + g_tree_destroy(s->domains);
> + }
> + if (s->endpoints) {
> + g_tree_destroy(s->endpoints);
> + }
Is it a must to free domians first then the endpoints here?
I see that virtio_iommu_put_domain() will unlink the domains and
endpoints, then in virtio_iommu_put_endpoint() you assert that
ep->domain is NULL. It's fine but I'm a bit curious on why not call
virtio_iommu_detach_endpoint_from_domain() too when put the endpoint
then iiuc we don't even need this ordering constraint. Though in
virtio_iommu_detach_endpoint_from_domain() we should also need:
if (!ep->domain)
return;
Otherwise it looks good to me. Thanks,
> + s->domains = g_tree_new_full((GCompareDataFunc)int_cmp,
> + NULL, NULL, virtio_iommu_put_domain);
> + s->endpoints = g_tree_new_full((GCompareDataFunc)int_cmp,
> + NULL, NULL, virtio_iommu_put_endpoint);
> }
>
> static void virtio_iommu_set_status(VirtIODevice *vdev, uint8_t status)
> @@ -301,6 +597,14 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
> vdc->vmsd = &vmstate_virtio_iommu_device;
> }
>
> +static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
> + void *data)
> +{
> + IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
> +
> + imrc->translate = virtio_iommu_translate;
> +}
> +
> static const TypeInfo virtio_iommu_info = {
> .name = TYPE_VIRTIO_IOMMU,
> .parent = TYPE_VIRTIO_DEVICE,
> @@ -309,9 +613,16 @@ static const TypeInfo virtio_iommu_info = {
> .class_init = virtio_iommu_class_init,
> };
>
> +static const TypeInfo virtio_iommu_memory_region_info = {
> + .parent = TYPE_IOMMU_MEMORY_REGION,
> + .name = TYPE_VIRTIO_IOMMU_MEMORY_REGION,
> + .class_init = virtio_iommu_memory_region_class_init,
> +};
> +
> static void virtio_register_types(void)
> {
> type_register_static(&virtio_iommu_info);
> + type_register_static(&virtio_iommu_memory_region_info);
> }
>
> type_init(virtio_register_types)
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 041f2c9390..2a2c2ecf83 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -28,6 +28,8 @@
> #define VIRTIO_IOMMU(obj) \
> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
>
> +#define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region"
> +
> typedef struct IOMMUDevice {
> void *viommu;
> PCIBus *bus;
> @@ -48,6 +50,7 @@ typedef struct VirtIOIOMMU {
> struct virtio_iommu_config config;
> uint64_t features;
> GHashTable *as_by_busptr;
> + IOMMUPciBus *iommu_pcibus_by_bus_num[PCI_BUS_MAX];
> PCIBus *primary_bus;
> GTree *domains;
> QemuMutex mutex;
> --
> 2.20.1
>
--
Peter Xu
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 00/10] VIRTIO-IOMMU device
2020-02-03 12:58 ` [PATCH v13 00/10] VIRTIO-IOMMU device Auger Eric
@ 2020-02-03 13:49 ` Peter Xu
2020-02-03 13:51 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Peter Xu @ 2020-02-03 13:49 UTC (permalink / raw)
To: Auger Eric
Cc: peter.maydell, kevin.tian, tnowicki, jean-philippe, quintela,
qemu-devel, dgilbert, bharatb.linux, qemu-arm, mst,
eric.auger.pro
On Mon, Feb 03, 2020 at 01:58:04PM +0100, Auger Eric wrote:
> Hi,
>
> On 1/25/20 6:19 PM, Eric Auger wrote:
> > This series implements the QEMU virtio-iommu device.
> >
> > This matches the v0.12 spec (voted) and the corresponding
> > virtio-iommu driver upstreamed in 5.3. All kernel dependencies
> > are resolved for DT integration. The virtio-iommu can be
> > instantiated in ARM virt using "-device virtio-iommu-pci".
> >
> > Non DT mode is not yet supported as it has non resolved kernel
> > dependencies [1].
> >
> > This feature targets 5.0.
> If possible I would like to make this feature upstream in 5.0. Do you
> guys have other comments/objections?
Hi, Eric, Sorry that I forgot to read the series after a long PTO.
I'll do it today.
--
Peter Xu
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 00/10] VIRTIO-IOMMU device
2020-02-03 13:49 ` Peter Xu
@ 2020-02-03 13:51 ` Auger Eric
0 siblings, 0 replies; 28+ messages in thread
From: Auger Eric @ 2020-02-03 13:51 UTC (permalink / raw)
To: Peter Xu
Cc: peter.maydell, kevin.tian, tnowicki, jean-philippe, quintela,
qemu-devel, dgilbert, bharatb.linux, qemu-arm, mst,
eric.auger.pro
Hi Peter,
On 2/3/20 2:49 PM, Peter Xu wrote:
> On Mon, Feb 03, 2020 at 01:58:04PM +0100, Auger Eric wrote:
>> Hi,
>>
>> On 1/25/20 6:19 PM, Eric Auger wrote:
>>> This series implements the QEMU virtio-iommu device.
>>>
>>> This matches the v0.12 spec (voted) and the corresponding
>>> virtio-iommu driver upstreamed in 5.3. All kernel dependencies
>>> are resolved for DT integration. The virtio-iommu can be
>>> instantiated in ARM virt using "-device virtio-iommu-pci".
>>>
>>> Non DT mode is not yet supported as it has non resolved kernel
>>> dependencies [1].
>>>
>>> This feature targets 5.0.
>> If possible I would like to make this feature upstream in 5.0. Do you
>> guys have other comments/objections?
>
> Hi, Eric, Sorry that I forgot to read the series after a long PTO.
>
> I'll do it today.
No worries ;-)
Thank you in advance
Eric
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support
2020-02-03 13:46 ` Michael S. Tsirkin
@ 2020-02-03 13:52 ` Auger Eric
0 siblings, 0 replies; 28+ messages in thread
From: Auger Eric @ 2020-02-03 13:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: peter.maydell, kevin.tian, tnowicki, quintela, qemu-devel,
peterx, dgilbert, bharatb.linux, qemu-arm, jean-philippe,
eric.auger.pro
Hi Michael,
On 2/3/20 2:46 PM, Michael S. Tsirkin wrote:
> On Mon, Feb 03, 2020 at 02:38:22PM +0100, Auger Eric wrote:
>> Hi Michael,
>>
>> On 2/3/20 2:28 PM, Michael S. Tsirkin wrote:
>>> On Mon, Feb 03, 2020 at 02:20:55PM +0100, Auger Eric wrote:
>>>> Hi Michael,
>>>>
>>>> On 2/3/20 2:03 PM, Michael S. Tsirkin wrote:
>>>>> On Sat, Jan 25, 2020 at 06:19:52PM +0100, Eric Auger wrote:
>>>>>> This patch adds virtio-iommu-pci, which is the pci proxy for
>>>>>> the virtio-iommu device.
>>>>>>
>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>>>>
>>>>> I commented on v11 of this patch:
>>>>>>> Could you send a smaller patchset without pci/acpi bits for now?
>>>>> And you answered:
>>>>>> Yes I am about to send v12.
>>>>>
>>>>> I guess this patch is here by mistake then?
>>>>>
>>>>> I think PCI devices should always have config space so guests are
>>>>> not tempted to find work-arounds. Right?
>>>> No it is not here by mistake. I removed everything related non DT
>>>> integration as we discussed.
>>>>
>>>> DT support is fully upstream even for virtio-iommu-pci.
>>>> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/iommu.txt
>>>>
>>>> So what's wrong implementing that at the moment. As we discussed we
>>>> would use the PCIe config space integration for non DT.
>>>>
>>>> If I use the MMIO based device, I am forced to lock an MMIO region for
>>>> it in the machvirt memory map:
>>>> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/virtio/mmio.txt
>>>>
>>>> I guess Peter (Maydell) will not be happy with this situation either.
>>>>
>>>> Thanks
>>>>
>>>> Eric
>>>
>>> I see. Can't we limit this to DT platforms for now then?
>> That is the case. virtio-iommu does not work with ACPI.
>
>
> Right. But nothing seems to prevent users from creating it
> on systems without dt, and there won't be an easy way for
> management to detect it when qemu added this support down the road.
OK I will add add a check
Thanks
Eric
>
>
>
>> For non DT the plan is to use what you suggested, ie. pass the binding
>> info through the PCIe config space.
>>
>> In that prospect I am waiting for Jean-Philippe's [RFC 00/13]
>> virtio-iommu on non-devicetree platforms respin
>> (https://lore.kernel.org/linux-iommu/20191203190136.00007171@intel.com/T/)
>> and especially patches 12 and 13 of the series, ie. binding info through
>> the PCIe config space.
>>
>> Thanks
>>
>> Eric
>>>
>>>
>>>
>>>>>
>>>>>> ---
>>>>>>
>>>>>> v11 -> v12:
>>>>>> - added Jean's R-b
>>>>>> - remove the array of intervals. Will be introduced later?
>>>>>>
>>>>>> v10 -> v11:
>>>>>> - add the reserved_regions array property
>>>>>>
>>>>>> v9 -> v10:
>>>>>> - include "hw/qdev-properties.h" header
>>>>>>
>>>>>> v8 -> v9:
>>>>>> - add the msi-bypass property
>>>>>> - create virtio-iommu-pci.c
>>>>>> ---
>>>>>> hw/virtio/Makefile.objs | 1 +
>>>>>> hw/virtio/virtio-iommu-pci.c | 88 ++++++++++++++++++++++++++++++++
>>>>>> include/hw/pci/pci.h | 1 +
>>>>>> include/hw/virtio/virtio-iommu.h | 1 +
>>>>>> qdev-monitor.c | 1 +
>>>>>> 5 files changed, 92 insertions(+)
>>>>>> create mode 100644 hw/virtio/virtio-iommu-pci.c
>>>>>>
>>>>>> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
>>>>>> index 2fd9da7410..4e4d39a0a4 100644
>>>>>> --- a/hw/virtio/Makefile.objs
>>>>>> +++ b/hw/virtio/Makefile.objs
>>>>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
>>>>>> obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
>>>>>> obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
>>>>>> obj-$(CONFIG_VIRTIO_BALLOON) += virtio-balloon-pci.o
>>>>>> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu-pci.o
>>>>>> obj-$(CONFIG_VIRTIO_9P) += virtio-9p-pci.o
>>>>>> obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi-pci.o
>>>>>> obj-$(CONFIG_VIRTIO_BLK) += virtio-blk-pci.o
>>>>>> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
>>>>>> new file mode 100644
>>>>>> index 0000000000..4cfae1f9df
>>>>>> --- /dev/null
>>>>>> +++ b/hw/virtio/virtio-iommu-pci.c
>>>>>> @@ -0,0 +1,88 @@
>>>>>> +/*
>>>>>> + * Virtio IOMMU PCI Bindings
>>>>>> + *
>>>>>> + * Copyright (c) 2019 Red Hat, Inc.
>>>>>> + * Written by Eric Auger
>>>>>> + *
>>>>>> + * This program is free software; you can redistribute it and/or modify
>>>>>> + * it under the terms of the GNU General Public License version 2 or
>>>>>> + * (at your option) any later version.
>>>>>> + */
>>>>>> +
>>>>>> +#include "qemu/osdep.h"
>>>>>> +
>>>>>> +#include "virtio-pci.h"
>>>>>> +#include "hw/virtio/virtio-iommu.h"
>>>>>> +#include "hw/qdev-properties.h"
>>>>>> +
>>>>>> +typedef struct VirtIOIOMMUPCI VirtIOIOMMUPCI;
>>>>>> +
>>>>>> +/*
>>>>>> + * virtio-iommu-pci: This extends VirtioPCIProxy.
>>>>>> + *
>>>>>> + */
>>>>>> +#define VIRTIO_IOMMU_PCI(obj) \
>>>>>> + OBJECT_CHECK(VirtIOIOMMUPCI, (obj), TYPE_VIRTIO_IOMMU_PCI)
>>>>>> +
>>>>>> +struct VirtIOIOMMUPCI {
>>>>>> + VirtIOPCIProxy parent_obj;
>>>>>> + VirtIOIOMMU vdev;
>>>>>> +};
>>>>>> +
>>>>>> +static Property virtio_iommu_pci_properties[] = {
>>>>>> + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
>>>>>> + DEFINE_PROP_END_OF_LIST(),
>>>>>> +};
>>>>>> +
>>>>>> +static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>>>>>> +{
>>>>>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev);
>>>>>> + DeviceState *vdev = DEVICE(&dev->vdev);
>>>>>> +
>>>>>> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>>>>>> + object_property_set_link(OBJECT(dev),
>>>>>> + OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
>>>>>> + "primary-bus", errp);
>>>>>> + object_property_set_bool(OBJECT(vdev), true, "realized", errp);
>>>>>> +}
>>>>>> +
>>>>>> +static void virtio_iommu_pci_class_init(ObjectClass *klass, void *data)
>>>>>> +{
>>>>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>>>>> + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
>>>>>> + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
>>>>>> + k->realize = virtio_iommu_pci_realize;
>>>>>> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>>>>> + dc->props = virtio_iommu_pci_properties;
>>>>>> + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
>>>>>> + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_IOMMU;
>>>>>> + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
>>>>>> + pcidev_k->class_id = PCI_CLASS_OTHERS;
>>>>>> +}
>>>>>> +
>>>>>> +static void virtio_iommu_pci_instance_init(Object *obj)
>>>>>> +{
>>>>>> + VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
>>>>>> +
>>>>>> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
>>>>>> + TYPE_VIRTIO_IOMMU);
>>>>>> +}
>>>>>> +
>>>>>> +static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
>>>>>> + .base_name = TYPE_VIRTIO_IOMMU_PCI,
>>>>>> + .generic_name = "virtio-iommu-pci",
>>>>>> + .transitional_name = "virtio-iommu-pci-transitional",
>>>>>> + .non_transitional_name = "virtio-iommu-pci-non-transitional",
>>>>>> + .instance_size = sizeof(VirtIOIOMMUPCI),
>>>>>> + .instance_init = virtio_iommu_pci_instance_init,
>>>>>> + .class_init = virtio_iommu_pci_class_init,
>>>>>> +};
>>>>>> +
>>>>>> +static void virtio_iommu_pci_register(void)
>>>>>> +{
>>>>>> + virtio_pci_types_register(&virtio_iommu_pci_info);
>>>>>> +}
>>>>>> +
>>>>>> +type_init(virtio_iommu_pci_register)
>>>>>> +
>>>>>> +
>>>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>>>>> index 2acd8321af..cfedf5a995 100644
>>>>>> --- a/include/hw/pci/pci.h
>>>>>> +++ b/include/hw/pci/pci.h
>>>>>> @@ -86,6 +86,7 @@ extern bool pci_available;
>>>>>> #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
>>>>>> #define PCI_DEVICE_ID_VIRTIO_VSOCK 0x1012
>>>>>> #define PCI_DEVICE_ID_VIRTIO_PMEM 0x1013
>>>>>> +#define PCI_DEVICE_ID_VIRTIO_IOMMU 0x1014
>>>>>>
>>>>>> #define PCI_VENDOR_ID_REDHAT 0x1b36
>>>>>> #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
>>>>>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>>>>>> index 2a2c2ecf83..f39aa0fbb4 100644
>>>>>> --- a/include/hw/virtio/virtio-iommu.h
>>>>>> +++ b/include/hw/virtio/virtio-iommu.h
>>>>>> @@ -25,6 +25,7 @@
>>>>>> #include "hw/pci/pci.h"
>>>>>>
>>>>>> #define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
>>>>>> +#define TYPE_VIRTIO_IOMMU_PCI "virtio-iommu-device-base"
>>>>>> #define VIRTIO_IOMMU(obj) \
>>>>>> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
>>>>>>
>>>>>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>>>>>> index 3465a1e2d0..97f4022b51 100644
>>>>>> --- a/qdev-monitor.c
>>>>>> +++ b/qdev-monitor.c
>>>>>> @@ -66,6 +66,7 @@ static const QDevAlias qdev_alias_table[] = {
>>>>>> { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
>>>>>> { "virtio-input-host-pci", "virtio-input-host",
>>>>>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>>>>>> + { "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>>>>>> { "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
>>>>>> { "virtio-keyboard-pci", "virtio-keyboard",
>>>>>> QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>>>>>> --
>>>>>> 2.20.1
>>>>>
>>>>>
>>>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 06/10] virtio-iommu: Implement fault reporting
2020-01-25 17:19 ` [PATCH v13 06/10] virtio-iommu: Implement fault reporting Eric Auger
@ 2020-02-03 13:55 ` Peter Xu
2020-02-03 15:01 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Peter Xu @ 2020-02-03 13:55 UTC (permalink / raw)
To: Eric Auger
Cc: peter.maydell, kevin.tian, tnowicki, jean-philippe, quintela,
qemu-devel, dgilbert, bharatb.linux, qemu-arm, mst,
eric.auger.pro
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
2020-02-03 13:49 ` Peter Xu
@ 2020-02-03 14:59 ` Auger Eric
2020-02-03 15:19 ` Peter Xu
0 siblings, 1 reply; 28+ messages in thread
From: Auger Eric @ 2020-02-03 14:59 UTC (permalink / raw)
To: Peter Xu
Cc: peter.maydell, kevin.tian, tnowicki, quintela, mst, qemu-devel,
dgilbert, bharatb.linux, qemu-arm, jean-philippe, eric.auger.pro
Hi Peter,
On 2/3/20 2:49 PM, Peter Xu wrote:
> On Sat, Jan 25, 2020 at 06:19:48PM +0100, Eric Auger wrote:
>> This patch implements the endpoint attach/detach to/from
>> a domain.
>>
>> Domain and endpoint internal datatypes are introduced.
>> Both are stored in RB trees. The domain owns a list of
>> endpoints attached to it. Also helpers to get/put
>> end points and domains are introduced.
>>
>> As for the IOMMU memory regions, a callback is called on
>> PCI bus enumeration that initializes for a given device
>> on the bus hierarchy an IOMMU memory region. The PCI bus
>> hierarchy is stored locally in IOMMUPciBus and IOMMUDevice
>> objects.
>>
>> At the time of the enumeration, the bus number may not be
>> computed yet.
>>
>> So operations that will need to retrieve the IOMMUdevice
>> and its IOMMU memory region from the bus number and devfn,
>> once the bus number is garanteed to be frozen, use an array
>> of IOMMUPciBus, lazily populated.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v12 -> v13:
>> - squashed v12 4, 5, 6 into this patch
>> - rename virtio_iommu_get_sid into virtio_iommu_get_bdf
>>
>> v11 -> v12:
>> - check the device is protected by the iommu on attach
>> - on detach, check the domain id the device is attached to matches
>> the one used in the detach command
>> - fix mapping ref counter and destroy the domain when no end-points
>> are attached anymore
>> ---
>> hw/virtio/trace-events | 6 +
>> hw/virtio/virtio-iommu.c | 315 ++++++++++++++++++++++++++++++-
>> include/hw/virtio/virtio-iommu.h | 3 +
>> 3 files changed, 322 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>> index f7141aa2f6..15595f8cd7 100644
>> --- a/hw/virtio/trace-events
>> +++ b/hw/virtio/trace-events
>> @@ -64,3 +64,9 @@ virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>> virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>> virtio_iommu_map(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64 " phys_start=0x%"PRIx64" flags=%d"
>> virtio_iommu_unmap(uint32_t domain_id, uint64_t virt_start, uint64_t virt_end) "domain=%d virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
>> +virtio_iommu_translate(const char *name, uint32_t rid, uint64_t iova, int flag) "mr=%s rid=%d addr=0x%"PRIx64" flag=%d"
>> +virtio_iommu_init_iommu_mr(char *iommu_mr) "init %s"
>> +virtio_iommu_get_endpoint(uint32_t ep_id) "Alloc endpoint=%d"
>> +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"
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index 9d1b997df7..e5cc94138b 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -23,6 +23,8 @@
>> #include "hw/qdev-properties.h"
>> #include "hw/virtio/virtio.h"
>> #include "sysemu/kvm.h"
>> +#include "qapi/error.h"
>> +#include "qemu/error-report.h"
>> #include "trace.h"
>>
>> #include "standard-headers/linux/virtio_ids.h"
>> @@ -30,19 +32,234 @@
>> #include "hw/virtio/virtio-bus.h"
>> #include "hw/virtio/virtio-access.h"
>> #include "hw/virtio/virtio-iommu.h"
>> +#include "hw/pci/pci_bus.h"
>> +#include "hw/pci/pci.h"
>>
>> /* Max size */
>> #define VIOMMU_DEFAULT_QUEUE_SIZE 256
>>
>> +typedef struct VirtIOIOMMUDomain {
>> + uint32_t id;
>> + GTree *mappings;
>> + QLIST_HEAD(, VirtIOIOMMUEndpoint) endpoint_list;
>> +} VirtIOIOMMUDomain;
>> +
>> +typedef struct VirtIOIOMMUEndpoint {
>> + uint32_t id;
>> + VirtIOIOMMUDomain *domain;
>> + QLIST_ENTRY(VirtIOIOMMUEndpoint) next;
>> +} VirtIOIOMMUEndpoint;
>> +
>> +typedef struct VirtIOIOMMUInterval {
>> + uint64_t low;
>> + uint64_t high;
>> +} VirtIOIOMMUInterval;
>> +
>> +static inline uint16_t virtio_iommu_get_bdf(IOMMUDevice *dev)
>> +{
>> + return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
>> +}
>> +
>> +/**
>> + * The bus number is used for lookup when SID based operations occur.
>> + * In that case we lazily populate the IOMMUPciBus array from the bus hash
>> + * table. At the time the IOMMUPciBus is created (iommu_find_add_as), the bus
>> + * numbers may not be always initialized yet.
>> + */
>> +static IOMMUPciBus *iommu_find_iommu_pcibus(VirtIOIOMMU *s, uint8_t bus_num)
>> +{
>> + IOMMUPciBus *iommu_pci_bus = s->iommu_pcibus_by_bus_num[bus_num];
>> +
>> + if (!iommu_pci_bus) {
>> + GHashTableIter iter;
>> +
>> + g_hash_table_iter_init(&iter, s->as_by_busptr);
>> + while (g_hash_table_iter_next(&iter, NULL, (void **)&iommu_pci_bus)) {
>> + if (pci_bus_num(iommu_pci_bus->bus) == bus_num) {
>> + s->iommu_pcibus_by_bus_num[bus_num] = iommu_pci_bus;
>> + return iommu_pci_bus;
>> + }
>> + }
>> + return NULL;
>> + }
>> + return iommu_pci_bus;
>> +}
>> +
>> +static IOMMUMemoryRegion *virtio_iommu_mr(VirtIOIOMMU *s, uint32_t sid)
>> +{
>> + uint8_t bus_n, devfn;
>> + IOMMUPciBus *iommu_pci_bus;
>> + IOMMUDevice *dev;
>> +
>> + bus_n = PCI_BUS_NUM(sid);
>> + iommu_pci_bus = iommu_find_iommu_pcibus(s, bus_n);
>> + if (iommu_pci_bus) {
>> + devfn = sid & PCI_DEVFN_MAX;
>> + dev = iommu_pci_bus->pbdev[devfn];
>> + if (dev) {
>> + return &dev->iommu_mr;
>> + }
>> + }
>> + return NULL;
>> +}
>> +
>> +static gint interval_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
>> +{
>> + VirtIOIOMMUInterval *inta = (VirtIOIOMMUInterval *)a;
>> + VirtIOIOMMUInterval *intb = (VirtIOIOMMUInterval *)b;
>> +
>> + if (inta->high < intb->low) {
>> + return -1;
>> + } else if (intb->high < inta->low) {
>> + return 1;
>> + } else {
>> + return 0;
>> + }
>> +}
>> +
>> +static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
>> +{
>> + QLIST_REMOVE(ep, next);
>> + g_tree_unref(ep->domain->mappings);
>
> Here domain->mapping is unreferenced for each endpoint, while at [1]
> below you only reference the domain->mappings if it's the first
> endpoint. Is that problematic?
in [1] I take a ref to the domain->mappings if it is *not* the 1st
endpoint. This aims at deleting the mappings gtree when the last EP is
detached from the domain.
This fixes the issue reported by Jean in:
https://patchwork.kernel.org/patch/11258267/#23046313
>
>> + ep->domain = NULL;
>> +}
>> +
>> +static VirtIOIOMMUEndpoint *virtio_iommu_get_endpoint(VirtIOIOMMU *s,
>> + uint32_t ep_id)
>> +{
>> + VirtIOIOMMUEndpoint *ep;
>> +
>> + ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
>> + if (ep) {
>> + return ep;
>> + }
>> + if (!virtio_iommu_mr(s, ep_id)) {
>> + return NULL;
>> + }
>> + ep = g_malloc0(sizeof(*ep));
>> + ep->id = ep_id;
>> + trace_virtio_iommu_get_endpoint(ep_id);
>> + g_tree_insert(s->endpoints, GUINT_TO_POINTER(ep_id), ep);
>> + return ep;
>> +}
>> +
>> +static void virtio_iommu_put_endpoint(gpointer data)
>> +{
>> + VirtIOIOMMUEndpoint *ep = (VirtIOIOMMUEndpoint *)data;
>> +
>> + assert(!ep->domain);
>> +
>> + trace_virtio_iommu_put_endpoint(ep->id);
>> + g_free(ep);
>> +}
>> +
>> +static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
>> + uint32_t domain_id)
>> +{
>> + VirtIOIOMMUDomain *domain;
>> +
>> + domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
>> + if (domain) {
>> + return domain;
>> + }
>> + domain = g_malloc0(sizeof(*domain));
>> + domain->id = domain_id;
>> + domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
>> + NULL, (GDestroyNotify)g_free,
>> + (GDestroyNotify)g_free);
>> + g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain);
>> + QLIST_INIT(&domain->endpoint_list);
>> + trace_virtio_iommu_get_domain(domain_id);
>> + return domain;
>> +}
>> +
>> +static void virtio_iommu_put_domain(gpointer data)
>> +{
>> + VirtIOIOMMUDomain *domain = (VirtIOIOMMUDomain *)data;
>> + VirtIOIOMMUEndpoint *iter, *tmp;
>> +
>> + QLIST_FOREACH_SAFE(iter, &domain->endpoint_list, next, tmp) {
>> + virtio_iommu_detach_endpoint_from_domain(iter);
>> + }
>
> Do you need to destroy the domain->mappings here? Is it leaked?
AFIU as we detach all EPs in the loop above, the whole "mappings" gtree
is destroyed so there is no leak.
>
>> + trace_virtio_iommu_put_domain(domain->id);
>> + g_free(domain);
>> +}
>> +
>> +static AddressSpace *virtio_iommu_find_add_as(PCIBus *bus, void *opaque,
>> + int devfn)
>> +{
>> + VirtIOIOMMU *s = opaque;
>> + IOMMUPciBus *sbus = g_hash_table_lookup(s->as_by_busptr, bus);
>> + static uint32_t mr_index;
>> + IOMMUDevice *sdev;
>> +
>> + if (!sbus) {
>> + sbus = g_malloc0(sizeof(IOMMUPciBus) +
>> + sizeof(IOMMUDevice *) * PCI_DEVFN_MAX);
>> + sbus->bus = bus;
>> + g_hash_table_insert(s->as_by_busptr, bus, sbus);
>> + }
>> +
>> + sdev = sbus->pbdev[devfn];
>> + if (!sdev) {
>> + char *name = g_strdup_printf("%s-%d-%d",
>> + TYPE_VIRTIO_IOMMU_MEMORY_REGION,
>> + mr_index++, devfn);
>> + sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
>> +
>> + sdev->viommu = s;
>> + sdev->bus = bus;
>> + sdev->devfn = devfn;
>> +
>> + trace_virtio_iommu_init_iommu_mr(name);
>> +
>> + memory_region_init_iommu(&sdev->iommu_mr, sizeof(sdev->iommu_mr),
>> + TYPE_VIRTIO_IOMMU_MEMORY_REGION,
>> + OBJECT(s), name,
>> + UINT64_MAX);
>> + address_space_init(&sdev->as,
>> + MEMORY_REGION(&sdev->iommu_mr), TYPE_VIRTIO_IOMMU);
>> + g_free(name);
>> + }
>> + return &sdev->as;
>> +}
>> +
>> static int virtio_iommu_attach(VirtIOIOMMU *s,
>> struct virtio_iommu_req_attach *req)
>> {
>> uint32_t domain_id = le32_to_cpu(req->domain);
>> uint32_t ep_id = le32_to_cpu(req->endpoint);
>> + VirtIOIOMMUDomain *domain;
>> + VirtIOIOMMUEndpoint *ep;
>>
>> trace_virtio_iommu_attach(domain_id, ep_id);
>>
>> - return VIRTIO_IOMMU_S_UNSUPP;
>> + ep = virtio_iommu_get_endpoint(s, ep_id);
>> + if (!ep) {
>> + return VIRTIO_IOMMU_S_NOENT;
>> + }
>> +
>> + if (ep->domain) {
>> + VirtIOIOMMUDomain *previous_domain = ep->domain;
>> + /*
>> + * the device is already attached to a domain,
>> + * detach it first
>> + */
>> + virtio_iommu_detach_endpoint_from_domain(ep);
>> + if (QLIST_EMPTY(&previous_domain->endpoint_list)) {
>> + g_tree_remove(s->domains, GUINT_TO_POINTER(previous_domain->id));
>> + }
>> + }
>> +
>> + domain = virtio_iommu_get_domain(s, domain_id);
>> + if (!QLIST_EMPTY(&domain->endpoint_list)) {
>> + g_tree_ref(domain->mappings);
>
> [1]
!QLIST_EMPTY
>
>> + }
>> + QLIST_INSERT_HEAD(&domain->endpoint_list, ep, next);
>> +
>> + ep->domain = domain;
>> +
>> + return VIRTIO_IOMMU_S_OK;
>> }
>>
>> static int virtio_iommu_detach(VirtIOIOMMU *s,
>> @@ -50,10 +267,34 @@ static int virtio_iommu_detach(VirtIOIOMMU *s,
>> {
>> uint32_t domain_id = le32_to_cpu(req->domain);
>> uint32_t ep_id = le32_to_cpu(req->endpoint);
>> + VirtIOIOMMUDomain *domain;
>> + VirtIOIOMMUEndpoint *ep;
>>
>> trace_virtio_iommu_detach(domain_id, ep_id);
>>
>> - return VIRTIO_IOMMU_S_UNSUPP;
>> + ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
>> + if (!ep) {
>> + return VIRTIO_IOMMU_S_NOENT;
>> + }
>> +
>> + domain = ep->domain;
>> +
>> + if (!domain || domain->id != domain_id) {
>> + return VIRTIO_IOMMU_S_INVAL;
>> + }
>> +
>> + virtio_iommu_detach_endpoint_from_domain(ep);
>> +
>> + /*
>> + * when the last EP is detached, simply remove the domain for
>> + * the domain list and destroy it. Note its mappings were already
>> + * freed by the ref count mechanism. Next operation involving
>> + * the same domain id will re-create one domain object.
>> + */
>> + if (QLIST_EMPTY(&domain->endpoint_list)) {
>> + g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
>> + }
>> + return VIRTIO_IOMMU_S_OK;
>> }
>>
>> static int virtio_iommu_map(VirtIOIOMMU *s,
>> @@ -172,6 +413,27 @@ out:
>> }
>> }
>>
>> +static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>> + IOMMUAccessFlags flag,
>> + int iommu_idx)
>> +{
>> + IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
>> + uint32_t sid;
>> +
>> + IOMMUTLBEntry entry = {
>> + .target_as = &address_space_memory,
>> + .iova = addr,
>> + .translated_addr = addr,
>> + .addr_mask = ~(hwaddr)0,
>> + .perm = IOMMU_NONE,
>> + };
>> +
>> + sid = virtio_iommu_get_bdf(sdev);
>> +
>> + trace_virtio_iommu_translate(mr->parent_obj.name, sid, addr, flag);
>> + return entry;
>> +}
>> +
>> static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
>> {
>> VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
>> @@ -218,6 +480,13 @@ static const VMStateDescription vmstate_virtio_iommu_device = {
>> .unmigratable = 1,
>> };
>>
>> +static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
>> +{
>> + guint ua = GPOINTER_TO_UINT(a);
>> + guint ub = GPOINTER_TO_UINT(b);
>> + return (ua > ub) - (ua < ub);
>> +}
>> +
>> static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>> {
>> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> @@ -226,6 +495,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>> virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
>> sizeof(struct virtio_iommu_config));
>>
>> + memset(s->iommu_pcibus_by_bus_num, 0, sizeof(s->iommu_pcibus_by_bus_num));
>> +
>> s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
>> virtio_iommu_handle_command);
>> s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
>> @@ -244,18 +515,43 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>> virtio_add_feature(&s->features, VIRTIO_IOMMU_F_MMIO);
>>
>> qemu_mutex_init(&s->mutex);
>> +
>> + s->as_by_busptr = g_hash_table_new_full(NULL, NULL, NULL, g_free);
>> +
>> + if (s->primary_bus) {
>> + pci_setup_iommu(s->primary_bus, virtio_iommu_find_add_as, s);
>> + } else {
>> + error_setg(errp, "VIRTIO-IOMMU is not attached to any PCI bus!");
>> + }
>> }
>>
>> static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
>> {
>> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> + VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
>> +
>> + g_tree_destroy(s->domains);
>> + g_tree_destroy(s->endpoints);
>>
>> virtio_cleanup(vdev);
>> }
>>
>> static void virtio_iommu_device_reset(VirtIODevice *vdev)
>> {
>> + VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
>> +
>> trace_virtio_iommu_device_reset();
>> +
>> + if (s->domains) {
>> + g_tree_destroy(s->domains);
>> + }
>> + if (s->endpoints) {
>> + g_tree_destroy(s->endpoints);
>> + }
>
> Is it a must to free domians first then the endpoints here?
>
> I see that virtio_iommu_put_domain() will unlink the domains and
> endpoints, then in virtio_iommu_put_endpoint() you assert that
> ep->domain is NULL. It's fine but I'm a bit curious on why not call
> virtio_iommu_detach_endpoint_from_domain() too when put the endpoint
> then iiuc we don't even need this ordering constraint. Though in
> virtio_iommu_detach_endpoint_from_domain() we should also need:
Yes today an EP is meant to be deleted if it is detached from any domain.
I may add virtio_iommu_detach_endpoint_from_domain in put_domain though.
>
> if (!ep->domain)
> return;
>
> Otherwise it looks good to me. Thanks,
Thanks
Eric
>
>> + s->domains = g_tree_new_full((GCompareDataFunc)int_cmp,
>> + NULL, NULL, virtio_iommu_put_domain);
>> + s->endpoints = g_tree_new_full((GCompareDataFunc)int_cmp,
>> + NULL, NULL, virtio_iommu_put_endpoint);
>> }
>>
>> static void virtio_iommu_set_status(VirtIODevice *vdev, uint8_t status)
>> @@ -301,6 +597,14 @@ static void virtio_iommu_class_init(ObjectClass *klass, void *data)
>> vdc->vmsd = &vmstate_virtio_iommu_device;
>> }
>>
>> +static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
>> + void *data)
>> +{
>> + IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
>> +
>> + imrc->translate = virtio_iommu_translate;
>> +}
>> +
>> static const TypeInfo virtio_iommu_info = {
>> .name = TYPE_VIRTIO_IOMMU,
>> .parent = TYPE_VIRTIO_DEVICE,
>> @@ -309,9 +613,16 @@ static const TypeInfo virtio_iommu_info = {
>> .class_init = virtio_iommu_class_init,
>> };
>>
>> +static const TypeInfo virtio_iommu_memory_region_info = {
>> + .parent = TYPE_IOMMU_MEMORY_REGION,
>> + .name = TYPE_VIRTIO_IOMMU_MEMORY_REGION,
>> + .class_init = virtio_iommu_memory_region_class_init,
>> +};
>> +
>> static void virtio_register_types(void)
>> {
>> type_register_static(&virtio_iommu_info);
>> + type_register_static(&virtio_iommu_memory_region_info);
>> }
>>
>> type_init(virtio_register_types)
>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>> index 041f2c9390..2a2c2ecf83 100644
>> --- a/include/hw/virtio/virtio-iommu.h
>> +++ b/include/hw/virtio/virtio-iommu.h
>> @@ -28,6 +28,8 @@
>> #define VIRTIO_IOMMU(obj) \
>> OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
>>
>> +#define TYPE_VIRTIO_IOMMU_MEMORY_REGION "virtio-iommu-memory-region"
>> +
>> typedef struct IOMMUDevice {
>> void *viommu;
>> PCIBus *bus;
>> @@ -48,6 +50,7 @@ typedef struct VirtIOIOMMU {
>> struct virtio_iommu_config config;
>> uint64_t features;
>> GHashTable *as_by_busptr;
>> + IOMMUPciBus *iommu_pcibus_by_bus_num[PCI_BUS_MAX];
>> PCIBus *primary_bus;
>> GTree *domains;
>> QemuMutex mutex;
>> --
>> 2.20.1
>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 06/10] virtio-iommu: Implement fault reporting
2020-02-03 13:55 ` Peter Xu
@ 2020-02-03 15:01 ` Auger Eric
0 siblings, 0 replies; 28+ messages in thread
From: Auger Eric @ 2020-02-03 15:01 UTC (permalink / raw)
To: Peter Xu
Cc: peter.maydell, kevin.tian, tnowicki, jean-philippe, quintela,
qemu-devel, dgilbert, bharatb.linux, qemu-arm, mst,
eric.auger.pro
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
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
2020-02-03 14:59 ` Auger Eric
@ 2020-02-03 15:19 ` Peter Xu
2020-02-03 17:46 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Peter Xu @ 2020-02-03 15:19 UTC (permalink / raw)
To: Auger Eric
Cc: peter.maydell, kevin.tian, tnowicki, quintela, mst, qemu-devel,
dgilbert, bharatb.linux, qemu-arm, jean-philippe, eric.auger.pro
On Mon, Feb 03, 2020 at 03:59:00PM +0100, Auger Eric wrote:
[...]
> >> +static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
> >> +{
> >> + QLIST_REMOVE(ep, next);
> >> + g_tree_unref(ep->domain->mappings);
> >
> > Here domain->mapping is unreferenced for each endpoint, while at [1]
> > below you only reference the domain->mappings if it's the first
> > endpoint. Is that problematic?
> in [1] I take a ref to the domain->mappings if it is *not* the 1st
> endpoint. This aims at deleting the mappings gtree when the last EP is
> detached from the domain.
>
> This fixes the issue reported by Jean in:
> https://patchwork.kernel.org/patch/11258267/#23046313
Ah OK. :)
However this is tricky. How about do explicit g_tree_destroy() in
virtio_iommu_detach() when it's the last endpoint? I see that you
have:
/*
* when the last EP is detached, simply remove the domain for
* the domain list and destroy it. Note its mappings were already
* freed by the ref count mechanism. Next operation involving
* the same domain id will re-create one domain object.
*/
if (QLIST_EMPTY(&domain->endpoint_list)) {
g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
}
Then it becomes:
if (QLIST_EMPTY(&domain->endpoint_list)) {
g_tree_destroy(domain->mappings);
g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
}
And also remove the trick in attach() so you take the domain ref
unconditionally. Would that work?
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
2020-02-03 15:19 ` Peter Xu
@ 2020-02-03 17:46 ` Auger Eric
2020-02-03 18:19 ` Peter Xu
0 siblings, 1 reply; 28+ messages in thread
From: Auger Eric @ 2020-02-03 17:46 UTC (permalink / raw)
To: Peter Xu
Cc: peter.maydell, kevin.tian, tnowicki, mst, quintela, qemu-devel,
dgilbert, bharatb.linux, qemu-arm, jean-philippe, eric.auger.pro
Hi Peter,
On 2/3/20 4:19 PM, Peter Xu wrote:
> On Mon, Feb 03, 2020 at 03:59:00PM +0100, Auger Eric wrote:
>
> [...]
>
>>>> +static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
>>>> +{
>>>> + QLIST_REMOVE(ep, next);
>>>> + g_tree_unref(ep->domain->mappings);
>>>
>>> Here domain->mapping is unreferenced for each endpoint, while at [1]
>>> below you only reference the domain->mappings if it's the first
>>> endpoint. Is that problematic?
>> in [1] I take a ref to the domain->mappings if it is *not* the 1st
>> endpoint. This aims at deleting the mappings gtree when the last EP is
>> detached from the domain.
>>
>> This fixes the issue reported by Jean in:
>> https://patchwork.kernel.org/patch/11258267/#23046313
>
> Ah OK. :)
>
> However this is tricky. How about do explicit g_tree_destroy() in
> virtio_iommu_detach() when it's the last endpoint? I see that you
> have:
>
> /*
> * when the last EP is detached, simply remove the domain for
> * the domain list and destroy it. Note its mappings were already
> * freed by the ref count mechanism. Next operation involving
> * the same domain id will re-create one domain object.
> */
> if (QLIST_EMPTY(&domain->endpoint_list)) {
> g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
> }
>
> Then it becomes:
>
> if (QLIST_EMPTY(&domain->endpoint_list)) {
> g_tree_destroy(domain->mappings);
> g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
> }
>
> And also remove the trick in attach() so you take the domain ref
> unconditionally. Would that work?
Yes I think so. On the other hand this ref counting mechanism is also
made for that purpose of destroying objects without being forced to
explicitly call the destroy function.
Thanks
Eric
>
> Thanks,
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
2020-02-03 17:46 ` Auger Eric
@ 2020-02-03 18:19 ` Peter Xu
2020-02-04 12:26 ` Auger Eric
0 siblings, 1 reply; 28+ messages in thread
From: Peter Xu @ 2020-02-03 18:19 UTC (permalink / raw)
To: Auger Eric
Cc: peter.maydell, kevin.tian, tnowicki, quintela, mst, qemu-devel,
dgilbert, bharatb.linux, qemu-arm, jean-philippe, eric.auger.pro
On Mon, Feb 03, 2020 at 06:46:36PM +0100, Auger Eric wrote:
> Hi Peter,
>
> On 2/3/20 4:19 PM, Peter Xu wrote:
> > On Mon, Feb 03, 2020 at 03:59:00PM +0100, Auger Eric wrote:
> >
> > [...]
> >
> >>>> +static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
> >>>> +{
> >>>> + QLIST_REMOVE(ep, next);
> >>>> + g_tree_unref(ep->domain->mappings);
> >>>
> >>> Here domain->mapping is unreferenced for each endpoint, while at [1]
> >>> below you only reference the domain->mappings if it's the first
> >>> endpoint. Is that problematic?
> >> in [1] I take a ref to the domain->mappings if it is *not* the 1st
> >> endpoint. This aims at deleting the mappings gtree when the last EP is
> >> detached from the domain.
> >>
> >> This fixes the issue reported by Jean in:
> >> https://patchwork.kernel.org/patch/11258267/#23046313
> >
> > Ah OK. :)
> >
> > However this is tricky. How about do explicit g_tree_destroy() in
> > virtio_iommu_detach() when it's the last endpoint? I see that you
> > have:
> >
> > /*
> > * when the last EP is detached, simply remove the domain for
> > * the domain list and destroy it. Note its mappings were already
> > * freed by the ref count mechanism. Next operation involving
> > * the same domain id will re-create one domain object.
> > */
> > if (QLIST_EMPTY(&domain->endpoint_list)) {
> > g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
> > }
> >
> > Then it becomes:
> >
> > if (QLIST_EMPTY(&domain->endpoint_list)) {
> > g_tree_destroy(domain->mappings);
> > g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
> > }
> >
> > And also remove the trick in attach() so you take the domain ref
> > unconditionally. Would that work?
> Yes I think so. On the other hand this ref counting mechanism is also
> made for that purpose of destroying objects without being forced to
> explicitly call the destroy function.
IMHO that's two different things. g_tree_destroy() should be the same
as g_tree_unref() here when the tree is empty. It's really a matter
of easy reading of code:
void
g_tree_destroy (GTree *tree)
{
g_return_if_fail (tree != NULL);
g_tree_remove_all (tree);
g_tree_unref (tree);
}
What we really changed here is to allow the ref/unref to be clearly
paired, i.e., for each EP it'll ref once and unref once. The prvious
solution has the trick in that the 1st EP don't ref, the latter EPs
ref, and when the domain quits it doesn't unref to match the first
ref. It's error prone to me. Then, if we can do it in the paired way
easily, I don't see why not...
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v13 03/10] virtio-iommu: Implement attach/detach command
2020-02-03 18:19 ` Peter Xu
@ 2020-02-04 12:26 ` Auger Eric
0 siblings, 0 replies; 28+ messages in thread
From: Auger Eric @ 2020-02-04 12:26 UTC (permalink / raw)
To: Peter Xu
Cc: peter.maydell, kevin.tian, tnowicki, quintela, mst, qemu-devel,
dgilbert, bharatb.linux, qemu-arm, jean-philippe, eric.auger.pro
Hi Peter,
On 2/3/20 7:19 PM, Peter Xu wrote:
> On Mon, Feb 03, 2020 at 06:46:36PM +0100, Auger Eric wrote:
>> Hi Peter,
>>
>> On 2/3/20 4:19 PM, Peter Xu wrote:
>>> On Mon, Feb 03, 2020 at 03:59:00PM +0100, Auger Eric wrote:
>>>
>>> [...]
>>>
>>>>>> +static void virtio_iommu_detach_endpoint_from_domain(VirtIOIOMMUEndpoint *ep)
>>>>>> +{
>>>>>> + QLIST_REMOVE(ep, next);
>>>>>> + g_tree_unref(ep->domain->mappings);
>>>>>
>>>>> Here domain->mapping is unreferenced for each endpoint, while at [1]
>>>>> below you only reference the domain->mappings if it's the first
>>>>> endpoint. Is that problematic?
>>>> in [1] I take a ref to the domain->mappings if it is *not* the 1st
>>>> endpoint. This aims at deleting the mappings gtree when the last EP is
>>>> detached from the domain.
>>>>
>>>> This fixes the issue reported by Jean in:
>>>> https://patchwork.kernel.org/patch/11258267/#23046313
>>>
>>> Ah OK. :)
>>>
>>> However this is tricky. How about do explicit g_tree_destroy() in
>>> virtio_iommu_detach() when it's the last endpoint? I see that you
>>> have:
>>>
>>> /*
>>> * when the last EP is detached, simply remove the domain for
>>> * the domain list and destroy it. Note its mappings were already
>>> * freed by the ref count mechanism. Next operation involving
>>> * the same domain id will re-create one domain object.
>>> */
>>> if (QLIST_EMPTY(&domain->endpoint_list)) {
>>> g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
>>> }
>>>
>>> Then it becomes:
>>>
>>> if (QLIST_EMPTY(&domain->endpoint_list)) {
>>> g_tree_destroy(domain->mappings);
>>> g_tree_remove(s->domains, GUINT_TO_POINTER(domain->id));
>>> }
>>>
>>> And also remove the trick in attach() so you take the domain ref
>>> unconditionally. Would that work?
>> Yes I think so. On the other hand this ref counting mechanism is also
>> made for that purpose of destroying objects without being forced to
>> explicitly call the destroy function.
>
> IMHO that's two different things. g_tree_destroy() should be the same
> as g_tree_unref() here when the tree is empty. It's really a matter
> of easy reading of code:
>
> void
> g_tree_destroy (GTree *tree)
> {
> g_return_if_fail (tree != NULL);
>
> g_tree_remove_all (tree);
> g_tree_unref (tree);
> }
>
> What we really changed here is to allow the ref/unref to be clearly
> paired, i.e., for each EP it'll ref once and unref once. The prvious
> solution has the trick in that the 1st EP don't ref, the latter EPs
> ref, and when the domain quits it doesn't unref to match the first
> ref. It's error prone to me. Then, if we can do it in the paired way
> easily, I don't see why not...
OK. I will respin according to your suggestion.
Thanks
Eric
>
> Thanks,
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2020-02-04 12:27 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-25 17:19 [PATCH v13 00/10] VIRTIO-IOMMU device Eric Auger
2020-01-25 17:19 ` [PATCH v13 01/10] virtio-iommu: Add skeleton Eric Auger
2020-01-25 17:19 ` [PATCH v13 02/10] virtio-iommu: Decode the command payload Eric Auger
2020-01-25 17:19 ` [PATCH v13 03/10] virtio-iommu: Implement attach/detach command Eric Auger
2020-02-03 13:49 ` Peter Xu
2020-02-03 14:59 ` Auger Eric
2020-02-03 15:19 ` Peter Xu
2020-02-03 17:46 ` Auger Eric
2020-02-03 18:19 ` Peter Xu
2020-02-04 12:26 ` Auger Eric
2020-01-25 17:19 ` [PATCH v13 04/10] virtio-iommu: Implement map/unmap Eric Auger
2020-01-25 17:19 ` [PATCH v13 05/10] virtio-iommu: Implement translate Eric Auger
2020-01-25 17:19 ` [PATCH v13 06/10] virtio-iommu: Implement fault reporting Eric Auger
2020-02-03 13:55 ` Peter Xu
2020-02-03 15:01 ` Auger Eric
2020-01-25 17:19 ` [PATCH v13 07/10] virtio-iommu-pci: Add virtio iommu pci support Eric Auger
2020-02-03 13:03 ` Michael S. Tsirkin
2020-02-03 13:20 ` Auger Eric
2020-02-03 13:28 ` Michael S. Tsirkin
2020-02-03 13:38 ` Auger Eric
2020-02-03 13:46 ` Michael S. Tsirkin
2020-02-03 13:52 ` Auger Eric
2020-01-25 17:19 ` [PATCH v13 08/10] hw/arm/virt: Add the virtio-iommu device tree mappings Eric Auger
2020-01-25 17:19 ` [PATCH v13 09/10] virtio-iommu: Support migration Eric Auger
2020-01-25 17:19 ` [PATCH v13 10/10] tests: Add virtio-iommu test Eric Auger
2020-02-03 12:58 ` [PATCH v13 00/10] VIRTIO-IOMMU device Auger Eric
2020-02-03 13:49 ` Peter Xu
2020-02-03 13:51 ` Auger Eric
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.