* [PATCH v3 01/10] hw/acpi: Add VIOT table
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
@ 2021-09-14 14:19 ` Jean-Philippe Brucker
2021-09-16 12:44 ` Eric Auger
` (2 more replies)
2021-09-14 14:19 ` [PATCH v3 02/10] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu Jean-Philippe Brucker
` (8 subsequent siblings)
9 siblings, 3 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:19 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
Add a function that generates a Virtual I/O Translation table (VIOT),
describing the topology of paravirtual IOMMUs. The table is created when
instantiating a virtio-iommu device. It contains a virtio-iommu node and
PCI Range nodes for endpoints managed by the IOMMU. By default, a single
node describes all PCI devices. When passing the "default_bus_bypass_iommu"
machine option and "bypass_iommu" PXB option, only buses that do not
bypass the IOMMU are described by PCI Range nodes.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
Sizes and types are hardcoded because it will now be the default style
https://lore.kernel.org/qemu-devel/20210708154617.1538485-1-imammedo@redhat.com/
---
hw/acpi/viot.h | 13 +++++
hw/acpi/viot.c | 112 ++++++++++++++++++++++++++++++++++++++++++++
hw/acpi/Kconfig | 4 ++
hw/acpi/meson.build | 1 +
4 files changed, 130 insertions(+)
create mode 100644 hw/acpi/viot.h
create mode 100644 hw/acpi/viot.c
diff --git a/hw/acpi/viot.h b/hw/acpi/viot.h
new file mode 100644
index 0000000000..4cef29a640
--- /dev/null
+++ b/hw/acpi/viot.h
@@ -0,0 +1,13 @@
+/*
+ * ACPI Virtual I/O Translation Table implementation
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef VIOT_H
+#define VIOT_H
+
+void build_viot(GArray *table_data, BIOSLinker *linker,
+ uint16_t virtio_iommu_bdf, const char *oem_id,
+ const char *oem_table_id);
+
+#endif /* VIOT_H */
diff --git a/hw/acpi/viot.c b/hw/acpi/viot.c
new file mode 100644
index 0000000000..e7f7605119
--- /dev/null
+++ b/hw/acpi/viot.c
@@ -0,0 +1,112 @@
+/*
+ * ACPI Virtual I/O Translation table implementation
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/viot.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_host.h"
+
+struct viot_pci_ranges {
+ GArray *blob;
+ size_t count;
+ uint16_t output_node;
+};
+
+/* Build PCI range for a given PCI host bridge */
+static int viot_host_bridges(Object *obj, void *opaque)
+{
+ struct viot_pci_ranges *pci_ranges = opaque;
+ GArray *blob = pci_ranges->blob;
+
+ if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
+ PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
+
+ if (bus && !pci_bus_bypass_iommu(bus)) {
+ int min_bus, max_bus;
+
+ pci_bus_range(bus, &min_bus, &max_bus);
+
+ /* Type (PCI range) */
+ build_append_int_noprefix(blob, 1, 1);
+ /* Reserved */
+ build_append_int_noprefix(blob, 0, 1);
+ /* Length */
+ build_append_int_noprefix(blob, 24, 2);
+ /* Endpoint start */
+ build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 4);
+ /* PCI Segment start */
+ build_append_int_noprefix(blob, 0, 2);
+ /* PCI Segment end */
+ build_append_int_noprefix(blob, 0, 2);
+ /* PCI BDF start */
+ build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 2);
+ /* PCI BDF end */
+ build_append_int_noprefix(blob, PCI_BUILD_BDF(max_bus, 0xff), 2);
+ /* Output node */
+ build_append_int_noprefix(blob, pci_ranges->output_node, 2);
+ /* Reserved */
+ build_append_int_noprefix(blob, 0, 6);
+
+ pci_ranges->count++;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Generate a VIOT table with one PCI-based virtio-iommu that manages PCI
+ * endpoints.
+ */
+void build_viot(GArray *table_data, BIOSLinker *linker,
+ uint16_t virtio_iommu_bdf, const char *oem_id,
+ const char *oem_table_id)
+{
+ /* The virtio-iommu node follows the 48-bytes header */
+ int viommu_off = 48;
+ AcpiTable table = { .sig = "VIOT", .rev = 0,
+ .oem_id = oem_id, .oem_table_id = oem_table_id };
+ struct viot_pci_ranges pci_ranges = {
+ .output_node = viommu_off,
+ .blob = g_array_new(false, true, 1),
+ };
+
+ /* Build the list of PCI ranges that this viommu manages */
+ object_child_foreach_recursive(object_get_root(), viot_host_bridges,
+ &pci_ranges);
+
+ /* ACPI table header */
+ acpi_table_begin(&table, table_data);
+ /* Node count */
+ build_append_int_noprefix(table_data, pci_ranges.count + 1, 2);
+ /* Node offset */
+ build_append_int_noprefix(table_data, viommu_off, 2);
+ /* Reserved */
+ build_append_int_noprefix(table_data, 0, 8);
+
+ /* Virtio-iommu node */
+ /* Type (virtio-pci IOMMU) */
+ build_append_int_noprefix(table_data, 3, 1);
+ /* Reserved */
+ build_append_int_noprefix(table_data, 0, 1);
+ /* Length */
+ build_append_int_noprefix(table_data, 16, 2);
+ /* PCI Segment */
+ build_append_int_noprefix(table_data, 0, 2);
+ /* PCI BDF number */
+ build_append_int_noprefix(table_data, virtio_iommu_bdf, 2);
+ /* Reserved */
+ build_append_int_noprefix(table_data, 0, 8);
+
+ /* PCI ranges found above */
+ g_array_append_vals(table_data, pci_ranges.blob->data,
+ pci_ranges.blob->len);
+ g_array_free(pci_ranges.blob, true);
+
+ acpi_table_end(linker, &table);
+}
+
diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
index 3b5e118c54..622b0b50b7 100644
--- a/hw/acpi/Kconfig
+++ b/hw/acpi/Kconfig
@@ -51,6 +51,10 @@ config ACPI_VMGENID
default y
depends on PC
+config ACPI_VIOT
+ bool
+ depends on ACPI
+
config ACPI_HW_REDUCED
bool
select ACPI
diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
index 7d8c0eb43e..adf6347bc4 100644
--- a/hw/acpi/meson.build
+++ b/hw/acpi/meson.build
@@ -20,6 +20,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'), if_false: files(
acpi_ss.add(when: 'CONFIG_ACPI_PIIX4', if_true: files('piix4.c'))
acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_true: files('pcihp.c'))
acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_false: files('acpi-pci-hotplug-stub.c'))
+acpi_ss.add(when: 'CONFIG_ACPI_VIOT', if_true: files('viot.c'))
acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 01/10] hw/acpi: Add VIOT table
2021-09-14 14:19 ` [PATCH v3 01/10] hw/acpi: Add VIOT table Jean-Philippe Brucker
@ 2021-09-16 12:44 ` Eric Auger
2021-09-20 8:06 ` Igor Mammedov
2021-09-20 8:35 ` Igor Mammedov
2 siblings, 0 replies; 22+ messages in thread
From: Eric Auger @ 2021-09-16 12:44 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, qemu-arm, pbonzini, ani, imammedo
Hi Jean,
On 9/14/21 4:19 PM, Jean-Philippe Brucker wrote:
> Add a function that generates a Virtual I/O Translation table (VIOT),
> describing the topology of paravirtual IOMMUs. The table is created when
> instantiating a virtio-iommu device. It contains a virtio-iommu node and
> PCI Range nodes for endpoints managed by the IOMMU. By default, a single
> node describes all PCI devices. When passing the "default_bus_bypass_iommu"
> machine option and "bypass_iommu" PXB option, only buses that do not
> bypass the IOMMU are described by PCI Range nodes.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> Sizes and types are hardcoded because it will now be the default style
> https://lore.kernel.org/qemu-devel/20210708154617.1538485-1-imammedo@redhat.com/
> ---
> hw/acpi/viot.h | 13 +++++
> hw/acpi/viot.c | 112 ++++++++++++++++++++++++++++++++++++++++++++
> hw/acpi/Kconfig | 4 ++
> hw/acpi/meson.build | 1 +
> 4 files changed, 130 insertions(+)
> create mode 100644 hw/acpi/viot.h
> create mode 100644 hw/acpi/viot.c
>
> diff --git a/hw/acpi/viot.h b/hw/acpi/viot.h
> new file mode 100644
> index 0000000000..4cef29a640
> --- /dev/null
> +++ b/hw/acpi/viot.h
> @@ -0,0 +1,13 @@
> +/*
> + * ACPI Virtual I/O Translation Table implementation
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef VIOT_H
> +#define VIOT_H
> +
> +void build_viot(GArray *table_data, BIOSLinker *linker,
> + uint16_t virtio_iommu_bdf, const char *oem_id,
> + const char *oem_table_id);
> +
> +#endif /* VIOT_H */
> diff --git a/hw/acpi/viot.c b/hw/acpi/viot.c
> new file mode 100644
> index 0000000000..e7f7605119
> --- /dev/null
> +++ b/hw/acpi/viot.c
> @@ -0,0 +1,112 @@
> +/*
> + * ACPI Virtual I/O Translation table implementation
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include "qemu/osdep.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/viot.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_host.h"
> +
> +struct viot_pci_ranges {
> + GArray *blob;
> + size_t count;
> + uint16_t output_node;
> +};
> +
> +/* Build PCI range for a given PCI host bridge */
> +static int viot_host_bridges(Object *obj, void *opaque)
nit: rename the function into something like build_pci_range_node()
which actually indicates what it does
> +{
> + struct viot_pci_ranges *pci_ranges = opaque;
> + GArray *blob = pci_ranges->blob;
> +
> + if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
> + PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
> +
> + if (bus && !pci_bus_bypass_iommu(bus)) {
> + int min_bus, max_bus;
> +
> + pci_bus_range(bus, &min_bus, &max_bus);
> +
> + /* Type (PCI range) */
> + build_append_int_noprefix(blob, 1, 1);
> + /* Reserved */
> + build_append_int_noprefix(blob, 0, 1);
> + /* Length */
> + build_append_int_noprefix(blob, 24, 2);
> + /* Endpoint start */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 4);
> + /* PCI Segment start */
> + build_append_int_noprefix(blob, 0, 2);
> + /* PCI Segment end */
> + build_append_int_noprefix(blob, 0, 2);
> + /* PCI BDF start */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 2);
> + /* PCI BDF end */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(max_bus, 0xff), 2);
> + /* Output node */
> + build_append_int_noprefix(blob, pci_ranges->output_node, 2);
> + /* Reserved */
> + build_append_int_noprefix(blob, 0, 6);
> +
> + pci_ranges->count++;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Generate a VIOT table with one PCI-based virtio-iommu that manages PCI
> + * endpoints.
> + */
> +void build_viot(GArray *table_data, BIOSLinker *linker,
> + uint16_t virtio_iommu_bdf, const char *oem_id,
> + const char *oem_table_id)
> +{
> + /* The virtio-iommu node follows the 48-bytes header */
> + int viommu_off = 48;
> + AcpiTable table = { .sig = "VIOT", .rev = 0,
> + .oem_id = oem_id, .oem_table_id = oem_table_id };
> + struct viot_pci_ranges pci_ranges = {
> + .output_node = viommu_off,
> + .blob = g_array_new(false, true, 1),
nit add clear comment as in the other places?
.blob = g_array_new(false, /* clear */ true, 1),
> + };
> +
> + /* Build the list of PCI ranges that this viommu manages */
> + object_child_foreach_recursive(object_get_root(), viot_host_bridges,
> + &pci_ranges);
> +
> + /* ACPI table header */
> + acpi_table_begin(&table, table_data);
> + /* Node count */
> + build_append_int_noprefix(table_data, pci_ranges.count + 1, 2);
> + /* Node offset */
> + build_append_int_noprefix(table_data, viommu_off, 2);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 8);
> +
> + /* Virtio-iommu node */
> + /* Type (virtio-pci IOMMU) */
> + build_append_int_noprefix(table_data, 3, 1);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 1);
> + /* Length */
> + build_append_int_noprefix(table_data, 16, 2);
> + /* PCI Segment */
> + build_append_int_noprefix(table_data, 0, 2);
> + /* PCI BDF number */
> + build_append_int_noprefix(table_data, virtio_iommu_bdf, 2);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 8);
> +
> + /* PCI ranges found above */
> + g_array_append_vals(table_data, pci_ranges.blob->data,
> + pci_ranges.blob->len);
> + g_array_free(pci_ranges.blob, true);
> +
> + acpi_table_end(linker, &table);
> +}
> +
> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> index 3b5e118c54..622b0b50b7 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -51,6 +51,10 @@ config ACPI_VMGENID
> default y
> depends on PC
>
> +config ACPI_VIOT
> + bool
> + depends on ACPI
> +
> config ACPI_HW_REDUCED
> bool
> select ACPI
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index 7d8c0eb43e..adf6347bc4 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -20,6 +20,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'), if_false: files(
> acpi_ss.add(when: 'CONFIG_ACPI_PIIX4', if_true: files('piix4.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_true: files('pcihp.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_false: files('acpi-pci-hotplug-stub.c'))
> +acpi_ss.add(when: 'CONFIG_ACPI_VIOT', if_true: files('viot.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
> acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
> acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested with default root bus and pxb-pci. Feel free to add
Tested-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 01/10] hw/acpi: Add VIOT table
2021-09-14 14:19 ` [PATCH v3 01/10] hw/acpi: Add VIOT table Jean-Philippe Brucker
2021-09-16 12:44 ` Eric Auger
@ 2021-09-20 8:06 ` Igor Mammedov
2021-10-01 15:30 ` Jean-Philippe Brucker
2021-09-20 8:35 ` Igor Mammedov
2 siblings, 1 reply; 22+ messages in thread
From: Igor Mammedov @ 2021-09-20 8:06 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Tue, 14 Sep 2021 15:19:56 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> Add a function that generates a Virtual I/O Translation table (VIOT),
> describing the topology of paravirtual IOMMUs. The table is created when
> instantiating a virtio-iommu device. It contains a virtio-iommu node and
> PCI Range nodes for endpoints managed by the IOMMU. By default, a single
> node describes all PCI devices. When passing the "default_bus_bypass_iommu"
> machine option and "bypass_iommu" PXB option, only buses that do not
> bypass the IOMMU are described by PCI Range nodes.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> Sizes and types are hardcoded because it will now be the default style
> https://lore.kernel.org/qemu-devel/20210708154617.1538485-1-imammedo@redhat.com/
> ---
> hw/acpi/viot.h | 13 +++++
> hw/acpi/viot.c | 112 ++++++++++++++++++++++++++++++++++++++++++++
> hw/acpi/Kconfig | 4 ++
> hw/acpi/meson.build | 1 +
> 4 files changed, 130 insertions(+)
> create mode 100644 hw/acpi/viot.h
> create mode 100644 hw/acpi/viot.c
>
> diff --git a/hw/acpi/viot.h b/hw/acpi/viot.h
> new file mode 100644
> index 0000000000..4cef29a640
> --- /dev/null
> +++ b/hw/acpi/viot.h
> @@ -0,0 +1,13 @@
> +/*
> + * ACPI Virtual I/O Translation Table implementation
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef VIOT_H
> +#define VIOT_H
> +
> +void build_viot(GArray *table_data, BIOSLinker *linker,
> + uint16_t virtio_iommu_bdf, const char *oem_id,
> + const char *oem_table_id);
> +
> +#endif /* VIOT_H */
> diff --git a/hw/acpi/viot.c b/hw/acpi/viot.c
> new file mode 100644
> index 0000000000..e7f7605119
> --- /dev/null
> +++ b/hw/acpi/viot.c
> @@ -0,0 +1,112 @@
> +/*
> + * ACPI Virtual I/O Translation table implementation
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include "qemu/osdep.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/viot.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_host.h"
> +
> +struct viot_pci_ranges {
> + GArray *blob;
> + size_t count;
> + uint16_t output_node;
> +};
> +
> +/* Build PCI range for a given PCI host bridge */
> +static int viot_host_bridges(Object *obj, void *opaque)
> +{
> + struct viot_pci_ranges *pci_ranges = opaque;
> + GArray *blob = pci_ranges->blob;
> +
> + if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
> + PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
> +
> + if (bus && !pci_bus_bypass_iommu(bus)) {
> + int min_bus, max_bus;
> +
> + pci_bus_range(bus, &min_bus, &max_bus);
> +
> + /* Type (PCI range) */
> + build_append_int_noprefix(blob, 1, 1);
> + /* Reserved */
> + build_append_int_noprefix(blob, 0, 1);
> + /* Length */
> + build_append_int_noprefix(blob, 24, 2);
> + /* Endpoint start */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 4);
> + /* PCI Segment start */
> + build_append_int_noprefix(blob, 0, 2);
> + /* PCI Segment end */
> + build_append_int_noprefix(blob, 0, 2);
> + /* PCI BDF start */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 2);
> + /* PCI BDF end */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(max_bus, 0xff), 2);
> + /* Output node */
> + build_append_int_noprefix(blob, pci_ranges->output_node, 2);
> + /* Reserved */
> + build_append_int_noprefix(blob, 0, 6);
> +
> + pci_ranges->count++;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Generate a VIOT table with one PCI-based virtio-iommu that manages PCI
> + * endpoints.
We usually put a pointer to spec/revision and chapter in it
that describes being implemented table, so reviewer would
have a reference to compare code with. Otherwise I have no
idea if implementation is correct or not.
ex: build_hmat_mpda
the same applies to viot_host_bridges
> + */
> +void build_viot(GArray *table_data, BIOSLinker *linker,
> + uint16_t virtio_iommu_bdf, const char *oem_id,
> + const char *oem_table_id)
> +{
> + /* The virtio-iommu node follows the 48-bytes header */
> + int viommu_off = 48;
> + AcpiTable table = { .sig = "VIOT", .rev = 0,
> + .oem_id = oem_id, .oem_table_id = oem_table_id };
> + struct viot_pci_ranges pci_ranges = {
> + .output_node = viommu_off,
> + .blob = g_array_new(false, true, 1),
> + };
> +
> + /* Build the list of PCI ranges that this viommu manages */
> + object_child_foreach_recursive(object_get_root(), viot_host_bridges,
> + &pci_ranges);
> +
> + /* ACPI table header */
> + acpi_table_begin(&table, table_data);
> + /* Node count */
> + build_append_int_noprefix(table_data, pci_ranges.count + 1, 2);
> + /* Node offset */
> + build_append_int_noprefix(table_data, viommu_off, 2);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 8);
> +
> + /* Virtio-iommu node */
> + /* Type (virtio-pci IOMMU) */
> + build_append_int_noprefix(table_data, 3, 1);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 1);
> + /* Length */
> + build_append_int_noprefix(table_data, 16, 2);
> + /* PCI Segment */
> + build_append_int_noprefix(table_data, 0, 2);
> + /* PCI BDF number */
> + build_append_int_noprefix(table_data, virtio_iommu_bdf, 2);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 8);
> +
> + /* PCI ranges found above */
> + g_array_append_vals(table_data, pci_ranges.blob->data,
> + pci_ranges.blob->len);
> + g_array_free(pci_ranges.blob, true);
> +
> + acpi_table_end(linker, &table);
> +}
> +
> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> index 3b5e118c54..622b0b50b7 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -51,6 +51,10 @@ config ACPI_VMGENID
> default y
> depends on PC
>
> +config ACPI_VIOT
> + bool
> + depends on ACPI
> +
> config ACPI_HW_REDUCED
> bool
> select ACPI
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index 7d8c0eb43e..adf6347bc4 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -20,6 +20,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'), if_false: files(
> acpi_ss.add(when: 'CONFIG_ACPI_PIIX4', if_true: files('piix4.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_true: files('pcihp.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_false: files('acpi-pci-hotplug-stub.c'))
> +acpi_ss.add(when: 'CONFIG_ACPI_VIOT', if_true: files('viot.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
> acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
> acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 01/10] hw/acpi: Add VIOT table
2021-09-20 8:06 ` Igor Mammedov
@ 2021-10-01 15:30 ` Jean-Philippe Brucker
0 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-01 15:30 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Mon, Sep 20, 2021 at 10:06:09AM +0200, Igor Mammedov wrote:
> > +/*
> > + * Generate a VIOT table with one PCI-based virtio-iommu that manages PCI
> > + * endpoints.
>
> We usually put a pointer to spec/revision and chapter in it
> that describes being implemented table, so reviewer would
> have a reference to compare code with. Otherwise I have no
> idea if implementation is correct or not.
The 6.5 ACPI specification isn't out yet unfortunately, so I can't put any
reference at the moment. I don't know when it will be published. The draft
specification is here: https://jpbrucker.net/virtio-iommu/viot/viot-v9.pdf
But that's only temporary. I can make a note to add the pointers once the
spec is out.
The acpica implementation could also be used for cross-reference:
https://github.com/acpica/acpica/commit/fc4e33319c1ee08f20f5c44853dd8426643f6dfd
Thanks,
Jean
>
> ex: build_hmat_mpda
>
> the same applies to viot_host_bridges
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 01/10] hw/acpi: Add VIOT table
2021-09-14 14:19 ` [PATCH v3 01/10] hw/acpi: Add VIOT table Jean-Philippe Brucker
2021-09-16 12:44 ` Eric Auger
2021-09-20 8:06 ` Igor Mammedov
@ 2021-09-20 8:35 ` Igor Mammedov
2 siblings, 0 replies; 22+ messages in thread
From: Igor Mammedov @ 2021-09-20 8:35 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Tue, 14 Sep 2021 15:19:56 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> Add a function that generates a Virtual I/O Translation table (VIOT),
> describing the topology of paravirtual IOMMUs. The table is created when
> instantiating a virtio-iommu device. It contains a virtio-iommu node and
> PCI Range nodes for endpoints managed by the IOMMU. By default, a single
> node describes all PCI devices. When passing the "default_bus_bypass_iommu"
> machine option and "bypass_iommu" PXB option, only buses that do not
> bypass the IOMMU are described by PCI Range nodes.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> Sizes and types are hardcoded because it will now be the default style
> https://lore.kernel.org/qemu-devel/20210708154617.1538485-1-imammedo@redhat.com/
> ---
> hw/acpi/viot.h | 13 +++++
> hw/acpi/viot.c | 112 ++++++++++++++++++++++++++++++++++++++++++++
> hw/acpi/Kconfig | 4 ++
> hw/acpi/meson.build | 1 +
> 4 files changed, 130 insertions(+)
> create mode 100644 hw/acpi/viot.h
> create mode 100644 hw/acpi/viot.c
>
> diff --git a/hw/acpi/viot.h b/hw/acpi/viot.h
> new file mode 100644
> index 0000000000..4cef29a640
> --- /dev/null
> +++ b/hw/acpi/viot.h
> @@ -0,0 +1,13 @@
> +/*
> + * ACPI Virtual I/O Translation Table implementation
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef VIOT_H
> +#define VIOT_H
> +
> +void build_viot(GArray *table_data, BIOSLinker *linker,
> + uint16_t virtio_iommu_bdf, const char *oem_id,
> + const char *oem_table_id);
> +
> +#endif /* VIOT_H */
> diff --git a/hw/acpi/viot.c b/hw/acpi/viot.c
> new file mode 100644
> index 0000000000..e7f7605119
> --- /dev/null
> +++ b/hw/acpi/viot.c
> @@ -0,0 +1,112 @@
> +/*
> + * ACPI Virtual I/O Translation table implementation
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include "qemu/osdep.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/viot.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_host.h"
> +
> +struct viot_pci_ranges {
> + GArray *blob;
> + size_t count;
> + uint16_t output_node;
> +};
> +
> +/* Build PCI range for a given PCI host bridge */
> +static int viot_host_bridges(Object *obj, void *opaque)
> +{
> + struct viot_pci_ranges *pci_ranges = opaque;
> + GArray *blob = pci_ranges->blob;
> +
> + if (object_dynamic_cast(obj, TYPE_PCI_HOST_BRIDGE)) {
> + PCIBus *bus = PCI_HOST_BRIDGE(obj)->bus;
> +
> + if (bus && !pci_bus_bypass_iommu(bus)) {
> + int min_bus, max_bus;
> +
> + pci_bus_range(bus, &min_bus, &max_bus);
> +
> + /* Type (PCI range) */
> + build_append_int_noprefix(blob, 1, 1);
> + /* Reserved */
> + build_append_int_noprefix(blob, 0, 1);
> + /* Length */
> + build_append_int_noprefix(blob, 24, 2);
> + /* Endpoint start */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 4);
> + /* PCI Segment start */
> + build_append_int_noprefix(blob, 0, 2);
> + /* PCI Segment end */
> + build_append_int_noprefix(blob, 0, 2);
> + /* PCI BDF start */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(min_bus, 0), 2);
> + /* PCI BDF end */
> + build_append_int_noprefix(blob, PCI_BUILD_BDF(max_bus, 0xff), 2);
> + /* Output node */
> + build_append_int_noprefix(blob, pci_ranges->output_node, 2);
> + /* Reserved */
> + build_append_int_noprefix(blob, 0, 6);
> +
> + pci_ranges->count++;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Generate a VIOT table with one PCI-based virtio-iommu that manages PCI
> + * endpoints.
> + */
> +void build_viot(GArray *table_data, BIOSLinker *linker,
> + uint16_t virtio_iommu_bdf, const char *oem_id,
> + const char *oem_table_id)
> +{
> + /* The virtio-iommu node follows the 48-bytes header */
> + int viommu_off = 48;
> + AcpiTable table = { .sig = "VIOT", .rev = 0,
> + .oem_id = oem_id, .oem_table_id = oem_table_id };
> + struct viot_pci_ranges pci_ranges = {
> + .output_node = viommu_off,
> + .blob = g_array_new(false, true, 1),
> + };
> +
> + /* Build the list of PCI ranges that this viommu manages */
> + object_child_foreach_recursive(object_get_root(), viot_host_bridges,
I'd limit scope to machine and re-use one that caller
already uses.
in arm case: virt_acpi_build(VirtMachineState *vms, ...
q35: acpi_build(..., MachineState *machine)
> + &pci_ranges);
> +
> + /* ACPI table header */
> + acpi_table_begin(&table, table_data);
> + /* Node count */
> + build_append_int_noprefix(table_data, pci_ranges.count + 1, 2);
> + /* Node offset */
> + build_append_int_noprefix(table_data, viommu_off, 2);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 8);
> +
> + /* Virtio-iommu node */
> + /* Type (virtio-pci IOMMU) */
> + build_append_int_noprefix(table_data, 3, 1);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 1);
> + /* Length */
> + build_append_int_noprefix(table_data, 16, 2);
> + /* PCI Segment */
> + build_append_int_noprefix(table_data, 0, 2);
> + /* PCI BDF number */
> + build_append_int_noprefix(table_data, virtio_iommu_bdf, 2);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 8);
> +
> + /* PCI ranges found above */
> + g_array_append_vals(table_data, pci_ranges.blob->data,
> + pci_ranges.blob->len);
> + g_array_free(pci_ranges.blob, true);
> +
> + acpi_table_end(linker, &table);
> +}
> +
> diff --git a/hw/acpi/Kconfig b/hw/acpi/Kconfig
> index 3b5e118c54..622b0b50b7 100644
> --- a/hw/acpi/Kconfig
> +++ b/hw/acpi/Kconfig
> @@ -51,6 +51,10 @@ config ACPI_VMGENID
> default y
> depends on PC
>
> +config ACPI_VIOT
> + bool
> + depends on ACPI
> +
> config ACPI_HW_REDUCED
> bool
> select ACPI
> diff --git a/hw/acpi/meson.build b/hw/acpi/meson.build
> index 7d8c0eb43e..adf6347bc4 100644
> --- a/hw/acpi/meson.build
> +++ b/hw/acpi/meson.build
> @@ -20,6 +20,7 @@ acpi_ss.add(when: 'CONFIG_ACPI_APEI', if_true: files('ghes.c'), if_false: files(
> acpi_ss.add(when: 'CONFIG_ACPI_PIIX4', if_true: files('piix4.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_true: files('pcihp.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_PCIHP', if_false: files('acpi-pci-hotplug-stub.c'))
> +acpi_ss.add(when: 'CONFIG_ACPI_VIOT', if_true: files('viot.c'))
> acpi_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('ich9.c', 'tco.c'))
> acpi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c'), if_false: files('ipmi-stub.c'))
> acpi_ss.add(when: 'CONFIG_PC', if_false: files('acpi-x86-stub.c'))
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 02/10] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
2021-09-14 14:19 ` [PATCH v3 01/10] hw/acpi: Add VIOT table Jean-Philippe Brucker
@ 2021-09-14 14:19 ` Jean-Philippe Brucker
2021-09-14 14:19 ` [PATCH v3 03/10] hw/arm/virt: Remove device tree restriction " Jean-Philippe Brucker
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:19 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
When a virtio-iommu is instantiated, describe it using the ACPI VIOT
table.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
hw/arm/virt-acpi-build.c | 7 +++++++
hw/arm/Kconfig | 1 +
2 files changed, 8 insertions(+)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index ebe9d1726f..23c2e1aaf2 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -55,6 +55,7 @@
#include "kvm_arm.h"
#include "migration/vmstate.h"
#include "hw/acpi/ghes.h"
+#include "hw/acpi/viot.h"
#define ARM_SPI_BASE 32
@@ -928,6 +929,12 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
}
#endif
+ if (vms->iommu == VIRT_IOMMU_VIRTIO) {
+ acpi_add_table(table_offsets, tables_blob);
+ build_viot(tables_blob, tables->linker, vms->virtio_iommu_bdf,
+ vms->oem_id, vms->oem_table_id);
+ }
+
/* XSDT is pointed to by RSDP */
xsdt = tables_blob->len;
build_xsdt(tables_blob, tables->linker, table_offsets, vms->oem_id,
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 78fdd1b935..8b04a1d53a 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -27,6 +27,7 @@ config ARM_VIRT
select DIMM
select ACPI_HW_REDUCED
select ACPI_APEI
+ select ACPI_VIOT
config CHEETAH
bool
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 03/10] hw/arm/virt: Remove device tree restriction for virtio-iommu
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
2021-09-14 14:19 ` [PATCH v3 01/10] hw/acpi: Add VIOT table Jean-Philippe Brucker
2021-09-14 14:19 ` [PATCH v3 02/10] hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu Jean-Philippe Brucker
@ 2021-09-14 14:19 ` Jean-Philippe Brucker
2021-09-14 14:19 ` [PATCH v3 04/10] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
` (6 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:19 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
virtio-iommu is now supported with ACPI VIOT as well as device tree.
Remove the restriction that prevents from instantiating a virtio-iommu
device under ACPI.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
hw/arm/virt.c | 10 ++--------
hw/virtio/virtio-iommu-pci.c | 7 -------
2 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 73e9c6bb7c..f238766aa1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2547,16 +2547,10 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
MachineClass *mc = MACHINE_GET_CLASS(machine);
if (device_is_dynamic_sysbus(mc, dev) ||
- (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);
}
- if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
- VirtMachineState *vms = VIRT_MACHINE(machine);
-
- if (!vms->bootinfo.firmware_loaded || !virt_is_acpi_enabled(vms)) {
- return HOTPLUG_HANDLER(machine);
- }
- }
return NULL;
}
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 770c286be7..f30eb16cbf 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -48,16 +48,9 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) {
- MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
-
- error_setg(errp,
- "%s machine fails to create iommu-map device tree bindings",
- mc->name);
error_append_hint(errp,
"Check your machine implements a hotplug handler "
"for the virtio-iommu-pci device\n");
- error_append_hint(errp, "Check the guest is booted without FW or with "
- "-no-acpi\n");
return;
}
for (int i = 0; i < s->nb_reserved_regions; i++) {
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 04/10] hw/arm/virt: Reject instantiation of multiple IOMMUs
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
` (2 preceding siblings ...)
2021-09-14 14:19 ` [PATCH v3 03/10] hw/arm/virt: Remove device tree restriction " Jean-Philippe Brucker
@ 2021-09-14 14:19 ` Jean-Philippe Brucker
2021-09-16 12:45 ` Eric Auger
2021-09-20 8:11 ` Igor Mammedov
2021-09-14 14:20 ` [PATCH v3 05/10] pc: Allow instantiating a virtio-iommu device Jean-Philippe Brucker
` (5 subsequent siblings)
9 siblings, 2 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:19 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
We do not support instantiating multiple IOMMUs. Before adding a
virtio-iommu, check that no other IOMMU is present. This will detect
both "iommu=smmuv3" machine parameter and another virtio-iommu instance.
Fixes: 70e89132c9 ("hw/arm/virt: Add the virtio-iommu device tree mappings")
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
hw/arm/virt.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f238766aa1..26069f943a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2471,6 +2471,11 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
PCIDevice *pdev = PCI_DEVICE(dev);
+ if (vms->iommu != VIRT_IOMMU_NONE) {
+ error_setg(errp, "virt machine does not support multiple IOMMUs");
+ return;
+ }
+
vms->iommu = VIRT_IOMMU_VIRTIO;
vms->virtio_iommu_bdf = pci_get_bdf(pdev);
create_virtio_iommu_dt_bindings(vms);
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 04/10] hw/arm/virt: Reject instantiation of multiple IOMMUs
2021-09-14 14:19 ` [PATCH v3 04/10] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
@ 2021-09-16 12:45 ` Eric Auger
2021-09-20 8:11 ` Igor Mammedov
1 sibling, 0 replies; 22+ messages in thread
From: Eric Auger @ 2021-09-16 12:45 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, qemu-arm, pbonzini, ani, imammedo
Hi Jean,
On 9/14/21 4:19 PM, Jean-Philippe Brucker wrote:
> We do not support instantiating multiple IOMMUs. Before adding a
> virtio-iommu, check that no other IOMMU is present. This will detect
> both "iommu=smmuv3" machine parameter and another virtio-iommu instance.
>
> Fixes: 70e89132c9 ("hw/arm/virt: Add the virtio-iommu device tree mappings")
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Tested-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/arm/virt.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f238766aa1..26069f943a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2471,6 +2471,11 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
> if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> PCIDevice *pdev = PCI_DEVICE(dev);
>
> + if (vms->iommu != VIRT_IOMMU_NONE) {
> + error_setg(errp, "virt machine does not support multiple IOMMUs");
> + return;
> + }
> +
> vms->iommu = VIRT_IOMMU_VIRTIO;
> vms->virtio_iommu_bdf = pci_get_bdf(pdev);
> create_virtio_iommu_dt_bindings(vms);
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 04/10] hw/arm/virt: Reject instantiation of multiple IOMMUs
2021-09-14 14:19 ` [PATCH v3 04/10] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
2021-09-16 12:45 ` Eric Auger
@ 2021-09-20 8:11 ` Igor Mammedov
1 sibling, 0 replies; 22+ messages in thread
From: Igor Mammedov @ 2021-09-20 8:11 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Tue, 14 Sep 2021 15:19:59 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> We do not support instantiating multiple IOMMUs. Before adding a
> virtio-iommu, check that no other IOMMU is present. This will detect
> both "iommu=smmuv3" machine parameter and another virtio-iommu instance.
>
> Fixes: 70e89132c9 ("hw/arm/virt: Add the virtio-iommu device tree mappings")
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> hw/arm/virt.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f238766aa1..26069f943a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2471,6 +2471,11 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
> if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> PCIDevice *pdev = PCI_DEVICE(dev);
>
> + if (vms->iommu != VIRT_IOMMU_NONE) {
> + error_setg(errp, "virt machine does not support multiple IOMMUs");
> + return;
> + }
can you move check into 'pre_plug' callback?
(plug should not fail and just finish up whatever was
verified/set by pre_plug, there are plans to remove errp
argument from 'plug' callback)
> +
> vms->iommu = VIRT_IOMMU_VIRTIO;
> vms->virtio_iommu_bdf = pci_get_bdf(pdev);
> create_virtio_iommu_dt_bindings(vms);
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 05/10] pc: Allow instantiating a virtio-iommu device
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
` (3 preceding siblings ...)
2021-09-14 14:19 ` [PATCH v3 04/10] hw/arm/virt: Reject instantiation of multiple IOMMUs Jean-Philippe Brucker
@ 2021-09-14 14:20 ` Jean-Philippe Brucker
2021-09-20 8:24 ` Igor Mammedov
2021-09-14 14:20 ` [PATCH v3 06/10] tests/acpi: allow updates of VIOT expected data files Jean-Philippe Brucker
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:20 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
Allow instantiating a virtio-iommu device by adding an ACPI Virtual I/O
Translation table (VIOT), which describes the relation between the
virtio-iommu and the endpoints it manages.
Add a hotplug handler for virtio-iommu on x86 and set the necessary
reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA
region is reserved for MSIs. DMA transactions to this range either
trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.
Although virtio-iommu does not support IRQ remapping it must be informed
of the reserved region so that it can forward DMA transactions targeting
this region.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
include/hw/i386/pc.h | 2 ++
hw/i386/acpi-build.c | 5 +++++
hw/i386/pc.c | 28 +++++++++++++++++++++++++++-
hw/i386/Kconfig | 1 +
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 82cf7b7e30..f3ba1ee4c0 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -45,6 +45,8 @@ typedef struct PCMachineState {
bool pit_enabled;
bool hpet_enabled;
bool default_bus_bypass_iommu;
+ bool virtio_iommu;
+ uint16_t virtio_iommu_bdf;
uint64_t max_fw_size;
/* ACPI Memory hotplug IO base address */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 547cd4ed9d..76845026d8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -71,6 +71,7 @@
#include "hw/acpi/ipmi.h"
#include "hw/acpi/hmat.h"
+#include "hw/acpi/viot.h"
/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
* -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
@@ -2593,6 +2594,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
x86ms->oem_table_id);
}
+ } else if (pcms->virtio_iommu) {
+ acpi_add_table(table_offsets, tables_blob);
+ build_viot(tables_blob, tables->linker, pcms->virtio_iommu_bdf,
+ x86ms->oem_id, x86ms->oem_table_id);
}
if (machine->nvdimms_state->is_enabled) {
nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7e523b913c..a31e950599 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -83,6 +83,7 @@
#include "hw/i386/intel_iommu.h"
#include "hw/net/ne2000-isa.h"
#include "standard-headers/asm-x86/bootparam.h"
+#include "hw/virtio/virtio-iommu.h"
#include "hw/virtio/virtio-pmem-pci.h"
#include "hw/virtio/virtio-mem-pci.h"
#include "hw/mem/memory-device.h"
@@ -798,6 +799,11 @@ void pc_machine_done(Notifier *notifier, void *data)
"irqchip support.");
exit(EXIT_FAILURE);
}
+
+ if (pcms->virtio_iommu && x86_iommu_get_default()) {
+ error_report("QEMU does not support multiple vIOMMUs for x86 yet.");
+ exit(EXIT_FAILURE);
+ }
}
void pc_guest_info_init(PCMachineState *pcms)
@@ -1368,6 +1374,14 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+ /* Declare the reserved MSI region */
+ char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
+ VIRTIO_IOMMU_RESV_MEM_T_MSI);
+
+ qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
+ qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
+ g_free(resv_prop_str);
}
}
@@ -1381,6 +1395,17 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
pc_virtio_md_pci_plug(hotplug_dev, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+ PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+ PCIDevice *pdev = PCI_DEVICE(dev);
+
+ if (pcms->virtio_iommu) {
+ error_setg(errp,
+ "QEMU does not support multiple vIOMMUs for x86 yet.");
+ return;
+ }
+ pcms->virtio_iommu = true;
+ pcms->virtio_iommu_bdf = pci_get_bdf(pdev);
}
}
@@ -1422,7 +1447,8 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
- object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+ object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
+ object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
return HOTPLUG_HANDLER(machine);
}
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index ddedcef0b2..13db05d557 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -54,6 +54,7 @@ config PC_ACPI
select ACPI_X86
select ACPI_CPU_HOTPLUG
select ACPI_MEMORY_HOTPLUG
+ select ACPI_VIOT
select SMBUS_EEPROM
select PFLASH_CFI01
depends on ACPI_SMBUS
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 05/10] pc: Allow instantiating a virtio-iommu device
2021-09-14 14:20 ` [PATCH v3 05/10] pc: Allow instantiating a virtio-iommu device Jean-Philippe Brucker
@ 2021-09-20 8:24 ` Igor Mammedov
2021-10-01 15:36 ` Jean-Philippe Brucker
0 siblings, 1 reply; 22+ messages in thread
From: Igor Mammedov @ 2021-09-20 8:24 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Tue, 14 Sep 2021 15:20:00 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> Allow instantiating a virtio-iommu device by adding an ACPI Virtual I/O
> Translation table (VIOT), which describes the relation between the
> virtio-iommu and the endpoints it manages.
>
> Add a hotplug handler for virtio-iommu on x86 and set the necessary
> reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA
> region is reserved for MSIs. DMA transactions to this range either
> trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.
>
> Although virtio-iommu does not support IRQ remapping it must be informed
> of the reserved region so that it can forward DMA transactions targeting
> this region.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> include/hw/i386/pc.h | 2 ++
> hw/i386/acpi-build.c | 5 +++++
> hw/i386/pc.c | 28 +++++++++++++++++++++++++++-
> hw/i386/Kconfig | 1 +
> 4 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 82cf7b7e30..f3ba1ee4c0 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -45,6 +45,8 @@ typedef struct PCMachineState {
> bool pit_enabled;
> bool hpet_enabled;
> bool default_bus_bypass_iommu;
> + bool virtio_iommu;
> + uint16_t virtio_iommu_bdf;
> uint64_t max_fw_size;
>
> /* ACPI Memory hotplug IO base address */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 547cd4ed9d..76845026d8 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -71,6 +71,7 @@
>
> #include "hw/acpi/ipmi.h"
> #include "hw/acpi/hmat.h"
> +#include "hw/acpi/viot.h"
>
> /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
> @@ -2593,6 +2594,10 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> build_dmar_q35(tables_blob, tables->linker, x86ms->oem_id,
> x86ms->oem_table_id);
> }
> + } else if (pcms->virtio_iommu) {
> + acpi_add_table(table_offsets, tables_blob);
> + build_viot(tables_blob, tables->linker, pcms->virtio_iommu_bdf,
> + x86ms->oem_id, x86ms->oem_table_id);
> }
> if (machine->nvdimms_state->is_enabled) {
> nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7e523b913c..a31e950599 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -83,6 +83,7 @@
> #include "hw/i386/intel_iommu.h"
> #include "hw/net/ne2000-isa.h"
> #include "standard-headers/asm-x86/bootparam.h"
> +#include "hw/virtio/virtio-iommu.h"
> #include "hw/virtio/virtio-pmem-pci.h"
> #include "hw/virtio/virtio-mem-pci.h"
> #include "hw/mem/memory-device.h"
> @@ -798,6 +799,11 @@ void pc_machine_done(Notifier *notifier, void *data)
> "irqchip support.");
> exit(EXIT_FAILURE);
> }
> +
> + if (pcms->virtio_iommu && x86_iommu_get_default()) {
> + error_report("QEMU does not support multiple vIOMMUs for x86 yet.");
> + exit(EXIT_FAILURE);
> + }
previous patch does similar check, doesn't it?
So is why it's not implement the same way?
> }
>
> void pc_guest_info_init(PCMachineState *pcms)
> @@ -1368,6 +1374,14 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> + /* Declare the reserved MSI region */
> + char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
> + VIRTIO_IOMMU_RESV_MEM_T_MSI);
add a comment describing where these values come from, pls.
> +
> + qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
> + qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
why not use qom setters directly
(they have error argument and can gracefully error out,
which is expected error handling in pre_plug)
and fix up similar (ab)use of setters in virt_machine_device_pre_plug_cb()
> + g_free(resv_prop_str);
> }
> }
>
> @@ -1381,6 +1395,17 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
> } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> pc_virtio_md_pci_plug(hotplug_dev, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> + PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> + PCIDevice *pdev = PCI_DEVICE(dev);
> +
> + if (pcms->virtio_iommu) {
> + error_setg(errp,
> + "QEMU does not support multiple vIOMMUs for x86 yet.");
> + return;
> + }
move to pre_plug please
> + pcms->virtio_iommu = true;
> + pcms->virtio_iommu_bdf = pci_get_bdf(pdev);
> }
> }
>
> @@ -1422,7 +1447,8 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
> object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
> object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
> - object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
> + object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> return HOTPLUG_HANDLER(machine);
> }
>
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index ddedcef0b2..13db05d557 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -54,6 +54,7 @@ config PC_ACPI
> select ACPI_X86
> select ACPI_CPU_HOTPLUG
> select ACPI_MEMORY_HOTPLUG
> + select ACPI_VIOT
> select SMBUS_EEPROM
> select PFLASH_CFI01
> depends on ACPI_SMBUS
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 05/10] pc: Allow instantiating a virtio-iommu device
2021-09-20 8:24 ` Igor Mammedov
@ 2021-10-01 15:36 ` Jean-Philippe Brucker
0 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-01 15:36 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Mon, Sep 20, 2021 at 10:24:40AM +0200, Igor Mammedov wrote:
> > + if (pcms->virtio_iommu && x86_iommu_get_default()) {
> > + error_report("QEMU does not support multiple vIOMMUs for x86 yet.");
> > + exit(EXIT_FAILURE);
> > + }
>
> previous patch does similar check, doesn't it?
> So is why it's not implement the same way?
The existing check for Intel and AMD IOMMUs is in x86_iommu_set_default(),
but virtio-iommu isn't an X86IOMMUState (because it already inherits
virtio object, and because X86IOMMUState is used for IRQ remapping which
isn't supported by virtio-iommu).
I'll move the check from X86IOMMUState into pre_plug to avoid the
duplication.
Thanks,
Jean
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 06/10] tests/acpi: allow updates of VIOT expected data files
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
` (4 preceding siblings ...)
2021-09-14 14:20 ` [PATCH v3 05/10] pc: Allow instantiating a virtio-iommu device Jean-Philippe Brucker
@ 2021-09-14 14:20 ` Jean-Philippe Brucker
2021-09-14 14:20 ` [PATCH v3 07/10] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
` (3 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:20 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
Create empty data files and allow updates for the upcoming VIOT tests.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
tests/data/acpi/q35/DSDT.viot | 0
tests/data/acpi/q35/VIOT.viot | 0
tests/data/acpi/virt/VIOT | 0
4 files changed, 3 insertions(+)
create mode 100644 tests/data/acpi/q35/DSDT.viot
create mode 100644 tests/data/acpi/q35/VIOT.viot
create mode 100644 tests/data/acpi/virt/VIOT
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..29b5b1eabc 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,4 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/VIOT",
+"tests/data/acpi/q35/DSDT.viot",
+"tests/data/acpi/q35/VIOT.viot",
diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT
new file mode 100644
index 0000000000..e69de29bb2
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 07/10] tests/acpi: add test cases for VIOT
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
` (5 preceding siblings ...)
2021-09-14 14:20 ` [PATCH v3 06/10] tests/acpi: allow updates of VIOT expected data files Jean-Philippe Brucker
@ 2021-09-14 14:20 ` Jean-Philippe Brucker
2021-09-20 8:39 ` Igor Mammedov
2021-09-14 14:20 ` [PATCH v3 08/10] tests/acpi: add expected VIOT blob for virt machine Jean-Philippe Brucker
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:20 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
Add two test cases for VIOT, one on the q35 machine and the other on
virt. To test complex topologies the q35 test has two PCIe buses that
bypass the IOMMU (and are therefore not described by VIOT), and two
buses that are translated by virtio-iommu.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
tests/qtest/bios-tables-test.c | 39 ++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 4f11d03055..f8bfe2f247 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1403,6 +1403,43 @@ static void test_acpi_virt_tcg(void)
free_test_data(&data);
}
+static void test_acpi_q35_viot(void)
+{
+ test_data data = {
+ .machine = MACHINE_Q35,
+ .variant = ".viot",
+ .blkdev = "virtio-blk,bus=pcie.0",
+ };
+
+ /*
+ * To keep things interesting, two buses bypass the IOMMU.
+ * VIOT should only describes the other two buses.
+ */
+ test_acpi_one("-machine default_bus_bypass_iommu=on "
+ "-device virtio-iommu "
+ "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 "
+ "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on "
+ "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0",
+ &data);
+ free_test_data(&data);
+}
+
+static void test_acpi_virt_viot(void)
+{
+ test_data data = {
+ .machine = "virt",
+ .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+ .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+ .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+ .ram_start = 0x40000000ULL,
+ .scan_len = 128ULL * 1024 * 1024,
+ };
+
+ test_acpi_one("-cpu cortex-a57 "
+ "-device virtio-iommu", &data);
+ free_test_data(&data);
+}
+
static void test_oem_fields(test_data *data)
{
int i;
@@ -1567,12 +1604,14 @@ int main(int argc, char *argv[])
if (strcmp(arch, "x86_64") == 0) {
qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
}
+ qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
} else if (strcmp(arch, "aarch64") == 0) {
qtest_add_func("acpi/virt", test_acpi_virt_tcg);
qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
+ qtest_add_func("acpi/virt/viot", test_acpi_virt_viot);
}
ret = g_test_run();
boot_sector_cleanup(disk);
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 07/10] tests/acpi: add test cases for VIOT
2021-09-14 14:20 ` [PATCH v3 07/10] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
@ 2021-09-20 8:39 ` Igor Mammedov
2021-10-01 15:37 ` Jean-Philippe Brucker
0 siblings, 1 reply; 22+ messages in thread
From: Igor Mammedov @ 2021-09-20 8:39 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Tue, 14 Sep 2021 15:20:02 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> Add two test cases for VIOT, one on the q35 machine and the other on
> virt. To test complex topologies the q35 test has two PCIe buses that
> bypass the IOMMU (and are therefore not described by VIOT), and two
> buses that are translated by virtio-iommu.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> tests/qtest/bios-tables-test.c | 39 ++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 4f11d03055..f8bfe2f247 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1403,6 +1403,43 @@ static void test_acpi_virt_tcg(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_q35_viot(void)
> +{
> + test_data data = {
> + .machine = MACHINE_Q35,
> + .variant = ".viot",
> + .blkdev = "virtio-blk,bus=pcie.0",
why is this necessary?
> + };
> +
> + /*
> + * To keep things interesting, two buses bypass the IOMMU.
> + * VIOT should only describes the other two buses.
> + */
> + test_acpi_one("-machine default_bus_bypass_iommu=on "
> + "-device virtio-iommu "
> + "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 "
> + "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on "
> + "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0",
> + &data);
> + free_test_data(&data);
> +}
> +
> +static void test_acpi_virt_viot(void)
> +{
> + test_data data = {
> + .machine = "virt",
> + .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> + .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> + .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> + .ram_start = 0x40000000ULL,
> + .scan_len = 128ULL * 1024 * 1024,
> + };
> +
> + test_acpi_one("-cpu cortex-a57 "
> + "-device virtio-iommu", &data);
> + free_test_data(&data);
> +}
> +
> static void test_oem_fields(test_data *data)
> {
> int i;
> @@ -1567,12 +1604,14 @@ int main(int argc, char *argv[])
> if (strcmp(arch, "x86_64") == 0) {
> qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
> }
> + qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
> } else if (strcmp(arch, "aarch64") == 0) {
> qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
> qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
> qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
> qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
> + qtest_add_func("acpi/virt/viot", test_acpi_virt_viot);
> }
> ret = g_test_run();
> boot_sector_cleanup(disk);
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v3 07/10] tests/acpi: add test cases for VIOT
2021-09-20 8:39 ` Igor Mammedov
@ 2021-10-01 15:37 ` Jean-Philippe Brucker
0 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-10-01 15:37 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Mon, Sep 20, 2021 at 10:39:36AM +0200, Igor Mammedov wrote:
> > +static void test_acpi_q35_viot(void)
> > +{
> > + test_data data = {
> > + .machine = MACHINE_Q35,
> > + .variant = ".viot",
>
> > + .blkdev = "virtio-blk,bus=pcie.0",
> why is this necessary?
It isn't actually, I think an earlier version of the patch needed it.
I'll remove this.
Thanks,
Jean
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 08/10] tests/acpi: add expected VIOT blob for virt machine
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
` (6 preceding siblings ...)
2021-09-14 14:20 ` [PATCH v3 07/10] tests/acpi: add test cases for VIOT Jean-Philippe Brucker
@ 2021-09-14 14:20 ` Jean-Philippe Brucker
2021-09-14 14:20 ` [PATCH v3 09/10] tests/acpi: add expected DSDT blob for VIOT test on q35 Jean-Philippe Brucker
2021-09-14 14:20 ` [PATCH v3 10/10] tests/acpi: add expected VIOT blob for q35 machine Jean-Philippe Brucker
9 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:20 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
The VIOT blob contains the following:
[000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table]
[004h 0004 4] Table Length : 00000058
[008h 0008 1] Revision : 00
[009h 0009 1] Checksum : 66
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPC "
[018h 0024 4] Oem Revision : 00000001
[01Ch 0028 4] Asl Compiler ID : "BXPC"
[020h 0032 4] Asl Compiler Revision : 00000001
[024h 0036 2] Node count : 0002
[026h 0038 2] Node offset : 0030
[028h 0040 8] Reserved : 0000000000000000
[030h 0048 1] Type : 03 [VirtIO-PCI IOMMU]
[031h 0049 1] Reserved : 00
[032h 0050 2] Length : 0010
[034h 0052 2] PCI Segment : 0000
[036h 0054 2] PCI BDF number : 0008
[038h 0056 8] Reserved : 0000000000000000
[040h 0064 1] Type : 01 [PCI Range]
[041h 0065 1] Reserved : 00
[042h 0066 2] Length : 0018
[044h 0068 4] Endpoint start : 00000000
[048h 0072 2] PCI Segment start : 0000
[04Ah 0074 2] PCI Segment end : 0000
[04Ch 0076 2] PCI BDF start : 0000
[04Eh 0078 2] PCI BDF end : 00FF
[050h 0080 2] Output node : 0030
[052h 0082 6] Reserved : 000000000000
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
tests/data/acpi/virt/VIOT | Bin 0 -> 88 bytes
2 files changed, 1 deletion(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 29b5b1eabc..fa213e4738 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1,3 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/VIOT",
"tests/data/acpi/q35/DSDT.viot",
"tests/data/acpi/q35/VIOT.viot",
diff --git a/tests/data/acpi/virt/VIOT b/tests/data/acpi/virt/VIOT
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..921f40d88c28ba2171a4d664e119914335309e7d 100644
GIT binary patch
literal 88
zcmWIZ^bd((0D?3pe`k+i1*eDrX9XZ&1PX!JAexE60Hgv8m>C3sGzXN&z`)2L0cSHX
I{D-Rq0Q5fy0RR91
literal 0
HcmV?d00001
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 09/10] tests/acpi: add expected DSDT blob for VIOT test on q35
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
` (7 preceding siblings ...)
2021-09-14 14:20 ` [PATCH v3 08/10] tests/acpi: add expected VIOT blob for virt machine Jean-Philippe Brucker
@ 2021-09-14 14:20 ` Jean-Philippe Brucker
2021-09-14 14:20 ` [PATCH v3 10/10] tests/acpi: add expected VIOT blob for q35 machine Jean-Philippe Brucker
9 siblings, 0 replies; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:20 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
The VIOT test instantiates two virtio devices and PCIe expander bridges,
so the DSDT has additional blocks:
+ Scope (\_SB)
+ {
+ Device (PC30)
+ {
+ Name (_UID, 0x30) // _UID: Unique ID
+ Name (_BBN, 0x30) // _BBN: BIOS Bus Number
+ Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID
+ Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID
+ Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities
+ {
+ CreateDWordField (Arg3, Zero, CDW1)
+ If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
+ {
+ CreateDWordField (Arg3, 0x04, CDW2)
+ CreateDWordField (Arg3, 0x08, CDW3)
+ Local0 = CDW3 /* \_SB_.PC30._OSC.CDW3 */
+ Local0 &= 0x1F
+ If ((Arg1 != One))
+ {
+ CDW1 |= 0x08
+ }
+
+ If ((CDW3 != Local0))
+ {
+ CDW1 |= 0x10
+ }
+
+ CDW3 = Local0
+ }
+ Else
+ {
+ CDW1 |= 0x04
+ }
+
+ Return (Arg3)
+ }
+
+ Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table
+ {
+ Local0 = Package (0x80){}
+ Local1 = Zero
+ While ((Local1 < 0x80))
+ {
+ Local2 = (Local1 >> 0x02)
+ Local3 = ((Local1 + Local2) & 0x03)
+ If ((Local3 == Zero))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKD,
+ Zero
+ }
+ }
+
+ If ((Local3 == One))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKA,
+ Zero
+ }
+ }
+
+ If ((Local3 == 0x02))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKB,
+ Zero
+ }
+ }
+
+ If ((Local3 == 0x03))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKC,
+ Zero
+ }
+ }
+
+ Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF)
+ Local4 [One] = (Local1 & 0x03)
+ Local0 [Local1] = Local4
+ Local1++
+ }
+
+ Return (Local0)
+ }
+
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
+ 0x0000, // Granularity
+ 0x0030, // Range Minimum
+ 0x0030, // Range Maximum
+ 0x0000, // Translation Offset
+ 0x0001, // Length
+ ,, )
+ })
+ }
+ }
+
+ Scope (\_SB)
+ {
+ Device (PC20)
+ {
+ Name (_UID, 0x20) // _UID: Unique ID
+ Name (_BBN, 0x20) // _BBN: BIOS Bus Number
+ Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID
+ Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID
+ Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities
+ {
+ CreateDWordField (Arg3, Zero, CDW1)
+ If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
+ {
+ CreateDWordField (Arg3, 0x04, CDW2)
+ CreateDWordField (Arg3, 0x08, CDW3)
+ Local0 = CDW3 /* \_SB_.PC20._OSC.CDW3 */
+ Local0 &= 0x1F
+ If ((Arg1 != One))
+ {
+ CDW1 |= 0x08
+ }
+
+ If ((CDW3 != Local0))
+ {
+ CDW1 |= 0x10
+ }
+
+ CDW3 = Local0
+ }
+ Else
+ {
+ CDW1 |= 0x04
+ }
+
+ Return (Arg3)
+ }
+
+ Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table
+ {
+ Local0 = Package (0x80){}
+ Local1 = Zero
+ While ((Local1 < 0x80))
+ {
+ Local2 = (Local1 >> 0x02)
+ Local3 = ((Local1 + Local2) & 0x03)
+ If ((Local3 == Zero))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKD,
+ Zero
+ }
+ }
+
+ If ((Local3 == One))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKA,
+ Zero
+ }
+ }
+
+ If ((Local3 == 0x02))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKB,
+ Zero
+ }
+ }
+
+ If ((Local3 == 0x03))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKC,
+ Zero
+ }
+ }
+
+ Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF)
+ Local4 [One] = (Local1 & 0x03)
+ Local0 [Local1] = Local4
+ Local1++
+ }
+
+ Return (Local0)
+ }
+
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
+ 0x0000, // Granularity
+ 0x0020, // Range Minimum
+ 0x0020, // Range Maximum
+ 0x0000, // Translation Offset
+ 0x0001, // Length
+ ,, )
+ })
+ }
+ }
+
+ Scope (\_SB)
+ {
+ Device (PC10)
+ {
+ Name (_UID, 0x10) // _UID: Unique ID
+ Name (_BBN, 0x10) // _BBN: BIOS Bus Number
+ Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */) // _HID: Hardware ID
+ Name (_CID, EisaId ("PNP0A03") /* PCI Bus */) // _CID: Compatible ID
+ Method (_OSC, 4, NotSerialized) // _OSC: Operating System Capabilities
+ {
+ CreateDWordField (Arg3, Zero, CDW1)
+ If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") /* PCI Host Bridge Device */))
+ {
+ CreateDWordField (Arg3, 0x04, CDW2)
+ CreateDWordField (Arg3, 0x08, CDW3)
+ Local0 = CDW3 /* \_SB_.PC10._OSC.CDW3 */
+ Local0 &= 0x1F
+ If ((Arg1 != One))
+ {
+ CDW1 |= 0x08
+ }
+
+ If ((CDW3 != Local0))
+ {
+ CDW1 |= 0x10
+ }
+
+ CDW3 = Local0
+ }
+ Else
+ {
+ CDW1 |= 0x04
+ }
+
+ Return (Arg3)
+ }
+
+ Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table
+ {
+ Local0 = Package (0x80){}
+ Local1 = Zero
+ While ((Local1 < 0x80))
+ {
+ Local2 = (Local1 >> 0x02)
+ Local3 = ((Local1 + Local2) & 0x03)
+ If ((Local3 == Zero))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKD,
+ Zero
+ }
+ }
+
+ If ((Local3 == One))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKA,
+ Zero
+ }
+ }
+
+ If ((Local3 == 0x02))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKB,
+ Zero
+ }
+ }
+
+ If ((Local3 == 0x03))
+ {
+ Local4 = Package (0x04)
+ {
+ Zero,
+ Zero,
+ LNKC,
+ Zero
+ }
+ }
+
+ Local4 [Zero] = ((Local2 << 0x10) | 0xFFFF)
+ Local4 [One] = (Local1 & 0x03)
+ Local0 [Local1] = Local4
+ Local1++
+ }
+
+ Return (Local0)
+ }
+
+ Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
+ {
+ WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
+ 0x0000, // Granularity
+ 0x0010, // Range Minimum
+ 0x0010, // Range Maximum
+ 0x0000, // Translation Offset
+ 0x0001, // Length
+ ,, )
+ })
+ }
+ }
+
Scope (\_SB.PCI0)
{
Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings
{
WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
0x0000, // Granularity
0x0000, // Range Minimum
- 0x00FF, // Range Maximum
+ 0x000F, // Range Maximum
0x0000, // Translation Offset
- 0x0100, // Length
+ 0x0010, // Length
,, )
[...]
+ Device (S10)
+ {
+ Name (_ADR, 0x00020000) // _ADR: Address
+ }
+
+ Device (S18)
+ {
+ Name (_ADR, 0x00030000) // _ADR: Address
+ }
+
+ Device (S20)
+ {
+ Name (_ADR, 0x00040000) // _ADR: Address
+ }
+
+ Device (S28)
+ {
+ Name (_ADR, 0x00050000) // _ADR: Address
+ }
+
+ Device (S30)
+ {
+ Name (_ADR, 0x00060000) // _ADR: Address
+ }
+
Method (PCNT, 0, NotSerialized)
{
}
}
}
}
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
tests/data/acpi/q35/DSDT.viot | Bin 0 -> 9415 bytes
2 files changed, 1 deletion(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index fa213e4738..27ab8d3ba8 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1,2 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/DSDT.viot",
"tests/data/acpi/q35/VIOT.viot",
diff --git a/tests/data/acpi/q35/DSDT.viot b/tests/data/acpi/q35/DSDT.viot
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..8308c1e4ea7751a7389fcf7b91eda15168bf1460 100644
GIT binary patch
literal 9415
zcmeHNO>7&-8J*>ymdlm2q$taOi3lg@UxI{^^4As(Lhe!|Es>%~Do#KHTuMqSJ1MeA
zVjzxTKvsan@u6tKI_QxO6ri{E)N2DYKvAFv9||-;Pd)UKYf;3fsPCKEk!DB=h)%sk
z0(SSCeQ&;bvpf5Sv+K3I_Fo=l%y^=>=2lv{;>!&;hd+ZcM#r>ws<F$gv+EVhT_Y8V
z8J$}xevD1g%s$~2H_FCucl=L$|LqTaqZhEvPo2A)z2Lpu{w5>Pwavh?=)fsWbz8-5
zXV0}7ZZ&UqD^<@f+_y}#>x!eO*)4cYW_CBK9?Pyybk|yL?9OY<^)4+8>hb<gdd6}(
zwSO*sJ%9eiPcFS(`21)8`1zgJtQY``*qhid1+?*R_}08Xa6ax|F>Vr_pIaEreB9@!
zMVUiq0-s9m*`kQ5TG)TBv-iB$EV#UllZ|>K@8%cKF1OiA&s*m|m}AVQ&Y{0Q92^|{
zNgOigI&*5KP+2l7jZ#~xG@y-Cb#^O_c8%GL-Tx-VVx3zK#WOFJ>RztV^t!q1v)xv^
zzyd1q11jph>syXLus`bitna4|4))n#>Z5*-{ibKLfBiSU&-Sfp(YCZT^?s7ta{n6^
zV+^hN-jmcC>hI3p*=noM<z0;Q^Jq$=+FXxVzGXWr(@#YukWihyr&o#|z~UN3(R#E>
zra5iRmJOfz%rMHvHrvMjLIl}5dym%EHV#?SGaAdZ;uolgI(v(&jrM}9J0`BTp<x|s
zvTVE)eJ5&~7j}8gi|$(OkP`<Qy*Y;SAo?K6%nJ_ZJl1TxhpkD4*N;~nuz(rn1!ka&
z>gl6C+HJV4wvqb6woF_a3S9KvMh+V<*V%2AE@Bk?&9-piQWUtweCZ-i6zjF7k@~x!
zQQ#Kq8?{`c>b;!n7{yh0g=HUWxTTAc&Lb2Z90WUr2(BAf7}e4FJ{9k??BfM+#-XUL
zfHr=J^|M~0;k9a0gVk_o+<)$`|870=ZtbJFuWD!h?A8LR>-AsygGc&T+2Bn7iWR{K
zxXJG$8@KHN+fPT0TeM;K<_5F<tJXx2iAUHZ>jnPp9%K7Gvr%=IS=R1iZv;COIlp?v
zpqs^Uzv2VA(QG#|D{t*)QZd|}Y_+~pV&d4q@iNQURBlBOlw}Y_P+^(L7}|l6ByeFw
zEC8p(InD!AhQvhu92?<GV8e)*5*lHg2`FbGBqmzJu~E*HEr>E=N@!Ft0p(1D#FSYu
zt^}6X6XQ$?jR~e~JVI4xT+<onObLw(rffVyRcAuencz$bP3SrisyZh$ofDeQ30)^b
zRi~-xG&P;3t`niE6Hg$er<0n_q^=X8s?*YRTAEHv*NITonbLHoG@U73Cqh*xo`uTw
zv^AZ!t`niEb5heesp*{5bs|)CPH8%)G@VnrPK2t?X-(&}rgK`?iBQ$)XgVEDr=#mc
zsOn5>I@6lYw5}7OsxzbM%xF3@x=w_u&KXVTjHYu&*NITonbmY=HJw>qCqh-{tfq5T
z(>bf_M5yXKrRhAS={%+DM5yY-OagbGd=AZNI_Gqq2vwb@HJzt5ou_r32vwbDII|Ew
z3D0n5F?`sa5lr_eB&HW)&uYxG8uP5qM5r>)Y0PsP^PJ8^s4~xM%<~%ayv{_ZG9S{I
z4{6MYbS6TT`7mcnmwA{o<um?a!ITefgv69ra9+zfujQQAb0QQuO&DlFG0<|Wiqzap
z4ICqJXrKs?Rs>N5GARsH;J76jj99XL29gTsp@AZl(qNzhD-BeEl7R~7p@AYqYovh+
zY#32a)j$RG&_EHQMH~wQ71%HeHF78!sDR2))d>R?)p8~bRDqI#B9yL328vMXgn<ey
zXTm@gC>f}L@|<L#2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!%b74x1xf}gpqxnticnrp
z7^uK<CJa=8l7R{+XOe*;lsaLc0?U~&Pz6c`DxjQ628vMXgn<eyXTm@gC>f}LawZul
zLa7r5DzKah16818paRO7WS|J8P8g`bawZH^fs%m=C})y^B9uB|paRR8Fi-_b1}dPO
zNd}5g>V$y`EN8+%6(|{~fN~}oC_<?d1}d<e2?JH2WS|1dnPi{{rA`>Az;Y%GRDqI#
z3Mglifg+STVW0xbnJ`cVN(L&RoJj_XQ0jz%3M^;BKouw%sDN@N87M-j69y`<oCyO}
zpk$x|%9&)K2&GOKsK9b23{-)VfeI*Rl7S+WI$@v!%b74x1xf}gpqxnticso=feI{V
z!ax-$8K{7ACK)I~sS^e&u$&14RiI>`0?L_Wpa`W-7$_peKoO}1icmFBgo=SGOc<!b
zBm-5LWS|NY2C6V&pbC==RAG{VDohxt!i0e;Ofpb~Nd~GgVW0{V2C6X0Kouq#sKSJS
zB9d1Z28sw*jCUp(C_-EY84^?WEesTq>{}QpBH6cOpa^B(l7S+`QqkEiER+rGKL4@0
zPd`ZSC1_XZ-1^B=QTmlis~WW4VKkcS1vu_{kv1-EJd<JVWxYhl%d{!erebDis_R${
z7$k34yE|B9N$y*z?9<hCx0wW>nNLqysSgt@$yb7|vpZB~#j&jhH+HOPMn`Bw6H70v
z6PP0u<dTPt<r-QHGSTc-`jj>E;S@_o;cg7iZ-yr^Rxh&Cd>v??nt;|6vgxi@ZLynl
zZ0+)rX<CnBK)6jF&vjePmT_k%Yy+jI@-TaGC_5HUzIFDZlD){Y7tua7doepcd$F@u
zBkf`-4a<F@b}Mp%^sbWL<>}p_^zJd~-7vj3B;LY;*0)ygDd|0)-Wy8q9h2S*)0c+R
z$4-#Gq@*wL^rfNnrDM{U!t~3->EkCzzpSKR=INJ*(k~yAemP8E9!{S)LHe?izRc5?
zhtijiNnZ}rSBBG1oFIKgNnhdVD?{lk$E2?yeT*Ju!|5H7UOqf9t-x>>>$iGYHo=EH
zb~LWT>V}80mDa1nTw|m<Q8u=^k#U*PtHT6jq&iVHwz`pVh9zJcXHpnuB_q{|va!{T
zjGL@p9i}QH)rqpP)s2jUS-m>UUq-4EWn-%wxvx*@)nQUIQk^IpTiwWgI;U5Mna)Uc
zqD-&OId`<4y*J&e74fPCuT|_Q*8H8d7sT6%igo#D#lO)kwd7rDVQ|8mCf&wHuJXp)
z!Ryz4k$dyix8At+=GLokv5w(3R#(Eib<(oR#+UY&wqavcAD%=@=U>{E-MRGx3JNUe
zHcEBFt=3zH+pN80U;#W&n`MJVUAN|8kv&7msM{*7<^cB!?HtyctM#fGFe=1dzjT@F
zL4!miCkt+)n$IufsTtzMQ9PS=+sm&+23CKEz3=Ap`77B<0;~6hWap8~PsQY`4H8NM
zX!(7(7|8<W56<?7PmqMbn#QvG$eGoZeLq`^ldO{^GKQ-{4P)!iE}wE+ZrwaahUwNz
zmokxqfNcfF_NQK^zik9;J23o!^*&v-f8F<uyIUyogRS9|T<~xqF3Mo;?r<qC%hqRu
zWWQ$}AkAmNxbuTYM|zH~d$4tz+Sp^)dT7<X=LXM=!1$ek%eQ4kgOQHGN${?1(Tl}W
zOuSj73OCsnU2<sK>MV>GH&Pecb8T~cH`kKxTB~gz)6L`WmXZ3=dBK%2eG^!b;ZlNL
z9WF#Sk8NG&z31tk#`wn{h6nCzbD!>RjDT99olaRM!)gP~L$gzOY1&Sa7`AieYW5rk
zPq|U*mhy`%9Pu=e=$>-cVswfR{BTU=pJmw}1G~VeBM#pPiZ@nDLBp-{mTCy+;_PMd
za4F0-+$B79@EozQD;Ddt8-3mTe|^Q9eK84ZYO^2{+2y09K5@5a?FR4mtO(l-ZoJvs
z#P18XHE{Y{042qHACGNQfpJ(+KtZ~{Wkn84ieX9gu*AiPym@`oau~fIyS>d=aHEI1
zZ*Lnc*uL%4;Mn)Y6kyiM+@53+EUK_P{^_dX9i`T_i(%L1MAu^XUDuwlYvDCxyLKV$
zT1Rv(cHedF3A@%|G^6ErZEaRqH-AgaKc2Xlvi>M<Z^p0|2XNVVU_Y=86XgabDfs&u
zcmD(XFrLJqVeCKrs|$XfekUIs(C=jWi!kkb3={9Qe=p9k-)Uz)$A=o<h%=wRaOZJ+
z!x7dd%INV)FU9dB)&3}^uNK@Go|d>ja7XY@LJD|Vin$V%E0ZMgB|}P#XFf^FC2|Ra
zfBJAS)~Ga^<0?TXjhS$b*7b$`g>9V1GvSAqk-oLm%_L$5+K%I=yBRZPMezKh=ovag
zN}Qh_U53uylgpP}F%h0zFV!~SD~p82*Bxq$owyUHX`SO*{GslwB<U)4_EN2UUVKD?
zF8NU_|GWrr#SDhod27LA_=*7si}GNJ4my~LE`&s2BckAl9OVUzM~IjxI6}mE!O{^T
K!LkW5BKALx71wzH
literal 0
HcmV?d00001
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v3 10/10] tests/acpi: add expected VIOT blob for q35 machine
2021-09-14 14:19 [PATCH v3 00/10] virtio-iommu: Add ACPI support Jean-Philippe Brucker
` (8 preceding siblings ...)
2021-09-14 14:20 ` [PATCH v3 09/10] tests/acpi: add expected DSDT blob for VIOT test on q35 Jean-Philippe Brucker
@ 2021-09-14 14:20 ` Jean-Philippe Brucker
2021-09-20 8:29 ` Igor Mammedov
9 siblings, 1 reply; 22+ messages in thread
From: Jean-Philippe Brucker @ 2021-09-14 14:20 UTC (permalink / raw)
To: eric.auger
Cc: peter.maydell, ehabkost, mst, richard.henderson, qemu-devel,
shannon.zhaosl, Jean-Philippe Brucker, qemu-arm, pbonzini, ani,
imammedo
The VIOT table generated for the q35 test is:
[000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table]
[004h 0004 4] Table Length : 00000070
[008h 0008 1] Revision : 00
[009h 0009 1] Checksum : 3D
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPC "
[018h 0024 4] Oem Revision : 00000001
[01Ch 0028 4] Asl Compiler ID : "BXPC"
[020h 0032 4] Asl Compiler Revision : 00000001
[024h 0036 2] Node count : 0003
[026h 0038 2] Node offset : 0030
[028h 0040 8] Reserved : 0000000000000000
[030h 0048 1] Type : 03 [VirtIO-PCI IOMMU]
[031h 0049 1] Reserved : 00
[032h 0050 2] Length : 0010
[034h 0052 2] PCI Segment : 0000
[036h 0054 2] PCI BDF number : 0010
[038h 0056 8] Reserved : 0000000000000000
[040h 0064 1] Type : 01 [PCI Range]
[041h 0065 1] Reserved : 00
[042h 0066 2] Length : 0018
[044h 0068 4] Endpoint start : 00001000
[048h 0072 2] PCI Segment start : 0000
[04Ah 0074 2] PCI Segment end : 0000
[04Ch 0076 2] PCI BDF start : 1000
[04Eh 0078 2] PCI BDF end : 10FF
[050h 0080 2] Output node : 0030
[052h 0082 6] Reserved : 000000000000
[058h 0088 1] Type : 01 [PCI Range]
[059h 0089 1] Reserved : 00
[05Ah 0090 2] Length : 0018
[05Ch 0092 4] Endpoint start : 00003000
[060h 0096 2] PCI Segment start : 0000
[062h 0098 2] PCI Segment end : 0000
[064h 0100 2] PCI BDF start : 3000
[066h 0102 2] PCI BDF end : 30FF
[068h 0104 2] Output node : 0030
[06Ah 0106 6] Reserved : 000000000000
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
tests/qtest/bios-tables-test-allowed-diff.h | 1 -
tests/data/acpi/q35/VIOT.viot | Bin 0 -> 112 bytes
2 files changed, 1 deletion(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 27ab8d3ba8..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,2 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/VIOT.viot",
diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..275c78fbe8e93190321d957c91c3f17551f865d4 100644
GIT binary patch
literal 112
zcmWIZ^baXu00LVle`k+i1*eDrX9XZ&1PX!JAex!M0Hgv8m>C3sGzdcgBZCBjEAU?c
OrV=a;;~4xmfH48g0SW;C
literal 0
HcmV?d00001
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v3 10/10] tests/acpi: add expected VIOT blob for q35 machine
2021-09-14 14:20 ` [PATCH v3 10/10] tests/acpi: add expected VIOT blob for q35 machine Jean-Philippe Brucker
@ 2021-09-20 8:29 ` Igor Mammedov
0 siblings, 0 replies; 22+ messages in thread
From: Igor Mammedov @ 2021-09-20 8:29 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: peter.maydell, ehabkost, shannon.zhaosl, mst, richard.henderson,
qemu-devel, eric.auger, qemu-arm, ani, pbonzini
On Tue, 14 Sep 2021 15:20:05 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> The VIOT table generated for the q35 test is:
I'd squash this into previous patch
> [000h 0000 4] Signature : "VIOT" [Virtual I/O Translation Table]
> [004h 0004 4] Table Length : 00000070
> [008h 0008 1] Revision : 00
> [009h 0009 1] Checksum : 3D
> [00Ah 0010 6] Oem ID : "BOCHS "
> [010h 0016 8] Oem Table ID : "BXPC "
> [018h 0024 4] Oem Revision : 00000001
> [01Ch 0028 4] Asl Compiler ID : "BXPC"
> [020h 0032 4] Asl Compiler Revision : 00000001
>
> [024h 0036 2] Node count : 0003
> [026h 0038 2] Node offset : 0030
> [028h 0040 8] Reserved : 0000000000000000
>
> [030h 0048 1] Type : 03 [VirtIO-PCI IOMMU]
> [031h 0049 1] Reserved : 00
> [032h 0050 2] Length : 0010
>
> [034h 0052 2] PCI Segment : 0000
> [036h 0054 2] PCI BDF number : 0010
> [038h 0056 8] Reserved : 0000000000000000
>
> [040h 0064 1] Type : 01 [PCI Range]
> [041h 0065 1] Reserved : 00
> [042h 0066 2] Length : 0018
>
> [044h 0068 4] Endpoint start : 00001000
> [048h 0072 2] PCI Segment start : 0000
> [04Ah 0074 2] PCI Segment end : 0000
> [04Ch 0076 2] PCI BDF start : 1000
> [04Eh 0078 2] PCI BDF end : 10FF
> [050h 0080 2] Output node : 0030
> [052h 0082 6] Reserved : 000000000000
>
> [058h 0088 1] Type : 01 [PCI Range]
> [059h 0089 1] Reserved : 00
> [05Ah 0090 2] Length : 0018
>
> [05Ch 0092 4] Endpoint start : 00003000
> [060h 0096 2] PCI Segment start : 0000
> [062h 0098 2] PCI Segment end : 0000
> [064h 0100 2] PCI BDF start : 3000
> [066h 0102 2] PCI BDF end : 30FF
> [068h 0104 2] Output node : 0030
> [06Ah 0106 6] Reserved : 000000000000
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 1 -
> tests/data/acpi/q35/VIOT.viot | Bin 0 -> 112 bytes
> 2 files changed, 1 deletion(-)
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index 27ab8d3ba8..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,2 +1 @@
> /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/q35/VIOT.viot",
> diff --git a/tests/data/acpi/q35/VIOT.viot b/tests/data/acpi/q35/VIOT.viot
> index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..275c78fbe8e93190321d957c91c3f17551f865d4 100644
> GIT binary patch
> literal 112
> zcmWIZ^baXu00LVle`k+i1*eDrX9XZ&1PX!JAex!M0Hgv8m>C3sGzdcgBZCBjEAU?c
> OrV=a;;~4xmfH48g0SW;C
>
> literal 0
> HcmV?d00001
>
^ permalink raw reply [flat|nested] 22+ messages in thread