Re: [Qemu-devel] [PATCH 05/13] acpi: add DMAR scope definition for root IOAPIC

[Marcel Apfelbaum <marcel.apfelbaum@gmail.com>]

On 02/19/2016 05:30 AM, Peter Xu wrote:
> To enable interrupt remapping for intel IOMMU device, each IOAPIC device
> in the system reported via ACPI MADT must be explicitly enumerated under
> one specific remapping hardware unit. This patch adds the root-complex
> IOAPIC into the default DMAR device.
>
> Please refer to VT-d spec 8.3.1.1 for more information.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/i386/acpi-build.c        | 23 +++++++++++++++++++++--
>   include/hw/acpi/acpi-defs.h | 15 +++++++++++++++
>   2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d9e4f91..1cefe43 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -76,6 +76,9 @@
>   #define ACPI_BUILD_DPRINTF(fmt, ...)
>   #endif
>
> +/* Default IOAPIC ID */
> +#define ACPI_BUILD_IOAPIC_ID 0x0
> +
>   typedef struct AcpiCpuInfo {
>       DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
>   } AcpiCpuInfo;
> @@ -392,7 +395,6 @@ build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu)
>       io_apic = acpi_data_push(table_data, sizeof *io_apic);
>       io_apic->type = ACPI_APIC_IO;
>       io_apic->length = sizeof(*io_apic);
> -#define ACPI_BUILD_IOAPIC_ID 0x0
>       io_apic->io_apic_id = ACPI_BUILD_IOAPIC_ID;
>       io_apic->address = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
>       io_apic->interrupt = cpu_to_le32(0);
> @@ -2511,6 +2513,9 @@ build_dmar_q35(GArray *table_data, GArray *linker)
>       AcpiDmarHardwareUnit *drhd;
>       uint8_t dmar_flags = 0;
>       IntelIOMMUState *intel_iommu = acpi_get_iommu();
> +    AcpiDmarDeviceScope *scope = NULL;
> +    /* Root complex IOAPIC use one path[0] only */
> +    uint16_t scope_size = sizeof(*scope) + sizeof(uint16_t);
>
>       assert(intel_iommu);
>
> @@ -2526,11 +2531,25 @@ build_dmar_q35(GArray *table_data, GArray *linker)
>       /* 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->length = cpu_to_le16(sizeof(*drhd) + scope_size);
>       drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
>       drhd->pci_segment = cpu_to_le16(0);
>       drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR);
>
> +    /* Scope definition for the root-complex IOAPIC */
> +    scope = acpi_data_push(table_data, scope_size);
> +    scope->entry_type = cpu_to_le16(ACPI_DMAR_DEV_SCOPE_TYPE_IOAPIC);
> +    scope->length = scope_size;
> +    /*
> +     * An arbitary but unique bus number, to be used to generate
> +     * source ID for IOAPIC device in BDF format.
> +     */
> +#define ACPI_IOAPIC_BUS_IR         (0xf0)
> +#define ACPI_IOAPIC_DEVFN_IR       (0x00)

Hi,

How do you know for sure there is no bus (or bus & device) having the number 0xf0 in the system?
Now that we support multiple Root Complexes, using the  pxb-pcie device we can simply add a PCI root bus like this:
     -device pxb-pcie,bus_nr=0xf0
or we can add enough switches to get to this number.

You could dynamically query for an unused PCI bus, but the number would change between the runs.
Or, I suppose you can reserve a slot on bus 0 for that. It is interesting how it works on a real machine.

Thanks,
Marcel


> +    scope->enumeration_id = cpu_to_le16(ACPI_BUILD_IOAPIC_ID);
> +    scope->bus = cpu_to_le16(ACPI_IOAPIC_BUS_IR);
> +    scope->path[0] = cpu_to_le16(ACPI_IOAPIC_DEVFN_IR);
> +
>       build_header(linker, table_data, (void *)(table_data->data + dmar_start),
>                    "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
>   }
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index c7a03d4..2430af6 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -556,6 +556,20 @@ enum {
>   /*
>    * Sub-structures for DMAR
>    */
> +
> +#define ACPI_DMAR_DEV_SCOPE_TYPE_IOAPIC     (0x03)
> +
> +/* Device scope structure for DRHD. */
> +struct AcpiDmarDeviceScope {
> +    uint8_t entry_type;
> +    uint8_t length;
> +    uint16_t reserved;
> +    uint8_t enumeration_id;
> +    uint8_t bus;
> +    uint16_t path[0];           /* list of dev:func pairs */
> +} QEMU_PACKED;
> +typedef struct AcpiDmarDeviceScope AcpiDmarDeviceScope;
> +
>   /* Type 0: Hardware Unit Definition */
>   struct AcpiDmarHardwareUnit {
>       uint16_t type;
> @@ -564,6 +578,7 @@ struct AcpiDmarHardwareUnit {
>       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[0];
>   } QEMU_PACKED;
>   typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit;
>
>