grub causing NVDIMMs to be treated as normal memory

grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

[-- Attachment #1: Type: text/plain, Size: 3922 bytes --]

We've noticed that some combinations of grub and old linux kernels

end up interpreting the UEFI memory map EfiPersistentMemory type 14

(formerly a reserved value) as regular memory in the linux e820

table, causing silent data corruption on the NVDIMMs.  That occurs

even though grub prints this message suggesting everything is safe:

    Unknown memory type 14, considering reserved



In broken versions of grub, the code parsing the UEFI memory map

has a "default" case that falls through to the

GRUB_EFI_BOOT_SERVICES_DATA case, which marks the memory range

as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.



In unbroken versions of grub, that "default" case falls through to

the GRUB_EFI_RESERVED_MEMORY_TYPE case, which marks the memory range

as GRUB_MACHINE_MEMORY_RESERVED and ends up in e820 as reserved.



Each of these kernels behaves differently:

* without EFI boot stub (pre-2011 kernel 3.3)

* with EFI boot stub (May 2015, kernel 4.1)

* with EFI boot stub, with May 2015 patch to understand type 14



Kernels that don't have the EFI boot stub are at the mercy of how

grub translates the memory map.  I'm not sure how the pre-4.1

EFI boot stub kernels worked. New kernels work fine.



Does anyone know how prevalent the use of dangerous combinations

might be?



Might other OS loaders (syslinux, elilo, ?) have issues like this?





Example broken grub code

========================

http://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/mmap/efi/mmap.c:



grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,

                     int avoid_efi_boot_services)

{

...

       case GRUB_EFI_UNUSABLE_MEMORY:

         hook (desc->physical_start, desc->num_pages * 4096,

              GRUB_MEMORY_BADRAM, hook_data);

         break;



       default:

         grub_printf ("Unknown memory type %d, considering reserved\n",

                     desc->type);



       case GRUB_EFI_BOOT_SERVICES_DATA:

         if (!avoid_efi_boot_services)

           {

             hook (desc->physical_start, desc->num_pages * 4096,

                  GRUB_MEMORY_AVAILABLE, hook_data);

             break;

           }

       case GRUB_EFI_RESERVED_MEMORY_TYPE:

       case GRUB_EFI_RUNTIME_SERVICES_DATA:

       case GRUB_EFI_MEMORY_MAPPED_IO:

       case GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE:

       case GRUB_EFI_PAL_CODE:

         hook (desc->physical_start, desc->num_pages * 4096,

              GRUB_MEMORY_RESERVED, hook_data);

         break;



Example unbroken grub code

==========================

https://github.com/jolicloud/grub2/blob/master/mmap/efi/mmap.c:



grub_machine_mmap_iterate (int NESTED_FUNC_ATTR (*hook) (grub_uint64_t,

                                                grub_uint64_t,

                                                grub_uint32_t))

{

...

       default:

         grub_printf ("Unknown memory type %d, considering reserved\n",

                     desc->type);



       case GRUB_EFI_RESERVED_MEMORY_TYPE:

       case GRUB_EFI_RUNTIME_SERVICES_DATA:

       case GRUB_EFI_UNUSABLE_MEMORY:

       case GRUB_EFI_MEMORY_MAPPED_IO:

       case GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE:

       case GRUB_EFI_PAL_CODE:

         hook (desc->physical_start, desc->num_pages * 4096,

              GRUB_MACHINE_MEMORY_RESERVED);

         break;



       case GRUB_EFI_LOADER_CODE:

       case GRUB_EFI_LOADER_DATA:

       case GRUB_EFI_BOOT_SERVICES_CODE:

       case GRUB_EFI_BOOT_SERVICES_DATA:

       case GRUB_EFI_CONVENTIONAL_MEMORY:

         hook (desc->physical_start, desc->num_pages * 4096,

              GRUB_MACHINE_MEMORY_AVAILABLE);

         break;


---
Robert Elliott, HPE Persistent Memory


[-- Attachment #2: Type: text/html, Size: 12391 bytes --]

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

On Wed, Nov 25, 2015 at 2:52 AM, Elliott, Robert (Persistent Memory)
<elliott@hpe.com> wrote:
> We've noticed that some combinations of grub and old linux kernels
>
> end up interpreting the UEFI memory map EfiPersistentMemory type 14
>
> (formerly a reserved value) as regular memory in the linux e820
>
> table, causing silent data corruption on the NVDIMMs.  That occurs
>
> even though grub prints this message suggesting everything is safe:
>
>     Unknown memory type 14, considering reserved
>
>
>
> In broken versions of grub, the code parsing the UEFI memory map
>
> has a "default" case that falls through to the
>
> GRUB_EFI_BOOT_SERVICES_DATA case,

This is fallout of 9be4c45dbe3c877d1f4856e99ee15133c6cd2261; thank you
for report!

> which marks the memory range
>
> as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.
>

According to EFI spec, EfiPersistentMemory is "A memory region that
operates as EfiConventionalMemory". Why it should be treated
differently?

Re: grub causing NVDIMMs to be treated as normal memory

[Dan Williams]

On Wed, Nov 25, 2015 at 6:08 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> On Wed, Nov 25, 2015 at 2:52 AM, Elliott, Robert (Persistent Memory)
> <elliott@hpe.com> wrote:
>> We've noticed that some combinations of grub and old linux kernels
>>
>> end up interpreting the UEFI memory map EfiPersistentMemory type 14
>>
>> (formerly a reserved value) as regular memory in the linux e820
>>
>> table, causing silent data corruption on the NVDIMMs.  That occurs
>>
>> even though grub prints this message suggesting everything is safe:
>>
>>     Unknown memory type 14, considering reserved
>>
>>
>>
>> In broken versions of grub, the code parsing the UEFI memory map
>>
>> has a "default" case that falls through to the
>>
>> GRUB_EFI_BOOT_SERVICES_DATA case,
>
> This is fallout of 9be4c45dbe3c877d1f4856e99ee15133c6cd2261; thank you
> for report!
>
>> which marks the memory range
>>
>> as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.
>>
>
> According to EFI spec, EfiPersistentMemory is "A memory region that
> operates as EfiConventionalMemory". Why it should be treated
> differently?

Because it needs to be reserved from the OS and attached via a driver.
See the ACPI 6 definition for "AddressRangePersistentMemory":

"OSPM must comprehend this memory as having non-volatile attributes
and handle distinct from conventional volatile memory. The memory
region supports byte-addressable nonvolatility"

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of
> Andrei Borzenkov
> Sent: Wednesday, November 25, 2015 8:08 AM
> To: The development of GNU GRUB <grub-devel@gnu.org>
> Cc: linux-nvdimm@lists.01.org
> Subject: Re: grub causing NVDIMMs to be treated as normal memory
> 
> On Wed, Nov 25, 2015 at 2:52 AM, Elliott, Robert (Persistent Memory)
> <elliott@hpe.com> wrote:
> > We've noticed that some combinations of grub and old linux kernels
> > end up interpreting the UEFI memory map EfiPersistentMemory type 14
> > (formerly a reserved value) as regular memory in the linux e820
> > table, causing silent data corruption on the NVDIMMs.  That occurs
> > even though grub prints this message suggesting everything is safe:
> >     Unknown memory type 14, considering reserved
> >
> > In broken versions of grub, the code parsing the UEFI memory map
> > has a "default" case that falls through to the
> > GRUB_EFI_BOOT_SERVICES_DATA case,
> 
> This is fallout of 9be4c45dbe3c877d1f4856e99ee15133c6cd2261; thank you
> for report!

Thanks, that patch was dated March 2012, which narrows the window.
Earlier versions fell through to the reserved case.
 
> > which marks the memory range
> > as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.
> 
> According to EFI spec, EfiPersistentMemory is "A memory region that
> operates as EfiConventionalMemory". Why it should be treated
> differently?

Hardware ensures that the data persists across reboots and power
cycles.  The address range must be managed by special drivers (e.g.,
the pmem block device driver) and not be included in the normal
memory management pool.  Some addresses might even hold registers
(for a block control window interface), not memory.

The ACPI NFIT (NVDIMM Firmware Interface Table) provides crucial
information for handling the address range.  The NFIT SPA (System
Physical Address) Range structures, for example, describe each
range in detail.

I'll work with the UEFI WG to fix that "operates as" wording.

---
Robert Elliott, HPE Persistent Memory

Re: grub causing NVDIMMs to be treated as normal memory

[Seth Goldberg]

 It may operate the same, but there may be persistent data in there that the OS wants to preserve across reboots.

 —S

On Nov 25, 2015, at 6:08 AM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> On Wed, Nov 25, 2015 at 2:52 AM, Elliott, Robert (Persistent Memory)
> <elliott@hpe.com> wrote:
>> We've noticed that some combinations of grub and old linux kernels
>> 
>> end up interpreting the UEFI memory map EfiPersistentMemory type 14
>> 
>> (formerly a reserved value) as regular memory in the linux e820
>> 
>> table, causing silent data corruption on the NVDIMMs.  That occurs
>> 
>> even though grub prints this message suggesting everything is safe:
>> 
>>    Unknown memory type 14, considering reserved
>> 
>> 
>> 
>> In broken versions of grub, the code parsing the UEFI memory map
>> 
>> has a "default" case that falls through to the
>> 
>> GRUB_EFI_BOOT_SERVICES_DATA case,
> 
> This is fallout of 9be4c45dbe3c877d1f4856e99ee15133c6cd2261; thank you
> for report!
> 
>> which marks the memory range
>> 
>> as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.
>> 
> 
> According to EFI spec, EfiPersistentMemory is "A memory region that
> operates as EfiConventionalMemory". Why it should be treated
> differently?
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

25.11.2015 02:52, Elliott, Robert (Persistent Memory) пишет:
> We've noticed that some combinations of grub and old linux kernels
> 
> end up interpreting the UEFI memory map EfiPersistentMemory type 14
> 
> (formerly a reserved value) as regular memory in the linux e820
> 
> table, causing silent data corruption on the NVDIMMs.  That occurs
> 
> even though grub prints this message suggesting everything is safe:
> 
>     Unknown memory type 14, considering reserved
> 
> 
> 
> In broken versions of grub, the code parsing the UEFI memory map
> 
> has a "default" case that falls through to the
> 
> GRUB_EFI_BOOT_SERVICES_DATA case, which marks the memory range
> 
> as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.
> 

Could you test if attached patch works for you (compile tested)?


[-- Attachment #2: efi-unknown-memory-type.patch --]
[-- Type: text/x-patch, Size: 2044 bytes --]

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] efi: really mark memory of unknown type as reserved

9be4c45dbe3c877d1f4856e99ee15133c6cd2261 added switch case between
fall through cases, causing all memory regions of unknown type to be
marked as available.

Move default case into its own block and add explicit FALLTHROUGH
annotation.

Reported by Elliott, Robert (Persistent Memory) <elliott@hpe.com>

---
 grub-core/mmap/efi/mmap.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/grub-core/mmap/efi/mmap.c b/grub-core/mmap/efi/mmap.c
index a77efe8..6b5f5d8 100644
--- a/grub-core/mmap/efi/mmap.c
+++ b/grub-core/mmap/efi/mmap.c
@@ -73,6 +73,7 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,
 		    GRUB_MEMORY_AVAILABLE, hook_data);
 	      break;
 	    }
+	  /* FALLTHROUGH */
 	case GRUB_EFI_RUNTIME_SERVICES_CODE:
 	  hook (desc->physical_start, desc->num_pages * 4096,
 		GRUB_MEMORY_CODE, hook_data);
@@ -83,10 +84,6 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,
 		GRUB_MEMORY_BADRAM, hook_data);
 	  break;
 
