On Tue, Jun 07, 2022 at 01:23:20PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jen...@samsung.com>
>
> The SRIOV series exposed an issued with how CC register writes are
> handled and how CSTS is set in response to that. Specifically, after
> applying the SRIOV series, the controller could end up in a state with
> CC.EN set to '1' but with CSTS.RDY cleared to '0', causing drivers to
> expect CSTS.RDY to transition to '1' but timing out.
>
> Clean this up.
>
> Signed-off-by: Klaus Jensen <k.jen...@samsung.com>
> ---
> v3:
> * clear intms/intmc/cc regardless of reset type
>
> hw/nvme/ctrl.c | 38 ++++++++++++++++----------------------
> 1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 658584d417fe..a558f5cb29c1 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -6190,10 +6190,15 @@ static void nvme_ctrl_reset(NvmeCtrl *n,
> NvmeResetType rst)
>
> if (pci_is_vf(pci_dev)) {
> sctrl = nvme_sctrl(n);
> +
> stl_le_p(&n->bar.csts, sctrl->scs ? 0 : NVME_CSTS_FAILED);
> } else {
> stl_le_p(&n->bar.csts, 0);
> }
> +
> + stl_le_p(&n->bar.intms, 0);
> + stl_le_p(&n->bar.intmc, 0);
> + stl_le_p(&n->bar.cc, 0);
> }
>
> static void nvme_ctrl_shutdown(NvmeCtrl *n)
> @@ -6405,20 +6410,21 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr
> offset, uint64_t data,
> nvme_irq_check(n);
> break;
> case NVME_REG_CC:
> + stl_le_p(&n->bar.cc, data);
> +
> trace_pci_nvme_mmio_cfg(data & 0xffffffff);
>
> - /* Windows first sends data, then sends enable bit */
> - if (!NVME_CC_EN(data) && !NVME_CC_EN(cc) &&
> - !NVME_CC_SHN(data) && !NVME_CC_SHN(cc))
> - {
> - cc = data;
> + if (NVME_CC_SHN(data) && !(NVME_CC_SHN(cc))) {
> + trace_pci_nvme_mmio_shutdown_set();
> + nvme_ctrl_shutdown(n);
> + csts &= ~(CSTS_SHST_MASK << CSTS_SHST_SHIFT);
> + csts |= NVME_CSTS_SHST_COMPLETE;
> + } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(cc)) {
> + trace_pci_nvme_mmio_shutdown_cleared();
> + csts &= ~(CSTS_SHST_MASK << CSTS_SHST_SHIFT);
> }
>
> if (NVME_CC_EN(data) && !NVME_CC_EN(cc)) {
> - cc = data;
> -
> - /* flush CC since nvme_start_ctrl() needs the value */
> - stl_le_p(&n->bar.cc, cc);
> if (unlikely(nvme_start_ctrl(n))) {
> trace_pci_nvme_err_startfail();
> csts = NVME_CSTS_FAILED;
> @@ -6429,22 +6435,10 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr
> offset, uint64_t data,
> } else if (!NVME_CC_EN(data) && NVME_CC_EN(cc)) {
> trace_pci_nvme_mmio_stopped();
> nvme_ctrl_reset(n, NVME_RESET_CONTROLLER);
> - cc = 0;
> - csts &= ~NVME_CSTS_READY;
> - }
>
> - if (NVME_CC_SHN(data) && !(NVME_CC_SHN(cc))) {
> - trace_pci_nvme_mmio_shutdown_set();
> - nvme_ctrl_shutdown(n);
> - cc = data;
> - csts |= NVME_CSTS_SHST_COMPLETE;
> - } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(cc)) {
> - trace_pci_nvme_mmio_shutdown_cleared();
> - csts &= ~NVME_CSTS_SHST_COMPLETE;
> - cc = data;
> + break;
> }
>
> - stl_le_p(&n->bar.cc, cc);
> stl_le_p(&n->bar.csts, csts);
>
> break;
> --
> 2.36.1
>
Reviewed-by: Lukasz Maniak <lukasz.man...@linux.intel.com>
