From 9c5a93b73a1109bb5ce6f5ccacf39a3c754fc297 Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: Fri, 25 May 2018 10:57:07 -0400 Subject: [PATCH] Fix for incorrect error message about parsing PCCT (rhbz 1435837) --- kernel.spec | 7 + ...eous-error-message-when-parsing-ACPI.patch | 240 ++++++++++++++++++ 2 files changed, 247 insertions(+) create mode 100644 mailbox-ACPI-erroneous-error-message-when-parsing-ACPI.patch diff --git a/kernel.spec b/kernel.spec index 943cded59..86feb341f 100644 --- a/kernel.spec +++ b/kernel.spec @@ -618,6 +618,10 @@ Patch502: input-rmi4-remove-the-need-for-artifical-IRQ.patch # rhbz 1509461 Patch503: v3-2-2-Input-synaptics---Lenovo-X1-Carbon-5-should-use-SMBUS-RMI.patch +# rbhz 1435837 +# https://www.spinics.net/lists/linux-acpi/msg82405.html +Patch504: mailbox-ACPI-erroneous-error-message-when-parsing-ACPI.patch + # END OF PATCH DEFINITIONS %endif @@ -1853,6 +1857,9 @@ fi # # %changelog +* Fri May 25 2018 Jeremy Cline +- Fix for incorrect error message about parsing PCCT (rhbz 1435837) + * Thu May 24 2018 Justin M. Forbes - 4.17.0-0.rc6.git2.1 - Linux v4.17-rc6-158-gbee797529d7c - Reenable debugging options. diff --git a/mailbox-ACPI-erroneous-error-message-when-parsing-ACPI.patch b/mailbox-ACPI-erroneous-error-message-when-parsing-ACPI.patch new file mode 100644 index 000000000..819bf1cae --- /dev/null +++ b/mailbox-ACPI-erroneous-error-message-when-parsing-ACPI.patch @@ -0,0 +1,240 @@ +From 8f8027c5f935bf02bdc8806c109ddbb0e402283c Mon Sep 17 00:00:00 2001 +From: Al Stone +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 +Reviewed-by: Prashanth Prakash +Signed-off-by: Rafael J. Wysocki +--- + 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