From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57458) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv18u-0000x5-NV for qemu-devel@nongnu.org; Fri, 14 Oct 2016 07:59:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bv18s-0005SK-Hf for qemu-devel@nongnu.org; Fri, 14 Oct 2016 07:59:15 -0400 References: <1476435295-21885-1-git-send-email-eric.auger@redhat.com> <1476435295-21885-2-git-send-email-eric.auger@redhat.com> <20161014113035.ebmsswsi632mopw4@hawk.localdomain> From: Auger Eric Message-ID: <4e6a3461-a3d9-d0f0-1954-b34bf542d775@redhat.com> Date: Fri, 14 Oct 2016 13:59:03 +0200 MIME-Version: 1.0 In-Reply-To: <20161014113035.ebmsswsi632mopw4@hawk.localdomain> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/2] ACPI: Add IORT Structure definition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Jones Cc: eric.auger.pro@gmail.com, prem.mallappa@broadcom.com, peter.maydell@linaro.org, qemu-arm@nongnu.org, qemu-devel@nongnu.org, shannon.zhao@linaro.org, christoffer.dall@linaro.org, tn@semihalf.com Hi Drew, On 14/10/2016 13:30, Andrew Jones wrote: > On Fri, Oct 14, 2016 at 10:54:54AM +0200, Eric Auger wrote: >> From: Prem Mallappa >> >> ACPI Spec 6.0 introduces IO Remapping Table Structure. This patch >> introduces the definitions required to describe the IO relationship >> between the PCIe root complex and the ITS. >> >> This conforms to: >> "IO Remapping Table System Software on ARM Platforms", >> Document number: ARM DEN 0049B, October 2015. >> >> Signed-off-by: Prem Mallappa >> Signed-off-by: Eric Auger >> >> --- >> >> v1 -> v2: >> - took into account Drew's comments: >> cleanup comments, remove most defines, add ACPI_IORT_NODE_HEADER_DEF >> --- >> include/hw/acpi/acpi-defs.h | 68 +++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 68 insertions(+) >> >> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h >> index 9c1b7cb..1cd1e69 100644 >> --- a/include/hw/acpi/acpi-defs.h >> +++ b/include/hw/acpi/acpi-defs.h >> @@ -609,4 +609,72 @@ typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit; >> /* Masks for Flags field above */ >> #define ACPI_DMAR_INCLUDE_PCI_ALL 1 >> >> +/* >> + * Input Output Remapping Table (IORT) >> + * Conforms to "IO Remapping Table System Software on ARM Platforms", >> + * Document number: ARM DEN 0049B, October 2015 >> + */ >> + >> +struct AcpiIortTable { >> + ACPI_TABLE_HEADER_DEF /* ACPI common table header */ >> + uint32_t node_count; >> + uint32_t node_offset; >> + uint32_t reserved; >> +} QEMU_PACKED; >> +typedef struct AcpiIortTable AcpiIortTable; >> + >> +/* >> + * IORT node types >> + */ >> + >> +#define ACPI_IORT_NODE_HEADER_DEF /* Node format common fields */ \ >> + uint8_t type; \ >> + uint16_t length; \ >> + uint8_t revision; \ >> + uint32_t reserved; \ >> + uint32_t mapping_count; \ >> + uint32_t mapping_offset;\ > > Last line shouldn't have the \ OK > >> + >> +/* Values for node Type above */ >> +enum { >> + ACPI_IORT_NODE_ITS_GROUP = 0x00, >> + ACPI_IORT_NODE_NAMED_COMPONENT = 0x01, >> + ACPI_IORT_NODE_PCI_ROOT_COMPLEX = 0x02, >> + ACPI_IORT_NODE_SMMU = 0x03, >> + ACPI_IORT_NODE_SMMU_V3 = 0x04 >> +}; >> + >> +struct AcpiIortIdMapping { >> + uint32_t input_base; >> + uint32_t id_count; >> + uint32_t output_base; >> + uint32_t output_reference; >> + uint32_t flags; >> +} QEMU_PACKED; >> +typedef struct AcpiIortIdMapping AcpiIortIdMapping; >> + >> +struct AcpiIortMemoryAccess { >> + uint32_t cache_coherency; >> + uint8_t hints; >> + uint16_t reserved; >> + uint8_t memory_flags; >> +} QEMU_PACKED; >> +typedef struct AcpiIortMemoryAccess AcpiIortMemoryAccess; >> + >> +struct AcpiIortItsGroup { >> + ACPI_IORT_NODE_HEADER_DEF >> + uint32_t its_count; >> + uint32_t identifiers[0]; >> +} QEMU_PACKED; >> +typedef struct AcpiIortItsGroup AcpiIortItsGroup; >> + >> +struct AcpiIortRC { >> + ACPI_IORT_NODE_HEADER_DEF >> + AcpiIortMemoryAccess memory_properties; >> + uint32_t ats_attribute; >> + uint32_t pci_segment_number; > > I think just 'pci_segment' like in a couple other structs, is > a descriptive enough name, i.e. _number could be dropped. In the past Shannon told me to use the same field names as the ones in the linux header, hence that choice. Thanks Eric > >> + AcpiIortIdMapping id_mapping_array[0]; >> +} QEMU_PACKED; >> +typedef struct AcpiIortRC AcpiIortRC; >> + >> #endif >> -- >> 2.5.5 >> > > Thanks, > drew >