Message ID | 1463912514-12658-3-git-send-email-davidkiarie4@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sun, May 22, 2016 at 01:21:52PM +0300, David Kiarie wrote: [...] > +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(AcpiTableHeader))); > + /* 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); > + > + AMDVIState *s = (AMDVIState *)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 > + * These 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 I failed to find Table 95 in the document. Is that typo? [...] > static > void acpi_build(AcpiBuildTables *tables, MachineState *machine) > { > @@ -2657,6 +2721,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 }; > @@ -2722,7 +2787,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); > } Nit: I'd prefer: if (type == Intel) { ... } else if (type == AMD) { ... } for better readability. Thanks, -- peterx
Hi all, On 24.05.2016 11:54, Peter Xu wrote: > On Sun, May 22, 2016 at 01:21:52PM +0300, David Kiarie wrote: > [...] >> +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(AcpiTableHeader))); >> + /* 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); >> + >> + AMDVIState *s = (AMDVIState *)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 >> + * These 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 > > I failed to find Table 95 in the document. Is that typo? I guess it should be "Table 75". David, am I right? On a side note, 2.0 specification you mention is rather outdated. Please consider referencing something newer, like 2.6. > > [...] > >> static >> void acpi_build(AcpiBuildTables *tables, MachineState *machine) >> { >> @@ -2657,6 +2721,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 }; >> @@ -2722,7 +2787,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); >> } > > Nit: I'd prefer: > > if (type == Intel) { > ... > } else if (type == AMD) { > ... > } > > for better readability. > > Thanks, > > -- peterx > Best, Valentine
On Tue, May 24, 2016 at 10:06 AM, Valentine Sinitsyn <valentine.sinitsyn@gmail.com> wrote: > Hi all, > > > On 24.05.2016 11:54, Peter Xu wrote: >> >> On Sun, May 22, 2016 at 01:21:52PM +0300, David Kiarie wrote: >> [...] >>> >>> +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(AcpiTableHeader))); >>> + /* 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); >>> + >>> + AMDVIState *s = (AMDVIState *)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 >>> + * These 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 >> >> >> I failed to find Table 95 in the document. Is that typo? > > I guess it should be "Table 75". David, am I right? > On a side note, 2.0 specification you mention is rather outdated. > Please consider referencing something newer, like 2.6. > > >> >> [...] >> >>> static >>> void acpi_build(AcpiBuildTables *tables, MachineState *machine) >>> { >>> @@ -2657,6 +2721,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 }; >>> @@ -2722,7 +2787,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); >>> } >> >> >> Nit: I'd prefer: >> >> if (type == Intel) { >> ... >> } else if (type == AMD) { >> ... >> } >> I missed this is the last version of the patch I should fix it in next version. On taking a closer look at this there might be larger problem where with the advent of -device <iommu-type> users can possibly emulate two IOMMUs at the same time ? A proposed solution was to have pci_setup_iommu check that DMA hook as not been setup yet and fail if yes. I should send a fix for that too. >> for better readability. >> >> Thanks, >> >> -- peterx >> > > Best, > Valentine
On Sat, Jun 18, 2016 at 11:18:29AM +0300, David Kiarie wrote: > On Tue, May 24, 2016 at 10:06 AM, Valentine Sinitsyn > <valentine.sinitsyn@gmail.com> wrote: > > Hi all, > > > > > > On 24.05.2016 11:54, Peter Xu wrote: > >> > >> On Sun, May 22, 2016 at 01:21:52PM +0300, David Kiarie wrote: > >> [...] > >>> > >>> +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(AcpiTableHeader))); > >>> + /* 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); > >>> + > >>> + AMDVIState *s = (AMDVIState *)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 > >>> + * These 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 > >> > >> > >> I failed to find Table 95 in the document. Is that typo? > > > > I guess it should be "Table 75". David, am I right? > > On a side note, 2.0 specification you mention is rather outdated. > > Please consider referencing something newer, like 2.6. > > > > > >> > >> [...] > >> > >>> static > >>> void acpi_build(AcpiBuildTables *tables, MachineState *machine) > >>> { > >>> @@ -2657,6 +2721,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 }; > >>> @@ -2722,7 +2787,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); > >>> } > >> > >> > >> Nit: I'd prefer: > >> > >> if (type == Intel) { > >> ... > >> } else if (type == AMD) { > >> ... > >> } > >> > > I missed this is the last version of the patch I should fix it in next version. > > On taking a closer look at this there might be larger problem where > with the advent of -device <iommu-type> users can possibly emulate two > IOMMUs at the same time ? A proposed solution was to have > pci_setup_iommu check that DMA hook as not been setup yet and fail if > yes. I should send a fix for that too. Currently we should only support single vIOMMU. If you are going to rebase to x86-iommu codes, there is a patch that includes the check: "[PATCH v9 02/25] x86-iommu: provide x86_iommu_get_default" by: assert(x86_iommu_default == NULL); Maybe we should print something more readable, like "multiple vIOMMUs are not supported yet", rather than an assertion fail. -- peterx
On 2016-06-18 14:32, Peter Xu wrote: > On Sat, Jun 18, 2016 at 11:18:29AM +0300, David Kiarie wrote: >> On Tue, May 24, 2016 at 10:06 AM, Valentine Sinitsyn >> <valentine.sinitsyn@gmail.com> wrote: >>> Hi all, >>> >>> >>> On 24.05.2016 11:54, Peter Xu wrote: >>>> >>>> On Sun, May 22, 2016 at 01:21:52PM +0300, David Kiarie wrote: >>>> [...] >>>>> >>>>> +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(AcpiTableHeader))); >>>>> + /* 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); >>>>> + >>>>> + AMDVIState *s = (AMDVIState *)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 >>>>> + * These 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 >>>> >>>> >>>> I failed to find Table 95 in the document. Is that typo? >>> >>> I guess it should be "Table 75". David, am I right? >>> On a side note, 2.0 specification you mention is rather outdated. >>> Please consider referencing something newer, like 2.6. >>> >>> >>>> >>>> [...] >>>> >>>>> static >>>>> void acpi_build(AcpiBuildTables *tables, MachineState *machine) >>>>> { >>>>> @@ -2657,6 +2721,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 }; >>>>> @@ -2722,7 +2787,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); >>>>> } >>>> >>>> >>>> Nit: I'd prefer: >>>> >>>> if (type == Intel) { >>>> ... >>>> } else if (type == AMD) { >>>> ... >>>> } >>>> >> >> I missed this is the last version of the patch I should fix it in next version. >> >> On taking a closer look at this there might be larger problem where >> with the advent of -device <iommu-type> users can possibly emulate two >> IOMMUs at the same time ? A proposed solution was to have >> pci_setup_iommu check that DMA hook as not been setup yet and fail if >> yes. I should send a fix for that too. > > Currently we should only support single vIOMMU. If you are going to > rebase to x86-iommu codes, there is a patch that includes the check: > > "[PATCH v9 02/25] x86-iommu: provide x86_iommu_get_default" > > by: > > assert(x86_iommu_default == NULL); > > Maybe we should print something more readable, like "multiple vIOMMUs > are not supported yet", rather than an assertion fail. You need proper error handling and a readable error message because nothing else will stop the user from doing -device intel-iommu -device amd-iommu. Jan
On Sat, Jun 18, 2016 at 02:34:26PM +0200, Jan Kiszka wrote: > On 2016-06-18 14:32, Peter Xu wrote: > > On Sat, Jun 18, 2016 at 11:18:29AM +0300, David Kiarie wrote: [...] > >> I missed this is the last version of the patch I should fix it in next version. > >> > >> On taking a closer look at this there might be larger problem where > >> with the advent of -device <iommu-type> users can possibly emulate two > >> IOMMUs at the same time ? A proposed solution was to have > >> pci_setup_iommu check that DMA hook as not been setup yet and fail if > >> yes. I should send a fix for that too. > > > > Currently we should only support single vIOMMU. If you are going to > > rebase to x86-iommu codes, there is a patch that includes the check: > > > > "[PATCH v9 02/25] x86-iommu: provide x86_iommu_get_default" > > > > by: > > > > assert(x86_iommu_default == NULL); > > > > Maybe we should print something more readable, like "multiple vIOMMUs > > are not supported yet", rather than an assertion fail. > > You need proper error handling and a readable error message because > nothing else will stop the user from doing -device intel-iommu -device > amd-iommu. Will do. Thanks. -- peterx
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index cedb74e..8d4bd01 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/i386/acpi-build.c b/hw/i386/acpi-build.c index 279f0d7..b0ee01b 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 @@ -2577,6 +2580,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(AcpiTableHeader))); + /* 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); + + AMDVIState *s = (AMDVIState *)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 + * These 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) { @@ -2635,16 +2709,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) { @@ -2657,6 +2721,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 }; @@ -2722,7 +2787,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 850a962..3de7f82 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -583,4 +583,17 @@ typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit; /* Masks for Flags field above */ #define ACPI_DMAR_INCLUDE_PCI_ALL 1 +/* IVRS constants */ +#define AMD_IOMMU_HOST_ADDRESS_WIDTH 40UL + +/* flags in the IVHD headers */ +#define IVHD_HT_TUNEN (1UL << 0) /* recommended setting for HtTunEn */ +#define IVHD_IOTLBSUP (1UL << 4) /* remote IOTLB support */ +#define IVHD_PREFSUP (1UL << 6) /* page prefetch support */ +#define IVHD_PPRSUP (1UL << 7) /* peripheral page service support */ + +#define IVHD_EFR_HATS 48 /* host address translation size */ +#define IVHD_EFR_GATS 48 /* guest address translation size */ +#define IVHD_EFR_GTSUP (1UL << 2) /* guest translation support */ + #endif diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 7eb51c7..b7b9c8a 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -362,6 +362,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/i386/acpi-build.c | 93 +++++++++++++++++++++++++++++++++++++++------ include/hw/acpi/acpi-defs.h | 13 +++++++ include/hw/acpi/aml-build.h | 1 + include/hw/boards.h | 6 +++ 5 files changed, 103 insertions(+), 12 deletions(-)