On Feb 17 18:44, Lukasz Maniak wrote: > From: Łukasz Gieryk <lukasz.gie...@linux.intel.com> > > The NVMe device defines two properties: max_ioqpairs, msix_qsize. Having > them as constants is problematic for SR-IOV support. > > SR-IOV introduces virtual resources (queues, interrupts) that can be > assigned to PF and its dependent VFs. Each device, following a reset, > should work with the configured number of queues. A single constant is > no longer sufficient to hold the whole state. > > This patch tries to solve the problem by introducing additional > variables in NvmeCtrl’s state. The variables for, e.g., managing queues > are therefore organized as: > - n->params.max_ioqpairs – no changes, constant set by the user > - n->(mutable_state) – (not a part of this patch) user-configurable, > specifies number of queues available _after_ > reset > - n->conf_ioqpairs - (new) used in all the places instead of the ‘old’ > n->params.max_ioqpairs; initialized in realize() > and updated during reset() to reflect user’s > changes to the mutable state > > Since the number of available i/o queues and interrupts can change in > runtime, buffers for sq/cqs and the MSIX-related structures are > allocated big enough to handle the limits, to completely avoid the > complicated reallocation. A helper function (nvme_update_msixcap_ts) > updates the corresponding capability register, to signal configuration > changes. > > Signed-off-by: Łukasz Gieryk <lukasz.gie...@linux.intel.com>
LGTM. Reviewed-by: Klaus Jensen <k.jen...@samsung.com> > --- > hw/nvme/ctrl.c | 52 ++++++++++++++++++++++++++++++++++---------------- > hw/nvme/nvme.h | 2 ++ > 2 files changed, 38 insertions(+), 16 deletions(-) > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > index 7c1dd80f21d..f1b4026e4f8 100644 > --- a/hw/nvme/ctrl.c > +++ b/hw/nvme/ctrl.c > @@ -445,12 +445,12 @@ static bool nvme_nsid_valid(NvmeCtrl *n, uint32_t nsid) > > static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid) > { > - return sqid < n->params.max_ioqpairs + 1 && n->sq[sqid] != NULL ? 0 : -1; > + return sqid < n->conf_ioqpairs + 1 && n->sq[sqid] != NULL ? 0 : -1; > } > > static int nvme_check_cqid(NvmeCtrl *n, uint16_t cqid) > { > - return cqid < n->params.max_ioqpairs + 1 && n->cq[cqid] != NULL ? 0 : -1; > + return cqid < n->conf_ioqpairs + 1 && n->cq[cqid] != NULL ? 0 : -1; > } > > static void nvme_inc_cq_tail(NvmeCQueue *cq) > @@ -4188,8 +4188,7 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeRequest > *req) > trace_pci_nvme_err_invalid_create_sq_cqid(cqid); > return NVME_INVALID_CQID | NVME_DNR; > } > - if (unlikely(!sqid || sqid > n->params.max_ioqpairs || > - n->sq[sqid] != NULL)) { > + if (unlikely(!sqid || sqid > n->conf_ioqpairs || n->sq[sqid] != NULL)) { > trace_pci_nvme_err_invalid_create_sq_sqid(sqid); > return NVME_INVALID_QID | NVME_DNR; > } > @@ -4541,8 +4540,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeRequest > *req) > trace_pci_nvme_create_cq(prp1, cqid, vector, qsize, qflags, > NVME_CQ_FLAGS_IEN(qflags) != 0); > > - if (unlikely(!cqid || cqid > n->params.max_ioqpairs || > - n->cq[cqid] != NULL)) { > + if (unlikely(!cqid || cqid > n->conf_ioqpairs || n->cq[cqid] != NULL)) { > trace_pci_nvme_err_invalid_create_cq_cqid(cqid); > return NVME_INVALID_QID | NVME_DNR; > } > @@ -4558,7 +4556,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeRequest > *req) > trace_pci_nvme_err_invalid_create_cq_vector(vector); > return NVME_INVALID_IRQ_VECTOR | NVME_DNR; > } > - if (unlikely(vector >= n->params.msix_qsize)) { > + if (unlikely(vector >= n->conf_msix_qsize)) { > trace_pci_nvme_err_invalid_create_cq_vector(vector); > return NVME_INVALID_IRQ_VECTOR | NVME_DNR; > } > @@ -5155,13 +5153,12 @@ defaults: > > break; > case NVME_NUMBER_OF_QUEUES: > - result = (n->params.max_ioqpairs - 1) | > - ((n->params.max_ioqpairs - 1) << 16); > + result = (n->conf_ioqpairs - 1) | ((n->conf_ioqpairs - 1) << 16); > trace_pci_nvme_getfeat_numq(result); > break; > case NVME_INTERRUPT_VECTOR_CONF: > iv = dw11 & 0xffff; > - if (iv >= n->params.max_ioqpairs + 1) { > + if (iv >= n->conf_ioqpairs + 1) { > return NVME_INVALID_FIELD | NVME_DNR; > } > > @@ -5316,10 +5313,10 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, > NvmeRequest *req) > > trace_pci_nvme_setfeat_numq((dw11 & 0xffff) + 1, > ((dw11 >> 16) & 0xffff) + 1, > - n->params.max_ioqpairs, > - n->params.max_ioqpairs); > - req->cqe.result = cpu_to_le32((n->params.max_ioqpairs - 1) | > - ((n->params.max_ioqpairs - 1) << 16)); > + n->conf_ioqpairs, > + n->conf_ioqpairs); > + req->cqe.result = cpu_to_le32((n->conf_ioqpairs - 1) | > + ((n->conf_ioqpairs - 1) << 16)); > break; > case NVME_ASYNCHRONOUS_EVENT_CONF: > n->features.async_config = dw11; > @@ -5757,8 +5754,24 @@ static void nvme_process_sq(void *opaque) > } > } > > +static void nvme_update_msixcap_ts(PCIDevice *pci_dev, uint32_t table_size) > +{ > + uint8_t *config; > + > + if (!msix_present(pci_dev)) { > + return; > + } > + > + assert(table_size > 0 && table_size <= pci_dev->msix_entries_nr); > + > + config = pci_dev->config + pci_dev->msix_cap; > + pci_set_word_by_mask(config + PCI_MSIX_FLAGS, PCI_MSIX_FLAGS_QSIZE, > + table_size - 1); > +} > + > static void nvme_ctrl_reset(NvmeCtrl *n, NvmeResetType rst) > { > + PCIDevice *pci_dev = &n->parent_obj; > NvmeNamespace *ns; > int i; > > @@ -5788,15 +5801,17 @@ static void nvme_ctrl_reset(NvmeCtrl *n, > NvmeResetType rst) > g_free(event); > } > > - if (!pci_is_vf(&n->parent_obj) && n->params.sriov_max_vfs) { > + if (!pci_is_vf(pci_dev) && n->params.sriov_max_vfs) { > if (rst != NVME_RESET_CONTROLLER) { > - pcie_sriov_pf_disable_vfs(&n->parent_obj); > + pcie_sriov_pf_disable_vfs(pci_dev); > } > } > > n->aer_queued = 0; > n->outstanding_aers = 0; > n->qs_created = false; > + > + nvme_update_msixcap_ts(pci_dev, n->conf_msix_qsize); > } > > static void nvme_ctrl_shutdown(NvmeCtrl *n) > @@ -6507,6 +6522,9 @@ static void nvme_init_state(NvmeCtrl *n) > NvmeSecCtrlEntry *sctrl; > int i; > > + n->conf_ioqpairs = n->params.max_ioqpairs; > + n->conf_msix_qsize = n->params.msix_qsize; > + > /* add one to max_ioqpairs to account for the admin queue pair */ > n->reg_size = pow2ceil(sizeof(NvmeBar) + > 2 * (n->params.max_ioqpairs + 1) * NVME_DB_SIZE); > @@ -6668,6 +6686,8 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice > *pci_dev, Error **errp) > } > } > > + nvme_update_msixcap_ts(pci_dev, n->conf_msix_qsize); > + > if (n->params.cmb_size_mb) { > nvme_init_cmb(n, pci_dev); > } > diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h > index 5ba07b62dff..314a2894759 100644 > --- a/hw/nvme/nvme.h > +++ b/hw/nvme/nvme.h > @@ -434,6 +434,8 @@ typedef struct NvmeCtrl { > uint64_t starttime_ms; > uint16_t temperature; > uint8_t smart_critical_warning; > + uint32_t conf_msix_qsize; > + uint32_t conf_ioqpairs; > > struct { > MemoryRegion mem; > -- > 2.25.1 > -- One of us - No more doubt, silence or taboo about mental illness.
signature.asc
Description: PGP signature