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


Groups > linux.kernel > #1452562 > unrolled thread

[PATCH] libnvdimm, nd_blk: mask off reserved status bits

Started byRoss Zwisler <ross.zwisler@linux.intel.com>
First post2016-07-29 23:00 +0200
Last post2016-08-08 21:40 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] libnvdimm, nd_blk: mask off reserved status bits Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-07-29 23:00 +0200
    Re: [PATCH] libnvdimm, nd_blk: mask off reserved status bits joeyli <jlee@suse.com> - 2016-08-01 08:30 +0200
    [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-08-02 20:40 +0200
      Re: [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-08-08 21:20 +0200
        Re: [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits Dan Williams <dan.j.williams@intel.com> - 2016-08-08 21:40 +0200

#1452562 — [PATCH] libnvdimm, nd_blk: mask off reserved status bits

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2016-07-29 23:00 +0200
Subject[PATCH] libnvdimm, nd_blk: mask off reserved status bits
Message-ID<s0n2V-1hC-17@gated-at.bofh.it>
The "NVDIMM Block Window Driver Writer's Guide":

http://pmem.io/documents/
http://pmem.io/documents/NVDIMM_DriverWritersGuide-July-2016.pdf

defines the layout of the block window status register.  For the July 2016
version of the spec linked to above, this happens in Figure 4 on page 26.

The only bits defined in this spec are bits 31, 5, 4, 2, 1 and 0.  The rest
of the bits in the status register are reserved, and there is a warning
following the diagram that says:

  Note: The driver cannot assume the value of the RESERVED bits in the
  status register are zero. These reserved bits need to be masked off, and
  the driver must avoid checking the state of those bits.

This change ensures that for hardware implementations that set these
reserved bits in the status register, the driver won't incorrectly fail the
block I/Os.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: stable@vger.kernel.org
---
 drivers/acpi/nfit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index 1f0e060..375c10f 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -1396,11 +1396,12 @@ static u32 read_blk_stat(struct nfit_blk *nfit_blk, unsigned int bw)
 {
 	struct nfit_blk_mmio *mmio = &nfit_blk->mmio[DCR];
 	u64 offset = nfit_blk->stat_offset + mmio->size * bw;
+	const u32 STATUS_MASK = 0x80000037;
 
 	if (mmio->num_lines)
 		offset = to_interleave_offset(offset, mmio);
 
-	return readl(mmio->addr.base + offset);
+	return readl(mmio->addr.base + offset) & STATUS_MASK;
 }
 
 static void write_blk_ctl(struct nfit_blk *nfit_blk, unsigned int bw,
-- 
2.9.0

[toc] | [next] | [standalone]


#1453002

Fromjoeyli <jlee@suse.com>
Date2016-08-01 08:30 +0200
Message-ID<s1eTD-2EU-3@gated-at.bofh.it>
In reply to#1452562
Hi Ross,

On Fri, Jul 29, 2016 at 02:59:12PM -0600, Ross Zwisler wrote:
> The "NVDIMM Block Window Driver Writer's Guide":
> 
> http://pmem.io/documents/
> http://pmem.io/documents/NVDIMM_DriverWritersGuide-July-2016.pdf
> 
> defines the layout of the block window status register.  For the July 2016
> version of the spec linked to above, this happens in Figure 4 on page 26.
> 
> The only bits defined in this spec are bits 31, 5, 4, 2, 1 and 0.  The rest
> of the bits in the status register are reserved, and there is a warning
> following the diagram that says:
> 
>   Note: The driver cannot assume the value of the RESERVED bits in the
>   status register are zero. These reserved bits need to be masked off, and
>   the driver must avoid checking the state of those bits.
> 
> This change ensures that for hardware implementations that set these
> reserved bits in the status register, the driver won't incorrectly fail the
> block I/Os.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/acpi/nfit.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index 1f0e060..375c10f 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c

The drivers/acpi/nfit.c is moved to drivers/acpi/nfit/core.c

> @@ -1396,11 +1396,12 @@ static u32 read_blk_stat(struct nfit_blk *nfit_blk, unsigned int bw)
>  {
>  	struct nfit_blk_mmio *mmio = &nfit_blk->mmio[DCR];
>  	u64 offset = nfit_blk->stat_offset + mmio->size * bw;
> +	const u32 STATUS_MASK = 0x80000037;
>  
>  	if (mmio->num_lines)
>  		offset = to_interleave_offset(offset, mmio);
>  
> -	return readl(mmio->addr.base + offset);
> +	return readl(mmio->addr.base + offset) & STATUS_MASK;
>  }
>  
>  static void write_blk_ctl(struct nfit_blk *nfit_blk, unsigned int bw,
> -- 
> 2.9.0

Other parts are good to me.

Reviewed-by: Lee, Chun-Yi <jlee@suse.com>

Regards
Joey Lee

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


#1455371 — [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2016-08-02 20:40 +0200
Subject[PATCH v2] libnvdimm, nd_blk: mask off reserved status bits
Message-ID<s1MLF-8fV-63@gated-at.bofh.it>
In reply to#1452562
The "NVDIMM Block Window Driver Writer's Guide":

http://pmem.io/documents/
http://pmem.io/documents/NVDIMM_DriverWritersGuide-July-2016.pdf

defines the layout of the block window status register.  For the July 2016
version of the spec linked to above, this happens in Figure 4 on page 26.

The only bits defined in this spec are bits 31, 5, 4, 2, 1 and 0.  The rest
of the bits in the status register are reserved, and there is a warning
following the diagram that says:

  Note: The driver cannot assume the value of the RESERVED bits in the
  status register are zero. These reserved bits need to be masked off, and
  the driver must avoid checking the state of those bits.

This change ensures that for hardware implementations that set these
reserved bits in the status register, the driver won't incorrectly fail the
block I/Os.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Lee, Chun-Yi <jlee@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: stable@vger.kernel.org # v4.2+
---

Changes from V1:
 - Rebased onto the v4.8 merge tree.  As Joey points out the ND BLK code
   recently moved from drivers/acpi/nfit.c to drivers/acpi/nfit/core.c.
   Since we were in the merge window for v4.8 I didn't know what to use as
   a baseline, so I just used v4.7, which was apparently incorrect.  Sorry
   about that.

 - Added Joey's reviewed-by.

For stable kernels v4.2 and beyond the v1 patch for drivers/acpi/nfit.c
applies cleanly and should be used.

---
 drivers/acpi/nfit/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 8c234dd..80cc7c0 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1527,11 +1527,12 @@ static u32 read_blk_stat(struct nfit_blk *nfit_blk, unsigned int bw)
 {
 	struct nfit_blk_mmio *mmio = &nfit_blk->mmio[DCR];
 	u64 offset = nfit_blk->stat_offset + mmio->size * bw;
+	const u32 STATUS_MASK = 0x80000037;
 
 	if (mmio->num_lines)
 		offset = to_interleave_offset(offset, mmio);
 
-	return readl(mmio->addr.base + offset);
+	return readl(mmio->addr.base + offset) & STATUS_MASK;
 }
 
 static void write_blk_ctl(struct nfit_blk *nfit_blk, unsigned int bw,
-- 
2.9.0

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


#1458006 — Re: [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2016-08-08 21:20 +0200
SubjectRe: [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits
Message-ID<s3YfD-5sp-15@gated-at.bofh.it>
In reply to#1455371
On Tue, Aug 02, 2016 at 12:26:53PM -0600, Ross Zwisler wrote:
> The "NVDIMM Block Window Driver Writer's Guide":
> 
> http://pmem.io/documents/
> http://pmem.io/documents/NVDIMM_DriverWritersGuide-July-2016.pdf
> 
> defines the layout of the block window status register.  For the July 2016
> version of the spec linked to above, this happens in Figure 4 on page 26.
> 
> The only bits defined in this spec are bits 31, 5, 4, 2, 1 and 0.  The rest
> of the bits in the status register are reserved, and there is a warning
> following the diagram that says:
> 
>   Note: The driver cannot assume the value of the RESERVED bits in the
>   status register are zero. These reserved bits need to be masked off, and
>   the driver must avoid checking the state of those bits.
> 
> This change ensures that for hardware implementations that set these
> reserved bits in the status register, the driver won't incorrectly fail the
> block I/Os.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reviewed-by: Lee, Chun-Yi <jlee@suse.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: stable@vger.kernel.org # v4.2+

Ping on this patch - through which tree should we merge this for v4.8-rc2?
Dan's nvdimm tree?

> ---
> 
> Changes from V1:
>  - Rebased onto the v4.8 merge tree.  As Joey points out the ND BLK code
>    recently moved from drivers/acpi/nfit.c to drivers/acpi/nfit/core.c.
>    Since we were in the merge window for v4.8 I didn't know what to use as
>    a baseline, so I just used v4.7, which was apparently incorrect.  Sorry
>    about that.
> 
>  - Added Joey's reviewed-by.
> 
> For stable kernels v4.2 and beyond the v1 patch for drivers/acpi/nfit.c
> applies cleanly and should be used.
> 
> ---
>  drivers/acpi/nfit/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 8c234dd..80cc7c0 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1527,11 +1527,12 @@ static u32 read_blk_stat(struct nfit_blk *nfit_blk, unsigned int bw)
>  {
>  	struct nfit_blk_mmio *mmio = &nfit_blk->mmio[DCR];
>  	u64 offset = nfit_blk->stat_offset + mmio->size * bw;
> +	const u32 STATUS_MASK = 0x80000037;
>  
>  	if (mmio->num_lines)
>  		offset = to_interleave_offset(offset, mmio);
>  
> -	return readl(mmio->addr.base + offset);
> +	return readl(mmio->addr.base + offset) & STATUS_MASK;
>  }
>  
>  static void write_blk_ctl(struct nfit_blk *nfit_blk, unsigned int bw,
> -- 
> 2.9.0
> 

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


#1458150 — Re: [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits

FromDan Williams <dan.j.williams@intel.com>
Date2016-08-08 21:40 +0200
SubjectRe: [PATCH v2] libnvdimm, nd_blk: mask off reserved status bits
Message-ID<s3Yz1-5Ay-77@gated-at.bofh.it>
In reply to#1458006
On Mon, Aug 8, 2016 at 12:10 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Tue, Aug 02, 2016 at 12:26:53PM -0600, Ross Zwisler wrote:
>> The "NVDIMM Block Window Driver Writer's Guide":
>>
>> http://pmem.io/documents/
>> http://pmem.io/documents/NVDIMM_DriverWritersGuide-July-2016.pdf
>>
>> defines the layout of the block window status register.  For the July 2016
>> version of the spec linked to above, this happens in Figure 4 on page 26.
>>
>> The only bits defined in this spec are bits 31, 5, 4, 2, 1 and 0.  The rest
>> of the bits in the status register are reserved, and there is a warning
>> following the diagram that says:
>>
>>   Note: The driver cannot assume the value of the RESERVED bits in the
>>   status register are zero. These reserved bits need to be masked off, and
>>   the driver must avoid checking the state of those bits.
>>
>> This change ensures that for hardware implementations that set these
>> reserved bits in the status register, the driver won't incorrectly fail the
>> block I/Os.
>>
>> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Reviewed-by: Lee, Chun-Yi <jlee@suse.com>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: stable@vger.kernel.org # v4.2+
>
> Ping on this patch - through which tree should we merge this for v4.8-rc2?
> Dan's nvdimm tree?

It's getting 0-day coverage on my libnvdimm-pending branch and will go
to Linus at the end of the week.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web