[PATCH v2] nvme: Check the PRINFO bit and the Metadata size before deciding the host buffer length

[PATCH v2] nvme: Check the PRINFO bit and the Metadata size before deciding the host buffer length

[Revanth Rajashekar]

According to NVMe spec v1.4, section 8.3.1, the PRINFO bit and
the metadata size play a vital role in deteriming the host buffer size.

If PRIFNO bit is set and MS==8, the host doesn't add the metadata buffer,
instead the controller adds it.

Changes from v1:
1. Remove the definition 'NVME_PRINFO_CTRL_BIT'
2. Remove variable 'control'
3. Remove test_bit and check the bit with the CPU arch's native endianness

Signed-off-by: Revanth Rajashekar <revanth.rajashekar@intel.com>
---
 drivers/nvme/host/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f320273fc672..03261e0abdcc 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1546,6 +1546,11 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 	meta_len = (io.nblocks + 1) * ns->ms;
 	metadata = nvme_to_user_ptr(io.metadata);

+	if (io.control & NVME_RW_PRINFO_PRACT && ns->ms == 8) {
+		meta_len = 0;
+		metadata = NULL;
+	}
+
 	if (ns->features & NVME_NS_EXT_LBAS) {
 		length += meta_len;
 		meta_len = 0;
--
2.17.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: Check the PRINFO bit and the Metadata size before deciding the host buffer length

[Christoph Hellwig]

> @@ -1546,6 +1546,11 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
>  	meta_len = (io.nblocks + 1) * ns->ms;
>  	metadata = nvme_to_user_ptr(io.metadata);
> 
> +	if (io.control & NVME_RW_PRINFO_PRACT && ns->ms == 8) {
> +		meta_len = 0;
> +		metadata = NULL;
> +	}
> +

Wouldn't something like:

	if ((io.control & NVME_RW_PRINFO_PRACT) &&
	    ns->ms == sizeof(struct t10_pi_tuple)) {
		/*
		 * Protection information is stripped/inserted by the
		 * controller.
		 */
		if (nvme_to_user_ptr(io.metadata))
			return -EINVAL;
		meta_len = 0;
		metadata = NULL;
	} else {
	 	meta_len = (io.nblocks + 1) * ns->ms;
		metadata = nvme_to_user_ptr(io.metadata);
	}

make a little more sense?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: Check the PRINFO bit and the Metadata size before deciding the host buffer length

[Rajashekar, Revanth]

Hi Christoph,
Thanks for reviewing.

On 1/12/2021 11:39 AM, Christoph Hellwig wrote:
>> @@ -1546,6 +1546,11 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
>>  	meta_len = (io.nblocks + 1) * ns->ms;
>>  	metadata = nvme_to_user_ptr(io.metadata);
>>
>> +	if (io.control & NVME_RW_PRINFO_PRACT && ns->ms == 8) {
>> +		meta_len = 0;
>> +		metadata = NULL;
>> +	}
>> +
> Wouldn't something like:
>
> 	if ((io.control & NVME_RW_PRINFO_PRACT) &&
> 	    ns->ms == sizeof(struct t10_pi_tuple)) {
> 		/*
> 		 * Protection information is stripped/inserted by the
> 		 * controller.
> 		 */
> 		if (nvme_to_user_ptr(io.metadata))
> 			return -EINVAL;
> 		meta_len = 0;
> 		metadata = NULL;
> 	} else {
> 	 	meta_len = (io.nblocks + 1) * ns->ms;
> 		metadata = nvme_to_user_ptr(io.metadata);
> 	}
>
> make a little more sense?
Yes! it definitely makes more sense.
Will send out a v3 for this patch.

Thanks!
Revanth

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme