| Message ID | 20200816151230.14524-1-yu.c.chen@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [RFC] ACPI: processor: Print more information when acpi_processor_evaluate_cst() failed | expand |
On Sun, Aug 16, 2020 at 5:12 PM Chen Yu <yu.c.chen@intel.com> wrote: > > Some platforms have bogus _CST which might cause expectd behavior > in the cpu idle driver. Some bogus _CST might be unable to be > disassembled by acpica-tools due to broken format. > Print extra log if the _CST extraction/verification failed. > This can be used to help the user narrow down why the cpu > idle driver fails to behave as expected. > > Suggested-by: Zhang Rui <rui.zhang@intel.com> > Signed-off-by: Chen Yu <yu.c.chen@intel.com> This is fine by me as a general idea, but please change the log level to "info". > --- > drivers/acpi/acpi_processor.c | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c > index b51ddf3bb616..c1d34c448edb 100644 > --- a/drivers/acpi/acpi_processor.c > +++ b/drivers/acpi/acpi_processor.c > @@ -798,22 +798,34 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, > memset(&cx, 0, sizeof(cx)); > > element = &cst->package.elements[i]; > - if (element->type != ACPI_TYPE_PACKAGE) > + if (element->type != ACPI_TYPE_PACKAGE) { > + acpi_handle_warn(handle, "_CST C%d type(%x) is not package, skip...\n", > + i, element->type); > continue; > + } > > - if (element->package.count != 4) > + if (element->package.count != 4) { > + acpi_handle_warn(handle, "_CST C%d package count(%d) is not 4, skip...\n", > + i, element->package.count); > continue; > + } > > obj = &element->package.elements[0]; > > - if (obj->type != ACPI_TYPE_BUFFER) > + if (obj->type != ACPI_TYPE_BUFFER) { > + acpi_handle_warn(handle, "_CST C%d package element[0] type(%x) is not buffer, skip...\n", > + i, obj->type); > continue; > + } > > reg = (struct acpi_power_register *)obj->buffer.pointer; > > obj = &element->package.elements[1]; > - if (obj->type != ACPI_TYPE_INTEGER) > + if (obj->type != ACPI_TYPE_INTEGER) { > + acpi_handle_warn(handle, "_CST C[%d] package element[1] type(%x) is not integer, skip...\n", > + i, obj->type); > continue; > + } > > cx.type = obj->integer.value; > /* > @@ -850,6 +862,8 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, > cx.entry_method = ACPI_CSTATE_HALT; > snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI HLT"); > } else { > + acpi_handle_warn(handle, "_CST C%d declares FIXED_HARDWARE C-state but not supported in hardware, skip...\n", > + i); > continue; > } > } else if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) { > @@ -857,6 +871,8 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, > snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI IOPORT 0x%x", > cx.address); > } else { > + acpi_handle_warn(handle, "_CST C%d space_id(%x) neither FIXED_HARDWARE nor SYSTEM_IO, skip...\n", > + i, reg->space_id); > continue; > } > > @@ -864,14 +880,20 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, > cx.valid = 1; > > obj = &element->package.elements[2]; > - if (obj->type != ACPI_TYPE_INTEGER) > + if (obj->type != ACPI_TYPE_INTEGER) { > + acpi_handle_warn(handle, "_CST C%d package element[2] type(%x) not integer, skip...\n", > + i, obj->type); > continue; > + } > > cx.latency = obj->integer.value; > > obj = &element->package.elements[3]; > - if (obj->type != ACPI_TYPE_INTEGER) > + if (obj->type != ACPI_TYPE_INTEGER) { > + acpi_handle_warn(handle, "_CST C%d package element[3] type(%x) not integer, skip...\n", > + i, obj->type); > continue; > + } > > memcpy(&info->states[++last_index], &cx, sizeof(cx)); > } > -- > 2.17.1 >
On Tue, Aug 18, 2020 at 12:46:12PM +0200, Rafael J. Wysocki wrote: > On Sun, Aug 16, 2020 at 5:12 PM Chen Yu <yu.c.chen@intel.com> wrote: > > > > Some platforms have bogus _CST which might cause expectd behavior > > in the cpu idle driver. Some bogus _CST might be unable to be > > disassembled by acpica-tools due to broken format. > > Print extra log if the _CST extraction/verification failed. > > This can be used to help the user narrow down why the cpu > > idle driver fails to behave as expected. > > > > Suggested-by: Zhang Rui <rui.zhang@intel.com> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com> > > This is fine by me as a general idea, but please change the log level to "info". > Okay, I will send a new version. Thanks! thanks, Chenyu
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index b51ddf3bb616..c1d34c448edb 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -798,22 +798,34 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, memset(&cx, 0, sizeof(cx)); element = &cst->package.elements[i]; - if (element->type != ACPI_TYPE_PACKAGE) + if (element->type != ACPI_TYPE_PACKAGE) { + acpi_handle_warn(handle, "_CST C%d type(%x) is not package, skip...\n", + i, element->type); continue; + } - if (element->package.count != 4) + if (element->package.count != 4) { + acpi_handle_warn(handle, "_CST C%d package count(%d) is not 4, skip...\n", + i, element->package.count); continue; + } obj = &element->package.elements[0]; - if (obj->type != ACPI_TYPE_BUFFER) + if (obj->type != ACPI_TYPE_BUFFER) { + acpi_handle_warn(handle, "_CST C%d package element[0] type(%x) is not buffer, skip...\n", + i, obj->type); continue; + } reg = (struct acpi_power_register *)obj->buffer.pointer; obj = &element->package.elements[1]; - if (obj->type != ACPI_TYPE_INTEGER) + if (obj->type != ACPI_TYPE_INTEGER) { + acpi_handle_warn(handle, "_CST C[%d] package element[1] type(%x) is not integer, skip...\n", + i, obj->type); continue; + } cx.type = obj->integer.value; /* @@ -850,6 +862,8 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, cx.entry_method = ACPI_CSTATE_HALT; snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI HLT"); } else { + acpi_handle_warn(handle, "_CST C%d declares FIXED_HARDWARE C-state but not supported in hardware, skip...\n", + i); continue; } } else if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) { @@ -857,6 +871,8 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, snprintf(cx.desc, ACPI_CX_DESC_LEN, "ACPI IOPORT 0x%x", cx.address); } else { + acpi_handle_warn(handle, "_CST C%d space_id(%x) neither FIXED_HARDWARE nor SYSTEM_IO, skip...\n", + i, reg->space_id); continue; } @@ -864,14 +880,20 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, cx.valid = 1; obj = &element->package.elements[2]; - if (obj->type != ACPI_TYPE_INTEGER) + if (obj->type != ACPI_TYPE_INTEGER) { + acpi_handle_warn(handle, "_CST C%d package element[2] type(%x) not integer, skip...\n", + i, obj->type); continue; + } cx.latency = obj->integer.value; obj = &element->package.elements[3]; - if (obj->type != ACPI_TYPE_INTEGER) + if (obj->type != ACPI_TYPE_INTEGER) { + acpi_handle_warn(handle, "_CST C%d package element[3] type(%x) not integer, skip...\n", + i, obj->type); continue; + } memcpy(&info->states[++last_index], &cx, sizeof(cx)); }
Some platforms have bogus _CST which might cause expectd behavior in the cpu idle driver. Some bogus _CST might be unable to be disassembled by acpica-tools due to broken format. Print extra log if the _CST extraction/verification failed. This can be used to help the user narrow down why the cpu idle driver fails to behave as expected. Suggested-by: Zhang Rui <rui.zhang@intel.com> Signed-off-by: Chen Yu <yu.c.chen@intel.com> --- drivers/acpi/acpi_processor.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-)