Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as read-only pmem

[Linda Knippers]

On 6/4/2016 7:01 AM, joeyli wrote:
> Hi Dan, 
> 
> Thanks for your review.
> 
> On Fri, Jun 03, 2016 at 12:27:34PM -0700, Dan Williams wrote:
>> On Fri, Jun 3, 2016 at 12:13 AM, Lee, Chun-Yi <joeyli.kernel@xxxxxxxxx> wrote:
>>> This patch adds codes to treat a volatile virtual CD region as a
>>> read-only pmem region, then read-only /dev/pmem* device can be mounted
>>> with iso9660.
>>>
>>> It's useful to work with the httpboot in EFI firmware to pull a remote
>>> ISO file to the local memory region for booting and installation.
>>>
>>> Wiki page of UEFI HTTPBoot with OVMF:
>>>         https://en.opensuse.org/UEFI_HTTPBoot_with_OVMF
>>>
>>> Signed-off-by: Lee, Chun-Yi <jlee@xxxxxxxx>
>>> Cc: Gary Lin <GLin@xxxxxxxx>
>>> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
>>> Cc: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx>
>>> Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
>>> ---
>>>  drivers/acpi/nfit.c          |  8 +++++++-
>>>  drivers/nvdimm/region_devs.c | 26 +++++++++++++++++++++++++-
>>>  include/linux/libnvdimm.h    |  2 ++
>>>  3 files changed, 34 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
>>> index 2215fc8..b100a17 100644
>>> --- a/drivers/acpi/nfit.c
>>> +++ b/drivers/acpi/nfit.c
>>> @@ -1949,6 +1949,7 @@ static int acpi_nfit_init_mapping(struct acpi_nfit_desc *acpi_desc,
>>>         switch (nfit_spa_type(spa)) {
>>>         case NFIT_SPA_PM:
>>>         case NFIT_SPA_VOLATILE:
>>> +       case NFIT_SPA_VCD:
>>>                 nd_mapping->start = memdev->address;
>>>                 nd_mapping->size = memdev->region_size;
>>>                 break;
>>
>> Why do we need to distinguish NFIT_SPA_VOLATILE vs NFIT_SPA_VCD, i.e.
>> what happens if something writes to a VCD device?
> 
> Actually I didn't try to write SPA-VCD device before. Every time I mount it
> that the system responses read-only:
> 
> # mount /dev/pmem0 /mnt/
> mount: /dev/pmem0 is write-protected, mounting read-only
> 
> If it can be written, then I think there have no difference between
> NFIT_SPA_VOLATILE with NFIT_SPA_VCD region.

It's not clear to me what the expectations for this type of device are, or
whether they should be read-only.  The ACPI spec is not helpful here.
The other Disk Region and CD Region types are also unclear.  Anyone
care to define them?

> I implemented this patch to treat VCD region as read-only pmem because the
> pmem region generates /dev/pmem* device that it can be mounted.

I'm a bit worried about this type of device showing up as a "pmem" device.
I realize they're described in the NFIT (not sure why but they are) but do
any of the operations that we support for other pmem devices work on these?
Do root device DSMs make any sense?  Are there other DSMs?  What will happen
if someone uses ndctl to reconfigure the device?

I'm especially concerned on systems that might have one of these devices
and also have NVDIMMs.  Do all the pmem devices get numbered different if
this device comes and goes across reboots?  (I know, we shouldn't rely on
those names but it will still confuse people.)  Can they be some other name
that better represents what they're trying to be?

> Maybe I missed. Does NFIT_SPA_VOLATILE region also generate a device in /dev
> that it can be mounted with filesystem? 

Yes.

-- ljk

> Then I think treat the VCD region as
> a read-only VOLATILE region that's also a solution.
> 
> 
> Thanks a lot!
> Joey Lee
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@xxxxxxxxxxxx
> https://lists.01.org/mailman/listinfo/linux-nvdimm
>