-	default:
-	  grub_printf ("Unknown memory type %d, considering reserved\n",
-		       desc->type);
-
 	case GRUB_EFI_BOOT_SERVICES_DATA:
 	  if (!avoid_efi_boot_services)
 	    {
@@ -94,6 +91,7 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,
 		    GRUB_MEMORY_AVAILABLE, hook_data);
 	      break;
 	    }
+	  /* FALLTHROUGH */
 	case GRUB_EFI_RESERVED_MEMORY_TYPE:
 	case GRUB_EFI_RUNTIME_SERVICES_DATA:
 	case GRUB_EFI_MEMORY_MAPPED_IO:
@@ -119,6 +117,13 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,
 	  hook (desc->physical_start, desc->num_pages * 4096,
 		GRUB_MEMORY_NVS, hook_data);
 	  break;
+	default:
+	  grub_printf ("Unknown memory type %d, considering reserved\n",
+		       desc->type);
+	  hook (desc->physical_start, desc->num_pages * 4096,
+		GRUB_MEMORY_RESERVED, hook_data);
+	  break;
+
 	}
     }
 
-- 
tg: (f9d1b44..) u/efi-unknown-memory-type (depends on: master)

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On
> Behalf Of Andrei Borzenkov
> Sent: Wednesday, November 25, 2015 12:37 PM
> To: The development of GNU GRUB <grub-devel@gnu.org>;
> dan.j.williams@intel.com; linux-nvdimm@lists.01.org
> Subject: Re: grub causing NVDIMMs to be treated as normal memory
> 
> 25.11.2015 02:52, Elliott, Robert (Persistent Memory) пишет:
> > We've noticed that some combinations of grub and old linux kernels
> >
> > end up interpreting the UEFI memory map EfiPersistentMemory type 14
> > (formerly a reserved value) as regular memory in the linux e820
> > table, causing silent data corruption on the NVDIMMs.  That occurs
> > even though grub prints this message suggesting everything is safe:
> >
> >     Unknown memory type 14, considering reserved
> >
> >
> >
> > In broken versions of grub, the code parsing the UEFI memory map
> > has a "default" case that falls through to the
> >
> > GRUB_EFI_BOOT_SERVICES_DATA case, which marks the memory range
> > as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.
> 
> Could you test if attached patch works for you (compile tested)?

Thanks.

I think I finally got that to compile with
    configure --with-platform=efi
    make

but have no clue how to install it and try it out.  I'm using a
fedora22 system, which has its own /sbin/grub2-install.  I 
don't understand how that differs from the grub-install in the
build directory or how to get either of them to work.

Anyway, we should create another patch that does:
* #define GRUB_EFI_PERSISTENT_MEMORY 14	per UEFI 2.5
* #define GRUB_E820_PERSISTENT_MEMORY 7	per ACPI 6.0
* add a GRUB_MEMORY_PMEM enum
* map GRUB_EFI_PERSISTENT_MEMORY -> GRUM_PMEMORY_MEM 
  -> GRUB_E820_PERSISTENT_MEMORY per ACPI 6.0

to explicitly handle the new types (in addition to handling
unknown values correctly).


---
Robert Elliott, HPE Persistent Memory

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

26.11.2015 03:12, Elliott, Robert (Persistent Memory) пишет:
> 
> 
>> -----Original Message-----
>> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On
>> Behalf Of Andrei Borzenkov
>> Sent: Wednesday, November 25, 2015 12:37 PM
>> To: The development of GNU GRUB <grub-devel@gnu.org>;
>> dan.j.williams@intel.com; linux-nvdimm@lists.01.org
>> Subject: Re: grub causing NVDIMMs to be treated as normal memory
>>
>> 25.11.2015 02:52, Elliott, Robert (Persistent Memory) пишет:
>>> We've noticed that some combinations of grub and old linux kernels
>>>
>>> end up interpreting the UEFI memory map EfiPersistentMemory type 14
>>> (formerly a reserved value) as regular memory in the linux e820
>>> table, causing silent data corruption on the NVDIMMs.  That occurs
>>> even though grub prints this message suggesting everything is safe:
>>>
>>>     Unknown memory type 14, considering reserved
>>>
>>>
>>>
>>> In broken versions of grub, the code parsing the UEFI memory map
>>> has a "default" case that falls through to the
>>>
>>> GRUB_EFI_BOOT_SERVICES_DATA case, which marks the memory range
>>> as GRUB_MEMORY_AVAILABLE and ends up in e820 as regular memory.
>>
>> Could you test if attached patch works for you (compile tested)?
> 
> Thanks.
> 
> I think I finally got that to compile with
>     configure --with-platform=efi
>     make
> 
> but have no clue how to install it and try it out.  I'm using a
> fedora22 system, which has its own /sbin/grub2-install.  I 
> don't understand how that differs from the grub-install in the
> build directory or how to get either of them to work.
> 

From the build directory

pkgdatadir=$PWD ./grub-install --bootloader-id testgrub -d grub-core

This should install grub in \EFI\testgrub on ESP and add EFI menu for
it. You can add --no-nvram to skip EFI menu update and load it manually
then. It will install modules in /boot/grub (instead of /boot/grub2), so
you should probably copy /boot/grub2/grub.cfg there.

Of course you can simply add patch to Fedora package and rebuild this
package.

> Anyway, we should create another patch that does:
> * #define GRUB_EFI_PERSISTENT_MEMORY 14	per UEFI 2.5
> * #define GRUB_E820_PERSISTENT_MEMORY 7	per ACPI 6.0
> * add a GRUB_MEMORY_PMEM enum
> * map GRUB_EFI_PERSISTENT_MEMORY -> GRUM_PMEMORY_MEM 
>   -> GRUB_E820_PERSISTENT_MEMORY per ACPI 6.0
> 
> to explicitly handle the new types (in addition to handling
> unknown values correctly).
> 

That is much more involved than obvious bug fix. It can be done later.

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: Andrei Borzenkov [mailto:arvidjaar@gmail.com]
...
> From the build directory
> 
> pkgdatadir=$PWD ./grub-install --bootloader-id testgrub -d grub-core
> 
> This should install grub in \EFI\testgrub on ESP and add EFI menu for
> it. You can add --no-nvram to skip EFI menu update and load it
> manually then. It will install modules in /boot/grub (instead of
> /boot/grub2), so you should probably copy /boot/grub2/grub.cfg there.

grub-install reported one complaint, which I ignored:
	Installing for x86_64-efi platform.
	./grub-install: warning: cannot open directory `/usr/local/share/locale': No such file or directory.
	Installation finished. No error reported.

I ran this to create a grub.cfg there like the one I had been using:
	grub2-mkconfig /boot/grub 

The system rebooted and ran the code, giving me a menu on the serial
port:
	GNU GRUB  version 2.02~beta2 menu 

Booting failed.  I had to change these with 'e':
* linuxefi to linux
* initrdefi to initrd

Hitting Ctrl-x to boot after that resulted in a successful boot with
these prints:
  Booting a command list

Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
Unknown memory type 14, considering reserved
[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Initializing cgroup subsys cpuacct
[    0.000000] Linux version 4.4.0-rc2+ (orange@s17) (gcc version 5.1.1 20150618 (Red Hat 5.1.1-4) (GCC) ) #87 SMP Mon Nov 23 16:12:10 CST 2015
[    0.000000] Command line: BOOT_IMAGE=/vmlinuz-4.4.0-rc2+ root=/dev/mapper/fedora_pmem03-root ro rd.lvm.lv=fedora_pmem03/swap rd.lvm.lv=fedora_pmem03/root rhgb quiet efi=debug libnvdimm.dyndbg nfit.dyndbg nd_pmem.dyndbg nd_btt.dyndbg ignore_loglevel console=tty0 console=ttyS0,115200
...


Next, I ran grub-mkconfig from the build directory:
	grub-mkconfig -o /boot/grub/grub.cfg
and it created a grub.cfg that used linux and initrd, but 
lacked all the linux command line options from /etc/default/grub.

Based on comments in the top of grub.cfg file, I copied
/etc/default/grub to /usr/local/etc/default/grub
and reran.  This preserved the linux command line options.

I also added this to /boot/grub/grub.cfg:
	set debug=mmap,linux

That results in a different looking boot menu with two top
level entries:
	*Fedora GNU/Linux
	 Advanced options for Fedora GNU/Linux

I let the timeout expire and it booted with these prints:
  Booting `Fedora GNU/Linux'

