Re: [PATCH V2 6/7] hw/block/nvme: support namespace attachment command

From: Minwoo Im <minwoo.im.dev@gmail.com>
To: Keith Busch <kbusch@kernel.org>
Cc: Klaus Jensen <its@irrelevant.dk>, Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [PATCH V2 6/7] hw/block/nvme: support namespace attachment command
Date: Sat, 27 Feb 2021 10:01:45 +0900	[thread overview]
Message-ID: <20210227010145.GA3670@localhost.localdomain> (raw)
In-Reply-To: <20210226175935.GB3949@redsun51.ssa.fujisawa.hgst.com>

On 21-02-27 02:59:35, Keith Busch wrote:
> On Thu, Feb 11, 2021 at 01:09:36AM +0900, Minwoo Im wrote:
> > @@ -183,6 +183,7 @@ static const uint32_t nvme_cse_acs[256] = {
> >      [NVME_ADM_CMD_SET_FEATURES]     = NVME_CMD_EFF_CSUPP,
> >      [NVME_ADM_CMD_GET_FEATURES]     = NVME_CMD_EFF_CSUPP,
> >      [NVME_ADM_CMD_ASYNC_EV_REQ]     = NVME_CMD_EFF_CSUPP,
> > +    [NVME_ADM_CMD_NS_ATTACHMENT]    = NVME_CMD_EFF_CSUPP,
> 
> Missing NVME_CMD_EFF_NIC for the attachment command.

Will do that!

> >  };
> >  
> >  static const uint32_t nvme_cse_iocs_none[256];
> > @@ -3766,6 +3767,62 @@ static uint16_t nvme_aer(NvmeCtrl *n, NvmeRequest *req)
> >      return NVME_NO_COMPLETE;
> >  }
> >  
> > +static void __nvme_select_ns_iocs(NvmeCtrl *n, NvmeNamespace *ns);
> > +static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
> > +{
> > +    NvmeNamespace *ns;
> > +    NvmeCtrl *ctrl;
> > +    uint16_t list[NVME_CONTROLLER_LIST_SIZE] = {};
> > +    uint32_t nsid = le32_to_cpu(req->cmd.nsid);
> > +    uint32_t dw10 = le32_to_cpu(req->cmd.cdw10);
> > +    bool attach = !(dw10 & 0xf);
> > +    uint16_t *nr_ids = &list[0];
> > +    uint16_t *ids = &list[1];
> > +    uint16_t ret;
> > +    int i;
> > +
> > +    trace_pci_nvme_ns_attachment(nvme_cid(req), dw10 & 0xf);
> > +
> > +    ns = nvme_subsys_ns(n->subsys, nsid);
> > +    if (!ns) {
> > +        return NVME_INVALID_FIELD | NVME_DNR;
> > +    }
> > +
> > +    ret = nvme_dma(n, (uint8_t *)list, 4096,
> > +                   DMA_DIRECTION_TO_DEVICE, req);
> > +    if (ret) {
> > +        return ret;
> > +    }
> > +
> > +    if (!*nr_ids) {
> > +        return NVME_NS_CTRL_LIST_INVALID | NVME_DNR;
> > +    }
> > +
> > +    for (i = 0; i < *nr_ids; i++) {
> > +        ctrl = nvme_subsys_ctrl(n->subsys, ids[i]);
> > +        if (!ctrl) {
> > +            return NVME_NS_CTRL_LIST_INVALID | NVME_DNR;
> > +        }
> > +
> > +        if (attach) {
> > +            if (nvme_ns_is_attached(ctrl, ns)) {
> > +                return NVME_NS_ALREADY_ATTACHED | NVME_DNR;
> > +            }
> > +
> > +            nvme_ns_attach(ctrl, ns);
> > +            __nvme_select_ns_iocs(ctrl, ns);
> > +        } else {
> > +            if (!nvme_ns_is_attached(ctrl, ns)) {
> > +                return NVME_NS_NOT_ATTACHED | NVME_DNR;
> > +            }
> > +
> > +            nvme_ns_detach(ctrl, ns);
> > +        }
> > +    }
> > +
> > +    return NVME_SUCCESS;
> > +}
> 
> Every controller that has newly attached the namespace needs to emit the
> Namespace Notify AER in order for the host to react correctly to the
> command.

Okay. will prepare next series.

Thanks!