2016-03-09 18:54:36

by Kani, Toshimitsu

[permalink] [raw]
Subject: [PATCH v3 0/4] Support persistent memory as reserved type in e820/EFI

ACPI 6 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 cause 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.

This patch-set adds remove_resource(), exports insert/remove_resource(),
and changes the NFIT driver to insert a PMEM entry to the iomem table.

[1] https://lists.gnu.org/archive/html/grub-devel/2015-11/msg00209.html

- Patch 1 fixes __request_region() to handle inheritance of attribute
properly. This patch is independent from this series.
- Patch 2 adds remove_resource() interface.
- Patch 3 exports insert_resource() and remove_resource().
- Patch 4 changes the ACPI nfit driver to call insert_resource() to
insert a PMEM range from NFIT.

---
v3:
- Drop devm_insert/remove_resource(), and export insert/remove_resource().
(Linus Torvalds)

v2:
- Change the NFIT driver to call insert_resource() to insert a PMEM entry
when necessary. (Dan Williams)
- The NFIT driver still needs to allow unloading. (Dan Williams)

---
This patch-set is based on the tip tree, which has the following patchset.
https://lkml.org/lkml/2016/1/26/886

---
Toshi Kani (4):
1/4 resource: Change __request_region to inherit from immediate parent
2/4 resource: Add remove_resource interface
3/4 resource: Export insert_resource and remove_resource
4/4 ACPI: Change NFIT driver to insert new resource

---
drivers/acpi/nfit.c | 48 ++++++++++++++++++++++++++++++++++++++++
include/linux/ioport.h | 1 +
kernel/resource.c | 60 +++++++++++++++++++++++++++++++++++++++++++-------
3 files changed, 101 insertions(+), 8 deletions(-)


2016-03-09 19:30:10

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support persistent memory as reserved type in e820/EFI

On Wed, Mar 9, 2016 at 11:46 AM, Toshi Kani <[email protected]> wrote:
> ACPI 6 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 cause 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.
>
> This patch-set adds remove_resource(), exports insert/remove_resource(),
> and changes the NFIT driver to insert a PMEM entry to the iomem table.
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2015-11/msg00209.html
>
> - Patch 1 fixes __request_region() to handle inheritance of attribute
> properly. This patch is independent from this series.
> - Patch 2 adds remove_resource() interface.
> - Patch 3 exports insert_resource() and remove_resource().
> - Patch 4 changes the ACPI nfit driver to call insert_resource() to
> insert a PMEM range from NFIT.
>
> ---
> v3:
> - Drop devm_insert/remove_resource(), and export insert/remove_resource().
> (Linus Torvalds)
>
> v2:
> - Change the NFIT driver to call insert_resource() to insert a PMEM entry
> when necessary. (Dan Williams)
> - The NFIT driver still needs to allow unloading. (Dan Williams)
>
> ---
> This patch-set is based on the tip tree, which has the following patchset.
> https://lkml.org/lkml/2016/1/26/886
>
> ---
> Toshi Kani (4):
> 1/4 resource: Change __request_region to inherit from immediate parent
> 2/4 resource: Add remove_resource interface
> 3/4 resource: Export insert_resource and remove_resource
> 4/4 ACPI: Change NFIT driver to insert new resource
>
> ---
> drivers/acpi/nfit.c | 48 ++++++++++++++++++++++++++++++++++++++++
> include/linux/ioport.h | 1 +
> kernel/resource.c | 60 +++++++++++++++++++++++++++++++++++++++++++-------
> 3 files changed, 101 insertions(+), 8 deletions(-)

Hi Toshi, this looks good to me and passes my testing.

I'll give it until the end of the day if there is any more feedback
and otherwise merge it into the pending libnvdimm updates for 4.6.

2016-03-09 19:42:25

by Kani, Toshimitsu

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Support persistent memory as reserved type in e820/EFI

On Wed, 2016-03-09 at 11:30 -0800, Dan Williams wrote:
> On Wed, Mar 9, 2016 at 11:46 AM, Toshi Kani <[email protected]> wrote:
 :
> > Toshi Kani (4):
> >  1/4 resource: Change __request_region to inherit from immediate parent
> >  2/4 resource: Add remove_resource interface
> >  3/4 resource: Export insert_resource and remove_resource
> >  4/4 ACPI: Change NFIT driver to insert new resource
> >
> > ---
> >  drivers/acpi/nfit.c    | 48 ++++++++++++++++++++++++++++++++++++++++
> >  include/linux/ioport.h |  1 +
> >  kernel/resource.c      | 60
> > +++++++++++++++++++++++++++++++++++++++++++-------
> >  3 files changed, 101 insertions(+), 8 deletions(-)
>
> Hi Toshi, this looks good to me and passes my testing.
>
> I'll give it until the end of the day if there is any more feedback
> and otherwise merge it into the pending libnvdimm updates for 4.6.

Great! Thanks Dan!
-Toshi