| Message ID | 1461969763-5193-3-git-send-email-davidkiarie4@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Apr 30, 2016 at 01:42:41AM +0300, David Kiarie wrote: > 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 | 93 +++++++++++++++++++++++++++++++++++++++------ > include/hw/acpi/acpi-defs.h | 14 +++++++ > include/hw/acpi/acpi.h | 16 ++++++++ > include/hw/acpi/aml-build.h | 1 + > include/hw/boards.h | 6 +++ > 7 files changed, 120 insertions(+), 25 deletions(-) > > 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..74ae994 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" > @@ -59,6 +60,8 @@ > #include "qapi/qmp/qint.h" > #include "qom/qom-qobject.h" > > +#include "hw/boards.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 > * a little bit, there should be plenty of free space since the DSDT > @@ -2598,6 +2601,77 @@ build_dmar_q35(GArray *table_data, GArray *linker) > "DMAR", table_data->len - dmar_start, 1, NULL, NULL); > } > > +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)); why won't build_header overwrite the extra fields? Even if not, that's too hacky. Pls find another way to do this. > + /* 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, 0x20, 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, 0x00000003, 4); > + /* end of device range */ > + build_append_int_noprefix(table_data, 0x00ffff04, 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 +2730,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 +2742,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 +2808,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 Why 39? > + > +/* 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) > + Why not open-code this? These have no comments so aren't really helpful for understanding the code anyway. > #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, > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 8d4fe56..dbe6745 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -61,6 +61,12 @@ typedef struct { > CPUArchId cpus[0]; > } CPUArchIdList; > > +typedef enum IommuType { > + TYPE_AMD, > + TYPE_INTEL, > + TYPE_NONE > +} IommuType; > + > /** > * MachineClass: > * @get_hotplug_handler: this function is called during bus-less > -- > 2.1.4
On Sun, May 1, 2016 at 4:45 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Sat, Apr 30, 2016 at 01:42:41AM +0300, David Kiarie wrote: >> 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 | 93 +++++++++++++++++++++++++++++++++++++++------ >> include/hw/acpi/acpi-defs.h | 14 +++++++ >> include/hw/acpi/acpi.h | 16 ++++++++ >> include/hw/acpi/aml-build.h | 1 + >> include/hw/boards.h | 6 +++ >> 7 files changed, 120 insertions(+), 25 deletions(-) >> >> 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..74ae994 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" >> @@ -59,6 +60,8 @@ >> #include "qapi/qmp/qint.h" >> #include "qom/qom-qobject.h" >> >> +#include "hw/boards.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 >> * a little bit, there should be plenty of free space since the DSDT >> @@ -2598,6 +2601,77 @@ build_dmar_q35(GArray *table_data, GArray *linker) >> "DMAR", table_data->len - dmar_start, 1, NULL, NULL); >> } >> >> +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)); > > why won't build_header overwrite the extra fields? > Even if not, that's too hacky. Pls find another way to do this. I was advised to do this by Igor, to get rid of structs defining IVRS table. He actually said it should be done acpi_data_push(table_data, (sizeof(acpi_table_header) - 2)); but looking at acpi_table_header, I noted there's an extra 2-byte field for internal use which corrupts IVRS. The other way I could do this, since I know the size of IVRS table, is to use a constant acpi_data_push(table_data, 0x30)); am not sure whether this isn't even more hacky otherwise I'd have to go back to using the original structs. > >> + /* 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, 0x20, 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, 0x00000003, 4); >> + /* end of device range */ >> + build_append_int_noprefix(table_data, 0x00ffff04, 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 +2730,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 +2742,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 +2808,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 > > Why 39? Well, good question. I actually did ask about what values should go into HATS, GATS and the address width and no-one seemed very sure about that. This the width on my machine, at least that is what 'cpuid' tells me but I am not on an AMD machine. I just checked this and it seems like 39 is not one of the allowed value the allowed values being 40, 48 and 52. > >> + >> +/* 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) >> + > > Why not open-code this? These have no comments so aren't > really helpful for understanding the code anyway. > > >> #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, >> diff --git a/include/hw/boards.h b/include/hw/boards.h >> index 8d4fe56..dbe6745 100644 >> --- a/include/hw/boards.h >> +++ b/include/hw/boards.h >> @@ -61,6 +61,12 @@ typedef struct { >> CPUArchId cpus[0]; >> } CPUArchIdList; >> >> +typedef enum IommuType { >> + TYPE_AMD, >> + TYPE_INTEL, >> + TYPE_NONE >> +} IommuType; >> + >> /** >> * MachineClass: >> * @get_hotplug_handler: this function is called during bus-less >> -- >> 2.1.4
On Mon, 2 May 2016 11:27:35 +0300 David Kiarie <davidkiarie4@gmail.com> wrote: > On Sun, May 1, 2016 at 4:45 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Sat, Apr 30, 2016 at 01:42:41AM +0300, David Kiarie wrote: > >> 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 | 93 +++++++++++++++++++++++++++++++++++++++------ > >> include/hw/acpi/acpi-defs.h | 14 +++++++ > >> include/hw/acpi/acpi.h | 16 ++++++++ > >> include/hw/acpi/aml-build.h | 1 + > >> include/hw/boards.h | 6 +++ > >> 7 files changed, 120 insertions(+), 25 deletions(-) > >> > >> 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; it's internal struct used for loading external tables leave it here and don't use, there is another public one for header. Looks like acpi/core.c needs some cleanup to avoid confusion. > >> > >> #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..74ae994 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" > >> @@ -59,6 +60,8 @@ > >> #include "qapi/qmp/qint.h" > >> #include "qom/qom-qobject.h" > >> > >> +#include "hw/boards.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 > >> * a little bit, there should be plenty of free space since the DSDT > >> @@ -2598,6 +2601,77 @@ build_dmar_q35(GArray *table_data, GArray *linker) > >> "DMAR", table_data->len - dmar_start, 1, NULL, NULL); > >> } > >> > >> +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)); > > > > why won't build_header overwrite the extra fields? > > Even if not, that's too hacky. Pls find another way to do this. > > I was advised to do this by Igor, to get rid of structs defining IVRS > table. He actually said it should be done > acpi_data_push(table_data, (sizeof(acpi_table_header) - 2)); > but looking at acpi_table_header, I noted there's an extra 2-byte > field for internal use which corrupts IVRS. IVRS table has standard ACPI header so just do as other tables do: for example: acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader)); > The other way I could do this, since I know the size of IVRS table, is > to use a constant > acpi_data_push(table_data, 0x30)); > am not sure whether this isn't even more hacky otherwise I'd have to > go back to using the original structs.
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..74ae994 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" @@ -59,6 +60,8 @@ #include "qapi/qmp/qint.h" #include "qom/qom-qobject.h" +#include "hw/boards.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 * a little bit, there should be plenty of free space since the DSDT @@ -2598,6 +2601,77 @@ build_dmar_q35(GArray *table_data, GArray *linker) "DMAR", table_data->len - dmar_start, 1, NULL, NULL); } +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, 0x20, 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, 0x00000003, 4); + /* end of device range */ + build_append_int_noprefix(table_data, 0x00ffff04, 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 +2730,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 +2742,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 +2808,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, diff --git a/include/hw/boards.h b/include/hw/boards.h index 8d4fe56..dbe6745 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -61,6 +61,12 @@ typedef struct { CPUArchId cpus[0]; } CPUArchIdList; +typedef enum IommuType { + TYPE_AMD, + TYPE_INTEL, + TYPE_NONE +} IommuType; + /** * MachineClass: * @get_hotplug_handler: this function is called during bus-less
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 | 93 +++++++++++++++++++++++++++++++++++++++------ include/hw/acpi/acpi-defs.h | 14 +++++++ include/hw/acpi/acpi.h | 16 ++++++++ include/hw/acpi/aml-build.h | 1 + include/hw/boards.h | 6 +++ 7 files changed, 120 insertions(+), 25 deletions(-)