| Message ID | 20201027135547.374946-5-philmd@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | block/nvme: Fix Aarch64 host | expand |
Hi Philippe, On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote: > Controllers have different capabilities and report them in the > CAP register. We are particularly interested by the page size > limits. > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block/nvme.c | 13 +++++++++++++ > block/trace-events | 2 ++ > 2 files changed, 15 insertions(+) > > diff --git a/block/nvme.c b/block/nvme.c > index 6f1d7f9b2a1..361b5772b7a 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -727,6 +727,19 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > * Initialization". */ > > cap = le64_to_cpu(regs->cap); > + trace_nvme_controller_capability_raw(cap); > + trace_nvme_controller_capability("Maximum Queue Entries Supported", > + 1 + NVME_CAP_MQES(cap)); > + trace_nvme_controller_capability("Contiguous Queues Required", > + NVME_CAP_CQR(cap)); I think this should be +1 too (0's based value) > + trace_nvme_controller_capability("Doorbell Stride", > + 2 << (2 + NVME_CAP_DSTRD(cap))); > + trace_nvme_controller_capability("Subsystem Reset Supported", > + NVME_CAP_NSSRS(cap)); > + trace_nvme_controller_capability("Memory Page Size Minimum", > + 1 << (12 + NVME_CAP_MPSMIN(cap))); > + trace_nvme_controller_capability("Memory Page Size Maximum", > + 1 << (12 + NVME_CAP_MPSMAX(cap))); > if (!NVME_CAP_CSS(cap)) { > error_setg(errp, "Device doesn't support NVMe command set"); > ret = -EINVAL; > diff --git a/block/trace-events b/block/trace-events > index 0955c85c783..b90b07b15fa 100644 > --- a/block/trace-events > +++ b/block/trace-events > @@ -134,6 +134,8 @@ qed_aio_write_postfill(void *s, void *acb, uint64_t start, size_t len, uint64_t > qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s %p acb %p ret %d offset %"PRIu64" len %zu" > > # nvme.c > +nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64 > +nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64 > nvme_kick(void *s, int queue) "s %p queue %d" > nvme_dma_flush_queue_wait(void *s) "s %p" > nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x" > Besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric
On 10/28/20 11:20 AM, Auger Eric wrote: > Hi Philippe, > > On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote: >> Controllers have different capabilities and report them in the >> CAP register. We are particularly interested by the page size >> limits. >> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> block/nvme.c | 13 +++++++++++++ >> block/trace-events | 2 ++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/block/nvme.c b/block/nvme.c >> index 6f1d7f9b2a1..361b5772b7a 100644 >> --- a/block/nvme.c >> +++ b/block/nvme.c >> @@ -727,6 +727,19 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, >> * Initialization". */ >> >> cap = le64_to_cpu(regs->cap); >> + trace_nvme_controller_capability_raw(cap); >> + trace_nvme_controller_capability("Maximum Queue Entries Supported", >> + 1 + NVME_CAP_MQES(cap)); >> + trace_nvme_controller_capability("Contiguous Queues Required", >> + NVME_CAP_CQR(cap)); > I think this should be +1 too (0's based value) This is a boolean: Contiguous Queues Required (CQR): This field is set to ‘1’ if the controller requires that I/O Submission Queues and I/O Completion Queues are required to be physically contiguous. This field is cleared to ‘0’ if the controller supports I/O Submission Queues and I/O Completion Queues that are not physically contiguous. If this field is set to ‘1’, then the Physically Contiguous bit (CDW11.PC) in the Create I/O Submission Queue and Create I/O Completion Queue commands shall be set to ‘1’. >> + trace_nvme_controller_capability("Doorbell Stride", >> + 2 << (2 + NVME_CAP_DSTRD(cap))); >> + trace_nvme_controller_capability("Subsystem Reset Supported", >> + NVME_CAP_NSSRS(cap)); >> + trace_nvme_controller_capability("Memory Page Size Minimum", >> + 1 << (12 + NVME_CAP_MPSMIN(cap))); >> + trace_nvme_controller_capability("Memory Page Size Maximum", >> + 1 << (12 + NVME_CAP_MPSMAX(cap))); >> if (!NVME_CAP_CSS(cap)) { >> error_setg(errp, "Device doesn't support NVMe command set"); >> ret = -EINVAL; >> diff --git a/block/trace-events b/block/trace-events >> index 0955c85c783..b90b07b15fa 100644 >> --- a/block/trace-events >> +++ b/block/trace-events >> @@ -134,6 +134,8 @@ qed_aio_write_postfill(void *s, void *acb, uint64_t start, size_t len, uint64_t >> qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s %p acb %p ret %d offset %"PRIu64" len %zu" >> >> # nvme.c >> +nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64 >> +nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64 >> nvme_kick(void *s, int queue) "s %p queue %d" >> nvme_dma_flush_queue_wait(void *s) "s %p" >> nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x" >> > Besides > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Thanks > > Eric >
Hi Philippe, On 10/28/20 11:25 AM, Philippe Mathieu-Daudé wrote: > On 10/28/20 11:20 AM, Auger Eric wrote: >> Hi Philippe, >> >> On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote: >>> Controllers have different capabilities and report them in the >>> CAP register. We are particularly interested by the page size >>> limits. >>> >>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> --- >>> block/nvme.c | 13 +++++++++++++ >>> block/trace-events | 2 ++ >>> 2 files changed, 15 insertions(+) >>> >>> diff --git a/block/nvme.c b/block/nvme.c >>> index 6f1d7f9b2a1..361b5772b7a 100644 >>> --- a/block/nvme.c >>> +++ b/block/nvme.c >>> @@ -727,6 +727,19 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, >>> * Initialization". */ >>> >>> cap = le64_to_cpu(regs->cap); >>> + trace_nvme_controller_capability_raw(cap); >>> + trace_nvme_controller_capability("Maximum Queue Entries Supported", >>> + 1 + NVME_CAP_MQES(cap)); >>> + trace_nvme_controller_capability("Contiguous Queues Required", >>> + NVME_CAP_CQR(cap)); >> I think this should be +1 too (0's based value) > > This is a boolean: > > Contiguous Queues Required (CQR): This field is set to ‘1’ if > the controller requires that I/O Submission Queues and I/O > Completion Queues are required to be physically contiguous. > This field is cleared to ‘0’ if the controller supports I/O > Submission Queues and I/O Completion Queues that are not > physically contiguous. If this field is set to ‘1’, then the > Physically Contiguous bit (CDW11.PC) in the Create I/O Submission > Queue and Create I/O Completion Queue commands shall be set to ‘1’. Oh I mixed with NCQR :-( sorry for the noise Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > >>> + trace_nvme_controller_capability("Doorbell Stride", >>> + 2 << (2 + NVME_CAP_DSTRD(cap))); >>> + trace_nvme_controller_capability("Subsystem Reset Supported", >>> + NVME_CAP_NSSRS(cap)); >>> + trace_nvme_controller_capability("Memory Page Size Minimum", >>> + 1 << (12 + NVME_CAP_MPSMIN(cap))); >>> + trace_nvme_controller_capability("Memory Page Size Maximum", >>> + 1 << (12 + NVME_CAP_MPSMAX(cap))); >>> if (!NVME_CAP_CSS(cap)) { >>> error_setg(errp, "Device doesn't support NVMe command set"); >>> ret = -EINVAL; >>> diff --git a/block/trace-events b/block/trace-events >>> index 0955c85c783..b90b07b15fa 100644 >>> --- a/block/trace-events >>> +++ b/block/trace-events >>> @@ -134,6 +134,8 @@ qed_aio_write_postfill(void *s, void *acb, uint64_t start, size_t len, uint64_t >>> qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s %p acb %p ret %d offset %"PRIu64" len %zu" >>> >>> # nvme.c >>> +nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64 >>> +nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64 >>> nvme_kick(void *s, int queue) "s %p queue %d" >>> nvme_dma_flush_queue_wait(void *s) "s %p" >>> nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x" >>> >> Besides >> Reviewed-by: Eric Auger <eric.auger@redhat.com> >> >> Thanks >> >> Eric >> > >
diff --git a/block/nvme.c b/block/nvme.c index 6f1d7f9b2a1..361b5772b7a 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -727,6 +727,19 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, * Initialization". */ cap = le64_to_cpu(regs->cap); + trace_nvme_controller_capability_raw(cap); + trace_nvme_controller_capability("Maximum Queue Entries Supported", + 1 + NVME_CAP_MQES(cap)); + trace_nvme_controller_capability("Contiguous Queues Required", + NVME_CAP_CQR(cap)); + trace_nvme_controller_capability("Doorbell Stride", + 2 << (2 + NVME_CAP_DSTRD(cap))); + trace_nvme_controller_capability("Subsystem Reset Supported", + NVME_CAP_NSSRS(cap)); + trace_nvme_controller_capability("Memory Page Size Minimum", + 1 << (12 + NVME_CAP_MPSMIN(cap))); + trace_nvme_controller_capability("Memory Page Size Maximum", + 1 << (12 + NVME_CAP_MPSMAX(cap))); if (!NVME_CAP_CSS(cap)) { error_setg(errp, "Device doesn't support NVMe command set"); ret = -EINVAL; diff --git a/block/trace-events b/block/trace-events index 0955c85c783..b90b07b15fa 100644 --- a/block/trace-events +++ b/block/trace-events @@ -134,6 +134,8 @@ qed_aio_write_postfill(void *s, void *acb, uint64_t start, size_t len, uint64_t qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s %p acb %p ret %d offset %"PRIu64" len %zu" # nvme.c +nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64 +nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64 nvme_kick(void *s, int queue) "s %p queue %d" nvme_dma_flush_queue_wait(void *s) "s %p" nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"