All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rao, Lei" <lei.rao@intel.com>
To: Chaitanya Kulkarni <chaitanyak@nvidia.com>,
	"kbusch@kernel.org" <kbusch@kernel.org>,
	"axboe@fb.com" <axboe@fb.com>, "hch@lst.de" <hch@lst.de>,
	"sagi@grimberg.me" <sagi@grimberg.me>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] nvme: clear the prp2 field of the nvme command.
Date: Tue, 29 Nov 2022 12:50:57 +0800	[thread overview]
Message-ID: <4cee0097-4f08-1990-112f-6e39229f59ef@intel.com> (raw)
In-Reply-To: <15de4902-03e7-0d2c-4b4c-45d713d0f1fd@nvidia.com>



On 11/29/2022 12:16 PM, Chaitanya Kulkarni wrote:
> On 11/28/22 17:47, Lei Rao wrote:
>> If the prp2 field is not filled in nvme_setup_prp_simple(), the prp2
>> field is garbage data. According to nvme spec, the prp2 is reserved if
>> the data transfer does not cross a memory page boundary. Writing a
>> reserved coded value into a controller property field produces undefined
>> results, so it needs to be cleared in nvme_setup_rw().
>>
>> Signed-off-by: Lei Rao <lei.rao@intel.com>
> 
> if it is reserved then controller shoule ignore this field, no ?

It's feasible for the controller to ignore this field. But our controller has
stricter checks, and if prp2 is not used but has a value, some warnings will be
printed. According to the NVMe spec, it seems to write a reserved field produces
an undefined result, so maybe clearing it is better.

Thanks,
Lei

> 
> not sure if original author wanted to avoid an extra assignment
> in the fast path with assumption that reserved fields should be
> ignored if it is then we should avoid this, if not then looks good
> 
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> 
> -ck
> 


  reply	other threads:[~2022-11-29  4:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-29  1:47 [PATCH] nvme: clear the prp2 field of the nvme command Lei Rao
2022-11-29  4:16 ` Chaitanya Kulkarni
2022-11-29  4:50   ` Rao, Lei [this message]
2022-11-29  8:43     ` hch
2022-11-29  8:41 ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2022-11-29  9:48 Lei Rao
2022-11-29 13:38 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4cee0097-4f08-1990-112f-6e39229f59ef@intel.com \
    --to=lei.rao@intel.com \
    --cc=axboe@fb.com \
    --cc=chaitanyak@nvidia.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.