NVMe driver init sequence

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

NVMe driver init sequence

Kinjal Patel
Hi,

I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).

The driver init sequence is,

1)      Enable controller (CC.EN=1)

2)      Wait for controller ready (CSTS.RDY=1)

3)      Set PCI bus master enable (BME=1)

As per NVMe spec, when NVMe controller becomes ready it has to process command.

"Enable (EN): When set to '1', then the controller shall process commands based on Submission Queue Tail doorbell writes"

And per PCI Express spec when BME is not set, the PCI Express Function is not allowed to issue any Memory or I/O requests.
"Bus Master Enable - Controls the ability of a PCI Express Endpoint to issue Memory95 and I/O Read/Write Requests, and
the ability of a Root or Switch Port to forward Memory and I/O Read/Write Requests in the Upstream direction"

Enabling controller before setting BME=1 is violation of spec, as controller has to accept commands but BME is prerequisite for that.

The Linux device driver init sequence is,

1)     Set PCI bus master enable (BME=1)

2)     Enable Controller (CC.EN=1)

3)     Wait for controller ready (CSTS.RDY=1)

The FreeBSD NVMe driver sequence should be changed to set BME=1 before attempting to enable controller.


Regards,
Kinjal Patel
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: NVMe driver init sequence

Warner Losh
This sounds right to me....  Most BIOSes seem to enable BME=1.

Warner

On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <[hidden email]
> wrote:

> Hi,
>
> I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).
>
> The driver init sequence is,
>
> 1)      Enable controller (CC.EN=1)
>
> 2)      Wait for controller ready (CSTS.RDY=1)
>
> 3)      Set PCI bus master enable (BME=1)
>
> As per NVMe spec, when NVMe controller becomes ready it has to process
> command.
>
> "Enable (EN): When set to '1', then the controller shall process commands
> based on Submission Queue Tail doorbell writes"
>
> And per PCI Express spec when BME is not set, the PCI Express Function is
> not allowed to issue any Memory or I/O requests.
> "Bus Master Enable - Controls the ability of a PCI Express Endpoint to
> issue Memory95 and I/O Read/Write Requests, and
> the ability of a Root or Switch Port to forward Memory and I/O Read/Write
> Requests in the Upstream direction"
>
> Enabling controller before setting BME=1 is violation of spec, as
> controller has to accept commands but BME is prerequisite for that.
>
> The Linux device driver init sequence is,
>
> 1)     Set PCI bus master enable (BME=1)
>
> 2)     Enable Controller (CC.EN=1)
>
> 3)     Wait for controller ready (CSTS.RDY=1)
>
> The FreeBSD NVMe driver sequence should be changed to set BME=1 before
> attempting to enable controller.
>
>
> Regards,
> Kinjal Patel
> _______________________________________________
> [hidden email] mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> To unsubscribe, send any mail to "[hidden email]"
>
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: NVMe driver init sequence

Kinjal Patel
Here is a the change I propose. This will make the init sequence right.

Index: sys/dev/nvme/nvme.c
===================================================================
--- sys/dev/nvme/nvme.c (revision 322672)
+++ sys/dev/nvme/nvme.c (working copy)
@@ -253,6 +253,9 @@
                return (status);
        }

