Clarification regarding "nvme discover" and setting IOSQES/IOCQES

[Keith Busch]

On Mon, Feb 08, 2021 at 10:59:43AM -0800, Sagi Grimberg wrote:
> 
> > Hi Sagi,
> > 
> > Yes. The Discovery Controller currently allows it. The problem, however,
> > is that the DC seems to be expecting these values to be non-zero.
> > 
> > I tried setting IOCQES=0 and IOSQES=0, which the DC allows (i.e. Prop
> > Set return status=0). However, when I follow this by a "Property Get -
> > Controller Status", the Status now shows "Controller Fatal Status (CFS)"
> > (see Base specs - Figure 79).
> 
> Yes, the oversight extends to the target that expects it (shared code
> with I/O controllers).
> 
> Does this fix your issue?

Seems like the Identify Controller SQES and CQES fields ought to be
updated too, but the spec isn't clear on this. Feels like ECN
material...

> --
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 546a10407385..a7578c601727 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2443,6 +2443,11 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl)
>  }
>  EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
> 
> +static inline bool nvme_discovery_ctrl(struct nvme_ctrl *ctrl)
> +{
> +       return ctrl->opts && ctrl->opts->discovery_nqn;
> +}
> +
>  int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
>  {
>         unsigned dev_page_min;
> @@ -2468,7 +2473,8 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl)
>                 ctrl->ctrl_config = NVME_CC_CSS_NVM;
>         ctrl->ctrl_config |= (NVME_CTRL_PAGE_SHIFT - 12) <<
> NVME_CC_MPS_SHIFT;
>         ctrl->ctrl_config |= NVME_CC_AMS_RR | NVME_CC_SHN_NONE;
> -       ctrl->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
> +       if (!nvme_discovery_ctrl(ctrl))
> +               ctrl->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
>         ctrl->ctrl_config |= NVME_CC_ENABLE;
> 
>         ret = ctrl->ops->reg_write32(ctrl, NVME_REG_CC, ctrl->ctrl_config);
> @@ -2888,11 +2894,6 @@ static const struct attribute_group
> *nvme_subsys_attrs_groups[] = {
>         NULL,
>  };
> 
> -static inline bool nvme_discovery_ctrl(struct nvme_ctrl *ctrl)
> -{
> -       return ctrl->opts && ctrl->opts->discovery_nqn;
> -}
> -
>  static bool nvme_validate_cntlid(struct nvme_subsystem *subsys,
>                 struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>  {
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 8ce4d59cc9e7..baa26a65d83d 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1107,9 +1107,14 @@ static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl)
>  {
>         lockdep_assert_held(&ctrl->lock);
> 
> -       if (nvmet_cc_iosqes(ctrl->cc) != NVME_NVM_IOSQES ||
> -           nvmet_cc_iocqes(ctrl->cc) != NVME_NVM_IOCQES ||
> -           nvmet_cc_mps(ctrl->cc) != 0 ||
> +       if (!ctrl->subsys->type == NVME_NQN_DISC &&
> +           (nvmet_cc_iosqes(ctrl->cc) != NVME_NVM_IOSQES ||
> +            nvmet_cc_iocqes(ctrl->cc) != NVME_NVM_IOCQES)) {
> +               ctrl->csts = NVME_CSTS_CFS;
> +               return;
> +       }
> +
> +       if (nvmet_cc_mps(ctrl->cc) != 0 ||
>             nvmet_cc_ams(ctrl->cc) != 0 ||
>             nvmet_cc_css(ctrl->cc) != 0) {
>                 ctrl->csts = NVME_CSTS_CFS;
> --
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme