Re: [PATCH v4 12/15] hw/nvme: Initialize capability structures for primary/secondary controllers

[Klaus Jensen]

On Jan 26 18:11, Lukasz Maniak wrote:
> From: Łukasz Gieryk 
> 
> With four new properties:
>  - sriov_v{i,q}_flexible,
>  - sriov_max_v{i,q}_per_vf,
> one can configure the number of available flexible resources, as well as
> the limits. The primary and secondary controller capability structures
> are initialized accordingly.
> 
> Since the number of available queues (interrupts) now varies between
> VF/PF, BAR size calculation is also adjusted.
> 
> Signed-off-by: Łukasz Gieryk 
> ---
>  hw/nvme/ctrl.c   | 142 ---
>  hw/nvme/nvme.h   |   4 ++
>  include/block/nvme.h |   5 ++
>  3 files changed, 144 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index e101cb7d7c..551c8795f2 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -6379,14 +6464,41 @@ static void nvme_init_state(NvmeCtrl *n)
>  n->starttime_ms = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
>  n->aer_reqs = g_new0(NvmeRequest *, n->params.aerl + 1);
>  
> -list->numcntl = cpu_to_le16(n->params.sriov_max_vfs);
> -for (i = 0; i < n->params.sriov_max_vfs; i++) {
> +list->numcntl = cpu_to_le16(max_vfs);
> +for (i = 0; i < max_vfs; i++) {
>  sctrl = &list->sec[i];
>  sctrl->pcid = cpu_to_le16(n->cntlid);
>  sctrl->vfn = cpu_to_le16(i + 1);
>  }
>  
>  cap->cntlid = cpu_to_le16(n->cntlid);
> +cap->crt = NVME_CRT_VQ | NVME_CRT_VI;
> +
> +if (pci_is_vf(&n->parent_obj)) {
> +cap->vqprt = cpu_to_le16(1 + n->conf_ioqpairs);
> +} else {
> +cap->vqprt = cpu_to_le16(1 + n->params.max_ioqpairs -
> + n->params.sriov_vq_flexible);
> +cap->vqfrt = cpu_to_le32(n->params.sriov_vq_flexible);
> +cap->vqrfap = cap->vqfrt;
> +cap->vqgran = cpu_to_le16(NVME_VF_RES_GRANULARITY);
> +cap->vqfrsm = n->params.sriov_max_vq_per_vf ?
> +cpu_to_le16(n->params.sriov_max_vq_per_vf) :
> +cap->vqfrt / n->params.sriov_max_vfs;

Getting a division by zero on non-sriov enabled controllers here.

> +}
> +
> +if (pci_is_vf(&n->parent_obj)) {
> +cap->viprt = cpu_to_le16(n->conf_msix_qsize);
> +} else {
> +cap->viprt = cpu_to_le16(n->params.msix_qsize -
> + n->params.sriov_vi_flexible);
> +cap->vifrt = cpu_to_le32(n->params.sriov_vi_flexible);
> +cap->virfap = cap->vifrt;
> +cap->vigran = cpu_to_le16(NVME_VF_RES_GRANULARITY);
> +cap->vifrsm = n->params.sriov_max_vi_per_vf ?
> +cpu_to_le16(n->params.sriov_max_vi_per_vf) :
> +cap->vifrt / n->params.sriov_max_vfs;

Same here.


signature.asc
Description: PGP signature

[PATCH v4 12/15] hw/nvme: Initialize capability structures for primary/secondary controllers

[Lukasz Maniak]

From: Łukasz Gieryk 

With four new properties:
 - sriov_v{i,q}_flexible,
 - sriov_max_v{i,q}_per_vf,
one can configure the number of available flexible resources, as well as
the limits. The primary and secondary controller capability structures
are initialized accordingly.

Since the number of available queues (interrupts) now varies between
VF/PF, BAR size calculation is also adjusted.

Signed-off-by: Łukasz Gieryk 
---
 hw/nvme/ctrl.c   | 142 ---
 hw/nvme/nvme.h   |   4 ++
 include/block/nvme.h |   5 ++
 3 files changed, 144 insertions(+), 7 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index e101cb7d7c..551c8795f2 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -36,6 +36,10 @@
  *  zoned.zasl=, \
  *  zoned.auto_transition=, \
  *  sriov_max_vfs= \
+ *  sriov_vq_flexible= \
+ *  sriov_vi_flexible= \
+ *  sriov_max_vi_per_vf= \
+ *  sriov_max_vq_per_vf= \
  *  subsys=
  *  -device nvme-ns,drive=,bus=,nsid=,\
  *  zoned=, \
@@ -113,6 +117,29 @@
  *   enables reporting of both SR-IOV and ARI capabilities by the NVMe device.
  *   Virtual function controllers will not report SR-IOV capability.
  *
+ *   NOTE: Single Root I/O Virtualization support is experimental.
+ *   All the related parameters may be subject to change.
+ *
+ * - `sriov_vq_flexible`
+ *   Indicates the total number of flexible queue resources assignable to all
+ *   the secondary controllers. Implicitly sets the number of primary
+ *   controller's private resources to `(max_ioqpairs - sriov_vq_flexible)`.
+ *
+ * - `sriov_vi_flexible`
+ *   Indicates the total number of flexible interrupt resources assignable to
+ *   all the secondary controllers. Implicitly sets the number of primary
+ *   controller's private resources to `(msix_qsize - sriov_vi_flexible)`.
+ *
+ * - `sriov_max_vi_per_vf`
+ *   Indicates the maximum number of virtual interrupt resources assignable
+ *   to a secondary controller. The default 0 resolves to
+ *   `(sriov_vi_flexible / sriov_max_vfs)`.
+ *
+ * - `sriov_max_vq_per_vf`
+ *   Indicates the maximum number of virtual queue resources assignable to
+ *   a secondary controller. The default 0 resolves to
+ *   `(sriov_vq_flexible / sriov_max_vfs)`.
+ *
  * nvme namespace device parameters
  * 
  * - `shared`
@@ -184,6 +211,7 @@
 #define NVME_NUM_FW_SLOTS 1
 #define NVME_DEFAULT_MAX_ZA_SIZE (128 * KiB)
 #define NVME_MAX_VFS 127
+#define NVME_VF_RES_GRANULARITY 1
 #define NVME_VF_OFFSET 0x1
 #define NVME_VF_STRIDE 1
 
@@ -6359,6 +6387,54 @@ static void nvme_check_constraints(NvmeCtrl *n, Error 
**errp)
 error_setg(errp, "PMR is not supported with SR-IOV");
 return;
 }
+
+if (!params->sriov_vq_flexible || !params->sriov_vi_flexible) {
+error_setg(errp, "both sriov_vq_flexible and sriov_vi_flexible"
+   " must be set for the use of SR-IOV");
+return;
+}
+
+if (params->sriov_vq_flexible < params->sriov_max_vfs * 2) {
+error_setg(errp, "sriov_vq_flexible must be greater than or equal"
+   " to %d (sriov_max_vfs * 2)", params->sriov_max_vfs * 
2);
+return;
+}
+
+if (params->max_ioqpairs < params->sriov_vq_flexible + 2) {
+error_setg(errp, "sriov_vq_flexible - max_ioqpairs (PF-private"
+   " queue resources) must be greater than or equal to 2");
+return;
+}
+
+if (params->sriov_vi_flexible < params->sriov_max_vfs) {
+error_setg(errp, "sriov_vi_flexible must be greater than or equal"
+   " to %d (sriov_max_vfs)", params->sriov_max_vfs);
+return;
+}
+
+if (params->msix_qsize < params->sriov_vi_flexible + 1) {
+error_setg(errp, "sriov_vi_flexible - msix_qsize (PF-private"
+   " interrupt resources) must be greater than or equal"
+   " to 1");
+return;
+}
+
+if (params->sriov_max_vi_per_vf &&
+(params->sriov_max_vi_per_vf - 1) % NVME_VF_RES_GRANULARITY) {
+error_setg(errp, "sriov_max_vi_per_vf must meet:"
+   " (X - 1) %% %d == 0 and X >= 1",
+   NVME_VF_RES_GRANULARITY);
+return;
+}
+
+if (params->sriov_max_vq_per_vf &&
+(params->sriov_max_vq_per_vf < 2 ||
+ (params->sriov_max_vq_per_vf - 1) % NVME_VF_RES_GRANULARITY)) {
+error_setg(errp, "sriov_max_vq_per_vf must meet:"
+   " (X - 1) %% %d == 0 and X >= 2",
+   NVME_VF_RES_GRANULARITY);
+return;
+}
 }
 }
 
@@ -6367,10 +6443,19 @@ static void nvme_init_state(NvmeCtrl *n)
 NvmePriCtrlCap *cap = &n->pri_ctrl_cap;
 NvmeS