Loading Linux 4.4.0-rc2+ ...
mmap/efi/mmap.c:66: EFI memory region 0x0-0x93000: 7
...[skipping rest of the entries]...
mmap/efi/mmap.c:66: EFI memory region 0x880000000-0xc80000000: 14
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000: 14
Unknown memory type 14, considering reserved
loader/i386/linux.c:253: prot_mode_mem = 0x2000000, prot_mode_target = 2000000,
prot_size = 12e9000
loader/i386/linux.c:877: bzImage, setup=0x4200, size=0x12e9000
Loading initial ramdisk ...
loader/i386/linux.c:1123: Initrd, addr=0x35dfd000, size=0x21f1ab1
mmap/efi/mmap.c:66: EFI memory region 0x0-0x93000: 7
...
mmap/efi/mmap.c:66: EFI memory region 0x880000000-0xc80000000: 14
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000: 14
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x0-0x93000: 7
...
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000: 14
Unknown memory type 14, considering reserved
loader/i386/linux.c:569: real_size = 6000, mmap_size = 2000
mmap/efi/mmap.c:66: EFI memory region 0x0-0x93000: 7
loader/i386/linux.c:423: addr = 10000, size = 80000, need_size = a000
mmap/efi/mmap.c:66: EFI memory region 0x93000-0x94000: 0
...
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000: 14
Unknown memory type 14, considering reserved
loader/i386/linux.c:581: real_mode_target = 86000, real_size = 6000,
efi_mmap_size = 4000
loader/i386/linux.c:598: real_mode_mem = 0x77926000
loader/i386/linux.c:608: code32_start = 2000000
mmap/efi/mmap.c:66: EFI memory region 0x0-0x93000: 7
...
mmap/efi/mmap.c:66: EFI memory region 0x880000000-0xc80000000: 14
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000: 14
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x0-0x93000: 7
...
mmap/efi/mmap.c:66: EFI memory region 0x880000000-0xc80000000: 14
Unknown memory type 14, considering reserved
mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000: 14
Unknown memory type 14, considering reserved



This is booting a new kernel with the EFI boot stub, so I can't
confirm that it fixes the issue of exposing the address range as
normal, but based on the print it's probably working.  I could
add prints in grub-core/loader/i386/linux.c grub_e820_add_region
to confirm the e820 contents at exit.


---
Robert Elliott, HPE Persistent Memory

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

26.11.2015 09:15, Elliott, Robert (Persistent Memory) пишет:
...

> ...
> mmap/efi/mmap.c:66: EFI memory region 0x880000000-0xc80000000: 14
> Unknown memory type 14, considering reserved
> mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000: 14
> Unknown memory type 14, considering reserved
> 
> 
> 
> This is booting a new kernel with the EFI boot stub, so I can't
> confirm that it fixes the issue of exposing the address range as
> normal, but based on the print it's probably working.  I could
> add prints in grub-core/loader/i386/linux.c grub_e820_add_region
> to confirm the e820 contents at exit.
> 

Thanks. If it results in wrong e820 type we have much larger problem so
I do not think it necessary.

I pushed it; as I understand this should serve as stopgap. If I get
around to implement persistent memory type would you test it? But as I
cannot tell when it happens, feel free to submit patch in the meantime :)

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: Andrei Borzenkov [mailto:arvidjaar@gmail.com]
> Sent: Thursday, November 26, 2015 10:55 AM
> To: Elliott, Robert (Persistent Memory) <elliott@hpe.com>; The
> development of GNU GRUB <grub-devel@gnu.org>;
> dan.j.williams@intel.com; linux-nvdimm@lists.01.org
> Subject: Re: grub causing NVDIMMs to be treated as normal memory
> 
> 26.11.2015 09:15, Elliott, Robert (Persistent Memory) пишет:
> ...
> 
> > ...
> > mmap/efi/mmap.c:66: EFI memory region 0x880000000-0xc80000000: 14
> > Unknown memory type 14, considering reserved
> > mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000:
> 14
> > Unknown memory type 14, considering reserved
> >
> >
> >
> > This is booting a new kernel with the EFI boot stub, so I can't
> > confirm that it fixes the issue of exposing the address range as
> > normal, but based on the print it's probably working.  I could
> > add prints in grub-core/loader/i386/linux.c grub_e820_add_region
> > to confirm the e820 contents at exit.
> >
> 
> Thanks. If it results in wrong e820 type we have much larger problem
> so I do not think it necessary.
> 
> I pushed it; as I understand this should serve as stopgap. If I get
> around to implement persistent memory type would you test it? But as
> I cannot tell when it happens, feel free to submit patch in the
> meantime :)

As an experiment, I:
* compiled a linux 4.4rc2 kernel with CONFIG_EFI_STUB disabled
* added some prints in the linux kernel setup_memory_map()
  to print the incoming bios_params e820 entries
* modified the grub patch to set the range to AVAILABLE rather 
  than RESERVED (simulating the bug, to see its impacts on
  the kernel)

Before (CONFIG_EFI_STUB=y, grub mapping to RESERVED):
bp: [mem 0x0000000000000000-0x0000000000092fff] usable
bp: [mem 0x0000000000093000-0x0000000000093fff] reserved
bp: [mem 0x0000000000094000-0x000000000009ffff] usable
bp: [mem 0x0000000000100000-0x000000006affbfff] usable
bp: [mem 0x000000006affc000-0x000000006b5fbfff] reserved
bp: [mem 0x000000006b5fc000-0x000000006b5fcfff] usable
bp: [mem 0x000000006b5fd000-0x000000006b67dfff] reserved
bp: [mem 0x000000006b67e000-0x00000000784fefff] usable
bp: [mem 0x00000000784ff000-0x00000000791fefff] reserved <--b
bp: [mem 0x00000000791ff000-0x000000007b5fefff] ACPI NVS
bp: [mem 0x000000007b5ff000-0x000000007b7fefff] ACPI data
bp: [mem 0x000000007b7ff000-0x000000007b7fffff] usable
bp: [mem 0x0000000100000000-0x000000087fffffff] usable <--a
bp: [mem 0x0000000c80000000-0x000000147fffffff] usable <--a
bp: [mem 0x0000000080000000-0x000000008fffffff] reserved
bp: [mem 0x0000000880000000-0x0000000c7fffffff] reserved <--a
bp: [mem 0x0000001480000000-0x0000001a7fffffff] reserved <--a

After (CONFIG_EFI_STUB=n, grub mapping to AVAILABLE):
bp: [mem 0x0000000000000000-0x0000000000092fff] usable
bp: [mem 0x0000000000093000-0x0000000000093fff] reserved
bp: [mem 0x0000000000094000-0x000000000009ffff] usable
bp: [mem 0x0000000000100000-0x000000006affbfff] usable
bp: [mem 0x000000006affc000-0x000000006b5fbfff] reserved
bp: [mem 0x000000006b5fc000-0x000000006b5fcfff] usable
bp: [mem 0x000000006b5fd000-0x000000006b67dfff] reserved
bp: [mem 0x000000006b67e000-0x00000000784fefff] usable
bp: [mem 0x00000000784ff000-0x00000000788fefff] reserved <--b
bp: [mem 0x00000000788ff000-0x00000000790fefff] type 20  <--b
bp: [mem 0x00000000790ff000-0x00000000791fefff] reserved <--b
bp: [mem 0x00000000791ff000-0x000000007b5fefff] ACPI NVS
bp: [mem 0x000000007b5ff000-0x000000007b7fefff] ACPI data
bp: [mem 0x000000007b7ff000-0x000000007b7fffff] usable
bp: [mem 0x0000000080000000-0x000000008fffffff] reserved
bp: [mem 0x0000000100000000-0x0000001a7fffffff] usable <--a


Note (a): The 088 and 148 ranges were merged into the 
010 and 0c8 usable ranges, as expected for this experiment.

The NVDIMM drivers in the kernel don't load (nmem devices load,
but namespaces fail and no pmem devices show up):
[   27.835319] nd_pmem namespace0.0: could not reserve region [0x0x0000000880000000:0x400000000]
[   27.835323]  ndbus1: nd_pmem.probe(namespace0.0) = -16
[   27.835355] nd_pmem namespace2.0: could not reserve region [0x0x0000001880000000:0x200000000]
[   27.835356]  ndbus1: nd_pmem.probe(namespace2.0) = -16
[   27.835373] nd_pmem namespace1.0: could not reserve region [0x0x0000001480000000:0x400000000]
[   27.835374]  ndbus1: nd_pmem.probe(namespace1.0) = -16

That shows up as normal memory in /proc/iomem and elsewhere:
100000000-1a7fffffff : System RAM

$ numactl --hardware
available: 1 nodes (0)
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
node 0 size: 104614 MB
node 0 free: 103303 MB
node distances:
node   0
  0:  10



Note (b): The internal GRUB_MEMORY_CODE (20) value is 
leaking through to the E820 table.

That appears to be from this patch on 2013-10-14:
	6de9ee86 Pass-through unknown E820 types
which deleted a switch statement mapping the internal codes 
to E820 values and included this:
-    case GRUB_MEMORY_CODE:
-    case GRUB_MEMORY_RESERVED:
-      ctx->cur.type = GRUB_E820_RESERVED;

based on this hope:
+/* GRUB types conveniently match E820 types.  */

That's only true for types 1 through 5:
typedef enum grub_memory_type
  {
    GRUB_MEMORY_AVAILABLE = 1,
    GRUB_MEMORY_RESERVED = 2,
    GRUB_MEMORY_ACPI = 3,
    GRUB_MEMORY_NVS = 4,
    GRUB_MEMORY_BADRAM = 5,
    GRUB_MEMORY_COREBOOT_TABLES = 16,
    GRUB_MEMORY_CODE = 20,
    /* This one is special: it's used internally but is never reported
       by firmware. Don't use -1 as it's used internally for other purposes. */
    GRUB_MEMORY_HOLE = -2,
    GRUB_MEMORY_MAX = 0x10000
  } grub_memory_type_t;


