[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 10/15] hw/nvme: Remove reg_size variable and update BAR0 s
From: |
Klaus Jensen |
Subject: |
Re: [PATCH v4 10/15] hw/nvme: Remove reg_size variable and update BAR0 size calculation |
Date: |
Fri, 11 Feb 2022 08:55:20 +0100 |
On Jan 26 18:11, Lukasz Maniak wrote:
> From: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
>
> The n->reg_size parameter unnecessarily splits the BAR0 size calculation
> in two phases; removed to simplify the code.
>
> With all the calculations done in one place, it seems the pow2ceil,
> applied originally to reg_size, is unnecessary. The rounding should
> happen as the last step, when BAR size includes Nvme registers, queue
> registers, and MSIX-related space.
>
> Finally, the size of the mmio memory region is extended to cover the 1st
> 4KiB padding (see the map below). Access to this range is handled as
> interaction with a non-existing queue and generates an error trace, so
> actually nothing changes, while the reg_size variable is no longer needed.
>
> --------------------
> | BAR0 |
> --------------------
> [Nvme Registers ]
> [Queues ]
> [power-of-2 padding] - removed in this patch
> [4KiB padding (1) ]
> [MSIX TABLE ]
> [4KiB padding (2) ]
> [MSIX PBA ]
> [power-of-2 padding]
>
> Signed-off-by: Łukasz Gieryk <lukasz.gieryk@linux.intel.com>
> ---
> hw/nvme/ctrl.c | 10 +++++-----
> hw/nvme/nvme.h | 1 -
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 426507ca8a..40eb6bd1a8 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -6372,9 +6372,6 @@ static void nvme_init_state(NvmeCtrl *n)
> 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);
> n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
> n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
> n->temperature = NVME_TEMPERATURE;
> @@ -6498,7 +6495,10 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice
> *pci_dev, Error **errp)
> pcie_ari_init(pci_dev, 0x100, 1);
> }
>
> - bar_size = QEMU_ALIGN_UP(n->reg_size, 4 * KiB);
> + /* add one to max_ioqpairs to account for the admin queue pair */
> + bar_size = sizeof(NvmeBar) +
> + 2 * (n->params.max_ioqpairs + 1) * NVME_DB_SIZE;
> + bar_size = QEMU_ALIGN_UP(bar_size, 4 * KiB);
> msix_table_offset = bar_size;
> msix_table_size = PCI_MSIX_ENTRY_SIZE * n->params.msix_qsize;
>
> @@ -6512,7 +6512,7 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice
> *pci_dev, Error **errp)
>
> memory_region_init(&n->bar0, OBJECT(n), "nvme-bar0", bar_size);
> memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n, "nvme",
> - n->reg_size);
> + msix_table_offset);
> memory_region_add_subregion(&n->bar0, 0, &n->iomem);
>
> if (pci_is_vf(pci_dev)) {
> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> index 927890b490..1401ac3904 100644
> --- a/hw/nvme/nvme.h
> +++ b/hw/nvme/nvme.h
> @@ -414,7 +414,6 @@ typedef struct NvmeCtrl {
> uint16_t max_prp_ents;
> uint16_t cqe_size;
> uint16_t sqe_size;
> - uint32_t reg_size;
> uint32_t max_q_ents;
> uint8_t outstanding_aers;
> uint32_t irq_status;
> --
> 2.25.1
>
Nice catch.
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
signature.asc
Description: PGP signature
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v4 10/15] hw/nvme: Remove reg_size variable and update BAR0 size calculation,
Klaus Jensen <=