From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751651AbcFEOq5 (ORCPT ); Sun, 5 Jun 2016 10:46:57 -0400 Received: from smtp.nue.novell.com ([195.135.221.5]:56534 "EHLO smtp.nue.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750954AbcFEOqy (ORCPT ); Sun, 5 Jun 2016 10:46:54 -0400 Date: Sun, 5 Jun 2016 22:45:36 +0800 From: joeyli To: Dan Williams Cc: "Lee, Chun-Yi" , Ross Zwisler , "Rafael J. Wysocki" , "linux-nvdimm@lists.01.org" , Linux ACPI , "linux-kernel@vger.kernel.org" , Gary Lin Subject: Re: [PATCH] libnvdimm, nfit: treat volatile virtual CD region as read-only pmem Message-ID: <20160605144536.GB4558@linux-rxt1.site> References: <1464938015-5489-1-git-send-email-jlee@suse.com> <20160604110156.GA4558@linux-rxt1.site> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jun 04, 2016 at 09:24:54AM -0700, Dan Williams wrote: > On Sat, Jun 4, 2016 at 4: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 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 > >> > Cc: Gary Lin > >> > Cc: Dan Williams > >> > Cc: Ross Zwisler > >> > Cc: "Rafael J. Wysocki" > >> > --- > >> > 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