---
Robert Elliott, HPE Persistent Memory

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

27.11.2015 02:24, Elliott, Robert (Persistent Memory) пишет:
> 
>> -----Original Message-----
>> From: Andrei Borzenkov [mailto:arvidjaar@gmail.com]
>> Sent: Thursday, November 26, 2015 10:55 AM
>> To: Elliott, Robert (Persistent Memory) <elliott@hpe.com>; The
>> development of GNU GRUB <grub-devel@gnu.org>;
>> dan.j.williams@intel.com; linux-nvdimm@lists.01.org
>> Subject: Re: grub causing NVDIMMs to be treated as normal memory
>>
>> 26.11.2015 09:15, Elliott, Robert (Persistent Memory) пишет:
>> ...
>>
>>> ...
>>> mmap/efi/mmap.c:66: EFI memory region 0x880000000-0xc80000000: 14
>>> Unknown memory type 14, considering reserved
>>> mmap/efi/mmap.c:66: EFI memory region 0x1480000000-0x1a80000000:
>> 14
>>> Unknown memory type 14, considering reserved
>>>
>>>
>>>
>>> This is booting a new kernel with the EFI boot stub, so I can't
>>> confirm that it fixes the issue of exposing the address range as
>>> normal, but based on the print it's probably working.  I could
>>> add prints in grub-core/loader/i386/linux.c grub_e820_add_region
>>> to confirm the e820 contents at exit.
>>>
>>
>> Thanks. If it results in wrong e820 type we have much larger problem
>> so I do not think it necessary.
>>
>> I pushed it; as I understand this should serve as stopgap. If I get
>> around to implement persistent memory type would you test it? But as
>> I cannot tell when it happens, feel free to submit patch in the
>> meantime :)
> 
> As an experiment, I:
> * compiled a linux 4.4rc2 kernel with CONFIG_EFI_STUB disabled
> * added some prints in the linux kernel setup_memory_map()
>   to print the incoming bios_params e820 entries
> * modified the grub patch to set the range to AVAILABLE rather 
>   than RESERVED (simulating the bug, to see its impacts on
>   the kernel)
> 
> Before (CONFIG_EFI_STUB=y, grub mapping to RESERVED):
> bp: [mem 0x0000000000000000-0x0000000000092fff] usable
> bp: [mem 0x0000000000093000-0x0000000000093fff] reserved
> bp: [mem 0x0000000000094000-0x000000000009ffff] usable
> bp: [mem 0x0000000000100000-0x000000006affbfff] usable
> bp: [mem 0x000000006affc000-0x000000006b5fbfff] reserved
> bp: [mem 0x000000006b5fc000-0x000000006b5fcfff] usable
> bp: [mem 0x000000006b5fd000-0x000000006b67dfff] reserved
> bp: [mem 0x000000006b67e000-0x00000000784fefff] usable
> bp: [mem 0x00000000784ff000-0x00000000791fefff] reserved <--b
> bp: [mem 0x00000000791ff000-0x000000007b5fefff] ACPI NVS
> bp: [mem 0x000000007b5ff000-0x000000007b7fefff] ACPI data
> bp: [mem 0x000000007b7ff000-0x000000007b7fffff] usable
> bp: [mem 0x0000000100000000-0x000000087fffffff] usable <--a
> bp: [mem 0x0000000c80000000-0x000000147fffffff] usable <--a
> bp: [mem 0x0000000080000000-0x000000008fffffff] reserved
> bp: [mem 0x0000000880000000-0x0000000c7fffffff] reserved <--a
> bp: [mem 0x0000001480000000-0x0000001a7fffffff] reserved <--a
> 
> After (CONFIG_EFI_STUB=n, grub mapping to AVAILABLE):
> bp: [mem 0x0000000000000000-0x0000000000092fff] usable
> bp: [mem 0x0000000000093000-0x0000000000093fff] reserved
> bp: [mem 0x0000000000094000-0x000000000009ffff] usable
> bp: [mem 0x0000000000100000-0x000000006affbfff] usable
> bp: [mem 0x000000006affc000-0x000000006b5fbfff] reserved
> bp: [mem 0x000000006b5fc000-0x000000006b5fcfff] usable
> bp: [mem 0x000000006b5fd000-0x000000006b67dfff] reserved
> bp: [mem 0x000000006b67e000-0x00000000784fefff] usable
> bp: [mem 0x00000000784ff000-0x00000000788fefff] reserved <--b
> bp: [mem 0x00000000788ff000-0x00000000790fefff] type 20  <--b
> bp: [mem 0x00000000790ff000-0x00000000791fefff] reserved <--b
> bp: [mem 0x00000000791ff000-0x000000007b5fefff] ACPI NVS
> bp: [mem 0x000000007b5ff000-0x000000007b7fefff] ACPI data
> bp: [mem 0x000000007b7ff000-0x000000007b7fffff] usable
> bp: [mem 0x0000000080000000-0x000000008fffffff] reserved
> bp: [mem 0x0000000100000000-0x0000001a7fffffff] usable <--a
> 
> 
> Note (a): The 088 and 148 ranges were merged into the 
> 010 and 0c8 usable ranges, as expected for this experiment.
> 
> The NVDIMM drivers in the kernel don't load (nmem devices load,
> but namespaces fail and no pmem devices show up):
> [   27.835319] nd_pmem namespace0.0: could not reserve region [0x0x0000000880000000:0x400000000]
> [   27.835323]  ndbus1: nd_pmem.probe(namespace0.0) = -16
> [   27.835355] nd_pmem namespace2.0: could not reserve region [0x0x0000001880000000:0x200000000]
> [   27.835356]  ndbus1: nd_pmem.probe(namespace2.0) = -16
> [   27.835373] nd_pmem namespace1.0: could not reserve region [0x0x0000001480000000:0x400000000]
> [   27.835374]  ndbus1: nd_pmem.probe(namespace1.0) = -16
> 
> That shows up as normal memory in /proc/iomem and elsewhere:
> 100000000-1a7fffffff : System RAM
> 

May be it is too early in the morning, but could you explain whether the
test outcome confirms the patch worked or not? :)

> $ numactl --hardware
> available: 1 nodes (0)
> node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
> node 0 size: 104614 MB
> node 0 free: 103303 MB
> node distances:
> node   0
>   0:  10
> 
> 
> 
> Note (b): The internal GRUB_MEMORY_CODE (20) value is 
> leaking through to the E820 table.
> 
> That appears to be from this patch on 2013-10-14:
> 	6de9ee86 Pass-through unknown E820 types

If we are discussing ACPI 6.0 systems here, it explicitly says that
values above 12 should be treated as reserved. Does it cause problems?

> which deleted a switch statement mapping the internal codes 
> to E820 values and included this:
> -    case GRUB_MEMORY_CODE:
> -    case GRUB_MEMORY_RESERVED:
> -      ctx->cur.type = GRUB_E820_RESERVED;
> 
> based on this hope:
> +/* GRUB types conveniently match E820 types.  */
> 
> That's only true for types 1 through 5:
> typedef enum grub_memory_type
>   {
>     GRUB_MEMORY_AVAILABLE = 1,
>     GRUB_MEMORY_RESERVED = 2,
>     GRUB_MEMORY_ACPI = 3,
>     GRUB_MEMORY_NVS = 4,
>     GRUB_MEMORY_BADRAM = 5,
>     GRUB_MEMORY_COREBOOT_TABLES = 16,
>     GRUB_MEMORY_CODE = 20,
>     /* This one is special: it's used internally but is never reported
>        by firmware. Don't use -1 as it's used internally for other purposes. */
>     GRUB_MEMORY_HOLE = -2,
>     GRUB_MEMORY_MAX = 0x10000
>   } grub_memory_type_t;
> 
> 
> ---
> Robert Elliott, HPE Persistent Memory
> 
> 
> 
> 

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: Andrei Borzenkov [mailto:arvidjaar@gmail.com]
> Sent: Thursday, November 26, 2015 9:58 PM
> To: Elliott, Robert (Persistent Memory) <elliott@hpe.com>; The
> development of GNU GRUB <grub-devel@gnu.org>;
> dan.j.williams@intel.com; linux-nvdimm@lists.01.org
> Subject: Re: grub causing NVDIMMs to be treated as normal memory
> 
> 27.11.2015 02:24, Elliott, Robert (Persistent Memory) пишет:
> >
...
> > Note (a): The 088 and 148 ranges were merged into the
> > 010 and 0c8 usable ranges, as expected for this experiment.
> >
> > The NVDIMM drivers in the kernel don't load (nmem devices load,
> > but namespaces fail and no pmem devices show up):
...
> > That shows up as normal memory in /proc/iomem and elsewhere:
> > 100000000-1a7fffffff : System RAM
> >
> 
> May be it is too early in the morning, but could you explain whether
> the test outcome confirms the patch worked or not? :)

Yes, that all worked as expected.  This description was more
for the linux-nvdimm folks (trying to understand the impact of
the issue).

> > Note (b): The internal GRUB_MEMORY_CODE (20) value is
> > leaking through to the E820 table.
> >
> > That appears to be from this patch on 2013-10-14:
> > 	6de9ee86 Pass-through unknown E820 types
> 
> If we are discussing ACPI 6.0 systems here, it explicitly says that
> values above 12 should be treated as reserved. Does it cause
> problems?

All undefined values are reserved for future standardization;
the meaning they might have in the future is unpredictable.

Software compatible with ACPI 6.0 is supposed to treat them as
reserved, but software compatible with a future version of ACPI
might interpret them as having some different meaning that isn't
compatible with GRUB_MEMORY_CODE.

Some companies used e820 type 12 to mean persistent memory without
getting that assigned by the ACPI WG, so that value was
contaminated.  We should probably mark 20 as contaminated too, 
given this issue.

Re: grub causing NVDIMMs to be treated as normal memory

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]

