Skip to contentSkip to conversation

nvme: Fix PRP list calculation for non-4k system page size

PATCH Superseded Brian King 3 participants 5 messages Mar 24, 2015 – Mar 31, 2015

Subsystems (2)
Block layer NVMe
lore
Share & tools
lore

Brian King

author
#0 lore raw

PATCH 1/1 nvme: Fix PRP list calculation for non-4k system page size

From: Murali Iyer <[email protected]>

PRP list calculation is supposed to be based on device's page size. Systems with page size larger than device's page size cause corruption to the name space as well as system memory with out this fix. Systems like x86 might not experience this issue because it uses PAGE_SIZE of 4K where as powerpc uses PAGE_SIZE of 64k while NVMe device's page size varies depending upon the vendor.

Signed-off-by Murali Iyer <mniyer at us.ibm.com>
Signed-off-by Brian King <brking at linux.vnet.ibm.com>

---

 drivers/block/nvme-core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/block/nvme-core.c renamed from drivers/block/nvme-core.c~nvme_power +2 -2
diff -puN drivers/block/nvme-core.c~nvme_power drivers/block/nvme-core.c
@@ -503,12 +503,12 @@ int nvme_setup_prps(struct nvme_dev *dev
 	struct scatterlist *sg = iod->sg;
 	int dma_len = sg_dma_len(sg);
 	u64 dma_addr = sg_dma_address(sg);
-	int offset = offset_in_page(dma_addr);
+	u32 page_size = dev->page_size;
+	int offset = dma_addr % page_size;
 	__le64 *prp_list;
 	__le64 **list = iod_list(iod);
 	dma_addr_t prp_dma;
 	int nprps, i;
-	u32 page_size = dev->page_size;
 
 	length -= (page_size - offset);
 	if (length <= 0)

_

#1 lore raw

PATCH 1/1 nvme: Fix PRP list calculation for non-4k system page size

Quoted 6 lines · Brian King

PRP list calculation is supposed to be based on device's page size. Systems with page size larger than device's page size cause corruption to the name space as well as system memory with out this fix. Systems like x86 might not experience this issue because it uses PAGE_SIZE of 4K where as powerpc uses PAGE_SIZE of 64k while NVMe device's page size varies depending upon the vendor.

Darn, I thought we had this working. I must have just gotten lucky when testing on an 8k page. Thanks for the fix.

drivers/block/nvme-core.c int nvme_setup_prps(struct nvme_dev *dev · line 507

Quoted 3 lines
drivers/block/nvme-core.c renamed from drivers/block/nvme-core.c~nvme_power +2 -1
-	int offset = offset_in_page(dma_addr);
+	u32 page_size = dev->page_size;
+	int offset = dma_addr % page_size;

Trying to micro-optimize the io path to have fewer divisions. This should work out the same if changed to:

int offset = dma_addr & (page_size - 1);

The page_size is always a power of two, but I don't think the compiler knows that.

author
#2 lore raw

PATCH v2 1/1 nvme: Fix PRP list calculation for non-4k system page size

From: Murali Iyer <[email protected]>

PRP list calculation is supposed to be based on device's page size. Systems with page size larger than device's page size cause corruption to the name space as well as system memory with out this fix. Systems like x86 might not experience this issue because it uses PAGE_SIZE of 4K where as powerpc uses PAGE_SIZE of 64k while NVMe device's page size varies depending upon the vendor.

Signed-off-by Murali Iyer <mniyer at us.ibm.com>
Signed-off-by Brian King <brking at linux.vnet.ibm.com>

---

 drivers/block/nvme-core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/block/nvme-core.c renamed from drivers/block/nvme-core.c~nvme_power +2 -2
diff -puN drivers/block/nvme-core.c~nvme_power drivers/block/nvme-core.c
@@ -503,12 +503,12 @@ int nvme_setup_prps(struct nvme_dev *dev
 	struct scatterlist *sg = iod->sg;
 	int dma_len = sg_dma_len(sg);
 	u64 dma_addr = sg_dma_address(sg);
-	int offset = offset_in_page(dma_addr);
+	u32 page_size = dev->page_size;
+	int offset = dma_addr & (page_size - 1);
 	__le64 *prp_list;
 	__le64 **list = iod_list(iod);
 	dma_addr_t prp_dma;
 	int nprps, i;
-	u32 page_size = dev->page_size;
 
 	length -= (page_size - offset);
 	if (length <= 0)

_

#3 lore raw

PATCH v2 1/1 nvme: Fix PRP list calculation for non-4k system page size

Quoted 8 lines · Brian King

From: Murali Iyer <mniyer at us.ibm.com>

PRP list calculation is supposed to be based on device's page size. Systems with page size larger than device's page size cause corruption to the name space as well as system memory with out this fix. Systems like x86 might not experience this issue because it uses PAGE_SIZE of 4K where as powerpc uses PAGE_SIZE of 64k while NVMe device's page size varies depending upon the vendor.

Thanks!

Acked-by Keith Busch <keith.busch at intel.com>
Quoted 2 lines
Signed-off-by Murali Iyer <mniyer at us.ibm.com>
Signed-off-by Brian King <brking at linux.vnet.ibm.com>
maintainer
#4 lore raw

PATCH v2 1/1 nvme: Fix PRP list calculation for non-4k system page size

Quoted 11 lines · Keith Busch
Quoted 9 lines · Brian King

From: Murali Iyer <mniyer at us.ibm.com>

PRP list calculation is supposed to be based on device's page size. Systems with page size larger than device's page size cause corruption to the name space as well as system memory with out this fix. Systems like x86 might not experience this issue because it uses PAGE_SIZE of 4K where as powerpc uses PAGE_SIZE of 64k while NVMe device's
page size varies depending upon the vendor.

Quoted 3 lines

Thanks!

Acked-by Keith Busch <keith.busch at intel.com>

Added for 4.1.

Signature · 1 line
Jens Axboe