From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8K3e-0000Hc-MX for qemu-devel@nongnu.org; Thu, 02 Jun 2016 00:16:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8K3Z-0007Gu-Ni for qemu-devel@nongnu.org; Thu, 02 Jun 2016 00:16:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39668) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8K3Z-0007Gl-Hv for qemu-devel@nongnu.org; Thu, 02 Jun 2016 00:16:29 -0400 Date: Thu, 2 Jun 2016 12:16:20 +0800 From: Peter Xu Message-ID: <20160602041620.GA3477@pxdev.xzpeter.org> References: <1464604298-16739-1-git-send-email-peterx@redhat.com> <1464604298-16739-5-git-send-email-peterx@redhat.com> <20160601145638.45ab838a@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160601145638.45ab838a@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [PATCH v8 04/25] acpi: add DMAR scope definition for root IOAPIC List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, ehabkost@redhat.com, mst@redhat.com, jasowang@redhat.com, rkrcmar@redhat.com, alex.williamson@redhat.com, jan.kiszka@web.de, wexu@redhat.com, pbonzini@redhat.com, marcel@redhat.com, davidkiarie4@gmail.com, rth@twiddle.net On Wed, Jun 01, 2016 at 02:56:38PM +0200, Igor Mammedov wrote: [...] > > @@ -2561,6 +2563,9 @@ build_dmar_q35(MachineState *ms, GArray *table_data, GArray *linker) > > AcpiTableDmar *dmar; > > AcpiDmarHardwareUnit *drhd; > > uint8_t dmar_flags = 0; > > + AcpiDmarDeviceScope *scope = NULL; > > + /* Root complex IOAPIC use one path[0] only */ > > + uint16_t scope_size = sizeof(*scope) + sizeof(uint16_t); > ioapic_scope_size > sizeof(scope->path[0]) /* space for IOxAPIC path */ Agree on both. > > > if (ms->iommu_intr) { > > /* enable INTR for the IOMMU device */ > > @@ -2574,11 +2579,19 @@ build_dmar_q35(MachineState *ms, GArray *table_data, GArray *linker) > > /* DMAR Remapping Hardware Unit Definition structure */ > > drhd = acpi_data_push(table_data, sizeof(*drhd)); > sizeof(*drhd) + ioapic_scope_size > > > 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); > no need for this as previous push took care about all space we need for scope > ioapic_scope = &drhd->scope[0]; Both work for me. I can do a switch. > > > + scope->entry_type = cpu_to_le16(ACPI_DMAR_DEV_SCOPE_TYPE_IOAPIC); > entry_type is 1 byte long, doing cpu_to_le16 on bigendian host > will always result in 0 being written there > > > + scope->length = scope_size; > > + scope->enumeration_id = cpu_to_le16(ACPI_BUILD_IOAPIC_ID); > > + scope->bus = cpu_to_le16(Q35_PSEUDO_BUS_PLATFORM); > ditto for above 2 fields Ah, didn't notice these lines after it worked. For the three places, I should remove cpu_to_le16(). [...] > > +/* > > + * Arbitary but unique BNF number for IOAPIC device. This is only > > + * used when interrupt remapping is enabled. > you encode it in DMAR unconditionally so not "only" Okay. Removing the 2nd sentence. Thanks for the review comments, Igor! -- peterx