What about this patch for the passing of pram?
diff --git a/grub-core/mmap/efi/mmap.c b/grub-core/mmap/efi/mmap.c
index 900a4d6..0c03c5d 100644
--- a/grub-core/mmap/efi/mmap.c
+++ b/grub-core/mmap/efi/mmap.c
@@ -118,6 +118,12 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook,
void *hook_data,
                GRUB_MEMORY_NVS, hook_data);
          break;

+       case GRUB_EFI_PERSISTENT_MEMORY:
+         hook (desc->physical_start, desc->num_pages * 4096,
+               GRUB_MEMORY_PRAM, hook_data);
+         break;
+
+
        default:
          grub_printf ("Unknown memory type %d, considering reserved\n",
                       desc->type);
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 24a05c5..2bbfe34 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -476,6 +476,7 @@ enum grub_efi_memory_type
     GRUB_EFI_MEMORY_MAPPED_IO,
     GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE,
     GRUB_EFI_PAL_CODE,
+    GRUB_EFI_PERSISTENT_MEMORY,
     GRUB_EFI_MAX_MEMORY_TYPE
   };
 typedef enum grub_efi_memory_type grub_efi_memory_type_t;
diff --git a/include/grub/memory.h b/include/grub/memory.h
index 083cfb6..1003a9c 100644
--- a/include/grub/memory.h
+++ b/include/grub/memory.h
@@ -30,6 +30,7 @@ typedef enum grub_memory_type
     GRUB_MEMORY_ACPI = 3,
     GRUB_MEMORY_NVS = 4,
     GRUB_MEMORY_BADRAM = 5,
