ACK: [PATCH][v2] acpi: update MADT GICC Structure and related tests

[Colin Ian King]

On 22/07/15 14:17, Alex Hung wrote:
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/acpidump/acpidump.c | 58 ++++++++++++++++++++++++++++++++------------
>  src/acpi/madt/madt.c         | 12 +++++++++
>  src/lib/include/fwts_acpi.h  |  2 ++
>  3 files changed, 56 insertions(+), 16 deletions(-)
> 
> diff --git a/src/acpi/acpidump/acpidump.c b/src/acpi/acpidump/acpidump.c
> index 9ff28cb..2bd2ce5 100644
> --- a/src/acpi/acpidump/acpidump.c
> +++ b/src/acpi/acpidump/acpidump.c
> @@ -953,22 +953,48 @@ static void acpidump_madt(fwts_framework *fw, const fwts_acpi_table_info *table)
>  			}
>  			break;
>  		case FWTS_ACPI_MADT_GIC_C_CPU_INTERFACE: {
> -				static const fwts_acpidump_field fields_madt_gic[] = {
> -					FIELD_UINT("  Reserved", 	fwts_acpi_madt_gic, reserved),
> -					FIELD_UINT("  GIC ID", 		fwts_acpi_madt_gic, gic_id),
> -					FIELD_UINT("  Processor UID", 	fwts_acpi_madt_gic, processor_uid),
> -					FIELD_UINT("  Flags", 		fwts_acpi_madt_gic, flags),
> -					FIELD_UINT("  Parking Protocol",fwts_acpi_madt_gic, parking_protocol_version),
> -					FIELD_UINT("  Perf. Int. GSIV",	fwts_acpi_madt_gic, performance_interrupt_gsiv),
> -					FIELD_UINT("  Parked Address",	fwts_acpi_madt_gic, parked_address),
> -					FIELD_UINT("  Phys. Base. Addr",fwts_acpi_madt_gic, physical_base_address),
> -					FIELD_UINT("  GICV",		fwts_acpi_madt_gic, gicv),
> -					FIELD_UINT("  GICH",		fwts_acpi_madt_gic, gich),
> -					FIELD_UINT("  VGIC Interrupt",	fwts_acpi_madt_gic, vgic),
> -					FIELD_UINT("  GICR Base. Addr", fwts_acpi_madt_gic, gicr_base_address),
> -					FIELD_UINT("  MPIDR",		fwts_acpi_madt_gic, mpidr),
> -				};
> -				__acpi_dump_table_fields(fw, data, fields_madt_gic, offset);
> +				/* ACPI 5.1 */
> +				if (table->length == 76) {
> +					static const fwts_acpidump_field fields_madt_gic[] = {
> +						FIELD_UINT("  Reserved", 	fwts_acpi_madt_gic, reserved),
> +						FIELD_UINT("  GIC ID", 		fwts_acpi_madt_gic, gic_id),
> +						FIELD_UINT("  Processor UID", 	fwts_acpi_madt_gic, processor_uid),
> +						FIELD_UINT("  Flags", 		fwts_acpi_madt_gic, flags),
> +						FIELD_UINT("  Parking Protocol",fwts_acpi_madt_gic, parking_protocol_version),
> +						FIELD_UINT("  Perf. Int. GSIV",	fwts_acpi_madt_gic, performance_interrupt_gsiv),
> +						FIELD_UINT("  Parked Address",	fwts_acpi_madt_gic, parked_address),
> +						FIELD_UINT("  Phys. Base. Addr",fwts_acpi_madt_gic, physical_base_address),
> +						FIELD_UINT("  GICV",		fwts_acpi_madt_gic, gicv),
> +						FIELD_UINT("  GICH",		fwts_acpi_madt_gic, gich),
> +						FIELD_UINT("  VGIC Interrupt",	fwts_acpi_madt_gic, vgic),
> +						FIELD_UINT("  GICR Base. Addr", fwts_acpi_madt_gic, gicr_base_address),
> +						FIELD_UINT("  MPIDR",		fwts_acpi_madt_gic, mpidr),
> +					};
> +					__acpi_dump_table_fields(fw, data, fields_madt_gic, offset);
> +				}
> +
> +				/* ACPI 6.0 */
> +				if (table->length == 80) {
> +					static const fwts_acpidump_field fields_madt_gic[] = {
> +						FIELD_UINT("  Reserved", 	fwts_acpi_madt_gic, reserved),
> +						FIELD_UINT("  GIC ID", 		fwts_acpi_madt_gic, gic_id),
> +						FIELD_UINT("  Processor UID", 	fwts_acpi_madt_gic, processor_uid),
> +						FIELD_UINT("  Flags", 		fwts_acpi_madt_gic, flags),
> +						FIELD_UINT("  Parking Protocol",fwts_acpi_madt_gic, parking_protocol_version),
> +						FIELD_UINT("  Perf. Int. GSIV",	fwts_acpi_madt_gic, performance_interrupt_gsiv),
> +						FIELD_UINT("  Parked Address",	fwts_acpi_madt_gic, parked_address),
> +						FIELD_UINT("  Phys. Base. Addr",fwts_acpi_madt_gic, physical_base_address),
> +						FIELD_UINT("  GICV",		fwts_acpi_madt_gic, gicv),
> +						FIELD_UINT("  GICH",		fwts_acpi_madt_gic, gich),
> +						FIELD_UINT("  VGIC Interrupt",	fwts_acpi_madt_gic, vgic),
> +						FIELD_UINT("  GICR Base. Addr", fwts_acpi_madt_gic, gicr_base_address),
> +						FIELD_UINT("  MPIDR",		fwts_acpi_madt_gic, mpidr),
> +						FIELD_UINT("  Processor Power Efficiency Class",	fwts_acpi_madt_gic, processor_power_efficiency_class),
> +						FIELD_UINTS("  Reserved", 	fwts_acpi_madt_gic, reserved2),
> +					};
> +					__acpi_dump_table_fields(fw, data, fields_madt_gic, offset);
> +				}
> +
>  				skip = sizeof(fwts_acpi_madt_gic);
>  			}
>  			break;
> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
> index 8afceb6..f776745 100644
> --- a/src/acpi/madt/madt.c
> +++ b/src/acpi/madt/madt.c
> @@ -235,6 +235,18 @@ static int madt_test1(fwts_framework *fw)
>  						gic->flags & 0xfffffffc);
>  				}
>  				skip = sizeof(fwts_acpi_madt_gic);
> +
> +				// new in ACPI 6.0
> +				if (table->length == 80) {
> +					uint32_t gic_reserve2 = gic->reserved2[0] + (gic->reserved2[1] << 4) + (gic->reserved2[2] << 8);
> +					if (gic_reserve2) {
> +						passed = false;
> +						fwts_failed(fw, LOG_LEVEL_LOW,
> +							"MADTGICCReserved2NonZero",
> +							"MADT GICC Structure second reserved field should be zero, "
> +							"instead got 0x%" PRIx32 ".", gic_reserve2);
> +					}
> +				}
>  			}
>  			break;
>  		case FWTS_ACPI_MADT_GIC_D_GOC_DISTRIBUTOR: {
> diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
> index e78af1c..37cc879 100644
> --- a/src/lib/include/fwts_acpi.h
> +++ b/src/lib/include/fwts_acpi.h
> @@ -472,6 +472,8 @@ typedef struct {
>  	uint32_t	vgic;
>  	uint64_t	gicr_base_address;
>  	uint64_t	mpidr;
> +	uint8_t		processor_power_efficiency_class;
> +	uint8_t		reserved2[3];
>  } __attribute__ ((packed)) fwts_acpi_madt_gic;
>  
>  /* New in ACPI 5.0, GICD, section 5.2.12.15 */
> 
Thanks Alex, apologies for being so pedantic/petty.

Acked-by: Colin Ian King <colin.king at canonical.com>