Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932741AbcCCWt3 (ORCPT ); Thu, 3 Mar 2016 17:49:29 -0500 Received: from mail-oi0-f54.google.com ([209.85.218.54]:33344 "EHLO mail-oi0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932336AbcCCWt1 (ORCPT ); Thu, 3 Mar 2016 17:49:27 -0500 MIME-Version: 1.0 In-Reply-To: <1456959056-12316-5-git-send-email-toshi.kani@hpe.com> References: <1456959056-12316-1-git-send-email-toshi.kani@hpe.com> <1456959056-12316-5-git-send-email-toshi.kani@hpe.com> Date: Thu, 3 Mar 2016 14:49:26 -0800 Message-ID: Subject: Re: [PATCH v2 4/4] ACPI: Change NFIT driver to insert new resource From: Dan Williams To: Toshi Kani Cc: Ingo Molnar , Borislav Petkov , "Rafael J. Wysocki" , Andrew Morton , "linux-nvdimm@lists.01.org" , Linux ACPI , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2879 Lines: 65 On Wed, Mar 2, 2016 at 2:50 PM, Toshi Kani wrote: > ACPI 6.0 defines persistent memory (PMEM) ranges in multiple > firmware interfaces, e820, EFI, and ACPI NFIT table. This EFI > change, however, leads to hit a bug in the grub bootloader, which > treats EFI_PERSISTENT_MEMORY type as regular memory and corrupts > stored user data [1]. > > Therefore, BIOS may set generic reserved type in e820 and EFI > to cover PMEM ranges. The kernel can initialize PMEM ranges > from ACPI NFIT table alone. > > This scheme causes a problem in the iomem table, though. On x86, > for instance, e820_reserve_resources() initializes top-level entries > (iomem_resource.child) from the e820 table at early boot-time. > This creates "reserved" entry for a PMEM range, which does not allow > region_intersects() to check with PMEM type. > > Change acpi_nfit_register_region() to call acpi_nfit_insert_resource(), > which calls devm_insert_resource() to insert a PMEM entry from NFIT > when the iomem table does not have a PMEM entry already. That is, > when a PMEM range is marked as reserved type in e820, it inserts > "Persistent Memory" entry, which results as follows. > > + "Persistent Memory" > + "reserved" > > This allows the EINJ driver, which calls region_intersects() to > check PMEM ranges, to work continuously even if BIOS sets reserved > type (or sets nothing) to PMEM ranges in e820 and EFI. > > [1]: https://lists.gnu.org/archive/html/grub-devel/2015-11/msg00209.html > Signed-off-by: Toshi Kani > Cc: Rafael J. Wysocki > Cc: Dan Williams > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: Andrew Morton > --- > drivers/acpi/nfit.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c > index fb53db1..d97b53f 100644 > --- a/drivers/acpi/nfit.c > +++ b/drivers/acpi/nfit.c > @@ -1571,6 +1571,30 @@ static int ars_status_process_records(struct nvdimm_bus *nvdimm_bus, > return 0; > } > > +static int acpi_nfit_insert_resource(struct acpi_nfit_desc *acpi_desc, > + struct nd_region_desc *ndr_desc) > +{ > + struct resource *res, *nd_res = ndr_desc->res; > + size_t size = nd_res->end - nd_res->start + 1; > + > + /* No operation if the region is already registered as PMEM */ > + if (region_intersects(nd_res->start, size, IORESOURCE_MEM, > + IORES_DESC_PERSISTENT_MEMORY) == REGION_INTERSECTS) > + return 0; > + > + res = devm_kzalloc(acpi_desc->dev, sizeof(*res), GFP_KERNEL); How about allocating this resource on the stack and then have devm_insert_resource handle the dynamic allocation (memdup) so we have one less failure point to handle in the driver.