* [PATCH v2 2/2] nvme: Fix error in nvme_setup_prps
2021-09-27 13:22 [PATCH v2 1/2] nvme: Enable FUA Jon Lin
@ 2021-09-27 13:22 ` Jon Lin
2021-09-28 0:44 ` Shawn Lin
2021-09-28 0:37 ` [PATCH v2 1/2] nvme: Enable FUA Shawn Lin
2021-09-29 3:17 ` Bin Meng
2 siblings, 1 reply; 5+ messages in thread
From: Jon Lin @ 2021-09-27 13:22 UTC (permalink / raw)
To: Bin Meng; +Cc: u-boot, zyf, Kever Yang, Lin Shawn, xxm, Jon Lin
Consulting to "NVM Express® Base Specification, revision 2.0".
If more PRP List pages are required, then the last entry of
the PRP List contains the Page Base Address of the next PRP
List page. The next PRP List page shall be memory page aligned.
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---
(no changes since v1)
drivers/nvme/nvme.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 5d05cb6e9e..07d6bea83c 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -100,7 +100,7 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64 *prp2,
}
nprps = DIV_ROUND_UP(length, page_size);
- num_pages = DIV_ROUND_UP(nprps, prps_per_page);
+ num_pages = DIV_ROUND_UP(nprps + 1, prps_per_page);
if (nprps > dev->prp_entry_num) {
free(dev->prp_pool);
@@ -119,10 +119,11 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64 *prp2,
prp_pool = dev->prp_pool;
i = 0;
while (nprps) {
- if (i == ((page_size >> 3) - 1)) {
- *(prp_pool + i) = cpu_to_le64((ulong)prp_pool +
+ if (i == prps_per_page) {
+ *(prp_pool + i) = *(prp_pool + i - 1);
+ *(prp_pool + i - 1) = cpu_to_le64((ulong)prp_pool +
page_size);
- i = 0;
+ i = 1;
prp_pool += page_size;
}
*(prp_pool + i++) = cpu_to_le64(dma_addr);
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 2/2] nvme: Fix error in nvme_setup_prps
2021-09-27 13:22 ` [PATCH v2 2/2] nvme: Fix error in nvme_setup_prps Jon Lin
@ 2021-09-28 0:44 ` Shawn Lin
0 siblings, 0 replies; 5+ messages in thread
From: Shawn Lin @ 2021-09-28 0:44 UTC (permalink / raw)
To: Jon Lin; +Cc: shawn.lin, u-boot, zyf, Kever Yang, xxm, Bin Meng
在 2021/9/27 21:22, Jon Lin 写道:
> Consulting to "NVM Express® Base Specification, revision 2.0".
>
> If more PRP List pages are required, then the last entry of
> the PRP List contains the Page Base Address of the next PRP
> List page. The next PRP List page shall be memory page aligned.
>
Yep, this is indeed a bug that we try to fix, which can be reproduced
with PM981 NVMe when booting kernel.
Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---
>
> (no changes since v1)
>
> drivers/nvme/nvme.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> index 5d05cb6e9e..07d6bea83c 100644
> --- a/drivers/nvme/nvme.c
> +++ b/drivers/nvme/nvme.c
> @@ -100,7 +100,7 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64 *prp2,
> }
>
> nprps = DIV_ROUND_UP(length, page_size);
> - num_pages = DIV_ROUND_UP(nprps, prps_per_page);
> + num_pages = DIV_ROUND_UP(nprps + 1, prps_per_page);
>
> if (nprps > dev->prp_entry_num) {
> free(dev->prp_pool);
> @@ -119,10 +119,11 @@ static int nvme_setup_prps(struct nvme_dev *dev, u64 *prp2,
> prp_pool = dev->prp_pool;
> i = 0;
> while (nprps) {
> - if (i == ((page_size >> 3) - 1)) {
> - *(prp_pool + i) = cpu_to_le64((ulong)prp_pool +
> + if (i == prps_per_page) {
> + *(prp_pool + i) = *(prp_pool + i - 1);
> + *(prp_pool + i - 1) = cpu_to_le64((ulong)prp_pool +
> page_size);
> - i = 0;
> + i = 1;
> prp_pool += page_size;
> }
> *(prp_pool + i++) = cpu_to_le64(dma_addr);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] nvme: Enable FUA
2021-09-27 13:22 [PATCH v2 1/2] nvme: Enable FUA Jon Lin
2021-09-27 13:22 ` [PATCH v2 2/2] nvme: Fix error in nvme_setup_prps Jon Lin
@ 2021-09-28 0:37 ` Shawn Lin
2021-09-29 3:17 ` Bin Meng
2 siblings, 0 replies; 5+ messages in thread
From: Shawn Lin @ 2021-09-28 0:37 UTC (permalink / raw)
To: Jon Lin, Bin Meng; +Cc: shawn.lin, u-boot, zyf, Kever Yang, xxm
在 2021/9/27 21:22, Jon Lin 写道:
> Most NVME devcies maintain data in internal cache for an uncertain
> times, and u-boot has no method to force NVME to flush cache.
> So this patch adds FUA to avoid data loss caused by power off after data
> programming.
>
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> Reviewed-by: Stefan Agner <stefan@agner.ch>
This sounds reasonable, so FWIW:
Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> (no changes since v1)
>
> drivers/nvme/nvme.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> index f6465ea7f4..5d05cb6e9e 100644
> --- a/drivers/nvme/nvme.c
> +++ b/drivers/nvme/nvme.c
> @@ -761,6 +761,9 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr,
> c.rw.appmask = 0;
> c.rw.metadata = 0;
>
> + /* Always enable FUA for data integrity */
> + c.rw.control |= NVME_RW_FUA;
> +
> while (total_lbas) {
> if (total_lbas < lbas) {
> lbas = (u16)total_lbas;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/2] nvme: Enable FUA
2021-09-27 13:22 [PATCH v2 1/2] nvme: Enable FUA Jon Lin
2021-09-27 13:22 ` [PATCH v2 2/2] nvme: Fix error in nvme_setup_prps Jon Lin
2021-09-28 0:37 ` [PATCH v2 1/2] nvme: Enable FUA Shawn Lin
@ 2021-09-29 3:17 ` Bin Meng
2 siblings, 0 replies; 5+ messages in thread
From: Bin Meng @ 2021-09-29 3:17 UTC (permalink / raw)
To: Jon Lin; +Cc: U-Boot Mailing List, zyf, Kever Yang, Lin Shawn, xxm
On Mon, Sep 27, 2021 at 9:22 PM Jon Lin <jon.lin@rock-chips.com> wrote:
>
> Most NVME devcies maintain data in internal cache for an uncertain
typo: devices
> times, and u-boot has no method to force NVME to flush cache.
> So this patch adds FUA to avoid data loss caused by power off after data
> programming.
>
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> Reviewed-by: Stefan Agner <stefan@agner.ch>
> ---
>
> (no changes since v1)
>
> drivers/nvme/nvme.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> index f6465ea7f4..5d05cb6e9e 100644
> --- a/drivers/nvme/nvme.c
> +++ b/drivers/nvme/nvme.c
> @@ -761,6 +761,9 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr,
> c.rw.appmask = 0;
> c.rw.metadata = 0;
>
> + /* Always enable FUA for data integrity */
> + c.rw.control |= NVME_RW_FUA;
I don't think we should blindly enable FUA, instead we should check
whether the Volatile Write Cache is enabled or not, and if enabled,
set FUA, or just completely disable Volatile Write Cache.
> +
> while (total_lbas) {
> if (total_lbas < lbas) {
> lbas = (u16)total_lbas;
> --
Regards,
Bin
^ permalink raw reply [flat|nested] 5+ messages in thread