linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Zheng, Lv" <lv.zheng@intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	"Brown, Len" <len.brown@intel.com>, Lv Zheng <zetalog@gmail.com>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	Linux ACPI <linux-acpi@vger.kernel.org>,
	"Moore, Robert" <robert.moore@intel.com>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@ml01.01.org>
Subject: RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
Date: Fri, 9 Dec 2016 02:15:54 +0000	[thread overview]
Message-ID: <1AE640813FDE7649BE1B193DEA596E886A2B23C3@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <CAPcyv4gVuay4uZu5qgDidWyj_LV9s+eRNW6CUZaELHxNtSeHtQ@mail.gmail.com>

Hi, Dan

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Dan
> Williams
> Sent: Friday, December 9, 2016 10:05 AM
> To: Zheng, Lv <lv.zheng@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Rafael J.
> Wysocki <rjw@rjwysocki.net>; Brown, Len <len.brown@intel.com>; Lv Zheng <zetalog@gmail.com>; Linux
> Kernel Mailing List <linux-kernel@vger.kernel.org>; Linux ACPI <linux-acpi@vger.kernel.org>; Moore,
> Robert <robert.moore@intel.com>; linux-nvdimm@lists.01.org <linux-nvdimm@ml01.01.org>
> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel
> 
> On Thu, Dec 8, 2016 at 5:59 PM, Zheng, Lv <lv.zheng@intel.com> wrote:
> > Hi, Rafael and Dan
> >
> >> From: Dan Williams [mailto:dan.j.williams@intel.com]
> >> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_unmap_memory() from Linux kernel
> >>
> >> On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >> > On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> >> >> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
> >> >>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
> >> >>>
> >> >>> This patch back ports Linux acpi_get_table_with_size() and
> >> >>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
> >> >>>
> >> >>> The 2 APIs are used by Linux as table management APIs for long time, it
> >> >>> contains a hidden logic that during the early stage, the mapped tables
> >> >>> should be unmapped before the early stage ends.
> >> >>>
> >> >>> During the early stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table_with_size();
> >> >>>  parse the table
> >> >>>  early_acpi_os_unmap_memory();
> >> >>> During the late stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
> >> >>> late stage.
> >> >>>
> >> >>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
> >> >>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
> >> >>> prevent ACPICA from using the wrong early mapped pointer during the late
> >> >>> stage as there is no API provided from ACPICA to be an inverse of
> >> >>> acpi_get_table() to forget the early mapped pointer.
> >> >>>
> >> >>> But how ACPICA can work with the early/late stage requirement? Inside of
> >> >>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
> >> >>> early stage, and they are carefully not transitioned to "VALIDATED" state
> >> >>> until the late stage. So the same logic is in fact implemented inside of
> >> >>> ACPICA in a different way. The gap is only that the feature is not provided
> >> >>> to the OSPMs in an accessible external API style.
> >> >>>
> >> >>> It then is possible to fix the gap by providing an inverse of
> >> >>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
> >> >>> combined:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>>  acpi_put_table();
> >> >>> In order to work easier with the current Linux code, acpi_get_table() and
> >> >>> acpi_put_table() is implemented in a usage counting based style:
> >> >>>  1. When the usage count of the table is increased from 0 to 1, table is
> >> >>>     mapped and .Pointer is set with the mapping address (VALIDATED);
> >> >>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
> >> >>>     is unset and the mapping address is unmapped (INVALIDATED).
> >> >>> So that we can deploy the new APIs to Linux with minimal effort by just
> >> >>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
> >> >>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
> >> >>>
> >> >>> Link: https://github.com/acpica/acpica/commit/cac67909
> >> >>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> >> >>> Signed-off-by: Bob Moore <robert.moore@intel.com>
> >> >>
> >> >> This commit in -next (071b39575679 ACPICA: Tables: Back port
> >> >> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
> >> >> kernel) causes a regression in my nfit/nvdimm test environment. The
> >> >> nfit produced by QEMU no longer results in a nvdimm bus being created.
> >> >>
> >> >> I have not root caused it, but I'm using the following command line
> >> >> options to create an nfit in qemu-2.6.  Reverting the commit leads
> >> >> compile failures.
> >> >
> >> > Would the build problems go away if you reverted "ACPICA: Tables:
> >> > Allow FADT to be customized with virtual address" (linux-next commit
> >> > cf334d3174f9) in addition to it?
> >>
> >> Yes, reverting those two commits gets me back to a functional environment:
> >>
> >> Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
> >> Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_un
> >
> > To Dan:
> > It seems in drivers/acpi/nfit/core.c.
> > The returned table size is used by the NFIT code.
> > I think it should be changed to use table_header->length.
> 
> Does the acpi core already validate that table_header->length is
> correct? i.e. is is possible that a broken implementation could have
> the wrong length in the header? I was assuming that was the purpose of
> the _with_size(), but maybe I was wrong?

