* [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG
@ 2021-09-30 18:50 Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 1/3] NOMERGE: virtio-iommu: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-30 18:50 UTC (permalink / raw)
To: eric.auger; +Cc: Jean-Philippe Brucker, qemu-devel, mst
Replace the VIRTIO_IOMMU_F_BYPASS feature with
VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch
global bypass on and off.
Add a boot-bypass option, which defaults to 'on' to be in line with
other vIOMMUs and to allow running firmware/bootloader that are unaware
of the IOMMU.
See the spec change for more rationale
https://lists.oasis-open.org/archives/virtio-dev/202109/msg00137.html
Jean-Philippe Brucker (3):
NOMERGE: virtio-iommu: Add definitions for
VIRTIO_IOMMU_F_BYPASS_CONFIG
virtio-iommu: Default to bypass during boot
virtio-iommu: Support bypass domain
include/hw/virtio/virtio-iommu.h | 1 +
include/standard-headers/linux/virtio_iommu.h | 10 +++-
hw/virtio/virtio-iommu.c | 60 ++++++++++++++++---
hw/virtio/trace-events | 4 +-
4 files changed, 64 insertions(+), 11 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] NOMERGE: virtio-iommu: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG
2021-09-30 18:50 [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
@ 2021-09-30 18:50 ` Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 2/3] virtio-iommu: Default to bypass during boot Jean-Philippe Brucker
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-30 18:50 UTC (permalink / raw)
To: eric.auger; +Cc: Jean-Philippe Brucker, qemu-devel, mst
Pull VIRTIO_IOMMU_F_BYPASS_CONFIG changes from Linux (not upstream yet).
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
include/standard-headers/linux/virtio_iommu.h | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/standard-headers/linux/virtio_iommu.h b/include/standard-headers/linux/virtio_iommu.h
index b9443b83a1..d14808e3fb 100644
--- a/include/standard-headers/linux/virtio_iommu.h
+++ b/include/standard-headers/linux/virtio_iommu.h
@@ -13,9 +13,10 @@
#define VIRTIO_IOMMU_F_INPUT_RANGE 0
#define VIRTIO_IOMMU_F_DOMAIN_RANGE 1
#define VIRTIO_IOMMU_F_MAP_UNMAP 2
-#define VIRTIO_IOMMU_F_BYPASS 3
+#define VIRTIO_IOMMU_F_BYPASS 3 /* Deprecated */
#define VIRTIO_IOMMU_F_PROBE 4
#define VIRTIO_IOMMU_F_MMIO 5
+#define VIRTIO_IOMMU_F_BYPASS_CONFIG 6
struct virtio_iommu_range_64 {
uint64_t start;
@@ -36,6 +37,8 @@ struct virtio_iommu_config {
struct virtio_iommu_range_32 domain_range;
/* Probe buffer size */
uint32_t probe_size;
+ uint8_t bypass;
+ uint8_t reserved[7];
};
/* Request types */
@@ -66,11 +69,14 @@ struct virtio_iommu_req_tail {
uint8_t reserved[3];
};
+#define VIRTIO_IOMMU_ATTACH_F_BYPASS (1 << 0)
+
struct virtio_iommu_req_attach {
struct virtio_iommu_req_head head;
uint32_t domain;
uint32_t endpoint;
- uint8_t reserved[8];
+ uint32_t flags;
+ uint8_t reserved[4];
struct virtio_iommu_req_tail tail;
};
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] virtio-iommu: Default to bypass during boot
2021-09-30 18:50 [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 1/3] NOMERGE: virtio-iommu: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
@ 2021-09-30 18:50 ` Jean-Philippe Brucker
2021-10-06 13:04 ` Eric Auger
2021-09-30 18:50 ` [PATCH 3/3] virtio-iommu: Support bypass domain Jean-Philippe Brucker
2022-01-11 9:02 ` [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Eric Auger
3 siblings, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-30 18:50 UTC (permalink / raw)
To: eric.auger; +Cc: Jean-Philippe Brucker, qemu-devel, mst
Currently the virtio-iommu device must be programmed before it allows
DMA from any PCI device. This can make the VM entirely unusable when a
virtio-iommu driver isn't present, for example in a bootloader that
loads the OS from storage.
Similarly to the other vIOMMU implementations, default to DMA bypassing
the IOMMU during boot. Add a "boot-bypass" option that lets users change
this behavior.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
include/hw/virtio/virtio-iommu.h | 1 +
hw/virtio/virtio-iommu.c | 28 +++++++++++++++++++++++-----
hw/virtio/trace-events | 4 ++--
3 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 273e35c04b..4c66989ca4 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -58,6 +58,7 @@ struct VirtIOIOMMU {
GTree *domains;
QemuMutex mutex;
GTree *endpoints;
+ bool boot_bypass;
};
#endif
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1b23e8e18c..82edeaa101 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -728,8 +728,7 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
.perm = IOMMU_NONE,
};
- bypass_allowed = virtio_vdev_has_feature(&s->parent_obj,
- VIRTIO_IOMMU_F_BYPASS);
+ bypass_allowed = s->config.bypass;
sid = virtio_iommu_get_bdf(sdev);
@@ -828,7 +827,8 @@ static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
config->input_range.start,
config->input_range.end,
config->domain_range.end,
- config->probe_size);
+ config->probe_size,
+ config->bypass);
memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
}
@@ -836,13 +836,29 @@ static void virtio_iommu_set_config(VirtIODevice *vdev,
const uint8_t *config_data)
{
struct virtio_iommu_config config;
+ VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
+
+ if (config.bypass != dev->config.bypass) {
+ if (!virtio_vdev_has_feature(vdev, VIRTIO_IOMMU_F_BYPASS_CONFIG)) {
+ virtio_error(vdev, "cannot set config.bypass");
+ return;
+ }
+ if (config.bypass != 0 && config.bypass != 1) {
+ warn_report("invalid config.bypass value '%d'", config.bypass);
+ dev->config.bypass = 0;
+ return;
+ }
+ dev->config.bypass = config.bypass;
+ }
+
trace_virtio_iommu_set_config(config.page_size_mask,
config.input_range.start,
config.input_range.end,
config.domain_range.end,
- config.probe_size);
+ config.probe_size,
+ config.bypass);
}
static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
@@ -986,6 +1002,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
s->config.input_range.end = -1UL;
s->config.domain_range.end = 32;
s->config.probe_size = VIOMMU_PROBE_SIZE;
+ s->config.bypass = s->boot_bypass;
virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX);
virtio_add_feature(&s->features, VIRTIO_RING_F_INDIRECT_DESC);
@@ -993,9 +1010,9 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
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);
virtio_add_feature(&s->features, VIRTIO_IOMMU_F_PROBE);
+ virtio_add_feature(&s->features, VIRTIO_IOMMU_F_BYPASS_CONFIG);
qemu_mutex_init(&s->mutex);
@@ -1169,6 +1186,7 @@ static const VMStateDescription vmstate_virtio_iommu = {
static Property virtio_iommu_properties[] = {
DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus, "PCI", PCIBus *),
+ DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8ed19e9d0c..6bc3821ba3 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -90,8 +90,8 @@ virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
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"
+virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x bypass=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, uint8_t bypass) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x bypass=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"
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] virtio-iommu: Support bypass domain
2021-09-30 18:50 [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 1/3] NOMERGE: virtio-iommu: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 2/3] virtio-iommu: Default to bypass during boot Jean-Philippe Brucker
@ 2021-09-30 18:50 ` Jean-Philippe Brucker
2021-10-06 13:10 ` Eric Auger
2022-01-11 9:02 ` [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Eric Auger
3 siblings, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-30 18:50 UTC (permalink / raw)
To: eric.auger; +Cc: Jean-Philippe Brucker, qemu-devel, mst
The driver can create a bypass domain by passing the
VIRTIO_IOMMU_ATTACH_F_BYPASS flag on the ATTACH request. Bypass domains
perform slightly better than domains with identity mappings since they
skip translation.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
hw/virtio/virtio-iommu.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 82edeaa101..4f0207a3eb 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -42,6 +42,7 @@
typedef struct VirtIOIOMMUDomain {
uint32_t id;
+ bool bypass;
GTree *mappings;
QLIST_HEAD(, VirtIOIOMMUEndpoint) endpoint_list;
} VirtIOIOMMUDomain;
@@ -257,12 +258,16 @@ static void virtio_iommu_put_endpoint(gpointer data)
}
static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
- uint32_t domain_id)
+ uint32_t domain_id,
+ bool bypass)
{
VirtIOIOMMUDomain *domain;
domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
if (domain) {
+ if (domain->bypass != bypass) {
+ return NULL;
+ }
return domain;
}
domain = g_malloc0(sizeof(*domain));
@@ -270,6 +275,7 @@ static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
NULL, (GDestroyNotify)g_free,
(GDestroyNotify)g_free);
+ domain->bypass = bypass;
g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain);
QLIST_INIT(&domain->endpoint_list);
trace_virtio_iommu_get_domain(domain_id);
@@ -333,11 +339,16 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
{
uint32_t domain_id = le32_to_cpu(req->domain);
uint32_t ep_id = le32_to_cpu(req->endpoint);
+ uint32_t flags = le32_to_cpu(req->flags);
VirtIOIOMMUDomain *domain;
VirtIOIOMMUEndpoint *ep;
trace_virtio_iommu_attach(domain_id, ep_id);
+ if (flags & ~VIRTIO_IOMMU_ATTACH_F_BYPASS) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
ep = virtio_iommu_get_endpoint(s, ep_id);
if (!ep) {
return VIRTIO_IOMMU_S_NOENT;
@@ -355,7 +366,12 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
}
}
- domain = virtio_iommu_get_domain(s, domain_id);
+ domain = virtio_iommu_get_domain(s, domain_id,
+ flags & VIRTIO_IOMMU_ATTACH_F_BYPASS);
+ if (!domain) {
+ /* Incompatible flags */
+ return VIRTIO_IOMMU_S_INVAL;
+ }
QLIST_INSERT_HEAD(&domain->endpoint_list, ep, next);
ep->domain = domain;
@@ -418,6 +434,10 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
return VIRTIO_IOMMU_S_NOENT;
}
+ if (domain->bypass) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
interval = g_malloc0(sizeof(*interval));
interval->low = virt_start;
@@ -463,6 +483,11 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
if (!domain) {
return VIRTIO_IOMMU_S_NOENT;
}
+
+ if (domain->bypass) {
+ return VIRTIO_IOMMU_S_INVAL;
+ }
+
interval.low = virt_start;
interval.high = virt_end;
@@ -779,6 +804,9 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
entry.perm = flag;
}
goto unlock;
+ } else if (ep->domain->bypass) {
+ entry.perm = flag;
+ goto unlock;
}
found = g_tree_lookup_extended(ep->domain->mappings, (gpointer)(&interval),
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] virtio-iommu: Default to bypass during boot
2021-09-30 18:50 ` [PATCH 2/3] virtio-iommu: Default to bypass during boot Jean-Philippe Brucker
@ 2021-10-06 13:04 ` Eric Auger
2021-10-08 10:21 ` Jean-Philippe Brucker
0 siblings, 1 reply; 12+ messages in thread
From: Eric Auger @ 2021-10-06 13:04 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: qemu-devel, mst
Hi jean,
On 9/30/21 8:50 PM, Jean-Philippe Brucker wrote:
> Currently the virtio-iommu device must be programmed before it allows
> DMA from any PCI device. This can make the VM entirely unusable when a
> virtio-iommu driver isn't present, for example in a bootloader that
> loads the OS from storage.
>
> Similarly to the other vIOMMU implementations, default to DMA bypassing
> the IOMMU during boot. Add a "boot-bypass" option that lets users change
> this behavior.
s/option/property
set to true by default.
Also mention this deprecates the legacy VIRTIO_IOMMU_F_BYPASS feature
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> include/hw/virtio/virtio-iommu.h | 1 +
> hw/virtio/virtio-iommu.c | 28 +++++++++++++++++++++++-----
> hw/virtio/trace-events | 4 ++--
> 3 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 273e35c04b..4c66989ca4 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -58,6 +58,7 @@ struct VirtIOIOMMU {
> GTree *domains;
> QemuMutex mutex;
> GTree *endpoints;
> + bool boot_bypass;
> };
>
> #endif
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1b23e8e18c..82edeaa101 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -728,8 +728,7 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> .perm = IOMMU_NONE,
> };
>
> - bypass_allowed = virtio_vdev_has_feature(&s->parent_obj,
> - VIRTIO_IOMMU_F_BYPASS);
> + bypass_allowed = s->config.bypass;
>
> sid = virtio_iommu_get_bdf(sdev);
>
> @@ -828,7 +827,8 @@ static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t *config_data)
> config->input_range.start,
> config->input_range.end,
> config->domain_range.end,
> - config->probe_size);
> + config->probe_size,
> + config->bypass);
> memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
> }
>
> @@ -836,13 +836,29 @@ static void virtio_iommu_set_config(VirtIODevice *vdev,
> const uint8_t *config_data)
> {
> struct virtio_iommu_config config;
> + VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
>
> memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
> +
> + if (config.bypass != dev->config.bypass) {
> + if (!virtio_vdev_has_feature(vdev, VIRTIO_IOMMU_F_BYPASS_CONFIG)) {
> + virtio_error(vdev, "cannot set config.bypass");
> + return;
> + }
> + if (config.bypass != 0 && config.bypass != 1) {
> + warn_report("invalid config.bypass value '%d'", config.bypass);
> + dev->config.bypass = 0;
Shouldn't we leave the old value instead. The spec proposal says it
takes 0 though.
> + return;
> + }
> + dev->config.bypass = config.bypass;
> + }
> +
> trace_virtio_iommu_set_config(config.page_size_mask,
> config.input_range.start,
> config.input_range.end,
> config.domain_range.end,
> - config.probe_size);
> + config.probe_size,
> + config.bypass);
> }
>
> static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
> @@ -986,6 +1002,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> s->config.input_range.end = -1UL;
> s->config.domain_range.end = 32;
> s->config.probe_size = VIOMMU_PROBE_SIZE;
> + s->config.bypass = s->boot_bypass;
>
> virtio_add_feature(&s->features, VIRTIO_RING_F_EVENT_IDX);
> virtio_add_feature(&s->features, VIRTIO_RING_F_INDIRECT_DESC);
> @@ -993,9 +1010,9 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> 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);
> virtio_add_feature(&s->features, VIRTIO_IOMMU_F_PROBE);
> + virtio_add_feature(&s->features, VIRTIO_IOMMU_F_BYPASS_CONFIG);
>
> qemu_mutex_init(&s->mutex);
>
> @@ -1169,6 +1186,7 @@ static const VMStateDescription vmstate_virtio_iommu = {
>
> static Property virtio_iommu_properties[] = {
> DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus, "PCI", PCIBus *),
> + DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 8ed19e9d0c..6bc3821ba3 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -90,8 +90,8 @@ virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
> 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"
> +virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_range, uint32_t probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_range=%d probe_size=0x%x bypass=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, uint8_t bypass) "page_size_mask=0x%"PRIx64" start=0x%"PRIx64" end=0x%"PRIx64" domain_bits=%d probe_size=0x%x bypass=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"
Thanks
Eric
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] virtio-iommu: Support bypass domain
2021-09-30 18:50 ` [PATCH 3/3] virtio-iommu: Support bypass domain Jean-Philippe Brucker
@ 2021-10-06 13:10 ` Eric Auger
0 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2021-10-06 13:10 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: qemu-devel, mst
Hi jean,
On 9/30/21 8:50 PM, Jean-Philippe Brucker wrote:
> The driver can create a bypass domain by passing the
> VIRTIO_IOMMU_ATTACH_F_BYPASS flag on the ATTACH request. Bypass domains
> perform slightly better than domains with identity mappings since they
> skip translation.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> hw/virtio/virtio-iommu.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 82edeaa101..4f0207a3eb 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -42,6 +42,7 @@
>
> typedef struct VirtIOIOMMUDomain {
> uint32_t id;
> + bool bypass;
> GTree *mappings;
> QLIST_HEAD(, VirtIOIOMMUEndpoint) endpoint_list;
> } VirtIOIOMMUDomain;
> @@ -257,12 +258,16 @@ static void virtio_iommu_put_endpoint(gpointer data)
> }
>
> static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
> - uint32_t domain_id)
> + uint32_t domain_id,
> + bool bypass)
> {
> VirtIOIOMMUDomain *domain;
>
> domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
> if (domain) {
> + if (domain->bypass != bypass) {
> + return NULL;
> + }
> return domain;
> }
> domain = g_malloc0(sizeof(*domain));
> @@ -270,6 +275,7 @@ static VirtIOIOMMUDomain *virtio_iommu_get_domain(VirtIOIOMMU *s,
> domain->mappings = g_tree_new_full((GCompareDataFunc)interval_cmp,
> NULL, (GDestroyNotify)g_free,
> (GDestroyNotify)g_free);
> + domain->bypass = bypass;
> g_tree_insert(s->domains, GUINT_TO_POINTER(domain_id), domain);
> QLIST_INIT(&domain->endpoint_list);
> trace_virtio_iommu_get_domain(domain_id);
> @@ -333,11 +339,16 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
> {
> uint32_t domain_id = le32_to_cpu(req->domain);
> uint32_t ep_id = le32_to_cpu(req->endpoint);
> + uint32_t flags = le32_to_cpu(req->flags);
> VirtIOIOMMUDomain *domain;
> VirtIOIOMMUEndpoint *ep;
>
> trace_virtio_iommu_attach(domain_id, ep_id);
>
> + if (flags & ~VIRTIO_IOMMU_ATTACH_F_BYPASS) {
> + return VIRTIO_IOMMU_S_INVAL;
> + }
> +
> ep = virtio_iommu_get_endpoint(s, ep_id);
> if (!ep) {
> return VIRTIO_IOMMU_S_NOENT;
> @@ -355,7 +366,12 @@ static int virtio_iommu_attach(VirtIOIOMMU *s,
> }
> }
>
> - domain = virtio_iommu_get_domain(s, domain_id);
> + domain = virtio_iommu_get_domain(s, domain_id,
> + flags & VIRTIO_IOMMU_ATTACH_F_BYPASS);
> + if (!domain) {
> + /* Incompatible flags */
Incompatible bypass flag
> + return VIRTIO_IOMMU_S_INVAL;
> + }
> QLIST_INSERT_HEAD(&domain->endpoint_list, ep, next);
>
> ep->domain = domain;
> @@ -418,6 +434,10 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
> return VIRTIO_IOMMU_S_NOENT;
> }
>
> + if (domain->bypass) {
> + return VIRTIO_IOMMU_S_INVAL;
> + }
> +
> interval = g_malloc0(sizeof(*interval));
>
> interval->low = virt_start;
> @@ -463,6 +483,11 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
> if (!domain) {
> return VIRTIO_IOMMU_S_NOENT;
> }
> +
> + if (domain->bypass) {
> + return VIRTIO_IOMMU_S_INVAL;
> + }
> +
> interval.low = virt_start;
> interval.high = virt_end;
>
> @@ -779,6 +804,9 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> entry.perm = flag;
> }
> goto unlock;
> + } else if (ep->domain->bypass) {
> + entry.perm = flag;
> + goto unlock;
> }
>
> found = g_tree_lookup_extended(ep->domain->mappings, (gpointer)(&interval),
Otherwise looks good to me.
Eric
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] virtio-iommu: Default to bypass during boot
2021-10-06 13:04 ` Eric Auger
@ 2021-10-08 10:21 ` Jean-Philippe Brucker
0 siblings, 0 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-08 10:21 UTC (permalink / raw)
To: Eric Auger; +Cc: qemu-devel, mst
On Wed, Oct 06, 2021 at 03:04:15PM +0200, Eric Auger wrote:
> > + if (config.bypass != dev->config.bypass) {
> > + if (!virtio_vdev_has_feature(vdev, VIRTIO_IOMMU_F_BYPASS_CONFIG)) {
> > + virtio_error(vdev, "cannot set config.bypass");
> > + return;
> > + }
> > + if (config.bypass != 0 && config.bypass != 1) {
> > + warn_report("invalid config.bypass value '%d'", config.bypass);
> > + dev->config.bypass = 0;
> Shouldn't we leave the old value instead. The spec proposal says it
> takes 0 though.
Yes keeping the old value is better, I'll change the spec. Writing a
different value is a driver bug since new values for the bypass field will
be keyed off a new feature bit.
Thanks,
Jean
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG
2021-09-30 18:50 [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
` (2 preceding siblings ...)
2021-09-30 18:50 ` [PATCH 3/3] virtio-iommu: Support bypass domain Jean-Philippe Brucker
@ 2022-01-11 9:02 ` Eric Auger
2022-01-11 10:13 ` Jean-Philippe Brucker
2022-01-11 15:40 ` Michael S. Tsirkin
3 siblings, 2 replies; 12+ messages in thread
From: Eric Auger @ 2022-01-11 9:02 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: qemu-devel, mst
Hi Jean, Michael,
On 9/30/21 8:50 PM, Jean-Philippe Brucker wrote:
> Replace the VIRTIO_IOMMU_F_BYPASS feature with
> VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch
> global bypass on and off.
>
> Add a boot-bypass option, which defaults to 'on' to be in line with
> other vIOMMUs and to allow running firmware/bootloader that are unaware
> of the IOMMU.
>
> See the spec change for more rationale
> https://lists.oasis-open.org/archives/virtio-dev/202109/msg00137.html
I guess the kernel bits should be merged in 5.17?
Thanks
Eric
>
> Jean-Philippe Brucker (3):
> NOMERGE: virtio-iommu: Add definitions for
> VIRTIO_IOMMU_F_BYPASS_CONFIG
> virtio-iommu: Default to bypass during boot
> virtio-iommu: Support bypass domain
>
> include/hw/virtio/virtio-iommu.h | 1 +
> include/standard-headers/linux/virtio_iommu.h | 10 +++-
> hw/virtio/virtio-iommu.c | 60 ++++++++++++++++---
> hw/virtio/trace-events | 4 +-
> 4 files changed, 64 insertions(+), 11 deletions(-)
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG
2022-01-11 9:02 ` [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Eric Auger
@ 2022-01-11 10:13 ` Jean-Philippe Brucker
2022-01-11 10:33 ` Eric Auger
2022-01-11 15:40 ` Michael S. Tsirkin
1 sibling, 1 reply; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-01-11 10:13 UTC (permalink / raw)
To: Eric Auger; +Cc: qemu-devel, mst
Hi Eric,
On Tue, Jan 11, 2022 at 10:02:12AM +0100, Eric Auger wrote:
> Hi Jean, Michael,
>
> On 9/30/21 8:50 PM, Jean-Philippe Brucker wrote:
> > Replace the VIRTIO_IOMMU_F_BYPASS feature with
> > VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch
> > global bypass on and off.
> >
> > Add a boot-bypass option, which defaults to 'on' to be in line with
> > other vIOMMUs and to allow running firmware/bootloader that are unaware
> > of the IOMMU.
> >
> > See the spec change for more rationale
> > https://lists.oasis-open.org/archives/virtio-dev/202109/msg00137.html
>
> I guess the kernel bits should be merged in 5.17?
Yes, they should. I can resend at 5.17-rc1 along with a commit updating
the linux headers. Or is there a specific process for synchronizing the
headers? Looking at git log it looks like scripts/update-linux-headers.sh
is run by whomever needs new UAPI features.
Thanks,
Jean
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG
2022-01-11 10:13 ` Jean-Philippe Brucker
@ 2022-01-11 10:33 ` Eric Auger
0 siblings, 0 replies; 12+ messages in thread
From: Eric Auger @ 2022-01-11 10:33 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: qemu-devel, mst
Hi Jean,
On 1/11/22 11:13 AM, Jean-Philippe Brucker wrote:
> Hi Eric,
>
> On Tue, Jan 11, 2022 at 10:02:12AM +0100, Eric Auger wrote:
>> Hi Jean, Michael,
>>
>> On 9/30/21 8:50 PM, Jean-Philippe Brucker wrote:
>>> Replace the VIRTIO_IOMMU_F_BYPASS feature with
>>> VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch
>>> global bypass on and off.
>>>
>>> Add a boot-bypass option, which defaults to 'on' to be in line with
>>> other vIOMMUs and to allow running firmware/bootloader that are unaware
>>> of the IOMMU.
>>>
>>> See the spec change for more rationale
>>> https://lists.oasis-open.org/archives/virtio-dev/202109/msg00137.html
>> I guess the kernel bits should be merged in 5.17?
> Yes, they should. I can resend at 5.17-rc1 along with a commit updating
> the linux headers. Or is there a specific process for synchronizing the
> headers? Looking at git log it looks like scripts/update-linux-headers.sh
> is run by whomever needs new UAPI features.
yes you need to generate the header update patch using
scripts/update-linux-headers.sh once the kernel pieces have landed
Thanks
Eric
>
> Thanks,
> Jean
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG
2022-01-11 9:02 ` [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Eric Auger
2022-01-11 10:13 ` Jean-Philippe Brucker
@ 2022-01-11 15:40 ` Michael S. Tsirkin
2022-01-11 16:24 ` Jean-Philippe Brucker
1 sibling, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2022-01-11 15:40 UTC (permalink / raw)
To: Eric Auger; +Cc: Jean-Philippe Brucker, qemu-devel
On Tue, Jan 11, 2022 at 10:02:12AM +0100, Eric Auger wrote:
> Hi Jean, Michael,
>
> On 9/30/21 8:50 PM, Jean-Philippe Brucker wrote:
> > Replace the VIRTIO_IOMMU_F_BYPASS feature with
> > VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch
> > global bypass on and off.
> >
> > Add a boot-bypass option, which defaults to 'on' to be in line with
> > other vIOMMUs and to allow running firmware/bootloader that are unaware
> > of the IOMMU.
> >
> > See the spec change for more rationale
> > https://lists.oasis-open.org/archives/virtio-dev/202109/msg00137.html
>
> I guess the kernel bits should be merged in 5.17?
>
> Thanks
>
> Eric
They are in fact in my tree and set to go into 5.16.
They've been in linux-next for a whole cycle now.
But if you feel I'm rushing things, pls let me know.
Also, pls let me know whether my tree actually works well for you!
> >
> > Jean-Philippe Brucker (3):
> > NOMERGE: virtio-iommu: Add definitions for
> > VIRTIO_IOMMU_F_BYPASS_CONFIG
> > virtio-iommu: Default to bypass during boot
> > virtio-iommu: Support bypass domain
> >
> > include/hw/virtio/virtio-iommu.h | 1 +
> > include/standard-headers/linux/virtio_iommu.h | 10 +++-
> > hw/virtio/virtio-iommu.c | 60 ++++++++++++++++---
> > hw/virtio/trace-events | 4 +-
> > 4 files changed, 64 insertions(+), 11 deletions(-)
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG
2022-01-11 15:40 ` Michael S. Tsirkin
@ 2022-01-11 16:24 ` Jean-Philippe Brucker
0 siblings, 0 replies; 12+ messages in thread
From: Jean-Philippe Brucker @ 2022-01-11 16:24 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Eric Auger, qemu-devel
Hi Michael,
On Tue, Jan 11, 2022 at 10:40:28AM -0500, Michael S. Tsirkin wrote:
> On Tue, Jan 11, 2022 at 10:02:12AM +0100, Eric Auger wrote:
> > Hi Jean, Michael,
> >
> > On 9/30/21 8:50 PM, Jean-Philippe Brucker wrote:
> > > Replace the VIRTIO_IOMMU_F_BYPASS feature with
> > > VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch
> > > global bypass on and off.
> > >
> > > Add a boot-bypass option, which defaults to 'on' to be in line with
> > > other vIOMMUs and to allow running firmware/bootloader that are unaware
> > > of the IOMMU.
> > >
> > > See the spec change for more rationale
> > > https://lists.oasis-open.org/archives/virtio-dev/202109/msg00137.html
> >
> > I guess the kernel bits should be merged in 5.17?
> >
> > Thanks
> >
> > Eric
>
> They are in fact in my tree and set to go into 5.16.
> They've been in linux-next for a whole cycle now.
> But if you feel I'm rushing things, pls let me know.
> Also, pls let me know whether my tree actually works well for you!
Thanks, unfortunately I just noticed that those patches are from an older
version of the series, the latest being v3
https://lore.kernel.org/linux-iommu/20211201173323.1045819-1-jean-philippe@linaro.org/
Since Joerg picked the latest one in his tree [1], I was wondering if you
could drop those patches?
51a4c54f35ae iommu/virtio: Support identity-mapped domains
523e55a406aa iommu/virtio: Pass end address to viommu_add_mapping()
97301219dfb2 iommu/virtio: Sort reserved regions
720552613526 iommu/virtio: Support bypass domains
01444b9c772f iommu/virtio: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG
Thanks,
Jean
[1] https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-01-11 16:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-30 18:50 [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 1/3] NOMERGE: virtio-iommu: Add definitions for VIRTIO_IOMMU_F_BYPASS_CONFIG Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 2/3] virtio-iommu: Default to bypass during boot Jean-Philippe Brucker
2021-10-06 13:04 ` Eric Auger
2021-10-08 10:21 ` Jean-Philippe Brucker
2021-09-30 18:50 ` [PATCH 3/3] virtio-iommu: Support bypass domain Jean-Philippe Brucker
2021-10-06 13:10 ` Eric Auger
2022-01-11 9:02 ` [PATCH 0/3] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG Eric Auger
2022-01-11 10:13 ` Jean-Philippe Brucker
2022-01-11 10:33 ` Eric Auger
2022-01-11 15:40 ` Michael S. Tsirkin
2022-01-11 16:24 ` Jean-Philippe Brucker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).