From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34042) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aXWLF-0000kG-2e for qemu-devel@nongnu.org; Sun, 21 Feb 2016 10:54:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aXWL9-0008AN-NS for qemu-devel@nongnu.org; Sun, 21 Feb 2016 10:54:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60280) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aXWL9-0008A8-EM for qemu-devel@nongnu.org; Sun, 21 Feb 2016 10:54:31 -0500 References: <1455852618-5224-1-git-send-email-peterx@redhat.com> <1455852618-5224-6-git-send-email-peterx@redhat.com> <56C9A1B8.6070909@gmail.com> <56C9A8D1.4010604@gmail.com> <56C9BE5D.60808@web.de> From: Marcel Apfelbaum Message-ID: <56C9DDB1.8000305@redhat.com> Date: Sun, 21 Feb 2016 17:54:25 +0200 MIME-Version: 1.0 In-Reply-To: <56C9BE5D.60808@web.de> 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 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka , Peter Xu , qemu-devel@nongnu.org Cc: Rita Sinha , ehabkost@redhat.com, mst@redhat.com, jasowang@redhat.com, pbonzini@redhat.com, imammedo@redhat.com, rth@twiddle.net On 02/21/2016 03:40 PM, Jan Kiszka wrote: > On 2016-02-21 13:08, Marcel Apfelbaum wrote: >> 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? > > We have the same problem over with VT-d and IR. > > I don't think the firmware is not the right place, otherwise there would > be an interface in hw to adjust that parameters. I think we should > simply make sure that the qemu user cannot assign devices to those > addresses as they are reserved for the platform devices (the HPET > requires another ID), or even reserve the hole bus for the platform. I understand, but it is the firmware that assign addresses, not QEMU/user. We need a way to tell the firmware not to use a certain address range, we can do that with fw_config I suppose. Reserving a bus might be easier, we take the bus number out from host bridges CRS ranges and each platform device can be assigned a slot. We would need to select the bus carefully to not collide with other PCI hierarchies. Maybe bus 0xFF. Thanks, Marcel > > Jan >