+    GRUB_MEMORY_PRAM = 7,
     GRUB_MEMORY_COREBOOT_TABLES = 16,
     GRUB_MEMORY_CODE = 20,
     /* This one is special: it's used internally but is never reported
>>> Note (b): The internal GRUB_MEMORY_CODE (20) value is
>>> leaking through to the E820 table.
>>>
>>> That appears to be from this patch on 2013-10-14:
>>> 	6de9ee86 Pass-through unknown E820 types
>>
>> If we are discussing ACPI 6.0 systems here, it explicitly says that
>> values above 12 should be treated as reserved. Does it cause
>> problems?
> 
> All undefined values are reserved for future standardization;
> the meaning they might have in the future is unpredictable.
> 
> Software compatible with ACPI 6.0 is supposed to treat them as
> reserved, but software compatible with a future version of ACPI
> might interpret them as having some different meaning that isn't
> compatible with GRUB_MEMORY_CODE.
> 
> Some companies used e820 type 12 to mean persistent memory without
> getting that assigned by the ACPI WG, so that value was
> contaminated.  We should probably mark 20 as contaminated too, 
> given this issue.
> 
I see now that we have leaked 16 (coreboot tables) as well. Could we
mark 16 as contaminated as well?
For memory code: should we just pass reserved in linux e820 or is it
better to keep doing this bug given possible reliance on it by other
software?
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

On Fri, Nov 27, 2015 at 2:08 PM, Vladimir 'φ-coder/phcoder' Serbinenko
<phcoder@gmail.com> wrote:
> What about this patch for the passing of pram?

It is incomplete. You need to handle make_efi_memtype() as well as
efiemu where I am not sure what is the right thing to do (and we
probably have zero chance to test it in real life anyway).

> diff --git a/grub-core/mmap/efi/mmap.c b/grub-core/mmap/efi/mmap.c
> index 900a4d6..0c03c5d 100644
> --- a/grub-core/mmap/efi/mmap.c
> +++ b/grub-core/mmap/efi/mmap.c
> @@ -118,6 +118,12 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook,
> void *hook_data,
>                 GRUB_MEMORY_NVS, hook_data);
>           break;
>
> +       case GRUB_EFI_PERSISTENT_MEMORY:
> +         hook (desc->physical_start, desc->num_pages * 4096,
> +               GRUB_MEMORY_PRAM, hook_data);
> +         break;
> +
> +

Extra empty line.

>         default:
>           grub_printf ("Unknown memory type %d, considering reserved\n",
>                        desc->type);
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index 24a05c5..2bbfe34 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h
> @@ -476,6 +476,7 @@ enum grub_efi_memory_type
>      GRUB_EFI_MEMORY_MAPPED_IO,
>      GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE,
>      GRUB_EFI_PAL_CODE,
> +    GRUB_EFI_PERSISTENT_MEMORY,
>      GRUB_EFI_MAX_MEMORY_TYPE
>    };
>  typedef enum grub_efi_memory_type grub_efi_memory_type_t;
> diff --git a/include/grub/memory.h b/include/grub/memory.h
> index 083cfb6..1003a9c 100644
> --- a/include/grub/memory.h
> +++ b/include/grub/memory.h
> @@ -30,6 +30,7 @@ typedef enum grub_memory_type
>      GRUB_MEMORY_ACPI = 3,
>      GRUB_MEMORY_NVS = 4,
>      GRUB_MEMORY_BADRAM = 5,
> +    GRUB_MEMORY_PRAM = 7,

GRUB_MEMORY_PERSISTENT is probably more clear.

>      GRUB_MEMORY_COREBOOT_TABLES = 16,
>      GRUB_MEMORY_CODE = 20,
>      /* This one is special: it's used internally but is never reported
>>>> Note (b): The internal GRUB_MEMORY_CODE (20) value is
>>>> leaking through to the E820 table.
>>>>
>>>> That appears to be from this patch on 2013-10-14:
>>>>     6de9ee86 Pass-through unknown E820 types
>>>
>>> If we are discussing ACPI 6.0 systems here, it explicitly says that
>>> values above 12 should be treated as reserved. Does it cause
>>> problems?
>>
>> All undefined values are reserved for future standardization;
>> the meaning they might have in the future is unpredictable.
>>
>> Software compatible with ACPI 6.0 is supposed to treat them as
>> reserved, but software compatible with a future version of ACPI
>> might interpret them as having some different meaning that isn't
>> compatible with GRUB_MEMORY_CODE.
>>
>> Some companies used e820 type 12 to mean persistent memory without
>> getting that assigned by the ACPI WG, so that value was
>> contaminated.  We should probably mark 20 as contaminated too,
>> given this issue.
>>
> I see now that we have leaked 16 (coreboot tables) as well. Could we
> mark 16 as contaminated as well?
> For memory code: should we just pass reserved in linux e820 or is it
> better to keep doing this bug given possible reliance on it by other
> software?

I think it is better to leave it as is as long as those values can be reserved.

Re: grub causing NVDIMMs to be treated as normal memory

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 1699 bytes --]

New version attached

>>      GRUB_MEMORY_COREBOOT_TABLES = 16,
>>      GRUB_MEMORY_CODE = 20,
>>      /* This one is special: it's used internally but is never reported
>>>>> Note (b): The internal GRUB_MEMORY_CODE (20) value is
>>>>> leaking through to the E820 table.
>>>>>
>>>>> That appears to be from this patch on 2013-10-14:
>>>>>     6de9ee86 Pass-through unknown E820 types
>>>>
>>>> If we are discussing ACPI 6.0 systems here, it explicitly says that
>>>> values above 12 should be treated as reserved. Does it cause
>>>> problems?
>>>
>>> All undefined values are reserved for future standardization;
>>> the meaning they might have in the future is unpredictable.
>>>
>>> Software compatible with ACPI 6.0 is supposed to treat them as
>>> reserved, but software compatible with a future version of ACPI
>>> might interpret them as having some different meaning that isn't
>>> compatible with GRUB_MEMORY_CODE.
>>>
>>> Some companies used e820 type 12 to mean persistent memory without
>>> getting that assigned by the ACPI WG, so that value was
>>> contaminated.  We should probably mark 20 as contaminated too,
>>> given this issue.
>>>
>> I see now that we have leaked 16 (coreboot tables) as well. Could we
>> mark 16 as contaminated as well?
>> For memory code: should we just pass reserved in linux e820 or is it
>> better to keep doing this bug given possible reliance on it by other
>> software?
> 
> I think it is better to leave it as is as long as those values can be reserved.
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 


[-- Attachment #1.2: pram.diff --]
[-- Type: application/x-patch, Size: 1643 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]

27.11.2015 16:55, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
> New version attached
> 

For completeness, there is lsmmap, but it is cosmetic.

What about multiboot(2)? It lists possible memory types. Do they
constitute binding API?

#define MULTIBOOT_MEMORY_AVAILABLE              1
#define MULTIBOOT_MEMORY_RESERVED               2
#define MULTIBOOT_MEMORY_ACPI_RECLAIMABLE       3
#define MULTIBOOT_MEMORY_NVS                    4
#define MULTIBOOT_MEMORY_BADRAM                 5


>>>      GRUB_MEMORY_COREBOOT_TABLES = 16,
>>>      GRUB_MEMORY_CODE = 20,
>>>      /* This one is special: it's used internally but is never reported
>>>>>> Note (b): The internal GRUB_MEMORY_CODE (20) value is
>>>>>> leaking through to the E820 table.
>>>>>>
>>>>>> That appears to be from this patch on 2013-10-14:
>>>>>>     6de9ee86 Pass-through unknown E820 types
>>>>>
>>>>> If we are discussing ACPI 6.0 systems here, it explicitly says that
>>>>> values above 12 should be treated as reserved. Does it cause
>>>>> problems?
>>>>
>>>> All undefined values are reserved for future standardization;
>>>> the meaning they might have in the future is unpredictable.
>>>>
>>>> Software compatible with ACPI 6.0 is supposed to treat them as
>>>> reserved, but software compatible with a future version of ACPI
>>>> might interpret them as having some different meaning that isn't
>>>> compatible with GRUB_MEMORY_CODE.
>>>>
>>>> Some companies used e820 type 12 to mean persistent memory without
>>>> getting that assigned by the ACPI WG, so that value was
>>>> contaminated.  We should probably mark 20 as contaminated too,
>>>> given this issue.
>>>>
>>> I see now that we have leaked 16 (coreboot tables) as well. Could we
>>> mark 16 as contaminated as well?
>>> For memory code: should we just pass reserved in linux e820 or is it
>>> better to keep doing this bug given possible reliance on it by other
>>> software?
>>
>> I think it is better to leave it as is as long as those values can be reserved.
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: grub-devel-bounces+elliott=hp.com@gnu.org [mailto:grub-devel-
> bounces+elliott=hp.com@gnu.org] On Behalf Of Vladimir 'f-
> coder/phcoder' Serbinenko
> Sent: Friday, November 27, 2015 5:08 AM
> To: The development of GNU GRUB <grub-devel@gnu.org>
> Subject: Re: grub causing NVDIMMs to be treated as normal memory
> 
> What about this patch for the passing of pram?
...
> --- a/include/grub/memory.h
> +++ b/include/grub/memory.h
> @@ -30,6 +30,7 @@ typedef enum grub_memory_type
>      GRUB_MEMORY_ACPI = 3,
>      GRUB_MEMORY_NVS = 4,
>      GRUB_MEMORY_BADRAM = 5,
> +    GRUB_MEMORY_PRAM = 7,

linux chose to use these abbreviations:
* PMEM for the ACPI standard type 7 
* PRAM for the non-standard type 12

You might want to use the same.

>      GRUB_MEMORY_COREBOOT_TABLES = 16,
>      GRUB_MEMORY_CODE = 20,
>      /* This one is special: it's used internally but is never
> reported


The patch works; UEFI type 14 ends up mapped into E820 type 7. With 
linux kernel 4.4rc2 with CONFIG_EFI_STUB disabled:
* UEFI memory map:
  * efi: mem51: [Persistent Memory  |   |  |NV|  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000880000000-0x0000000c7fffffff] (34 GiB for 16 GiB)
  * efi: mem52: [Persistent Memory  |   |  |NV|  |  |  |  |   |WB|WT|WC|UC] range=[0x0000001480000000-0x0000001a7fffffff] (82 GiB for 24 GiB)
* E820 entries in bios_params at startup:
  * bp: [mem 0x0000000880000000-0x0000000c7fffffff] persistent (type 7)
  * bp: [mem 0x0000001480000000-0x0000001a7fffffff] persistent (type 7)


A few more places that might need fixes:

grub-core/commands/lsmmap.c 
* names[] array - as Andre mentioned
* you might want to decode e820 type 12 there too, providing more
  visibility in case it's a legacy BIOS system that still reports
  the pre-standard e820 type 12.

grub-core/efiemu/mm.c:
* efiemu_alloc_requests reqorder[] - add GRUB_EFI_PERSISTENT_MEMORY
  (although we don't want to encourage ever allocating these
   ranges, UNUSABLE_MEMORY is already there)
* fill_hook - map GRUB_MEMORY_PMEM to GRUB_EFI_PERSISTENT_MEMORY
* grub_efiemu_mmap_iterate - handle GRUB_EFI_PERSISTENT_MEMORY.
  Also, this function includes MAX_MEMORY_TYPE in the case statement
  and doesn't have a default case... it should treat all the reserved
  types the same, not handle that one differently
* grub_efiemu_mmap_sort_and_uniq priority[] - add
  GRUB_EFI_PERSISTENT_MEMORY
* I would not bother translating e820 type 12 here 

grub-core/mmap/efi/mmap.c:
* make_efi_memtype - map GRUB_MEMORY_PMEM to GRUB_EFI_PERSISTENT_MEMORY


> >>> Note (b): The internal GRUB_MEMORY_CODE (20) value is
> >>> leaking through to the E820 table.
> >>>
> >>> That appears to be from this patch on 2013-10-14:
> >>> 	6de9ee86 Pass-through unknown E820 types
> >>
> >> If we are discussing ACPI 6.0 systems here, it explicitly says
> that
> >> values above 12 should be treated as reserved. Does it cause
> >> problems?
> >
> > All undefined values are reserved for future standardization;
> > the meaning they might have in the future is unpredictable.
> >
> > Software compatible with ACPI 6.0 is supposed to treat them as
> > reserved, but software compatible with a future version of ACPI
> > might interpret them as having some different meaning that isn't
> > compatible with GRUB_MEMORY_CODE.
> >
> > Some companies used e820 type 12 to mean persistent memory without
> > getting that assigned by the ACPI WG, so that value was
> > contaminated.  We should probably mark 20 as contaminated too,
> > given this issue.
> >
> I see now that we have leaked 16 (coreboot tables) as well. Could we
> mark 16 as contaminated as well?

Yes, if it is possible for that to make it to e820.  If there is
some reason that cannot happen, then we don't need to worry about
that value.

I will prepare an ACPI WG proposal to mark those values as "OEM
defined", since the problem has been there for several years.

> For memory code: should we just pass reserved in linux e820 or is it
> better to keep doing this bug given possible reliance on it by other
> software?

If you can define a standard meaning for 16 and 20, that'd be more
useful than marking them as OEM defined.  There will always be a mix
of software that interprets it as unusable vs. follows this new
advice.

---
Robert Elliott, HPE Persistent Memory

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> grub-core/efiemu/mm.c:
> * efiemu_alloc_requests reqorder[] - add GRUB_EFI_PERSISTENT_MEMORY
>   (although we don't want to encourage ever allocating these
>    ranges, UNUSABLE_MEMORY is already there)

Reviewing that code, since this array has one entry per 
EFI memory type:
static grub_size_t requested_memory[GRUB_EFI_MAX_MEMORY_TYPE];

but grub_efiemu_request_memalign() is the only function
that sets the entries to non-zero values, and that function
filters out GRUB_EFI_LOADER_CODE:
	if (type >= GRUB_EFI_MAX_MEMORY_TYPE || type <= GRUB_EFI_LOADER_CODE)
	  return -2;

then the efiemu_alloc_requests() reqorder[] array entry for
GRUB_EFI_LOADER_CODE is not used.

Should the <= really be <, or should that entry be removed from
the reqorder[] array?  Those lines were all created together in 
patch 5caf964d.

Regardless, I think it's best to add GRUB_EFI_PERSISTENT_MEMORY
to that filter and keep it out of the reqorder[] array, so it's
handled like GRUB_EFI_RESERVED_MEMORY_TYPE (which is the only
value < GRUB_EFI_LOADER_CODE):

  if (type <= GRUB_EFI_LOADER_CODE || type == GRUB_EFI_PERSISTENT_MEMORY ||
        type >= GRUB_EFI_MAX_MEMORY_SIZE)


---
Robert Elliott, HPE Persistent Memory

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

> > -----Original Message-----
> > From: grub-devel-bounces+elliott=hp.com@gnu.org [mailto:grub-devel-
> > bounces+elliott=hp.com@gnu.org] On Behalf Of Vladimir 'f-
> > coder/phcoder' Serbinenko
> > Sent: Friday, November 27, 2015 5:08 AM
> > To: The development of GNU GRUB <grub-devel@gnu.org>
> > Subject: Re: grub causing NVDIMMs to be treated as normal memory
> >
> > What about this patch for the passing of pram?
> ...
> > --- a/include/grub/memory.h
> > +++ b/include/grub/memory.h
> > @@ -30,6 +30,7 @@ typedef enum grub_memory_type
> >      GRUB_MEMORY_ACPI = 3,
> >      GRUB_MEMORY_NVS = 4,
> >      GRUB_MEMORY_BADRAM = 5,
> > +    GRUB_MEMORY_PRAM = 7,
...

Here is a proposed patch series expanding upon that.  I compiled
the efiemu changes but am not sure how to test them.  I did test
that lsnvimm properly displays the new type name.

I suggest bumping the version number so we have a way to
distinguish grubs that destroy NVDIMM data from ones that
don't.  It looks like grub has been stuck on "2.02~beta2" for
about two years, which is very confusing.  An update to NEWS
should be included with that patch.


Robert Elliott (4):
  Translate UEFI persistent memory type
  Add persistent memory type to efiemu
  efiemu: Handle all reserved UEFI memory map types the same way
  Bump version to 2.03

 configure.ac                |  2 +-
 grub-core/commands/lsmmap.c |  2 ++
 grub-core/efiemu/mm.c       | 25 ++++++++++++++++++++++---
 grub-core/mmap/efi/mmap.c   | 11 +++++++++++
 include/grub/efi/api.h      |  1 +
 include/grub/memory.h       |  2 ++
 6 files changed, 39 insertions(+), 4 deletions(-)

---
Robert Elliott, HPE Persistent Memory


[-- Attachment #2: 0001-Translate-UEFI-persistent-memory-type.patch --]
[-- Type: application/octet-stream, Size: 3487 bytes --]

From bd13098e80422444d60e08cb856093bf671df2bf Mon Sep 17 00:00:00 2001
From: Robert Elliott <elliott@hpe.com>
Date: Thu, 3 Dec 2015 11:38:36 -0600
Subject: [PATCH 1/4] Translate UEFI persistent memory type
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Define
* GRUB_EFI_PERSISTENT_MEMORY (UEFI memory map type 14) per UEFI 2.5
* GRUB_EFI_PMEM (E820 type 7) per ACPI 3.0
* GRUB_EFI_PRAM (E820 unofficial type 12) per ACPI 3.0

and translate GRUB_EFI_PERSISTENT_MEMORY to GRUB_EFI_PMEM in
grub_efi_mmap_iterate().

Includes
* adding the E820 names to lsmmap
* handling the E820 types in make_efi_memtype()

Suggested-by: Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>
Suggested-by: Andrei Borzenkov <arvidjaar@gmail.com>
---
 grub-core/commands/lsmmap.c |  2 ++
 grub-core/mmap/efi/mmap.c   | 11 +++++++++++
 include/grub/efi/api.h      |  1 +
 include/grub/memory.h       |  2 ++
 4 files changed, 16 insertions(+)

diff --git a/grub-core/commands/lsmmap.c b/grub-core/commands/lsmmap.c
index 4b504fd..1ff7276 100644
--- a/grub-core/commands/lsmmap.c
+++ b/grub-core/commands/lsmmap.c
@@ -37,6 +37,8 @@ static const char *names[] =
        is required to save accross hibernations.  */
     [GRUB_MEMORY_NVS] = N_("ACPI non-volatile storage RAM"),
     [GRUB_MEMORY_BADRAM] = N_("faulty RAM (BadRAM)"),
