* nvme lightnvm comments
@ 2015-11-20 10:50 Christoph Hellwig
2015-11-20 11:06 ` Matias Bjørling
2015-11-21 11:12 ` Christoph Hellwig
0 siblings, 2 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-11-20 10:50 UTC (permalink / raw)
Hi Matias,
two small comments on the NVME lighnvm support:
1) please don't claim the existing Qemu NVMe PCI ID for lightnvm
support. We might need the actual vendor specific data for
qemu purposes. Please allocate a different PCI ID for a qemu
emulated lightnvm nvme device
2) please build the lightnvm.o object conditionally in the makefile
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-20 10:50 nvme lightnvm comments Christoph Hellwig
@ 2015-11-20 11:06 ` Matias Bjørling
2015-11-20 16:47 ` Christoph Hellwig
2015-11-21 11:12 ` Christoph Hellwig
1 sibling, 1 reply; 12+ messages in thread
From: Matias Bjørling @ 2015-11-20 11:06 UTC (permalink / raw)
Hi Christoph,
On 11/20/2015 11:50 AM, Christoph Hellwig wrote:
> Hi Matias,
>
> two small comments on the NVME lighnvm support:
>
> 1) please don't claim the existing Qemu NVMe PCI ID for lightnvm
> support. We might need the actual vendor specific data for
> qemu purposes. Please allocate a different PCI ID for a qemu
> emulated lightnvm nvme device
I'll look into it.
> 2) please build the lightnvm.o object conditionally in the makefile
Then the functions
nvme_nvm_register
nvme_nvm_unregister
nvme_nvm_register
have to be #ifdef'ed in the pci file. Do you want that instead? It was
on purpose the other way around, so that the nvme_nvm_* functions was
null functions if lightnvm isn't compiled in.
>
> Thanks!
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-20 11:06 ` Matias Bjørling
@ 2015-11-20 16:47 ` Christoph Hellwig
2015-11-20 21:26 ` Matias Bjørling
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2015-11-20 16:47 UTC (permalink / raw)
On Fri, Nov 20, 2015@12:06:36PM +0100, Matias Bj?rling wrote:
> > 2) please build the lightnvm.o object conditionally in the makefile
>
> Then the functions
>
> nvme_nvm_register
> nvme_nvm_unregister
> nvme_nvm_register
>
> have to be #ifdef'ed in the pci file. Do you want that instead? It was on
> purpose the other way around, so that the nvme_nvm_* functions was null
> functions if lightnvm isn't compiled in.
Just keep the stubs as inlines in the header.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-20 16:47 ` Christoph Hellwig
@ 2015-11-20 21:26 ` Matias Bjørling
2015-11-21 8:19 ` Christoph Hellwig
0 siblings, 1 reply; 12+ messages in thread
From: Matias Bjørling @ 2015-11-20 21:26 UTC (permalink / raw)
On 11/20/2015 05:47 PM, Christoph Hellwig wrote:
> On Fri, Nov 20, 2015@12:06:36PM +0100, Matias Bj?rling wrote:
>>> 2) please build the lightnvm.o object conditionally in the makefile
>>
>> Then the functions
>>
>> nvme_nvm_register
>> nvme_nvm_unregister
>> nvme_nvm_register
>>
>> have to be #ifdef'ed in the pci file. Do you want that instead? It was on
>> purpose the other way around, so that the nvme_nvm_* functions was null
>> functions if lightnvm isn't compiled in.
>
> Just keep the stubs as inlines in the header.
>
That'll do.
Do you have any plans for exposing the completion_entry on
req_completion in the driver? I'm interested in the fields ->result and
->rsvd in struct nvme_completion. They contain the completion bits for
each of the PPA entries. Which I'll need higher up in the stack to
perform correct retries and so forth.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-20 21:26 ` Matias Bjørling
@ 2015-11-21 8:19 ` Christoph Hellwig
0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-11-21 8:19 UTC (permalink / raw)
On Fri, Nov 20, 2015@10:26:27PM +0100, Matias Bj?rling wrote:
> Do you have any plans for exposing the completion_entry on req_completion in
> the driver? I'm interested in the fields ->result and ->rsvd in struct
> nvme_completion. They contain the completion bits for each of the PPA
> entries. Which I'll need higher up in the stack to perform correct retries
> and so forth.
->result is available already for passthrough commands. But for Fabrics
I actually need the full CQE as well, and I'm working on that at the
moment. Stay tuned!
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-20 10:50 nvme lightnvm comments Christoph Hellwig
2015-11-20 11:06 ` Matias Bjørling
@ 2015-11-21 11:12 ` Christoph Hellwig
2015-11-22 18:17 ` Matias Bjørling
1 sibling, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2015-11-21 11:12 UTC (permalink / raw)
Btw, another one: it seems like LightNVM devices still claim to
use the NVM I/O set per CAP and CC, which seems a little odd.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-21 11:12 ` Christoph Hellwig
@ 2015-11-22 18:17 ` Matias Bjørling
2015-11-23 15:45 ` Christoph Hellwig
0 siblings, 1 reply; 12+ messages in thread
From: Matias Bjørling @ 2015-11-22 18:17 UTC (permalink / raw)
On 11/21/2015 12:12 PM, Christoph Hellwig wrote:
> Btw, another one: it seems like LightNVM devices still claim to
> use the NVM I/O set per CAP and CC, which seems a little odd.
>
Oh, where have you seen it? Thought I had it changed everywhere.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-22 18:17 ` Matias Bjørling
@ 2015-11-23 15:45 ` Christoph Hellwig
2015-11-23 16:03 ` Keith Busch
2015-11-23 19:01 ` Matias Bjørling
0 siblings, 2 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-11-23 15:45 UTC (permalink / raw)
On Sun, Nov 22, 2015@07:17:39PM +0100, Matias Bj?rling wrote:
> On 11/21/2015 12:12 PM, Christoph Hellwig wrote:
> >Btw, another one: it seems like LightNVM devices still claim to
> >use the NVM I/O set per CAP and CC, which seems a little odd.
> >
>
> Oh, where have you seen it? Thought I had it changed everywhere.
nvme_configure_admin_queue still hardcodes then NVM comman set when
enabling the controller.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-23 15:45 ` Christoph Hellwig
@ 2015-11-23 16:03 ` Keith Busch
2015-11-23 19:53 ` Christoph Hellwig
2015-11-23 19:01 ` Matias Bjørling
1 sibling, 1 reply; 12+ messages in thread
From: Keith Busch @ 2015-11-23 16:03 UTC (permalink / raw)
On Mon, Nov 23, 2015@07:45:57AM -0800, Christoph Hellwig wrote:
> On Sun, Nov 22, 2015@07:17:39PM +0100, Matias Bj?rling wrote:
> > On 11/21/2015 12:12 PM, Christoph Hellwig wrote:
> > >Btw, another one: it seems like LightNVM devices still claim to
> > >use the NVM I/O set per CAP and CC, which seems a little odd.
> > >
> >
> > Oh, where have you seen it? Thought I had it changed everywhere.
>
> nvme_configure_admin_queue still hardcodes then NVM comman set when
> enabling the controller.
We also thought it'd have been cleaner to have a different command set,
but the committee hasn't taken on assigning these reserved bits for any
new purposes yet. LightNVM detection is done with the vendor + device
id at the moment.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-23 15:45 ` Christoph Hellwig
2015-11-23 16:03 ` Keith Busch
@ 2015-11-23 19:01 ` Matias Bjørling
1 sibling, 0 replies; 12+ messages in thread
From: Matias Bjørling @ 2015-11-23 19:01 UTC (permalink / raw)
On 11/23/2015 04:45 PM, Christoph Hellwig wrote:
> On Sun, Nov 22, 2015@07:17:39PM +0100, Matias Bj?rling wrote:
>> On 11/21/2015 12:12 PM, Christoph Hellwig wrote:
>>> Btw, another one: it seems like LightNVM devices still claim to
>>> use the NVM I/O set per CAP and CC, which seems a little odd.
>>>
>>
>> Oh, where have you seen it? Thought I had it changed everywhere.
>
> nvme_configure_admin_queue still hardcodes then NVM comman set when
> enabling the controller.
>
I might be wrong. That is the normal nvme command set. It isn't the
lightnvm command set. In the previous implementation, it was called
NVME_CC_CSS_LIGHTNVM.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-23 16:03 ` Keith Busch
@ 2015-11-23 19:53 ` Christoph Hellwig
2015-11-23 20:09 ` Matias Bjørling
0 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2015-11-23 19:53 UTC (permalink / raw)
On Mon, Nov 23, 2015@04:03:20PM +0000, Keith Busch wrote:
> We also thought it'd have been cleaner to have a different command set,
> but the committee hasn't taken on assigning these reserved bits for any
> new purposes yet. LightNVM detection is done with the vendor + device
> id at the moment.
I saw the vendor + device, but even with that I'd rather see a reserved
value in the command set in addition. Claiming that we're NVM if we're
not is a bad idea.
While we're at it - the NVMe driver really should check for these
reserved bits and reject a PCIe device that doesn't claim to implement
the NVM command set.
^ permalink raw reply [flat|nested] 12+ messages in thread
* nvme lightnvm comments
2015-11-23 19:53 ` Christoph Hellwig
@ 2015-11-23 20:09 ` Matias Bjørling
0 siblings, 0 replies; 12+ messages in thread
From: Matias Bjørling @ 2015-11-23 20:09 UTC (permalink / raw)
On 11/23/2015 08:53 PM, Christoph Hellwig wrote:
> On Mon, Nov 23, 2015@04:03:20PM +0000, Keith Busch wrote:
>> We also thought it'd have been cleaner to have a different command set,
>> but the committee hasn't taken on assigning these reserved bits for any
>> new purposes yet. LightNVM detection is done with the vendor + device
>> id at the moment.
>
> I saw the vendor + device, but even with that I'd rather see a reserved
> value in the command set in addition. Claiming that we're NVM if we're
> not is a bad idea.
>
Well, I would too. However, we can't add a new mode without breaking
compatibility at some other point in time. We're working on the
standardization process.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-11-23 20:09 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-20 10:50 nvme lightnvm comments Christoph Hellwig
2015-11-20 11:06 ` Matias Bjørling
2015-11-20 16:47 ` Christoph Hellwig
2015-11-20 21:26 ` Matias Bjørling
2015-11-21 8:19 ` Christoph Hellwig
2015-11-21 11:12 ` Christoph Hellwig
2015-11-22 18:17 ` Matias Bjørling
2015-11-23 15:45 ` Christoph Hellwig
2015-11-23 16:03 ` Keith Busch
2015-11-23 19:53 ` Christoph Hellwig
2015-11-23 20:09 ` Matias Bjørling
2015-11-23 19:01 ` Matias Bjørling
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.