[PATCH v2 1/2] nvme: Enable FUA

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 ---
(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;

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);

在 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;
} *(prp_pool + i++) = cpu_to_le64(dma_addr);i = 1; prp_pool += page_size;

在 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;

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
participants (3)
-
Bin Meng
-
Jon Lin
-
Shawn Lin