From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50630) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aXSor-0000qE-29 for qemu-devel@nongnu.org; Sun, 21 Feb 2016 07:08:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aXSon-0004Kz-Q0 for qemu-devel@nongnu.org; Sun, 21 Feb 2016 07:08:56 -0500 Received: from mail-wm0-x230.google.com ([2a00:1450:400c:c09::230]:35434) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aXSon-0004Kv-EU for qemu-devel@nongnu.org; Sun, 21 Feb 2016 07:08:53 -0500 Received: by mail-wm0-x230.google.com with SMTP id c200so134408097wme.0 for ; Sun, 21 Feb 2016 04:08:53 -0800 (PST) References: <1455852618-5224-1-git-send-email-peterx@redhat.com> <1455852618-5224-6-git-send-email-peterx@redhat.com> <56C9A1B8.6070909@gmail.com> From: Marcel Apfelbaum Message-ID: <56C9A8D1.4010604@gmail.com> Date: Sun, 21 Feb 2016 14:08:49 +0200 MIME-Version: 1.0 In-Reply-To: <56C9A1B8.6070909@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 05/13] acpi: add DMAR scope definition for root IOAPIC Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu , qemu-devel@nongnu.org Cc: ehabkost@redhat.com, mst@redhat.com, jasowang@redhat.com, imammedo@redhat.com, pbonzini@redhat.com, rth@twiddle.net On 02/21/2016 01:38 PM, Marcel Apfelbaum wrote: > 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 >> --- >> 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. thinking about it more, maybe we should let the firmware to assign the bus/dev/fun for the IO APIC? Thanks, Marcel > > 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; >> >> >