[v2,1/2] ACPI: Add IORT Structure definition
diff mbox

Message ID 1476435295-21885-2-git-send-email-eric.auger@redhat.com
State New
Headers show

Commit Message

Auger Eric Oct. 14, 2016, 8:54 a.m. UTC
From: Prem Mallappa <prem.mallappa@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.mallappa@broadcom.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

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(+)

Comments

Andrew Jones Oct. 14, 2016, 11:30 a.m. UTC | #1
On Fri, Oct 14, 2016 at 10:54:54AM +0200, Eric Auger wrote:
> From: Prem Mallappa <prem.mallappa@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.mallappa@broadcom.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> 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 \

> +
> +/* 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.

> +    AcpiIortIdMapping id_mapping_array[0];
> +} QEMU_PACKED;
> +typedef struct AcpiIortRC AcpiIortRC;
> +
>  #endif
> -- 
> 2.5.5
>

Thanks,
drew
Auger Eric Oct. 14, 2016, 11:59 a.m. UTC | #2
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 <prem.mallappa@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.mallappa@broadcom.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> 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 
>
Igor Mammedov Oct. 14, 2016, 12:42 p.m. UTC | #3
On Fri, 14 Oct 2016 10:54:54 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> From: Prem Mallappa <prem.mallappa@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.mallappa@broadcom.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Preferred way to build/pack ACPI tables info is to use
build_append_foo() functions.
That way you won't have to care about endianness avoiding
mistakes at assignment time. Also you won't need to declare
intermediate structures to do packing and will have more
compact code (only a single function) and better documented
at that if every build_append_foo() is accompanied by
comment matching field description from ACPI spec.
Pls see build_amd_iommu() as example.

If you redo this series that way it will become single patch.

> 
> ---
> 
> 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;\
> +
> +/* 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;
> +    AcpiIortIdMapping id_mapping_array[0];
> +} QEMU_PACKED;
> +typedef struct AcpiIortRC AcpiIortRC;
> +
>  #endif
Andrew Jones Oct. 14, 2016, 1:32 p.m. UTC | #4
On Fri, Oct 14, 2016 at 02:42:18PM +0200, Igor Mammedov wrote:
> On Fri, 14 Oct 2016 10:54:54 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
> 
> > From: Prem Mallappa <prem.mallappa@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.mallappa@broadcom.com>
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> Preferred way to build/pack ACPI tables info is to use
> build_append_foo() functions.

We've raised this before with Shannon, but it was already too late
for most tables. Now all tables in hw/arm/virt-acpi-build.c are
done in the style of this series, and I'd prefer we keep the style
consistent. If I ever get some free time to experiment then I might
try rewriting all of them in the build_append_ style to see how it
looks though.

Thanks,
drew
Auger Eric Oct. 14, 2016, 2:03 p.m. UTC | #5
Hi Igor,

On 14/10/2016 15:32, Andrew Jones wrote:
> On Fri, Oct 14, 2016 at 02:42:18PM +0200, Igor Mammedov wrote:
>> On Fri, 14 Oct 2016 10:54:54 +0200
>> Eric Auger <eric.auger@redhat.com> wrote:
>>
>>> From: Prem Mallappa <prem.mallappa@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.mallappa@broadcom.com>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> Preferred way to build/pack ACPI tables info is to use
>> build_append_foo() functions.
> 
> We've raised this before with Shannon, but it was already too late
> for most tables. Now all tables in hw/arm/virt-acpi-build.c are
> done in the style of this series, and I'd prefer we keep the style
> consistent. If I ever get some free time to experiment then I might
> try rewriting all of them in the build_append_ style to see how it
> looks though.

Thank you for the info. Well so let's keep it as is if nobody objects.

Thanks

Eric
> 
> Thanks,
> drew
>

Patch
diff mbox

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;\
+
+/* 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;
+    AcpiIortIdMapping id_mapping_array[0];
+} QEMU_PACKED;
+typedef struct AcpiIortRC AcpiIortRC;
+
 #endif