+    [GRUB_MEMORY_PMEM] = N_("persistent memory"),
+    [GRUB_MEMORY_PRAM] = N_("persistent memory (legacy)"),
     [GRUB_MEMORY_COREBOOT_TABLES] = N_("RAM holding coreboot tables"),
     [GRUB_MEMORY_CODE] = N_("RAM holding firmware code")
   };
diff --git a/grub-core/mmap/efi/mmap.c b/grub-core/mmap/efi/mmap.c
index 900a4d6..5acd405 100644
--- a/grub-core/mmap/efi/mmap.c
+++ b/grub-core/mmap/efi/mmap.c
@@ -118,6 +118,11 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,
 		GRUB_MEMORY_NVS, hook_data);
 	  break;
 
+	case GRUB_EFI_PERSISTENT_MEMORY:
+	  hook (desc->physical_start, desc->num_pages * 4096,
+		GRUB_MEMORY_PMEM, hook_data);
+	break;
+
 	default:
 	  grub_printf ("Unknown memory type %d, considering reserved\n",
 		       desc->type);
@@ -147,6 +152,12 @@ make_efi_memtype (int type)
       /* No way to remove a chunk of memory from EFI mmap.
 	 So mark it as unusable. */
     case GRUB_MEMORY_HOLE:
+    /*
+     * AllocatePages() does not support GRUB_EFI_PERSISTENT_MEMORY,
+     * so no translation for GRUB_MEMORY_PMEM or GRUB_MEMORY_PRAM.
+     */
+    case GRUB_MEMORY_PMEM:
+    case GRUB_MEMORY_PRAM:
     case GRUB_MEMORY_RESERVED:
       return GRUB_EFI_UNUSABLE_MEMORY;
 
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 24a05c5..2bbfe34 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -476,6 +476,7 @@ enum grub_efi_memory_type
     GRUB_EFI_MEMORY_MAPPED_IO,
     GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE,
     GRUB_EFI_PAL_CODE,
+    GRUB_EFI_PERSISTENT_MEMORY,
     GRUB_EFI_MAX_MEMORY_TYPE
   };
 typedef enum grub_efi_memory_type grub_efi_memory_type_t;
diff --git a/include/grub/memory.h b/include/grub/memory.h
index 083cfb6..1894c9d 100644
--- a/include/grub/memory.h
+++ b/include/grub/memory.h
@@ -30,6 +30,8 @@ typedef enum grub_memory_type
     GRUB_MEMORY_ACPI = 3,
     GRUB_MEMORY_NVS = 4,
     GRUB_MEMORY_BADRAM = 5,
+    GRUB_MEMORY_PMEM = 7,
+    GRUB_MEMORY_PRAM = 12,
     GRUB_MEMORY_COREBOOT_TABLES = 16,
     GRUB_MEMORY_CODE = 20,
     /* This one is special: it's used internally but is never reported
-- 
2.4.3


[-- Attachment #3: 0002-Add-persistent-memory-type-to-efiemu.patch --]
[-- Type: application/octet-stream, Size: 2626 bytes --]

From ab33f80b8f81b547ca5a70cca301de55396a7b24 Mon Sep 17 00:00:00 2001
From: Robert Elliott <elliott@hpe.com>
Date: Thu, 3 Dec 2015 11:40:34 -0600
Subject: [PATCH 2/4] Add persistent memory type to efiemu

Includes
* filtering the UEFI type in grub_efiemu_request_memalign()
* handling the UEFI type in grub_efiemu_mmap_iterate()
* handling the UEFI type in grub_efiemu_mmap_sort_and_uniq()
---
 grub-core/efiemu/mm.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/grub-core/efiemu/mm.c b/grub-core/efiemu/mm.c
index d4a4f3a..c843994 100644
--- a/grub-core/efiemu/mm.c
+++ b/grub-core/efiemu/mm.c
@@ -99,7 +99,8 @@ grub_efiemu_request_memalign (grub_size_t align, grub_size_t size,
   grub_size_t align_overhead;
   struct grub_efiemu_memrequest *ret, *cur, *prev;
   /* Check that the request is correct */
-  if (type >= GRUB_EFI_MAX_MEMORY_TYPE || type <= GRUB_EFI_LOADER_CODE)
+  if (type <= GRUB_EFI_LOADER_CODE || type == GRUB_EFI_PERSISTENT_MEMORY ||
+	type >= GRUB_EFI_MAX_MEMORY_TYPE)
     return -2;
 
   /* Add new size to requested size */
@@ -166,6 +167,13 @@ efiemu_alloc_requests (void)
       GRUB_EFI_MEMORY_MAPPED_IO,
       GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE,
       GRUB_EFI_PAL_CODE
+
+      /*
+       * These are not allocatable:
+       * GRUB_EFI_RESERVED_MEMORY_TYPE
+       * GRUB_EFI_PERSISTENT_MEMORY
+       * >= GRUB_EFI_MAX_MEMORY_TYPE
+       */
     };
 
   /* Compute total memory needed */
@@ -402,6 +410,10 @@ fill_hook (grub_uint64_t addr, grub_uint64_t size, grub_memory_type_t type,
 	return grub_efiemu_add_to_mmap (addr, size,
 					GRUB_EFI_ACPI_MEMORY_NVS);
 
+      case GRUB_MEMORY_PRAM:
+      case GRUB_MEMORY_PMEM:
+	return grub_efiemu_add_to_mmap (addr, size,
+					GRUB_EFI_PERSISTENT_MEMORY);
       default:
 	grub_dprintf ("efiemu",
 		      "Unknown memory type %d. Assuming unusable\n", type);
@@ -468,6 +480,12 @@ grub_efiemu_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
 	hook (efiemu_mmap[i].physical_start, efiemu_mmap[i].num_pages * 4096,
 	      GRUB_MEMORY_NVS, hook_data);
 	break;
+
+      case GRUB_EFI_PERSISTENT_MEMORY:
+	hook (efiemu_mmap[i].physical_start, efiemu_mmap[i].num_pages * 4096,
+	      GRUB_MEMORY_PMEM, hook_data);
+	break;
+
       }
 
   return 0;
@@ -503,7 +521,8 @@ grub_efiemu_mmap_sort_and_uniq (void)
       [GRUB_EFI_ACPI_MEMORY_NVS] = 3,
       [GRUB_EFI_MEMORY_MAPPED_IO] = 4,
       [GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE] = 4,
-      [GRUB_EFI_PAL_CODE] = 4
+      [GRUB_EFI_PAL_CODE] = 4,
+      [GRUB_EFI_PERSISTENT_MEMORY] = 4
     };
 
   int i, j, k, done;
-- 
2.4.3


[-- Attachment #4: 0003-efiemu-Handle-all-reserved-UEFI-memory-map-types-the.patch --]
[-- Type: application/octet-stream, Size: 1053 bytes --]

From d5402b81367475c79620cae3949c32df3ab38258 Mon Sep 17 00:00:00 2001
From: Robert Elliott <elliott@hpe.com>
Date: Thu, 3 Dec 2015 11:43:09 -0600
Subject: [PATCH 3/4] efiemu: Handle all reserved UEFI memory map types the
 same way

In grub_efiemu_mmap_iterate, handle all undefined UEFI memory map
types the same - don't handle MAX_MEMORY_TYPE (which is not really
a type) differently and ignore the rest of the undefined types.
---
 grub-core/efiemu/mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/efiemu/mm.c b/grub-core/efiemu/mm.c
index c843994..13af4ad 100644
--- a/grub-core/efiemu/mm.c
+++ b/grub-core/efiemu/mm.c
@@ -457,7 +457,7 @@ grub_efiemu_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
       case GRUB_EFI_MEMORY_MAPPED_IO:
       case GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE:
       case GRUB_EFI_PAL_CODE:
-      case GRUB_EFI_MAX_MEMORY_TYPE:
+      default:
 	hook (efiemu_mmap[i].physical_start, efiemu_mmap[i].num_pages * 4096,
 	      GRUB_MEMORY_RESERVED, hook_data);
 	break;
-- 
2.4.3