That should always be correct.
In acpi_tb_init_table_descriptor(), table_desc->length is set by table_header->length.
In acpi_tb_validate_table(), which calls acpi_tb_acquire_table(), acpi_os_map_memory() always uses table_desc->length.

Thanks and best regards
Lv

> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-12-09  2:16 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-30  7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
2016-11-30  7:20 ` [PATCH 01/11] ACPICA: Namespace: Add acpi_ns_handle_to_name() Lv Zheng
2016-11-30  7:20 ` [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()" Lv Zheng
2016-11-30 22:30   ` Rafael J. Wysocki
2016-12-01  7:50     ` Zheng, Lv
2016-12-01 13:29       ` Rafael J. Wysocki
2016-11-30  7:21 ` [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value Lv Zheng
2016-11-30 23:07   ` Rafael J. Wysocki
2016-12-01  8:00     ` Zheng, Lv
2016-12-01 13:30       ` Rafael J. Wysocki
2016-11-30  7:21 ` [PATCH 05/11] ACPICA: Tables: Cleanup acpi_tb_install_and_load_table() Lv Zheng
2016-11-30  7:21 ` [PATCH 06/11] ACPICA: Tables: Add acpi_tb_unload_table() Lv Zheng
2016-11-30  7:21 ` [PATCH 07/11] ACPICA: Tables: Add an error message complaining driver bugs Lv Zheng
2016-11-30  7:21 ` [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel Lv Zheng
2016-12-08  1:11   ` Dan Williams
2016-12-08 13:18     ` Rafael J. Wysocki
2016-12-08 19:04       ` Dan Williams
2016-12-09  1:59         ` Zheng, Lv
2016-12-09  2:04           ` Dan Williams
2016-12-09  2:15             ` Zheng, Lv [this message]
2016-12-09  2:27             ` Zheng, Lv
2016-12-09  2:05           ` Rafael J. Wysocki
2016-12-09  2:23             ` Zheng, Lv
2016-12-09  1:49     ` Zheng, Lv
2016-12-09  1:57       ` Dan Williams
2016-11-30  7:21 ` [PATCH 09/11] ACPICA: Tables: Allow FADT to be customized with virtual address Lv Zheng
2016-11-30  7:21 ` [PATCH 10/11] ACPICA: Utilities: Add new decode function for parser values Lv Zheng
2016-11-30  7:22 ` [PATCH 11/11] ACPICA: Update version to 20161117 Lv Zheng
2016-12-09  2:21 ` [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size() Lv Zheng
2016-12-09  3:48   ` Rafael J. Wysocki
2016-12-09  6:09     ` Zheng, Lv

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1AE640813FDE7649BE1B193DEA596E886A2B23C3@SHSMSX101.ccr.corp.intel.com \
    --to=lv.zheng@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@ml01.01.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=robert.moore@intel.com \
    --cc=zetalog@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).