Hi, > From: rjwysocki@xxxxxxxxx [mailto:rjwysocki@xxxxxxxxx] On Behalf Of Rafael J. Wysocki > Subject: Re: [PATCH v2] ACPI / OSL: Fix a regression by returning acpi_table_header.length instead of > 0 from acpi_get_table_with_size() > > On Fri, Dec 9, 2016 at 8:15 AM, Lv Zheng <lv.zheng@xxxxxxxxx> wrote: > > The following wrong commit triggers issues in acpi_get_table_with_size() > > users: > > Subject: ACPICA: Tables: Back port acpi_get_table_with_size() and > > early_acpi_os_unmap_memory() from Linux kernel > > > > The function is invented to be a replacement of acpi_get_table() during > > early stage so that the early mapped pointer will not be stored in ACPICA > > core and thus the late stage acpi_get_table() won't return a wrong pointer. > > However the mapping size is returned just because it is required by > > early_acpi_os_unmap_memory() to unmap the pointer during early stage. > > > > As the mapping size equals to the acpi_table_header.length > > (see acpi_tb_init_table_descriptor() and acpi_tb_validate_table()), when > > such a convenient result is returned, driver code will start to use it > > instead of accessing acpi_table_header to obtain the length. > > > > So the commit can trigger problems in such drivers as it returns 0 now. And > > it did bring a trouble to drivers/acpi/nfit/core.c. So before cleaning up > > the drivers, we should keep the old semantics of the API. > > > > This patch fixes the wrong commit by returning the acpi_table_header.length > > from acpi_get_table_with_size(). > > Well, the way this works isn't particularly straightforward. I don't know what's your preference here? Shall we drop the 2 wrong commits, and merge fixed ones to replace them in linux-pm/linux-next? Or can we just merge this fix patch on top of the wrong commits? > > Wouldn't it be better to simply drop the check (if it is redundant) > from the NFIT driver? At this point, I'm not sure if NFIT is the only driver broken by the wrong commit. My plan was: 1. Keep old behavior working by still providing acpi_get_table_with_size()/early_acpi_os_unmap_memory(); 2. Use a set of patches to cleanup acpi_get_table_with_size()/early_acpi_os_unmap_memory() users, So that we can carefully examine every caller, split strange use cases into separate commits to avoid regressions; 3. Remove acpi_get_table_with_size() and early_acpi_os_unmap_memory() if there is no driver using them. Thanks and best regards Lv > > > Reported-by: Dan Williams <dan.j.williams@xxxxxxxxx> > > Cc: Dan Williams <dan.j.williams@xxxxxxxxx> > > Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx> > > --- > > drivers/acpi/osl.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > > index 5bef0f65..d0b273f 100644 > > --- a/drivers/acpi/osl.c > > +++ b/drivers/acpi/osl.c > > @@ -445,8 +445,16 @@ void __ref acpi_os_unmap_memory(void *virt, acpi_size size) > > > > status = acpi_get_table(signature, instance, out_table); > > if (ACPI_SUCCESS(status)) { > > - /* No longer used by early_acpi_os_unmap_memory() */ > > - *tbl_size = 0; > > + /* > > + * "tbl_size" is no longer used by > > + * early_acpi_os_unmap_memory(), but is still used by the > > + * ACPI table drivers. So sets it to the length of the > > + * table when the tbl_size is requested. > > + * "out_table" is not sanity checked as AE_BAD_PARAMETER > > + * is returned if it is NULL. > > + */ > > + if (tbl_size && *out_table) > > + *tbl_size = (*out_table)->length; > > } > > > > return (status); > > -- > > 1.7.10 > > > > Thanks, > Rafael ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
