| Message ID | 20210907144814.741785-19-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | acpi: refactor error prone build_header() and packed structures usage in ACPI tables | expand |
On 9/7/21 4:47 PM, Igor Mammedov wrote: > it replaces error-prone pointer arithmetic for build_header() API, > with 2 calls to start and finish table creation, > which hides offsets magic from API user. > > While at it switch to build_append_int_noprefix() to build > table entries tables. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v3: > - rebase on top 26863366b29 (hw/i386/acpi-build: Add DMAR support to bypass iommu) > - s/acpi_init_table|acpi_table_composed/acpi_table_begin|acpi_table_end/ > > CC: wangxingang5@huawei.com > CC: marcel.apfelbaum@gmail.com > --- > include/hw/acpi/acpi-defs.h | 68 -------------------------- > hw/i386/acpi-build.c | 97 ++++++++++++++++++++----------------- > 2 files changed, 53 insertions(+), 112 deletions(-) > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > index d293304f9c..c4f0a202e8 100644 > --- a/include/hw/acpi/acpi-defs.h > +++ b/include/hw/acpi/acpi-defs.h > @@ -358,74 +358,6 @@ struct AcpiGenericTimerTable { > } QEMU_PACKED; > typedef struct AcpiGenericTimerTable AcpiGenericTimerTable; > > -/* DMAR - DMA Remapping table r2.2 */ > -struct AcpiTableDmar { > - ACPI_TABLE_HEADER_DEF > - uint8_t host_address_width; /* Maximum DMA physical addressability */ > - uint8_t flags; > - uint8_t reserved[10]; > -} QEMU_PACKED; > -typedef struct AcpiTableDmar AcpiTableDmar; > - > -/* Masks for Flags field above */ > -#define ACPI_DMAR_INTR_REMAP 1 > -#define ACPI_DMAR_X2APIC_OPT_OUT (1 << 1) > - > -/* Values for sub-structure type for DMAR */ > -enum { > - ACPI_DMAR_TYPE_HARDWARE_UNIT = 0, /* DRHD */ > - ACPI_DMAR_TYPE_RESERVED_MEMORY = 1, /* RMRR */ > - ACPI_DMAR_TYPE_ATSR = 2, /* ATSR */ > - ACPI_DMAR_TYPE_HARDWARE_AFFINITY = 3, /* RHSR */ > - ACPI_DMAR_TYPE_ANDD = 4, /* ANDD */ > - ACPI_DMAR_TYPE_RESERVED = 5 /* Reserved for furture use */ > -}; > - > -/* > - * Sub-structures for DMAR > - */ > - > -/* Device scope structure for DRHD. */ > -struct AcpiDmarDeviceScope { > - uint8_t entry_type; > - uint8_t length; > - uint16_t reserved; > - uint8_t enumeration_id; > - uint8_t bus; > - struct { > - uint8_t device; > - uint8_t function; > - } path[]; > -} QEMU_PACKED; > -typedef struct AcpiDmarDeviceScope AcpiDmarDeviceScope; > - > -/* Type 0: Hardware Unit Definition */ > -struct AcpiDmarHardwareUnit { > - uint16_t type; > - uint16_t length; > - uint8_t flags; > - uint8_t reserved; > - uint16_t pci_segment; /* The PCI Segment associated with this unit */ > - uint64_t address; /* Base address of remapping hardware register-set */ > - AcpiDmarDeviceScope scope[]; > -} QEMU_PACKED; > -typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit; > - > -/* Type 2: Root Port ATS Capability Reporting Structure */ > -struct AcpiDmarRootPortATS { > - uint16_t type; > - uint16_t length; > - uint8_t flags; > - uint8_t reserved; > - uint16_t pci_segment; > - AcpiDmarDeviceScope scope[]; > -} QEMU_PACKED; > -typedef struct AcpiDmarRootPortATS AcpiDmarRootPortATS; > - > -/* Masks for Flags field above */ > -#define ACPI_DMAR_INCLUDE_PCI_ALL 1 > -#define ACPI_DMAR_ATSR_ALL_PORTS 1 > - > /* > * Input Output Remapping Table (IORT) > * Conforms to "IO Remapping Table System Software on ARM Platforms", > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 51e0ba07b6..2875c4f393 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2064,8 +2064,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) > static void > insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) > { > + const size_t device_scope_size = 6 /* device scope structure */ + > + 2 /* 1 path entry */; > GArray *scope_blob = opaque; > - AcpiDmarDeviceScope *scope = NULL; > > if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) { > /* Dmar Scope Type: 0x02 for PCI Bridge */ > @@ -2076,8 +2077,7 @@ insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) > } > > /* length */ > - build_append_int_noprefix(scope_blob, > - sizeof(*scope) + sizeof(scope->path[0]), 1); > + build_append_int_noprefix(scope_blob, device_scope_size, 1); > /* reserved */ > build_append_int_noprefix(scope_blob, 0, 2); > /* enumeration_id */ > @@ -2109,26 +2109,31 @@ dmar_host_bridges(Object *obj, void *opaque) > } > > /* > - * VT-d spec 8.1 DMA Remapping Reporting Structure > - * (version Oct. 2014 or later) > + * Intel ® Virtualization Technology for Directed I/O > + * Architecture Specification. Revision 3.3 > + * 8.1 DMA Remapping Reporting Structure > */ > static void > build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, > const char *oem_table_id) > { > - int dmar_start = table_data->len; > - > - AcpiTableDmar *dmar; > - AcpiDmarHardwareUnit *drhd; > - AcpiDmarRootPortATS *atsr; > uint8_t dmar_flags = 0; > + uint8_t rsvd10[10] = {}; > + /* Root complex IOAPIC uses one path only */ > + const size_t ioapic_scope_size = 6 /* device scope structure */ + > + 2 /* 1 path entry */; > X86IOMMUState *iommu = x86_iommu_get_default(); > - AcpiDmarDeviceScope *scope = NULL; > - /* Root complex IOAPIC use one path[0] only */ > - size_t ioapic_scope_size = sizeof(*scope) + sizeof(scope->path[0]); > IntelIOMMUState *intel_iommu = INTEL_IOMMU_DEVICE(iommu); > GArray *scope_blob = g_array_new(false, true, 1); > > + AcpiTable table = { .sig = "DMAR", .rev = 1, .oem_id = oem_id, > + .oem_table_id = oem_table_id }; > + > + assert(iommu); > + if (x86_iommu_ir_supported(iommu)) { > + dmar_flags |= 0x1; /* Flags: 0x1: INT_REMAP */ > + } > + > /* > * A PCI bus walk, for each PCI host bridge. > * Insert scope for each PCI bridge and endpoint device which > @@ -2137,48 +2142,52 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, > object_child_foreach_recursive(object_get_root(), > dmar_host_bridges, scope_blob); > > - assert(iommu); > - if (x86_iommu_ir_supported(iommu)) { > - dmar_flags |= 0x1; /* Flags: 0x1: INT_REMAP */ > - } why this move? > - > - dmar = acpi_data_push(table_data, sizeof(*dmar)); > - dmar->host_address_width = intel_iommu->aw_bits - 1; > - dmar->flags = dmar_flags; > - > - /* DMAR Remapping Hardware Unit Definition structure */ > - drhd = acpi_data_push(table_data, sizeof(*drhd) + ioapic_scope_size); > - drhd->type = cpu_to_le16(ACPI_DMAR_TYPE_HARDWARE_UNIT); > - drhd->length = > - cpu_to_le16(sizeof(*drhd) + ioapic_scope_size + scope_blob->len); > - drhd->flags = 0; /* Don't include all pci device */ > - drhd->pci_segment = cpu_to_le16(0); > - drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR); > + acpi_table_begin(&table, table_data); > + /* Host Address Width */ > + build_append_int_noprefix(table_data, intel_iommu->aw_bits - 1, 1); > + build_append_int_noprefix(table_data, dmar_flags, 1); /* Flags */ > + g_array_append_vals(table_data, rsvd10, sizeof(rsvd10)); /* Reserved */ > + > + /* 8.3 DMAR Remapping Hardware Unit Definition structure */ > + build_append_int_noprefix(table_data, 0, 2); /* Type */ > + /* Length */ > + build_append_int_noprefix(table_data, > + 16 + ioapic_scope_size + scope_blob->len, 2); > + /* Flags */ > + build_append_int_noprefix(table_data, 0 /* Don't include all pci device */ , > + 1); > + build_append_int_noprefix(table_data, 0 , 1); /* Reserved */ > + build_append_int_noprefix(table_data, 0 , 2); /* Segment Number */ > + /* Register Base Address */ > + build_append_int_noprefix(table_data, Q35_HOST_BRIDGE_IOMMU_ADDR , 8); > > /* Scope definition for the root-complex IOAPIC. See VT-d spec > * 8.3.1 (version Oct. 2014 or later). */ > - scope = &drhd->scope[0]; > - scope->entry_type = 0x03; /* Type: 0x03 for IOAPIC */ > - scope->length = ioapic_scope_size; > - scope->enumeration_id = ACPI_BUILD_IOAPIC_ID; > - scope->bus = Q35_PSEUDO_BUS_PLATFORM; > - scope->path[0].device = PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC); > - scope->path[0].function = PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC); > + build_append_int_noprefix(table_data, 0x03 /* IOAPIC */, 1); /* Type */ > + build_append_int_noprefix(table_data, ioapic_scope_size, 1); /* Length */ > + build_append_int_noprefix(table_data, 0, 2); /* Reserved */ > + /* Enumeration ID */ > + build_append_int_noprefix(table_data, ACPI_BUILD_IOAPIC_ID, 1); > + /* Start Bus Number */ > + build_append_int_noprefix(table_data, Q35_PSEUDO_BUS_PLATFORM, 1); > + /* Path, {Device, Function} pair */ > + build_append_int_noprefix(table_data, PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC), 1); > + build_append_int_noprefix(table_data, PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC), 1); > > /* Add scope found above */ > g_array_append_vals(table_data, scope_blob->data, scope_blob->len); > g_array_free(scope_blob, true); > > if (iommu->dt_supported) { > - atsr = acpi_data_push(table_data, sizeof(*atsr)); > - atsr->type = cpu_to_le16(ACPI_DMAR_TYPE_ATSR); > - atsr->length = cpu_to_le16(sizeof(*atsr)); > - atsr->flags = ACPI_DMAR_ATSR_ALL_PORTS; > - atsr->pci_segment = cpu_to_le16(0); > + /* 8.5 Root Port ATS Capability Reporting Structure */ > + build_append_int_noprefix(table_data, 2, 2); /* Type */ > + build_append_int_noprefix(table_data, 8, 2); /* Length */ 8 + no device scope > + build_append_int_noprefix(table_data, 1 /* ALL_PORTS */, 1); /* Flags */ > + build_append_int_noprefix(table_data, 0, 1); /* Reserved */ > + build_append_int_noprefix(table_data, 0, 2); /* Segment Number */ > } > > - build_header(linker, table_data, (void *)(table_data->data + dmar_start), > - "DMAR", table_data->len - dmar_start, 1, oem_id, oem_table_id); > + acpi_table_end(linker, &table); > } > > /* > Besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric
On Wed, 22 Sep 2021 11:19:05 +0200 Eric Auger <eauger@redhat.com> wrote: > On 9/7/21 4:47 PM, Igor Mammedov wrote: > > it replaces error-prone pointer arithmetic for build_header() API, > > with 2 calls to start and finish table creation, > > which hides offsets magic from API user. > > > > While at it switch to build_append_int_noprefix() to build > > table entries tables. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v3: > > - rebase on top 26863366b29 (hw/i386/acpi-build: Add DMAR support to bypass iommu) > > - s/acpi_init_table|acpi_table_composed/acpi_table_begin|acpi_table_end/ > > > > CC: wangxingang5@huawei.com > > CC: marcel.apfelbaum@gmail.com > > --- > > include/hw/acpi/acpi-defs.h | 68 -------------------------- > > hw/i386/acpi-build.c | 97 ++++++++++++++++++++----------------- > > 2 files changed, 53 insertions(+), 112 deletions(-) > > > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > > index d293304f9c..c4f0a202e8 100644 > > --- a/include/hw/acpi/acpi-defs.h > > +++ b/include/hw/acpi/acpi-defs.h > > @@ -358,74 +358,6 @@ struct AcpiGenericTimerTable { > > } QEMU_PACKED; > > typedef struct AcpiGenericTimerTable AcpiGenericTimerTable; > > > > -/* DMAR - DMA Remapping table r2.2 */ > > -struct AcpiTableDmar { > > - ACPI_TABLE_HEADER_DEF > > - uint8_t host_address_width; /* Maximum DMA physical addressability */ > > - uint8_t flags; > > - uint8_t reserved[10]; > > -} QEMU_PACKED; > > -typedef struct AcpiTableDmar AcpiTableDmar; > > - > > -/* Masks for Flags field above */ > > -#define ACPI_DMAR_INTR_REMAP 1 > > -#define ACPI_DMAR_X2APIC_OPT_OUT (1 << 1) > > - > > -/* Values for sub-structure type for DMAR */ > > -enum { > > - ACPI_DMAR_TYPE_HARDWARE_UNIT = 0, /* DRHD */ > > - ACPI_DMAR_TYPE_RESERVED_MEMORY = 1, /* RMRR */ > > - ACPI_DMAR_TYPE_ATSR = 2, /* ATSR */ > > - ACPI_DMAR_TYPE_HARDWARE_AFFINITY = 3, /* RHSR */ > > - ACPI_DMAR_TYPE_ANDD = 4, /* ANDD */ > > - ACPI_DMAR_TYPE_RESERVED = 5 /* Reserved for furture use */ > > -}; > > - > > -/* > > - * Sub-structures for DMAR > > - */ > > - > > -/* Device scope structure for DRHD. */ > > -struct AcpiDmarDeviceScope { > > - uint8_t entry_type; > > - uint8_t length; > > - uint16_t reserved; > > - uint8_t enumeration_id; > > - uint8_t bus; > > - struct { > > - uint8_t device; > > - uint8_t function; > > - } path[]; > > -} QEMU_PACKED; > > -typedef struct AcpiDmarDeviceScope AcpiDmarDeviceScope; > > - > > -/* Type 0: Hardware Unit Definition */ > > -struct AcpiDmarHardwareUnit { > > - uint16_t type; > > - uint16_t length; > > - uint8_t flags; > > - uint8_t reserved; > > - uint16_t pci_segment; /* The PCI Segment associated with this unit */ > > - uint64_t address; /* Base address of remapping hardware register-set */ > > - AcpiDmarDeviceScope scope[]; > > -} QEMU_PACKED; > > -typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit; > > - > > -/* Type 2: Root Port ATS Capability Reporting Structure */ > > -struct AcpiDmarRootPortATS { > > - uint16_t type; > > - uint16_t length; > > - uint8_t flags; > > - uint8_t reserved; > > - uint16_t pci_segment; > > - AcpiDmarDeviceScope scope[]; > > -} QEMU_PACKED; > > -typedef struct AcpiDmarRootPortATS AcpiDmarRootPortATS; > > - > > -/* Masks for Flags field above */ > > -#define ACPI_DMAR_INCLUDE_PCI_ALL 1 > > -#define ACPI_DMAR_ATSR_ALL_PORTS 1 > > - > > /* > > * Input Output Remapping Table (IORT) > > * Conforms to "IO Remapping Table System Software on ARM Platforms", > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 51e0ba07b6..2875c4f393 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -2064,8 +2064,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) > > static void > > insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) > > { > > + const size_t device_scope_size = 6 /* device scope structure */ + > > + 2 /* 1 path entry */; > > GArray *scope_blob = opaque; > > - AcpiDmarDeviceScope *scope = NULL; > > > > if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) { > > /* Dmar Scope Type: 0x02 for PCI Bridge */ > > @@ -2076,8 +2077,7 @@ insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) > > } > > > > /* length */ > > - build_append_int_noprefix(scope_blob, > > - sizeof(*scope) + sizeof(scope->path[0]), 1); > > + build_append_int_noprefix(scope_blob, device_scope_size, 1); > > /* reserved */ > > build_append_int_noprefix(scope_blob, 0, 2); > > /* enumeration_id */ > > @@ -2109,26 +2109,31 @@ dmar_host_bridges(Object *obj, void *opaque) > > } > > > > /* > > - * VT-d spec 8.1 DMA Remapping Reporting Structure > > - * (version Oct. 2014 or later) > > + * Intel ® Virtualization Technology for Directed I/O > > + * Architecture Specification. Revision 3.3 > > + * 8.1 DMA Remapping Reporting Structure > > */ > > static void > > build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, > > const char *oem_table_id) > > { > > - int dmar_start = table_data->len; > > - > > - AcpiTableDmar *dmar; > > - AcpiDmarHardwareUnit *drhd; > > - AcpiDmarRootPortATS *atsr; > > uint8_t dmar_flags = 0; > > + uint8_t rsvd10[10] = {}; > > + /* Root complex IOAPIC uses one path only */ > > + const size_t ioapic_scope_size = 6 /* device scope structure */ + > > + 2 /* 1 path entry */; > > X86IOMMUState *iommu = x86_iommu_get_default(); > > - AcpiDmarDeviceScope *scope = NULL; > > - /* Root complex IOAPIC use one path[0] only */ > > - size_t ioapic_scope_size = sizeof(*scope) + sizeof(scope->path[0]); > > IntelIOMMUState *intel_iommu = INTEL_IOMMU_DEVICE(iommu); > > GArray *scope_blob = g_array_new(false, true, 1); > > > > + AcpiTable table = { .sig = "DMAR", .rev = 1, .oem_id = oem_id, > > + .oem_table_id = oem_table_id }; > > + > > + assert(iommu); > > + if (x86_iommu_ir_supported(iommu)) { > > + dmar_flags |= 0x1; /* Flags: 0x1: INT_REMAP */ > > + } > > + > > /* > > * A PCI bus walk, for each PCI host bridge. > > * Insert scope for each PCI bridge and endpoint device which > > @@ -2137,48 +2142,52 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, > > object_child_foreach_recursive(object_get_root(), > > dmar_host_bridges, scope_blob); > > > > - assert(iommu); > > - if (x86_iommu_ir_supported(iommu)) { > > - dmar_flags |= 0x1; /* Flags: 0x1: INT_REMAP */ > > - } > why this move? it's not really necessary/related, probably it was moved to be closer to other variables initialization. > > - > > - dmar = acpi_data_push(table_data, sizeof(*dmar)); > > - dmar->host_address_width = intel_iommu->aw_bits - 1; > > - dmar->flags = dmar_flags; > > - > > - /* DMAR Remapping Hardware Unit Definition structure */ > > - drhd = acpi_data_push(table_data, sizeof(*drhd) + ioapic_scope_size); > > - drhd->type = cpu_to_le16(ACPI_DMAR_TYPE_HARDWARE_UNIT); > > - drhd->length = > > - cpu_to_le16(sizeof(*drhd) + ioapic_scope_size + scope_blob->len); > > - drhd->flags = 0; /* Don't include all pci device */ > > - drhd->pci_segment = cpu_to_le16(0); > > - drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR); > > + acpi_table_begin(&table, table_data); > > + /* Host Address Width */ > > + build_append_int_noprefix(table_data, intel_iommu->aw_bits - 1, 1); > > + build_append_int_noprefix(table_data, dmar_flags, 1); /* Flags */ > > + g_array_append_vals(table_data, rsvd10, sizeof(rsvd10)); /* Reserved */ > > + > > + /* 8.3 DMAR Remapping Hardware Unit Definition structure */ > > + build_append_int_noprefix(table_data, 0, 2); /* Type */ > > + /* Length */ > > + build_append_int_noprefix(table_data, > > + 16 + ioapic_scope_size + scope_blob->len, 2); > > + /* Flags */ > > + build_append_int_noprefix(table_data, 0 /* Don't include all pci device */ , > > + 1); > > + build_append_int_noprefix(table_data, 0 , 1); /* Reserved */ > > + build_append_int_noprefix(table_data, 0 , 2); /* Segment Number */ > > + /* Register Base Address */ > > + build_append_int_noprefix(table_data, Q35_HOST_BRIDGE_IOMMU_ADDR , 8); > > > > /* Scope definition for the root-complex IOAPIC. See VT-d spec > > * 8.3.1 (version Oct. 2014 or later). */ > > - scope = &drhd->scope[0]; > > - scope->entry_type = 0x03; /* Type: 0x03 for IOAPIC */ > > - scope->length = ioapic_scope_size; > > - scope->enumeration_id = ACPI_BUILD_IOAPIC_ID; > > - scope->bus = Q35_PSEUDO_BUS_PLATFORM; > > - scope->path[0].device = PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC); > > - scope->path[0].function = PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC); > > + build_append_int_noprefix(table_data, 0x03 /* IOAPIC */, 1); /* Type */ > > + build_append_int_noprefix(table_data, ioapic_scope_size, 1); /* Length */ > > + build_append_int_noprefix(table_data, 0, 2); /* Reserved */ > > + /* Enumeration ID */ > > + build_append_int_noprefix(table_data, ACPI_BUILD_IOAPIC_ID, 1); > > + /* Start Bus Number */ > > + build_append_int_noprefix(table_data, Q35_PSEUDO_BUS_PLATFORM, 1); > > + /* Path, {Device, Function} pair */ > > + build_append_int_noprefix(table_data, PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC), 1); > > + build_append_int_noprefix(table_data, PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC), 1); > > > > /* Add scope found above */ > > g_array_append_vals(table_data, scope_blob->data, scope_blob->len); > > g_array_free(scope_blob, true); > > > > if (iommu->dt_supported) { > > - atsr = acpi_data_push(table_data, sizeof(*atsr)); > > - atsr->type = cpu_to_le16(ACPI_DMAR_TYPE_ATSR); > > - atsr->length = cpu_to_le16(sizeof(*atsr)); > > - atsr->flags = ACPI_DMAR_ATSR_ALL_PORTS; > > - atsr->pci_segment = cpu_to_le16(0); > > + /* 8.5 Root Port ATS Capability Reporting Structure */ > > + build_append_int_noprefix(table_data, 2, 2); /* Type */ > > + build_append_int_noprefix(table_data, 8, 2); /* Length */ > 8 + no device scope yep, but given no device scope implemented I didn't see a need to mention it at all. > > + build_append_int_noprefix(table_data, 1 /* ALL_PORTS */, 1); /* Flags */ > > + build_append_int_noprefix(table_data, 0, 1); /* Reserved */ > > + build_append_int_noprefix(table_data, 0, 2); /* Segment Number */ > > } > > > > - build_header(linker, table_data, (void *)(table_data->data + dmar_start), > > - "DMAR", table_data->len - dmar_start, 1, oem_id, oem_table_id); > > + acpi_table_end(linker, &table); > > } > > > > /* > > > Besides > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Eric >
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index d293304f9c..c4f0a202e8 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -358,74 +358,6 @@ struct AcpiGenericTimerTable { } QEMU_PACKED; typedef struct AcpiGenericTimerTable AcpiGenericTimerTable; -/* DMAR - DMA Remapping table r2.2 */ -struct AcpiTableDmar { - ACPI_TABLE_HEADER_DEF - uint8_t host_address_width; /* Maximum DMA physical addressability */ - uint8_t flags; - uint8_t reserved[10]; -} QEMU_PACKED; -typedef struct AcpiTableDmar AcpiTableDmar; - -/* Masks for Flags field above */ -#define ACPI_DMAR_INTR_REMAP 1 -#define ACPI_DMAR_X2APIC_OPT_OUT (1 << 1) - -/* Values for sub-structure type for DMAR */ -enum { - ACPI_DMAR_TYPE_HARDWARE_UNIT = 0, /* DRHD */ - ACPI_DMAR_TYPE_RESERVED_MEMORY = 1, /* RMRR */ - ACPI_DMAR_TYPE_ATSR = 2, /* ATSR */ - ACPI_DMAR_TYPE_HARDWARE_AFFINITY = 3, /* RHSR */ - ACPI_DMAR_TYPE_ANDD = 4, /* ANDD */ - ACPI_DMAR_TYPE_RESERVED = 5 /* Reserved for furture use */ -}; - -/* - * Sub-structures for DMAR - */ - -/* Device scope structure for DRHD. */ -struct AcpiDmarDeviceScope { - uint8_t entry_type; - uint8_t length; - uint16_t reserved; - uint8_t enumeration_id; - uint8_t bus; - struct { - uint8_t device; - uint8_t function; - } path[]; -} QEMU_PACKED; -typedef struct AcpiDmarDeviceScope AcpiDmarDeviceScope; - -/* Type 0: Hardware Unit Definition */ -struct AcpiDmarHardwareUnit { - uint16_t type; - uint16_t length; - uint8_t flags; - uint8_t reserved; - uint16_t pci_segment; /* The PCI Segment associated with this unit */ - uint64_t address; /* Base address of remapping hardware register-set */ - AcpiDmarDeviceScope scope[]; -} QEMU_PACKED; -typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit; - -/* Type 2: Root Port ATS Capability Reporting Structure */ -struct AcpiDmarRootPortATS { - uint16_t type; - uint16_t length; - uint8_t flags; - uint8_t reserved; - uint16_t pci_segment; - AcpiDmarDeviceScope scope[]; -} QEMU_PACKED; -typedef struct AcpiDmarRootPortATS AcpiDmarRootPortATS; - -/* Masks for Flags field above */ -#define ACPI_DMAR_INCLUDE_PCI_ALL 1 -#define ACPI_DMAR_ATSR_ALL_PORTS 1 - /* * Input Output Remapping Table (IORT) * Conforms to "IO Remapping Table System Software on ARM Platforms", diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 51e0ba07b6..2875c4f393 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2064,8 +2064,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) static void insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) { + const size_t device_scope_size = 6 /* device scope structure */ + + 2 /* 1 path entry */; GArray *scope_blob = opaque; - AcpiDmarDeviceScope *scope = NULL; if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) { /* Dmar Scope Type: 0x02 for PCI Bridge */ @@ -2076,8 +2077,7 @@ insert_scope(PCIBus *bus, PCIDevice *dev, void *opaque) } /* length */ - build_append_int_noprefix(scope_blob, - sizeof(*scope) + sizeof(scope->path[0]), 1); + build_append_int_noprefix(scope_blob, device_scope_size, 1); /* reserved */ build_append_int_noprefix(scope_blob, 0, 2); /* enumeration_id */ @@ -2109,26 +2109,31 @@ dmar_host_bridges(Object *obj, void *opaque) } /* - * VT-d spec 8.1 DMA Remapping Reporting Structure - * (version Oct. 2014 or later) + * Intel ® Virtualization Technology for Directed I/O + * Architecture Specification. Revision 3.3 + * 8.1 DMA Remapping Reporting Structure */ static void build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, const char *oem_table_id) { - int dmar_start = table_data->len; - - AcpiTableDmar *dmar; - AcpiDmarHardwareUnit *drhd; - AcpiDmarRootPortATS *atsr; uint8_t dmar_flags = 0; + uint8_t rsvd10[10] = {}; + /* Root complex IOAPIC uses one path only */ + const size_t ioapic_scope_size = 6 /* device scope structure */ + + 2 /* 1 path entry */; X86IOMMUState *iommu = x86_iommu_get_default(); - AcpiDmarDeviceScope *scope = NULL; - /* Root complex IOAPIC use one path[0] only */ - size_t ioapic_scope_size = sizeof(*scope) + sizeof(scope->path[0]); IntelIOMMUState *intel_iommu = INTEL_IOMMU_DEVICE(iommu); GArray *scope_blob = g_array_new(false, true, 1); + AcpiTable table = { .sig = "DMAR", .rev = 1, .oem_id = oem_id, + .oem_table_id = oem_table_id }; + + assert(iommu); + if (x86_iommu_ir_supported(iommu)) { + dmar_flags |= 0x1; /* Flags: 0x1: INT_REMAP */ + } + /* * A PCI bus walk, for each PCI host bridge. * Insert scope for each PCI bridge and endpoint device which @@ -2137,48 +2142,52 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker, const char *oem_id, object_child_foreach_recursive(object_get_root(), dmar_host_bridges, scope_blob); - assert(iommu); - if (x86_iommu_ir_supported(iommu)) { - dmar_flags |= 0x1; /* Flags: 0x1: INT_REMAP */ - } - - dmar = acpi_data_push(table_data, sizeof(*dmar)); - dmar->host_address_width = intel_iommu->aw_bits - 1; - dmar->flags = dmar_flags; - - /* DMAR Remapping Hardware Unit Definition structure */ - drhd = acpi_data_push(table_data, sizeof(*drhd) + ioapic_scope_size); - drhd->type = cpu_to_le16(ACPI_DMAR_TYPE_HARDWARE_UNIT); - drhd->length = - cpu_to_le16(sizeof(*drhd) + ioapic_scope_size + scope_blob->len); - drhd->flags = 0; /* Don't include all pci device */ - drhd->pci_segment = cpu_to_le16(0); - drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR); + acpi_table_begin(&table, table_data); + /* Host Address Width */ + build_append_int_noprefix(table_data, intel_iommu->aw_bits - 1, 1); + build_append_int_noprefix(table_data, dmar_flags, 1); /* Flags */ + g_array_append_vals(table_data, rsvd10, sizeof(rsvd10)); /* Reserved */ + + /* 8.3 DMAR Remapping Hardware Unit Definition structure */ + build_append_int_noprefix(table_data, 0, 2); /* Type */ + /* Length */ + build_append_int_noprefix(table_data, + 16 + ioapic_scope_size + scope_blob->len, 2); + /* Flags */ + build_append_int_noprefix(table_data, 0 /* Don't include all pci device */ , + 1); + build_append_int_noprefix(table_data, 0 , 1); /* Reserved */ + build_append_int_noprefix(table_data, 0 , 2); /* Segment Number */ + /* Register Base Address */ + build_append_int_noprefix(table_data, Q35_HOST_BRIDGE_IOMMU_ADDR , 8); /* Scope definition for the root-complex IOAPIC. See VT-d spec * 8.3.1 (version Oct. 2014 or later). */ - scope = &drhd->scope[0]; - scope->entry_type = 0x03; /* Type: 0x03 for IOAPIC */ - scope->length = ioapic_scope_size; - scope->enumeration_id = ACPI_BUILD_IOAPIC_ID; - scope->bus = Q35_PSEUDO_BUS_PLATFORM; - scope->path[0].device = PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC); - scope->path[0].function = PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC); + build_append_int_noprefix(table_data, 0x03 /* IOAPIC */, 1); /* Type */ + build_append_int_noprefix(table_data, ioapic_scope_size, 1); /* Length */ + build_append_int_noprefix(table_data, 0, 2); /* Reserved */ + /* Enumeration ID */ + build_append_int_noprefix(table_data, ACPI_BUILD_IOAPIC_ID, 1); + /* Start Bus Number */ + build_append_int_noprefix(table_data, Q35_PSEUDO_BUS_PLATFORM, 1); + /* Path, {Device, Function} pair */ + build_append_int_noprefix(table_data, PCI_SLOT(Q35_PSEUDO_DEVFN_IOAPIC), 1); + build_append_int_noprefix(table_data, PCI_FUNC(Q35_PSEUDO_DEVFN_IOAPIC), 1); /* Add scope found above */ g_array_append_vals(table_data, scope_blob->data, scope_blob->len); g_array_free(scope_blob, true); if (iommu->dt_supported) { - atsr = acpi_data_push(table_data, sizeof(*atsr)); - atsr->type = cpu_to_le16(ACPI_DMAR_TYPE_ATSR); - atsr->length = cpu_to_le16(sizeof(*atsr)); - atsr->flags = ACPI_DMAR_ATSR_ALL_PORTS; - atsr->pci_segment = cpu_to_le16(0); + /* 8.5 Root Port ATS Capability Reporting Structure */ + build_append_int_noprefix(table_data, 2, 2); /* Type */ + build_append_int_noprefix(table_data, 8, 2); /* Length */ + build_append_int_noprefix(table_data, 1 /* ALL_PORTS */, 1); /* Flags */ + build_append_int_noprefix(table_data, 0, 1); /* Reserved */ + build_append_int_noprefix(table_data, 0, 2); /* Segment Number */ } - build_header(linker, table_data, (void *)(table_data->data + dmar_start), - "DMAR", table_data->len - dmar_start, 1, oem_id, oem_table_id); + acpi_table_end(linker, &table); } /*
it replaces error-prone pointer arithmetic for build_header() API, with 2 calls to start and finish table creation, which hides offsets magic from API user. While at it switch to build_append_int_noprefix() to build table entries tables. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v3: - rebase on top 26863366b29 (hw/i386/acpi-build: Add DMAR support to bypass iommu) - s/acpi_init_table|acpi_table_composed/acpi_table_begin|acpi_table_end/ CC: wangxingang5@huawei.com CC: marcel.apfelbaum@gmail.com --- include/hw/acpi/acpi-defs.h | 68 -------------------------- hw/i386/acpi-build.c | 97 ++++++++++++++++++++----------------- 2 files changed, 53 insertions(+), 112 deletions(-)