public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
@ 2017-04-13 16:37 Ard Biesheuvel
  2017-04-18 10:07 ` Alexei Fedorov
  2017-04-18 17:56 ` Ard Biesheuvel
  0 siblings, 2 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2017-04-13 16:37 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm; +Cc: liming.gao, lorenzo.pieralisi, Ard Biesheuvel

This adds #defines and struct typedefs for the various node types in
the ACPI 6.0 IO Remapping Table (IORT).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Include/IndustryStandard/IoRemappingTable.h | 187 ++++++++++++++++++++
 1 file changed, 187 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
new file mode 100644
index 000000000000..674cb611961d
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
@@ -0,0 +1,187 @@
+/** @file
+  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049B
+
+  Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#ifndef __IO_REMAPPING_TABLE_H__
+#define __IO_REMAPPING_TABLE_H__
+
+#include <IndustryStandard/Acpi.h>
+
+#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION        0x0
+
+#define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
+#define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
+#define EFI_ACPI_IORT_TYPE_ROOT_COMPLEX             0x2
+#define EFI_ACPI_IORT_TYPE_SMMUv1v2                 0x3
+#define EFI_ACPI_IORT_TYPE_SMMUv3                   0x4
+
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA           BIT0
+
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_TR         BIT0
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_WA         BIT1
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_RA         BIT2
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_AHO        BIT3
+
+#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM          BIT0
+#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS         BIT1
+
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v1             0x0
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v2             0x1
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU400         0x2
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500         0x3
+
+#define EFI_ACPI_IORT_SMMUv1v2_FLAG_DVM             BIT0
+#define EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK        BIT1
+
+#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL       0x0
+#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_EDGE        0x1
+
+#define EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE    BIT0
+#define EFI_ACPI_IORT_SMMUv3_FLAG_HTTU_OVERRIDE     BIT1
+
+#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
+#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
+
+#define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
+
+#pragma pack(1)
+
+//
+// Table header
+//
+typedef struct {
+  EFI_ACPI_DESCRIPTION_HEADER             Header;
+  INT32                                   NumNodes;
+  INT32                                   NodeOffset;
+  INT32                                   Reserved;
+} EFI_ACPI_6_0_IO_REMAPPING_TABLE;
+
+//
+// Definition for ID mapping table shared by all node types
+//
+typedef struct {
+  UINT32                                  InputBase;
+  UINT32                                  NumIds;
+  UINT32                                  OutputBase;
+  UINT32                                  OutputReference;
+  UINT32                                  Flags;
+} EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE;
+
+//
+// Header definition shared by all node types
+//
+typedef struct {
+  UINT8                                   Type;
+  UINT16                                  Length;
+  UINT8                                   Revision;
+  UINT32                                  Reserved;
+  UINT32                                  NumIdMappings;
+  UINT32                                  IdReference;
+} EFI_ACPI_6_0_IO_REMAPPING_NODE;
+
+//
+// Node type 0: ITS node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT32                                  NumIts;
+/*
+  UINT32                                  ItsIdentifiers[0];
+*/
+} EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
+
+//
+// Node type 1: root complex node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT32                                  CacheCoherent;
+  UINT8                                   AllocationHints;
+  UINT16                                  Reserved;
+  UINT8                                   MemoryAccessFlags;
+
+  UINT32                                  AtsAttribute;
+  UINT32                                  PciSegmentNumber;
+} EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
+
+//
+// Node type 2: named component node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT32                                  Flags;
+  UINT32                                  CacheCoherent;
+  UINT8                                   AllocationHints;
+  UINT16                                  Reserved;
+  UINT8                                   MemoryAccessFlags;
+  UINT8                                   AddressSizeLimit;
+/*
+  CHAR8                                   ObjectName[0];
+*/
+} EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE;
+
+//
+// Node type 3: SMMUv1 or SMMUv2 node
+//
+typedef struct {
+  UINT32                                  Interrupt;
+  UINT32                                  InterruptFlags;
+} EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT;
+
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT64                                  Base;
+  UINT64                                  Span;
+  UINT32                                  Model;
+  UINT32                                  Flags;
+  UINT32                                  GlobalInterruptArrayRef;
+  UINT32                                  NumContextInterrupts;
+  UINT32                                  ContextInterruptArrayRef;
+  UINT32                                  NumPmuInterrupts;
+  UINT32                                  PmuInterruptArrayRef;
+
+  UINT32                                  SMMU_NSgIrpt;
+  UINT32                                  SMMU_NSgIrptFlags;
+  UINT32                                  SMMU_NSgCfgIrpt;
+  UINT32                                  SMMU_NSgCfgIrptFlags;
+
+/*
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  ContextInterrupt[0];
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  PmuInterrupt[0];
+*/
+} EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE;
+
+//
+// Node type 4: SMMUv4 node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT64                                  Base;
+  UINT32                                  Flags;
+  UINT32                                  Reserved;
+  UINT64                                  VatosAddress;
+  UINT32                                  Model;
+  UINT32                                  Event;
+  UINT32                                  Pri;
+  UINT32                                  Gerr;
+  UINT32                                  Sync;
+} EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
+
+#pragma pack()
+
+#endif
-- 
2.9.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-13 16:37 [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT Ard Biesheuvel
@ 2017-04-18 10:07 ` Alexei Fedorov
  2017-04-18 10:13   ` Ard Biesheuvel
  2017-04-18 17:56 ` Ard Biesheuvel
  1 sibling, 1 reply; 10+ messages in thread
From: Alexei Fedorov @ 2017-04-18 10:07 UTC (permalink / raw)
  To: ard.biesheuvel@linaro.org, edk2-devel@lists.01.org,
	leif.lindholm@linaro.org
  Cc: Lorenzo Pieralisi, liming.gao@intel.com, Evan Lloyd


Aren't comments like

//
// Table header
//

,

//
// Definition for ID mapping table shared by all node types
//

etc.  not allowed according to EDK II C Coding Standards Specification?

6.2.3 Avoid comments where the opening comment characters are alone on a line.

//

// VIOLATION: Horror Vacui

//



________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Ard Biesheuvel <ard.biesheuvel@linaro.org>
Sent: 13 April 2017 17:37
To: edk2-devel@lists.01.org; leif.lindholm@linaro.org
Cc: Lorenzo Pieralisi; liming.gao@intel.com; ard.biesheuvel@linaro.org
Subject: [edk2] [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT

This adds #defines and struct typedefs for the various node types in
the ACPI 6.0 IO Remapping Table (IORT).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Include/IndustryStandard/IoRemappingTable.h | 187 ++++++++++++++++++++
 1 file changed, 187 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
new file mode 100644
index 000000000000..674cb611961d
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
@@ -0,0 +1,187 @@
+/** @file
+  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049B
+
+  Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#ifndef __IO_REMAPPING_TABLE_H__
+#define __IO_REMAPPING_TABLE_H__
+
+#include <IndustryStandard/Acpi.h>
+
+#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION        0x0
+
+#define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
+#define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
+#define EFI_ACPI_IORT_TYPE_ROOT_COMPLEX             0x2
+#define EFI_ACPI_IORT_TYPE_SMMUv1v2                 0x3
+#define EFI_ACPI_IORT_TYPE_SMMUv3                   0x4
+
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA           BIT0
+
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_TR         BIT0
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_WA         BIT1
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_RA         BIT2
+#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_AHO        BIT3
+
+#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM          BIT0
+#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS         BIT1
+
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v1             0x0
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v2             0x1
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU400         0x2
+#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500         0x3
+
+#define EFI_ACPI_IORT_SMMUv1v2_FLAG_DVM             BIT0
+#define EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK        BIT1
+
+#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL       0x0
+#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_EDGE        0x1
+
+#define EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE    BIT0
+#define EFI_ACPI_IORT_SMMUv3_FLAG_HTTU_OVERRIDE     BIT1
+
+#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
+#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
+
+#define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
+
+#pragma pack(1)
+
+//
+// Table header
+//
+typedef struct {
+  EFI_ACPI_DESCRIPTION_HEADER             Header;
+  INT32                                   NumNodes;
+  INT32                                   NodeOffset;
+  INT32                                   Reserved;
+} EFI_ACPI_6_0_IO_REMAPPING_TABLE;
+
+//
+// Definition for ID mapping table shared by all node types
+//
+typedef struct {
+  UINT32                                  InputBase;
+  UINT32                                  NumIds;
+  UINT32                                  OutputBase;
+  UINT32                                  OutputReference;
+  UINT32                                  Flags;
+} EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE;
+
+//
+// Header definition shared by all node types
+//
+typedef struct {
+  UINT8                                   Type;
+  UINT16                                  Length;
+  UINT8                                   Revision;
+  UINT32                                  Reserved;
+  UINT32                                  NumIdMappings;
+  UINT32                                  IdReference;
+} EFI_ACPI_6_0_IO_REMAPPING_NODE;
+
+//
+// Node type 0: ITS node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT32                                  NumIts;
+/*
+  UINT32                                  ItsIdentifiers[0];
+*/
+} EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
+
+//
+// Node type 1: root complex node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT32                                  CacheCoherent;
+  UINT8                                   AllocationHints;
+  UINT16                                  Reserved;
+  UINT8                                   MemoryAccessFlags;
+
+  UINT32                                  AtsAttribute;
+  UINT32                                  PciSegmentNumber;
+} EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
+
+//
+// Node type 2: named component node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT32                                  Flags;
+  UINT32                                  CacheCoherent;
+  UINT8                                   AllocationHints;
+  UINT16                                  Reserved;
+  UINT8                                   MemoryAccessFlags;
+  UINT8                                   AddressSizeLimit;
+/*
+  CHAR8                                   ObjectName[0];
+*/
+} EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE;
+
+//
+// Node type 3: SMMUv1 or SMMUv2 node
+//
+typedef struct {
+  UINT32                                  Interrupt;
+  UINT32                                  InterruptFlags;
+} EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT;
+
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT64                                  Base;
+  UINT64                                  Span;
+  UINT32                                  Model;
+  UINT32                                  Flags;
+  UINT32                                  GlobalInterruptArrayRef;
+  UINT32                                  NumContextInterrupts;
+  UINT32                                  ContextInterruptArrayRef;
+  UINT32                                  NumPmuInterrupts;
+  UINT32                                  PmuInterruptArrayRef;
+
+  UINT32                                  SMMU_NSgIrpt;
+  UINT32                                  SMMU_NSgIrptFlags;
+  UINT32                                  SMMU_NSgCfgIrpt;
+  UINT32                                  SMMU_NSgCfgIrptFlags;
+
+/*
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  ContextInterrupt[0];
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  PmuInterrupt[0];
+*/
+} EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE;
+
+//
+// Node type 4: SMMUv4 node
+//
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
+
+  UINT64                                  Base;
+  UINT32                                  Flags;
+  UINT32                                  Reserved;
+  UINT64                                  VatosAddress;
+  UINT32                                  Model;
+  UINT32                                  Event;
+  UINT32                                  Pri;
+  UINT32                                  Gerr;
+  UINT32                                  Sync;
+} EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
+
+#pragma pack()
+
+#endif
--
2.9.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-18 10:07 ` Alexei Fedorov
@ 2017-04-18 10:13   ` Ard Biesheuvel
  2017-04-18 11:01     ` Alexei Fedorov
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-04-18 10:13 UTC (permalink / raw)
  To: Alexei Fedorov, Kinney, Michael D, liming.gao@intel.com,
	leif.lindholm@linaro.org
  Cc: edk2-devel@lists.01.org, Lorenzo Pieralisi, Evan Lloyd

On 18 April 2017 at 11:07, Alexei Fedorov <Alexei.Fedorov@arm.com> wrote:
>
> Aren't comments like
>
> //
> // Table header
> //
>
> ,
>
> //
> // Definition for ID mapping table shared by all node types
> //
>
> etc.  not allowed according to EDK II C Coding Standards Specification?
>
> 6.2.3 Avoid comments where the opening comment characters are alone on a
> line.
>
> //
>
> // VIOLATION: Horror Vacui
>
> //
>

You must be joking

$ git grep -E '/\*\*\s$' MdePkg/ MdeModulePkg/ |wc -l
16193

IOW, there are more than 16000 occurrences of opening comment
characters alone on a line in MdePkg+MdeModulePkg only


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-18 10:13   ` Ard Biesheuvel
@ 2017-04-18 11:01     ` Alexei Fedorov
  2017-04-18 11:11       ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Alexei Fedorov @ 2017-04-18 11:01 UTC (permalink / raw)
  To: ard.biesheuvel@linaro.org, Kinney, Michael D,
	liming.gao@intel.com, leif.lindholm@linaro.org
  Cc: edk2-devel@lists.01.org, Lorenzo Pieralisi, Evan Lloyd,
	Mitch Ishihara, Sami Mujawar, Girish Pathak

So, should we follow the Coding Standards Specification or can just ditch it?


-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: 18 April 2017 11:13
To: Alexei Fedorov <Alexei.Fedorov@arm.com>; Kinney, Michael D <michael.d.kinney@intel.com>; liming.gao@intel.com; leif.lindholm@linaro.org
Cc: edk2-devel@lists.01.org; Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>; Evan Lloyd <Evan.Lloyd@arm.com>
Subject: Re: [edk2] [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT

On 18 April 2017 at 11:07, Alexei Fedorov <Alexei.Fedorov@arm.com> wrote:
>
> Aren't comments like
>
> //
> // Table header
> //
>
> ,
>
> //
> // Definition for ID mapping table shared by all node types //
>
> etc.  not allowed according to EDK II C Coding Standards Specification?
>
> 6.2.3 Avoid comments where the opening comment characters are alone on
> a line.
>
> //
>
> // VIOLATION: Horror Vacui
>
> //
>

You must be joking

$ git grep -E '/\*\*\s$' MdePkg/ MdeModulePkg/ |wc -l
16193

IOW, there are more than 16000 occurrences of opening comment characters alone on a line in MdePkg+MdeModulePkg only
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-18 11:01     ` Alexei Fedorov
@ 2017-04-18 11:11       ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2017-04-18 11:11 UTC (permalink / raw)
  To: Alexei Fedorov
  Cc: Kinney, Michael D, liming.gao@intel.com, leif.lindholm@linaro.org,
	edk2-devel@lists.01.org, Lorenzo Pieralisi, Evan Lloyd,
	Mitch Ishihara, Sami Mujawar, Girish Pathak

On 18 April 2017 at 12:01, Alexei Fedorov <Alexei.Fedorov@arm.com> wrote:
> So, should we follow the Coding Standards Specification or can just ditch it?
>

We all know that Tianocore/EDK2 has a steep learning curve, especially
when it comes to developers coming from the Linux/open source side.

This page

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications

lists 11 (!) different specs one must adhere to when contributing code
to EDK2. Some of them are mandatory or your code will not build, or
will build but will not run.

In my opinion, adhering to the specs is already difficult enough
without having to follow arbitrary rules which are so blatantly
violated in core areas of the tree (MdePkg and MdeModulePkg). So my
advice is simply to ignore the coding standard altogether, and in my
reviews, I am usually very lax about violations.

-- 
Ard.





>
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: 18 April 2017 11:13
> To: Alexei Fedorov <Alexei.Fedorov@arm.com>; Kinney, Michael D <michael.d.kinney@intel.com>; liming.gao@intel.com; leif.lindholm@linaro.org
> Cc: edk2-devel@lists.01.org; Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>; Evan Lloyd <Evan.Lloyd@arm.com>
> Subject: Re: [edk2] [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
>
> On 18 April 2017 at 11:07, Alexei Fedorov <Alexei.Fedorov@arm.com> wrote:
>>
>> Aren't comments like
>>
>> //
>> // Table header
>> //
>>
>> ,
>>
>> //
>> // Definition for ID mapping table shared by all node types //
>>
>> etc.  not allowed according to EDK II C Coding Standards Specification?
>>
>> 6.2.3 Avoid comments where the opening comment characters are alone on
>> a line.
>>
>> //
>>
>> // VIOLATION: Horror Vacui
>>
>> //
>>
>
> You must be joking
>
> $ git grep -E '/\*\*\s$' MdePkg/ MdeModulePkg/ |wc -l
> 16193
>
> IOW, there are more than 16000 occurrences of opening comment characters alone on a line in MdePkg+MdeModulePkg only
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-13 16:37 [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT Ard Biesheuvel
  2017-04-18 10:07 ` Alexei Fedorov
@ 2017-04-18 17:56 ` Ard Biesheuvel
  2017-04-18 20:54   ` Kinney, Michael D
  1 sibling, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-04-18 17:56 UTC (permalink / raw)
  To: edk2-devel@lists.01.org, Leif Lindholm, Kinney, Michael D,
	Gao, Liming
  Cc: Lorenzo Pieralisi, Ard Biesheuvel

On 13 April 2017 at 17:37, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> This adds #defines and struct typedefs for the various node types in
> the ACPI 6.0 IO Remapping Table (IORT).
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 187 ++++++++++++++++++++
>  1 file changed, 187 insertions(+)
>

Liming, Michael,

Are there any concerns with this patch?

Thanks,
Ard.


> diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> new file mode 100644
> index 000000000000..674cb611961d
> --- /dev/null
> +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> @@ -0,0 +1,187 @@
> +/** @file
> +  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049B
> +
> +  Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#ifndef __IO_REMAPPING_TABLE_H__
> +#define __IO_REMAPPING_TABLE_H__
> +
> +#include <IndustryStandard/Acpi.h>
> +
> +#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION        0x0
> +
> +#define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
> +#define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
> +#define EFI_ACPI_IORT_TYPE_ROOT_COMPLEX             0x2
> +#define EFI_ACPI_IORT_TYPE_SMMUv1v2                 0x3
> +#define EFI_ACPI_IORT_TYPE_SMMUv3                   0x4
> +
> +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA           BIT0
> +
> +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_TR         BIT0
> +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_WA         BIT1
> +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_RA         BIT2
> +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_AHO        BIT3
> +
> +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM          BIT0
> +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS         BIT1
> +
> +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v1             0x0
> +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v2             0x1
> +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU400         0x2
> +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500         0x3
> +
> +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_DVM             BIT0
> +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK        BIT1
> +
> +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL       0x0
> +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_EDGE        0x1
> +
> +#define EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE    BIT0
> +#define EFI_ACPI_IORT_SMMUv3_FLAG_HTTU_OVERRIDE     BIT1
> +
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
> +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
> +
> +#define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
> +
> +#pragma pack(1)
> +
> +//
> +// Table header
> +//
> +typedef struct {
> +  EFI_ACPI_DESCRIPTION_HEADER             Header;
> +  INT32                                   NumNodes;
> +  INT32                                   NodeOffset;
> +  INT32                                   Reserved;
> +} EFI_ACPI_6_0_IO_REMAPPING_TABLE;
> +
> +//
> +// Definition for ID mapping table shared by all node types
> +//
> +typedef struct {
> +  UINT32                                  InputBase;
> +  UINT32                                  NumIds;
> +  UINT32                                  OutputBase;
> +  UINT32                                  OutputReference;
> +  UINT32                                  Flags;
> +} EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE;
> +
> +//
> +// Header definition shared by all node types
> +//
> +typedef struct {
> +  UINT8                                   Type;
> +  UINT16                                  Length;
> +  UINT8                                   Revision;
> +  UINT32                                  Reserved;
> +  UINT32                                  NumIdMappings;
> +  UINT32                                  IdReference;
> +} EFI_ACPI_6_0_IO_REMAPPING_NODE;
> +
> +//
> +// Node type 0: ITS node
> +//
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> +
> +  UINT32                                  NumIts;
> +/*
> +  UINT32                                  ItsIdentifiers[0];
> +*/
> +} EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
> +
> +//
> +// Node type 1: root complex node
> +//
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> +
> +  UINT32                                  CacheCoherent;
> +  UINT8                                   AllocationHints;
> +  UINT16                                  Reserved;
> +  UINT8                                   MemoryAccessFlags;
> +
> +  UINT32                                  AtsAttribute;
> +  UINT32                                  PciSegmentNumber;
> +} EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
> +
> +//
> +// Node type 2: named component node
> +//
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> +
> +  UINT32                                  Flags;
> +  UINT32                                  CacheCoherent;
> +  UINT8                                   AllocationHints;
> +  UINT16                                  Reserved;
> +  UINT8                                   MemoryAccessFlags;
> +  UINT8                                   AddressSizeLimit;
> +/*
> +  CHAR8                                   ObjectName[0];
> +*/
> +} EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE;
> +
> +//
> +// Node type 3: SMMUv1 or SMMUv2 node
> +//
> +typedef struct {
> +  UINT32                                  Interrupt;
> +  UINT32                                  InterruptFlags;
> +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT;
> +
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> +
> +  UINT64                                  Base;
> +  UINT64                                  Span;
> +  UINT32                                  Model;
> +  UINT32                                  Flags;
> +  UINT32                                  GlobalInterruptArrayRef;
> +  UINT32                                  NumContextInterrupts;
> +  UINT32                                  ContextInterruptArrayRef;
> +  UINT32                                  NumPmuInterrupts;
> +  UINT32                                  PmuInterruptArrayRef;
> +
> +  UINT32                                  SMMU_NSgIrpt;
> +  UINT32                                  SMMU_NSgIrptFlags;
> +  UINT32                                  SMMU_NSgCfgIrpt;
> +  UINT32                                  SMMU_NSgCfgIrptFlags;
> +
> +/*
> +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  ContextInterrupt[0];
> +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  PmuInterrupt[0];
> +*/
> +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE;
> +
> +//
> +// Node type 4: SMMUv4 node
> +//
> +typedef struct {
> +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> +
> +  UINT64                                  Base;
> +  UINT32                                  Flags;
> +  UINT32                                  Reserved;
> +  UINT64                                  VatosAddress;
> +  UINT32                                  Model;
> +  UINT32                                  Event;
> +  UINT32                                  Pri;
> +  UINT32                                  Gerr;
> +  UINT32                                  Sync;
> +} EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
> +
> +#pragma pack()
> +
> +#endif
> --
> 2.9.3
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-18 17:56 ` Ard Biesheuvel
@ 2017-04-18 20:54   ` Kinney, Michael D
  2017-04-18 22:21     ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Kinney, Michael D @ 2017-04-18 20:54 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org, Leif Lindholm,
	Gao, Liming, Kinney, Michael D
  Cc: Lorenzo Pieralisi

Ard,

1) Please add the link to the document in the file header
2) Why are there commented out /* */ fields in 2 of the structures?
3) Why isn’t there a #define for the table signature?

