On 7/13/21 9:24 PM, Klaus Jensen wrote:
> From: Klaus Jensen <[email protected]>
>
> The new PMR test unearthed a long-standing issue with MMIO reads on
> big-endian hosts.
>
> Fix this by unconditionally storing all controller registers in little
> endian.
>
> Cc: Gollu Appalanaidu <[email protected]>
> Reported-by: Peter Maydell <[email protected]>
> Signed-off-by: Klaus Jensen <[email protected]>
> ---
> hw/nvme/ctrl.c | 304 ++++++++++++++++++++++++++++---------------------
> 1 file changed, 174 insertions(+), 130 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 0449cc4dee9b..ddac9344a74e 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -439,10 +439,12 @@ static uint8_t nvme_sq_empty(NvmeSQueue *sq)
>
> static void nvme_irq_check(NvmeCtrl *n)
> {
> + uint32_t intms = le32_to_cpu(n->bar.intms);
> +
> if (msix_enabled(&(n->parent_obj))) {
> return;
> }
> - if (~n->bar.intms & n->irq_status) {
> + if (~intms & n->irq_status) {
> pci_irq_assert(&n->parent_obj);
> } else {
> pci_irq_deassert(&n->parent_obj);
> @@ -1289,7 +1291,7 @@ static void nvme_post_cqes(void *opaque)
> if (ret) {
> trace_pci_nvme_err_addr_write(addr);
> trace_pci_nvme_err_cfs();
> - n->bar.csts = NVME_CSTS_FAILED;
> + n->bar.csts = cpu_to_le64(NVME_CSTS_FAILED);
The load/store API is safer than the cpu_to_X() one because
it removes alignment problems. Here it becomes:
stq_le_p(&n->bar.csts, NVME_CSTS_FAILED);
> break;
> }
> QTAILQ_REMOVE(&cq->req_list, req, entry);
> @@ -4022,7 +4024,7 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeRequest
> *req)
> trace_pci_nvme_err_invalid_create_sq_sqid(sqid);
> return NVME_INVALID_QID | NVME_DNR;
> }
> - if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
> + if (unlikely(!qsize || qsize > NVME_CAP_MQES(le64_to_cpu(n->bar.cap)))) {
And here:
if (unlikely(!qsize || qsize > NVME_CAP_MQES(ldq_le_p(&n->bar.cap)))) {
> trace_pci_nvme_err_invalid_create_sq_size(qsize);
> return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
> }
However for the BAR is likely aligned, so using the cpu_to() API
shouldn't be a problem until we try to deprecate/remove it.
