[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 05/14] hw/block/nvme: Add support for Namespace Types
From: |
Keith Busch |
Subject: |
Re: [PATCH v5 05/14] hw/block/nvme: Add support for Namespace Types |
Date: |
Thu, 1 Oct 2020 09:29:22 -0600 |
On Thu, Oct 01, 2020 at 11:22:46AM +0000, Niklas Cassel wrote:
> On Mon, Sep 28, 2020 at 11:35:19AM +0900, Dmitry Fomichev wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> > @@ -2222,6 +2328,30 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr
> > offset, uint64_t data,
> > break;
> > case 0x14: /* CC */
> > trace_pci_nvme_mmio_cfg(data & 0xffffffff);
> > +
> > + if (NVME_CC_CSS(data) != NVME_CC_CSS(n->bar.cc)) {
> > + if (NVME_CC_EN(n->bar.cc)) {
>
> I just saw this print when doing controller reset on a live system.
>
> Added a debug print:
> nvme_write_bar WRITING: 0x0 previous: 0x464061
>
> so the second if-statement has to be:
>
> if (NVME_CC_EN(n->bar.cc) && NVME_CC_EN(data)) {
>
> Sorry for introducing the bug in the first place.
No worries.
I don't think the check should be here at all, really. The only check for valid
CSS should be in nvme_start_ctrl(), which I posted yesterday.
> > + NVME_GUEST_ERR(pci_nvme_err_change_css_when_enabled,
> > + "changing selected command set when
> > enabled");
> > + } else {
> > + switch (NVME_CC_CSS(data)) {
> > + case CSS_NVM_ONLY:
> > + trace_pci_nvme_css_nvm_cset_selected_by_host(data &
> > +
> > 0xffffffff);
> > + break;
> > + case CSS_CSI:
> > + NVME_SET_CC_CSS(n->bar.cc, CSS_CSI);
> > + trace_pci_nvme_css_all_csets_sel_by_host(data &
> > 0xffffffff);
> > + break;
> > + case CSS_ADMIN_ONLY:
> > + break;
> > + default:
> > + NVME_GUEST_ERR(pci_nvme_ub_unknown_css_value,
> > + "unknown value in CC.CSS field");
> > + }
> > + }
> > + }