[-- Attachment #5: 0004-Bump-version-to-2.03.patch --]
[-- Type: application/octet-stream, Size: 860 bytes --]

From a7708374059439b37f1de10e222bbe95e40a5867 Mon Sep 17 00:00:00 2001
From: Robert Elliott <elliott@hpe.com>
Date: Thu, 3 Dec 2015 11:52:36 -0600
Subject: [PATCH 4/4] Bump version to 2.03

Among other changes, version 2.03 indicates the patches are in place
to not map NVDIMMs to normal memory.
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 3300545..95f94bf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -31,7 +31,7 @@ dnl (such as BUILD_CC, BUILD_CFLAGS, etc.) for the build type and variables
 dnl with the prefix "TARGET_" (such as TARGET_CC, TARGET_CFLAGS, etc.) are
 dnl used for the target type. See INSTALL for full list of variables.
 
-AC_INIT([GRUB],[2.02~beta2],[bug-grub@gnu.org])
+AC_INIT([GRUB],[2.03],[bug-grub@gnu.org])
 
 AC_CONFIG_AUX_DIR([build-aux])
 
-- 
2.4.3

Re: grub causing NVDIMMs to be treated as normal memory

[Andrei Borzenkov]

03.12.2015 20:50, Elliott, Robert (Persistent Memory) пишет:
> From bd13098e80422444d60e08cb856093bf671df2bf Mon Sep 17 00:00:00 2001
> From: Robert Elliott <elliott@hpe.com>
> Date: Thu, 3 Dec 2015 11:38:36 -0600
> Subject: [PATCH 1/4] Translate UEFI persistent memory type
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Define
> * GRUB_EFI_PERSISTENT_MEMORY (UEFI memory map type 14) per UEFI 2.5
> * GRUB_EFI_PMEM (E820 type 7) per ACPI 3.0
> * GRUB_EFI_PRAM (E820 unofficial type 12) per ACPI 3.0
> 

Well, ACPI talks about AddressRangePersistentMemory and "OEM defined".
Let's not confuse future hackers.

GRUB_MEMORY_PERSISTENT
GRUB_MEMORY_PERSISTENT_LEGACY

makes them obvious; PMEM and PRAM not. Neither PMEM nor PRAM are found
in ACPI spec.

> and translate GRUB_EFI_PERSISTENT_MEMORY to GRUB_EFI_PMEM in
> grub_efi_mmap_iterate().
> 
> Includes
> * adding the E820 names to lsmmap
> * handling the E820 types in make_efi_memtype()
> 
> Suggested-by: Vladimir 'П†-coder/phcoder' Serbinenko <phcoder@gmail.com>
> Suggested-by: Andrei Borzenkov <arvidjaar@gmail.com>
> ---
>  grub-core/commands/lsmmap.c |  2 ++
>  grub-core/mmap/efi/mmap.c   | 11 +++++++++++
>  include/grub/efi/api.h      |  1 +
>  include/grub/memory.h       |  2 ++
>  4 files changed, 16 insertions(+)
> 
> diff --git a/grub-core/commands/lsmmap.c b/grub-core/commands/lsmmap.c
> index 4b504fd..1ff7276 100644
> --- a/grub-core/commands/lsmmap.c
> +++ b/grub-core/commands/lsmmap.c
> @@ -37,6 +37,8 @@ static const char *names[] =
>         is required to save accross hibernations.  */
>      [GRUB_MEMORY_NVS] = N_("ACPI non-volatile storage RAM"),
>      [GRUB_MEMORY_BADRAM] = N_("faulty RAM (BadRAM)"),
> +    [GRUB_MEMORY_PMEM] = N_("persistent memory"),
> +    [GRUB_MEMORY_PRAM] = N_("persistent memory (legacy)"),

visually "persistent RAM" and "persistent RAM (legacy)" aligns better
with surrounding text.

>      [GRUB_MEMORY_COREBOOT_TABLES] = N_("RAM holding coreboot tables"),
>      [GRUB_MEMORY_CODE] = N_("RAM holding firmware code")
>    };
> diff --git a/grub-core/mmap/efi/mmap.c b/grub-core/mmap/efi/mmap.c
> index 900a4d6..5acd405 100644
> --- a/grub-core/mmap/efi/mmap.c
> +++ b/grub-core/mmap/efi/mmap.c
> @@ -118,6 +118,11 @@ grub_efi_mmap_iterate (grub_memory_hook_t hook, void *hook_data,
>  		GRUB_MEMORY_NVS, hook_data);
>  	  break;
>  
> +	case GRUB_EFI_PERSISTENT_MEMORY:
> +	  hook (desc->physical_start, desc->num_pages * 4096,
> +		GRUB_MEMORY_PMEM, hook_data);
> +	break;
> +
>  	default:
>  	  grub_printf ("Unknown memory type %d, considering reserved\n",
>  		       desc->type);
> @@ -147,6 +152,12 @@ make_efi_memtype (int type)
>        /* No way to remove a chunk of memory from EFI mmap.
>  	 So mark it as unusable. */
>      case GRUB_MEMORY_HOLE:
> +    /*
> +     * AllocatePages() does not support GRUB_EFI_PERSISTENT_MEMORY,
> +     * so no translation for GRUB_MEMORY_PMEM or GRUB_MEMORY_PRAM.
> +     */
> +    case GRUB_MEMORY_PMEM:
> +    case GRUB_MEMORY_PRAM:
>      case GRUB_MEMORY_RESERVED:
>        return GRUB_EFI_UNUSABLE_MEMORY;
>  
> diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> index 24a05c5..2bbfe34 100644
> --- a/include/grub/efi/api.h
> +++ b/include/grub/efi/api.h
> @@ -476,6 +476,7 @@ enum grub_efi_memory_type
>      GRUB_EFI_MEMORY_MAPPED_IO,
>      GRUB_EFI_MEMORY_MAPPED_IO_PORT_SPACE,
>      GRUB_EFI_PAL_CODE,
> +    GRUB_EFI_PERSISTENT_MEMORY,
>      GRUB_EFI_MAX_MEMORY_TYPE
>    };
>  typedef enum grub_efi_memory_type grub_efi_memory_type_t;
> diff --git a/include/grub/memory.h b/include/grub/memory.h
> index 083cfb6..1894c9d 100644
> --- a/include/grub/memory.h
> +++ b/include/grub/memory.h
> @@ -30,6 +30,8 @@ typedef enum grub_memory_type
>      GRUB_MEMORY_ACPI = 3,
>      GRUB_MEMORY_NVS = 4,
>      GRUB_MEMORY_BADRAM = 5,
> +    GRUB_MEMORY_PMEM = 7,
> +    GRUB_MEMORY_PRAM = 12,
>      GRUB_MEMORY_COREBOOT_TABLES = 16,
>      GRUB_MEMORY_CODE = 20,
>      /* This one is special: it's used internally but is never reported
> -- 2.4.3


Regarding efiemu - this is used in one very specific case - to boot OS X
kernel out of CSM emulation. It is unlikely we'll see persistent memory
on any platform where it is still needed; I doubt anyone can actually
test it, so it is probably better to leave it as is.

RE: grub causing NVDIMMs to be treated as normal memory

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: grub-devel-bounces+elliott=hp.com@gnu.org [mailto:grub-devel-
> bounces+elliott=hp.com@gnu.org] On Behalf Of Andrei Borzenkov
> Sent: Tuesday, December 08, 2015 11:16 AM
> To: grub-devel@gnu.org
> Subject: Re: grub causing NVDIMMs to be treated as normal memory
> 
> 03.12.2015 20:50, Elliott, Robert (Persistent Memory) пишет:
> > From bd13098e80422444d60e08cb856093bf671df2bf Mon Sep 17 00:00:00
> 2001
> > From: Robert Elliott <elliott@hpe.com>
> > Date: Thu, 3 Dec 2015 11:38:36 -0600
> > Subject: [PATCH 1/4] Translate UEFI persistent memory type
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > Define
> > * GRUB_EFI_PERSISTENT_MEMORY (UEFI memory map type 14) per UEFI 2.5
> > * GRUB_EFI_PMEM (E820 type 7) per ACPI 3.0
> > * GRUB_EFI_PRAM (E820 unofficial type 12) per ACPI 3.0
> >
> 
> Well, ACPI talks about AddressRangePersistentMemory and "OEM
> defined". Let's not confuse future hackers.
> 
> GRUB_MEMORY_PERSISTENT
> GRUB_MEMORY_PERSISTENT_LEGACY
> 
> makes them obvious; PMEM and PRAM not. Neither PMEM nor PRAM are
> found in ACPI spec.

I like that.

> > and translate GRUB_EFI_PERSISTENT_MEMORY to GRUB_EFI_PMEM in
> > grub_efi_mmap_iterate().
> >
> > Includes
> > * adding the E820 names to lsmmap
> > * handling the E820 types in make_efi_memtype()
> >
> > Suggested-by: Vladimir 'П†-coder/phcoder' Serbinenko
> <phcoder@gmail.com>
> > Suggested-by: Andrei Borzenkov <arvidjaar@gmail.com>
> > ---
> >  grub-core/commands/lsmmap.c |  2 ++
> >  grub-core/mmap/efi/mmap.c   | 11 +++++++++++
> >  include/grub/efi/api.h      |  1 +
> >  include/grub/memory.h       |  2 ++
> >  4 files changed, 16 insertions(+)
> >
> > diff --git a/grub-core/commands/lsmmap.c b/grub-
> core/commands/lsmmap.c
> > index 4b504fd..1ff7276 100644
> > --- a/grub-core/commands/lsmmap.c
> > +++ b/grub-core/commands/lsmmap.c
> > @@ -37,6 +37,8 @@ static const char *names[] =
> >         is required to save accross hibernations.  */
> >      [GRUB_MEMORY_NVS] = N_("ACPI non-volatile storage RAM"),
> >      [GRUB_MEMORY_BADRAM] = N_("faulty RAM (BadRAM)"),
> > +    [GRUB_MEMORY_PMEM] = N_("persistent memory"),
> > +    [GRUB_MEMORY_PRAM] = N_("persistent memory (legacy)"),
> 
> visually "persistent RAM" and "persistent RAM (legacy)" aligns better
> with surrounding text.

That'd be OK, since it is defined as "byte-accessible", which has 
the same meaning as the R in random access memory.

Based on Google hits, though. it's a much less popular term:
* "Persistent memory" has 224000 hits 
* "Persistent RAM" has 7380 hits

> Regarding efiemu - this is used in one very specific case - to boot
> OS X kernel out of CSM emulation. It is unlikely we'll see persistent
> memory on any platform where it is still needed; I doubt anyone can
> actually test it, so it is probably better to leave it as is.

I don't mind dropping the persistent -> persistent translation
given that limited use case.  I think we should continue to fix
the special handling of MAX_MEMORY_TYPE, though, since the value
may change over time, and it wouldn't make sense for this
translation to change.

What do you think about the 2.03 tag idea?

I'll post a v2 series tomorrow.

Re: grub causing NVDIMMs to be treated as normal memory

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

On 28.11.2015 07:41, Elliott, Robert (Persistent Memory) wrote:
> If you can define a standard meaning for 16 and 20, that'd be more
> useful than marking them as OEM defined.  There will always be a mix
> of software that interprets it as unusable vs. follows this new
> advice.
16 would be "RAM holding coreboot tables as defined in coreboot lbio.h"
20 would be E820 counterpart of EFI_RUNTIME_SERVICES_CODE


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]