| Message ID | 1476435295-21885-2-git-send-email-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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 >
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
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
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 >
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