All of lore.kernel.org
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: Dmitry Tikhov <d.tihov@yadro.com>
Cc: kbusch@kernel.org, ddtikhov@gmail.com, qemu-devel@nongnu.org,
	qemu-block@nongnu.org, linux@yadro.com
Subject: Re: [PATCH 2/2] hw/nvme: fix copy cmd for pi enabled namespaces
Date: Wed, 20 Apr 2022 12:04:30 +0200	[thread overview]
Message-ID: <Yl/ari5v/o8vuveT@apples> (raw)
In-Reply-To: <20220420090336.10124-3-d.tihov@yadro.com>

[-- Attachment #1: Type: text/plain, Size: 1068 bytes --]

On Apr 20 12:03, Dmitry Tikhov wrote:
> Current implementation have two problems:
> First in the read part of copy command. Because there is no metadata
> mangling before nvme_dif_check invocation, reftag error is thrown for
> blocks of namespace that have not been previously written to.

Yes, this is definitely a bug and the fix is good, thanks!

> Second in the write part. Reftag in the protection information section
> of the source metadata should not be copied as is to the destination.

Hmm, says who?

> Source range start lba and destination range start lba could differ so
> recalculation of reftag is always needed.
> 

If PRACT is 0, we really should not touch the protection information. My
interpretation of the Copy command is that it is simply just screwed if
used with PRACT 0 and Type 1. PRACT bit is specifically to allow the
controller to generate appropriate PI in this case.

On the other hand, I can totally see your interpretation as valid as
well. Let me ask some spec people about this, and I will get back to
you.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2022-04-20 10:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-20  9:03 [PATCH 0/2] Fix nvme copy command with pi metadata Dmitry Tikhov
2022-04-20  9:03 ` [PATCH 1/2] hw/nvme: refactor check of disabled dif Dmitry Tikhov
2022-04-20  9:03 ` [PATCH 2/2] hw/nvme: fix copy cmd for pi enabled namespaces Dmitry Tikhov
2022-04-20 10:04   ` Klaus Jensen [this message]
2022-04-20 19:16     ` Klaus Jensen
2022-04-21  7:41       ` Dmitry Tikhov
2022-04-21 10:13         ` Klaus Jensen

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=Yl/ari5v/o8vuveT@apples \
    --to=its@irrelevant.dk \
    --cc=d.tihov@yadro.com \
    --cc=ddtikhov@gmail.com \
    --cc=kbusch@kernel.org \
    --cc=linux@yadro.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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.