Thanks,

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, April 18, 2017 10:57 AM
> To: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
> 
> On 13 April 2017 at 17:37, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > This adds #defines and struct typedefs for the various node types in
> > the ACPI 6.0 IO Remapping Table (IORT).
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 187 ++++++++++++++++++++
> >  1 file changed, 187 insertions(+)
> >
> 
> Liming, Michael,
> 
> Are there any concerns with this patch?
> 
> Thanks,
> Ard.
> 
> 
> > diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> > new file mode 100644
> > index 000000000000..674cb611961d
> > --- /dev/null
> > +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> > @@ -0,0 +1,187 @@
> > +/** @file
> > +  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049B
> > +
> > +  Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD
> License
> > +  which accompanies this distribution.  The full text of the license may be
> found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> > +**/
> > +
> > +#ifndef __IO_REMAPPING_TABLE_H__
> > +#define __IO_REMAPPING_TABLE_H__
> > +
> > +#include <IndustryStandard/Acpi.h>
> > +
> > +#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION        0x0
> > +
> > +#define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
> > +#define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
> > +#define EFI_ACPI_IORT_TYPE_ROOT_COMPLEX             0x2
> > +#define EFI_ACPI_IORT_TYPE_SMMUv1v2                 0x3
> > +#define EFI_ACPI_IORT_TYPE_SMMUv3                   0x4
> > +
> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA           BIT0
> > +
> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_TR         BIT0
> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_WA         BIT1
> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_RA         BIT2
> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_AHO        BIT3
> > +
> > +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM          BIT0
> > +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS         BIT1
> > +
> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v1             0x0
> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v2             0x1
> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU400         0x2
> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500         0x3
> > +
> > +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_DVM             BIT0
> > +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK        BIT1
> > +
> > +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL       0x0
> > +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_EDGE        0x1
> > +
> > +#define EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE    BIT0
> > +#define EFI_ACPI_IORT_SMMUv3_FLAG_HTTU_OVERRIDE     BIT1
> > +
> > +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
> > +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
> > +
> > +#define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
> > +
> > +#pragma pack(1)
> > +
> > +//
> > +// Table header
> > +//
> > +typedef struct {
> > +  EFI_ACPI_DESCRIPTION_HEADER             Header;
> > +  INT32                                   NumNodes;
> > +  INT32                                   NodeOffset;
> > +  INT32                                   Reserved;
> > +} EFI_ACPI_6_0_IO_REMAPPING_TABLE;
> > +
> > +//
> > +// Definition for ID mapping table shared by all node types
> > +//
> > +typedef struct {
> > +  UINT32                                  InputBase;
> > +  UINT32                                  NumIds;
> > +  UINT32                                  OutputBase;
> > +  UINT32                                  OutputReference;
> > +  UINT32                                  Flags;
> > +} EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE;
> > +
> > +//
> > +// Header definition shared by all node types
> > +//
> > +typedef struct {
> > +  UINT8                                   Type;
> > +  UINT16                                  Length;
> > +  UINT8                                   Revision;
> > +  UINT32                                  Reserved;
> > +  UINT32                                  NumIdMappings;
> > +  UINT32                                  IdReference;
> > +} EFI_ACPI_6_0_IO_REMAPPING_NODE;
> > +
> > +//
> > +// Node type 0: ITS node
> > +//
> > +typedef struct {
> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> > +
> > +  UINT32                                  NumIts;
> > +/*
> > +  UINT32                                  ItsIdentifiers[0];
> > +*/
> > +} EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
> > +
> > +//
> > +// Node type 1: root complex node
> > +//
> > +typedef struct {
> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> > +
> > +  UINT32                                  CacheCoherent;
> > +  UINT8                                   AllocationHints;
> > +  UINT16                                  Reserved;
> > +  UINT8                                   MemoryAccessFlags;
> > +
> > +  UINT32                                  AtsAttribute;
> > +  UINT32                                  PciSegmentNumber;
> > +} EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
> > +
> > +//
> > +// Node type 2: named component node
> > +//
> > +typedef struct {
> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> > +
> > +  UINT32                                  Flags;
> > +  UINT32                                  CacheCoherent;
> > +  UINT8                                   AllocationHints;
> > +  UINT16                                  Reserved;
> > +  UINT8                                   MemoryAccessFlags;
> > +  UINT8                                   AddressSizeLimit;
> > +/*
> > +  CHAR8                                   ObjectName[0];
> > +*/
> > +} EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE;
> > +
> > +//
> > +// Node type 3: SMMUv1 or SMMUv2 node
> > +//
> > +typedef struct {
> > +  UINT32                                  Interrupt;
> > +  UINT32                                  InterruptFlags;
> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT;
> > +
> > +typedef struct {
> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> > +
> > +  UINT64                                  Base;
> > +  UINT64                                  Span;
> > +  UINT32                                  Model;
> > +  UINT32                                  Flags;
> > +  UINT32                                  GlobalInterruptArrayRef;
> > +  UINT32                                  NumContextInterrupts;
> > +  UINT32                                  ContextInterruptArrayRef;
> > +  UINT32                                  NumPmuInterrupts;
> > +  UINT32                                  PmuInterruptArrayRef;
> > +
> > +  UINT32                                  SMMU_NSgIrpt;
> > +  UINT32                                  SMMU_NSgIrptFlags;
> > +  UINT32                                  SMMU_NSgCfgIrpt;
> > +  UINT32                                  SMMU_NSgCfgIrptFlags;
> > +
> > +/*
> > +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  ContextInterrupt[0];
> > +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  PmuInterrupt[0];
> > +*/
> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE;
> > +
> > +//
> > +// Node type 4: SMMUv4 node
> > +//
> > +typedef struct {
> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> > +
> > +  UINT64                                  Base;
> > +  UINT32                                  Flags;
> > +  UINT32                                  Reserved;
> > +  UINT64                                  VatosAddress;
> > +  UINT32                                  Model;
> > +  UINT32                                  Event;
> > +  UINT32                                  Pri;
> > +  UINT32                                  Gerr;
> > +  UINT32                                  Sync;
> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
> > +
> > +#pragma pack()
> > +
> > +#endif
> > --
> > 2.9.3
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-18 20:54   ` Kinney, Michael D
@ 2017-04-18 22:21     ` Ard Biesheuvel
  2017-04-18 23:51       ` Kinney, Michael D
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-04-18 22:21 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Gao, Liming,
	Lorenzo Pieralisi

On 18 April 2017 at 21:54, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
> Ard,
>
> 1) Please add the link to the document in the file header

OK

> 2) Why are there commented out /* */ fields in 2 of the structures?

Those are subtables/subfields that may legally occur 0 or more times,
so they should not be accounted for in the size of the respective
structures. So they are essentially for documentation purposes only,
and when used in .aslc code, the table should be composed using an
ad-hoc struct typedef that concatenates them together.

> 3) Why isn’t there a #define for the table signature?
>

OK, I will add that.

-- 
Ard.


>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Tuesday, April 18, 2017 10:57 AM
>> To: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
>> Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>
>> Subject: Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
>>
>> On 13 April 2017 at 17:37, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > This adds #defines and struct typedefs for the various node types in
>> > the ACPI 6.0 IO Remapping Table (IORT).
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > ---
>> >  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 187 ++++++++++++++++++++
>> >  1 file changed, 187 insertions(+)
>> >
>>
>> Liming, Michael,
>>
>> Are there any concerns with this patch?
>>
>> Thanks,
>> Ard.
>>
>>
>> > diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
>> b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
>> > new file mode 100644
>> > index 000000000000..674cb611961d
>> > --- /dev/null
>> > +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
>> > @@ -0,0 +1,187 @@
>> > +/** @file
>> > +  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049B
>> > +
>> > +  Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
>> > +
>> > +  This program and the accompanying materials
>> > +  are licensed and made available under the terms and conditions of the BSD
>> License
>> > +  which accompanies this distribution.  The full text of the license may be
>> found at
>> > +  http://opensource.org/licenses/bsd-license.php
>> > +
>> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>> IMPLIED.
>> > +**/
>> > +
>> > +#ifndef __IO_REMAPPING_TABLE_H__
>> > +#define __IO_REMAPPING_TABLE_H__
>> > +
>> > +#include <IndustryStandard/Acpi.h>
>> > +
>> > +#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION        0x0
>> > +
>> > +#define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
>> > +#define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
>> > +#define EFI_ACPI_IORT_TYPE_ROOT_COMPLEX             0x2
>> > +#define EFI_ACPI_IORT_TYPE_SMMUv1v2                 0x3
>> > +#define EFI_ACPI_IORT_TYPE_SMMUv3                   0x4
>> > +
>> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA           BIT0
>> > +
>> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_TR         BIT0
>> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_WA         BIT1
>> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_RA         BIT2
>> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_AHO        BIT3
>> > +
>> > +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM          BIT0
>> > +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS         BIT1
>> > +
>> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v1             0x0
>> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v2             0x1
>> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU400         0x2
>> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500         0x3
>> > +
>> > +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_DVM             BIT0
>> > +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK        BIT1
>> > +
>> > +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL       0x0
>> > +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_EDGE        0x1
>> > +
>> > +#define EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE    BIT0
>> > +#define EFI_ACPI_IORT_SMMUv3_FLAG_HTTU_OVERRIDE     BIT1
>> > +
>> > +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
>> > +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
>> > +
>> > +#define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
>> > +
>> > +#pragma pack(1)
>> > +
>> > +//
>> > +// Table header
>> > +//
>> > +typedef struct {
>> > +  EFI_ACPI_DESCRIPTION_HEADER             Header;
>> > +  INT32                                   NumNodes;
>> > +  INT32                                   NodeOffset;
>> > +  INT32                                   Reserved;
>> > +} EFI_ACPI_6_0_IO_REMAPPING_TABLE;
>> > +
>> > +//
>> > +// Definition for ID mapping table shared by all node types
>> > +//
>> > +typedef struct {
>> > +  UINT32                                  InputBase;
>> > +  UINT32                                  NumIds;
>> > +  UINT32                                  OutputBase;
>> > +  UINT32                                  OutputReference;
>> > +  UINT32                                  Flags;
>> > +} EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE;
>> > +
>> > +//
>> > +// Header definition shared by all node types
>> > +//
>> > +typedef struct {
>> > +  UINT8                                   Type;
>> > +  UINT16                                  Length;
>> > +  UINT8                                   Revision;
>> > +  UINT32                                  Reserved;
>> > +  UINT32                                  NumIdMappings;
>> > +  UINT32                                  IdReference;
>> > +} EFI_ACPI_6_0_IO_REMAPPING_NODE;
>> > +
>> > +//
>> > +// Node type 0: ITS node
>> > +//
>> > +typedef struct {
>> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
>> > +
>> > +  UINT32                                  NumIts;
>> > +/*
>> > +  UINT32                                  ItsIdentifiers[0];
>> > +*/
>> > +} EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
>> > +
>> > +//
>> > +// Node type 1: root complex node
>> > +//
>> > +typedef struct {
>> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
>> > +
>> > +  UINT32                                  CacheCoherent;
>> > +  UINT8                                   AllocationHints;
>> > +  UINT16                                  Reserved;
>> > +  UINT8                                   MemoryAccessFlags;
>> > +
>> > +  UINT32                                  AtsAttribute;
>> > +  UINT32                                  PciSegmentNumber;
>> > +} EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
>> > +
>> > +//
>> > +// Node type 2: named component node
>> > +//
>> > +typedef struct {
>> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
>> > +
>> > +  UINT32                                  Flags;
>> > +  UINT32                                  CacheCoherent;
>> > +  UINT8                                   AllocationHints;
>> > +  UINT16                                  Reserved;
>> > +  UINT8                                   MemoryAccessFlags;
>> > +  UINT8                                   AddressSizeLimit;
>> > +/*
>> > +  CHAR8                                   ObjectName[0];
>> > +*/
>> > +} EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE;
>> > +
>> > +//
>> > +// Node type 3: SMMUv1 or SMMUv2 node
>> > +//
>> > +typedef struct {
>> > +  UINT32                                  Interrupt;
>> > +  UINT32                                  InterruptFlags;
>> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT;
>> > +
>> > +typedef struct {
>> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
>> > +
>> > +  UINT64                                  Base;
>> > +  UINT64                                  Span;
>> > +  UINT32                                  Model;
>> > +  UINT32                                  Flags;
>> > +  UINT32                                  GlobalInterruptArrayRef;
>> > +  UINT32                                  NumContextInterrupts;
>> > +  UINT32                                  ContextInterruptArrayRef;
>> > +  UINT32                                  NumPmuInterrupts;
>> > +  UINT32                                  PmuInterruptArrayRef;
>> > +
>> > +  UINT32                                  SMMU_NSgIrpt;
>> > +  UINT32                                  SMMU_NSgIrptFlags;
>> > +  UINT32                                  SMMU_NSgCfgIrpt;
>> > +  UINT32                                  SMMU_NSgCfgIrptFlags;
>> > +
>> > +/*
>> > +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  ContextInterrupt[0];
>> > +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  PmuInterrupt[0];
>> > +*/
>> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE;
>> > +
>> > +//
>> > +// Node type 4: SMMUv4 node
>> > +//
>> > +typedef struct {
>> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
>> > +
>> > +  UINT64                                  Base;
>> > +  UINT32                                  Flags;
>> > +  UINT32                                  Reserved;
>> > +  UINT64                                  VatosAddress;
>> > +  UINT32                                  Model;
>> > +  UINT32                                  Event;
>> > +  UINT32                                  Pri;
>> > +  UINT32                                  Gerr;
>> > +  UINT32                                  Sync;
>> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
>> > +
>> > +#pragma pack()
>> > +
>> > +#endif
>> > --
>> > 2.9.3
>> >


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-18 22:21     ` Ard Biesheuvel
@ 2017-04-18 23:51       ` Kinney, Michael D
  2017-04-19  7:03         ` Ard Biesheuvel
  0 siblings, 1 reply; 10+ messages in thread
From: Kinney, Michael D @ 2017-04-18 23:51 UTC (permalink / raw)
  To: Ard Biesheuvel, Kinney, Michael D
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Gao, Liming,
	Lorenzo Pieralisi

Ard,

I found IORT signature defined in Acpi60.h and Acpi61.h, so no 
changes required there.

  ///
  /// "IORT" I/O Remapping Table
  ///
  #define EFI_ACPI_6_1_IO_REMAPPING_TABLE_SIGNATURE  SIGNATURE_32('I', 'O', 'R', 'T')

I do see some other structures in Acpixx.h that have commented out fields
for the same reason you provided, so the commented out fields look ok.
Acpixx.h uses // style comments instead of /* */ style.

Are any of these structures defined in the ACPI 6.0 Spec?  Or are they
only defined in the ARM spec.  If any of them are in the ACPI 6.0 Spec,
then those should go into the Acpi60.h and Acpi61.h files.

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, April 18, 2017 3:21 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Gao,
> Liming <liming.gao@intel.com>; Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Subject: Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
> 
> On 18 April 2017 at 21:54, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
> > Ard,
> >
> > 1) Please add the link to the document in the file header
> 
> OK
> 
> > 2) Why are there commented out /* */ fields in 2 of the structures?
> 
> Those are subtables/subfields that may legally occur 0 or more times,
> so they should not be accounted for in the size of the respective
> structures. So they are essentially for documentation purposes only,
> and when used in .aslc code, the table should be composed using an
> ad-hoc struct typedef that concatenates them together.
> 
> > 3) Why isn’t there a #define for the table signature?
> >
> 
> OK, I will add that.
> 
> --
> Ard.
> 
> 
> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: Tuesday, April 18, 2017 10:57 AM
> >> To: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
> >> Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
> >> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Ard Biesheuvel
> >> <ard.biesheuvel@linaro.org>
> >> Subject: Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0
> IORT
> >>
> >> On 13 April 2017 at 17:37, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> > This adds #defines and struct typedefs for the various node types in
> >> > the ACPI 6.0 IO Remapping Table (IORT).
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> > ---
> >> >  MdePkg/Include/IndustryStandard/IoRemappingTable.h | 187
> ++++++++++++++++++++
> >> >  1 file changed, 187 insertions(+)
> >> >
> >>
> >> Liming, Michael,
> >>
> >> Are there any concerns with this patch?
> >>
> >> Thanks,
> >> Ard.
> >>
> >>
> >> > diff --git a/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> >> b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> >> > new file mode 100644
> >> > index 000000000000..674cb611961d
> >> > --- /dev/null
> >> > +++ b/MdePkg/Include/IndustryStandard/IoRemappingTable.h
> >> > @@ -0,0 +1,187 @@
> >> > +/** @file
> >> > +  ACPI IO Remapping Table (IORT) as specified in ARM spec DEN0049B
> >> > +
> >> > +  Copyright (c) 2017, Linaro Limited. All rights reserved.<BR>
> >> > +
> >> > +  This program and the accompanying materials
> >> > +  are licensed and made available under the terms and conditions of the BSD
> >> License
> >> > +  which accompanies this distribution.  The full text of the license may be
> >> found at
> >> > +  http://opensource.org/licenses/bsd-license.php
> >> > +
> >> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> >> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> >> IMPLIED.
> >> > +**/
> >> > +
> >> > +#ifndef __IO_REMAPPING_TABLE_H__
> >> > +#define __IO_REMAPPING_TABLE_H__
> >> > +
> >> > +#include <IndustryStandard/Acpi.h>
> >> > +
> >> > +#define EFI_ACPI_IO_REMAPPING_TABLE_REVISION        0x0
> >> > +
> >> > +#define EFI_ACPI_IORT_TYPE_ITS_GROUP                0x0
> >> > +#define EFI_ACPI_IORT_TYPE_NAMED_COMP               0x1
> >> > +#define EFI_ACPI_IORT_TYPE_ROOT_COMPLEX             0x2
> >> > +#define EFI_ACPI_IORT_TYPE_SMMUv1v2                 0x3
> >> > +#define EFI_ACPI_IORT_TYPE_SMMUv3                   0x4
> >> > +
> >> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA           BIT0
> >> > +
> >> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_TR         BIT0
> >> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_WA         BIT1
> >> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_RA         BIT2
> >> > +#define EFI_ACPI_IORT_MEM_ACCESS_PROP_AH_AHO        BIT3
> >> > +
> >> > +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM          BIT0
> >> > +#define EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS         BIT1
> >> > +
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v1             0x0
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_v2             0x1
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU400         0x2
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_MODEL_MMU500         0x3
> >> > +
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_DVM             BIT0
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_FLAG_COH_WALK        BIT1
> >> > +
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_LEVEL       0x0
> >> > +#define EFI_ACPI_IORT_SMMUv1v2_INT_FLAG_EDGE        0x1
> >> > +
> >> > +#define EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE    BIT0
> >> > +#define EFI_ACPI_IORT_SMMUv3_FLAG_HTTU_OVERRIDE     BIT1
> >> > +
> >> > +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED  0x0
> >> > +#define EFI_ACPI_IORT_ROOT_COMPLEX_ATS_SUPPORTED    0x1
> >> > +
> >> > +#define EFI_ACPI_IORT_ID_MAPPING_FLAGS_SINGLE       BIT0
> >> > +
> >> > +#pragma pack(1)
> >> > +
> >> > +//
> >> > +// Table header
> >> > +//
> >> > +typedef struct {
> >> > +  EFI_ACPI_DESCRIPTION_HEADER             Header;
> >> > +  INT32                                   NumNodes;
> >> > +  INT32                                   NodeOffset;
> >> > +  INT32                                   Reserved;
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_TABLE;
> >> > +
> >> > +//
> >> > +// Definition for ID mapping table shared by all node types
> >> > +//
> >> > +typedef struct {
> >> > +  UINT32                                  InputBase;
> >> > +  UINT32                                  NumIds;
> >> > +  UINT32                                  OutputBase;
> >> > +  UINT32                                  OutputReference;
> >> > +  UINT32                                  Flags;
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE;
> >> > +
> >> > +//
> >> > +// Header definition shared by all node types
> >> > +//
> >> > +typedef struct {
> >> > +  UINT8                                   Type;
> >> > +  UINT16                                  Length;
> >> > +  UINT8                                   Revision;
> >> > +  UINT32                                  Reserved;
> >> > +  UINT32                                  NumIdMappings;
> >> > +  UINT32                                  IdReference;
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_NODE;
> >> > +
> >> > +//
> >> > +// Node type 0: ITS node
> >> > +//
> >> > +typedef struct {
> >> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> >> > +
> >> > +  UINT32                                  NumIts;
> >> > +/*
> >> > +  UINT32                                  ItsIdentifiers[0];
> >> > +*/
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
> >> > +
> >> > +//
> >> > +// Node type 1: root complex node
> >> > +//
> >> > +typedef struct {
> >> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> >> > +
> >> > +  UINT32                                  CacheCoherent;
> >> > +  UINT8                                   AllocationHints;
> >> > +  UINT16                                  Reserved;
> >> > +  UINT8                                   MemoryAccessFlags;
> >> > +
> >> > +  UINT32                                  AtsAttribute;
> >> > +  UINT32                                  PciSegmentNumber;
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
> >> > +
> >> > +//
> >> > +// Node type 2: named component node
> >> > +//
> >> > +typedef struct {
> >> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> >> > +
> >> > +  UINT32                                  Flags;
> >> > +  UINT32                                  CacheCoherent;
> >> > +  UINT8                                   AllocationHints;
> >> > +  UINT16                                  Reserved;
> >> > +  UINT8                                   MemoryAccessFlags;
> >> > +  UINT8                                   AddressSizeLimit;
> >> > +/*
> >> > +  CHAR8                                   ObjectName[0];
> >> > +*/
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_NAMED_COMP_NODE;
> >> > +
> >> > +//
> >> > +// Node type 3: SMMUv1 or SMMUv2 node
> >> > +//
> >> > +typedef struct {
> >> > +  UINT32                                  Interrupt;
> >> > +  UINT32                                  InterruptFlags;
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_INT;
> >> > +
> >> > +typedef struct {
> >> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> >> > +
> >> > +  UINT64                                  Base;
> >> > +  UINT64                                  Span;
> >> > +  UINT32                                  Model;
> >> > +  UINT32                                  Flags;
> >> > +  UINT32                                  GlobalInterruptArrayRef;
> >> > +  UINT32                                  NumContextInterrupts;
> >> > +  UINT32                                  ContextInterruptArrayRef;
> >> > +  UINT32                                  NumPmuInterrupts;
> >> > +  UINT32                                  PmuInterruptArrayRef;
> >> > +
> >> > +  UINT32                                  SMMU_NSgIrpt;
> >> > +  UINT32                                  SMMU_NSgIrptFlags;
> >> > +  UINT32                                  SMMU_NSgCfgIrpt;
> >> > +  UINT32                                  SMMU_NSgCfgIrptFlags;
> >> > +
> >> > +/*
> >> > +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  ContextInterrupt[0];
> >> > +  EFI_ACPI_6_0_IO_REMAPPING_SMMU_CTX_INT  PmuInterrupt[0];
> >> > +*/
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU_NODE;
> >> > +
> >> > +//
> >> > +// Node type 4: SMMUv4 node
> >> > +//
> >> > +typedef struct {
> >> > +  EFI_ACPI_6_0_IO_REMAPPING_NODE          Node;
> >> > +
> >> > +  UINT64                                  Base;
> >> > +  UINT32                                  Flags;
> >> > +  UINT32                                  Reserved;
> >> > +  UINT64                                  VatosAddress;
> >> > +  UINT32                                  Model;
> >> > +  UINT32                                  Event;
> >> > +  UINT32                                  Pri;
> >> > +  UINT32                                  Gerr;
> >> > +  UINT32                                  Sync;
> >> > +} EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
> >> > +
> >> > +#pragma pack()
> >> > +
> >> > +#endif
> >> > --
> >> > 2.9.3
> >> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT
  2017-04-18 23:51       ` Kinney, Michael D
@ 2017-04-19  7:03         ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2017-04-19  7:03 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Gao, Liming,
	Lorenzo Pieralisi

On 19 April 2017 at 00:51, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
> Ard,
>
> I found IORT signature defined in Acpi60.h and Acpi61.h, so no
> changes required there.
>
>   ///
>   /// "IORT" I/O Remapping Table
>   ///
>   #define EFI_ACPI_6_1_IO_REMAPPING_TABLE_SIGNATURE  SIGNATURE_32('I', 'O', 'R', 'T')
>

Ah yes, I did spot that but I didn't remember it when replying before.

> I do see some other structures in Acpixx.h that have commented out fields
> for the same reason you provided, so the commented out fields look ok.
> Acpixx.h uses // style comments instead of /* */ style.
>

OK, I will change to // style then -- I have no preference either way

> Are any of these structures defined in the ACPI 6.0 Spec?  Or are they
> only defined in the ARM spec.  If any of them are in the ACPI 6.0 Spec,
> then those should go into the Acpi60.h and Acpi61.h files.
>

No. The ACPI spec mentions IORT but only as a reserved table. The
structure is only defined in the DEN0049 ARM spec.

-- 
Ard.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-04-19  7:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-13 16:37 [PATCH] MdePkg/IndustryStandard: add definitions for ACPI 6.0 IORT Ard Biesheuvel
2017-04-18 10:07 ` Alexei Fedorov
2017-04-18 10:13   ` Ard Biesheuvel
2017-04-18 11:01     ` Alexei Fedorov
2017-04-18 11:11       ` Ard Biesheuvel
2017-04-18 17:56 ` Ard Biesheuvel
2017-04-18 20:54   ` Kinney, Michael D
2017-04-18 22:21     ` Ard Biesheuvel
2017-04-18 23:51       ` Kinney, Michael D
2017-04-19  7:03         ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox