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