From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031369AbbD2A3A (ORCPT ); Tue, 28 Apr 2015 20:29:00 -0400 Received: from mail-la0-f50.google.com ([209.85.215.50]:34265 "EHLO mail-la0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031166AbbD2A26 (ORCPT ); Tue, 28 Apr 2015 20:28:58 -0400 MIME-Version: 1.0 In-Reply-To: References: <20150428181203.35812.60474.stgit@dwillia2-desk3.amr.corp.intel.com> <20150428182514.35812.12126.stgit@dwillia2-desk3.amr.corp.intel.com> From: Andy Lutomirski Date: Tue, 28 Apr 2015 17:28:36 -0700 Message-ID: Subject: Re: [Linux-nvdimm] [PATCH v2 11/20] libnd, nd_pmem: add libnd support to the pmem driver To: Phil Pokorny Cc: Dan Williams , linux-nvdimm , "linux-kernel@vger.kernel.org" , Christoph Hellwig , Jens Axboe , "H. Peter Anvin" , Ingo Molnar Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 28, 2015 at 5:17 PM, Phil Pokorny wrote: > On Tue, Apr 28, 2015 at 3:58 PM, Andy Lutomirski wrote: >> On Tue, Apr 28, 2015 at 3:21 PM, Phil Pokorny >> wrote: >>> On Tue, Apr 28, 2015 at 2:04 PM, Andy Lutomirski wrote: >>>> On Tue, Apr 28, 2015 at 11:25 AM, Dan Williams wrote: >>>>> +config ND_E820 >>>>> + tristate "E820: Support the E820-type-12 PMEM convention" >>>>> + depends on X86_PMEM_LEGACY >>>>> + default m if X86_PMEM_LEGACY >>>>> + select LIBND >>>>> + help >>>>> + Prior to ACPI 6 some platforms advertised peristent memory >>>>> + via type-12 e820 memory ranges. Create a libnd bus and >>>>> + attach an instance of the pmem driver to these ranges. >>>>> + >>>> >>>> How about something like: >>>> >>>> "This driver allows libnd to work with legacy, pre-ACPI 6 NVDIMMs. >>>> This enables such devices to be exposed as block devices using PMEM. >>>> >>>> The legacy NVDIMM interface is problematic. This driver will not work >>>> if you boot using UEFI, and some NVDIMMs and motherboards that work >>>> with this driver may require proprietary code in order to work >>>> reliably." >>> >>> Perhaps not "problematic" but "requires a BIOS in Legacy mode" >>> >>> It might also mention that if you use the kernel command line >>> memmap=nn!ss syntax it adds >>> a type 12 region to the e820 map and so you would want this support. >>> >>> If you have a motherboard with UEFI support for NVDIMM's that would be >>> the recommended >>> configuration. >> >> This is such a mess that I think this driver should maybe flat-out >> refuse to load in this type of configuration without some scary module >> option. I have some NVDIMMs that report as type 12 but need two extra >> out-of-tree drivers to work safely. First, they need i2c_imc or the >> equivalent (I'll try to resubmit that soon). Second, they need secret >> magic NDAed register poking. The latter is very problematic. > > My current experience is that things may be changing to something of a de-facto > standard in the area of register poking. In which case, we should be > able to ask > the de-facto vendor standard to be released under a non-NDA license so we can > write a proper user-space library for it. Or at worst, get a > proprietary source utility > that can do the poking. > > The vendor isn't going to sell anything if they don't provide the > tools their resellers > and customers need. > I suspect that the vendor will soon be done selling this particular part as they move toward something more standard. Dunno. > >> At the very least, I think we should discourage people who don't >> really know what they're doing from using this driver without care. > > What would be the fun in that... > > But seriously, speaking as Penguin Computing and a retailer of > hardware, I'd rather > not have the kernel telling my customers what's safe and what isn't > when it's a matter > of opinion. We provide a solution with support and having to tell my > customers: "you > need to load the module with the 'THIS_IS_UNSAFE' argument set to 3" > isn't productive. It could be that you load with i_promise_i_have_an_nvdimm_driver_too=1 or, better yet, if loaded without the magic option but with the magic driver it figures it out and initializes anyway. > > Another intersesting possibility of the memmap= directive to declare a > type 12 region of > of memory is that you can test the driver (without the persistance) on > any arbitrary region > of memory in a machine. Other comments on this patch set talked about > having to put > virtual test hardware in qemu or kvm. Aside from the register poking, > just adding > memmap=xx!yy to the command line gives you something pmem can attach to and > you can use to test with. I suppose you could even simulate > persistance by saving off > the contents and restoring it on a controlled reboot. That's definitely useful. Anyway, I don't object strongly to the driver as is. Anyone with a legacy NVDIMM is already dependent on all kinds of things going right (correct power supply, ADR, all pins wired correctly, correct BIOS version, no EFI, lack of pcommit not being a problem, etc). --Andy