[PATCH 1/2] acpi: Add SPCR ACPI table check (LP: #1433604)

Colin Ian King colin.king at canonical.com
Thu Apr 2 09:13:28 UTC 2015


On 02/04/15 10:00, Alex Hung wrote:
> On 04/02/2015 12:12 AM, Colin King wrote:
>> From: Colin Ian King <colin.king at canonical.com>
>>
>> Signed-off-by: Colin Ian King <colin.king at canonical.com>
>> ---
>>  src/Makefile.am      |   1 +
>>  src/acpi/spcr/spcr.c | 292 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 293 insertions(+)
>>  create mode 100644 src/acpi/spcr/spcr.c
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 875b91f..62c63ef 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -51,6 +51,7 @@ fwts_SOURCES = main.c 				\
>>  	acpi/s3/s3.c 				\
>>  	acpi/s3power/s3power.c 			\
>>  	acpi/s4/s4.c 				\
>> +	acpi/spcr/spcr.c 			\
>>  	acpi/syntaxcheck/syntaxcheck.c 		\
>>  	acpi/wakealarm/wakealarm.c 		\
>>  	acpi/wmi/wmi.c 				\
>> diff --git a/src/acpi/spcr/spcr.c b/src/acpi/spcr/spcr.c
>> new file mode 100644
>> index 0000000..4bcbd7a
>> --- /dev/null
>> +++ b/src/acpi/spcr/spcr.c
>> @@ -0,0 +1,292 @@
>> +/*
>> + * Copyright (C) 2015 Canonical
>> + *
>> + * Portions of this code original from the Linux-ready Firmware Developer Kit
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
>> + *
>> + */
>> +#include "fwts.h"
>> +
>> +#ifdef FWTS_ARCH_INTEL
> 
> This table seems to apply to non-Intel arch as some fields are ARM-related.

Yep, cut-n-paste error. Thanks for seeing that one.

> 
>> +
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <sys/io.h>
>> +#include <unistd.h>
>> +#include <inttypes.h>
>> +#include <string.h>
>> +
>> +static const fwts_acpi_table_spcr *spcr;
>> +
>> +static int spcr_init(fwts_framework *fw)
>> +{
>> +	fwts_acpi_table_info *table;
>> +
>> +	if (fwts_acpi_find_table(fw, "SPCR", 0, &table) != FWTS_OK) {
>> +		fwts_log_error(fw, "Cannot read ACPI tables.");
>> +		return FWTS_ERROR;
>> +	}
>> +	if (table == NULL || (table && table->length == 0)) {
>> +		fwts_log_error(fw, "ACPI SPCR table does not exist, skipping test");
>> +		return FWTS_SKIP;
>> +	}
>> +	spcr = (const fwts_acpi_table_spcr*)table->data;
>> +
>> +	return FWTS_OK;
>> +}
>> +
>> +/*
>> + *  For SPCR and serial port types refer to:
>> + *	https://msdn.microsoft.com/en-gb/library/windows/hardware/dn639132%28v=vs.85%29.aspx
>> + *	https://msdn.microsoft.com/en-us/library/windows/hardware/dn639131%28v=vs.85%29.aspx
>> + */
>> +static int spcr_test1(fwts_framework *fw)
>> +{
>> +	char *str;
>> +	bool reserved = false;
>> +	bool not_pci = false;
>> +
>> +	/* Assuming revision 2 */
>> +	switch (spcr->interface_type) {
>> +	case 0x00:
>> +		str = "16550 compatible";
>> +		break;
>> +	case 0x01:
>> +		str = "16450 compatible";
>> +		break;
>> +	case 0x03:
>> +		str = "ARM PL011 UART";
>> +		break;
>> +	case 0x02:
>> +	case 0x04 ... 0x09:
>> +		str = "Reserved (Do not Use)";
>> +		reserved = true;
>> +		break;
>> +	default:
>> +		str = "Reserved";
>> +		reserved = true;
>> +		break;
>> +	}
>> +
>> +	fwts_log_info_verbatum(fw, "Serial Interface: %s", str);
>> +	if (reserved) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRInterfaceReserved",
>> +			"SPCR Serial interface type 0x%2.2" PRIx8
>> +			"is a reserved interface", spcr->interface_type);
>> +	}
>> +
>> +	if ((spcr->reserved1[0] |
>> +	     spcr->reserved1[1] |
>> +	     spcr->reserved1[2])) {
>> +		fwts_failed(fw, LOG_LEVEL_LOW,
>> +			"SPCRReservedNonZero",
>> +			"SPCR reserved field must be zero, got "
>> +			"0x%2.2" PRIx8 "%2.2" PRIx8 "%2.2" PRIx8 " instead",
>> +				spcr->reserved1[0],
>> +				spcr->reserved1[1],
>> +				spcr->reserved1[2]);
>> +	}
>> +
>> +	if (spcr->interrupt_type == 0) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRUnknownInterruptType",
>> +			"SPCR interrupt type field is zero, expecting support bits to be set");
>> +	}
>> +	if (spcr->interrupt_type & 0xf0) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRIllegalReservedInterruptType",
>> +			"SPCR interrupt type reserved bits are non-zero zero, got 0x%" PRIx8,
>> +				spcr->interrupt_type);
>> +	}
>> +
>> +	/* Check PC-AT compatible UART IRQs */
>> +	if (spcr->interrupt_type & 1) {
>> +		switch (spcr->irq) {
>> +		case  2 ...  7:
>> +		case  9 ... 12:
>> +		case 14 ... 15:
>> +			break;
>> +		default:
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"SPCRIllegalIRQ",
>> +				"SPCR PC-AT compatible IRQ 0x%" PRIx8 " is invalid", spcr->irq);
>> +			break;
>> +		}
>> +	}
>> +
>> +	reserved = false;
>> +	switch (spcr->baud_rate) {
>> +	case 0x03:
>> +		str = "9600";
>> +		break;
>> +	case 0x04:
>> +		str = "19200";
>> +		break;
>> +	case 0x06:
>> +		str = "57600";
>> +		break;
>> +	case 0x07:
>> +		str = "115200";
>> +		break;
>> +	default:
>> +		str = "Reserved";
>> +		reserved = true;
>> +	}
>> +	fwts_log_info_verbatum(fw, "Baud Rate:        %s", str);
>> +	if (reserved) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRBaudRateReserved",
>> +			"SPCR Serial baud rate type 0x%2.2" PRIx8
>> +			"is a reserved baud rate", spcr->baud_rate);
>> +	}
>> +
>> +	if (spcr->parity & 0xfe) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRParityReserved",
>> +			"SPCR parity reserved bits 1..7 are non-zero, value is 0x%2.2" PRIx8,
>> +			spcr->parity);
>> +	}
> 
> Should we also check stop bits as below:
> 
> if (spcr->stop_bits != 1) {
> 	fwts_failed(fw, LOG_LEVEL_HIGH,
> 		"SPCRStopbitsReserved",
> 		....
> }

Good point. I'll do that.

> 
>> +
>> +	if (spcr->flow_control & 0xfe) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRFlowControlReserved",
>> +			"SPCR flow control reserved bits 1..7 are non-zero, value is 0x%2.2" PRIx8,
>> +			spcr->flow_control);
>> +	}
>> +
>> +	reserved = false;
>> +	switch (spcr->terminal_type) {
>> +	case 0x00:
>> +		str = "VT100";
>> +		break;
>> +	case 0x01:
>> +		str = "VT100+";
>> +		break;
>> +	case 0x02:
>> +		str = "VT-UTF8";
>> +		break;
>> +	case 0x03:
>> +		str = "ANSI";
>> +		break;
>> +	default:
>> +		str = "Reserved";
>> +		reserved = true;
>> +	}
>> +	fwts_log_info_verbatum(fw, "Terminal Type:    %s", str);
>> +	if (reserved) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRTerminalTypeReserved",
>> +			"SPCR terminal type type 0x%2.2" PRIx8
>> +			"is a reserved terminal type", spcr->terminal_type);
>> +	}
>> +
>> +	if (spcr->reserved2) {
>> +		fwts_failed(fw, LOG_LEVEL_LOW,
>> +			"SPCRReservedNonZero",
>> +			"SPCR reserved field must be zero, got "
>> +			"0x%2.2" PRIx8 " instead", spcr->reserved2);
>> +	}
>> +
>> +	/* According to the spec, any of these values indicate NOT a PCI device */
>> +	if ((spcr->pci_device_id == 0xffff) ||
>> +	    (spcr->pci_vendor_id == 0xffff) ||
>> +	    (spcr->pci_bus_number == 0) ||
>> +	    (spcr->pci_device_number == 0) ||
>> +	    (spcr->pci_function_number == 0) ||
>> +	    ((spcr->pci_flags & 1) == 0))
>> +		not_pci = true;
> 
> I think the SPCR spec says the other way around - if it is not a PCI
> device, all of above needs to be true. Since it seems below also checks
> all of them, we can probably just check pci_device_id & pci_vendor_id -
> either of them cannot be 0xffff according to PCI standard and pci_flags
> acccording to SPCR spec. The 0 value of bus/device/function is valid in
> PCI spec.

