[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writ
From: |
Minwoo Im |
Subject: |
Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes |
Date: |
Mon, 18 Jan 2021 21:41:00 +0900 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
On 21-01-18 10:46:55, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> 64 bit registers like ASQ and ACQ should be writable by both a hi/lo 32
> bit write combination as well as a plain 64 bit write. The spec does not
> define ordering on the hi/lo split, but the code currently assumes that
> the low order bits are written first. Additionally, the code does not
> consider that another address might already have been written into the
> register, causing the OR'ing to result in a bad address.
>
> Fix this by explicitly overwriting only the low or high order bits for
> 32 bit writes.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/block/nvme.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index bd7e258c3c26..40b9f8ccfc0e 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -3781,19 +3781,21 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr
> offset, uint64_t data,
> trace_pci_nvme_mmio_aqattr(data & 0xffffffff);
> break;
> case 0x28: /* ASQ */
> - n->bar.asq = data;
> + n->bar.asq = size == 8 ? data :
> + (n->bar.asq & ~0xffffffff) | (data & 0xffffffff);
^^^^^^^^^^^
If this one is to unmask lower 32bits other than higher 32bits, then
it should be:
(n->bar.asq & ~0xffffffffULL)
Also, maybe we should take care of lower than 4bytes as:
.min_access_size = 2,
.max_access_size = 8,
Even we have some error messages up there with:
if (unlikely(size < sizeof(uint32_t))) {
NVME_GUEST_ERR(pci_nvme_ub_mmiowr_toosmall,
"MMIO write smaller than 32-bits,"
" offset=0x%"PRIx64", size=%u",
offset, size);
/* should be ignored, fall through for now */
}
It's a fall-thru error, and that's it. I would prefer not to have this
error and support for 2bytes access also, OR do not support for 2bytes
access for this MMIO area.
Thanks!
- [PATCH v2 00/12] hw/block/nvme: misc cmb/pmr patches and bump to v1.4, Klaus Jensen, 2021/01/18
- [PATCH v2 01/12] hw/block/nvme: add size to mmio read/write trace events, Klaus Jensen, 2021/01/18
- [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes, Klaus Jensen, 2021/01/18
- Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes,
Minwoo Im <=
- Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes, Klaus Jensen, 2021/01/18
- Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes, Minwoo Im, 2021/01/18
- Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes, Klaus Jensen, 2021/01/18
- Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes, Minwoo Im, 2021/01/18
- Re: [PATCH v2 02/12] hw/block/nvme: fix 64 bit register hi/lo split writes, Keith Busch, 2021/01/19
[PATCH v2 03/12] hw/block/nvme: indicate CMB support through controller capabilities register, Klaus Jensen, 2021/01/18
[PATCH v2 04/12] hw/block/nvme: move msix table and pba to BAR 0, Klaus Jensen, 2021/01/18
[PATCH v2 05/12] hw/block/nvme: allow cmb and pmr to coexist, Klaus Jensen, 2021/01/18