Re: [PATCH v3 1/1] hw/nvme: CMIC.MCTRS should be set automatically for multi-controller subsystems or by parameter

[alan.adamson@oracle.com]

Reposting.


Alan

On 5/1/25 11:45 AM, Alan Adamson wrote:
> If there are multiple controllers in a subsystem, CMIC.MCTRS should be set to on
> for all controllers. For single controller subsystems, CMIC.MCTRS will be off by
> default. A new subsystem specific parameter will allow setting CMIC.MCTRS for
> single controller subsystems.
>
> New NVMe Subsystem QEMU Parameter (See NVMe Specification for details):
>      <subsystem>,cmic-mctrs=BOOLEAN (default: off)
>
> Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
> ---
>   hw/nvme/ctrl.c   | 15 ++++++++++++++-
>   hw/nvme/nvme.h   |  2 ++
>   hw/nvme/subsys.c |  1 +
>   3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index fd935507bc02..72e45f3a7f78 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -8880,7 +8880,20 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
>       id->psd[0].enlat = cpu_to_le32(0x10);
>       id->psd[0].exlat = cpu_to_le32(0x4);
>   
> -    id->cmic |= NVME_CMIC_MULTI_CTRL;
> +    n->subsys->total_ctrls++;
> +
> +    /* Check if there are more than 2 controllers or cmic.mctrs is enabled */
> +    if (n->subsys->params.cmic_mctrs || (n->subsys->total_ctrls > 2)) {
> +        id->cmic |= NVME_CMIC_MULTI_CTRL;
> +    } else if (n->subsys->total_ctrls == 2) {
> +        /*
> +         * When the 2nd controller on this subsys is inited, CMIC.MCTRS
> +         * needs to be set. Also need to go back and set CMIC.MCTRS
> +         * on the first controller.
> +         */
> +        id->cmic |= NVME_CMIC_MULTI_CTRL;
> +        n->subsys->ctrls[0]->id_ctrl.cmic |= NVME_CMIC_MULTI_CTRL;
> +    }
>       ctratt |= NVME_CTRATT_ENDGRPS;
>   
>       id->endgidmax = cpu_to_le16(0x1);
> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> index b5c9378ea4e5..061e7046550b 100644
> --- a/hw/nvme/nvme.h
> +++ b/hw/nvme/nvme.h
> @@ -116,7 +116,9 @@ typedef struct NvmeSubsystem {
>               uint16_t nruh;
>               uint32_t nrg;
>           } fdp;
> +        bool         cmic_mctrs;
>       } params;
> +    uint8_t          total_ctrls;
>   } NvmeSubsystem;
>   
>   int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
> diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
> index 38271d78c8bd..c644fdf0be5e 100644
> --- a/hw/nvme/subsys.c
> +++ b/hw/nvme/subsys.c
> @@ -216,6 +216,7 @@ static const Property nvme_subsystem_props[] = {
>                        NVME_DEFAULT_RU_SIZE),
>       DEFINE_PROP_UINT32("fdp.nrg", NvmeSubsystem, params.fdp.nrg, 1),
>       DEFINE_PROP_UINT16("fdp.nruh", NvmeSubsystem, params.fdp.nruh, 0),
> +    DEFINE_PROP_BOOL("cmic.mctrs", NvmeSubsystem, params.cmic_mctrs, false),
>   };
>   
>   static void nvme_subsys_class_init(ObjectClass *oc, const void *data)