Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1412858 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] libnvdimm, nfit: treat volatile virtual CD region as read-only pmem "Lee, Chun-Yi" <joeyli.kernel@gmail.com> - 2016-06-03 09:20 +0200
    Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as  read-only pmem Dan Williams <dan.j.williams@intel.com> - 2016-06-03 21:30 +0200
      Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as  read-only pmem joeyli <jlee@suse.com> - 2016-06-04 13:10 +0200
        Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as  read-only pmem Dan Williams <dan.j.williams@intel.com> - 2016-06-04 18:30 +0200
          Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as  read-only pmem joeyli <jlee@suse.com> - 2016-06-05 16:50 +0200
          Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as  read-only pmem joeyli <jlee@suse.com> - 2016-06-05 17:00 +0200
        Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as  read-only pmem Linda Knippers <linda.knippers@hpe.com> - 2016-06-10 00:10 +0200
          Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as  read-only pmem Dan Williams <dan.j.williams@intel.com> - 2016-06-10 00:40 +0200

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

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@suse.com>
Cc: Gary Lin <GLin@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 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;
@@ -1995,7 +1996,7 @@ static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc,
 	if (nfit_spa->nd_region)
 		return 0;
 
-	if (spa->range_index == 0) {
+	if (spa->range_index == 0 && nfit_spa_type(spa) != NFIT_SPA_VCD) {
 		dev_dbg(acpi_desc->dev, "%s: detected invalid spa index\n",
 				__func__);
 		return 0;
@@ -2059,6 +2060,11 @@ static int acpi_nfit_register_region(struct acpi_nfit_desc *acpi_desc,
 				ndr_desc);
 		if (!nfit_spa->nd_region)
 			rc = -ENOMEM;
+	} else if (nfit_spa_type(spa) == NFIT_SPA_VCD) {
+		nfit_spa->nd_region = nvdimm_vcd_region_create(nvdimm_bus,
+				ndr_desc);
+		if (!nfit_spa->nd_region)
+			rc = -ENOMEM;
 	}
 
  out:
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index 40fcfea..f155941 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -56,9 +56,19 @@ static struct device_type nd_volatile_device_type = {
 	.release = nd_region_release,
 };
 
+static struct device_type nd_vcd_device_type = {
+	.name = "nd_vcd",
+	.release = nd_region_release,
+};
+
+bool is_nd_vcd(struct device *dev)
+{
+	return dev ? dev->type == &nd_vcd_device_type : false;
+}
+
 bool is_nd_pmem(struct device *dev)
 {
-	return dev ? dev->type == &nd_pmem_device_type : false;
+	return dev ? dev->type == &nd_pmem_device_type || is_nd_vcd(dev) : false;
 }
 
 bool is_nd_blk(struct device *dev)
@@ -338,6 +348,9 @@ static ssize_t read_only_store(struct device *dev,
 	int rc = strtobool(buf, &ro);
 	struct nd_region *nd_region = to_nd_region(dev);
 
+	if (is_nd_vcd(dev))
+		return -ENXIO;
+
 	if (rc)
 		return rc;
 
@@ -687,6 +700,9 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
 			ro = 1;
 	}
 
+	if (dev_type == &nd_vcd_device_type)
+		ro = 1;
+
 	if (dev_type == &nd_blk_device_type) {
 		struct nd_blk_region_desc *ndbr_desc;
 		struct nd_blk_region *ndbr;
@@ -774,6 +790,14 @@ struct nd_region *nvdimm_pmem_region_create(struct nvdimm_bus *nvdimm_bus,
 }
 EXPORT_SYMBOL_GPL(nvdimm_pmem_region_create);
 
+struct nd_region *nvdimm_vcd_region_create(struct nvdimm_bus *nvdimm_bus,
+		struct nd_region_desc *ndr_desc)
+{
+	return nd_region_create(nvdimm_bus, ndr_desc, &nd_vcd_device_type,
+			__func__);
+}
+EXPORT_SYMBOL_GPL(nvdimm_vcd_region_create);
+
 struct nd_region *nvdimm_blk_region_create(struct nvdimm_bus *nvdimm_bus,
 		struct nd_region_desc *ndr_desc)
 {
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 0c3c30c..0a1f949 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -145,6 +145,8 @@ u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
 int nvdimm_bus_check_dimm_count(struct nvdimm_bus *nvdimm_bus, int dimm_count);
 struct nd_region *nvdimm_pmem_region_create(struct nvdimm_bus *nvdimm_bus,
 		struct nd_region_desc *ndr_desc);
+struct nd_region *nvdimm_vcd_region_create(struct nvdimm_bus *nvdimm_bus,
+		struct nd_region_desc *ndr_desc);
 struct nd_region *nvdimm_blk_region_create(struct nvdimm_bus *nvdimm_bus,
 		struct nd_region_desc *ndr_desc);
 struct nd_region *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
-- 
2.1.4

[toc] | [next] | [standalone]

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

On Fri, Jun 3, 2016 at 12:13 AM, Lee, Chun-Yi <joeyli.kernel@gmail.com> 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@suse.com>
> Cc: Gary Lin <GLin@suse.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
>  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?

[toc] | [prev] | [next] | [standalone]

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

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@gmail.com> 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@suse.com>
> > Cc: Gary Lin <GLin@suse.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > ---
> >  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.

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.

Maybe I missed. Does NFIT_SPA_VOLATILE region also generate a device in /dev
that it can be mounted with filesystem? Then I think treat the VCD region as
a read-only VOLATILE region that's also a solution.


Thanks a lot!
Joey Lee

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jun 4, 2016 at 4:01 AM, joeyli <jlee@suse.com> 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@gmail.com> 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@suse.com>
>> > Cc: Gary Lin <GLin@suse.com>
>> > Cc: Dan Williams <dan.j.williams@intel.com>
>> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> > ---
>> >  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.
>
> 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.
>
> Maybe I missed. Does NFIT_SPA_VOLATILE region also generate a device in /dev
> that it can be mounted with filesystem? Then I think treat the VCD region as
> a read-only VOLATILE region that's also a solution.

My question is why does it need to be read-only?  If it's a volatile
region, does it matter if we allow writes at the block device level?
Especially if it is formatted as iso9660, it won't be writable through
the filesystem anyway.

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jun 04, 2016 at 09:24:54AM -0700, Dan Williams wrote:
> On Sat, Jun 4, 2016 at 4:01 AM, joeyli <jlee@suse.com> 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@gmail.com> 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@suse.com>
> >> > Cc: Gary Lin <GLin@suse.com>
> >> > Cc: Dan Williams <dan.j.williams@intel.com>
> >> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> > ---
> >> >  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.
> >
> > 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.
> >
> > Maybe I missed. Does NFIT_SPA_VOLATILE region also generate a device in /dev
> > that it can be mounted with filesystem? Then I think treat the VCD region as
> > a read-only VOLATILE region that's also a solution.
> 
> My question is why does it need to be read-only?  If it's a volatile
> region, does it matter if we allow writes at the block device level?
> Especially if it is formatted as iso9660, it won't be writable through
> the filesystem anyway.

The httpboot function of EFI firmware downloads iso image from http server
, then it allocates a big enough continuity memory region and extract iso to
the region.

Actually you are right, it does not need to be read-only. It's similar with
a volatile region, just the type is NFIT_SPA_VCD and it contains the data
that extracted from a iso file.

Do you have better approach to treat the SPA_VCD region as a block device
that it can be mounted by iso9660? Maybe I missed something... Can I mount a
volatile region with iso9660? If yes, maybe just treat VCD SPA as a VOLATILE
region?


Thanks a lot!
Joey Lee

[toc] | [prev] | [next] | [standalone]

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

On Sat, Jun 04, 2016 at 09:24:54AM -0700, Dan Williams wrote:
> On Sat, Jun 4, 2016 at 4:01 AM, joeyli <jlee@suse.com> 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@gmail.com> 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@suse.com>
> >> > Cc: Gary Lin <GLin@suse.com>
> >> > Cc: Dan Williams <dan.j.williams@intel.com>
> >> > Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> >> > ---
> >> >  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.
> >
> > 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.
> >
> > Maybe I missed. Does NFIT_SPA_VOLATILE region also generate a device in /dev
> > that it can be mounted with filesystem? Then I think treat the VCD region as
> > a read-only VOLATILE region that's also a solution.
> 
> My question is why does it need to be read-only?  If it's a volatile
> region, does it matter if we allow writes at the block device level?
> Especially if it is formatted as iso9660, it won't be writable through
> the filesystem anyway.

The httpboot function of EFI firmware downloads iso image from http server
, then it allocates a big enough continuity memory region and extract iso to
the region.

Actually you are right, it does not need to be read-only. It's similar with
a volatile region, just the type is NFIT_SPA_VCD and it contains the data
that extracted from a iso file.

Do you have better approach to treat the SPA_VCD region as a block device
that it can be mounted by iso9660? Maybe I missed something... Can I mount a
volatile region with iso9660? If yes, maybe just treat VCD SPA as a VOLATILE
region?


Thanks a lot!
Joey Lee

[toc] | [prev] | [next] | [standalone]

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

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@gmail.com> 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@suse.com>
>>> Cc: Gary Lin <GLin@suse.com>
>>> Cc: Dan Williams <dan.j.williams@intel.com>
>>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>> ---
>>>  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@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
> 

[toc] | [prev] | [next] | [standalone]

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

On Thu, Jun 9, 2016 at 3:08 PM, Linda Knippers <linda.knippers@hpe.com> wrote:
> 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@gmail.com> 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@suse.com>
>>>> Cc: Gary Lin <GLin@suse.com>
>>>> Cc: Dan Williams <dan.j.williams@intel.com>
>>>> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>>> ---
>>>>  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?

It would be just another pmem device, so it would support all of them.

> Do root device DSMs make any sense?

Root device DSMs take a physical address so they could apply, but I
assume a firmware could simply refuse to run any operations against
them.

> Are there other DSMs?

Not that I can think of...

> What will happen if someone uses ndctl to reconfigure the device?

It won't know or care about the difference.  Unless there's a negative
side effect of allowing writes to a "volatile cd" it would be yet
another pmem 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?

If they all go through the same driver they should have the same
naming scheme.  Software should be identifying the block device via
blkid, not kernel device name.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web