OK - so I'll do the following:

	pci = true;
	if ((spcr->pci_device_id == 0xffff) &&
	    (spcr->pci_vendor_id == 0xffff) &&
	    (spcr->pci_bus_number == 0)
		...
		pci = false;

	/* Now check all PCI specific fields */
	if (pci) {
		if (spcr->pci_device_id == 0xffff) {
			... fail..
		}

> 
> 
>> +
>> +	/* Now validate all pci specific fields if not-PCI enabled */
>> +	if (not_pci) {
>> +		if (spcr->pci_device_id != 0xffff) {
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"SPCRPciDeviceID",
>> +				"SPCR PCI device ID is 0x%4.4" PRIx16
>> +				", expecting 0xffff for non-PCI device",
>> +				spcr->pci_device_id);
>> +		}
>> +		if (spcr->pci_vendor_id != 0xffff) {
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"SPCRPciVendorID",
>> +				"SPCR PCI vendor ID is 0x%4.4" PRIx16
>> +				", expecting 0xffff for non-PCI device",
>> +				spcr->pci_vendor_id);
>> +		}
>> +		if (spcr->pci_bus_number != 0x00) {
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"SPCRPciBusNumber",
>> +				"SPCR PCI bus number is 0x%2.2" PRIx8
>> +				", expecting 0x00 for non-PCI device",
>> +				spcr->pci_bus_number);
>> +		}
>> +		if (spcr->pci_device_number != 0x00) {
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"SPCRPciDeviceNumber",
>> +				"SPCR PCI device number is 0x%2.2" PRIx8
>> +				", expecting 0x00 for non-PCI device",
>> +				spcr->pci_device_number);
>> +		}
>> +		if (spcr->pci_function_number != 0x00) {
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"SPCRPciFunctionNumber",
>> +				"SPCR PCI function number is 0x%2.2" PRIx8
>> +				", expecting 0x00 for non-PCI device",
>> +				spcr->pci_function_number);
>> +		}
>> +		if ((spcr->pci_flags & 1) != 0) {
>> +			fwts_failed(fw, LOG_LEVEL_HIGH,
>> +				"SPCRPciFlagsBit0",
>> +				"SPCR PCI flags compatibility bit 0 is %" PRIx32
>> +				", expecting 0 for non-PCI device",
>> +				spcr->pci_flags & 1);
>> +		}
>> +	}
>> +
>> +	if (spcr->pci_flags & 0xfffe) {
>> +		fwts_failed(fw, LOG_LEVEL_HIGH,
>> +			"SPCRPciFlags",
>> +			"SPCR PCI flags reserved bits 1-31 are %" PRIx32
>> +			", expecting 0",
>> +			spcr->pci_flags);
>> +	}
>> +
>> +	if (spcr->reserved3) {
>> +		fwts_failed(fw, LOG_LEVEL_LOW,
>> +			"SPCRReservedNonZero",
>> +			"SPCR reserved field must be zero, got "
>> +			"0x%2.2" PRIx8 " instead", spcr->reserved3);
>> +	}
>> +
>> +	return FWTS_OK;
>> +}
>> +
>> +static fwts_framework_minor_test spcr_tests[] = {
>> +	{ spcr_test1, "SPCR Serial Port Console Redirection Table test." },
>> +	{ NULL, NULL }
>> +};
>> +
>> +static fwts_framework_ops spcr_ops = {
>> +	.description = "SPCR Serial Port Console Redirection Table test.",
>> +	.init        = spcr_init,
>> +	.minor_tests = spcr_tests
>> +};
>> +
>> +FWTS_REGISTER("spcr", &spcr_ops, FWTS_TEST_ANYTIME, FWTS_FLAG_BATCH | FWTS_FLAG_ROOT_PRIV)
>> +
>> +#endif
>>
> 
> 




More information about the fwts-devel mailing list