| Message ID | 20201027135547.374946-1-philmd@redhat.com |
|---|---|
| Headers | show |
| Series | block/nvme: Fix Aarch64 host | expand |
On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote: > From the specification chapter 3.1.8 "AQA - Admin Queue Attributes" > the Admin Submission Queue Size field is a 0’s based value: > > Admin Submission Queue Size (ASQS): > > Defines the size of the Admin Submission Queue in entries. > Enabling a controller while this field is cleared to 00h > produces undefined results. The minimum size of the Admin > Submission Queue is two entries. The maximum size of the > Admin Submission Queue is 4096 entries. > This is a 0’s based value. > > This bug has never been hit because the device initialization > uses a single command synchronously :) > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > block/nvme.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/block/nvme.c b/block/nvme.c > index 2dfcf8c41d7..d5df30ec074 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -789,9 +789,9 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, > goto out; > } > s->queue_count = 1; > - QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000); > - regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << AQA_ACQS_SHIFT) | > - (NVME_QUEUE_SIZE << AQA_ASQS_SHIFT)); > + QEMU_BUILD_BUG_ON((NVME_QUEUE_SIZE - 1) & 0xF000); > + regs->aqa = cpu_to_le32(((NVME_QUEUE_SIZE - 1) << AQA_ACQS_SHIFT) | > + ((NVME_QUEUE_SIZE - 1) << AQA_ASQS_SHIFT)); > regs->asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova); > regs->acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova); > >
On Tue, Oct 27, 2020 at 02:55:37PM +0100, Philippe Mathieu-Daudé wrote: > From the specification chapter 3.1.8 "AQA - Admin Queue Attributes" > the Admin Submission Queue Size field is a 0’s based value: > > Admin Submission Queue Size (ASQS): > > Defines the size of the Admin Submission Queue in entries. > Enabling a controller while this field is cleared to 00h > produces undefined results. The minimum size of the Admin > Submission Queue is two entries. The maximum size of the > Admin Submission Queue is 4096 entries. > This is a 0’s based value. > > This bug has never been hit because the device initialization > uses a single command synchronously :) > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block/nvme.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Hi Philippe, On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote: > Add a bit of tracing, clean around to finally fix few bugs. > In particular, restore NVMe on Aarch64 host. > > Eric Auger (4): > block/nvme: Change size and alignment of IDENTIFY response buffer > block/nvme: Change size and alignment of queue > block/nvme: Change size and alignment of prp_list_pages > block/nvme: Align iov's va and size on host page size> > Philippe Mathieu-Daudé (21): > MAINTAINERS: Cover 'block/nvme.h' file > block/nvme: Use hex format to display offset in trace events > block/nvme: Report warning with warn_report() > block/nvme: Trace controller capabilities > block/nvme: Trace nvme_poll_queue() per queue > block/nvme: Improve nvme_free_req_queue_wait() trace information > block/nvme: Trace queue pair creation/deletion > block/nvme: Simplify device reset > block/nvme: Move definitions before structure declarations > block/nvme: Use unsigned integer for queue counter/size > block/nvme: Make nvme_identify() return boolean indicating error > block/nvme: Make nvme_init_queue() return boolean indicating error > block/nvme: Introduce Completion Queue definitions > block/nvme: Use definitions instead of magic values in add_io_queue() > block/nvme: Correctly initialize Admin Queue Attributes > block/nvme: Simplify ADMIN queue access > block/nvme: Simplify nvme_cmd_sync() > block/nvme: Pass AioContext argument to nvme_add_io_queue() > block/nvme: Set request_alignment at initialization > block/nvme: Correct minimum device page size > block/nvme: Fix use of write-only doorbells page on Aarch64 arch > > include/block/nvme.h | 17 ++-- > block/nvme.c | 208 ++++++++++++++++++++++++------------------- > MAINTAINERS | 2 + > block/trace-events | 30 ++++--- > 4 files changed, 148 insertions(+), 109 deletions(-) > I have tested the series on ARM with both 4kB and 64kB pages and it works for me. Feel free to add: Tested-by: Eric Auger <eric.auger@redhat.com> Thanks Eric
On 10/28/20 7:10 PM, Auger Eric wrote: > Hi Philippe, > > On 10/27/20 2:55 PM, Philippe Mathieu-Daudé wrote: >> Add a bit of tracing, clean around to finally fix few bugs. >> In particular, restore NVMe on Aarch64 host. >> >> Eric Auger (4): >> block/nvme: Change size and alignment of IDENTIFY response buffer >> block/nvme: Change size and alignment of queue >> block/nvme: Change size and alignment of prp_list_pages >> block/nvme: Align iov's va and size on host page size> >> Philippe Mathieu-Daudé (21): >> MAINTAINERS: Cover 'block/nvme.h' file >> block/nvme: Use hex format to display offset in trace events >> block/nvme: Report warning with warn_report() >> block/nvme: Trace controller capabilities >> block/nvme: Trace nvme_poll_queue() per queue >> block/nvme: Improve nvme_free_req_queue_wait() trace information >> block/nvme: Trace queue pair creation/deletion >> block/nvme: Simplify device reset >> block/nvme: Move definitions before structure declarations >> block/nvme: Use unsigned integer for queue counter/size >> block/nvme: Make nvme_identify() return boolean indicating error >> block/nvme: Make nvme_init_queue() return boolean indicating error >> block/nvme: Introduce Completion Queue definitions >> block/nvme: Use definitions instead of magic values in add_io_queue() >> block/nvme: Correctly initialize Admin Queue Attributes >> block/nvme: Simplify ADMIN queue access >> block/nvme: Simplify nvme_cmd_sync() >> block/nvme: Pass AioContext argument to nvme_add_io_queue() >> block/nvme: Set request_alignment at initialization >> block/nvme: Correct minimum device page size >> block/nvme: Fix use of write-only doorbells page on Aarch64 arch >> >> include/block/nvme.h | 17 ++-- >> block/nvme.c | 208 ++++++++++++++++++++++++------------------- >> MAINTAINERS | 2 + >> block/trace-events | 30 ++++--- >> 4 files changed, 148 insertions(+), 109 deletions(-) >> > > I have tested the series on ARM with both 4kB and 64kB pages and it > works for me. > > Feel free to add: > Tested-by: Eric Auger <eric.auger@redhat.com> Thanks! Phil.