+    /* make device bus-master before attempting to enable controller */
+    pci_enable_busmaster(dev);
+
        /*
         * Reset controller twice to ensure we do a transition from cc.en==1
         *  to cc.en==0.  This is because we don't really know what status
@@ -270,8 +273,6 @@
                return (status);
        }

-       pci_enable_busmaster(dev);
-
        ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
        ctrlr->config_hook.ich_arg = ctrlr;



regards,
Kinjal Patel

From: [hidden email] [mailto:[hidden email]] On Behalf Of Warner Losh
Sent: Thursday, August 17, 2017 3:10 PM
To: Kinjal Patel
Cc: [hidden email]
Subject: Re: NVMe driver init sequence

This sounds right to me....  Most BIOSes seem to enable BME=1.

Warner

On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <[hidden email]<mailto:[hidden email]>> wrote:
Hi,

I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).

The driver init sequence is,

1)      Enable controller (CC.EN=1)

2)      Wait for controller ready (CSTS.RDY=1)

3)      Set PCI bus master enable (BME=1)

As per NVMe spec, when NVMe controller becomes ready it has to process command.

"Enable (EN): When set to '1', then the controller shall process commands based on Submission Queue Tail doorbell writes"

And per PCI Express spec when BME is not set, the PCI Express Function is not allowed to issue any Memory or I/O requests.
"Bus Master Enable - Controls the ability of a PCI Express Endpoint to issue Memory95 and I/O Read/Write Requests, and
the ability of a Root or Switch Port to forward Memory and I/O Read/Write Requests in the Upstream direction"

Enabling controller before setting BME=1 is violation of spec, as controller has to accept commands but BME is prerequisite for that.

The Linux device driver init sequence is,

1)     Set PCI bus master enable (BME=1)

2)     Enable Controller (CC.EN=1)

3)     Wait for controller ready (CSTS.RDY=1)

The FreeBSD NVMe driver sequence should be changed to set BME=1 before attempting to enable controller.


Regards,
Kinjal Patel
_______________________________________________
[hidden email]<mailto:[hidden email]> mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]<mailto:[hidden email]>"

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: NVMe driver init sequence

Kinjal Patel
In reply to this post by Warner Losh
Created a bug #22166 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221616)
Updated the description and patch to it.

Regards,
Kinjal Patel

From: Kinjal Patel
Sent: Friday, August 18, 2017 1:51 PM
To: 'Warner Losh'
Cc: [hidden email]
Subject: RE: NVMe driver init sequence

Here is a the change I propose. This will make the init sequence right.

Index: sys/dev/nvme/nvme.c
===================================================================
--- sys/dev/nvme/nvme.c (revision 322672)
+++ sys/dev/nvme/nvme.c (working copy)
@@ -253,6 +253,9 @@
                return (status);
        }

+    /* make device bus-master before attempting to enable controller */
+    pci_enable_busmaster(dev);
+
        /*
         * Reset controller twice to ensure we do a transition from cc.en==1
         *  to cc.en==0.  This is because we don't really know what status
@@ -270,8 +273,6 @@
                return (status);
        }

-       pci_enable_busmaster(dev);
-
        ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
        ctrlr->config_hook.ich_arg = ctrlr;



regards,
Kinjal Patel

From: [hidden email]<mailto:[hidden email]> [mailto:[hidden email]] On Behalf Of Warner Losh
Sent: Thursday, August 17, 2017 3:10 PM
To: Kinjal Patel
Cc: [hidden email]<mailto:[hidden email]>
Subject: Re: NVMe driver init sequence

This sounds right to me....  Most BIOSes seem to enable BME=1.

Warner

On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <[hidden email]<mailto:[hidden email]>> wrote:
Hi,

I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).

The driver init sequence is,

1)      Enable controller (CC.EN=1)

2)      Wait for controller ready (CSTS.RDY=1)

3)      Set PCI bus master enable (BME=1)

As per NVMe spec, when NVMe controller becomes ready it has to process command.

"Enable (EN): When set to '1', then the controller shall process commands based on Submission Queue Tail doorbell writes"

And per PCI Express spec when BME is not set, the PCI Express Function is not allowed to issue any Memory or I/O requests.
"Bus Master Enable - Controls the ability of a PCI Express Endpoint to issue Memory95 and I/O Read/Write Requests, and
the ability of a Root or Switch Port to forward Memory and I/O Read/Write Requests in the Upstream direction"

Enabling controller before setting BME=1 is violation of spec, as controller has to accept commands but BME is prerequisite for that.

The Linux device driver init sequence is,

1)     Set PCI bus master enable (BME=1)

2)     Enable Controller (CC.EN=1)

3)     Wait for controller ready (CSTS.RDY=1)

The FreeBSD NVMe driver sequence should be changed to set BME=1 before attempting to enable controller.


Regards,
Kinjal Patel
_______________________________________________
[hidden email]<mailto:[hidden email]> mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]<mailto:[hidden email]>"

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: NVMe driver init sequence

Jim Harris-2
On Fri, Aug 18, 2017 at 2:04 PM, Kinjal Patel
<[hidden email]> wrote:

> Created a bug #22166 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221616)
> Updated the description and patch to it.
>
> Regards,
> Kinjal Patel
>
> From: Kinjal Patel
> Sent: Friday, August 18, 2017 1:51 PM
> To: 'Warner Losh'
> Cc: [hidden email]
> Subject: RE: NVMe driver init sequence
>
> Here is a the change I propose. This will make the init sequence right.

Looks good to me.

>
> Index: sys/dev/nvme/nvme.c
> ===================================================================
> --- sys/dev/nvme/nvme.c (revision 322672)
> +++ sys/dev/nvme/nvme.c (working copy)
> @@ -253,6 +253,9 @@
>                 return (status);
>         }
>
> +    /* make device bus-master before attempting to enable controller */
> +    pci_enable_busmaster(dev);
> +
>         /*
>          * Reset controller twice to ensure we do a transition from cc.en==1
>          *  to cc.en==0.  This is because we don't really know what status
> @@ -270,8 +273,6 @@
>                 return (status);
>         }
>
> -       pci_enable_busmaster(dev);
> -
>         ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
>         ctrlr->config_hook.ich_arg = ctrlr;
>
>
>
> regards,
> Kinjal Patel
>
> From: [hidden email]<mailto:[hidden email]> [mailto:[hidden email]] On Behalf Of Warner Losh
> Sent: Thursday, August 17, 2017 3:10 PM
> To: Kinjal Patel
> Cc: [hidden email]<mailto:[hidden email]>
> Subject: Re: NVMe driver init sequence
>
> This sounds right to me....  Most BIOSes seem to enable BME=1.
>
> Warner
>
> On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <[hidden email]<mailto:[hidden email]>> wrote:
> Hi,
>
> I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).
>
> The driver init sequence is,
>
> 1)      Enable controller (CC.EN=1)
>
> 2)      Wait for controller ready (CSTS.RDY=1)
>
> 3)      Set PCI bus master enable (BME=1)
>
> As per NVMe spec, when NVMe controller becomes ready it has to process command.
>
> "Enable (EN): When set to '1', then the controller shall process commands based on Submission Queue Tail doorbell writes"
>
> And per PCI Express spec when BME is not set, the PCI Express Function is not allowed to issue any Memory or I/O requests.
> "Bus Master Enable - Controls the ability of a PCI Express Endpoint to issue Memory95 and I/O Read/Write Requests, and
> the ability of a Root or Switch Port to forward Memory and I/O Read/Write Requests in the Upstream direction"
>
> Enabling controller before setting BME=1 is violation of spec, as controller has to accept commands but BME is prerequisite for that.
>
> The Linux device driver init sequence is,
>
> 1)     Set PCI bus master enable (BME=1)
>
> 2)     Enable Controller (CC.EN=1)
>
> 3)     Wait for controller ready (CSTS.RDY=1)
>
> The FreeBSD NVMe driver sequence should be changed to set BME=1 before attempting to enable controller.
>
>
> Regards,
> Kinjal Patel
> _______________________________________________
> [hidden email]<mailto:[hidden email]> mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> To unsubscribe, send any mail to "[hidden email]<mailto:[hidden email]>"
>
> _______________________________________________
> [hidden email] mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> To unsubscribe, send any mail to "[hidden email]"
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: NVMe driver init sequence

Warner Losh
Greetings,

I've just committed r322872 which implements this, with a slightly tweaked
comment.

Warner

On Fri, Aug 18, 2017 at 4:09 PM, Jim Harris <[hidden email]> wrote:

> On Fri, Aug 18, 2017 at 2:04 PM, Kinjal Patel
> <[hidden email]> wrote:
> > Created a bug #22166 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=
> 221616)
> > Updated the description and patch to it.
> >
> > Regards,
> > Kinjal Patel
> >
> > From: Kinjal Patel
> > Sent: Friday, August 18, 2017 1:51 PM
> > To: 'Warner Losh'
> > Cc: [hidden email]
> > Subject: RE: NVMe driver init sequence
> >
> > Here is a the change I propose. This will make the init sequence right.
>
> Looks good to me.
>
> >
> > Index: sys/dev/nvme/nvme.c
> > ===================================================================
> > --- sys/dev/nvme/nvme.c (revision 322672)
> > +++ sys/dev/nvme/nvme.c (working copy)
> > @@ -253,6 +253,9 @@
> >                 return (status);
> >         }
> >
> > +    /* make device bus-master before attempting to enable controller */
> > +    pci_enable_busmaster(dev);
> > +
> >         /*
> >          * Reset controller twice to ensure we do a transition from
> cc.en==1
> >          *  to cc.en==0.  This is because we don't really know what
> status
> > @@ -270,8 +273,6 @@
> >                 return (status);
> >         }
> >
> > -       pci_enable_busmaster(dev);
> > -
> >         ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
> >         ctrlr->config_hook.ich_arg = ctrlr;
> >
> >
> >
> > regards,
> > Kinjal Patel
> >
> > From: [hidden email]<mailto:[hidden email]> [mailto:[hidden email]]
> On Behalf Of Warner Losh
> > Sent: Thursday, August 17, 2017 3:10 PM
> > To: Kinjal Patel
> > Cc: [hidden email]<mailto:[hidden email]>
> > Subject: Re: NVMe driver init sequence
> >
> > This sounds right to me....  Most BIOSes seem to enable BME=1.
> >
> > Warner
> >
> > On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <
> [hidden email]<mailto:[hidden email]>>
> wrote:
> > Hi,
> >
> > I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).
> >
> > The driver init sequence is,
> >
> > 1)      Enable controller (CC.EN=1)
> >
> > 2)      Wait for controller ready (CSTS.RDY=1)
> >
> > 3)      Set PCI bus master enable (BME=1)
> >
> > As per NVMe spec, when NVMe controller becomes ready it has to process
> command.
> >
> > "Enable (EN): When set to '1', then the controller shall process
> commands based on Submission Queue Tail doorbell writes"
> >
> > And per PCI Express spec when BME is not set, the PCI Express Function
> is not allowed to issue any Memory or I/O requests.
> > "Bus Master Enable - Controls the ability of a PCI Express Endpoint to
> issue Memory95 and I/O Read/Write Requests, and
> > the ability of a Root or Switch Port to forward Memory and I/O
> Read/Write Requests in the Upstream direction"
> >
> > Enabling controller before setting BME=1 is violation of spec, as
> controller has to accept commands but BME is prerequisite for that.
> >
> > The Linux device driver init sequence is,
> >
> > 1)     Set PCI bus master enable (BME=1)
> >
> > 2)     Enable Controller (CC.EN=1)
> >
> > 3)     Wait for controller ready (CSTS.RDY=1)
> >
> > The FreeBSD NVMe driver sequence should be changed to set BME=1 before
> attempting to enable controller.
> >
> >
> > Regards,
> > Kinjal Patel
> > _______________________________________________
> > [hidden email]<mailto:[hidden email]> mailing
> list
> > https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> > To unsubscribe, send any mail to "freebsd-drivers-unsubscribe@
> freebsd.org<mailto:[hidden email]>"
> >
> > _______________________________________________
> > [hidden email] mailing list
> > https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> > To unsubscribe, send any mail to "freebsd-drivers-unsubscribe@
> freebsd.org"
>
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: NVMe driver init sequence

Kinjal Patel
Great!
Thanks Warner.


Regards,
Kinjal Patel

From: [hidden email] [mailto:[hidden email]] On Behalf Of Warner Losh
Sent: Thursday, August 24, 2017 8:17 PM
To: Jim Harris
Cc: Kinjal Patel; [hidden email]
Subject: Re: NVMe driver init sequence

Greetings,

I've just committed r322872 which implements this, with a slightly tweaked comment.

Warner

On Fri, Aug 18, 2017 at 4:09 PM, Jim Harris <[hidden email]<mailto:[hidden email]>> wrote:
On Fri, Aug 18, 2017 at 2:04 PM, Kinjal Patel
<[hidden email]<mailto:[hidden email]>> wrote:

> Created a bug #22166 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221616)
> Updated the description and patch to it.
>
> Regards,
> Kinjal Patel
>
> From: Kinjal Patel
> Sent: Friday, August 18, 2017 1:51 PM
> To: 'Warner Losh'
> Cc: [hidden email]<mailto:[hidden email]>
> Subject: RE: NVMe driver init sequence
>
> Here is a the change I propose. This will make the init sequence right.

Looks good to me.

>
> Index: sys/dev/nvme/nvme.c
> ===================================================================
> --- sys/dev/nvme/nvme.c (revision 322672)
> +++ sys/dev/nvme/nvme.c (working copy)
> @@ -253,6 +253,9 @@
>                 return (status);
>         }
>
> +    /* make device bus-master before attempting to enable controller */
> +    pci_enable_busmaster(dev);
> +
>         /*
>          * Reset controller twice to ensure we do a transition from cc.en==1
>          *  to cc.en==0.  This is because we don't really know what status
> @@ -270,8 +273,6 @@
>                 return (status);
>         }
>
> -       pci_enable_busmaster(dev);
> -
>         ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
>         ctrlr->config_hook.ich_arg = ctrlr;
>
>
>
> regards,
> Kinjal Patel
>
> From: [hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>> [mailto:[hidden email]<mailto:[hidden email]>] On Behalf Of Warner Losh
> Sent: Thursday, August 17, 2017 3:10 PM
> To: Kinjal Patel
> Cc: [hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>>
> Subject: Re: NVMe driver init sequence
>
> This sounds right to me....  Most BIOSes seem to enable BME=1.
>
> Warner
>
> On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <[hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>>> wrote:
> Hi,
>
> I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).
>
> The driver init sequence is,
>
> 1)      Enable controller (CC.EN=1)
>
> 2)      Wait for controller ready (CSTS.RDY=1)
>
> 3)      Set PCI bus master enable (BME=1)
>
> As per NVMe spec, when NVMe controller becomes ready it has to process command.
>
> "Enable (EN): When set to '1', then the controller shall process commands based on Submission Queue Tail doorbell writes"
>
> And per PCI Express spec when BME is not set, the PCI Express Function is not allowed to issue any Memory or I/O requests.
> "Bus Master Enable - Controls the ability of a PCI Express Endpoint to issue Memory95 and I/O Read/Write Requests, and
> the ability of a Root or Switch Port to forward Memory and I/O Read/Write Requests in the Upstream direction"
>
> Enabling controller before setting BME=1 is violation of spec, as controller has to accept commands but BME is prerequisite for that.
>
> The Linux device driver init sequence is,
>
> 1)     Set PCI bus master enable (BME=1)
>
> 2)     Enable Controller (CC.EN=1)
>
> 3)     Wait for controller ready (CSTS.RDY=1)
>
> The FreeBSD NVMe driver sequence should be changed to set BME=1 before attempting to enable controller.
>
>
> Regards,
> Kinjal Patel
> _______________________________________________
> [hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>> mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> To unsubscribe, send any mail to "[hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>>"
>
> _______________________________________________
> [hidden email]<mailto:[hidden email]> mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> To unsubscribe, send any mail to "[hidden email]<mailto:[hidden email]>"

_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: NVMe driver init sequence

Warner Losh
BTW, I just noticed you reported this against 10.3R. Is there some reason
(perhaps a customer with an issue) that you reported it there? I'm trying
to decide if I need to MFC this change to 11.x and 10.x (the latter has
some time urgency due to upcoming 10.4R release).

Warner

On Fri, Aug 25, 2017 at 9:54 AM, Kinjal Patel <[hidden email]
> wrote:

> Great!
>
> Thanks Warner.
>
>
>
>
>
> Regards,
>
> Kinjal Patel
>
>
>
> *From:* [hidden email] [mailto:[hidden email]] *On Behalf Of *Warner
> Losh
> *Sent:* Thursday, August 24, 2017 8:17 PM
> *To:* Jim Harris
> *Cc:* Kinjal Patel; [hidden email]
>
> *Subject:* Re: NVMe driver init sequence
>
>
>
> Greetings,
>
>
>
> I've just committed r322872 which implements this, with a slightly tweaked
> comment.
>
>
>
> Warner
>
>
>
> On Fri, Aug 18, 2017 at 4:09 PM, Jim Harris <[hidden email]> wrote:
>
> On Fri, Aug 18, 2017 at 2:04 PM, Kinjal Patel
> <[hidden email]> wrote:
> > Created a bug #22166 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=
> 221616)
> > Updated the description and patch to it.
> >
> > Regards,
> > Kinjal Patel
> >
> > From: Kinjal Patel
> > Sent: Friday, August 18, 2017 1:51 PM
> > To: 'Warner Losh'
> > Cc: [hidden email]
> > Subject: RE: NVMe driver init sequence
> >
> > Here is a the change I propose. This will make the init sequence right.
>
> Looks good to me.
>
> >
> > Index: sys/dev/nvme/nvme.c
> > ===================================================================
> > --- sys/dev/nvme/nvme.c (revision 322672)
> > +++ sys/dev/nvme/nvme.c (working copy)
> > @@ -253,6 +253,9 @@
> >                 return (status);
> >         }
> >
> > +    /* make device bus-master before attempting to enable controller */
> > +    pci_enable_busmaster(dev);
> > +
> >         /*
> >          * Reset controller twice to ensure we do a transition from
> cc.en==1
> >          *  to cc.en==0.  This is because we don't really know what
> status
> > @@ -270,8 +273,6 @@
> >                 return (status);
> >         }
> >
> > -       pci_enable_busmaster(dev);
> > -
> >         ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
> >         ctrlr->config_hook.ich_arg = ctrlr;
> >
> >
> >
> > regards,
> > Kinjal Patel
> >
> > From: [hidden email]<mailto:[hidden email]> [mailto:[hidden email]]
> On Behalf Of Warner Losh
> > Sent: Thursday, August 17, 2017 3:10 PM
> > To: Kinjal Patel
> > Cc: [hidden email]<mailto:[hidden email]>
> > Subject: Re: NVMe driver init sequence
> >
> > This sounds right to me....  Most BIOSes seem to enable BME=1.
> >
> > Warner
> >
> > On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <
> [hidden email]<mailto:[hidden email]>>
> wrote:
> > Hi,
> >
> > I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).
> >
> > The driver init sequence is,
> >
> > 1)      Enable controller (CC.EN=1)
> >
> > 2)      Wait for controller ready (CSTS.RDY=1)
> >
> > 3)      Set PCI bus master enable (BME=1)
> >
> > As per NVMe spec, when NVMe controller becomes ready it has to process
> command.
> >
> > "Enable (EN): When set to '1', then the controller shall process
> commands based on Submission Queue Tail doorbell writes"
> >
> > And per PCI Express spec when BME is not set, the PCI Express Function
> is not allowed to issue any Memory or I/O requests.
> > "Bus Master Enable - Controls the ability of a PCI Express Endpoint to
> issue Memory95 and I/O Read/Write Requests, and
> > the ability of a Root or Switch Port to forward Memory and I/O
> Read/Write Requests in the Upstream direction"
> >
> > Enabling controller before setting BME=1 is violation of spec, as
> controller has to accept commands but BME is prerequisite for that.
> >
> > The Linux device driver init sequence is,
> >
> > 1)     Set PCI bus master enable (BME=1)
> >
> > 2)     Enable Controller (CC.EN=1)
> >
> > 3)     Wait for controller ready (CSTS.RDY=1)
> >
> > The FreeBSD NVMe driver sequence should be changed to set BME=1 before
> attempting to enable controller.
> >
> >
> > Regards,
> > Kinjal Patel
> > _______________________________________________
> > [hidden email]<mailto:[hidden email]> mailing
> list
> > https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> > To unsubscribe, send any mail to "freebsd-drivers-unsubscribe@
> freebsd.org<mailto:[hidden email]>"
>
> >
> > _______________________________________________
> > [hidden email] mailing list
> > https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> > To unsubscribe, send any mail to "freebsd-drivers-unsubscribe@
> freebsd.org"
>
>
>
_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

RE: NVMe driver init sequence

Kinjal Patel
I have observed this behavior in our lab on 10.3-STABLE. So I have reported against it.
If you can bring in the change for 10.4R that will be great (lot of users stick to 10 based releases for their development). And in case, if customer faces this issue, I can point to this commit ☺.

Regards,
Kinjal Patel

From: [hidden email] [mailto:[hidden email]] On Behalf Of Warner Losh
Sent: Friday, August 25, 2017 9:00 AM
To: Kinjal Patel
Cc: Jim Harris; [hidden email]
Subject: Re: NVMe driver init sequence

BTW, I just noticed you reported this against 10.3R. Is there some reason (perhaps a customer with an issue) that you reported it there? I'm trying to decide if I need to MFC this change to 11.x and 10.x (the latter has some time urgency due to upcoming 10.4R release).

Warner

On Fri, Aug 25, 2017 at 9:54 AM, Kinjal Patel <[hidden email]<mailto:[hidden email]>> wrote:
Great!
Thanks Warner.


Regards,
Kinjal Patel

From: [hidden email]<mailto:[hidden email]> [mailto:[hidden email]<mailto:[hidden email]>] On Behalf Of Warner Losh
Sent: Thursday, August 24, 2017 8:17 PM
To: Jim Harris
Cc: Kinjal Patel; [hidden email]<mailto:[hidden email]>

Subject: Re: NVMe driver init sequence

Greetings,

I've just committed r322872 which implements this, with a slightly tweaked comment.

Warner

On Fri, Aug 18, 2017 at 4:09 PM, Jim Harris <[hidden email]<mailto:[hidden email]>> wrote:
On Fri, Aug 18, 2017 at 2:04 PM, Kinjal Patel
<[hidden email]<mailto:[hidden email]>> wrote:

> Created a bug #22166 (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221616)
> Updated the description and patch to it.
>
> Regards,
> Kinjal Patel
>
> From: Kinjal Patel
> Sent: Friday, August 18, 2017 1:51 PM
> To: 'Warner Losh'
> Cc: [hidden email]<mailto:[hidden email]>
> Subject: RE: NVMe driver init sequence
>
> Here is a the change I propose. This will make the init sequence right.

Looks good to me.

>
> Index: sys/dev/nvme/nvme.c
> ===================================================================
> --- sys/dev/nvme/nvme.c (revision 322672)
> +++ sys/dev/nvme/nvme.c (working copy)
> @@ -253,6 +253,9 @@
>                 return (status);
>         }
>
> +    /* make device bus-master before attempting to enable controller */
> +    pci_enable_busmaster(dev);
> +
>         /*
>          * Reset controller twice to ensure we do a transition from cc.en==1
>          *  to cc.en==0.  This is because we don't really know what status
> @@ -270,8 +273,6 @@
>                 return (status);
>         }
>
> -       pci_enable_busmaster(dev);
> -
>         ctrlr->config_hook.ich_func = nvme_ctrlr_start_config_hook;
>         ctrlr->config_hook.ich_arg = ctrlr;
>
>
>
> regards,
> Kinjal Patel
>
> From: [hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>> [mailto:[hidden email]<mailto:[hidden email]>] On Behalf Of Warner Losh
> Sent: Thursday, August 17, 2017 3:10 PM
> To: Kinjal Patel
> Cc: [hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>>
> Subject: Re: NVMe driver init sequence
>
> This sounds right to me....  Most BIOSes seem to enable BME=1.
>
> Warner
>
> On Thu, Aug 17, 2017 at 2:45 PM, Kinjal Patel <[hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>>> wrote:
> Hi,
>
> I have a query on NVMe driver init sequence in FreeBSD 10.3(stable).
>
> The driver init sequence is,
>
> 1)      Enable controller (CC.EN=1)
>
> 2)      Wait for controller ready (CSTS.RDY=1)
>
> 3)      Set PCI bus master enable (BME=1)
>
> As per NVMe spec, when NVMe controller becomes ready it has to process command.
>
> "Enable (EN): When set to '1', then the controller shall process commands based on Submission Queue Tail doorbell writes"
>
> And per PCI Express spec when BME is not set, the PCI Express Function is not allowed to issue any Memory or I/O requests.
> "Bus Master Enable - Controls the ability of a PCI Express Endpoint to issue Memory95 and I/O Read/Write Requests, and
> the ability of a Root or Switch Port to forward Memory and I/O Read/Write Requests in the Upstream direction"
>
> Enabling controller before setting BME=1 is violation of spec, as controller has to accept commands but BME is prerequisite for that.
>
> The Linux device driver init sequence is,
>
> 1)     Set PCI bus master enable (BME=1)
>
> 2)     Enable Controller (CC.EN=1)
>
> 3)     Wait for controller ready (CSTS.RDY=1)
>
> The FreeBSD NVMe driver sequence should be changed to set BME=1 before attempting to enable controller.
>
>
> Regards,
> Kinjal Patel
> _______________________________________________
> [hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>> mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> To unsubscribe, send any mail to "[hidden email]<mailto:[hidden email]><mailto:[hidden email]<mailto:[hidden email]>>"
>
> _______________________________________________
> [hidden email]<mailto:[hidden email]> mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
> To unsubscribe, send any mail to "[hidden email]<mailto:[hidden email]>"


_______________________________________________
[hidden email] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-drivers
To unsubscribe, send any mail to "[hidden email]"