Re: [PATCH 1/2] nvme: updated shared header for copy command

[Minwoo Im]

Hello,

On 20-11-24 08:14:17, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Add new data structures and types for the Simple Copy command.
> 
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
> ---
>  include/block/nvme.h | 45 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index e95ff6ca9b37..b56efd6a89af 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -472,6 +472,7 @@ enum NvmeIoCommands {
>      NVME_CMD_COMPARE            = 0x05,
>      NVME_CMD_WRITE_ZEROES       = 0x08,
>      NVME_CMD_DSM                = 0x09,
> +    NVME_CMD_COPY               = 0x19,
>  };
>  
>  typedef struct QEMU_PACKED NvmeDeleteQ {
> @@ -603,6 +604,35 @@ typedef struct QEMU_PACKED NvmeDsmRange {
>      uint64_t    slba;
>  } NvmeDsmRange;
>  
> +enum {
> +    NVME_COPY_FORMAT_0 = 0x0,
> +};
> +
> +typedef struct NvmeCopyCmd {
> +    uint8_t     opcode;
> +    uint8_t     flags;
> +    uint16_t    cid;
> +    uint32_t    nsid;
> +    uint32_t    rsvd2[4];
> +    NvmeCmdDptr dptr;
> +    uint64_t    sdlba;
> +    uint32_t    cdw12;
> +    uint32_t    cdw13;
> +    uint32_t    ilbrt;
> +    uint16_t    lbat;
> +    uint16_t    lbatm;
> +} NvmeCopyCmd;
> +
> +typedef struct NvmeCopySourceRange {
> +    uint8_t  rsvd0[8];
> +    uint64_t slba;
> +    uint16_t nlb;
> +    uint8_t  rsvd18[6];
> +    uint32_t eilbrt;
> +    uint16_t elbatm;
> +    uint16_t elbat;
> +} NvmeCopySourceRange;
> +
>  enum NvmeAsyncEventRequest {
>      NVME_AER_TYPE_ERROR                     = 0,
>      NVME_AER_TYPE_SMART                     = 1,
> @@ -680,6 +710,7 @@ enum NvmeStatusCodes {
>      NVME_CONFLICTING_ATTRS      = 0x0180,
>      NVME_INVALID_PROT_INFO      = 0x0181,
>      NVME_WRITE_TO_RO            = 0x0182,
> +    NVME_CMD_SIZE_LIMIT         = 0x0183,
>      NVME_WRITE_FAULT            = 0x0280,
>      NVME_UNRECOVERED_READ       = 0x0281,
>      NVME_E2E_GUARD_ERROR        = 0x0282,
> @@ -831,7 +862,7 @@ typedef struct QEMU_PACKED NvmeIdCtrl {
>      uint8_t     nvscc;
>      uint8_t     rsvd531;
>      uint16_t    acwu;
> -    uint8_t     rsvd534[2];
> +    uint16_t    ocfs;
>      uint32_t    sgls;
>      uint8_t     rsvd540[228];
>      uint8_t     subnqn[256];
> @@ -854,6 +885,11 @@ enum NvmeIdCtrlOncs {
>      NVME_ONCS_FEATURES      = 1 << 4,
>      NVME_ONCS_RESRVATIONS   = 1 << 5,
>      NVME_ONCS_TIMESTAMP     = 1 << 6,
> +    NVME_ONCS_COPY          = 1 << 8,
> +};
> +
> +enum NvmeIdCtrlOcfs {
> +    NVME_OCFS_COPY_FORMAT_0 = 1 << NVME_COPY_FORMAT_0,

I'd prefer (1 << 0) to (1 << enum) which is more obvious and same style
with enum NvmeIdCtrlOncs.

But I'm fine with both cases.

Please add:

Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>