| 
							
							
							
						 |  |  | @ -1,240 +0,0 @@ | 
		
	
		
			
				|  |  |  |  | From 8f8027c5f935bf02bdc8806c109ddbb0e402283c Mon Sep 17 00:00:00 2001 | 
		
	
		
			
				|  |  |  |  | From: Al Stone <ahs3@redhat.com> | 
		
	
		
			
				|  |  |  |  | Date: Wed, 16 May 2018 16:01:41 -0600 | 
		
	
		
			
				|  |  |  |  | Subject: [PATCH] mailbox: PCC: erroneous error message when parsing ACPI PCCT | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | There have been multiple reports of the following error message: | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | [    0.068293] Error parsing PCC subspaces from PCCT | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | This error message is not correct.  In multiple cases examined, the PCCT | 
		
	
		
			
				|  |  |  |  | (Platform Communications Channel Table) concerned is actually properly | 
		
	
		
			
				|  |  |  |  | constructed; the problem is that acpi_pcc_probe() which reads the PCCT | 
		
	
		
			
				|  |  |  |  | is making the assumption that the only valid PCCT is one that contains | 
		
	
		
			
				|  |  |  |  | subtables of one of two types: ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE or | 
		
	
		
			
				|  |  |  |  | ACPI_PCCT_TYPE_HW_REDUCED_TYPE2.  The number of subtables of these | 
		
	
		
			
				|  |  |  |  | types are counted and as long as there is at least one of the desired | 
		
	
		
			
				|  |  |  |  | types, the acpi_pcc_probe() succeeds.  When no subtables of these types | 
		
	
		
			
				|  |  |  |  | are found, regardless of whether or not any other subtable types are | 
		
	
		
			
				|  |  |  |  | present, the error mentioned above is reported. | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | In the cases reported to me personally, the PCCT contains exactly one | 
		
	
		
			
				|  |  |  |  | subtable of type ACPI_PCCT_TYPE_GENERIC_SUBSPACE.  The function | 
		
	
		
			
				|  |  |  |  | acpi_pcc_probe() does not count it as a valid subtable, so believes | 
		
	
		
			
				|  |  |  |  | there to be no valid subtables, and hence outputs the error message. | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | An example of the PCCT being reported as erroneous yet perfectly fine | 
		
	
		
			
				|  |  |  |  | is the following: | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  |                     Signature : "PCCT" | 
		
	
		
			
				|  |  |  |  |                  Table Length : 0000006E | 
		
	
		
			
				|  |  |  |  |                      Revision : 05 | 
		
	
		
			
				|  |  |  |  |                      Checksum : A9 | 
		
	
		
			
				|  |  |  |  |                        Oem ID : "XXXXXX" | 
		
	
		
			
				|  |  |  |  |                  Oem Table ID : "XXXXX   " | 
		
	
		
			
				|  |  |  |  |                  Oem Revision : 00002280 | 
		
	
		
			
				|  |  |  |  |               Asl Compiler ID : "XXXX" | 
		
	
		
			
				|  |  |  |  |         Asl Compiler Revision : 00000002 | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  |         Flags (decoded below) : 00000001 | 
		
	
		
			
				|  |  |  |  |                      Platform : 1 | 
		
	
		
			
				|  |  |  |  |                      Reserved : 0000000000000000 | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  |                 Subtable Type : 00 [Generic Communications Subspace] | 
		
	
		
			
				|  |  |  |  |                        Length : 3E | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  |                      Reserved : 000000000000 | 
		
	
		
			
				|  |  |  |  |                  Base Address : 00000000DCE43018 | 
		
	
		
			
				|  |  |  |  |                Address Length : 0000000000001000 | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  |             Doorbell Register : [Generic Address Structure] | 
		
	
		
			
				|  |  |  |  |                      Space ID : 01 [SystemIO] | 
		
	
		
			
				|  |  |  |  |                     Bit Width : 08 | 
		
	
		
			
				|  |  |  |  |                    Bit Offset : 00 | 
		
	
		
			
				|  |  |  |  |          Encoded Access Width : 01 [Byte Access:8] | 
		
	
		
			
				|  |  |  |  |                       Address : 0000000000001842 | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  |                 Preserve Mask : 00000000000000FD | 
		
	
		
			
				|  |  |  |  |                    Write Mask : 0000000000000002 | 
		
	
		
			
				|  |  |  |  |               Command Latency : 00001388 | 
		
	
		
			
				|  |  |  |  |           Maximum Access Rate : 00000000 | 
		
	
		
			
				|  |  |  |  |       Minimum Turnaround Time : 0000 | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | To fix this, we count up all of the possible subtable types for the | 
		
	
		
			
				|  |  |  |  | PCCT, and only report an error when there are none (which could mean | 
		
	
		
			
				|  |  |  |  | either no subtables, or no valid subtables), or there are too many. | 
		
	
		
			
				|  |  |  |  | We also change the logic so that if there is a valid subtable, we | 
		
	
		
			
				|  |  |  |  | do try to initialize it per the PCCT subtable contents.  This is a | 
		
	
		
			
				|  |  |  |  | change in functionality; previously, the probe would have returned | 
		
	
		
			
				|  |  |  |  | right after the error message and would not have tried to use any | 
		
	
		
			
				|  |  |  |  | other subtable definition. | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | Tested on my personal laptop which showed the error previously; the | 
		
	
		
			
				|  |  |  |  | error message no longer appears and the laptop appears to operate | 
		
	
		
			
				|  |  |  |  | normally. | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | Signed-off-by: Al Stone <ahs3@redhat.com> | 
		
	
		
			
				|  |  |  |  | Reviewed-by: Prashanth Prakash <pprakash@codeaurora.org> | 
		
	
		
			
				|  |  |  |  | Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> | 
		
	
		
			
				|  |  |  |  | ---
 | 
		
	
		
			
				|  |  |  |  |  drivers/mailbox/pcc.c | 81 ++++++++++++++++++++++++--------------------------- | 
		
	
		
			
				|  |  |  |  |  1 file changed, 38 insertions(+), 43 deletions(-) | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  | diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
 | 
		
	
		
			
				|  |  |  |  | index 3ef7f036ceea..fc3c237daef2 100644
 | 
		
	
		
			
				|  |  |  |  | --- a/drivers/mailbox/pcc.c
 | 
		
	
		
			
				|  |  |  |  | +++ b/drivers/mailbox/pcc.c
 | 
		
	
		
			
				|  |  |  |  | @@ -373,33 +373,24 @@ static const struct mbox_chan_ops pcc_chan_ops = {
 | 
		
	
		
			
				|  |  |  |  |  }; | 
		
	
		
			
				|  |  |  |  | 
 | 
		
	
		
			
				|  |  |  |  |  /** | 
		
	
		
			
				|  |  |  |  | - * parse_pcc_subspace - Parse the PCC table and verify PCC subspace
 | 
		
	
		
			
				|  |  |  |  | - *		entries. There should be one entry per PCC client.
 | 
		
	
		
			
				|  |  |  |  | + * parse_pcc_subspaces -- Count PCC subspaces defined
 | 
		
	
		
			
				|  |  |  |  |   * @header: Pointer to the ACPI subtable header under the PCCT. | 
		
	
		
			
				|  |  |  |  |   * @end: End of subtable entry. | 
		
	
		
			
				|  |  |  |  |   * | 
		
	
		
			
				|  |  |  |  | - * Return: 0 for Success, else errno.
 | 
		
	
		
			
				|  |  |  |  | + * Return: If we find a PCC subspace entry of a valid type, return 0.
 | 
		
	
		
			
				|  |  |  |  | + *	Otherwise, return -EINVAL.
 | 
		
	
		
			
				|  |  |  |  |   * | 
		
	
		
			
				|  |  |  |  |   * This gets called for each entry in the PCC table. | 
		
	
		
			
				|  |  |  |  |   */ | 
		
	
		
			
				|  |  |  |  |  static int parse_pcc_subspace(struct acpi_subtable_header *header, | 
		
	
		
			
				|  |  |  |  |  		const unsigned long end) | 
		
	
		
			
				|  |  |  |  |  { | 
		
	
		
			
				|  |  |  |  | -	struct acpi_pcct_hw_reduced *pcct_ss;
 | 
		
	
		
			
				|  |  |  |  | -
 | 
		
	
		
			
				|  |  |  |  | -	if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
 | 
		
	
		
			
				|  |  |  |  | -		pcct_ss = (struct acpi_pcct_hw_reduced *) header;
 | 
		
	
		
			
				|  |  |  |  | +	struct acpi_pcct_subspace *ss = (struct acpi_pcct_subspace *) header;
 | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -		if ((pcct_ss->header.type !=
 | 
		
	
		
			
				|  |  |  |  | -				ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE)
 | 
		
	
		
			
				|  |  |  |  | -		    && (pcct_ss->header.type !=
 | 
		
	
		
			
				|  |  |  |  | -				ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2)) {
 | 
		
	
		
			
				|  |  |  |  | -			pr_err("Incorrect PCC Subspace type detected\n");
 | 
		
	
		
			
				|  |  |  |  | -			return -EINVAL;
 | 
		
	
		
			
				|  |  |  |  | -		}
 | 
		
	
		
			
				|  |  |  |  | -	}
 | 
		
	
		
			
				|  |  |  |  | +	if (ss->header.type < ACPI_PCCT_TYPE_RESERVED)
 | 
		
	
		
			
				|  |  |  |  | +		return 0;
 | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	return 0;
 | 
		
	
		
			
				|  |  |  |  | +	return -EINVAL;
 | 
		
	
		
			
				|  |  |  |  |  } | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  |  /** | 
		
	
		
			
				|  |  |  |  | @@ -449,8 +440,8 @@ static int __init acpi_pcc_probe(void)
 | 
		
	
		
			
				|  |  |  |  |  	struct acpi_table_header *pcct_tbl; | 
		
	
		
			
				|  |  |  |  |  	struct acpi_subtable_header *pcct_entry; | 
		
	
		
			
				|  |  |  |  |  	struct acpi_table_pcct *acpi_pcct_tbl; | 
		
	
		
			
				|  |  |  |  | +	struct acpi_subtable_proc proc[ACPI_PCCT_TYPE_RESERVED];
 | 
		
	
		
			
				|  |  |  |  |  	int count, i, rc; | 
		
	
		
			
				|  |  |  |  | -	int sum = 0;
 | 
		
	
		
			
				|  |  |  |  |  	acpi_status status = AE_OK; | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  |  	/* Search for PCCT */ | 
		
	
		
			
				|  |  |  |  | @@ -459,43 +450,41 @@ static int __init acpi_pcc_probe(void)
 | 
		
	
		
			
				|  |  |  |  |  	if (ACPI_FAILURE(status) || !pcct_tbl) | 
		
	
		
			
				|  |  |  |  |  		return -ENODEV; | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	count = acpi_table_parse_entries(ACPI_SIG_PCCT,
 | 
		
	
		
			
				|  |  |  |  | -			sizeof(struct acpi_table_pcct),
 | 
		
	
		
			
				|  |  |  |  | -			ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE,
 | 
		
	
		
			
				|  |  |  |  | -			parse_pcc_subspace, MAX_PCC_SUBSPACES);
 | 
		
	
		
			
				|  |  |  |  | -	sum += (count > 0) ? count : 0;
 | 
		
	
		
			
				|  |  |  |  | -
 | 
		
	
		
			
				|  |  |  |  | -	count = acpi_table_parse_entries(ACPI_SIG_PCCT,
 | 
		
	
		
			
				|  |  |  |  | -			sizeof(struct acpi_table_pcct),
 | 
		
	
		
			
				|  |  |  |  | -			ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2,
 | 
		
	
		
			
				|  |  |  |  | -			parse_pcc_subspace, MAX_PCC_SUBSPACES);
 | 
		
	
		
			
				|  |  |  |  | -	sum += (count > 0) ? count : 0;
 | 
		
	
		
			
				|  |  |  |  | +	/* Set up the subtable handlers */
 | 
		
	
		
			
				|  |  |  |  | +	for (i = ACPI_PCCT_TYPE_GENERIC_SUBSPACE;
 | 
		
	
		
			
				|  |  |  |  | +	     i < ACPI_PCCT_TYPE_RESERVED; i++) {
 | 
		
	
		
			
				|  |  |  |  | +		proc[i].id = i;
 | 
		
	
		
			
				|  |  |  |  | +		proc[i].count = 0;
 | 
		
	
		
			
				|  |  |  |  | +		proc[i].handler = parse_pcc_subspace;
 | 
		
	
		
			
				|  |  |  |  | +	}
 | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	if (sum == 0 || sum >= MAX_PCC_SUBSPACES) {
 | 
		
	
		
			
				|  |  |  |  | -		pr_err("Error parsing PCC subspaces from PCCT\n");
 | 
		
	
		
			
				|  |  |  |  | +	count = acpi_table_parse_entries_array(ACPI_SIG_PCCT,
 | 
		
	
		
			
				|  |  |  |  | +			sizeof(struct acpi_table_pcct), proc,
 | 
		
	
		
			
				|  |  |  |  | +			ACPI_PCCT_TYPE_RESERVED, MAX_PCC_SUBSPACES);
 | 
		
	
		
			
				|  |  |  |  | +	if (count == 0 || count > MAX_PCC_SUBSPACES) {
 | 
		
	
		
			
				|  |  |  |  | +		pr_warn("Invalid PCCT: %d PCC subspaces\n", count);
 | 
		
	
		
			
				|  |  |  |  |  		return -EINVAL; | 
		
	
		
			
				|  |  |  |  |  	} | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	pcc_mbox_channels = kzalloc(sizeof(struct mbox_chan) *
 | 
		
	
		
			
				|  |  |  |  | -			sum, GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  | +	pcc_mbox_channels = kzalloc(sizeof(struct mbox_chan) * count, GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  |  	if (!pcc_mbox_channels) { | 
		
	
		
			
				|  |  |  |  |  		pr_err("Could not allocate space for PCC mbox channels\n"); | 
		
	
		
			
				|  |  |  |  |  		return -ENOMEM; | 
		
	
		
			
				|  |  |  |  |  	} | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	pcc_doorbell_vaddr = kcalloc(sum, sizeof(void *), GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  | +	pcc_doorbell_vaddr = kcalloc(count, sizeof(void *), GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  |  	if (!pcc_doorbell_vaddr) { | 
		
	
		
			
				|  |  |  |  |  		rc = -ENOMEM; | 
		
	
		
			
				|  |  |  |  |  		goto err_free_mbox; | 
		
	
		
			
				|  |  |  |  |  	} | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	pcc_doorbell_ack_vaddr = kcalloc(sum, sizeof(void *), GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  | +	pcc_doorbell_ack_vaddr = kcalloc(count, sizeof(void *), GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  |  	if (!pcc_doorbell_ack_vaddr) { | 
		
	
		
			
				|  |  |  |  |  		rc = -ENOMEM; | 
		
	
		
			
				|  |  |  |  |  		goto err_free_db_vaddr; | 
		
	
		
			
				|  |  |  |  |  	} | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	pcc_doorbell_irq = kcalloc(sum, sizeof(int), GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  | +	pcc_doorbell_irq = kcalloc(count, sizeof(int), GFP_KERNEL);
 | 
		
	
		
			
				|  |  |  |  |  	if (!pcc_doorbell_irq) { | 
		
	
		
			
				|  |  |  |  |  		rc = -ENOMEM; | 
		
	
		
			
				|  |  |  |  |  		goto err_free_db_ack_vaddr; | 
		
	
		
			
				|  |  |  |  | @@ -509,18 +498,24 @@ static int __init acpi_pcc_probe(void)
 | 
		
	
		
			
				|  |  |  |  |  	if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL) | 
		
	
		
			
				|  |  |  |  |  		pcc_mbox_ctrl.txdone_irq = true; | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	for (i = 0; i < sum; i++) {
 | 
		
	
		
			
				|  |  |  |  | +	for (i = 0; i < count; i++) {
 | 
		
	
		
			
				|  |  |  |  |  		struct acpi_generic_address *db_reg; | 
		
	
		
			
				|  |  |  |  | -		struct acpi_pcct_hw_reduced *pcct_ss;
 | 
		
	
		
			
				|  |  |  |  | +		struct acpi_pcct_subspace *pcct_ss;
 | 
		
	
		
			
				|  |  |  |  |  		pcc_mbox_channels[i].con_priv = pcct_entry; | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -		pcct_ss = (struct acpi_pcct_hw_reduced *) pcct_entry;
 | 
		
	
		
			
				|  |  |  |  | +		if (pcct_entry->type == ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE ||
 | 
		
	
		
			
				|  |  |  |  | +		    pcct_entry->type == ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2) {
 | 
		
	
		
			
				|  |  |  |  | +			struct acpi_pcct_hw_reduced *pcct_hrss;
 | 
		
	
		
			
				|  |  |  |  | +
 | 
		
	
		
			
				|  |  |  |  | +			pcct_hrss = (struct acpi_pcct_hw_reduced *) pcct_entry;
 | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -		if (pcc_mbox_ctrl.txdone_irq) {
 | 
		
	
		
			
				|  |  |  |  | -			rc = pcc_parse_subspace_irq(i, pcct_ss);
 | 
		
	
		
			
				|  |  |  |  | -			if (rc < 0)
 | 
		
	
		
			
				|  |  |  |  | -				goto err;
 | 
		
	
		
			
				|  |  |  |  | +			if (pcc_mbox_ctrl.txdone_irq) {
 | 
		
	
		
			
				|  |  |  |  | +				rc = pcc_parse_subspace_irq(i, pcct_hrss);
 | 
		
	
		
			
				|  |  |  |  | +				if (rc < 0)
 | 
		
	
		
			
				|  |  |  |  | +					goto err;
 | 
		
	
		
			
				|  |  |  |  | +			}
 | 
		
	
		
			
				|  |  |  |  |  		} | 
		
	
		
			
				|  |  |  |  | +		pcct_ss = (struct acpi_pcct_subspace *) pcct_entry;
 | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  |  		/* If doorbell is in system memory cache the virt address */ | 
		
	
		
			
				|  |  |  |  |  		db_reg = &pcct_ss->doorbell_register; | 
		
	
		
			
				|  |  |  |  | @@ -531,7 +526,7 @@ static int __init acpi_pcc_probe(void)
 | 
		
	
		
			
				|  |  |  |  |  			((unsigned long) pcct_entry + pcct_entry->length); | 
		
	
		
			
				|  |  |  |  |  	} | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -	pcc_mbox_ctrl.num_chans = sum;
 | 
		
	
		
			
				|  |  |  |  | +	pcc_mbox_ctrl.num_chans = count;
 | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  |  	pr_info("Detected %d PCC Subspaces\n", pcc_mbox_ctrl.num_chans); | 
		
	
		
			
				|  |  |  |  |   | 
		
	
		
			
				|  |  |  |  | -- 
 | 
		
	
		
			
				|  |  |  |  | 2.14.3 |