[RFC,2/2] e820: Add the NvDIMM Memory type (type-12)
diff mbox

Message ID 54E1D19B.30405@plexistor.com
State New, archived
Headers show

Commit Message

Boaz Harrosh Feb. 16, 2015, 11:16 a.m. UTC
There are multiple vendors of DDR3 NvDIMMs out in the market today.
At various stages of development/production. It is estimated that
there are already more the 100ds of thousands chips sold to
testers and sites.

All the vendors I know of, tagged these chips as type-12 memory.

[THIS IS A CALL FOR ALL NvDIMM Vendors if you have used other
 then type-12 NvDIMM, please email me and I will add support
 for it]

Now the ACPI comity, as far as I know, did not yet define a
standard type for NvDIMM. Also, as far as I know any NvDIMM
standard will be defined for DDR4. So DDR3 NvDIMM will continue
to be supported by the individual vendors.

I Wish and call the ACPI comity to Define that NvDIMM is type-12.
Also for DDR4

In this patch I name type-12 "NvDIMM-12", but if there
are positive responses I would like to just name it "NvDIMM",
For any kind of DDR-X.

Actually is there any reason why I should not submit the final
version as "NvDIMM" from the get go. Then if the ACPI defines
another new type-Y Memory we can name both types "NvDIMM", No?
Please comment

Signed-off-by: Boaz Harrosh <boaz@plexistor.com>
---
 arch/x86/include/uapi/asm/e820.h | 2 +-
 arch/x86/kernel/e820.c           | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Andy Lutomirski Feb. 16, 2015, 9:04 p.m. UTC | #1
On 02/16/2015 03:16 AM, Boaz Harrosh wrote:
>
> There are multiple vendors of DDR3 NvDIMMs out in the market today.
> At various stages of development/production. It is estimated that
> there are already more the 100ds of thousands chips sold to
> testers and sites.
>
> All the vendors I know of, tagged these chips as type-12 memory.
>
> [THIS IS A CALL FOR ALL NvDIMM Vendors if you have used other
>   then type-12 NvDIMM, please email me and I will add support
>   for it]
>
> Now the ACPI comity, as far as I know, did not yet define a
> standard type for NvDIMM. Also, as far as I know any NvDIMM
> standard will be defined for DDR4. So DDR3 NvDIMM will continue
> to be supported by the individual vendors.
>
> I Wish and call the ACPI comity to Define that NvDIMM is type-12.
> Also for DDR4

I really really hope you're kidding.

As a proud (?) owner of a couple of these devices, holy crap the type 12 
enumeration mechanism is terrible.  Let's see:

1. Some GRUB versions can't handle it.  Yes, I got that fixed upstream. 
  But still.

2. UEFI can't handle it (no real e820 map means no type 12).

3. There's no way to enumerate the layout of the devices, whether 
they're working, whether they're non-volatile, who made them, how to 
talk to them (presumably via the not-yet-upstreamed i2c_imc driver, but 
that's delayed due to a whole different clusterfs*k that I'm slowly 
working on).

4. Whose bright idea was it to extend e820, for Pete's sake?

>
> In this patch I name type-12 "NvDIMM-12", but if there
> are positive responses I would like to just name it "NvDIMM",
> For any kind of DDR-X.

How about "evil non-standard device that might be an NVDIMM"?

>
> Actually is there any reason why I should not submit the final
> version as "NvDIMM" from the get go. Then if the ACPI defines
> another new type-Y Memory we can name both types "NvDIMM", No?
> Please comment

I hope that ACPI *never* defines a new e820 memory type.  Enumerate 
these things for real.

To be clear, I have no real objection to fixing the "bug" in the memory 
region code, but adding request_nonstandard_memory_region_exclusive() 
might be a better idea.

--Andy
Boaz Harrosh Feb. 17, 2015, 9:09 a.m. UTC | #2
On 02/16/2015 11:04 PM, Andy Lutomirski wrote:
> On 02/16/2015 03:16 AM, Boaz Harrosh wrote:
>>
<>
> 
> I really really hope you're kidding.
> 
> As a proud (?) owner of a couple of these devices, holy crap the type 12 
> enumeration mechanism is terrible.  Let's see:
> 
> 1. Some GRUB versions can't handle it.  Yes, I got that fixed upstream. 
>   But still.
> 
> 2. UEFI can't handle it (no real e820 map means no type 12).
> 
> 3. There's no way to enumerate the layout of the devices, whether 
> they're working, whether they're non-volatile, who made them, how to 
> talk to them (presumably via the not-yet-upstreamed i2c_imc driver, but 
> that's delayed due to a whole different clusterfs*k that I'm slowly 
> working on).
> 
> 4. Whose bright idea was it to extend e820, for Pete's sake?
> 
>>
>> In this patch I name type-12 "NvDIMM-12", but if there
>> are positive responses I would like to just name it "NvDIMM",
>> For any kind of DDR-X.
> 
> How about "evil non-standard device that might be an NVDIMM"?
> 

OK Got it I'll change the name to something like "unkown-12"
or something like that.

>>
>> Actually is there any reason why I should not submit the final
>> version as "NvDIMM" from the get go. Then if the ACPI defines
>> another new type-Y Memory we can name both types "NvDIMM", No?
>> Please comment
> 
> I hope that ACPI *never* defines a new e820 memory type.  Enumerate 
> these things for real.
> 

