On May 11 17:11, Alexander Mikhalitsyn wrote:
> From: Alexander Mikhalitsyn <[email protected]>
> 
> Let's block migration for cases we don't support:
> - SR-IOV
> - CMB
> - PMR
> - SPDM
> 
> No functional changes here, because NVMe migration is
> not supported at all as of this commit.
> 
> Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Reviewed-by: Klaus Jensen <[email protected]>

> ---
>  hw/nvme/ctrl.c       | 206 +++++++++++++++++++++++++++++++++++++++++++
>  hw/nvme/nvme.h       |   3 +
>  include/block/nvme.h |  12 +++
>  3 files changed, 221 insertions(+)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index be6c7028cb5..167b8da6342 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -207,6 +207,7 @@
>  #include "hw/pci/msix.h"
>  #include "hw/pci/pcie_sriov.h"
>  #include "system/spdm-socket.h"
> +#include "migration/blocker.h"
>  #include "migration/vmstate.h"
>  
>  #include "nvme.h"
> @@ -250,6 +251,7 @@ static const bool nvme_feature_support[NVME_FID_MAX] = {
>      [NVME_COMMAND_SET_PROFILE]      = true,
>      [NVME_FDP_MODE]                 = true,
>      [NVME_FDP_EVENTS]               = true,
> +    /* if you add something here, please update 
> nvme_set_migration_blockers() */
>  };
>  
>  static const uint32_t nvme_feature_cap[NVME_FID_MAX] = {
> @@ -4601,6 +4603,7 @@ static uint16_t nvme_io_mgmt_send(NvmeCtrl *n, 
> NvmeRequest *req)
>          return 0;
>      case NVME_IOMS_MO_RUH_UPDATE:
>          return nvme_io_mgmt_send_ruh_update(n, req);
> +    /* if you add something here, please update 
> nvme_set_migration_blockers() */
>      default:
>          return NVME_INVALID_FIELD | NVME_DNR;
>      };
> @@ -7520,6 +7523,10 @@ static uint16_t nvme_security_receive(NvmeCtrl *n, 
> NvmeRequest *req)
>  
>  static uint16_t nvme_directive_send(NvmeCtrl *n, NvmeRequest *req)
>  {
> +    /*
> +     * When adding a new dtype handling here,
> +     * please also update nvme_set_migration_blockers().
> +     */
>      return NVME_INVALID_FIELD | NVME_DNR;
>  }
>  
> @@ -9210,6 +9217,199 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
> *pci_dev)
>      }
>  }
>  
> +#define BLOCKER_FEATURES_MAX_LEN 256
> +
> +static inline void nvme_add_blocker_feature(char *blocker_features, const 
> char *feature)
> +{
> +    if (strlen(blocker_features) > 0) {
> +        g_strlcat(blocker_features, ", ", BLOCKER_FEATURES_MAX_LEN);
> +    }
> +    g_strlcat(blocker_features, feature, BLOCKER_FEATURES_MAX_LEN);
> +}
> +
> +static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev, 
> Error **errp)
> +{
> +    uint64_t unsupported_cap, cap = ldq_le_p(&n->bar.cap);
> +    char blocker_features[BLOCKER_FEATURES_MAX_LEN] = "";
> +    bool adm_cmd_security_checked = false;
> +    bool cmd_io_mgmt_checked = false;
> +    bool cmd_zone_checked = false;
> +
> +    /*
> +     * Idea of this function is simple, we iterate over all Command Sets and
> +     * for each supported command we provide a special handling logic to
> +     * determine if we should block migration or not.
> +     *
> +     * For instance, we have NVME_ADM_CMD_NS_ATTACHMENT and it is always
> +     * available to the guest, but if there is only 1 namespace, then it is
> +     * safe to allow migration, but if there are more, then we need to block
> +     * migration because we don't handle this in migration code yet.
> +     */
> +    for (int opcode = 0; opcode < sizeof(n->cse.acs) / 
> sizeof(n->cse.acs[0]); opcode++) {
> +        /* Is command supported? */
> +        if (!n->cse.acs[opcode]) {
> +            continue;
> +        }
> +
> +        switch (opcode) {
> +        case NVME_ADM_CMD_DELETE_SQ:
> +        case NVME_ADM_CMD_CREATE_SQ:
> +        case NVME_ADM_CMD_GET_LOG_PAGE:
> +        case NVME_ADM_CMD_DELETE_CQ:
> +        case NVME_ADM_CMD_CREATE_CQ:
> +        case NVME_ADM_CMD_IDENTIFY:
> +        case NVME_ADM_CMD_ABORT:
> +        case NVME_ADM_CMD_SET_FEATURES:
> +        case NVME_ADM_CMD_GET_FEATURES:
> +        case NVME_ADM_CMD_ASYNC_EV_REQ:
> +        case NVME_ADM_CMD_DBBUF_CONFIG:
> +        case NVME_ADM_CMD_FORMAT_NVM:
> +        case NVME_ADM_CMD_DIRECTIVE_SEND:
> +        case NVME_ADM_CMD_DIRECTIVE_RECV:
> +            break;
> +        case NVME_ADM_CMD_NS_ATTACHMENT:
> +            int namespaces_num = 0;
> +            for (int i = 1; i <= NVME_MAX_NAMESPACES; i++) {
> +                NvmeNamespace *ns = nvme_subsys_ns(n->subsys, i);
> +                if (!ns) {
> +                    continue;
> +                }
> +
> +                namespaces_num++;
> +            }
> +
> +            if (namespaces_num > 1) {
> +                nvme_add_blocker_feature(blocker_features, "Namespace 
> Attachment");
> +            }
> +
> +            break;
> +        case NVME_ADM_CMD_VIRT_MNGMT:
> +            if (n->params.sriov_max_vfs) {
> +                nvme_add_blocker_feature(blocker_features, "SR-IOV");
> +            }
> +
> +            break;
> +        case NVME_ADM_CMD_SECURITY_SEND:
> +        case NVME_ADM_CMD_SECURITY_RECV:
> +            if (adm_cmd_security_checked) {
> +                break;
> +            }
> +
> +            if (pci_dev->spdm_port) {
> +                nvme_add_blocker_feature(blocker_features, "SPDM");
> +            }
> +
> +            adm_cmd_security_checked = true;
> +
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +    }
> +
> +    for (int opcode = 0; opcode < sizeof(n->cse.iocs.nvm) / 
> sizeof(n->cse.iocs.nvm[0]); opcode++) {
> +        if (!n->cse.iocs.nvm[opcode]) {
> +            continue;
> +        }
> +
> +        switch (opcode) {
> +        case NVME_CMD_FLUSH:
> +        case NVME_CMD_WRITE:
> +        case NVME_CMD_READ:
> +        case NVME_CMD_COMPARE:
> +        case NVME_CMD_WRITE_ZEROES:
> +        case NVME_CMD_DSM:
> +        case NVME_CMD_VERIFY:
> +        case NVME_CMD_COPY:
> +            break;
> +        case NVME_CMD_IO_MGMT_RECV:
> +        case NVME_CMD_IO_MGMT_SEND:
> +            if (cmd_io_mgmt_checked) {
> +                break;
> +            }
> +
> +            /* check for NVME_IOMS_MO_RUH_UPDATE */
> +            if (n->subsys->params.fdp.enabled) {
> +                nvme_add_blocker_feature(blocker_features, "FDP");
> +            }
> +
> +            cmd_io_mgmt_checked = true;
> +
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +    }
> +
> +    for (int opcode = 0; opcode < sizeof(n->cse.iocs.zoned) / 
> sizeof(n->cse.iocs.zoned[0]); opcode++) {
> +        /*
> +         * If command isn't supported or we have the same command
> +         * in n->cse.iocs.nvm, then we can skip it here.
> +         */
> +        if (!n->cse.iocs.zoned[opcode] || n->cse.iocs.nvm[opcode]) {
> +            continue;
> +        }
> +
> +        switch (opcode) {
> +        case NVME_CMD_ZONE_APPEND:
> +        case NVME_CMD_ZONE_MGMT_SEND:
> +        case NVME_CMD_ZONE_MGMT_RECV:
> +            if (cmd_zone_checked) {
> +                break;
> +            }
> +
> +            for (int i = 1; i <= NVME_MAX_NAMESPACES; i++) {
> +                NvmeNamespace *ns = nvme_subsys_ns(n->subsys, i);
> +                if (!ns) {
> +                    continue;
> +                }
> +
> +                if (ns->params.zoned) {
> +                    nvme_add_blocker_feature(blocker_features, "Zoned 
> Namespace");
> +                    break;
> +                }
> +            }
> +
> +            cmd_zone_checked = true;
> +
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> +    }
> +
> +    /*
> +     * Try our best to explicitly detect all not supported caps,
> +     * to let users know what features cause migration to be blocked,
> +     * but in case we miss handling here, everything else will be
> +     * covered by unsupported_cap check.
> +     */
> +    if (NVME_CAP_CMBS(cap)) {
> +        nvme_add_blocker_feature(blocker_features, "CMB");
> +        cap &= ~((uint64_t)CAP_CMBS_MASK << CAP_CMBS_SHIFT);
> +    }
> +
> +    if (NVME_CAP_PMRS(cap)) {
> +        nvme_add_blocker_feature(blocker_features, "PMR");
> +        cap &= ~((uint64_t)CAP_PMRS_MASK << CAP_PMRS_SHIFT);
> +    }
> +
> +    unsupported_cap = cap & ~NVME_MIGRATION_SUPPORTED_CAP_BITS;
> +    if (unsupported_cap) {
> +        nvme_add_blocker_feature(blocker_features, "unknown capability");
> +    }
> +
> +    assert(n->migration_blocker == NULL);
> +    if (strlen(blocker_features) > 0) {
> +        error_setg(&n->migration_blocker, "Migration is not supported for 
> %s", blocker_features);
> +        if (migrate_add_blocker(&n->migration_blocker, errp) < 0) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
>  {
>      int cntlid;
> @@ -9315,6 +9515,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
> **errp)
>  
>          n->subsys->namespaces[ns->params.nsid] = ns;
>      }
> +
> +    if (!nvme_set_migration_blockers(n, pci_dev, errp)) {
> +        return;
> +    }
>  }
>  
>  static void nvme_exit(PCIDevice *pci_dev)
> @@ -9367,6 +9571,8 @@ static void nvme_exit(PCIDevice *pci_dev)
>      }
>  
>      memory_region_del_subregion(&n->bar0, &n->iomem);
> +
> +    migrate_del_blocker(&n->migration_blocker);
>  }
>  
>  static const Property nvme_props[] = {
> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> index d66f7dc82d5..457b6637249 100644
> --- a/hw/nvme/nvme.h
> +++ b/hw/nvme/nvme.h
> @@ -666,6 +666,9 @@ typedef struct NvmeCtrl {
>  
>      /* Socket mapping to SPDM over NVMe Security In/Out commands */
>      int spdm_socket;
> +
> +    /* Migration-related stuff */
> +    Error *migration_blocker;
>  } NvmeCtrl;
>  
>  typedef enum NvmeResetType {
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 9d7159ed7a7..a7f586fc801 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -141,6 +141,18 @@ enum NvmeCapMask {
>  #define NVME_CAP_SET_CMBS(cap, val)   \
>      ((cap) |= (uint64_t)((val) & CAP_CMBS_MASK)   << CAP_CMBS_SHIFT)
>  
> +#define NVME_MIGRATION_SUPPORTED_CAP_BITS ( \
> +      ((uint64_t)CAP_MQES_MASK   << CAP_MQES_SHIFT)   \
> +    | ((uint64_t)CAP_CQR_MASK    << CAP_CQR_SHIFT)    \
> +    | ((uint64_t)CAP_AMS_MASK    << CAP_AMS_SHIFT)    \
> +    | ((uint64_t)CAP_TO_MASK     << CAP_TO_SHIFT)     \
> +    | ((uint64_t)CAP_DSTRD_MASK  << CAP_DSTRD_SHIFT)  \
> +    | ((uint64_t)CAP_NSSRS_MASK  << CAP_NSSRS_SHIFT)  \
> +    | ((uint64_t)CAP_CSS_MASK    << CAP_CSS_SHIFT)    \
> +    | ((uint64_t)CAP_MPSMIN_MASK << CAP_MPSMIN_SHIFT) \
> +    | ((uint64_t)CAP_MPSMAX_MASK << CAP_MPSMAX_SHIFT) \
> +)
> +
>  enum NvmeCapCss {
>      NVME_CAP_CSS_NCSS    = 1 << 0,
>      NVME_CAP_CSS_IOCSS   = 1 << 6,
> -- 
> 2.47.3
> 
> 

Attachment: signature.asc
Description: PGP signature

Reply via email to