diff mbox

[V9,2/4] hw/i386: ACPI table for AMD IOMMU

Message ID 1461535977-331-3-git-send-email-davidkiarie4@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Kiarie April 24, 2016, 10:12 p.m. UTC
Add IVRS table for AMD IOMMU. Generate IVRS or DMAR
depending on emulated IOMMU

Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
---
 hw/acpi/aml-build.c         |   2 +-
 hw/acpi/core.c              |  13 ------
 hw/i386/acpi-build.c        | 101 +++++++++++++++++++++++++++++++++++++++-----
 include/hw/acpi/acpi-defs.h |  14 ++++++
 include/hw/acpi/acpi.h      |  16 +++++++
 include/hw/acpi/aml-build.h |   1 +
 6 files changed, 122 insertions(+), 25 deletions(-)

Comments

Jan Kiszka April 29, 2016, 6:09 a.m. UTC | #1
On 2016-04-25 00:12, David Kiarie wrote:
> Add IVRS table for AMD IOMMU. Generate IVRS or DMAR
> depending on emulated IOMMU

It seems you lack scope descriptions for the PCI devices in the system.
At least, this is what our jailhouse config generator complains about
right now (didn't look into details yet). If so, the guest OS will
likely not configure the IOMMU appropriately as it thinks that the
devices are passed through anyway.

On Intel, there is an easy way to state "catch them all" in the ACPI
table. Maybe AMD has this as well so that you don't need to add
individual devices.

Jan
David Kiarie April 29, 2016, 8:28 a.m. UTC | #2
On Fri, Apr 29, 2016 at 9:09 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2016-04-25 00:12, David Kiarie wrote:
>> Add IVRS table for AMD IOMMU. Generate IVRS or DMAR
>> depending on emulated IOMMU
>
> It seems you lack scope descriptions for the PCI devices in the system.
> At least, this is what our jailhouse config generator complains about
> right now (didn't look into details yet). If so, the guest OS will
> likely not configure the IOMMU appropriately as it thinks that the
> devices are passed through anyway.
>
> On Intel, there is an easy way to state "catch them all" in the ACPI
> table. Maybe AMD has this as well so that you don't need to add
> individual devices.

This is what am currently doing. From what I can see, linux correctly
detects the range but I will have a look at it again before sending
the next version.

There was another issue raised by Michael about denying VFIO in case a
user requests for IOMMU support. I did look at this but since VFIO is
started with '-device' I didn't find a way to deny it in case of IOMMU
support except by parsing command line arguments which turned out to
be a bit ugly.

We could exclude VFIO from translation by IOMMU by excluding it from
the scope descriptions but am not quite sure this will remedy all the
problems relating to VFIO and IOMMU. This will also require the device
scope to explicitly list all the devices.

>
> Jan
Valentine Sinitsyn May 4, 2016, 7:45 a.m. UTC | #3
On 29.04.2016 11:09, Jan Kiszka wrote:
> On 2016-04-25 00:12, David Kiarie wrote:
>> Add IVRS table for AMD IOMMU. Generate IVRS or DMAR
>> depending on emulated IOMMU
>
> It seems you lack scope descriptions for the PCI devices in the system.
> At least, this is what our jailhouse config generator complains about
> right now (didn't look into details yet). If so, the guest OS will
> likely not configure the IOMMU appropriately as it thinks that the
> devices are passed through anyway.
>
> On Intel, there is an easy way to state "catch them all" in the ACPI
> table. Maybe AMD has this as well so that you don't need to add
> individual devices.
There is. IVHD Device Entry Type 1 does just that.

Valentine
Valentine Sinitsyn May 4, 2016, 7:50 a.m. UTC | #4
On 29.04.2016 13:28, David Kiarie wrote:
> On Fri, Apr 29, 2016 at 9:09 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2016-04-25 00:12, David Kiarie wrote:
>>> Add IVRS table for AMD IOMMU. Generate IVRS or DMAR
>>> depending on emulated IOMMU
>>
>> It seems you lack scope descriptions for the PCI devices in the system.
>> At least, this is what our jailhouse config generator complains about
>> right now (didn't look into details yet). If so, the guest OS will
>> likely not configure the IOMMU appropriately as it thinks that the
>> devices are passed through anyway.
>>
>> On Intel, there is an easy way to state "catch them all" in the ACPI
>> table. Maybe AMD has this as well so that you don't need to add
>> individual devices.
>
> This is what am currently doing. From what I can see, linux correctly
> detects the range but I will have a look at it again before sending
> the next version.
>
> There was another issue raised by Michael about denying VFIO in case a
> user requests for IOMMU support. I did look at this but since VFIO is
> started with '-device' I didn't find a way to deny it in case of IOMMU
> support except by parsing command line arguments which turned out to
> be a bit ugly.
>
> We could exclude VFIO from translation by IOMMU by excluding it from
> the scope descriptions but am not quite sure this will remedy all the
> problems relating to VFIO and IOMMU. This will also require the device
> scope to explicitly list all the devices.
You may also try listing everything "up to" VFIO, then everything 
"after" the VFIO, I guess.

Valentine
diff mbox

Patch

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index ab89ca6..da11bf8 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -227,7 +227,7 @@  static void build_extop_package(GArray *package, uint8_t op)
     build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
 }
 
-static void build_append_int_noprefix(GArray *table, uint64_t value, int size)
+void build_append_int_noprefix(GArray *table, uint64_t value, int size)
 {
     int i;
 
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 7925a1a..ee0e687 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -29,19 +29,6 @@ 
 #include "qapi-visit.h"
 #include "qapi-event.h"
 
-struct acpi_table_header {
-    uint16_t _length;         /* our length, not actual part of the hdr */
-                              /* allows easier parsing for fw_cfg clients */
-    char sig[4];              /* ACPI signature (4 ASCII characters) */
-    uint32_t length;          /* Length of table, in bytes, including header */
-    uint8_t revision;         /* ACPI Specification minor version # */
-    uint8_t checksum;         /* To make sum of entire table == 0 */
-    char oem_id[6];           /* OEM identification */
-    char oem_table_id[8];     /* OEM table identification */
-    uint32_t oem_revision;    /* OEM revision number */
-    char asl_compiler_id[4];  /* ASL compiler vendor ID */
-    uint32_t asl_compiler_revision; /* ASL compiler revision number */
-} QEMU_PACKED;
 
 #define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
 #define ACPI_TABLE_PFX_SIZE sizeof(uint16_t)  /* size of the extra prefix */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 6477003..19efc68 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -52,6 +52,7 @@ 
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-host/q35.h"
 #include "hw/i386/intel_iommu.h"
+#include "hw/i386/amd_iommu.h"
 #include "hw/timer/hpet.h"
 
 #include "hw/acpi/aml-build.h"
@@ -118,6 +119,12 @@  typedef struct AcpiBuildPciBusHotplugState {
     bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
+typedef enum IommuType {
+    TYPE_AMD,
+    TYPE_INTEL,
+    TYPE_NONE
+} IommuType;
+
 static void acpi_get_pm_info(AcpiPmInfo *pm)
 {
     Object *piix = piix4_pm_find();
@@ -2598,6 +2605,81 @@  build_dmar_q35(GArray *table_data, GArray *linker)
                  "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
 }
 
+#define IOMMU_IVRS_LENGTH 0x30
+/* increase ivdb length if more devices are added */
+#define IOMMU_IVDB_LENGTH 0x20
+
+static void
+build_amd_iommu(GArray *table_data, GArray *linker)
+{
+    int iommu_start = table_data->len;
+    bool iommu_ambig;
+
+    /* IVRS definition  - table header has an extra 2-byte field */
+    acpi_data_push(table_data, (sizeof(acpi_table_header) - 2));
+    /* common virtualization information */
+    build_append_int_noprefix(table_data, AMD_IOMMU_HOST_ADDRESS_WIDTH << 8, 4);
+    /* reserved */
+    build_append_int_noprefix(table_data, 0, 8);
+
+    AMDIOMMUState *s = (AMDIOMMUState *)object_resolve_path_type("",
+                        TYPE_AMD_IOMMU_DEVICE, &iommu_ambig);
+
+    /* IVDB definition - type 10h */
+    if (!iommu_ambig) {
+        /* IVHD definition - type 10h */
+        build_append_int_noprefix(table_data, 0x10, 1);
+        /* virtualization flags */
+        build_append_int_noprefix(table_data, (IVHD_HT_TUNEN |
+                     IVHD_PPRSUP | IVHD_IOTLBSUP | IVHD_PREFSUP), 1);
+        /* ivhd length */
+        build_append_int_noprefix(table_data, IOMMU_IVDB_LENGTH, 2);
+        /* iommu device id */
+        build_append_int_noprefix(table_data, PCI_DEVICE_ID_RD890_IOMMU, 2);
+        /* offset of capability registers */
+        build_append_int_noprefix(table_data, s->capab_offset, 2);
+        /* mmio base register */
+        build_append_int_noprefix(table_data, s->mmio.addr, 8);
+        /* pci segment */
+        build_append_int_noprefix(table_data, 0, 2);
+        /* interrupt numbers */
+        build_append_int_noprefix(table_data, 0, 2);
+        /* feature reporting */
+        build_append_int_noprefix(table_data, (IVHD_EFR_GTSUP |
+                    IVHD_EFR_HATS | IVHD_EFR_GATS), 4);
+        /* Add device flags here
+         *   This is are 4-byte device entries currently reporting the range of
+         *   devices 00h - ffffh; all devices
+         *   Device setting affecting all devices should be made here
+         *
+         *   Refer to
+         *   (http://developer.amd.com/wordpress/media/2012/10/488821.pdf)
+         *   Table 95
+         */
+        /* start of device range, 4-byte entries */
+        build_append_int_noprefix(table_data, 0x03000000, 4);
+        /* end of device range */
+        build_append_int_noprefix(table_data, 0x04ffff00, 4);
+    }
+
+    build_header(linker, table_data, (void *)(table_data->data + iommu_start),
+                 "IVRS", table_data->len - iommu_start, 1, NULL, NULL);
+}
+
+static IommuType has_iommu(void)
+{
+    bool ambiguous;
+
+    if (object_resolve_path_type("", TYPE_AMD_IOMMU_DEVICE, &ambiguous)
+            && !ambiguous)
+        return TYPE_AMD;
+    else if (object_resolve_path_type("", TYPE_INTEL_IOMMU_DEVICE, &ambiguous)
+            && !ambiguous)
+        return TYPE_INTEL;
+    else
+        return TYPE_NONE;
+}
+
 static GArray *
 build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
 {
@@ -2656,16 +2738,6 @@  static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
     return true;
 }
 
-static bool acpi_has_iommu(void)
-{
-    bool ambiguous;
-    Object *intel_iommu;
-
-    intel_iommu = object_resolve_path_type("", TYPE_INTEL_IOMMU_DEVICE,
-                                           &ambiguous);
-    return intel_iommu && !ambiguous;
-}
-
 static
 void acpi_build(AcpiBuildTables *tables, MachineState *machine)
 {
@@ -2678,6 +2750,7 @@  void acpi_build(AcpiBuildTables *tables, MachineState *machine)
     AcpiMcfgInfo mcfg;
     PcPciInfo pci;
     uint8_t *u;
+    IommuType IOMMUType = has_iommu();
     size_t aml_len = 0;
     GArray *tables_blob = tables->table_data;
     AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
@@ -2743,7 +2816,13 @@  void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         acpi_add_table(table_offsets, tables_blob);
         build_mcfg_q35(tables_blob, tables->linker, &mcfg);
     }
-    if (acpi_has_iommu()) {
+
+    if (IOMMUType == TYPE_AMD) {
+        acpi_add_table(table_offsets, tables_blob);
+        build_amd_iommu(tables_blob, tables->linker);
+    }
+
+    if (IOMMUType == TYPE_INTEL) {
         acpi_add_table(table_offsets, tables_blob);
         build_dmar_q35(tables_blob, tables->linker);
     }
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index c7a03d4..114227a 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -570,4 +570,18 @@  typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit;
 /* Masks for Flags field above */
 #define ACPI_DMAR_INCLUDE_PCI_ALL   1
 
+/* IVRS constants */
+#define AMD_IOMMU_HOST_ADDRESS_WIDTH 39UL
+
+/* flags in the IVHD headers */
+#define IVHD_HT_TUNEN    (1UL << 0)
+#define IVHD_IOTLBSUP    (1UL << 4)
+#define IVHD_PREFSUP     (1UL << 6)
+#define IVHD_PPRSUP      (1UL << 7)
+
+/* features in the IVHD headers */
+#define IVHD_EFR_HATS       48
+#define IVHD_EFR_GATS       48
+#define IVHD_EFR_GTSUP  (1UL << 2)
+
 #endif
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index e0978c8..a708b84 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -201,4 +201,20 @@  struct AcpiSlicOem {
 };
 int acpi_get_slic_oem(AcpiSlicOem *oem);
 
+struct acpi_table_header {
+    uint16_t _length;         /* our length, not actual part of the hdr */
+                              /* allows easier parsing for fw_cfg clients */
+    char sig[4];              /* ACPI signature (4 ASCII characters) */
+    uint32_t length;          /* Length of table, in bytes, including header */
+    uint8_t revision;         /* ACPI Specification minor version # */
+    uint8_t checksum;         /* To make sum of entire table == 0 */
+    char oem_id[6];           /* OEM identification */
+    char oem_table_id[8];     /* OEM table identification */
+    uint32_t oem_revision;    /* OEM revision number */
+    char asl_compiler_id[4];  /* ASL compiler vendor ID */
+    uint32_t asl_compiler_revision; /* ASL compiler revision number */
+} QEMU_PACKED;
+
+typedef struct acpi_table_header acpi_table_header;
+
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 2c994b3..d25e9a5 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -355,6 +355,7 @@  Aml *aml_derefof(Aml *arg);
 Aml *aml_sizeof(Aml *arg);
 Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
 
+void build_append_int_noprefix(GArray *table, uint64_t value, int size);
 void
 build_header(GArray *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,