On Wed, Oct 14, 2020 at 06:42:04AM +0900, Dmitry Fomichev wrote:
> From: Niklas Cassel <niklas.cas...@wdc.com>
>
> Define the structures and constants required to implement
> Namespace Types support.
>
> Namespace Types introduce a new command set, "I/O Command Sets",
> that allows the host to retrieve the command sets associated with
> a namespace. Introduce support for the command set and enable
> detection for the NVM Command Set.
>
> The new workflows for identify commands rely heavily on zero-filled
> identify structs. E.g., certain CNS commands are defined to return
> a zero-filled identify struct when an inactive namespace NSID
> is supplied.
>
> Add a helper function in order to avoid code duplication when
> reporting zero-filled identify structures.
>
> Signed-off-by: Niklas Cassel <niklas.cas...@wdc.com>
> Signed-off-by: Dmitry Fomichev <dmitry.fomic...@wdc.com>
> ---
(snip)
> @@ -2090,6 +2199,27 @@ static void nvme_clear_ctrl(NvmeCtrl *n)
> n->bar.cc = 0;
> }
>
> +static void nvme_select_ns_iocs(NvmeCtrl *n)
> +{
> + NvmeNamespace *ns;
> + int i;
> +
> + for (i = 1; i <= n->num_namespaces; i++) {
> + ns = nvme_ns(n, i);
> + if (!ns) {
> + continue;
> + }
> + ns->iocs = nvme_cse_iocs_none;
> + switch (ns->csi) {
> + case NVME_CSI_NVM:
> + if (NVME_CC_CSS(n->bar.cc) != NVME_CC_CSS_ADMIN_ONLY) {
> + ns->iocs = nvme_cse_iocs_nvm;
> + }
> + break;
> + }
> + }
> +}
> +
> static int nvme_start_ctrl(NvmeCtrl *n)
> {
> uint32_t page_bits = NVME_CC_MPS(n->bar.cc) + 12;
> @@ -2188,6 +2318,8 @@ static int nvme_start_ctrl(NvmeCtrl *n)
>
> QTAILQ_INIT(&n->aer_queue);
>
> + nvme_select_ns_iocs(n);
> +
> return 0;
> }
>
> @@ -2655,7 +2787,6 @@ int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace
> *ns, Error **errp)
> trace_pci_nvme_register_namespace(nsid);
>
> n->namespaces[nsid - 1] = ns;
> - ns->iocs = nvme_cse_iocs_nvm;
>
> return 0;
> }
Considering how tightly coupled the three above diffs are with the
Commands Supported and Effects log, and since patch 1 already adds
the ns->iocs checking in nvme_admin_cmd() and nvme_io_cmd(),
and since these three diffs are not really related to NS types,
I think they should be moved to patch 1.
It really helps the reviewer if both the ns->iocs assignment
and checking is done in the same patch, and introduced as early
as possible. And since this code is needed/valid simply to check
if ADMIN_ONLY is selected (even before NS Types were introduced),
I don't see any reason not to introduce them in to patch 1
together with the other ns->iocs stuff.
(We were always able to select a I/O Command Set using CC.CSS
(Admin only/None, or NVM), NS types simply introduced the ability
to select/enable more than one command set at the same time.)
Kind regards,
Niklas
