[PATCH 0/5] ahci: nvme remap support

[Christoph Hellwig]

On Mon, Oct 24, 2016 at 10:46:29AM -0700, Dan Williams wrote:
> On Mon, Oct 24, 2016 at 8:16 AM, Christoph Hellwig <hch at lst.de> wrote:
> > On Mon, Oct 24, 2016 at 10:46:29AM -0400, Keith Busch wrote:
> >> Amber is aware of this and was supportive in having Intel open the
> >> specs to enable this hardware.
> >
> > Ok, let's get the spec out first.
> 
> The patch contents are it.

Well. that's simply not acceptable.  We will need a theory of
operation for this device to support it, because the patch as-is
simply doesn't make sense.

We'll also need to know where such device might show up, and why
Linux support for it matters.

> > Do we expect to be able to use the
> > AHCI interface and the NVMe interface at the same time?
> 
> Yes, simultaneous access.

Yikes.  I'm really tempted to say this is not acceptable - Intel
really must know better.

> That said, the driver seems to already comprehend instances where the
> device does not support nvme_reset_subsystem() requests. I don't know
> how often those resets need to be issued in practice.

It's not about how often reset are needed (and there are controller
resets in addition to function level resets), but how they are
defined to work.  What state is a controller in after a host initiated
reset, after a PCIe hotplug or even warm plug.  What state is the
PCI device in when the controller is in a failed state?

> > If it's the latter let's keep AHCI entirely out
> > of the game - add the affected PCI IDs to the NVMe driver itself, add
> > a quirk for them and implement the enable sequence inside the NVMe
> > driver.
> 
> The PCI ID of the AHCI device is not uniquely identifiable when in this mode.
> 
> We could flip the arrangement and have the ahci device as the platform
> device, but I'm not sure this makes the nvme reset problem much
> better.  If we allow subsystem resets at all they would still need to
> be coordinated across 2 devices/drivers to reinitialize pci registers.

I think the simple answer is to not support this device.  It's not like
Intel doesn't know the AHCI and NVMe specs and had any reaoson to come
up with a piece of junk like this.