I kind of like it KISS style. Current code just works and there is nothing
new to write.
[I hope you guys take the lead and not completely re-invent the wheel all
 over again]

But I hear you. If there is anything I can help with please feel free ...

> To be clear, I have no real objection to fixing the "bug" in the memory 
> region code, but adding request_nonstandard_memory_region_exclusive() 
> might be a better idea.
> 

OK Funny ;-)
Please remember that even the VRAM on an PCIE card is 
request_memory_region_exclusive(), or any kind of Vendore specific card
with "memory access". So off course a DDR bus device is "memory_region"
(It is called /proc/iomem after all)

> --Andy
> 

Thanks
Boaz
Andy Lutomirski Feb. 17, 2015, 7:04 p.m. UTC | #3
On Feb 17, 2015 1:09 AM, "Boaz Harrosh" <boaz@plexistor.com> wrote:
>
> On 02/16/2015 11:04 PM, Andy Lutomirski wrote:
> > On 02/16/2015 03:16 AM, Boaz Harrosh wrote:
> >>
> <>
> >
> > I really really hope you're kidding.
> >
> > As a proud (?) owner of a couple of these devices, holy crap the type 12
> > enumeration mechanism is terrible.  Let's see:
> >
> > 1. Some GRUB versions can't handle it.  Yes, I got that fixed upstream.
> >   But still.
> >
> > 2. UEFI can't handle it (no real e820 map means no type 12).
> >
> > 3. There's no way to enumerate the layout of the devices, whether
> > they're working, whether they're non-volatile, who made them, how to
> > talk to them (presumably via the not-yet-upstreamed i2c_imc driver, but
> > that's delayed due to a whole different clusterfs*k that I'm slowly
> > working on).
> >
> > 4. Whose bright idea was it to extend e820, for Pete's sake?
> >
> >>
> >> In this patch I name type-12 "NvDIMM-12", but if there
> >> are positive responses I would like to just name it "NvDIMM",
> >> For any kind of DDR-X.
> >
> > How about "evil non-standard device that might be an NVDIMM"?
> >
>
> OK Got it I'll change the name to something like "unkown-12"
> or something like that.
>
> >>
> >> Actually is there any reason why I should not submit the final
> >> version as "NvDIMM" from the get go. Then if the ACPI defines
> >> another new type-Y Memory we can name both types "NvDIMM", No?
> >> Please comment
> >
> > I hope that ACPI *never* defines a new e820 memory type.  Enumerate
> > these things for real.
> >
>
> I kind of like it KISS style. Current code just works and there is nothing
> new to write.
> [I hope you guys take the lead and not completely re-invent the wheel all
>  over again]

The trouble is that it doesn't really just work.  My devices have a
string of NDA-required things one ought to do prior to touching the
e820-listed memory.  And, if you have NVDIMMs from multiple vendors,
or even just one that doesn't match your BIOS, then I think you lose.

>
> But I hear you. If there is anything I can help with please feel free ...

I think the scarier bits are being worked on.  I'm not really in the loop.

>
> > To be clear, I have no real objection to fixing the "bug" in the memory
> > region code, but adding request_nonstandard_memory_region_exclusive()
> > might be a better idea.
> >
>
> OK Funny ;-)
> Please remember that even the VRAM on an PCIE card is
> request_memory_region_exclusive(), or any kind of Vendore specific card
> with "memory access". So off course a DDR bus device is "memory_region"
> (It is called /proc/iomem after all)

Fair enough.

--Andy

>
> > --Andy
> >
>
> Thanks
> Boaz
>

Patch
diff mbox

diff --git a/arch/x86/include/uapi/asm/e820.h b/arch/x86/include/uapi/asm/e820.h
index d993e33..a0977e0 100644
--- a/arch/x86/include/uapi/asm/e820.h
+++ b/arch/x86/include/uapi/asm/e820.h
@@ -32,7 +32,7 @@ 
 #define E820_ACPI	3
 #define E820_NVS	4
 #define E820_UNUSABLE	5
-
+#define E820_NvDIMM_12 12
 
 /*
  * reserved RAM used by kernel itself
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 8cfd25f..17f7496 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -149,6 +149,9 @@  static void __init e820_print_type(u32 type)
 	case E820_UNUSABLE:
 		printk(KERN_CONT "unusable");
 		break;
+	case E820_NvDIMM_12:
+		printk(KERN_CONT "NvDIMM-12");
+		break;
 	default:
 		printk(KERN_CONT "type %u", type);
 		break;
@@ -908,6 +911,7 @@  static inline const char *e820_type_to_string(int e820_type)
 	case E820_NVS:	return "ACPI Non-volatile Storage";
 	case E820_UNUSABLE:	return "Unusable memory";
 	case E820_RESERVED:	return "reserved";
+	case E820_NvDIMM_12:	return "NvDIMM-12";
 	default:	return "reserved-unkown";
 	}
 }
@@ -922,6 +926,7 @@  static bool _is_reserved_type(int e820_type)
 	case E820_UNUSABLE:
 		return false;
 	case E820_RESERVED:
+	case E820_NvDIMM_12:
 	default:
 		return true;
 	}