Hi Michael,
2014-08-14 19:06 GMT+08:00 Michael S. Tsirkin <m...@redhat.com>:
> On Mon, Aug 11, 2014 at 03:05:00PM +0800, Le Tan wrote:
>> Expose Intel IOMMU to the BIOS. If object of TYPE_INTEL_IOMMU_DEVICE exists,
>> add DMAR table to ACPI RSDT table. For now the DMAR table indicates that
>> there
>> is only one hardware unit without INTR_REMAP capability on the platform.
>>
>> Signed-off-by: Le Tan <tamlokv...@gmail.com>
>
> Could you add a unit test please?
>
>> ---
>> hw/i386/acpi-build.c | 41 ++++++++++++++++++++++++++++++
>> hw/i386/acpi-defs.h | 70
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 111 insertions(+)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 816c6d9..595f501 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -47,6 +47,7 @@
>> #include "hw/i386/ich9.h"
>> #include "hw/pci/pci_bus.h"
>> #include "hw/pci-host/q35.h"
>> +#include "hw/i386/intel_iommu.h"
>>
>> #include "hw/i386/q35-acpi-dsdt.hex"
>> #include "hw/i386/acpi-dsdt.hex"
>> @@ -1350,6 +1351,31 @@ build_mcfg_q35(GArray *table_data, GArray *linker,
>> AcpiMcfgInfo *info)
>> }
>>
>> static void
>> +build_dmar_q35(GArray *table_data, GArray *linker)
>> +{
>> + int dmar_start = table_data->len;
>> +
>> + AcpiTableDmar *dmar;
>> + AcpiDmarHardwareUnit *drhd;
>> +
>> + dmar = acpi_data_push(table_data, sizeof(*dmar));
>> + dmar->host_address_width = VTD_HOST_ADDRESS_WIDTH - 1;
>> + dmar->flags = 0; /* No intr_remap for now */
>> +
>> + /* DMAR Remapping Hardware Unit Definition structure */
>> + drhd = acpi_data_push(table_data, sizeof(*drhd));
>> + drhd->type = cpu_to_le16(ACPI_DMAR_TYPE_HARDWARE_UNIT);
>> + drhd->length = cpu_to_le16(sizeof(*drhd)); /* No device scope now */
>> + drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
>> + drhd->pci_segment = cpu_to_le16(0);
>> + drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR);
>> +
>> + build_header(linker, table_data, (void *)(table_data->data +
>> dmar_start),
>> + "DMAR", table_data->len - dmar_start, 1);
>> +}
>> +
>> +
>> +static void
>> build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc)
>> {
>> AcpiTableHeader *dsdt;
>> @@ -1470,6 +1496,17 @@ 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(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>> {
>> @@ -1539,6 +1576,10 @@ void acpi_build(PcGuestInfo *guest_info,
>> AcpiBuildTables *tables)
>> acpi_add_table(table_offsets, tables->table_data);
>> build_mcfg_q35(tables->table_data, tables->linker, &mcfg);
>> }
>> + if (acpi_has_iommu()) {
>> + acpi_add_table(table_offsets, tables->table_data);
>> + build_dmar_q35(tables->table_data, tables->linker);
>> + }
>>
>> /* Add tables supplied by user (if any) */
>> for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
>> diff --git a/hw/i386/acpi-defs.h b/hw/i386/acpi-defs.h
>> index e93babb..9674825 100644
>> --- a/hw/i386/acpi-defs.h
>> +++ b/hw/i386/acpi-defs.h
>> @@ -314,4 +314,74 @@ struct AcpiTableMcfg {
>> } QEMU_PACKED;
>> typedef struct AcpiTableMcfg AcpiTableMcfg;
>>
>> +/* 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 (2)
>> +
>> +/*
>> + * DMAR sub-structures (Follow DMA Remapping table)
>> + */
>> +#define ACPI_DMAR_SUB_HEADER_DEF /* Common ACPI DMAR sub-structure header
>> */\
>> + uint16_t type; \
>> + uint16_t length;
>
> Really necessary? cleaner to just open-code, it's
> only used once ...
For now it is just used once. I wrote this because I thought that it
is convenient to add other sub-tables in the future. To make it
cleaner, I will remove it later.
Thanks very much!
Le
>> +
>> +/* 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, correspond to Type in ACPI_DMAR_SUB_HEADER_DEF
>> + */
>> +
>> +/* DMAR Device Scope structures */
>> +struct AcpiDmarDeviceScope {
>> + uint8_t type;
>> + uint8_t length;
>> + uint16_t reserved;
>> + uint8_t enumeration_id;
>> + uint8_t start_bus_number;
>> + uint8_t path[0];
>> +} QEMU_PACKED;
>> +typedef struct AcpiDmarDeviceScope AcpiDmarDeviceScope;
>> +
>> +/* Values for type in struct AcpiDmarDeviceScope */
>> +enum {
>> + ACPI_DMAR_SCOPE_TYPE_NOT_USED = 0,
>> + ACPI_DMAR_SCOPE_TYPE_ENDPOINT = 1,
>> + ACPI_DMAR_SCOPE_TYPE_BRIDGE = 2,
>> + ACPI_DMAR_SCOPE_TYPE_IOAPIC = 3,
>> + ACPI_DMAR_SCOPE_TYPE_HPET = 4,
>> + ACPI_DMAR_SCOPE_TYPE_ACPI = 5,
>> + ACPI_DMAR_SCOPE_TYPE_RESERVED = 6 /* Reserved for future use */
>> +};
>> +
>> +/* 0: Hardware Unit Definition */
>> +struct AcpiDmarHardwareUnit {
>> + ACPI_DMAR_SUB_HEADER_DEF
>> + 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
>> */
>> +} QEMU_PACKED;
>> +typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit;
>> +
>> +/* Masks for Flags field above */
>> +#define ACPI_DMAR_INCLUDE_PCI_ALL (1)
>> +
>> +
>
> Don't add two empty lines in a row.
> Same in other places.
>
>> #endif
>> --
>> 1.9.1
