public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christoph Hellwig <hch@lst.de>,
	linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
	Jens Axboe <axboe@fb.com>, Keith Busch <kbusch@kernel.org>,
	Paul Pawlowski <paul@mrarm.io>
Subject: Re: [PATCH 2/3] nvme: Retrieve the required IO queue entry size from the controller
Date: Tue, 16 Jul 2019 11:33:01 +0200	[thread overview]
Message-ID: <20190716093301.GA32562@lst.de> (raw)
In-Reply-To: <ad18ff8d004225e102076f8e1fb617916617f337.camel@kernel.crashing.org>

On Tue, Jul 16, 2019 at 04:21:14PM +1000, Benjamin Herrenschmidt wrote:
> > Actually, this doesn't work on a "real" nvme controller, to change CC
> > values the controller needs to be disabled.
> 
> Not really. The specs says that MPS, AMD and CSS need to be set before
> enabling, but IOCQES and IOSQES can be modified later as long as there
> is no IO queue created yet.

I guess that is true based on the spec.

> This is necessary otherwise there's a chicken and egg problem. You need
> the admin queue to do the controller id in order to get the sizes and
> for that you need the controller to be enabled.
> 
> Note: This is not a huge issue anyway since I only update the register
> if the required size isn't 6 which is probably never going to be the
> case on non-Apple HW.

Yes, but the whole point of making you go down the route is so that
we can share the code with eventual real nvme controllers that can
support a larger SQE size.

> >   So back to the version
> > you circulated to me in private mail that just sets q->sqes and has a
> > comment that this is magic for The Apple controller.  If/when we get
> > standardized large SQE support we'll need to discover that earlier or
> > do a disable/enable dance.  Sorry for misleading you down this road and
> > creating the extra work.  
> 
> I think it's still ok, let me know...

Ok, let's go with this series then unless the other maintainers have
objections.

I'm still not sure if we want to queue this up for 5.3 (new hardware
enablement) or wait a bit, though.

  reply	other threads:[~2019-07-16  9:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-16  0:46 [PATCH 1/3] nvme: Pass the queue to SQ_SIZE/CQ_SIZE macros Benjamin Herrenschmidt
2019-07-16  0:46 ` [PATCH 2/3] nvme: Retrieve the required IO queue entry size from the controller Benjamin Herrenschmidt
2019-07-16  6:04   ` Christoph Hellwig
2019-07-16  6:21     ` Benjamin Herrenschmidt
2019-07-16  9:33       ` Christoph Hellwig [this message]
2019-07-16 10:58         ` Benjamin Herrenschmidt
2019-07-16 12:05           ` Christoph Hellwig
2019-07-16 12:17             ` Benjamin Herrenschmidt
2019-07-16 12:25               ` Christoph Hellwig
2019-07-16  0:46 ` [PATCH 3/3] nvme: Add support for Apple 2018+ models Benjamin Herrenschmidt
2019-07-16  6:06   ` Christoph Hellwig
2019-07-16  6:22     ` Benjamin Herrenschmidt
2019-07-16  0:49 ` [PATCH 1/3] nvme: Pass the queue to SQ_SIZE/CQ_SIZE macros Benjamin Herrenschmidt
2019-07-16  5:59 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190716093301.GA32562@lst.de \
    --to=hch@lst.de \
    --cc=axboe@fb.com \
    --cc=benh@kernel.crashing.org \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=paul@mrarm.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox