Intel NVMe reset issue

Intel NVMe reset issue

[Panah, Khosrow]

All,

While testing Intel NVMe driver on Linux, I noticed there are instances controller fails to initialize IDT controller. The problem is related to Intel Linux driver does not wait for "Enable" bit to clear when attempting to setup admin queue.  I am suggesting the following changes to "nvme_configure_admin_queue" routine:

***************
*** 1039,1044 ****
--- 1058,1078 ----
        dev->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;

        writel(0, &dev->bar->cc);
+       timeout = (5 * HZ) + jiffies;
+       while (!result && (readl(&dev->bar->csts) & NVME_CSTS_RDY)) {
+               msleep(100);
+               if (fatal_signal_pending(current))
+                       result = -EINTR;
+               if (time_after(jiffies, timeout)) {
+                       dev_err(&dev->pci_dev->dev,
+                               "Device busy; aborting initialisation\n");
+                       result = -ENODEV;
+               }
+       }
+       if (result) {
+               nvme_free_queue_mem(nvmeq);
+               return result;
+       }
        writel(aqa, &dev->bar->aqa);
        writeq(nvmeq->sq_dma_addr, &dev->bar->asq);
        writeq(nvmeq->cq_dma_addr, &dev->bar->acq);

Khosrow

Intel NVMe reset issue

[Busch, Keith]

I think this looks good. Just curious, where does the 5 second timeout come from? That sounds reasonable to me, but I don't see that ECN033 specifies what the timeout should be for CSTS.RDY to transition from 1 to 0 after setting CC.EN to 0.

Reviewed-by: Keith Busch <keith.busch at intel.com>

