Re: [PATCH] hw/nvme: add new command abort case

[Klaus Jensen]

On Apr 20 11:20, Dmitry Tikhov wrote:
> NVMe command set specification for end-to-end data protection formatted
> namespace states:
> 
>     o If the Reference Tag Check bit of the PRCHK field is set to ‘1’ and
>       the namespace is formatted for Type 3 protection, then the
>       controller:
>           ▪ should not compare the protection Information Reference Tag
>             field to the computed reference tag; and
>           ▪ may ignore the ILBRT and EILBRT fields. If a command is
>             aborted as a result of the Reference Tag Check bit of the
>             PRCHK field being set to ‘1’, then that command should be
>             aborted with a status code of Invalid Protection Information,
>             but may be aborted with a status code of Invalid Field in
>             Command.
> 
> Currently qemu compares reftag in the nvme_dif_prchk function whenever
> Reference Tag Check bit is set in the command. For type 3 namespaces
> however, caller of nvme_dif_prchk - nvme_dif_check does not increment
> reftag for each subsequent logical block. That way commands incorporating
> more than one logical block for type 3 formatted namespaces with reftag
> check bit set, always fail with End-to-end Reference Tag Check Error.
> Comply with spec by handling case of set Reference Tag Check
> bit in the type 3 formatted namespace.
> 

Note the "should" and "may" in your quote. What QEMU does right now is
compliant with v1.4. That is, the reftag must NOT be incremented
- it is the same for the first and all subsequent logical blocks.

I'm a bit hesitant to follow v2.0 here, since we do not report v2.0
compliance yet. I'm honestly also a bit perplexed as to how the NVMe TWG
ended up considering this backwards compatible. As far as I can tell
this breaks current hosts that do set the reference tag check bit,
provides a valid ILBRT/EILBRT and expects it to succeed.

signature.asc
Description: PGP signature