On Thu, Oct 13, 2016 at 12:55:42PM +0200, Eric Auger wrote:
> From: Prem Mallappa <prem.malla...@broadcom.com>
>
> 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 <prem.malla...@broadcom.com>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
> ---
> include/hw/acpi/acpi-defs.h | 91
> +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 91 insertions(+)
>
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 9c1b7cb..9ad3c01 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -609,4 +609,95 @@ 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 subtables
s/subtables/Nodes/
> + */
> +
> +struct AcpiIortNode {
> + uint8_t type;
> + uint16_t length;
> + uint8_t revision;
> + uint32_t reserved;
> + uint32_t mapping_count;
> + uint32_t mapping_offset;
> +} QEMU_PACKED;
> +typedef struct AcpiIortNode AcpiIortNode;
The above should be a define like ACPI_TABLE_HEADER_DEF, as it's not
a unique node type. The fields are just common node header fields.
> +
> +/* Values for subtable Type above */
s/subtable/node/
> +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;
> +
> +/* Masks for Flags field above for IORT subtable */
> +#define ACPI_IORT_ID_SINGLE_MAPPING (1)
We don't need to introduce defines/enums for all flags. Sometimes
it makes the code easier to read, but sometimes it's just cruft.
Everything is already documented in the spec, so a comment at
assignment time is usually enough. See the SPCR generation for an
example of attempting to minimize a reproduction of the spec.
> +
> +struct AcpiIortMemoryAccess {
> + uint32_t cache_coherency;
> + uint8_t hints;
> + uint16_t reserved;
> + uint8_t memory_flags;
> +} QEMU_PACKED;
> +typedef struct AcpiIortMemoryAccess AcpiIortMemoryAccess;
> +
> +/* Values for cache_coherency field above */
> +#define ACPI_IORT_NODE_COHERENT 0x00000001
> +#define ACPI_IORT_NODE_NOT_COHERENT 0x00000000
I'd replace these defines with comments at assignment time.
> +
> +/* Masks for Hints field above */
> +#define ACPI_IORT_HT_TRANSIENT (1)
> +#define ACPI_IORT_HT_WRITE (1 << 1)
> +#define ACPI_IORT_HT_READ (1 << 2)
> +#define ACPI_IORT_HT_OVERRIDE (1 << 3)
I'd drop these, I see they're not used anyway.
> +
> +/* Masks for memory_flags field above */
> +#define ACPI_IORT_MF_COHERENCY (1)
> +#define ACPI_IORT_MF_ATTRIBUTES (1 << 1)
And these can go.
> +
> +/*
> + * IORT node specific subtables
> + */
No need for the above header, we're already under
/* IORT Nodes */
> +
> +struct AcpiIortItsGroup {
> + AcpiIortNode iort_node;
ACPI_IORT_NODE_HEADER_DEF
> + uint32_t its_count;
> + uint32_t identifiers[0];
> +} QEMU_PACKED;
> +typedef struct AcpiIortItsGroup AcpiIortItsGroup;
> +
> +struct AcpiIortRC {
> + AcpiIortNode iort_node;
ACPI_IORT_NODE_HEADER_DEF
> + AcpiIortMemoryAccess memory_properties;
> + uint32_t ats_attribute;
> + uint32_t pci_segment_number;
> + AcpiIortIdMapping id_mapping_array[0];
> +} QEMU_PACKED;
> +typedef struct AcpiIortRC AcpiIortRC;
> +
> #endif
> --
> 2.5.5
>
>
Thanks,
drew