> -----Original Message-----
> From: linux-nvme-bounces at lists.infradead.org [mailto:linux-nvme-
> bounces at lists.infradead.org] On Behalf Of Panah, Khosrow
> Sent: Thursday, November 01, 2012 12:37 PM
> To: linux-nvme at lists.infradead.org
> Cc: Kong, Kwok
> Subject: Intel NVMe reset issue
> 
> All,
> 
> While testing Intel NVMe driver on Linux, I noticed there are instances
> controller fails to initialize IDT controller. The problem is related
> to Intel Linux driver does not wait for "Enable" bit to clear when
> attempting to setup admin queue.  I am suggesting the following changes
> to "nvme_configure_admin_queue" routine:
> 
> ***************
> *** 1039,1044 ****
> --- 1058,1078 ----
>         dev->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
> 
>         writel(0, &dev->bar->cc);
> +       timeout = (5 * HZ) + jiffies;
> +       while (!result && (readl(&dev->bar->csts) & NVME_CSTS_RDY)) {
> +               msleep(100);
> +               if (fatal_signal_pending(current))
> +                       result = -EINTR;
> +               if (time_after(jiffies, timeout)) {
> +                       dev_err(&dev->pci_dev->dev,
> +                               "Device busy; aborting
> initialisation\n");
> +                       result = -ENODEV;
> +               }
> +       }
> +       if (result) {
> +               nvme_free_queue_mem(nvmeq);
> +               return result;
> +       }
>         writel(aqa, &dev->bar->aqa);
>         writeq(nvmeq->sq_dma_addr, &dev->bar->asq);
>         writeq(nvmeq->cq_dma_addr, &dev->bar->acq);
> 
> Khosrow
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme

Intel NVMe reset issue

[Panah, Khosrow]

Busch,

I used the 5 seconds as the worst case scenario to allow controller transition of CC.EN from 1 to 0. I concur that I have seen a timeout value specified in specification neither.

Khosrow


-----Original Message-----
From: Busch, Keith [mailto:keith.busch@intel.com] 
Sent: Thursday, November 01, 2012 2:13 PM
To: Panah, Khosrow; linux-nvme at lists.infradead.org
Cc: Kong, Kwok
Subject: RE: Intel NVMe reset issue

I think this looks good. Just curious, where does the 5 second timeout come from? That sounds reasonable to me, but I don't see that ECN033 specifies what the timeout should be for CSTS.RDY to transition from 1 to 0 after setting CC.EN to 0.

Reviewed-by: Keith Busch <keith.busch at intel.com>

> -----Original Message-----
> From: linux-nvme-bounces at lists.infradead.org [mailto:linux-nvme- 
> bounces at lists.infradead.org] On Behalf Of Panah, Khosrow
> Sent: Thursday, November 01, 2012 12:37 PM
> To: linux-nvme at lists.infradead.org
> Cc: Kong, Kwok
> Subject: Intel NVMe reset issue
> 
> All,
> 
> While testing Intel NVMe driver on Linux, I noticed there are 
> instances controller fails to initialize IDT controller. The problem 
> is related to Intel Linux driver does not wait for "Enable" bit to 
> clear when attempting to setup admin queue.  I am suggesting the 
> following changes to "nvme_configure_admin_queue" routine:
> 
> ***************
> *** 1039,1044 ****
> --- 1058,1078 ----
>         dev->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
> 
>         writel(0, &dev->bar->cc);
> +       timeout = (5 * HZ) + jiffies;
> +       while (!result && (readl(&dev->bar->csts) & NVME_CSTS_RDY)) {
> +               msleep(100);
> +               if (fatal_signal_pending(current))
> +                       result = -EINTR;
> +               if (time_after(jiffies, timeout)) {
> +                       dev_err(&dev->pci_dev->dev,
> +                               "Device busy; aborting
> initialisation\n");
> +                       result = -ENODEV;
> +               }
> +       }
> +       if (result) {
> +               nvme_free_queue_mem(nvmeq);
> +               return result;
> +       }
>         writel(aqa, &dev->bar->aqa);
>         writeq(nvmeq->sq_dma_addr, &dev->bar->asq);
>         writeq(nvmeq->cq_dma_addr, &dev->bar->acq);
> 
> Khosrow
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme

Intel NVMe reset issue

[Matthew Wilcox]

On Thu, Nov 01, 2012@09:45:33PM +0000, Panah, Khosrow wrote:
> Busch,
> 
> I used the 5 seconds as the worst case scenario to allow controller transition of CC.EN from 1 to 0. I concur that I have seen a timeout value specified in specification neither.

It's probably not in the latest ECN, but in the meeting last week, we
decided that the NVME_CAP_TIMEOUT applied to both the wait for RDY and
the wait for EN.  ie controller manufacturers should specify it to be
the maximum of the two waits.

By the way, this is not the 'Intel NVMe driver'.  This is the Linux
NVMe driver.  I only happen to work at Intel ... if I were to leave, I
would remain the NVMe maintainer.  NB: I have no plans to leave Intel :-)

> Khosrow
> 
> 
> -----Original Message-----
> From: Busch, Keith [mailto:keith.busch at intel.com] 
> Sent: Thursday, November 01, 2012 2:13 PM
> To: Panah, Khosrow; linux-nvme at lists.infradead.org
> Cc: Kong, Kwok
> Subject: RE: Intel NVMe reset issue
> 
> I think this looks good. Just curious, where does the 5 second timeout come from? That sounds reasonable to me, but I don't see that ECN033 specifies what the timeout should be for CSTS.RDY to transition from 1 to 0 after setting CC.EN to 0.
> 
> Reviewed-by: Keith Busch <keith.busch at intel.com>
> 
> > -----Original Message-----
> > From: linux-nvme-bounces at lists.infradead.org [mailto:linux-nvme- 
> > bounces at lists.infradead.org] On Behalf Of Panah, Khosrow
> > Sent: Thursday, November 01, 2012 12:37 PM
> > To: linux-nvme at lists.infradead.org
> > Cc: Kong, Kwok
> > Subject: Intel NVMe reset issue
> > 
> > All,
> > 
> > While testing Intel NVMe driver on Linux, I noticed there are 
> > instances controller fails to initialize IDT controller. The problem 
> > is related to Intel Linux driver does not wait for "Enable" bit to 
> > clear when attempting to setup admin queue.  I am suggesting the 
> > following changes to "nvme_configure_admin_queue" routine:
> > 
> > ***************
> > *** 1039,1044 ****
> > --- 1058,1078 ----
> >         dev->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;
> > 
> >         writel(0, &dev->bar->cc);
> > +       timeout = (5 * HZ) + jiffies;
> > +       while (!result && (readl(&dev->bar->csts) & NVME_CSTS_RDY)) {
> > +               msleep(100);
> > +               if (fatal_signal_pending(current))
> > +                       result = -EINTR;
> > +               if (time_after(jiffies, timeout)) {
> > +                       dev_err(&dev->pci_dev->dev,
> > +                               "Device busy; aborting
> > initialisation\n");
> > +                       result = -ENODEV;
> > +               }
> > +       }
> > +       if (result) {
> > +               nvme_free_queue_mem(nvmeq);
> > +               return result;
> > +       }
> >         writel(aqa, &dev->bar->aqa);
> >         writeq(nvmeq->sq_dma_addr, &dev->bar->asq);
> >         writeq(nvmeq->cq_dma_addr, &dev->bar->acq);
> > 
> > Khosrow
> > 
> > 
> > _______________________________________________
> > Linux-nvme mailing list
> > Linux-nvme at lists.infradead.org
> > http://merlin.infradead.org/mailman/listinfo/linux-nvme
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme

Intel NVMe reset issue

[Kong, Kwok]

It is a typo in the description to "wait for "Enable" bit to clear". It should have been "wait for CSTS.RDY bit to clear".  His code is to wait for the CSTS.RDY bit.

i.e.

+       while (!result && (readl(&dev->bar->csts) & NVME_CSTS_RDY)) {


He should also use NVME_CAP_TIMEOUT as the timeout and not 5 seconds per Matthew's last message.


-Kwok




From: Gurpreet Anand [mailto:Gurpreet.Anand@sandisk.com] 
Sent: Thursday, November 01, 2012 7:12 PM
To: Panah, Khosrow
Cc: linux-nvme at lists.infradead.org; Kong, Kwok
Subject: Re: Intel NVMe reset issue

Our interpretation of the spec suggests that driver should not poll on CC.EN but on CSTS.RDY after setting CC.EN which we see being done correctly. Can you explain why the controller needs to clear CC.EN? From the controller perspective it is a read-only bit.

Keith's response does make sense where the driver still writes to CC.EN.

Panah, what is the issue you are trying to solve here?

Thanks,
Gurpreet?


On Nov 1, 2012,@11:38 AM, "Panah, Khosrow" <Khosrow.Panah@idt.com> wrote:
All,

While testing Intel NVMe driver on Linux, I noticed there are instances controller fails to initialize IDT controller. The problem is related to Intel Linux driver does not wait for "Enable" bit to clear when attempting to setup admin queue. ?I am suggesting the following changes to "nvme_configure_admin_queue" routine:

***************
*** 1039,1044 ****
--- 1058,1078 ----
???????dev->ctrl_config |= NVME_CC_IOSQES | NVME_CC_IOCQES;

???????writel(0, &dev->bar->cc);
+ ??????timeout = (5 * HZ) + jiffies;
+ ??????while (!result && (readl(&dev->bar->csts) & NVME_CSTS_RDY)) {
+ ??????????????msleep(100);
+ ??????????????if (fatal_signal_pending(current))
+ ??????????????????????result = -EINTR;
+ ??????????????if (time_after(jiffies, timeout)) {
+ ??????????????????????dev_err(&dev->pci_dev->dev,
+ ??????????????????????????????"Device busy; aborting initialisation\n");
+ ??????????????????????result = -ENODEV;
+ ??????????????}
+ ??????}
+ ??????if (result) {
+ ??????????????nvme_free_queue_mem(nvmeq);
+ ??????????????return result;
+ ??????}
???????writel(aqa, &dev->bar->aqa);
???????writeq(nvmeq->sq_dma_addr, &dev->bar->asq);
???????writeq(nvmeq->cq_dma_addr, &dev->bar->acq);

Khosrow


_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://merlin.infradead.org/mailman/listinfo/linux-nvme

________________________________________

PLEASE NOTE: The information contained in this electronic mail message is intended only for the use of the designated recipient(s) named above. If the reader of this message is not the intended recipient, you are hereby notified that you have received this message in error and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender by telephone or e-mail (as shown above) immediately and destroy any and all copies of this message in your possession (whether hard copies or electronically stored copies).