On Mon, 2 May 2016 11:27:35 +0300 David Kiarie <davidkiar...@gmail.com> wrote:
> On Sun, May 1, 2016 at 4:45 PM, Michael S. Tsirkin <m...@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 <davidkiar...@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.