| Message ID | 20201026105504.4023620-5-philmd@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | util/vfio-helpers: Allow using multiple MSIX IRQs | expand |
Hi, On 10/26/20 11:54 AM, Philippe Mathieu-Daudé wrote: > Controllers have different capabilities and report them in the > CAP register. We are particularly interested by the page size > limits. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block/nvme.c | 10 ++++++++++ > block/trace-events | 1 + > 2 files changed, 11 insertions(+) > > diff --git a/block/nvme.c b/block/nvme.c > index 5abd7257cac..3b6d3972ec2 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -720,6 +720,16 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > * Initialization". */ > > cap = le64_to_cpu(regs->cap); > + trace_nvme_controller_capability("Maximum Queue Entries Supported", > + NVME_CAP_MQES(cap)); > + trace_nvme_controller_capability("Contiguous Queues Required", > + NVME_CAP_CQR(cap)); > + trace_nvme_controller_capability("Subsystem Reset Supported", nit extra space > + NVME_CAP_NSSRS(cap)); > + trace_nvme_controller_capability("Memory Page Size Minimum", > + NVME_CAP_MPSMIN(cap)); > + trace_nvme_controller_capability("Memory Page Size Maximum", > + NVME_CAP_MPSMAX(cap)); Don't you want to trace the true MPSMIN/MAX (I mean with the shift)? I would personally group those traces into a single one and avoid this generic trace format but well ... Thanks Eric > 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 0e351c3fa3d..3f141dc6801 100644 > --- a/block/trace-events > +++ b/block/trace-events > @@ -134,6 +134,7 @@ 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(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" >
On Mon, Oct 26, 2020 at 11:54:49AM +0100, Philippe Mathieu-Daudé wrote: > Controllers have different capabilities and report them in the > CAP register. We are particularly interested by the page size > limits. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block/nvme.c | 10 ++++++++++ > block/trace-events | 1 + > 2 files changed, 11 insertions(+) > > diff --git a/block/nvme.c b/block/nvme.c > index 5abd7257cac..3b6d3972ec2 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -720,6 +720,16 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > * Initialization". */ > > cap = le64_to_cpu(regs->cap); > + trace_nvme_controller_capability("Maximum Queue Entries Supported", > + NVME_CAP_MQES(cap)); > + trace_nvme_controller_capability("Contiguous Queues Required", > + NVME_CAP_CQR(cap)); > + trace_nvme_controller_capability("Subsystem Reset Supported", > + NVME_CAP_NSSRS(cap)); > + trace_nvme_controller_capability("Memory Page Size Minimum", > + NVME_CAP_MPSMIN(cap)); > + trace_nvme_controller_capability("Memory Page Size Maximum", > + NVME_CAP_MPSMAX(cap)); This works well for printf-style tracing but it can be tedious to strcmp() or parse strings from SystemTap and other tracing scripts. Another way of expressing these trace events is: trace_nvme_controller_capabilities(NVME_CAP_MQES(cap), NVME_CAP_CQR(cap), ...); Or: trace_nvme_controller_capability_mqes(NVME_CAP_MQES(cap)); trace_nvme_controller_capability_cqr(NVME_CAP_CQR(cap)); One view is that tracing is like printf (the style used in this patch). Another view is that tracing produces records that scripts can process (the style I showed). Trace events defined in one style may be hard to use in the other style (i.e. less human-readable vs harder to process in a script). Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/block/nvme.c b/block/nvme.c index 5abd7257cac..3b6d3972ec2 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -720,6 +720,16 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, * Initialization". */ cap = le64_to_cpu(regs->cap); + trace_nvme_controller_capability("Maximum Queue Entries Supported", + NVME_CAP_MQES(cap)); + trace_nvme_controller_capability("Contiguous Queues Required", + NVME_CAP_CQR(cap)); + trace_nvme_controller_capability("Subsystem Reset Supported", + NVME_CAP_NSSRS(cap)); + trace_nvme_controller_capability("Memory Page Size Minimum", + NVME_CAP_MPSMIN(cap)); + trace_nvme_controller_capability("Memory Page Size Maximum", + 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 0e351c3fa3d..3f141dc6801 100644 --- a/block/trace-events +++ b/block/trace-events @@ -134,6 +134,7 @@ 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(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"
Controllers have different capabilities and report them in the CAP register. We are particularly interested by the page size limits. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- block/nvme.c | 10 ++++++++++ block/trace-events | 1 + 2 files changed, 11 insertions(+)