Does the machine or machines work properly with Windows? This is always one of our early questions. Bob > -----Original Message----- > From: Hans de Goede [mailto:hdegoede@xxxxxxxxxx] > Sent: Tuesday, February 28, 2017 6:32 AM > To: Zheng, Lv <lv.zheng@xxxxxxxxx>; Rafael J . Wysocki > <rjw@xxxxxxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx>; Moore, Robert > <robert.moore@xxxxxxxxx> > Cc: linux-acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx > Subject: Re: [PATCH] ACPICA: Detect duplicate SSDT tables > > Hi, > > On 28-02-17 06:19, Zheng, Lv wrote: > > Hi, > > > >> From: Hans de Goede [mailto:hdegoede@xxxxxxxxxx] > >> Subject: [PATCH] ACPICA: Detect duplicate SSDT tables > >> > >> Some machines have the exact (byte for byte) same SSDT tables > >> multiple times in the root_table_list. > > > > Could you give a machine list here? > > Currently I'm seeing this on a GPD win machine: > > http://www.gpd.hk/gpdwin.asp > > I thought I was seeing it on more machines, but those have different > apci table loading errors... > > >> Detect this and silently skip the duplicates rather then printing a > >> scary looking set of errors. > > > > Why will this matter to OSPMs? > > Not sure what you mean with OSPMs but I can tell you why this matters in > general, Linux distributions like e.g. Fedora have been putting a lot of > work in a smooth boot experience where end users do not get any scary > text messages. For some more embedded like systems this even is a hard > requirement. > > The kernel supports quiet kernel cmdline argument to silence normal > kernel messages, which is part of what is needed but messages with a log > level of error still get shown, breaking the "no scary text messages" > product requirement. > > > And should we add non-costless steps just in order to reduce errors, > > Yes we should, work on that front has been happening for years, also the > CPU cost of this check is quite small, memcmp will only happen on > identically sized tables and even then it will exit as soon as a single > byte differs. > > > while the errors are on the contrary useful (in1dicating platform > issues)? > > These errors are useful for developers / during testing but not in > production setups, esp. in the case of duplicate tables where not > loading the duplicate leads to 0 bad side effects. > > I've an alternative proposal though, since this check just fixes a small > part of the early boot messages caused by SSDT loading and since the > code itself chooses to ignore any errors: > > /* Ignore errors while loading tables, get as many as possible > */ > > How about setting a global flag while loading these tables and making > > ACPI_EXCEPTION calls log the exceptions with a log level of warning as > well as turning the final: > > ACPI_ERROR((AE_INFO, > "%u table load failures, %u successful", > tables_failed, tables_loaded)); > > Into a warning ? > > Regards, > > Hans > > > > > > > > Thanks > > Lv > > > >> > >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > >> --- > >> drivers/acpi/acpica/tbxfload.c | 41 > >> ++++++++++++++++++++++++++++++++++++++++- > >> 1 file changed, 40 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/acpi/acpica/tbxfload.c > >> b/drivers/acpi/acpica/tbxfload.c index 82019c0..1971cd7 100644 > >> --- a/drivers/acpi/acpica/tbxfload.c > >> +++ b/drivers/acpi/acpica/tbxfload.c > >> @@ -125,6 +125,44 @@ ACPI_EXPORT_SYMBOL_INIT(acpi_load_tables) > >> > >> > /*********************************************************************** > ******** > >> * > >> + * FUNCTION: acpi_tb_find_duplicate_ssdt > >> + * > >> + * PARAMETERS: table - validated acpi_table_desc of table > to check > >> + * index - index of table to find a duplicate > of > >> + * > >> + * RETURN: TRUE if a duplicate is found, FALSE if not > >> + * > >> + * DESCRIPTION: Private helper function for acpi_tb_load_namespace > to > >> + * avoid trying to load duplicate ssdt tables > >> + * > >> + > >> +******************************************************************** > >> +**********/ static u8 acpi_tb_find_duplicate_ssdt(struct > >> +acpi_table_desc *table, u32 index) { > >> + struct acpi_table_desc *dup; > >> + u32 i; > >> + > >> + for (i = 0; i < index; ++i) { > >> + dup = &acpi_gbl_root_table_list.tables[i]; > >> + > >> + if (!acpi_gbl_root_table_list.tables[i].address || > >> + (!ACPI_COMPARE_NAME(dup->signature.ascii, ACPI_SIG_SSDT) > >> + && !ACPI_COMPARE_NAME(dup->signature.ascii, > >> + ACPI_SIG_PSDT) > >> + && !ACPI_COMPARE_NAME(dup->signature.ascii, > >> + ACPI_SIG_OSDT)) > >> + || ACPI_FAILURE(acpi_tb_validate_table(dup)) > >> + || dup->length != table->length) { > >> + continue; > >> + } > >> + > >> + if (memcmp(dup->pointer, table->pointer, table->length) == 0) > >> + return TRUE; > >> + } > >> + return FALSE; > >> +} > >> + > >> +/******************************************************************* > >> +************ > >> + * > >> * FUNCTION: acpi_tb_load_namespace > >> * > >> * PARAMETERS: None > >> @@ -212,7 +250,8 @@ acpi_status acpi_tb_load_namespace(void) > >> ACPI_SIG_PSDT) > >> && !ACPI_COMPARE_NAME(table->signature.ascii, > >> ACPI_SIG_OSDT)) > >> - || ACPI_FAILURE(acpi_tb_validate_table(table))) { > >> + || ACPI_FAILURE(acpi_tb_validate_table(table)) > >> + || acpi_tb_find_duplicate_ssdt(table, i)) { > >> continue; > >> } > >> > >> -- > >> 2.9.3 > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
